Update ses callback to interpret hard and soft bounces.

If the notification has a status == sending then update the status otherwise do not update the status.
In other words do not change the status more than once.
This commit is contained in:
Rebecca Law
2016-05-17 15:38:49 +01:00
parent 4762aef1aa
commit 281a93b32d
6 changed files with 212 additions and 62 deletions

View File

@@ -5,10 +5,16 @@ from app.clients import STATISTICS_DELIVERED, STATISTICS_FAILURE
from app.clients.email import (EmailClientException, EmailClient)
ses_response_map = {
'Bounce': {
"message": 'Bounced',
'Permanent': {
"message": 'Hard bounced',
"success": False,
"notification_status": 'failed',
"notification_status": 'permanent-failure',
"notification_statistics_status": STATISTICS_FAILURE
},
'Temporary': {
"message": 'Soft bounced',
"success": False,
"notification_status": 'temporary-failure',
"notification_statistics_status": STATISTICS_FAILURE
},
'Delivery': {

View File

@@ -1,4 +1,4 @@
from sqlalchemy import (desc, func, Integer)
from sqlalchemy import (desc, func, Integer, and_)
from sqlalchemy.sql.expression import cast
from datetime import (
@@ -188,11 +188,9 @@ def update_notification_status_by_id(notification_id, status, notification_stati
def update_notification_status_by_reference(reference, status, notification_statistics_status):
count = db.session.query(Notification).filter_by(
reference=reference
).update({
Notification.status: status
})
count = db.session.query(Notification).filter(Notification.reference == reference,
Notification.status == 'sending').update(
{Notification.status: status})
if count == 1:
notification = Notification.query.filter_by(

View File

@@ -58,6 +58,11 @@ def process_ses_response():
), 400
notification_type = ses_message['notificationType']
if notification_type == 'Bounce':
if ses_message['bounce']['bounceType'] == 'Permanent':
notification_type = ses_message['bounce']['bounceType'] # permanent or not
else:
notification_type = 'Temporary'
try:
aws_response_dict = get_aws_responses(notification_type)
except KeyError:
@@ -87,12 +92,14 @@ def process_ses_response():
notification_status,
notification_statistics_status
) == 0:
message = "SES callback failed: notification either not found or already updated " \
"from sending. Status {}".format(notification_status)
current_app.logger.info(
"SES callback failed: notification not found. Status {}".format(notification_status)
message
)
return jsonify(
result="error",
message="SES callback failed: notification not found. Status {}".format(notification_status)
message=message
), 404
if not aws_response_dict['success']:

View File

@@ -11,10 +11,18 @@ def test_should_return_correct_details_for_delivery():
assert response_dict['success']
def test_should_return_correct_details_for_bounced():
response_dict = get_aws_responses('Bounce')
assert response_dict['message'] == 'Bounced'
assert response_dict['notification_status'] == 'failed'
def test_should_return_correct_details_for_hard_bounced():
response_dict = get_aws_responses('Permanent')
assert response_dict['message'] == 'Hard bounced'
assert response_dict['notification_status'] == 'permanent-failure'
assert response_dict['notification_statistics_status'] == 'failure'
assert not response_dict['success']
def test_should_return_correct_details_for_soft_bounced():
response_dict = get_aws_responses('Temporary')
assert response_dict['message'] == 'Soft bounced'
assert response_dict['notification_status'] == 'temporary-failure'
assert response_dict['notification_statistics_status'] == 'failure'
assert not response_dict['success']

View File

@@ -84,6 +84,33 @@ def test_should_by_able_to_update_status_by_id(sample_notification):
).one().sms_failed == 0
def test_should_not_update_status_one_notification_status_is_delivered(sample_email_template, ses_provider):
data = _notification_json(sample_email_template)
notification = Notification(**data)
dao_create_notification(
notification,
sample_email_template.template_type,
ses_provider.identifier)
assert Notification.query.get(notification.id).status == "sending"
update_notification_reference_by_id(notification.id, 'reference')
update_notification_status_by_reference('reference', 'delivered', 'delivered')
assert Notification.query.get(notification.id).status == 'delivered'
update_notification_status_by_reference('reference', 'delivered', 'temporary-failure')
assert Notification.query.get(notification.id).status == 'delivered'
assert NotificationStatistics.query.filter_by(
service_id=sample_email_template.service.id
).one().emails_delivered == 1
assert NotificationStatistics.query.filter_by(
service_id=sample_email_template.service.id
).one().emails_requested == 1
assert NotificationStatistics.query.filter_by(
service_id=sample_email_template.service.id
).one().emails_failed == 0
def test_should_be_able_to_record_statistics_failure_for_sms(sample_notification):
assert Notification.query.get(sample_notification.id).status == 'sending'
count = update_notification_status_by_id(sample_notification.id, 'delivered', 'failure')

View File

@@ -1163,9 +1163,10 @@ def test_firetext_callback_should_update_notification_status(notify_api, sample_
updated = get_notification_by_id(sample_notification.id)
assert updated.status == 'delivered'
assert get_notification_by_id(sample_notification.id).status == 'delivered'
assert dao_get_notification_statistics_for_service(sample_notification.service_id)[0].sms_delivered == 1
assert dao_get_notification_statistics_for_service(sample_notification.service_id)[0].sms_requested == 1
assert dao_get_notification_statistics_for_service(sample_notification.service_id)[0].sms_failed == 0
stats = dao_get_notification_statistics_for_service(sample_notification.service_id)[0]
assert stats.sms_delivered == 1
assert stats.sms_requested == 1
assert stats.sms_failed == 0
def test_firetext_callback_should_update_notification_status_failed(notify_api, sample_notification):
@@ -1188,9 +1189,10 @@ def test_firetext_callback_should_update_notification_status_failed(notify_api,
sample_notification.id
)
assert get_notification_by_id(sample_notification.id).status == 'failed'
assert dao_get_notification_statistics_for_service(sample_notification.service_id)[0].sms_delivered == 0
assert dao_get_notification_statistics_for_service(sample_notification.service_id)[0].sms_requested == 1
assert dao_get_notification_statistics_for_service(sample_notification.service_id)[0].sms_failed == 1
stats = dao_get_notification_statistics_for_service(sample_notification.service_id)[0]
assert stats.sms_delivered == 0
assert stats.sms_requested == 1
assert stats.sms_failed == 1
def test_firetext_callback_should_update_notification_status_sent(notify_api, notify_db, notify_db_session):
@@ -1214,9 +1216,10 @@ def test_firetext_callback_should_update_notification_status_sent(notify_api, no
notification.id
)
assert get_notification_by_id(notification.id).status == 'delivered'
assert dao_get_notification_statistics_for_service(notification.service_id)[0].sms_delivered == 1
assert dao_get_notification_statistics_for_service(notification.service_id)[0].sms_requested == 1
assert dao_get_notification_statistics_for_service(notification.service_id)[0].sms_failed == 0
stats = dao_get_notification_statistics_for_service(notification.service_id)[0]
assert stats.sms_delivered == 1
assert stats.sms_requested == 1
assert stats.sms_failed == 0
def test_firetext_callback_should_update_multiple_notification_status_sent(notify_api, notify_db, notify_db_session):
@@ -1247,9 +1250,10 @@ def test_firetext_callback_should_update_multiple_notification_status_sent(notif
),
headers=[('Content-Type', 'application/x-www-form-urlencoded')])
assert dao_get_notification_statistics_for_service(notification1.service_id)[0].sms_delivered == 3
assert dao_get_notification_statistics_for_service(notification1.service_id)[0].sms_requested == 3
assert dao_get_notification_statistics_for_service(notification1.service_id)[0].sms_failed == 0
stats = dao_get_notification_statistics_for_service(notification1.service_id)[0]
assert stats.sms_delivered == 3
assert stats.sms_requested == 3
assert stats.sms_failed == 0
def test_process_mmg_response_return_200_when_cid_is_send_sms_code(notify_api):
@@ -1386,7 +1390,7 @@ def test_ses_callback_should_fail_if_notification_cannot_be_found(notify_db, not
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 not found. Status delivered'
assert json_resp['message'] == 'SES callback failed: notification either not found or already updated from sending. Status delivered' # noqa
def test_ses_callback_should_update_notification_status(
@@ -1415,9 +1419,10 @@ def test_ses_callback_should_update_notification_status(
assert json_resp['result'] == 'success'
assert json_resp['message'] == 'SES callback succeeded'
assert get_notification_by_id(notification.id).status == 'delivered'
assert dao_get_notification_statistics_for_service(notification.service_id)[0].emails_delivered == 1
assert dao_get_notification_statistics_for_service(notification.service_id)[0].emails_requested == 1
assert dao_get_notification_statistics_for_service(notification.service_id)[0].emails_failed == 0
stats = dao_get_notification_statistics_for_service(notification.service_id)[0]
assert stats.emails_delivered == 1
assert stats.emails_requested == 1
assert stats.emails_failed == 0
def test_ses_callback_should_update_multiple_notification_status_sent(
@@ -1469,9 +1474,127 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
),
headers=[('Content-Type', 'application/x-www-form-urlencoded')])
assert dao_get_notification_statistics_for_service(notification1.service_id)[0].emails_delivered == 3
assert dao_get_notification_statistics_for_service(notification1.service_id)[0].emails_requested == 3
assert dao_get_notification_statistics_for_service(notification1.service_id)[0].emails_failed == 0
stats = dao_get_notification_statistics_for_service(notification1.service_id)[0]
assert stats.emails_delivered == 3
assert stats.emails_requested == 3
assert stats.emails_failed == 0
def test_ses_callback_should_update_record_statsd(
notify_api,
notify_db,
notify_db_session,
sample_email_template,
mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr')
notification = create_sample_notification(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
)
assert get_notification_by_id(notification.id).status == 'sending'
client.post(
path='/notifications/email/ses',
data=ses_notification_callback(),
headers=[('Content-Type', 'text/plain; charset=UTF-8')]
)
app.statsd_client.incr.assert_called_once_with("notifications.callback.ses.delivered")
def test_ses_callback_should_set_status_to_temporary_failure(notify_api,
notify_db,
notify_db_session,
sample_email_template):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification = create_sample_notification(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
)
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'
assert get_notification_by_id(notification.id).status == 'temporary-failure'
stats = dao_get_notification_statistics_for_service(notification.service_id)[0]
assert stats.emails_delivered == 0
assert stats.emails_requested == 1
assert stats.emails_failed == 1
def test_ses_callback_should_not_set_status_once_status_is_delivered(notify_api,
notify_db,
notify_db_session,
sample_email_template):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification = create_sample_notification(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref',
status='delivered'
)
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' # noqa
assert get_notification_by_id(notification.id).status == 'delivered'
def test_ses_callback_should_set_status_to_permanent_failure(notify_api,
notify_db,
notify_db_session,
sample_email_template):
with notify_api.test_request_context():
with notify_api.test_client() as client:
notification = create_sample_notification(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
)
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'
assert get_notification_by_id(notification.id).status == 'permanent-failure'
stats = dao_get_notification_statistics_for_service(notification.service_id)[0]
assert stats.emails_delivered == 0
assert stats.emails_requested == 1
assert stats.emails_failed == 1
def test_should_handle_invite_email_callbacks(notify_api, notify_db, notify_db_session):
@@ -1551,33 +1674,6 @@ def test_should_record_sms_request_in_statsd(notify_api, notify_db, notify_db_se
app.statsd_client.incr.assert_called_once_with("notifications.api.sms")
def test_ses_callback_should_update_record_statsd(
notify_api,
notify_db,
notify_db_session,
sample_email_template,
mocker):
with notify_api.test_request_context():
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr')
notification = create_sample_notification(
notify_db,
notify_db_session,
template=sample_email_template,
reference='ref'
)
assert get_notification_by_id(notification.id).status == 'sending'
client.post(
path='/notifications/email/ses',
data=ses_notification_callback(),
headers=[('Content-Type', 'text/plain; charset=UTF-8')]
)
app.statsd_client.incr.assert_called_once_with("notifications.callback.ses.delivered")
def test_process_mmg_response_records_statsd(notify_api, sample_notification, mocker):
with notify_api.test_client() as client:
mocker.patch('app.statsd_client.incr')
@@ -1630,4 +1726,12 @@ def ses_missing_notification_id_callback():
def ses_invalid_notification_type_callback():
return b'{\n "Type" : "Notification",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Unknown\\",\\"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\\",\\"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" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa
return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Unknown\\",\\"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\\",\\"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" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa
def ses_hard_bounce_callback():
return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Permanent\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"ref\\",\\"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\\",\\"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" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa
def ses_soft_bounce_callback():
return b'{\n "Type" : "Notification",\n "MessageId" : "ref",\n "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n "Message" : "{\\"notificationType\\":\\"Bounce\\",\\"bounce\\":{\\"bounceType\\":\\"Undetermined\\",\\"bounceSubType\\":\\"General\\"}, \\"mail\\":{\\"messageId\\":\\"ref\\",\\"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\\",\\"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" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}' # noqa