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

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

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')
+    errors = process_ses_response('nonsense')
-    assert status == 400
     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()))
+    errors = 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()
 
@@ -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()))
+    errors = 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()
 
@@ -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()))
+    errors = 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()
 
@@ -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()))
+        errors = process_ses_response(json.loads(ses_notification_callback()))
-        assert status == 200
+        assert errors is None
-        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
@@ -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')))
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref1'))) is None
-    resp2 = process_ses_response(json.loads(ses_notification_callback(ref='ref2')))
+    assert process_ses_response(json.loads(ses_notification_callback(ref='ref2'))) is None
-    resp3 = process_ses_response(json.loads(ses_notification_callback(ref='ref3')))
+    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'
-
+    assert process_ses_response(json.loads(ses_soft_bounce_callback())) is None
-    _, 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 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()))
+    error = 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'
     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 process_ses_response(json.loads(ses_hard_bounce_callback())) is None
-
-    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)