From cfd42a2eb9abaf9e659693daae1b25f2cbefe6ca Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 3 Jun 2019 15:16:46 +0100 Subject: [PATCH] Update subquery to be more efficient. Update subquery to run again but for test keys. Test data is never inserted in Notifications so they need to be deleted separately now given the join to NotificationHistory. --- app/dao/notifications_dao.py | 24 +++++++++++++++---- ...t_notification_dao_delete_notifications.py | 10 +++++++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 2f14a4190..299349c0a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -336,21 +336,35 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim return deleted -def _delete_notifications( - deleted, notification_type, date_to_delete_from, service_id, query_limit): - +def _delete_notifications(deleted, notification_type, date_to_delete_from, service_id, query_limit): subquery = db.session.query( Notification.id + ).join(NotificationHistory, NotificationHistory.id == Notification.id).filter( + Notification.notification_type == notification_type, + Notification.service_id == service_id, + Notification.created_at < date_to_delete_from, + ).limit(query_limit).subquery() + + deleted += _delete_for_query(subquery) + + subquery_for_test_keys = db.session.query( + Notification.id ).filter( Notification.notification_type == notification_type, Notification.service_id == service_id, Notification.created_at < date_to_delete_from, - NotificationHistory.id == Notification.id + Notification.key_type == KEY_TYPE_TEST ).limit(query_limit).subquery() + deleted += _delete_for_query(subquery_for_test_keys) + + return deleted + + +def _delete_for_query(subquery): number_deleted = db.session.query(Notification).filter( Notification.id.in_(subquery)).delete(synchronize_session='fetch') - deleted += number_deleted + deleted = number_deleted db.session.commit() while number_deleted > 0: number_deleted = db.session.query(Notification).filter( diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index 030161698..22190a4a1 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -181,6 +181,14 @@ def test_delete_notifications_keep_data_for_days_of_retention_is_longer(sample_s mock_get_s3.assert_not_called() +def test_delete_notifications_with_test_keys(sample_template, mocker): + mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") + create_notification(template=sample_template, key_type='test', created_at=datetime.utcnow() - timedelta(days=8)) + assert len(Notification.query.all()) == 1 + delete_notifications_older_than_retention_by_type('sms') + assert len(Notification.query.filter_by().all()) == 0 + + def test_delete_notifications_delete_notification_type_for_default_time_if_no_days_of_retention_for_type( sample_service, mocker ): @@ -212,7 +220,7 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee mock_get_s3.assert_not_called() -@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +@pytest.mark.parametrize('notification_type', ['sms']) @freeze_time("2016-01-10 12:00:00.000000") def test_should_not_delete_notification_if_history_does_not_exist(sample_service, notification_type, mocker): mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")