diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 22b0d468b..c65eb8e81 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -1,8 +1,3 @@ -from datetime import ( - timedelta, - datetime, - date -) from flask import current_app from notifications_utils.statsd_decorators import statsd from sqlalchemy import desc, and_ @@ -10,8 +5,8 @@ from sqlalchemy.orm import aliased from app import db from app.dao.dao_utils import transactional -from app.models import InboundSms -from app.utils import get_london_midnight_in_utc +from app.models import InboundSms, Service, ServiceDataRetention, SMS_TYPE +from app.utils import midnight_n_days_ago @transactional @@ -20,7 +15,7 @@ def dao_create_inbound_sms(inbound_sms): def dao_get_inbound_sms_for_service(service_id, limit=None, user_number=None): - start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) + start_date = midnight_n_days_ago(6) q = InboundSms.query.filter( InboundSms.service_id == service_id, InboundSms.created_at >= start_date @@ -60,21 +55,63 @@ def dao_get_paginated_inbound_sms_for_service_for_public_api( def dao_count_inbound_sms_for_service(service_id): - start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) + start_date = midnight_n_days_ago(6) return InboundSms.query.filter( InboundSms.service_id == service_id, InboundSms.created_at >= start_date ).count() +def _delete_inbound_sms(datetime_to_delete_from, query_filter): + query_limit = 10000 + + subquery = db.session.query( + InboundSms.id + ).filter( + InboundSms.created_at < datetime_to_delete_from, + *query_filter + ).limit( + query_limit + ).subquery() + + deleted = 0 + # set to nonzero just to enter the loop + number_deleted = 1 + while number_deleted > 0: + number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch') + deleted += number_deleted + + return deleted + + @statsd(namespace="dao") @transactional -def delete_inbound_sms_created_more_than_a_week_ago(): - seven_days_ago = datetime.utcnow() - timedelta(days=7) +def delete_inbound_sms_older_than_retention(): + current_app.logger.info('Deleting inbound sms for services with flexible data retention') - deleted = db.session.query(InboundSms).filter( - InboundSms.created_at < seven_days_ago - ).delete(synchronize_session='fetch') + flexible_data_retention = ServiceDataRetention.query.join( + ServiceDataRetention.service, + Service.inbound_number + ).filter( + ServiceDataRetention.notification_type == SMS_TYPE + ).all() + + deleted = 0 + for f in flexible_data_retention: + n_days_ago = midnight_n_days_ago(f.days_of_retention) + + current_app.logger.info("Deleting inbound sms for service id: {}".format(f.service_id)) + deleted += _delete_inbound_sms(n_days_ago, query_filter=[InboundSms.service_id == f.service_id]) + + current_app.logger.info('Deleting inbound sms for services without flexible data retention') + + seven_days_ago = midnight_n_days_ago(7) + + deleted += _delete_inbound_sms(seven_days_ago, query_filter=[ + InboundSms.service_id.notin_(x.service_id for x in flexible_data_retention), + ]) + + current_app.logger.info('Deleted {} inbound sms'.format(deleted)) return deleted @@ -108,7 +145,7 @@ def dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service( ORDER BY t1.created_at DESC; LIMIT 50 OFFSET :page """ - start_date = get_london_midnight_in_utc(date.today() - timedelta(days=6)) + start_date = midnight_n_days_ago(6) t2 = aliased(InboundSms) q = db.session.query( InboundSms diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index 6fd43a47b..cb0822cdf 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -1,19 +1,18 @@ -from datetime import datetime, timedelta +from datetime import datetime +from itertools import product from freezegun import freeze_time 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, + delete_inbound_sms_older_than_retention, dao_get_inbound_sms_by_id, dao_get_paginated_inbound_sms_for_service_for_public_api, dao_get_paginated_most_recent_inbound_sms_by_user_number_for_service ) from tests.conftest import set_config -from tests.app.db import create_inbound_sms, create_service - -from app.models import InboundSms +from tests.app.db import create_inbound_sms, create_service, create_service_data_retention def test_get_all_inbound_sms(sample_service): @@ -93,28 +92,37 @@ def test_count_inbound_sms_for_service_filters_messages_older_than_seven_days(sa assert dao_count_inbound_sms_for_service(sample_service.id) == 1 -@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() +@freeze_time("2017-06-08 12:00:00") +def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session): + service_without_retention = create_service(service_name='without retention') + service_with_three_day_retention = create_service(service_name='three days') + service_with_month_retention = create_service(service_name='thirty days') - assert len(InboundSms.query.all()) == 0 + services = [service_with_three_day_retention, service_without_retention, service_with_month_retention] + create_service_data_retention(service_with_month_retention.id, notification_type='sms', days_of_retention=30) + create_service_data_retention(service_with_three_day_retention.id, notification_type='sms', days_of_retention=3) + # email retention doesn't affect anything + create_service_data_retention(service_with_three_day_retention.id, notification_type='email', days_of_retention=4) -@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) + dates = [ + datetime(2017, 6, 4, 23, 00), # just before three days + datetime(2017, 6, 4, 22, 59), # older than three days + datetime(2017, 5, 31, 23, 00), # just before seven days + datetime(2017, 5, 31, 22, 59), # older than seven days + datetime(2017, 5, 7, 22, 59), # older than thirty days + ] - 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) + for date, service in product(dates, services): + create_inbound_sms(service, created_at=date) - delete_inbound_sms_created_more_than_a_week_ago() + deleted_count = delete_inbound_sms_older_than_retention() - assert len(InboundSms.query.all()) == 2 + # four deleted for the 3-day service, two for the default seven days one, one for the 30 day + assert deleted_count == 7 + assert dao_count_inbound_sms_for_service(service_without_retention.id) == 3 + assert dao_count_inbound_sms_for_service(service_with_three_day_retention.id) == 1 + assert dao_count_inbound_sms_for_service(service_with_month_retention.id) == 4 def test_get_inbound_sms_by_id_returns(sample_service): diff --git a/tests/app/db.py b/tests/app/db.py index 2aa6e3ae1..023e5fe30 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,3 +1,4 @@ +import random import uuid from datetime import datetime, date @@ -318,10 +319,18 @@ def create_inbound_sms( provider="mmg", created_at=None ): + if not service.inbound_number: + create_inbound_number( + # create random inbound number + notify_number or '07{:09}'.format(random.randint(0, 1e9 - 1)), + provider=provider, + service_id=service.id + ) + inbound = InboundSms( service=service, created_at=created_at or datetime.utcnow(), - notify_number=notify_number or service.get_default_sms_sender(), + notify_number=service.get_inbound_number(), user_number=user_number, provider_date=provider_date or datetime.utcnow(), provider_reference=provider_reference or 'foo',