diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index 2da41869c..ebf71d1f7 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -1,119 +1,52 @@ import json +from flask import current_app from notifications_utils.statsd_decorators import statsd - -from app import ( - db, - DATETIME_FORMAT, - notify_celery, - encryption -) -from app.dao.notifications_dao import ( - get_notification_by_id, -) - -from app.dao.service_callback_api_dao import get_service_callback_api_for_service from requests import ( HTTPError, request, RequestException ) -from flask import current_app + +from app import ( + notify_celery, + encryption +) from app.config import QueueNames @notify_celery.task(bind=True, name="send-delivery-status", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") def send_delivery_status_to_service(self, notification_id, - encrypted_status_update=None + encrypted_status_update ): - if not encrypted_status_update: - process_update_with_notification_id(self, notification_id=notification_id) - else: - try: - status_update = encryption.decrypt(encrypted_status_update) - - data = { - "id": str(notification_id), - "reference": status_update['notification_client_reference'], - "to": status_update['notification_to'], - "status": status_update['notification_status'], - "created_at": status_update['notification_created_at'], - "completed_at": status_update['notification_updated_at'], - "sent_at": status_update['notification_sent_at'], - "notification_type": status_update['notification_type'] - } - - response = request( - method="POST", - url=status_update['service_callback_api_url'], - data=json.dumps(data), - headers={ - 'Content-Type': 'application/json', - 'Authorization': 'Bearer {}'.format(status_update['service_callback_api_bearer_token']) - }, - timeout=60 - ) - current_app.logger.info('send_delivery_status_to_service sending {} to {}, response {}'.format( - notification_id, - status_update['service_callback_api_url'], - response.status_code - )) - response.raise_for_status() - except RequestException as e: - current_app.logger.warning( - "send_delivery_status_to_service request failed for notification_id: {} and url: {}. exc: {}".format( - notification_id, - status_update['service_callback_api_url'], - e - ) - ) - if not isinstance(e, HTTPError) or e.response.status_code >= 500: - try: - self.retry(queue=QueueNames.RETRY) - except self.MaxRetriesExceededError: - current_app.logger.exception( - """Retry: send_delivery_status_to_service has retried the max num of times - for notification: {}""".format(notification_id) - ) - - -def process_update_with_notification_id(self, notification_id): - retry = False try: - notification = get_notification_by_id(notification_id) - service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) - if not service_callback_api: - # No delivery receipt API info set - return - - # Release DB connection before performing an external HTTP request - db.session.close() + status_update = encryption.decrypt(encrypted_status_update) data = { "id": str(notification_id), - "reference": str(notification.client_reference), - "to": notification.to, - "status": notification.status, - "created_at": notification.created_at.strftime(DATETIME_FORMAT), - "completed_at": notification.updated_at.strftime(DATETIME_FORMAT), - "sent_at": notification.sent_at.strftime(DATETIME_FORMAT), - "notification_type": notification.notification_type + "reference": status_update['notification_client_reference'], + "to": status_update['notification_to'], + "status": status_update['notification_status'], + "created_at": status_update['notification_created_at'], + "completed_at": status_update['notification_updated_at'], + "sent_at": status_update['notification_sent_at'], + "notification_type": status_update['notification_type'] } response = request( method="POST", - url=service_callback_api.url, + url=status_update['service_callback_api_url'], data=json.dumps(data), headers={ 'Content-Type': 'application/json', - 'Authorization': 'Bearer {}'.format(service_callback_api.bearer_token) + 'Authorization': 'Bearer {}'.format(status_update['service_callback_api_bearer_token']) }, timeout=60 ) current_app.logger.info('send_delivery_status_to_service sending {} to {}, response {}'.format( notification_id, - service_callback_api.url, + status_update['service_callback_api_url'], response.status_code )) response.raise_for_status() @@ -121,26 +54,18 @@ def process_update_with_notification_id(self, notification_id): current_app.logger.warning( "send_delivery_status_to_service request failed for notification_id: {} and url: {}. exc: {}".format( notification_id, - service_callback_api.url, + status_update['service_callback_api_url'], e ) ) if not isinstance(e, HTTPError) or e.response.status_code >= 500: - retry = True - except Exception as e: - current_app.logger.exception( - 'Unhandled exception when sending callback for notification {}'.format(notification_id) - ) - retry = True - - if retry: - try: - self.retry(queue=QueueNames.RETRY) - except self.MaxRetriesExceededError: - current_app.logger.exception( - """Retry: send_delivery_status_to_service has retried the max num of times - for notification: {}""".format(notification_id) - ) + try: + self.retry(queue=QueueNames.RETRY) + except self.MaxRetriesExceededError: + current_app.logger.exception( + """Retry: send_delivery_status_to_service has retried the max num of times + for notification: {}""".format(notification_id) + ) def create_encrypted_callback_data(notification, service_callback_api): diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index 3816d65ae..4fbc858b1 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -1,22 +1,17 @@ -import uuid import json from datetime import datetime -from requests import RequestException import pytest import requests_mock -from sqlalchemy.exc import SQLAlchemyError from app import (DATETIME_FORMAT, encryption) - +from app.celery.service_callback_tasks import send_delivery_status_to_service from tests.app.db import ( create_notification, create_service_callback_api, create_service, create_template ) -from app.celery.service_callback_tasks import send_delivery_status_to_service -from app.config import QueueNames @pytest.mark.parametrize("notification_type", @@ -157,148 +152,3 @@ def _set_up_encrypted_data(callback_api, notification): } encrypted_status_update = encryption.encrypt(data) return encrypted_status_update - - -# We are updating the task to take everything it needs so that there are no db calls. -# The following tests will be deleted once that is complete. -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) -def test_send_delivery_status_to_service_post_https_request_to_service( - notify_db_session, notification_type): - callback_api, template = _set_up_test_data(notification_type) - datestr = datetime(2017, 6, 20) - - notification = create_notification(template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status='sent' - ) - - with requests_mock.Mocker() as request_mock: - request_mock.post(callback_api.url, - json={}, - status_code=200) - send_delivery_status_to_service(notification.id) - - mock_data = { - "id": str(notification.id), - "reference": str(notification.client_reference), - "to": notification.to, - "status": notification.status, - "created_at": datestr.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request - "completed_at": datestr.strftime(DATETIME_FORMAT), # the last time the status was updated - "sent_at": datestr.strftime(DATETIME_FORMAT), # the time the email was sent - "notification_type": notification_type - } - - assert request_mock.call_count == 1 - assert request_mock.request_history[0].url == callback_api.url - assert request_mock.request_history[0].method == 'POST' - assert request_mock.request_history[0].text == json.dumps(mock_data) - assert request_mock.request_history[0].headers["Content-type"] == "application/json" - assert request_mock.request_history[0].headers["Authorization"] == "Bearer {}".format(callback_api.bearer_token) - - -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) -def test_send_delivery_status_to_service_does_not_sent_request_when_service_callback_api_does_not_exist( - notify_db_session, mocker, notification_type): - service = create_service(restricted=True) - template = create_template(service=service, template_type=notification_type, subject='Hello') - datestr = datetime(2017, 6, 20) - - notification = create_notification(template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status='sent' - ) - mocked = mocker.patch("requests.request") - send_delivery_status_to_service(notification.id) - - assert mocked.call_count == 0 - - -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) -def test_send_delivery_status_to_service_retries_if_request_returns_500(notify_db_session, - mocker, - notification_type): - callback_api, template = _set_up_test_data(notification_type) - datestr = datetime(2017, 6, 20) - notification = create_notification(template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status='sent' - ) - mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') - with requests_mock.Mocker() as request_mock: - request_mock.post(callback_api.url, - json={}, - status_code=500) - send_delivery_status_to_service(notification.id) - - assert mocked.call_count == 1 - assert mocked.call_args[1]['queue'] == 'retry-tasks' - - -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) -def test_send_delivery_status_to_service_retries_if_request_throws_unknown(notify_db_session, - mocker, - notification_type): - - callback_api, template = _set_up_test_data(notification_type) - datestr = datetime(2017, 6, 20) - notification = create_notification(template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status='sent' - ) - - mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') - mocker.patch("app.celery.tasks.request", side_effect=RequestException()) - - send_delivery_status_to_service(notification.id) - - assert mocked.call_count == 1 - assert mocked.call_args[1]['queue'] == 'retry-tasks' - - -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) -def test_send_delivery_status_to_service_does_not_retries_if_request_returns_404( - notify_db_session, - mocker, - notification_type -): - callback_api, template = _set_up_test_data(notification_type) - datestr = datetime(2017, 6, 20) - notification = create_notification(template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status='sent' - ) - mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') - with requests_mock.Mocker() as request_mock: - request_mock.post(callback_api.url, - json={}, - status_code=404) - send_delivery_status_to_service(notification.id) - - assert mocked.call_count == 0 - - -def test_send_delivery_status_to_service_retries_if_database_error(client, mocker): - notification_id = uuid.uuid4() - db_call = mocker.patch('app.celery.service_callback_tasks.get_notification_by_id', side_effect=SQLAlchemyError) - retry = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') - - send_delivery_status_to_service(notification_id) - - db_call.assert_called_once_with(notification_id) - retry.assert_called_once_with(queue=QueueNames.RETRY)