From 062084dcc483977c30e094a8b8cf62231d8881a3 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Thu, 3 Aug 2017 18:05:42 +0100
Subject: [PATCH 01/25] Allow for both an endpoint callback and SQS consumption

---
 app/celery/tasks.py                             | 7 +++++++
 app/notifications/notifications_ses_callback.py | 8 +++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index bece59f7c..ec3cf0a7e 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -60,6 +60,7 @@ from app.models import (
     SMS_TYPE,
 )
 from app.notifications.process_notifications import persist_notification
+from app.notifications.notifications_ses_callback import process_ses_response
 from app.service.utils import service_allowed_to_send_to
 from app.statsd_decorators import statsd
 from notifications_utils.s3 import s3upload
@@ -530,3 +531,9 @@ def process_incomplete_job(job_id):
             process_row(row_number, recipient, personalisation, template, job, job.service)
 
     job_complete(job, job.service, template, resumed=True)
+
+
+@notify_celery.task(bind=True, name='process-ses-result')
+@statsd(namespace="tasks")
+def process_ses_results(self, response):
+    process_ses_response(response)
diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index b1d0ba589..101b6bbe1 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -27,11 +27,13 @@ register_errors(ses_callback_blueprint)
 
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
-def process_ses_response():
+def sns_callback_handler():
+    process_ses_response(json.loads(request.data))
+
+
+def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        ses_request = json.loads(request.data)
-
         subscribed_topic = autoconfirm_subscription(ses_request)
         if subscribed_topic:
             current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))

From 7611d473b40c526ecf160fb988fb2e63a5b2fb81 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 12:16:25 +0100
Subject: [PATCH 02/25] We no longer have guaranteed access to a request
 context

---
 .../notifications_ses_callback.py             | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 101b6bbe1..62af94317 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -28,7 +28,11 @@ register_errors(ses_callback_blueprint)
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
 def sns_callback_handler():
-    process_ses_response(json.loads(request.data))
+    errors, status, kwargs = process_ses_response(json.loads(request.data))
+    if errors:
+        raise InvalidRequest(errors, status)
+
+    return jsonify(**kwargs), status
 
 
 def process_ses_response(ses_request):
@@ -37,18 +41,16 @@ def process_ses_response(ses_request):
         subscribed_topic = autoconfirm_subscription(ses_request)
         if subscribed_topic:
             current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
