From 25d17779375f922ff060cdc3d923cd8fd4334681 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 28 Mar 2017 13:14:46 +0100 Subject: [PATCH] ensure we're passing through api keys and key types from notifications when we made the change to async persist notifications, we forgot to pass through api_key_id and key_type. in send_sms/email, for legacy reasons, they default to None/KEY_TYPE_NORMAL, so regardless of what your api key was set up as, we would send real messages! TODO: Once the PaaS transition is complete and the task changes are reverted, remove the api_key_id and key_type params from the send_* tasks entirely, as those are only called from the csv job flow, and don't need them --- app/celery/tasks.py | 15 +++++- app/notifications/rest.py | 14 ++--- app/v2/notifications/post_notifications.py | 14 ++--- tests/app/celery/test_tasks.py | 62 +++++++++++++++++++--- 4 files changed, 85 insertions(+), 20 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9b4c2490a..86c13156d 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -124,7 +124,14 @@ def process_row(row_number, recipient, personalisation, template, job, service): def send_notification_to_persist_queue( - notification_id, service, template_type, encrypted, priority=False, research_mode=False + notification_id, + service, + template_type, + encrypted, + api_key_id, + key_type, + priority=False, + research_mode=False ): queues = { SMS_TYPE: 'db-sms', @@ -150,8 +157,12 @@ def send_notification_to_persist_queue( str(service.id), notification_id, encrypted, - datetime.utcnow().strftime(DATETIME_FORMAT) + datetime.utcnow().strftime(DATETIME_FORMAT), ), + kwargs={ + 'api_key_id': api_key_id, + 'key_type': key_type + }, queue=queue_name ) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index dd707b588..fb321d2fd 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -147,12 +147,14 @@ def send_notification(notification_type): if not simulated: tasks.send_notification_to_persist_queue( - notification_model.id, - service, - template.template_type, - encrypted, - template.process_type == PRIORITY, - service.research_mode or api_user.key_type == KEY_TYPE_TEST + notification_id=notification_model.id, + service=service, + template_type=template.template_type, + encrypted=encrypted, + api_key_id=str(notification_model.api_key_id), + key_type=api_user.key_type, + priority=template.process_type == PRIORITY, + research_mode=service.research_mode or api_user.key_type == KEY_TYPE_TEST ) # queue_name = 'notify' if template.process_type == PRIORITY else None # send_notification_to_queue(notification=notification_model, diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 4b78d441d..0335c7ff9 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -68,12 +68,14 @@ def post_notification(notification_type): if not simulated: tasks.send_notification_to_persist_queue( - notification.id, - service, - template.template_type, - encrypted, - template.process_type == PRIORITY, - service.research_mode or api_user.key_type == KEY_TYPE_TEST + notification_id=notification.id, + service=service, + template_type=template.template_type, + encrypted=encrypted, + api_key_id=str(notification.api_key_id), + key_type=api_user.key_type, + priority=template.process_type == PRIORITY, + research_mode=service.research_mode or api_user.key_type == KEY_TYPE_TEST ) # not doing this during paas migration # queue_name = 'notify' if template.process_type == PRIORITY else None diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 01b80c3fb..05088481c 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1,6 +1,6 @@ import uuid from datetime import datetime -from unittest.mock import Mock +from unittest.mock import Mock, ANY import pytest from flask import current_app @@ -65,17 +65,22 @@ def test_should_have_decorated_tasks_functions(): def email_job_with_placeholders(notify_db, notify_db_session, sample_email_template_with_placeholders): return sample_job(notify_db, notify_db_session, template=sample_email_template_with_placeholders) +# --- send_notification_to_persist_queue tests -- # + @freeze_time("2016-01-01 11:09:00.061258") def test_should_put_sms_messages_in_db_sms_queue(sample_service, sample_template, mocker): mocker.patch('app.celery.tasks.send_sms.apply_async') - send_notification_to_persist_queue("notification-id", sample_service, sample_template.template_type, "encrypted") + send_notification_to_persist_queue( + 'notification-id', sample_service, sample_template.template_type, 'encrypted', 'api_key_id', 'key_type' + ) tasks.send_sms.apply_async.assert_called_once_with( (str(sample_service.id), "notification-id", "encrypted", "2016-01-01T11:09:00.061258Z"), + kwargs=ANY, queue="db-sms" ) @@ -84,7 +89,8 @@ def test_should_put_sms_messages_in_db_sms_queue(sample_service, sample_template def test_should_put_messages_in_priority_queue(sample_service, sample_template, mocker): mocker.patch('app.celery.tasks.send_sms.apply_async') send_notification_to_persist_queue( - "notification-id", sample_service, sample_template.template_type, "encrypted", True + 'notification-id', sample_service, sample_template.template_type, 'encrypted', 'api_key_id', 'key_type', + priority=True ) tasks.send_sms.apply_async.assert_called_once_with( @@ -92,15 +98,47 @@ def test_should_put_messages_in_priority_queue(sample_service, sample_template, "notification-id", "encrypted", "2016-01-01T11:09:00.061258Z"), + kwargs=ANY, queue="notify" ) +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_pass_through_api_key_info(sample_service, sample_template, mocker): + mocker.patch('app.celery.tasks.send_sms.apply_async') + api_key_id = 'foo' + key_type = 'bar' + send_notification_to_persist_queue( + "notification-id", + sample_service, + sample_template.template_type, + "encrypted", + api_key_id, + key_type + ) + + tasks.send_sms.apply_async.assert_called_once_with( + ANY, + kwargs={ + 'api_key_id': api_key_id, + 'key_type': key_type + }, + queue=ANY + ) + + @freeze_time("2016-01-01 11:09:00.061258") def test_should_put_messages_in_research_mode_queue(sample_service, sample_template, mocker): mocker.patch('app.celery.tasks.send_sms.apply_async') send_notification_to_persist_queue( - "notification-id", sample_service, sample_template.template_type, "encrypted", False, True + "notification-id", + sample_service, + sample_template.template_type, + "encrypted", + 'api_key_id', + 'key_type', + priority=False, + research_mode=True ) tasks.send_sms.apply_async.assert_called_once_with( @@ -108,6 +146,7 @@ def test_should_put_messages_in_research_mode_queue(sample_service, sample_templ "notification-id", "encrypted", "2016-01-01T11:09:00.061258Z"), + kwargs=ANY, queue="research-mode" ) @@ -116,7 +155,14 @@ def test_should_put_messages_in_research_mode_queue(sample_service, sample_templ def test_should_put_messages_in_research_mode_queue_overriding_priority_mode(sample_service, sample_template, mocker): mocker.patch('app.celery.tasks.send_sms.apply_async') send_notification_to_persist_queue( - "notification-id", sample_service, sample_template.template_type, "encrypted", True, True + "notification-id", + sample_service, + sample_template.template_type, + "encrypted", + 'api_key_id', + 'key_type', + priority=True, + research_mode=True ) tasks.send_sms.apply_async.assert_called_once_with( @@ -124,6 +170,7 @@ def test_should_put_messages_in_research_mode_queue_overriding_priority_mode(sam "notification-id", "encrypted", "2016-01-01T11:09:00.061258Z"), + kwargs=ANY, queue="research-mode" ) @@ -132,7 +179,7 @@ def test_should_put_messages_in_research_mode_queue_overriding_priority_mode(sam def test_should_put_email_messages_in_db_email_queue(sample_service, sample_email_template, mocker): mocker.patch('app.celery.tasks.send_email.apply_async') send_notification_to_persist_queue( - "notification-id", sample_service, sample_email_template.template_type, "encrypted" + 'notification-id', sample_service, sample_email_template.template_type, 'encrypted', 'api_key_id', 'key_type' ) tasks.send_email.apply_async.assert_called_once_with( @@ -140,6 +187,7 @@ def test_should_put_email_messages_in_db_email_queue(sample_service, sample_emai "notification-id", "encrypted", "2016-01-01T11:09:00.061258Z"), + kwargs=ANY, queue="db-email" ) @@ -454,6 +502,8 @@ def test_process_row_sends_letter_task(template_type, research_mode, expected_fu ), queue=expected_queue ) + + # -------- send_sms and send_email tests -------- #