diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 457877ed0..6a9cfd055 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -9,6 +9,7 @@ from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from app import notify_celery from app import performance_platform_client +from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago from app.dao.jobs_dao import dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than_limited_by from app.dao.notifications_dao import ( @@ -226,3 +227,21 @@ def timeout_job_statistics(): if updated: current_app.logger.info( "Timeout period reached for {} job statistics, failure count has been updated.".format(updated)) + + +@notify_celery.task(name="delete-inbound-sms") +@statsd(namespace="tasks") +def delete_inbound_sms_older_than_seven_days(): + try: + start = datetime.utcnow() + deleted = delete_inbound_sms_created_more_than_a_week_ago() + current_app.logger.info( + "Delete inbound sms job started {} finished {} deleted {} inbound sms notifications".format( + start, + datetime.utcnow(), + deleted + ) + ) + except SQLAlchemyError as e: + current_app.logger.exception("Failed to delete inbound sms notifications") + raise diff --git a/app/config.py b/app/config.py index 1cac131e2..01aca1964 100644 --- a/app/config.py +++ b/app/config.py @@ -2,6 +2,7 @@ from datetime import timedelta from celery.schedules import crontab from kombu import Exchange, Queue import os + from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST if os.environ.get('VCAP_SERVICES'): @@ -168,6 +169,11 @@ class Config(object): 'schedule': crontab(minute=40, hour=0), 'options': {'queue': QueueNames.PERIODIC} }, + 'delete-inbound-sms': { + 'task': 'delete-inbound-sms', + 'schedule': crontab(minute=0, hour=1), + 'options': {'queue': QueueNames.PERIODIC} + }, 'send-daily-performance-platform-stats': { 'task': 'send-daily-performance-platform-stats', 'schedule': crontab(minute=0, hour=2), diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 597748731..3411aeb22 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -1,6 +1,13 @@ +from datetime import ( + timedelta, + datetime +) + + from app import db from app.dao.dao_utils import transactional from app.models import InboundSms +from app.statsd_decorators import statsd @transactional @@ -28,3 +35,15 @@ def dao_count_inbound_sms_for_service(service_id): return InboundSms.query.filter( InboundSms.service_id == service_id ).count() + + +@statsd(namespace="dao") +@transactional +def delete_inbound_sms_created_more_than_a_week_ago(): + seven_days_ago = datetime.utcnow() - timedelta(days=7) + + deleted = db.session.query(InboundSms).filter( + InboundSms.created_at < seven_days_ago + ).delete(synchronize_session='fetch') + + return deleted diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index f4ff2a24f..22da11966 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -2,7 +2,8 @@ import functools from datetime import ( datetime, timedelta, - date) + date +) from flask import current_app @@ -26,6 +27,7 @@ from app.models import ( NotificationHistory, NotificationStatistics, Template, + ScheduledNotification, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, NOTIFICATION_SENDING, @@ -35,7 +37,8 @@ from app.models import ( NOTIFICATION_PERMANENT_FAILURE, KEY_TYPE_NORMAL, KEY_TYPE_TEST, LETTER_TYPE, - NOTIFICATION_SENT, ScheduledNotification) + NOTIFICATION_SENT, +) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 7a347e35c..f706db28c 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -5,19 +5,23 @@ from functools import partial from flask import current_app from freezegun import freeze_time -from app.celery.scheduled_tasks import s3, timeout_job_statistics, delete_sms_notifications_older_than_seven_days, \ - delete_letter_notifications_older_than_seven_days, delete_email_notifications_older_than_seven_days, \ - send_scheduled_notifications from app.celery import scheduled_tasks from app.celery.scheduled_tasks import ( + delete_email_notifications_older_than_seven_days, + delete_inbound_sms_older_than_seven_days, + delete_invitations, + delete_notifications_created_more_than_a_week_ago_by_type, + delete_letter_notifications_older_than_seven_days, + delete_sms_notifications_older_than_seven_days, delete_verify_codes, remove_csv_files, - delete_notifications_created_more_than_a_week_ago_by_type, - delete_invitations, - timeout_notifications, run_scheduled_jobs, + s3, send_daily_performance_platform_stats, - switch_current_sms_provider_on_slow_delivery + send_scheduled_notifications, + switch_current_sms_provider_on_slow_delivery, + timeout_job_statistics, + timeout_notifications ) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.dao.jobs_dao import dao_get_job_by_id @@ -71,7 +75,8 @@ def prepare_current_provider(restore_provider_details): def test_should_have_decorated_tasks_functions(): assert delete_verify_codes.__wrapped__.__name__ == 'delete_verify_codes' - assert delete_notifications_created_more_than_a_week_ago_by_type.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago_by_type' # noqa + assert delete_notifications_created_more_than_a_week_ago_by_type.__wrapped__.__name__ == \ + 'delete_notifications_created_more_than_a_week_ago_by_type' assert timeout_notifications.__wrapped__.__name__ == 'timeout_notifications' assert delete_invitations.__wrapped__.__name__ == 'delete_invitations' assert run_scheduled_jobs.__wrapped__.__name__ == 'run_scheduled_jobs' @@ -79,6 +84,8 @@ def test_should_have_decorated_tasks_functions(): assert send_daily_performance_platform_stats.__wrapped__.__name__ == 'send_daily_performance_platform_stats' assert switch_current_sms_provider_on_slow_delivery.__wrapped__.__name__ == \ 'switch_current_sms_provider_on_slow_delivery' + assert delete_inbound_sms_older_than_seven_days.__wrapped__.__name__ == \ + 'delete_inbound_sms_older_than_seven_days' def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api, mocker): @@ -440,3 +447,9 @@ def test_timeout_job_statistics_called_with_notification_timeout(notify_api, moc dao_mock = mocker.patch('app.celery.scheduled_tasks.dao_timeout_job_statistics') timeout_job_statistics() dao_mock.assert_called_once_with(999) + + +def test_should_call_delete_inbound_sms_older_than_seven_days(notify_api, mocker): + mocker.patch('app.celery.scheduled_tasks.delete_inbound_sms_created_more_than_a_week_ago') + delete_inbound_sms_older_than_seven_days() + assert scheduled_tasks.delete_inbound_sms_created_more_than_a_week_ago.call_count == 1 diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index f0fc6d8b3..2731562c4 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -1,11 +1,16 @@ -from datetime import datetime +from datetime import datetime, timedelta from freezegun import freeze_time -from app.dao.inbound_sms_dao import dao_get_inbound_sms_for_service, dao_count_inbound_sms_for_service - +from app.dao.inbound_sms_dao import ( + dao_get_inbound_sms_for_service, + dao_count_inbound_sms_for_service, + delete_inbound_sms_created_more_than_a_week_ago +) from tests.app.db import create_inbound_sms, create_service +from app.models import InboundSms + def test_get_all_inbound_sms(sample_service): inbound = create_inbound_sms(sample_service) @@ -57,3 +62,27 @@ def test_count_inbound_sms_for_service(notify_db_session): create_inbound_sms(service_two) assert dao_count_inbound_sms_for_service(service_one.id) == 2 + + +@freeze_time("2017-01-01 12:00:00") +def test_should_delete_inbound_sms_older_than_seven_days(sample_service): + older_than_seven_days = datetime.utcnow() - timedelta(days=7, seconds=1) + create_inbound_sms(sample_service, created_at=older_than_seven_days) + delete_inbound_sms_created_more_than_a_week_ago() + + assert len(InboundSms.query.all()) == 0 + + +@freeze_time("2017-01-01 12:00:00") +def test_should_not_delete_inbound_sms_before_seven_days(sample_service): + yesterday = datetime.utcnow() - timedelta(days=1) + just_before_seven_days = datetime.utcnow() - timedelta(days=6, hours=23, minutes=59, seconds=59) + older_than_seven_days = datetime.utcnow() - timedelta(days=7, seconds=1) + + create_inbound_sms(sample_service, created_at=yesterday) + create_inbound_sms(sample_service, created_at=just_before_seven_days) + create_inbound_sms(sample_service, created_at=older_than_seven_days) + + delete_inbound_sms_created_more_than_a_week_ago() + + assert len(InboundSms.query.all()) == 2 diff --git a/tests/app/db.py b/tests/app/db.py index 8fcced1b1..2e658023d 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -2,6 +2,7 @@ from datetime import datetime import uuid +from app.dao.inbound_sms_dao import dao_create_inbound_sms from app.dao.jobs_dao import dao_create_job from app.models import ( InboundSms, @@ -12,6 +13,7 @@ from app.models import ( ScheduledNotification, ServicePermission, Job, + InboundSms, EMAIL_TYPE, SMS_TYPE, KEY_TYPE_NORMAL, @@ -151,14 +153,15 @@ def create_notification( return notification -def create_job(template, - notification_count=1, - created_at=None, - job_status='pending', - scheduled_for=None, - processing_started=None, - original_file_name='some.csv'): - +def create_job( + template, + notification_count=1, + created_at=None, + job_status='pending', + scheduled_for=None, + processing_started=None, + original_file_name='some.csv' +): data = { 'id': uuid.uuid4(), 'service_id': template.service_id, @@ -193,11 +196,12 @@ def create_inbound_sms( user_number='447700900111', provider_date=None, provider_reference=None, - content='Hello' + content='Hello', + created_at=None ): inbound = InboundSms( service=service, - created_at=datetime.utcnow(), + created_at=created_at or datetime.utcnow(), notify_number=notify_number or service.sms_sender, user_number=user_number, provider_date=provider_date or datetime.utcnow(),