Update rest end point to wrote to the DB not the queue.

We wrote to the queue as a performance optimisation , however dev found it confusing to not have immediate access to the notification as it may be perished some minutes later under periods of load. Additionally we had a couple of DB issues which led to us dropping notifications.

Pushing the DB write to earlier in the flow makes the system a little  more robust in the early days, we may want to change this when the traffic increases.
This commit is contained in:
Martyn Inglis
2016-09-07 13:47:22 +01:00
parent 255ffbd7f6
commit 770b0c2838
2 changed files with 176 additions and 136 deletions

View File

@@ -14,8 +14,9 @@ from notifications_utils.template import Template
from notifications_utils.renderers import PassThrough from notifications_utils.renderers import PassThrough
from app.clients.email.aws_ses import get_aws_responses from app.clients.email.aws_ses import get_aws_responses
from app import api_user, encryption, create_uuid, DATETIME_FORMAT, DATE_FORMAT, statsd_client from app import api_user, encryption, create_uuid, DATETIME_FORMAT, DATE_FORMAT, statsd_client
from app.dao.notifications_dao import dao_create_notification
from app.dao.services_dao import dao_fetch_todays_stats_for_service from app.dao.services_dao import dao_fetch_todays_stats_for_service
from app.models import KEY_TYPE_TEAM, KEY_TYPE_TEST from app.models import KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, KEY_TYPE_NORMAL, EMAIL_TYPE
from app.dao import ( from app.dao import (
templates_dao, templates_dao,
services_dao, services_dao,
@@ -44,8 +45,10 @@ from app.errors import (
register_errors, register_errors,
InvalidRequest InvalidRequest
) )
from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider
register_errors(notifications) register_errors(notifications)
from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider
@notifications.route('/notifications/email/ses', methods=['POST']) @notifications.route('/notifications/email/ses', methods=['POST'])
@@ -194,10 +197,7 @@ def send_notification(notification_type):
service_stats = sum(row.count for row in dao_fetch_todays_stats_for_service(service.id)) service_stats = sum(row.count for row in dao_fetch_todays_stats_for_service(service.id))
if all(( if all((api_user.key_type != KEY_TYPE_TEST, service_stats >= service.message_limit)):
api_user.key_type != KEY_TYPE_TEST,
service_stats >= service.message_limit
)):
error = 'Exceeded send limits ({}) for today'.format(service.message_limit) error = 'Exceeded send limits ({}) for today'.format(service.message_limit)
raise InvalidRequest(error, status_code=429) raise InvalidRequest(error, status_code=429)
@@ -242,14 +242,14 @@ def send_notification(notification_type):
raise InvalidRequest(errors, status_code=400) raise InvalidRequest(errors, status_code=400)
if all(( if all((
api_user.key_type != KEY_TYPE_TEST, api_user.key_type != KEY_TYPE_TEST,
service.restricted or api_user.key_type == KEY_TYPE_TEAM, service.restricted or api_user.key_type == KEY_TYPE_TEAM,
not allowed_to_send_to( not allowed_to_send_to(
notification['to'], notification['to'],
itertools.chain.from_iterable( itertools.chain.from_iterable(
[user.mobile_number, user.email_address] for user in service.users [user.mobile_number, user.email_address] for user in service.users
)
) )
)
)): )):
if (api_user.key_type == KEY_TYPE_TEAM): if (api_user.key_type == KEY_TYPE_TEAM):
message = 'Cant send to this recipient using a team-only API key' message = 'Cant send to this recipient using a team-only API key'
@@ -265,34 +265,15 @@ def send_notification(notification_type):
notification_id = create_uuid() notification_id = create_uuid()
notification.update({"template_version": template.version}) notification.update({"template_version": template.version})
if notification_type == SMS_TYPE: persist_notification(
send_sms.apply_async( service,
( notification_id,
service_id, notification,
notification_id, datetime.utcnow().strftime(DATETIME_FORMAT),
encryption.encrypt(notification), notification_type,
datetime.utcnow().strftime(DATETIME_FORMAT) str(api_user.id),
), api_user.key_type
kwargs={ )
'api_key_id': str(api_user.id),
'key_type': api_user.key_type
},
queue='db-sms'
)
else:
send_email.apply_async(
(
service_id,
notification_id,
encryption.encrypt(notification),
datetime.utcnow().strftime(DATETIME_FORMAT)
),
kwargs={
'api_key_id': str(api_user.id),
'key_type': api_user.key_type
},
queue='db-email'
)
return jsonify( return jsonify(
data=get_notification_return_data( data=get_notification_return_data(
@@ -323,3 +304,28 @@ def get_notification_statistics_for_day():
) )
data, errors = notifications_statistics_schema.dump(statistics, many=True) data, errors = notifications_statistics_schema.dump(statistics, many=True)
return jsonify(data=data), 200 return jsonify(data=data), 200
def persist_notification(
service,
notification_id,
notification,
created_at,
notification_type,
api_key_id=None,
key_type=KEY_TYPE_NORMAL,
):
dao_create_notification(
Notification.from_api_request(
created_at, notification, notification_id, service.id, notification_type, api_key_id, key_type
)
)
if notification_type == SMS_TYPE:
send_sms_to_provider.apply_async((str(service.id), str(notification_id)), queue='send-sms')
if notification_type == EMAIL_TYPE:
send_email_to_provider.apply_async((str(service.id), str(notification_id)), queue='send-email')
current_app.logger.info(
"{} {} created at {}".format(notification_type, notification_id, created_at)
)

View File

@@ -1,3 +1,4 @@
import uuid
from datetime import datetime from datetime import datetime
import random import random
import string import string
@@ -10,7 +11,8 @@ from notifications_python_client.authentication import create_jwt_token
import app import app
from app import encryption from app import encryption
from app.models import ApiKey, KEY_TYPE_TEAM, KEY_TYPE_TEST from app.dao import notifications_dao
from app.models import ApiKey, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification
from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template
from app.dao.services_dao import dao_update_service from app.dao.services_dao import dao_update_service
from app.dao.api_key_dao import save_model_api_key from app.dao.api_key_dao import save_model_api_key
@@ -26,7 +28,7 @@ from tests.app.conftest import (
def test_create_sms_should_reject_if_missing_required_fields(notify_api, sample_api_key, mocker): def test_create_sms_should_reject_if_missing_required_fields(notify_api, sample_api_key, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
data = {} data = {}
auth_header = create_authorization_header(service_id=sample_api_key.service_id) auth_header = create_authorization_header(service_id=sample_api_key.service_id)
@@ -46,7 +48,7 @@ def test_create_sms_should_reject_if_missing_required_fields(notify_api, sample_
def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
data = { data = {
'to': 'invalid', 'to': 'invalid',
@@ -60,7 +62,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker):
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert json_resp['result'] == 'error' assert json_resp['result'] == 'error'
assert len(json_resp['message'].keys()) == 1 assert len(json_resp['message'].keys()) == 1
assert 'Invalid phone number: Must not contain letters or symbols' in json_resp['message']['to'] assert 'Invalid phone number: Must not contain letters or symbols' in json_resp['message']['to']
@@ -70,7 +72,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker):
def test_send_notification_invalid_template_id(notify_api, sample_template, mocker, fake_uuid): def test_send_notification_invalid_template_id(notify_api, sample_template, mocker, fake_uuid):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
data = { data = {
'to': '+447700900855', 'to': '+447700900855',
@@ -84,7 +86,7 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert response.status_code == 404 assert response.status_code == 404
test_string = 'No result found' test_string = 'No result found'
@@ -95,7 +97,7 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock
def test_send_notification_with_placeholders_replaced(notify_api, sample_email_template_with_placeholders, mocker): def test_send_notification_with_placeholders_replaced(notify_api, sample_email_template_with_placeholders, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.tasks.send_email_to_provider.apply_async')
data = { data = {
'to': 'ok@ok.com', 'to': 'ok@ok.com',
@@ -115,16 +117,12 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t
notification_id = response_data['notification']['id'] notification_id = response_data['notification']['id']
data.update({"template_version": sample_email_template_with_placeholders.version}) data.update({"template_version": sample_email_template_with_placeholders.version})
app.celery.tasks.send_email.apply_async.assert_called_once_with( app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
(str(sample_email_template_with_placeholders.service.id), (str(sample_email_template_with_placeholders.service.id),
notification_id, str(notification_id)),
ANY, queue="send-email"
"2016-01-01T11:09:00.061258"),
kwargs=ANY,
queue="db-email"
) )
assert response.status_code == 201 assert response.status_code == 201
assert encryption.decrypt(app.celery.tasks.send_email.apply_async.call_args[0][0][2]) == data
assert response_data['body'] == 'Hello Jo\nThis is an email from GOV.UK' assert response_data['body'] == 'Hello Jo\nThis is an email from GOV.UK'
assert response_data['subject'] == 'Jo' assert response_data['subject'] == 'Jo'
@@ -152,7 +150,7 @@ def test_should_not_send_notification_for_archived_template(notify_api, sample_t
def test_send_notification_with_missing_personalisation(notify_api, sample_template_with_placeholders, mocker): def test_send_notification_with_missing_personalisation(notify_api, sample_template_with_placeholders, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
data = { data = {
'to': '+447700900855', 'to': '+447700900855',
@@ -169,7 +167,7 @@ def test_send_notification_with_missing_personalisation(notify_api, sample_templ
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert 'Missing personalisation: name' in json_resp['message']['template'] assert 'Missing personalisation: name' in json_resp['message']['template']
@@ -180,7 +178,7 @@ def test_send_notification_with_too_much_personalisation_data(
): ):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
data = { data = {
'to': '+447700900855', 'to': '+447700900855',
@@ -197,7 +195,7 @@ def test_send_notification_with_too_much_personalisation_data(
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert 'Personalisation not needed for template: foo' in json_resp['message']['template'] assert 'Personalisation not needed for template: foo' in json_resp['message']['template']
@@ -206,7 +204,7 @@ def test_send_notification_with_too_much_personalisation_data(
def test_should_not_send_sms_if_restricted_and_not_a_service_user(notify_api, sample_template, mocker): def test_should_not_send_sms_if_restricted_and_not_a_service_user(notify_api, sample_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
sample_template.service.restricted = True sample_template.service.restricted = True
dao_update_service(sample_template.service) dao_update_service(sample_template.service)
@@ -224,7 +222,7 @@ def test_should_not_send_sms_if_restricted_and_not_a_service_user(notify_api, sa
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert [( assert [(
@@ -236,7 +234,7 @@ def test_should_not_send_sms_if_restricted_and_not_a_service_user(notify_api, sa
def test_should_send_sms_if_restricted_and_a_service_user(notify_api, sample_template, mocker): def test_should_send_sms_if_restricted_and_a_service_user(notify_api, sample_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
sample_template.service.restricted = True sample_template.service.restricted = True
dao_update_service(sample_template.service) dao_update_service(sample_template.service)
@@ -252,14 +250,14 @@ def test_should_send_sms_if_restricted_and_a_service_user(notify_api, sample_tem
data=json.dumps(data), data=json.dumps(data),
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
assert app.celery.tasks.send_sms.apply_async.called assert app.celery.provider_tasks.send_sms_to_provider.apply_async.called
assert response.status_code == 201 assert response.status_code == 201
def test_should_send_email_if_restricted_and_a_service_user(notify_api, sample_email_template, mocker): def test_should_send_email_if_restricted_and_a_service_user(notify_api, sample_email_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
sample_email_template.service.restricted = True sample_email_template.service.restricted = True
dao_update_service(sample_email_template.service) dao_update_service(sample_email_template.service)
@@ -275,14 +273,14 @@ def test_should_send_email_if_restricted_and_a_service_user(notify_api, sample_e
data=json.dumps(data), data=json.dumps(data),
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
assert app.celery.tasks.send_email.apply_async.called assert app.celery.provider_tasks.send_email_to_provider.apply_async.called
assert response.status_code == 201 assert response.status_code == 201
def test_should_not_allow_template_from_another_service(notify_api, service_factory, sample_user, mocker): def test_should_not_allow_template_from_another_service(notify_api, service_factory, sample_user, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
service_1 = service_factory.get('service 1', user=sample_user, email_from='service.1') service_1 = service_factory.get('service 1', user=sample_user, email_from='service.1')
service_2 = service_factory.get('service 2', user=sample_user, email_from='service.2') service_2 = service_factory.get('service 2', user=sample_user, email_from='service.2')
@@ -301,7 +299,7 @@ def test_should_not_allow_template_from_another_service(notify_api, service_fact
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert response.status_code == 404 assert response.status_code == 404
test_string = 'No result found' test_string = 'No result found'
@@ -325,7 +323,7 @@ def test_should_not_allow_template_content_too_large(
): ):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
template = create_sample_template( template = create_sample_template(
notify_db, notify_db,
notify_db_session, notify_db_session,
@@ -362,7 +360,7 @@ def test_should_not_allow_template_content_too_large(
def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker): def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
data = { data = {
@@ -379,18 +377,9 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
response_data = json.loads(response.data)['data'] response_data = json.loads(response.data)['data']
notification_id = response_data['notification']['id'] notification_id = response_data['notification']['id']
assert app.encryption.encrypt.call_args[0][0]['to'] == '+447700900855'
assert app.encryption.encrypt.call_args[0][0]['template'] == str(sample_template.id)
assert app.encryption.encrypt.call_args[0][0]['template_version'] == sample_template.version
app.celery.tasks.send_sms.apply_async.assert_called_once_with( app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with(
(str(sample_template.service_id), (str(sample_template.service_id), notification_id), queue='send-sms')
notification_id,
"something_encrypted",
"2016-01-01T11:09:00.061258"),
kwargs=ANY,
queue="db-sms"
)
assert response.status_code == 201 assert response.status_code == 201
assert notification_id assert notification_id
assert 'subject' not in response_data assert 'subject' not in response_data
@@ -401,7 +390,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
def test_create_email_should_reject_if_missing_required_fields(notify_api, sample_api_key, mocker): def test_create_email_should_reject_if_missing_required_fields(notify_api, sample_api_key, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
data = {} data = {}
auth_header = create_authorization_header(service_id=sample_api_key.service_id) auth_header = create_authorization_header(service_id=sample_api_key.service_id)
@@ -412,7 +401,7 @@ def test_create_email_should_reject_if_missing_required_fields(notify_api, sampl
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_email.apply_async.assert_not_called() app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called()
assert json_resp['result'] == 'error' assert json_resp['result'] == 'error'
assert 'Missing data for required field.' in json_resp['message']['to'][0] assert 'Missing data for required field.' in json_resp['message']['to'][0]
assert 'Missing data for required field.' in json_resp['message']['template'][0] assert 'Missing data for required field.' in json_resp['message']['template'][0]
@@ -422,7 +411,7 @@ def test_create_email_should_reject_if_missing_required_fields(notify_api, sampl
def test_should_reject_email_notification_with_bad_email(notify_api, sample_email_template, mocker): def test_should_reject_email_notification_with_bad_email(notify_api, sample_email_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
to_address = "bad-email" to_address = "bad-email"
data = { data = {
'to': to_address, 'to': to_address,
@@ -436,7 +425,7 @@ def test_should_reject_email_notification_with_bad_email(notify_api, sample_emai
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
data = json.loads(response.get_data(as_text=True)) data = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_email.apply_async.assert_not_called() app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert data['result'] == 'error' assert data['result'] == 'error'
assert data['message']['to'][0] == 'Not a valid email address' assert data['message']['to'][0] == 'Not a valid email address'
@@ -446,7 +435,7 @@ def test_should_reject_email_notification_with_template_id_that_cant_be_found(
notify_api, sample_email_template, mocker, fake_uuid): notify_api, sample_email_template, mocker, fake_uuid):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
data = { data = {
'to': 'ok@ok.com', 'to': 'ok@ok.com',
'template': fake_uuid 'template': fake_uuid
@@ -459,7 +448,7 @@ def test_should_reject_email_notification_with_template_id_that_cant_be_found(
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
data = json.loads(response.get_data(as_text=True)) data = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_email.apply_async.assert_not_called() app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called()
assert response.status_code == 404 assert response.status_code == 404
assert data['result'] == 'error' assert data['result'] == 'error'
test_string = 'No result found' test_string = 'No result found'
@@ -469,7 +458,7 @@ def test_should_reject_email_notification_with_template_id_that_cant_be_found(
def test_should_not_allow_email_template_from_another_service(notify_api, service_factory, sample_user, mocker): def test_should_not_allow_email_template_from_another_service(notify_api, service_factory, sample_user, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
service_1 = service_factory.get('service 1', template_type='email', user=sample_user, service_1 = service_factory.get('service 1', template_type='email', user=sample_user,
email_from='service.1') email_from='service.1')
@@ -491,7 +480,7 @@ def test_should_not_allow_email_template_from_another_service(notify_api, servic
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_email.apply_async.assert_not_called() app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called()
assert response.status_code == 404 assert response.status_code == 404
test_string = 'No result found' test_string = 'No result found'
@@ -501,7 +490,7 @@ def test_should_not_allow_email_template_from_another_service(notify_api, servic
def test_should_not_send_email_if_restricted_and_not_a_service_user(notify_api, sample_email_template, mocker): def test_should_not_send_email_if_restricted_and_not_a_service_user(notify_api, sample_email_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
sample_email_template.service.restricted = True sample_email_template.service.restricted = True
dao_update_service(sample_email_template.service) dao_update_service(sample_email_template.service)
@@ -519,7 +508,7 @@ def test_should_not_send_email_if_restricted_and_not_a_service_user(notify_api,
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_email.apply_async.assert_not_called() app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert [( assert [(
@@ -532,7 +521,7 @@ def test_should_not_send_email_if_restricted_and_not_a_service_user(notify_api,
def test_should_allow_valid_email_notification(notify_api, sample_email_template, mocker): def test_should_allow_valid_email_notification(notify_api, sample_email_template, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
data = { data = {
@@ -549,16 +538,9 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template
assert response.status_code == 201 assert response.status_code == 201
response_data = json.loads(response.get_data(as_text=True))['data'] response_data = json.loads(response.get_data(as_text=True))['data']
notification_id = response_data['notification']['id'] notification_id = response_data['notification']['id']
assert app.encryption.encrypt.call_args[0][0]['to'] == 'ok@ok.com' app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
assert app.encryption.encrypt.call_args[0][0]['template'] == str(sample_email_template.id) (str(sample_email_template.service_id), notification_id),
assert app.encryption.encrypt.call_args[0][0]['template_version'] == sample_email_template.version queue="send-email"
app.celery.tasks.send_email.apply_async.assert_called_once_with(
(str(sample_email_template.service_id),
notification_id,
"something_encrypted",
"2016-01-01T11:09:00.061258"),
kwargs=ANY,
queue="db-email"
) )
assert response.status_code == 201 assert response.status_code == 201
@@ -577,7 +559,7 @@ def test_should_block_api_call_if_over_day_limit_for_restricted_and_live_service
restricted): restricted):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=restricted) service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=restricted)
@@ -607,7 +589,7 @@ def test_should_block_api_call_if_over_day_limit_for_restricted_and_live_service
def test_should_block_api_call_if_over_day_limit_regardless_of_type(notify_db, notify_db_session, notify_api, mocker): def test_should_block_api_call_if_over_day_limit_regardless_of_type(notify_db, notify_db_session, notify_api, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=True) service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=True)
@@ -637,7 +619,7 @@ def test_should_block_api_call_if_over_day_limit_regardless_of_type(notify_db, n
def test_should_allow_api_call_if_under_day_limit_regardless_of_type(notify_db, notify_db_session, notify_api, mocker): def test_should_allow_api_call_if_under_day_limit_regardless_of_type(notify_db, notify_db_session, notify_api, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
mocker.patch('app.encryption.encrypt', return_value="something_encrypted") mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
service = create_sample_service(notify_db, notify_db_session, limit=2) service = create_sample_service(notify_db, notify_db_session, limit=2)
@@ -663,7 +645,7 @@ def test_should_allow_api_call_if_under_day_limit_regardless_of_type(notify_db,
def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session, mocker): def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session, mocker):
with notify_api.test_request_context(): with notify_api.test_request_context():
with notify_api.test_client() as client: with notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
email_template = create_sample_email_template(notify_db, notify_db.session, content='hello\nthere') email_template = create_sample_email_template(notify_db, notify_db.session, content='hello\nthere')
data = { data = {
@@ -683,7 +665,7 @@ def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session
def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api, sample_email_template, mocker): def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api, sample_email_template, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client: with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
data = { data = {
'to': "not-someone-we-trust@email-address.com", 'to': "not-someone-we-trust@email-address.com",
@@ -701,7 +683,7 @@ def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_email.apply_async.assert_not_called() app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert [ assert [
@@ -711,7 +693,7 @@ def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api
def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api, sample_template, mocker): def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api, sample_template, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client: with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
data = { data = {
'to': '07123123123', 'to': '07123123123',
@@ -726,7 +708,7 @@ def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api,
headers=[('Content-Type', 'application/json'), auth_header]) headers=[('Content-Type', 'application/json'), auth_header])
json_resp = json.loads(response.get_data(as_text=True)) json_resp = json.loads(response.get_data(as_text=True))
app.celery.tasks.send_sms.apply_async.assert_not_called() app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called()
assert response.status_code == 400 assert response.status_code == 400
assert [ assert [
@@ -734,9 +716,10 @@ def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api,
] == json_resp['message']['to'] ] == json_resp['message']['to']
def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample_email_template, mocker): def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample_email_template, fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client: with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
data = { data = {
'to': sample_email_template.service.created_by.email_address, 'to': sample_email_template.service.created_by.email_address,
@@ -754,23 +737,20 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample
data=json.dumps(data), data=json.dumps(data),
headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
app.celery.tasks.send_email.apply_async.assert_called_once_with( app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
ANY, (str(sample_email_template.service.id), fake_uuid),
kwargs={ queue='send-email')
'api_key_id': str(api_key.id),
'key_type': api_key.key_type
},
queue='db-email')
assert response.status_code == 201 assert response.status_code == 201
@pytest.mark.parametrize('restricted', [True, False]) @pytest.mark.parametrize('restricted', [True, False])
@pytest.mark.parametrize('limit', [0, 1]) @pytest.mark.parametrize('limit', [0, 1])
def test_should_send_email_to_anyone_with_test_key( def test_should_send_email_to_anyone_with_test_key(
notify_api, sample_email_template, mocker, restricted, limit notify_api, sample_email_template, mocker, restricted, limit, fake_uuid
): ):
with notify_api.test_request_context(), notify_api.test_client() as client: with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
data = { data = {
'to': 'anyone123@example.com', 'to': 'anyone123@example.com',
@@ -793,20 +773,15 @@ def test_should_send_email_to_anyone_with_test_key(
headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]
) )
app.celery.tasks.send_email.apply_async.assert_called_once_with( app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
ANY, (str(sample_email_template.service.id), fake_uuid), queue='send-email')
kwargs={
'api_key_id': str(api_key.id),
'key_type': api_key.key_type
},
queue='db-email'
)
assert response.status_code == 201 assert response.status_code == 201
def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_template, mocker): def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_template, fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client: with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.tasks.send_sms.apply_async') mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
data = { data = {
'to': sample_template.service.created_by.mobile_number, 'to': sample_template.service.created_by.mobile_number,
@@ -824,11 +799,70 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t
data=json.dumps(data), data=json.dumps(data),
headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
app.celery.tasks.send_sms.apply_async.assert_called_once_with( app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with(
ANY, (str(sample_template.service.id), fake_uuid), queue='send-sms')
kwargs={
'api_key_id': str(api_key.id),
'key_type': api_key.key_type
},
queue='db-sms')
assert response.status_code == 201 assert response.status_code == 201
def test_should_persist_sms_notification(notify_api, sample_template, fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
data = {
'to': sample_template.service.created_by.mobile_number,
'template': sample_template.id
}
api_key = ApiKey(
service=sample_template.service,
name='team_key',
created_by=sample_template.created_by,
key_type=KEY_TYPE_TEAM)
save_model_api_key(api_key)
auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id))
response = client.post(
path='/notifications/sms',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with(
(str(sample_template.service.id), fake_uuid), queue='send-sms')
assert response.status_code == 201
notification = notifications_dao.get_notification_by_id(fake_uuid)
assert notification.to == sample_template.service.created_by.mobile_number
assert notification.template_id == sample_template.id
assert notification.notification_type == 'sms'
def test_should_persist_email_notification(notify_api, sample_email_template, fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
data = {
'to': sample_email_template.service.created_by.email_address,
'template': sample_email_template.id
}
api_key = ApiKey(
service=sample_email_template.service,
name='team_key',
created_by=sample_email_template.created_by,
key_type=KEY_TYPE_TEAM)
save_model_api_key(api_key)
auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id))
response = client.post(
path='/notifications/email',
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))])
app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
(str(sample_email_template.service.id), fake_uuid), queue='send-email')
assert response.status_code == 201
notification = notifications_dao.get_notification_by_id(fake_uuid)
assert notification.to == sample_email_template.service.created_by.email_address
assert notification.template_id == sample_email_template.id
assert notification.notification_type == 'email'