+            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
 
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors, 400, {}
 
         ses_message = json.loads(ses_request['Message'])
         errors = validate_callback_data(data=ses_message, fields=['notificationType'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors, 400, {}
 
         notification_type = ses_message['notificationType']
         if notification_type == 'Bounce':
@@ -60,7 +62,7 @@ def process_ses_response(ses_request):
             aws_response_dict = get_aws_responses(notification_type)
         except KeyError:
             error = "{} callback failed: status {} not found".format(client_name, notification_type)
-            raise InvalidRequest(error, status_code=400)
+            return error, 400, {}
 
         notification_status = aws_response_dict['notification_status']
 
@@ -73,7 +75,7 @@ def process_ses_response(ses_request):
             if not notification:
                 error = "SES callback failed: notification either not found or already updated " \
                         "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                raise InvalidRequest(error, status_code=404)
+                return error, 404, {}
 
             if not aws_response_dict['success']:
                 current_app.logger.info(
@@ -98,14 +100,12 @@ def process_ses_response(ses_request):
 
             create_outcome_notification_statistic_tasks(notification)
 
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
+            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
 
         except KeyError:
-            message = "SES callback failed: messageId missing"
-            raise InvalidRequest(message, status_code=400)
+            error = "SES callback failed: messageId missing"
+            return error, 400, {}
 
-    except ValueError as ex:
+    except ValueError:
         error = "{} callback failed: invalid json".format(client_name)
-        raise InvalidRequest(error, status_code=400)
+        return error, 400, {}

From 61b665c908fb68d6f37f93ccdc5089a9aa7fbe86 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 13:50:25 +0100
Subject: [PATCH 03/25] Update tests

---
 .../notifications_ses_callback.py             |   6 -
 .../test_notifications_ses_callback.py        | 179 ++++--------------
 2 files changed, 34 insertions(+), 151 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 62af94317..6d3c2807e 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -15,7 +15,6 @@ from app.dao import (
 )
 from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks
 from app.notifications.process_client_response import validate_callback_data
-from app.notifications.utils import autoconfirm_subscription
 
 ses_callback_blueprint = Blueprint('notifications_ses_callback', __name__)
 
@@ -38,11 +37,6 @@ def sns_callback_handler():
 def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        subscribed_topic = autoconfirm_subscription(ses_request)
-        if subscribed_topic:
-            current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
-            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
-
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
             return errors, 400, {}
diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 0a02b5d58..de2241f69 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -3,11 +3,10 @@ from unittest.mock import call
 
 from flask import json
 from freezegun import freeze_time
-from requests import HTTPError
-import pytest
 
 from app import statsd_client
 from app.dao.notifications_dao import get_notification_by_id
+from app.notifications.notifications_ses_callback import process_ses_response
 from tests.app.conftest import sample_notification as create_sample_notification
 
 
@@ -24,68 +23,9 @@ def test_ses_callback_should_fail_if_invalid_json(client, mocker):
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
-    response = client.post(
-        path='/notifications/email/ses',
-        data="nonsense",
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: invalid json'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_should_autoconfirm_subscriptions(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    endpoint = json.loads(ses_confirmation_callback())['SubscribeURL']
-    rmock.request(
-        "GET",
-        endpoint,
-        json={"status": "success"},
-        status_code=200)
-
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_confirmation_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-
-    assert rmock.called
-    assert rmock.request_history[0].url == endpoint
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_autoconfirm_raises_exception_if_not_200(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    endpoint = json.loads(ses_confirmation_callback())['SubscribeURL']
-    rmock.request(
-        "GET",
-        endpoint,
-        json={"status": "not allowed"},
-        status_code=405)
-
-    with pytest.raises(HTTPError) as exc:
-        client.post(
-            path='/notifications/email/ses',
-            data=ses_confirmation_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-
-    assert rmock.called
-    assert rmock.request_history[0].url == endpoint
-    assert exc.value.response.status_code == 405
+    errors, status, _ = process_ses_response('nonsense')
+    assert status == 400
+    assert errors == 'SES callback failed: invalid json'
     stats_mock.assert_not_called()
 
 
@@ -94,15 +34,9 @@ def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_invalid_notification_type_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: status Unknown not found'
+    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
+    assert status == 400
+    assert errors == 'SES callback failed: status Unknown not found'
     stats_mock.assert_not_called()
 
 
@@ -111,15 +45,9 @@ def test_ses_callback_should_fail_if_missing_message_id(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_missing_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: messageId missing'
+    errors, status, _ = process_ses_response(json.loads(ses_missing_notification_id_callback()))
+    assert status == 400
+    assert errors == 'SES callback failed: messageId missing'
     stats_mock.assert_not_called()
 
 
@@ -128,15 +56,9 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_invalid_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
+    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
+    assert status == 404
+    assert errors == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
     stats_mock.assert_not_called()
 
 
@@ -164,15 +86,10 @@ def test_ses_callback_should_update_notification_status(
 
         assert get_notification_by_id(notification.id).status == 'sending'
 
-        response = client.post(
-            path='/notifications/email/ses',
-            data=ses_notification_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-        json_resp = json.loads(response.get_data(as_text=True))
-        assert response.status_code == 200
-        assert json_resp['result'] == 'success'
-        assert json_resp['message'] == 'SES callback succeeded'
+        _, status, res = process_ses_response(json.loads(ses_notification_callback()))
+        assert status == 200
+        assert res['result'] == 'success'
+        assert res['message'] == 'SES callback succeeded'
         assert get_notification_by_id(notification.id).status == 'delivered'
         statsd_client.timing_with_dates.assert_any_call(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
@@ -216,25 +133,13 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref1'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp2 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref2'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp3 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref3'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
+    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
 
-    assert resp1.status_code == 200
-    assert resp2.status_code == 200
-    assert resp3.status_code == 200
+    assert resp1[1] == 200
+    assert resp2[1] == 200
+    assert resp3[1] == 200
     stats_mock.assert_has_calls([
         call(notification1),
         call(notification2),
@@ -262,15 +167,10 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
+    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    assert status == 200
+    assert res['result'] == 'success'
+    assert res['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
     stats_mock.assert_called_once_with(notification)
 
@@ -295,15 +195,9 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
+    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    assert status == 404
+    assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
     stats_mock.assert_not_called()
 
@@ -328,15 +222,10 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_hard_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
+    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+    assert status == 200
+    assert res['result'] == 'success'
+    assert res['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
     stats_mock.assert_called_once_with(notification)
 

From fe5756a1aa11e55bf25c304b037d3d4b6aee7604 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 14:15:27 +0100
Subject: [PATCH 04/25] Fix tests

---
 .../notifications/test_notifications_ses_callback.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index de2241f69..94c93e7bf 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -133,9 +133,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
-    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
-    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
+    resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
 
     assert resp1[1] == 200
     assert resp2[1] == 200
@@ -167,7 +167,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'
@@ -195,7 +195,7 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert status == 404
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
@@ -222,7 +222,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+    _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'

From 6e6c3c050bc62d2388f43d6cb2444ba2eb8aaebd Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 15:59:54 +0100
Subject: [PATCH 05/25] Temporary test fix

This should be removed when the SES endpoint is removed
---
 app/notifications/notifications_ses_callback.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 6d3c2807e..45053d615 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -37,6 +37,11 @@ def sns_callback_handler():
 def process_ses_response(ses_request):
     client_name = 'SES'
     try:
+
+        # TODO: remove this check once the sns_callback_handler is removed
+        if not isinstance(ses_request, dict):
+            ses_request = json.loads(ses_request)
+
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
             return errors, 400, {}

From 70a627ff21a74149b315fa6462035f07e828932f Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Thu, 3 Aug 2017 18:05:42 +0100
Subject: [PATCH 06/25] Allow for both an endpoint callback and SQS consumption

---
 app/celery/tasks.py                             | 7 +++++++
 app/notifications/notifications_ses_callback.py | 8 +++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index bece59f7c..ec3cf0a7e 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -60,6 +60,7 @@ from app.models import (
     SMS_TYPE,
 )
 from app.notifications.process_notifications import persist_notification
+from app.notifications.notifications_ses_callback import process_ses_response
 from app.service.utils import service_allowed_to_send_to
 from app.statsd_decorators import statsd
 from notifications_utils.s3 import s3upload
@@ -530,3 +531,9 @@ def process_incomplete_job(job_id):
             process_row(row_number, recipient, personalisation, template, job, job.service)
 
     job_complete(job, job.service, template, resumed=True)
+
+
+@notify_celery.task(bind=True, name='process-ses-result')
+@statsd(namespace="tasks")
+def process_ses_results(self, response):
+    process_ses_response(response)
diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index b1d0ba589..101b6bbe1 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -27,11 +27,13 @@ register_errors(ses_callback_blueprint)
 
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
-def process_ses_response():
+def sns_callback_handler():
+    process_ses_response(json.loads(request.data))
+
+
+def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        ses_request = json.loads(request.data)
-
         subscribed_topic = autoconfirm_subscription(ses_request)
         if subscribed_topic:
             current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))

From ea26a308487f8726ed6d5b3af187df9b54f06de8 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 12:16:25 +0100
Subject: [PATCH 07/25] We no longer have guaranteed access to a request
 context

---
 .../notifications_ses_callback.py             | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 101b6bbe1..62af94317 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -28,7 +28,11 @@ register_errors(ses_callback_blueprint)
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
 def sns_callback_handler():
-    process_ses_response(json.loads(request.data))
+    errors, status, kwargs = process_ses_response(json.loads(request.data))
+    if errors:
+        raise InvalidRequest(errors, status)
+
+    return jsonify(**kwargs), status
 
 
 def process_ses_response(ses_request):
@@ -37,18 +41,16 @@ def process_ses_response(ses_request):
         subscribed_topic = autoconfirm_subscription(ses_request)
         if subscribed_topic:
             current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
+            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
 
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors, 400, {}
 
         ses_message = json.loads(ses_request['Message'])
         errors = validate_callback_data(data=ses_message, fields=['notificationType'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors, 400, {}
 
         notification_type = ses_message['notificationType']
         if notification_type == 'Bounce':
@@ -60,7 +62,7 @@ def process_ses_response(ses_request):
             aws_response_dict = get_aws_responses(notification_type)
         except KeyError:
             error = "{} callback failed: status {} not found".format(client_name, notification_type)
-            raise InvalidRequest(error, status_code=400)
+            return error, 400, {}
 
         notification_status = aws_response_dict['notification_status']
 
@@ -73,7 +75,7 @@ def process_ses_response(ses_request):
             if not notification:
                 error = "SES callback failed: notification either not found or already updated " \
                         "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                raise InvalidRequest(error, status_code=404)
+                return error, 404, {}
 
             if not aws_response_dict['success']:
                 current_app.logger.info(
@@ -98,14 +100,12 @@ def process_ses_response(ses_request):
 
             create_outcome_notification_statistic_tasks(notification)
 
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
+            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
 
         except KeyError:
-            message = "SES callback failed: messageId missing"
-            raise InvalidRequest(message, status_code=400)
+            error = "SES callback failed: messageId missing"
+            return error, 400, {}
 
-    except ValueError as ex:
+    except ValueError:
         error = "{} callback failed: invalid json".format(client_name)
-        raise InvalidRequest(error, status_code=400)
+        return error, 400, {}

From 7173fb2d703b990491e8307537d31384d87fe0d5 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 13:50:25 +0100
Subject: [PATCH 08/25] Update tests

---
 .../notifications_ses_callback.py             |   6 -
 .../test_notifications_ses_callback.py        | 179 ++++--------------
 2 files changed, 34 insertions(+), 151 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 62af94317..6d3c2807e 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -15,7 +15,6 @@ from app.dao import (
 )
 from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks
 from app.notifications.process_client_response import validate_callback_data
-from app.notifications.utils import autoconfirm_subscription
 
 ses_callback_blueprint = Blueprint('notifications_ses_callback', __name__)
 
@@ -38,11 +37,6 @@ def sns_callback_handler():
 def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        subscribed_topic = autoconfirm_subscription(ses_request)
-        if subscribed_topic:
-            current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
-            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
-
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
             return errors, 400, {}
diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 0a02b5d58..de2241f69 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -3,11 +3,10 @@ from unittest.mock import call
 
 from flask import json
 from freezegun import freeze_time
-from requests import HTTPError
-import pytest
 
 from app import statsd_client
 from app.dao.notifications_dao import get_notification_by_id
+from app.notifications.notifications_ses_callback import process_ses_response
 from tests.app.conftest import sample_notification as create_sample_notification
 
 
@@ -24,68 +23,9 @@ def test_ses_callback_should_fail_if_invalid_json(client, mocker):
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
-    response = client.post(
-        path='/notifications/email/ses',
-        data="nonsense",
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: invalid json'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_should_autoconfirm_subscriptions(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    endpoint = json.loads(ses_confirmation_callback())['SubscribeURL']
-    rmock.request(
-        "GET",
-        endpoint,
-        json={"status": "success"},
-        status_code=200)
-
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_confirmation_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-
-    assert rmock.called
-    assert rmock.request_history[0].url == endpoint
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_autoconfirm_raises_exception_if_not_200(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    endpoint = json.loads(ses_confirmation_callback())['SubscribeURL']
-    rmock.request(
-        "GET",
-        endpoint,
-        json={"status": "not allowed"},
-        status_code=405)
-
-    with pytest.raises(HTTPError) as exc:
-        client.post(
-            path='/notifications/email/ses',
-            data=ses_confirmation_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-
-    assert rmock.called
-    assert rmock.request_history[0].url == endpoint
-    assert exc.value.response.status_code == 405
+    errors, status, _ = process_ses_response('nonsense')
+    assert status == 400
+    assert errors == 'SES callback failed: invalid json'
     stats_mock.assert_not_called()
 
 
@@ -94,15 +34,9 @@ def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_invalid_notification_type_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: status Unknown not found'
+    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
+    assert status == 400
+    assert errors == 'SES callback failed: status Unknown not found'
     stats_mock.assert_not_called()
 
 
@@ -111,15 +45,9 @@ def test_ses_callback_should_fail_if_missing_message_id(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_missing_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: messageId missing'
+    errors, status, _ = process_ses_response(json.loads(ses_missing_notification_id_callback()))
+    assert status == 400
+    assert errors == 'SES callback failed: messageId missing'
     stats_mock.assert_not_called()
 
 
@@ -128,15 +56,9 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_invalid_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
+    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
+    assert status == 404
+    assert errors == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
     stats_mock.assert_not_called()
 
 
@@ -164,15 +86,10 @@ def test_ses_callback_should_update_notification_status(
 
         assert get_notification_by_id(notification.id).status == 'sending'
 
-        response = client.post(
-            path='/notifications/email/ses',
-            data=ses_notification_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-        json_resp = json.loads(response.get_data(as_text=True))
-        assert response.status_code == 200
-        assert json_resp['result'] == 'success'
-        assert json_resp['message'] == 'SES callback succeeded'
+        _, status, res = process_ses_response(json.loads(ses_notification_callback()))
+        assert status == 200
+        assert res['result'] == 'success'
+        assert res['message'] == 'SES callback succeeded'
         assert get_notification_by_id(notification.id).status == 'delivered'
         statsd_client.timing_with_dates.assert_any_call(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
@@ -216,25 +133,13 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref1'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp2 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref2'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp3 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref3'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
+    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
 
-    assert resp1.status_code == 200
-    assert resp2.status_code == 200
-    assert resp3.status_code == 200
+    assert resp1[1] == 200
+    assert resp2[1] == 200
+    assert resp3[1] == 200
     stats_mock.assert_has_calls([
         call(notification1),
         call(notification2),
@@ -262,15 +167,10 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
+    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    assert status == 200
+    assert res['result'] == 'success'
+    assert res['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
     stats_mock.assert_called_once_with(notification)
 
@@ -295,15 +195,9 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
+    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    assert status == 404
+    assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
     stats_mock.assert_not_called()
 
@@ -328,15 +222,10 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_hard_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
+    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+    assert status == 200
+    assert res['result'] == 'success'
+    assert res['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
     stats_mock.assert_called_once_with(notification)
 

From 5abd2a527a1921264377a1d69aea0a0a0a600a34 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 14:15:27 +0100
Subject: [PATCH 09/25] Fix tests

---
 .../notifications/test_notifications_ses_callback.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index de2241f69..94c93e7bf 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -133,9 +133,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
-    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
-    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
+    resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
 
     assert resp1[1] == 200
     assert resp2[1] == 200
@@ -167,7 +167,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'
@@ -195,7 +195,7 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert status == 404
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
@@ -222,7 +222,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+    _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'

From 368da49469a0ee72a25903f4ccf6b8d1ace1c961 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 15:59:54 +0100
Subject: [PATCH 10/25] Temporary test fix

This should be removed when the SES endpoint is removed
---
 app/notifications/notifications_ses_callback.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 6d3c2807e..45053d615 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -37,6 +37,11 @@ def sns_callback_handler():
 def process_ses_response(ses_request):
     client_name = 'SES'
     try:
+
+        # TODO: remove this check once the sns_callback_handler is removed
+        if not isinstance(ses_request, dict):
+            ses_request = json.loads(ses_request)
+
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
             return errors, 400, {}

From b405f100c2fed98900325428c6725717dac5d1b9 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Tue, 24 Oct 2017 15:39:35 +0100
Subject: [PATCH 11/25] Removed the HTTP error and arguments from
 notification_ses_callback.py

In preparation for moving the SNS notification to an SES queue remove
the HTTP errors codes and arguments as the method will now be run by
a celery task. Also made the callback http method return more generic
codes as this will be removed in the longer term.

- Removed errors and arguments returned from process_ses_response
- Updated tests
---
 .../notifications_ses_callback.py             | 21 +++++-----
 .../test_notifications_ses_callback.py        | 42 ++++++-------------
 2 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 45053d615..4780b4bd7 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -27,11 +27,12 @@ register_errors(ses_callback_blueprint)
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
 def sns_callback_handler():
-    errors, status, kwargs = process_ses_response(json.loads(request.data))
+    errors = process_ses_response(json.loads(request.data))
     if errors:
-        raise InvalidRequest(errors, status)
+        current_app.logger.error(errors)
+        raise InvalidRequest(errors, 400)
 
-    return jsonify(**kwargs), status
+    return 200
 
 
 def process_ses_response(ses_request):
@@ -44,12 +45,12 @@ def process_ses_response(ses_request):
 
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
-            return errors, 400, {}
+            return errors
 
         ses_message = json.loads(ses_request['Message'])
         errors = validate_callback_data(data=ses_message, fields=['notificationType'], client_name=client_name)
         if errors:
-            return errors, 400, {}
+            return errors
 
         notification_type = ses_message['notificationType']
         if notification_type == 'Bounce':
@@ -61,7 +62,7 @@ def process_ses_response(ses_request):
             aws_response_dict = get_aws_responses(notification_type)
         except KeyError:
             error = "{} callback failed: status {} not found".format(client_name, notification_type)
-            return error, 400, {}
+            return error
 
         notification_status = aws_response_dict['notification_status']
 
@@ -74,7 +75,7 @@ def process_ses_response(ses_request):
             if not notification:
                 error = "SES callback failed: notification either not found or already updated " \
                         "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                return error, 404, {}
+                return error
 
             if not aws_response_dict['success']:
                 current_app.logger.info(
@@ -99,12 +100,12 @@ def process_ses_response(ses_request):
 
             create_outcome_notification_statistic_tasks(notification)
 
-            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
+            return
 
         except KeyError:
             error = "SES callback failed: messageId missing"
-            return error, 400, {}
+            return error
 
     except ValueError:
         error = "{} callback failed: invalid json".format(client_name)
-        return error, 400, {}
+        return error
diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 94c93e7bf..0250f2044 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -16,15 +16,14 @@ def test_ses_callback_should_not_need_auth(client):
         data=ses_notification_callback(),
         headers=[('Content-Type', 'text/plain; charset=UTF-8')]
     )
-    assert response.status_code == 404
+    assert response.status_code == 400
 
 
 def test_ses_callback_should_fail_if_invalid_json(client, mocker):
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-    errors, status, _ = process_ses_response('nonsense')
-    assert status == 400
+    errors = process_ses_response('nonsense')
     assert errors == 'SES callback failed: invalid json'
     stats_mock.assert_not_called()
 
@@ -34,8 +33,7 @@ def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
-    assert status == 400
+    errors = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
     assert errors == 'SES callback failed: status Unknown not found'
     stats_mock.assert_not_called()
 
@@ -45,8 +43,7 @@ def test_ses_callback_should_fail_if_missing_message_id(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    errors, status, _ = process_ses_response(json.loads(ses_missing_notification_id_callback()))
-    assert status == 400
+    errors = process_ses_response(json.loads(ses_missing_notification_id_callback()))
     assert errors == 'SES callback failed: messageId missing'
     stats_mock.assert_not_called()
 
@@ -56,8 +53,7 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
-    assert status == 404
+    errors = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
     assert errors == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
     stats_mock.assert_not_called()
 
@@ -86,10 +82,8 @@ def test_ses_callback_should_update_notification_status(
 
         assert get_notification_by_id(notification.id).status == 'sending'
 
-        _, status, res = process_ses_response(json.loads(ses_notification_callback()))
-        assert status == 200
-        assert res['result'] == 'success'
-        assert res['message'] == 'SES callback succeeded'
+        errors = process_ses_response(json.loads(ses_notification_callback()))
+        assert errors is None
         assert get_notification_by_id(notification.id).status == 'delivered'
         statsd_client.timing_with_dates.assert_any_call(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
@@ -133,13 +127,10 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
-    resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
-    resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref1'))) is None
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref2'))) is None
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref3'))) is None
 
-    assert resp1[1] == 200
-    assert resp2[1] == 200
-    assert resp3[1] == 200
     stats_mock.assert_has_calls([
         call(notification1),
         call(notification2),
@@ -167,10 +158,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
-    assert status == 200
-    assert res['result'] == 'success'
-    assert res['message'] == 'SES callback succeeded'
+    assert process_ses_response(json.loads(ses_soft_bounce_callback())) is None
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
     stats_mock.assert_called_once_with(notification)
 
@@ -195,8 +183,7 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
-    assert status == 404
+    error = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
     stats_mock.assert_not_called()
@@ -222,10 +209,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
-    assert status == 200
-    assert res['result'] == 'success'
-    assert res['message'] == 'SES callback succeeded'
+    assert process_ses_response(json.loads(ses_hard_bounce_callback())) is None
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
     stats_mock.assert_called_once_with(notification)
 

From e293b208488e9624f9d9b09bd6f7f186ddadfc59 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Tue, 24 Oct 2017 16:58:39 +0100
Subject: [PATCH 12/25] Retry process_ses_results if it fails

If the SES task fails retry every 5 mins for an hour.

- Updated the process_ses_results task to retry
- Added tests to check for retry
---
 app/celery/tasks.py            |  7 +++--
 tests/app/celery/test_tasks.py | 52 ++++++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index ec3cf0a7e..27a3b1e01 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -533,7 +533,10 @@ def process_incomplete_job(job_id):
     job_complete(job, job.service, template, resumed=True)
 
 
-@notify_celery.task(bind=True, name='process-ses-result')
+@notify_celery.task(bind=True, name="process-ses-result", max_retries=12, default_retry_delay=300000)
 @statsd(namespace="tasks")
 def process_ses_results(self, response):
-    process_ses_response(response)
+    errors = process_ses_response(response)
+    if errors:
+        current_app.logger.error(errors)
+        self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error")
diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py
index e018a4f32..39bce8e69 100644
--- a/tests/app/celery/test_tasks.py
+++ b/tests/app/celery/test_tasks.py
@@ -1,3 +1,4 @@
+import codecs
 import json
 import uuid
 from datetime import datetime, timedelta
@@ -26,8 +27,8 @@ from app.celery.tasks import (
     process_incomplete_jobs,
     get_template_class,
     s3,
-    send_inbound_sms_to_service
-)
+    send_inbound_sms_to_service,
+    process_ses_results)
 from app.config import QueueNames
 from app.dao import jobs_dao, services_dao
 from app.models import (
@@ -1381,3 +1382,50 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template):
     assert completed_job.job_status == JOB_STATUS_FINISHED
 
     assert mock_letter_saver.call_count == 8
+
+
+def test_process_ses_results(notify_db, notify_db_session, sample_email_template):
+
+    create_sample_notification(
+        notify_db,
+        notify_db_session,
+        template=sample_email_template,
+        reference='ref1',
+        sent_at=datetime.utcnow(),
+        status='sending')
+
+    response = json.loads(ses_notification_callback())
+    assert process_ses_results(response=response) is None
+
+
+def test_process_ses_results_retry_called(notify_db, mocker):
+    mocked = mocker.patch('app.celery.tasks.process_ses_results.retry')
+    response = json.loads(ses_notification_callback())
+    process_ses_results(response=response)
+    assert mocked.call_count != 0
+
+
+def ses_notification_callback():
+    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
+           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
+           '\n  "Message" : "{\\"notificationType\\":\\"Delivery\\",' \
+           '\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",' \
+           '\\"source\\":\\"test@test-domain.com\\",' \
+           '\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",' \
+           '\\"sendingAccountId\\":\\"123456789012\\",' \
+           '\\"messageId\\":\\"ref1\\",' \
+           '\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},' \
+           '\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",' \
+           '\\"processingTimeMillis\\":658,' \
+           '\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],' \
+           '\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",' \
+           '\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",' \
+           '\n  "Timestamp" : "2016-03-14T12:35:26.665Z",\n  "SignatureVersion" : "1",' \
+           '\n  "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUt' \
+           'OowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYL' \
+           'VSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMA' \
+           'PmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",' \
+           '\n  "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750' \
+           'dd426d95ee9390147a5624348ee.pem",' \
+           '\n  "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&S' \
+           'ubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}'

From 9c9581e80831f09e50b88290ff8fb6399f95db43 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Thu, 26 Oct 2017 12:09:14 +0100
Subject: [PATCH 13/25] Review Changes

- Updated the retry and max_retries of the process_ses_results celery
task to be the same as other retry strategies in that file
- Provided a message with the 200 to be similar to how other responses
are handled
---
 app/celery/tasks.py                             | 2 +-
 app/notifications/notifications_ses_callback.py | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index 27a3b1e01..df1d7bffd 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -533,7 +533,7 @@ def process_incomplete_job(job_id):
     job_complete(job, job.service, template, resumed=True)
 
 
-@notify_celery.task(bind=True, name="process-ses-result", max_retries=12, default_retry_delay=300000)
+@notify_celery.task(bind=True, name="process-ses-result", max_retries=5, default_retry_delay=300)
 @statsd(namespace="tasks")
 def process_ses_results(self, response):
     errors = process_ses_response(response)
diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 4780b4bd7..2ed34037b 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -32,7 +32,9 @@ def sns_callback_handler():
         current_app.logger.error(errors)
         raise InvalidRequest(errors, 400)
 
-    return 200
+    return jsonify(
+        result="success", message="SES callback succeeded"
+    ), 200
 
 
 def process_ses_response(ses_request):

From 865cb6656e1d961d25f6063a11bb75da5f6e210c Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Thu, 3 Aug 2017 18:05:42 +0100
Subject: [PATCH 14/25] Allow for both an endpoint callback and SQS consumption

---
 app/celery/tasks.py                             | 7 +++++++
 app/notifications/notifications_ses_callback.py | 8 +++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index a042d7f0a..e4dcdafda 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -63,6 +63,7 @@ from app.models import (
     SMS_TYPE,
 )
 from app.notifications.process_notifications import persist_notification
+from app.notifications.notifications_ses_callback import process_ses_response
 from app.service.utils import service_allowed_to_send_to
 from app.statsd_decorators import statsd
 from notifications_utils.s3 import s3upload
@@ -547,3 +548,9 @@ def process_incomplete_job(job_id):
             process_row(row_number, recipient, personalisation, template, job, job.service)
 
     job_complete(job, job.service, template, resumed=True)
+
+
+@notify_celery.task(bind=True, name='process-ses-result')
+@statsd(namespace="tasks")
+def process_ses_results(self, response):
+    process_ses_response(response)
diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index b1d0ba589..101b6bbe1 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -27,11 +27,13 @@ register_errors(ses_callback_blueprint)
 
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
-def process_ses_response():
+def sns_callback_handler():
+    process_ses_response(json.loads(request.data))
+
+
+def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        ses_request = json.loads(request.data)
-
         subscribed_topic = autoconfirm_subscription(ses_request)
         if subscribed_topic:
             current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))

From 892eaede132e87d5ecb78c7ef946b0900f04813e Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 12:16:25 +0100
Subject: [PATCH 15/25] We no longer have guaranteed access to a request
 context

---
 .../notifications_ses_callback.py             | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 101b6bbe1..62af94317 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -28,7 +28,11 @@ register_errors(ses_callback_blueprint)
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
 def sns_callback_handler():
-    process_ses_response(json.loads(request.data))
+    errors, status, kwargs = process_ses_response(json.loads(request.data))
+    if errors:
+        raise InvalidRequest(errors, status)
+
+    return jsonify(**kwargs), status
 
 
 def process_ses_response(ses_request):
@@ -37,18 +41,16 @@ def process_ses_response(ses_request):
         subscribed_topic = autoconfirm_subscription(ses_request)
         if subscribed_topic:
             current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
+            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
 
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors, 400, {}
 
         ses_message = json.loads(ses_request['Message'])
         errors = validate_callback_data(data=ses_message, fields=['notificationType'], client_name=client_name)
         if errors:
-            raise InvalidRequest(errors, status_code=400)
+            return errors, 400, {}
 
         notification_type = ses_message['notificationType']
         if notification_type == 'Bounce':
@@ -60,7 +62,7 @@ def process_ses_response(ses_request):
             aws_response_dict = get_aws_responses(notification_type)
         except KeyError:
             error = "{} callback failed: status {} not found".format(client_name, notification_type)
-            raise InvalidRequest(error, status_code=400)
+            return error, 400, {}
 
         notification_status = aws_response_dict['notification_status']
 
@@ -73,7 +75,7 @@ def process_ses_response(ses_request):
             if not notification:
                 error = "SES callback failed: notification either not found or already updated " \
                         "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                raise InvalidRequest(error, status_code=404)
+                return error, 404, {}
 
             if not aws_response_dict['success']:
                 current_app.logger.info(
@@ -98,14 +100,12 @@ def process_ses_response(ses_request):
 
             create_outcome_notification_statistic_tasks(notification)
 
-            return jsonify(
-                result="success", message="SES callback succeeded"
-            ), 200
+            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
 
         except KeyError:
-            message = "SES callback failed: messageId missing"
-            raise InvalidRequest(message, status_code=400)
+            error = "SES callback failed: messageId missing"
+            return error, 400, {}
 
-    except ValueError as ex:
+    except ValueError:
         error = "{} callback failed: invalid json".format(client_name)
-        raise InvalidRequest(error, status_code=400)
+        return error, 400, {}

From 644b31ba7582df1d1fc986e0be48ea8a22c13fcd Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 13:50:25 +0100
Subject: [PATCH 16/25] Update tests

---
 .../notifications_ses_callback.py             |   6 -
 .../test_notifications_ses_callback.py        | 179 ++++--------------
 2 files changed, 34 insertions(+), 151 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 62af94317..6d3c2807e 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -15,7 +15,6 @@ from app.dao import (
 )
 from app.celery.statistics_tasks import create_outcome_notification_statistic_tasks
 from app.notifications.process_client_response import validate_callback_data
-from app.notifications.utils import autoconfirm_subscription
 
 ses_callback_blueprint = Blueprint('notifications_ses_callback', __name__)
 
@@ -38,11 +37,6 @@ def sns_callback_handler():
 def process_ses_response(ses_request):
     client_name = 'SES'
     try:
-        subscribed_topic = autoconfirm_subscription(ses_request)
-        if subscribed_topic:
-            current_app.logger.info("Automatically subscribed to topic: {}".format(subscribed_topic))
-            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
-
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
             return errors, 400, {}
diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 0a02b5d58..de2241f69 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -3,11 +3,10 @@ from unittest.mock import call
 
 from flask import json
 from freezegun import freeze_time
-from requests import HTTPError
-import pytest
 
 from app import statsd_client
 from app.dao.notifications_dao import get_notification_by_id
+from app.notifications.notifications_ses_callback import process_ses_response
 from tests.app.conftest import sample_notification as create_sample_notification
 
 
@@ -24,68 +23,9 @@ def test_ses_callback_should_fail_if_invalid_json(client, mocker):
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
-    response = client.post(
-        path='/notifications/email/ses',
-        data="nonsense",
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: invalid json'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_should_autoconfirm_subscriptions(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    endpoint = json.loads(ses_confirmation_callback())['SubscribeURL']
-    rmock.request(
-        "GET",
-        endpoint,
-        json={"status": "success"},
-        status_code=200)
-
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_confirmation_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-
-    assert rmock.called
-    assert rmock.request_history[0].url == endpoint
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
-    stats_mock.assert_not_called()
-
-
-def test_ses_callback_autoconfirm_raises_exception_if_not_200(client, rmock, mocker):
-    stats_mock = mocker.patch(
-        'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
-    )
-
-    endpoint = json.loads(ses_confirmation_callback())['SubscribeURL']
-    rmock.request(
-        "GET",
-        endpoint,
-        json={"status": "not allowed"},
-        status_code=405)
-
-    with pytest.raises(HTTPError) as exc:
-        client.post(
-            path='/notifications/email/ses',
-            data=ses_confirmation_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-
-    assert rmock.called
-    assert rmock.request_history[0].url == endpoint
-    assert exc.value.response.status_code == 405
+    errors, status, _ = process_ses_response('nonsense')
+    assert status == 400
+    assert errors == 'SES callback failed: invalid json'
     stats_mock.assert_not_called()
 
 
@@ -94,15 +34,9 @@ def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_invalid_notification_type_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: status Unknown not found'
+    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
+    assert status == 400
+    assert errors == 'SES callback failed: status Unknown not found'
     stats_mock.assert_not_called()
 
 
@@ -111,15 +45,9 @@ def test_ses_callback_should_fail_if_missing_message_id(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_missing_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 400
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: messageId missing'
+    errors, status, _ = process_ses_response(json.loads(ses_missing_notification_id_callback()))
+    assert status == 400
+    assert errors == 'SES callback failed: messageId missing'
     stats_mock.assert_not_called()
 
 
@@ -128,15 +56,9 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_invalid_notification_id_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
+    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
+    assert status == 404
+    assert errors == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
     stats_mock.assert_not_called()
 
 
@@ -164,15 +86,10 @@ def test_ses_callback_should_update_notification_status(
 
         assert get_notification_by_id(notification.id).status == 'sending'
 
-        response = client.post(
-            path='/notifications/email/ses',
-            data=ses_notification_callback(),
-            headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-        )
-        json_resp = json.loads(response.get_data(as_text=True))
-        assert response.status_code == 200
-        assert json_resp['result'] == 'success'
-        assert json_resp['message'] == 'SES callback succeeded'
+        _, status, res = process_ses_response(json.loads(ses_notification_callback()))
+        assert status == 200
+        assert res['result'] == 'success'
+        assert res['message'] == 'SES callback succeeded'
         assert get_notification_by_id(notification.id).status == 'delivered'
         statsd_client.timing_with_dates.assert_any_call(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
@@ -216,25 +133,13 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref1'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp2 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref2'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    resp3 = client.post(
-        path='/notifications/email/ses',
-        data=ses_notification_callback(ref='ref3'),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
+    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
 
-    assert resp1.status_code == 200
-    assert resp2.status_code == 200
-    assert resp3.status_code == 200
+    assert resp1[1] == 200
+    assert resp2[1] == 200
+    assert resp3[1] == 200
     stats_mock.assert_has_calls([
         call(notification1),
         call(notification2),
@@ -262,15 +167,10 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
+    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    assert status == 200
+    assert res['result'] == 'success'
+    assert res['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
     stats_mock.assert_called_once_with(notification)
 
@@ -295,15 +195,9 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_soft_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 404
-    assert json_resp['result'] == 'error'
-    assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
+    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    assert status == 404
+    assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
     stats_mock.assert_not_called()
 
@@ -328,15 +222,10 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    response = client.post(
-        path='/notifications/email/ses',
-        data=ses_hard_bounce_callback(),
-        headers=[('Content-Type', 'text/plain; charset=UTF-8')]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
-    assert json_resp['result'] == 'success'
-    assert json_resp['message'] == 'SES callback succeeded'
+    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+    assert status == 200
+    assert res['result'] == 'success'
+    assert res['message'] == 'SES callback succeeded'
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
     stats_mock.assert_called_once_with(notification)
 

From 263131f6ba79f359da443d69c35e51fd16610f14 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 14:15:27 +0100
Subject: [PATCH 17/25] Fix tests

---
 .../notifications/test_notifications_ses_callback.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index de2241f69..94c93e7bf 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -133,9 +133,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
-    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
-    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
+    resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
 
     assert resp1[1] == 200
     assert resp2[1] == 200
@@ -167,7 +167,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'
@@ -195,7 +195,7 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
-    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+    error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert status == 404
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
@@ -222,7 +222,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
-    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+    _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'

From 3443dcea53729b7f09465657ac88177eadfa6eaa Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 15:59:54 +0100
Subject: [PATCH 18/25] Temporary test fix

This should be removed when the SES endpoint is removed
---
 app/notifications/notifications_ses_callback.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 6d3c2807e..45053d615 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -37,6 +37,11 @@ def sns_callback_handler():
 def process_ses_response(ses_request):
     client_name = 'SES'
     try:
+
+        # TODO: remove this check once the sns_callback_handler is removed
+        if not isinstance(ses_request, dict):
+            ses_request = json.loads(ses_request)
+
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
             return errors, 400, {}

From f10893d3835cdf590be06f605bfdd3fe977cada5 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Thu, 3 Aug 2017 18:05:42 +0100
Subject: [PATCH 19/25] Allow for both an endpoint callback and SQS consumption

---
 tests/app/celery/test_csv_files/email.csv           |  2 ++
 tests/app/celery/test_csv_files/empty.csv           |  1 +
 tests/app/celery/test_csv_files/multiple_email.csv  | 11 +++++++++++
 tests/app/celery/test_csv_files/multiple_letter.csv | 11 +++++++++++
 tests/app/celery/test_csv_files/multiple_sms.csv    | 11 +++++++++++
 tests/app/celery/test_csv_files/sms.csv             |  2 ++
 6 files changed, 38 insertions(+)
 create mode 100644 tests/app/celery/test_csv_files/email.csv
 create mode 100644 tests/app/celery/test_csv_files/empty.csv
 create mode 100644 tests/app/celery/test_csv_files/multiple_email.csv
 create mode 100644 tests/app/celery/test_csv_files/multiple_letter.csv
 create mode 100644 tests/app/celery/test_csv_files/multiple_sms.csv
 create mode 100644 tests/app/celery/test_csv_files/sms.csv

diff --git a/tests/app/celery/test_csv_files/email.csv b/tests/app/celery/test_csv_files/email.csv
new file mode 100644
index 000000000..f0cefee69
--- /dev/null
+++ b/tests/app/celery/test_csv_files/email.csv
@@ -0,0 +1,2 @@
+email_address
+test@test.com
diff --git a/tests/app/celery/test_csv_files/empty.csv b/tests/app/celery/test_csv_files/empty.csv
new file mode 100644
index 000000000..64e5bd0a3
--- /dev/null
+++ b/tests/app/celery/test_csv_files/empty.csv
@@ -0,0 +1 @@
+phone number
diff --git a/tests/app/celery/test_csv_files/multiple_email.csv b/tests/app/celery/test_csv_files/multiple_email.csv
new file mode 100644
index 000000000..5da15797d
--- /dev/null
+++ b/tests/app/celery/test_csv_files/multiple_email.csv
@@ -0,0 +1,11 @@
+EMAILADDRESS
+test1@test.com
+test2@test.com
+test3@test.com
+test4@test.com
+test5@test.com
+test6@test.com
+test7@test.com
+test8@test.com
+test9@test.com
+test0@test.com
diff --git a/tests/app/celery/test_csv_files/multiple_letter.csv b/tests/app/celery/test_csv_files/multiple_letter.csv
new file mode 100644
index 000000000..0e9d847b8
--- /dev/null
+++ b/tests/app/celery/test_csv_files/multiple_letter.csv
@@ -0,0 +1,11 @@
+address_line_1, address_line_2, address_line_3
+name1, street1, town1, postcode1
+name2, street2, town2, postcode2
+name3, street3, town3, postcode3
+name4, street4, town4, postcode4
+name5, street5, town5, postcode5
+name6, street6, town6, postcode6
+name7, street7, town7, postcode7
+name8, street8, town8, postcode8
+name9, street9, town9, postcode9
+name0, street0, town0, postcode0
\ No newline at end of file
diff --git a/tests/app/celery/test_csv_files/multiple_sms.csv b/tests/app/celery/test_csv_files/multiple_sms.csv
new file mode 100644
index 000000000..2ecad9140
--- /dev/null
+++ b/tests/app/celery/test_csv_files/multiple_sms.csv
@@ -0,0 +1,11 @@
+PhoneNumber,Name
++441234123121,chris
++441234123122,chris
++441234123123,chris
++441234123124,chris
++441234123125,chris
++441234123126,chris
++441234123127,chris
++441234123128,chris
++441234123129,chris
++441234123120,chris
diff --git a/tests/app/celery/test_csv_files/sms.csv b/tests/app/celery/test_csv_files/sms.csv
new file mode 100644
index 000000000..728639972
--- /dev/null
+++ b/tests/app/celery/test_csv_files/sms.csv
@@ -0,0 +1,2 @@
+PHONE NUMBER, IGNORE THIS COLUMN
++441234123123, nope

From dada25beb1807c9b95f1177ce2f68ee57a466561 Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 13:50:25 +0100
Subject: [PATCH 20/25] Update tests

---
 .../test_notifications_ses_callback.py         | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 94c93e7bf..8394b82c0 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -133,9 +133,15 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
+<<<<<<< HEAD
     resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
     resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
     resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
+=======
+    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
+    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
+    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
+>>>>>>> Update tests
 
     assert resp1[1] == 200
     assert resp2[1] == 200
@@ -167,7 +173,11 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
+<<<<<<< HEAD
     _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
+=======
+    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+>>>>>>> Update tests
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'
@@ -195,7 +205,11 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
 
     assert get_notification_by_id(notification.id).status == 'delivered'
 
+<<<<<<< HEAD
     error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
+=======
+    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
+>>>>>>> Update tests
     assert status == 404
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
@@ -222,7 +236,11 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
 
     assert get_notification_by_id(notification.id).status == 'sending'
 
+<<<<<<< HEAD
     _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
+=======
+    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
+>>>>>>> Update tests
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'

From 46144708cb81081adbaee5937f6f3894a0eaa63a Mon Sep 17 00:00:00 2001
From: Athanasios Voutsadakis
 <athanasios.voutsadakis@digital.cabinet-office.gov.uk>
Date: Fri, 4 Aug 2017 14:15:27 +0100
Subject: [PATCH 21/25] Fix tests

---
 .../test_notifications_ses_callback.py        | 23 +++----------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 8394b82c0..5d6ec9943 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -133,15 +133,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-<<<<<<< HEAD
     resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
     resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
     resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
-=======
-    resp1 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref1')))
-    resp2 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref2')))
-    resp3 = process_ses_response(json.loads(data=ses_notification_callback(ref='ref3')))
->>>>>>> Update tests
 
     assert resp1[1] == 200
     assert resp2[1] == 200
@@ -173,11 +167,8 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     )
     assert get_notification_by_id(notification.id).status == 'sending'
 
-<<<<<<< HEAD
     _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
-=======
-    _, status, res = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
->>>>>>> Update tests
+
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'
@@ -204,12 +195,8 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
     )
 
     assert get_notification_by_id(notification.id).status == 'delivered'
-
-<<<<<<< HEAD
     error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
-=======
-    error, status, _ = process_ses_response(json.loads(data=ses_soft_bounce_callback()))
->>>>>>> Update tests
+
     assert status == 404
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
@@ -235,12 +222,8 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
     )
 
     assert get_notification_by_id(notification.id).status == 'sending'
-
-<<<<<<< HEAD
     _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
-=======
-    _, status, res = process_ses_response(json.loads(data=ses_hard_bounce_callback()))
->>>>>>> Update tests
+
     assert status == 200
     assert res['result'] == 'success'
     assert res['message'] == 'SES callback succeeded'

From 0494ef3ea76cbbc894628653150e411f58d9fff3 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Tue, 24 Oct 2017 15:39:35 +0100
Subject: [PATCH 22/25] Removed the HTTP error and arguments from
 notification_ses_callback.py

In preparation for moving the SNS notification to an SES queue remove
the HTTP errors codes and arguments as the method will now be run by
a celery task. Also made the callback http method return more generic
codes as this will be removed in the longer term.

- Removed errors and arguments returned from process_ses_response
- Updated tests
---
 .../notifications_ses_callback.py             | 21 +++++----
 .../test_notifications_ses_callback.py        | 46 ++++++-------------
 2 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 45053d615..4780b4bd7 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -27,11 +27,12 @@ register_errors(ses_callback_blueprint)
 
 @ses_callback_blueprint.route('/notifications/email/ses', methods=['POST'])
 def sns_callback_handler():
-    errors, status, kwargs = process_ses_response(json.loads(request.data))
+    errors = process_ses_response(json.loads(request.data))
     if errors:
-        raise InvalidRequest(errors, status)
+        current_app.logger.error(errors)
+        raise InvalidRequest(errors, 400)
 
-    return jsonify(**kwargs), status
+    return 200
 
 
 def process_ses_response(ses_request):
@@ -44,12 +45,12 @@ def process_ses_response(ses_request):
 
         errors = validate_callback_data(data=ses_request, fields=['Message'], client_name=client_name)
         if errors:
-            return errors, 400, {}
+            return errors
 
         ses_message = json.loads(ses_request['Message'])
         errors = validate_callback_data(data=ses_message, fields=['notificationType'], client_name=client_name)
         if errors:
-            return errors, 400, {}
+            return errors
 
         notification_type = ses_message['notificationType']
         if notification_type == 'Bounce':
@@ -61,7 +62,7 @@ def process_ses_response(ses_request):
             aws_response_dict = get_aws_responses(notification_type)
         except KeyError:
             error = "{} callback failed: status {} not found".format(client_name, notification_type)
-            return error, 400, {}
+            return error
 
         notification_status = aws_response_dict['notification_status']
 
@@ -74,7 +75,7 @@ def process_ses_response(ses_request):
             if not notification:
                 error = "SES callback failed: notification either not found or already updated " \
                         "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                return error, 404, {}
+                return error
 
             if not aws_response_dict['success']:
                 current_app.logger.info(
@@ -99,12 +100,12 @@ def process_ses_response(ses_request):
 
             create_outcome_notification_statistic_tasks(notification)
 
-            return [], 200, {'result': "success", 'message': "SES callback succeeded"}
+            return
 
         except KeyError:
             error = "SES callback failed: messageId missing"
-            return error, 400, {}
+            return error
 
     except ValueError:
         error = "{} callback failed: invalid json".format(client_name)
-        return error, 400, {}
+        return error
diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py
index 5d6ec9943..cfcb4a4b8 100644
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -16,15 +16,14 @@ def test_ses_callback_should_not_need_auth(client):
         data=ses_notification_callback(),
         headers=[('Content-Type', 'text/plain; charset=UTF-8')]
     )
-    assert response.status_code == 404
+    assert response.status_code == 400
 
 
 def test_ses_callback_should_fail_if_invalid_json(client, mocker):
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-    errors, status, _ = process_ses_response('nonsense')
-    assert status == 400
+    errors = process_ses_response('nonsense')
     assert errors == 'SES callback failed: invalid json'
     stats_mock.assert_not_called()
 
@@ -34,8 +33,7 @@ def test_ses_callback_should_fail_if_invalid_notification_type(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
-    assert status == 400
+    errors = process_ses_response(json.loads(ses_invalid_notification_type_callback()))
     assert errors == 'SES callback failed: status Unknown not found'
     stats_mock.assert_not_called()
 
@@ -45,8 +43,7 @@ def test_ses_callback_should_fail_if_missing_message_id(client, mocker):
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    errors, status, _ = process_ses_response(json.loads(ses_missing_notification_id_callback()))
-    assert status == 400
+    errors = process_ses_response(json.loads(ses_missing_notification_id_callback()))
     assert errors == 'SES callback failed: messageId missing'
     stats_mock.assert_not_called()
 
@@ -56,8 +53,7 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
 
-    errors, status, _ = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
-    assert status == 404
+    errors = process_ses_response(json.loads(ses_invalid_notification_id_callback()))
     assert errors == 'SES callback failed: notification either not found or already updated from sending. Status delivered for notification reference missing'  # noqa
     stats_mock.assert_not_called()
 
@@ -86,10 +82,8 @@ def test_ses_callback_should_update_notification_status(
 
         assert get_notification_by_id(notification.id).status == 'sending'
 
-        _, status, res = process_ses_response(json.loads(ses_notification_callback()))
-        assert status == 200
-        assert res['result'] == 'success'
-        assert res['message'] == 'SES callback succeeded'
+        errors = process_ses_response(json.loads(ses_notification_callback()))
+        assert errors is None
         assert get_notification_by_id(notification.id).status == 'delivered'
         statsd_client.timing_with_dates.assert_any_call(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
@@ -133,13 +127,10 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         sent_at=datetime.utcnow(),
         status='sending')
 
-    resp1 = process_ses_response(json.loads(ses_notification_callback(ref='ref1')))
-    resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
-    resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref1'))) is None
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref2'))) is None
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref3'))) is None
 
-    assert resp1[1] == 200
-    assert resp2[1] == 200
-    assert resp3[1] == 200
     stats_mock.assert_has_calls([
         call(notification1),
         call(notification2),
@@ -166,12 +157,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
         sent_at=datetime.utcnow()
     )
     assert get_notification_by_id(notification.id).status == 'sending'
-
-    _, status, res = process_ses_response(json.loads(ses_soft_bounce_callback()))
-
-    assert status == 200
-    assert res['result'] == 'success'
-    assert res['message'] == 'SES callback succeeded'
+    assert process_ses_response(json.loads(ses_soft_bounce_callback())) is None
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
     stats_mock.assert_called_once_with(notification)
 
@@ -195,9 +181,7 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
     )
 
     assert get_notification_by_id(notification.id).status == 'delivered'
-    error, status, _ = process_ses_response(json.loads(ses_soft_bounce_callback()))
-
-    assert status == 404
+    error = process_ses_response(json.loads(ses_soft_bounce_callback()))
     assert error == 'SES callback failed: notification either not found or already updated from sending. Status temporary-failure for notification reference ref'  # noqa
     assert get_notification_by_id(notification.id).status == 'delivered'
     stats_mock.assert_not_called()
@@ -222,11 +206,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
     )
 
     assert get_notification_by_id(notification.id).status == 'sending'
-    _, status, res = process_ses_response(json.loads(ses_hard_bounce_callback()))
-
-    assert status == 200
-    assert res['result'] == 'success'
-    assert res['message'] == 'SES callback succeeded'
+    assert process_ses_response(json.loads(ses_hard_bounce_callback())) is None
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
     stats_mock.assert_called_once_with(notification)
 

From c2f2d39d22a1baec4b57f32fd8b90031b7c95917 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Tue, 24 Oct 2017 16:58:39 +0100
Subject: [PATCH 23/25] Retry process_ses_results if it fails

If the SES task fails retry every 5 mins for an hour.

- Updated the process_ses_results task to retry
- Added tests to check for retry
---
 app/celery/tasks.py            |  7 +++--
 tests/app/celery/test_tasks.py | 52 ++++++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index e4dcdafda..e2bda04da 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -550,7 +550,10 @@ def process_incomplete_job(job_id):
     job_complete(job, job.service, template, resumed=True)
 
 
-@notify_celery.task(bind=True, name='process-ses-result')
+@notify_celery.task(bind=True, name="process-ses-result", max_retries=12, default_retry_delay=300000)
 @statsd(namespace="tasks")
 def process_ses_results(self, response):
-    process_ses_response(response)
+    errors = process_ses_response(response)
+    if errors:
+        current_app.logger.error(errors)
+        self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error")
diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py
index e018a4f32..39bce8e69 100644
--- a/tests/app/celery/test_tasks.py
+++ b/tests/app/celery/test_tasks.py
@@ -1,3 +1,4 @@
+import codecs
 import json
 import uuid
 from datetime import datetime, timedelta
@@ -26,8 +27,8 @@ from app.celery.tasks import (
     process_incomplete_jobs,
     get_template_class,
     s3,
-    send_inbound_sms_to_service
-)
+    send_inbound_sms_to_service,
+    process_ses_results)
 from app.config import QueueNames
 from app.dao import jobs_dao, services_dao
 from app.models import (
@@ -1381,3 +1382,50 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template):
     assert completed_job.job_status == JOB_STATUS_FINISHED
 
     assert mock_letter_saver.call_count == 8
+
+
+def test_process_ses_results(notify_db, notify_db_session, sample_email_template):
+
+    create_sample_notification(
+        notify_db,
+        notify_db_session,
+        template=sample_email_template,
+        reference='ref1',
+        sent_at=datetime.utcnow(),
+        status='sending')
+
+    response = json.loads(ses_notification_callback())
+    assert process_ses_results(response=response) is None
+
+
+def test_process_ses_results_retry_called(notify_db, mocker):
+    mocked = mocker.patch('app.celery.tasks.process_ses_results.retry')
+    response = json.loads(ses_notification_callback())
+    process_ses_results(response=response)
+    assert mocked.call_count != 0
+
+
+def ses_notification_callback():
+    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
+           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
+           '\n  "Message" : "{\\"notificationType\\":\\"Delivery\\",' \
+           '\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",' \
+           '\\"source\\":\\"test@test-domain.com\\",' \
+           '\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",' \
+           '\\"sendingAccountId\\":\\"123456789012\\",' \
+           '\\"messageId\\":\\"ref1\\",' \
+           '\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},' \
+           '\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",' \
+           '\\"processingTimeMillis\\":658,' \
+           '\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],' \
+           '\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",' \
+           '\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",' \
+           '\n  "Timestamp" : "2016-03-14T12:35:26.665Z",\n  "SignatureVersion" : "1",' \
+           '\n  "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUt' \
+           'OowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYL' \
+           'VSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMA' \
+           'PmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",' \
+           '\n  "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750' \
+           'dd426d95ee9390147a5624348ee.pem",' \
+           '\n  "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&S' \
+           'ubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}'

From d6cff97b7be6a8c6ffaddff92f6de56ed114893e Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Thu, 26 Oct 2017 12:09:14 +0100
Subject: [PATCH 24/25] Review Changes

- Updated the retry and max_retries of the process_ses_results celery
task to be the same as other retry strategies in that file
- Provided a message with the 200 to be similar to how other responses
are handled
---
 app/celery/tasks.py                             | 2 +-
 app/notifications/notifications_ses_callback.py | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index e2bda04da..23101dccd 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -550,7 +550,7 @@ def process_incomplete_job(job_id):
     job_complete(job, job.service, template, resumed=True)
 
 
-@notify_celery.task(bind=True, name="process-ses-result", max_retries=12, default_retry_delay=300000)
+@notify_celery.task(bind=True, name="process-ses-result", max_retries=5, default_retry_delay=300)
 @statsd(namespace="tasks")
 def process_ses_results(self, response):
     errors = process_ses_response(response)
diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py
index 4780b4bd7..2ed34037b 100644
--- a/app/notifications/notifications_ses_callback.py
+++ b/app/notifications/notifications_ses_callback.py
@@ -32,7 +32,9 @@ def sns_callback_handler():
         current_app.logger.error(errors)
         raise InvalidRequest(errors, 400)
 
-    return 200
+    return jsonify(
+        result="success", message="SES callback succeeded"
+    ), 200
 
 
 def process_ses_response(ses_request):

From 14cbdb28a5e93a648a566738e7deb644d224bd22 Mon Sep 17 00:00:00 2001
From: Richard Chapman <richard.chapman@digital.cabinet-office.gov.uk>
Date: Thu, 26 Oct 2017 12:44:03 +0100
Subject: [PATCH 25/25] Deleted Local Test Files

Deleted files which are not required and were duplicated from the main
test files.
---
 tests/app/celery/test_csv_files/email.csv           |  2 --
 tests/app/celery/test_csv_files/empty.csv           |  1 -
 tests/app/celery/test_csv_files/multiple_email.csv  | 11 -----------
 tests/app/celery/test_csv_files/multiple_letter.csv | 11 -----------
 tests/app/celery/test_csv_files/multiple_sms.csv    | 11 -----------
 tests/app/celery/test_csv_files/sms.csv             |  2 --
 6 files changed, 38 deletions(-)
 delete mode 100644 tests/app/celery/test_csv_files/email.csv
 delete mode 100644 tests/app/celery/test_csv_files/empty.csv
 delete mode 100644 tests/app/celery/test_csv_files/multiple_email.csv
 delete mode 100644 tests/app/celery/test_csv_files/multiple_letter.csv
 delete mode 100644 tests/app/celery/test_csv_files/multiple_sms.csv
 delete mode 100644 tests/app/celery/test_csv_files/sms.csv

diff --git a/tests/app/celery/test_csv_files/email.csv b/tests/app/celery/test_csv_files/email.csv
deleted file mode 100644
index f0cefee69..000000000
--- a/tests/app/celery/test_csv_files/email.csv
+++ /dev/null
@@ -1,2 +0,0 @@
-email_address
-test@test.com
diff --git a/tests/app/celery/test_csv_files/empty.csv b/tests/app/celery/test_csv_files/empty.csv
deleted file mode 100644
index 64e5bd0a3..000000000
--- a/tests/app/celery/test_csv_files/empty.csv
+++ /dev/null
@@ -1 +0,0 @@
-phone number
diff --git a/tests/app/celery/test_csv_files/multiple_email.csv b/tests/app/celery/test_csv_files/multiple_email.csv
deleted file mode 100644
index 5da15797d..000000000
--- a/tests/app/celery/test_csv_files/multiple_email.csv
+++ /dev/null
@@ -1,11 +0,0 @@
-EMAILADDRESS
-test1@test.com
-test2@test.com
-test3@test.com
-test4@test.com
-test5@test.com
-test6@test.com
-test7@test.com
-test8@test.com
-test9@test.com
-test0@test.com
diff --git a/tests/app/celery/test_csv_files/multiple_letter.csv b/tests/app/celery/test_csv_files/multiple_letter.csv
deleted file mode 100644
index 0e9d847b8..000000000
--- a/tests/app/celery/test_csv_files/multiple_letter.csv
+++ /dev/null
@@ -1,11 +0,0 @@
-address_line_1, address_line_2, address_line_3
-name1, street1, town1, postcode1
-name2, street2, town2, postcode2
-name3, street3, town3, postcode3
-name4, street4, town4, postcode4
-name5, street5, town5, postcode5
-name6, street6, town6, postcode6
-name7, street7, town7, postcode7
-name8, street8, town8, postcode8
-name9, street9, town9, postcode9
-name0, street0, town0, postcode0
\ No newline at end of file
diff --git a/tests/app/celery/test_csv_files/multiple_sms.csv b/tests/app/celery/test_csv_files/multiple_sms.csv
deleted file mode 100644
index 2ecad9140..000000000
--- a/tests/app/celery/test_csv_files/multiple_sms.csv
+++ /dev/null
@@ -1,11 +0,0 @@
-PhoneNumber,Name
-+441234123121,chris
-+441234123122,chris
-+441234123123,chris
-+441234123124,chris
-+441234123125,chris
-+441234123126,chris
-+441234123127,chris
-+441234123128,chris
-+441234123129,chris
-+441234123120,chris
diff --git a/tests/app/celery/test_csv_files/sms.csv b/tests/app/celery/test_csv_files/sms.csv
deleted file mode 100644
index 728639972..000000000
--- a/tests/app/celery/test_csv_files/sms.csv
+++ /dev/null
@@ -1,2 +0,0 @@
-PHONE NUMBER, IGNORE THIS COLUMN
-+441234123123, nope