diff --git a/app/__init__.py b/app/__init__.py index 7b7da7cfe..17c81d24d 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -14,6 +14,7 @@ from app.clients.email.aws_ses import AwsSesClient from app.encryption import Encryption DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%f" +DATE_FORMAT = "%Y-%m-%d" db = SQLAlchemy() ma = Marshmallow() diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9d019fd84..b3f090127 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -1,4 +1,4 @@ -from app import create_uuid, DATETIME_FORMAT +from app import create_uuid, DATETIME_FORMAT, DATE_FORMAT from app import notify_celery, encryption, firetext_client, aws_ses_client from app.clients.email.aws_ses import AwsSesClientException from app.clients.sms.firetext import FiretextClientException @@ -8,7 +8,8 @@ from app.dao.notifications_dao import ( dao_create_notification, dao_update_notification, delete_failed_notifications_created_more_than_a_week_ago, - delete_successful_notifications_created_more_than_a_day_ago + delete_successful_notifications_created_more_than_a_day_ago, + dao_get_notification_statistics_for_service_and_day ) from app.dao.jobs_dao import dao_update_job, dao_get_job_by_id from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago @@ -90,6 +91,27 @@ def delete_invitations(): def process_job(job_id): start = datetime.utcnow() job = dao_get_job_by_id(job_id) + + service = job.service + + stats = dao_get_notification_statistics_for_service_and_day( + service_id=service.id, + day=job.created_at.strftime(DATE_FORMAT) + ) + + total_sent = 0 + if stats: + total_sent = stats.emails_requested + stats.sms_requested + + if total_sent + job.notification_count > service.limit: + job.status = 'sending limits exceeded' + job.processing_finished = datetime.utcnow() + dao_update_job(job) + current_app.logger.info( + "Job {} size {} error. Sending limits {} exceeded".format(job_id, job.notification_count, service.limit) + ) + return + job.status = 'in progress' dao_update_job(job) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 378316b80..95c3fb54d 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -11,6 +11,13 @@ def dao_get_notification_statistics_for_service(service_id): ).order_by(desc(NotificationStatistics.day)).all() +def dao_get_notification_statistics_for_service_and_day(service_id, day): + return NotificationStatistics.query.filter_by( + service_id=service_id, + day=day + ).order_by(desc(NotificationStatistics.day)).first() + + def dao_create_notification(notification, notification_type): try: if notification.job_id: diff --git a/app/models.py b/app/models.py index 1c2a10d3b..d4784d489 100644 --- a/app/models.py +++ b/app/models.py @@ -157,7 +157,7 @@ class Template(db.Model): subject = db.Column(db.Text, index=False, unique=True, nullable=True) -JOB_STATUS_TYPES = ['pending', 'in progress', 'finished'] +JOB_STATUS_TYPES = ['pending', 'in progress', 'finished', 'sending limits exceeded'] class Job(db.Model): diff --git a/app/notifications/rest.py b/app/notifications/rest.py index c50d2cdae..438d6fce0 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -8,9 +8,9 @@ from flask import ( url_for ) -from utils.template import Template, NeededByTemplateError, NoPlaceholderForDataError +from utils.template import Template -from app import api_user, encryption, create_uuid, DATETIME_FORMAT +from app import api_user, encryption, create_uuid, DATETIME_FORMAT, DATE_FORMAT from app.authentication.auth import require_admin from app.dao import ( templates_dao, @@ -127,6 +127,21 @@ def send_notification(notification_type): assert False service_id = api_user['client'] + service = services_dao.dao_fetch_service_by_id(api_user['client']) + + service_stats = notifications_dao.dao_get_notification_statistics_for_service_and_day( + service_id, + datetime.utcnow().strftime(DATE_FORMAT) + ) + + print(service_stats) + + if service_stats: + total_sms_count = service_stats.sms_requested + total_email_count = service_stats.emails_requested + + if total_email_count + total_sms_count >= service.limit: + return jsonify(result="error", message='Exceeded send limits ({}) for today'.format(service.limit)), 429 notification, errors = ( sms_template_notification_schema if notification_type == 'sms' else email_notification_schema @@ -167,7 +182,6 @@ def send_notification(notification_type): } ), 400 - service = services_dao.dao_fetch_service_by_id(api_user['client']) notification_id = create_uuid() if notification_type == 'sms': diff --git a/app/service/rest.py b/app/service/rest.py index 20aaa6e33..4ded0168c 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -28,9 +28,7 @@ from app.models import ApiKey from app.schemas import ( service_schema, api_key_schema, - user_schema, - permission_schema, - invited_user_schema + user_schema ) from app.errors import register_errors diff --git a/migrations/versions/0037_more_job_states.py b/migrations/versions/0037_more_job_states.py new file mode 100644 index 000000000..ef6e5671b --- /dev/null +++ b/migrations/versions/0037_more_job_states.py @@ -0,0 +1,30 @@ +"""empty message + +Revision ID: 0037_more_job_states +Revises: 0036_notification_stats +Create Date: 2016-03-08 11:16:25.659463 + +""" + +# revision identifiers, used by Alembic. +revision = '0037_more_job_states' +down_revision = '0036_notification_stats' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + op.drop_column('jobs', 'status') + op.execute('DROP TYPE job_status_types') + job_status_types = sa.Enum('pending', 'in progress', 'finished', 'sending limits exceeded', name='job_status_types') + job_status_types.create(op.get_bind()) + op.add_column('jobs', sa.Column('status', job_status_types, nullable=True)) + op.get_bind() + op.execute("update jobs set status='finished'") + op.alter_column('jobs', 'status', nullable=False) + + +def downgrade(): + op.drop_column('jobs', 'status') + op.execute('DROP TYPE job_status_types') diff --git a/migrations/versions/0038_reduce_limits.py b/migrations/versions/0038_reduce_limits.py new file mode 100644 index 000000000..e9d9fd1f3 --- /dev/null +++ b/migrations/versions/0038_reduce_limits.py @@ -0,0 +1,21 @@ +"""empty message + +Revision ID: 0038_reduce_limits +Revises: 0037_more_job_states +Create Date: 2016-03-08 11:16:25.659463 + +""" + +# revision identifiers, used by Alembic. +revision = '0038_reduce_limits' +down_revision = '0037_more_job_states' + +from alembic import op + + +def upgrade(): + op.execute('update services set "limit" = 50') + + +def downgrade(): + pass \ No newline at end of file diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 54fa72998..bb2a7bb3b 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -28,7 +28,10 @@ from freezegun import freeze_time from tests.app.conftest import ( sample_service, sample_user, - sample_template + sample_template, + sample_job, + sample_email_template, + sample_notification ) @@ -79,7 +82,110 @@ def test_should_process_sms_job(sample_job, mocker): assert job.status == 'finished' -def test_should_not_create_send_task_for_empty_file(sample_job, sample_template, mocker): +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_not_process_sms_job_if_would_exceed_send_limits(notify_db, notify_db_session, mocker): + service = sample_service(notify_db, notify_db_session, limit=9) + job = sample_job(notify_db, notify_db_session, service=service, notification_count=10) + + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) + mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + process_job(job.id) + + s3.get_job_from_s3.assert_not_called() + job = jobs_dao.dao_get_job_by_id(job.id) + assert job.status == 'sending limits exceeded' + tasks.send_sms.apply_async.assert_not_called() + + +def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify_db, notify_db_session, mocker): + service = sample_service(notify_db, notify_db_session, limit=1) + job = sample_job(notify_db, notify_db_session, service=service) + + sample_notification(notify_db, notify_db_session, service=service, job=job) + + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) + mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + process_job(job.id) + + job = jobs_dao.dao_get_job_by_id(job.id) + assert job.status == 'sending limits exceeded' + s3.get_job_from_s3.assert_not_called() + tasks.send_sms.apply_async.assert_not_called() + + +def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(notify_db, notify_db_session, mocker): + service = sample_service(notify_db, notify_db_session, limit=1) + template = sample_email_template(notify_db, notify_db_session, service=service) + job = sample_job(notify_db, notify_db_session, service=service, template=template) + + sample_notification(notify_db, notify_db_session, service=service, job=job) + + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email')) + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + process_job(job.id) + + job = jobs_dao.dao_get_job_by_id(job.id) + assert job.status == 'sending limits exceeded' + s3.get_job_from_s3.assert_not_called() + tasks.send_email.apply_async.assert_not_called() + + +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_not_process_email_job_if_would_exceed_send_limits(notify_db, notify_db_session, mocker): + service = sample_service(notify_db, notify_db_session, limit=0) + template = sample_email_template(notify_db, notify_db_session, service=service) + job = sample_job(notify_db, notify_db_session, service=service, template=template) + + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email')) + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + process_job(job.id) + + s3.get_job_from_s3.assert_not_called + job = jobs_dao.dao_get_job_by_id(job.id) + assert job.status == 'sending limits exceeded' + tasks.send_email.apply_async.assert_not_called + + +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_process_sms_job_if_exactly_on_send_limits(notify_db, notify_db_session, mocker): + service = sample_service(notify_db, notify_db_session, limit=10) + template = sample_email_template(notify_db, notify_db_session, service=service) + job = sample_job(notify_db, notify_db_session, service=service, template=template, notification_count=10) + + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_email')) + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + process_job(job.id) + + s3.get_job_from_s3.assert_called_once_with(job.bucket_name, job.id) + job = jobs_dao.dao_get_job_by_id(job.id) + assert job.status == 'finished' + tasks.send_email.apply_async.assert_called_with( + (str(job.service_id), + "uuid", + job.template.subject, + "{}@{}".format(job.service.email_from, "test.notify.com"), + "something_encrypted", + "2016-01-01T11:09:00.061258"), + queue="bulk-email" + ) + + +def test_should_not_create_send_task_for_empty_file(sample_job, mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('empty')) mocker.patch('app.celery.tasks.send_sms.apply_async') @@ -92,7 +198,7 @@ def test_should_not_create_send_task_for_empty_file(sample_job, sample_template, @freeze_time("2016-01-01 11:09:00.061258") -def test_should_process_email_job(sample_email_job, sample_template, mocker): +def test_should_process_email_job(sample_email_job, mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email')) mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 57a1b8a7e..ca637ae0b 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -101,12 +101,13 @@ def sample_service(notify_db, notify_db_session, service_name="Sample service", user=None, - restricted=False): + restricted=False, + limit=1000): if user is None: user = sample_user(notify_db, notify_db_session) data = { 'name': service_name, - 'limit': 1000, + 'limit': limit, 'active': False, 'restricted': restricted, 'email_from': email_safe(service_name) @@ -197,7 +198,8 @@ def sample_api_key(notify_db, def sample_job(notify_db, notify_db_session, service=None, - template=None): + template=None, + notification_count=1): if service is None: service = sample_service(notify_db, notify_db_session) if template is None: @@ -213,7 +215,7 @@ def sample_job(notify_db, 'bucket_name': bucket_name, 'file_name': file_name, 'original_file_name': 'some.csv', - 'notification_count': 1 + 'notification_count': notification_count } job = Job(**data) dao_create_job(job) diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 5a6cec591..e3c32e05c 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -1,5 +1,8 @@ import pytest import uuid +from app import ( + DATE_FORMAT +) from freezegun import freeze_time from sqlalchemy.exc import SQLAlchemyError, IntegrityError from app.models import Notification, Job, NotificationStatistics @@ -12,7 +15,8 @@ from app.dao.notifications_dao import ( get_notifications_for_job, dao_get_notification_statistics_for_service, delete_successful_notifications_created_more_than_a_day_ago, - delete_failed_notifications_created_more_than_a_week_ago + delete_failed_notifications_created_more_than_a_week_ago, + dao_get_notification_statistics_for_service_and_day ) from tests.app.conftest import sample_job from tests.app.conftest import sample_notification @@ -34,10 +38,48 @@ def test_should_be_able_to_get_statistics_for_a_service(sample_template): assert len(stats) == 1 assert stats[0].emails_requested == 0 assert stats[0].sms_requested == 1 - assert stats[0].day == notification.created_at.strftime('%Y-%m-%d') + assert stats[0].day == notification.created_at.strftime(DATE_FORMAT) assert stats[0].service_id == notification.service_id +def test_should_be_able_to_get_statistics_for_a_service_for_a_day(sample_template): + now = datetime.utcnow() + data = { + 'to': '+44709123456', + 'service': sample_template.service, + 'service_id': sample_template.service.id, + 'template': sample_template, + 'created_at': now + } + + notification = Notification(**data) + dao_create_notification(notification, sample_template.template_type) + stat = dao_get_notification_statistics_for_service_and_day( + sample_template.service.id, now.strftime(DATE_FORMAT) + ) + assert stat.emails_requested == 0 + assert stat.sms_requested == 1 + assert stat.day == notification.created_at.strftime(DATE_FORMAT) + assert stat.service_id == notification.service_id + + +def test_should_return_none_if_no_statistics_for_a_service_for_a_day(sample_template): + now = datetime.utcnow() + data = { + 'to': '+44709123456', + 'service': sample_template.service, + 'service_id': sample_template.service.id, + 'template': sample_template, + 'created_at': now + } + + notification = Notification(**data) + dao_create_notification(notification, sample_template.template_type) + assert not dao_get_notification_statistics_for_service_and_day( + sample_template.service.id, (datetime.utcnow() - timedelta(days=1)).strftime(DATE_FORMAT) + ) + + def test_should_be_able_to_get_all_statistics_for_a_service(sample_template): data = { 'to': '+44709123456', @@ -91,13 +133,13 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days(sam assert len(stats) == 3 assert stats[0].emails_requested == 0 assert stats[0].sms_requested == 1 - assert stats[0].day == today.strftime('%Y-%m-%d') + assert stats[0].day == today.strftime(DATE_FORMAT) assert stats[1].emails_requested == 0 assert stats[1].sms_requested == 1 - assert stats[1].day == yesterday.strftime('%Y-%m-%d') + assert stats[1].day == yesterday.strftime(DATE_FORMAT) assert stats[2].emails_requested == 0 assert stats[2].sms_requested == 1 - assert stats[2].day == two_days_ago.strftime('%Y-%m-%d') + assert stats[2].day == two_days_ago.strftime(DATE_FORMAT) def test_should_be_empty_list_if_no_statistics_for_a_service(sample_service): diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index d7bbbae09..cdc7c1957 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -2,7 +2,7 @@ from datetime import datetime import uuid import app.celery.tasks from tests import create_authorization_header -from tests.app.conftest import sample_notification, sample_job, sample_service +from tests.app.conftest import sample_notification, sample_job, sample_service, sample_email_template, sample_template from flask import json from app.models import Service from app.dao.templates_dao import dao_get_all_templates_for_service @@ -96,7 +96,6 @@ def test_get_all_notifications_newest_first(notify_api, notify_db, notify_db_ses def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: - service_1 = sample_service(notify_db, notify_db_session, service_name="1") service_2 = sample_service(notify_db, notify_db_session, service_name="2") @@ -288,6 +287,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): 'template': sample_template.id } auth_header = create_authorization_header( + service_id=sample_template.service.id, request_body=json.dumps(data), path='/notifications/sms', method='POST') @@ -299,7 +299,6 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): json_resp = json.loads(response.get_data(as_text=True)) app.celery.tasks.send_sms.apply_async.assert_not_called() - assert json_resp['result'] == 'error' assert len(json_resp['message'].keys()) == 1 assert 'Invalid phone number: Must not contain letters or symbols' in json_resp['message']['to'] @@ -371,9 +370,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_templat assert response.status_code == 201 -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_client() as client: mocker.patch('app.celery.tasks.send_sms.apply_async') @@ -404,7 +401,7 @@ def test_send_notification_with_missing_personalisation( def test_send_notification_with_too_much_personalisation_data( - notify_api, sample_template_with_placeholders, mocker + notify_api, sample_template_with_placeholders, mocker ): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -759,3 +756,105 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template ) assert response.status_code == 201 assert notification_id + + +@freeze_time("2016-01-01 12:00:00.061258") +def test_should_block_api_call_if_over_day_limit(notify_db, notify_db_session, notify_api, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + + service = sample_service(notify_db, notify_db_session, limit=1) + email_template = sample_email_template(notify_db, notify_db_session, service=service) + sample_notification( + notify_db, notify_db_session, template=email_template, service=service, created_at=datetime.utcnow() + ) + + data = { + 'to': 'ok@ok.com', + 'template': email_template.id + } + + auth_header = create_authorization_header( + request_body=json.dumps(data), + path='/notifications/email', + method='POST', + service_id=service.id + ) + + response = client.post( + path='/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + json_resp = json.loads(response.get_data(as_text=True)) + + assert response.status_code == 429 + assert 'Exceeded send limits (1) for today' in json_resp['message'] + + +@freeze_time("2016-01-01 12:00:00.061258") +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_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + + service = sample_service(notify_db, notify_db_session, limit=1) + email_template = sample_email_template(notify_db, notify_db_session, service=service) + sms_template = sample_template(notify_db, notify_db_session, service=service) + sample_notification( + notify_db, notify_db_session, template=email_template, service=service, created_at=datetime.utcnow() + ) + + data = { + 'to': '+447234123123', + 'template': sms_template.id + } + + auth_header = create_authorization_header( + request_body=json.dumps(data), + path='/notifications/sms', + method='POST', + service_id=service.id + ) + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 429 + assert 'Exceeded send limits (1) for today' in json_resp['message'] + + +@freeze_time("2016-01-01 12:00:00.061258") +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_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + + service = sample_service(notify_db, notify_db_session, limit=2) + email_template = sample_email_template(notify_db, notify_db_session, service=service) + sms_template = sample_template(notify_db, notify_db_session, service=service) + sample_notification(notify_db, notify_db_session, template=email_template, service=service) + + data = { + 'to': '+447634123123', + 'template': sms_template.id + } + + auth_header = create_authorization_header( + request_body=json.dumps(data), + path='/notifications/sms', + method='POST', + service_id=service.id + ) + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 201