darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-08-13 18:37:58 -04:00
Code Issues Packages Projects Releases Wiki Activity

This option removes the subquery all together.

Browse Source 
It has been recommended that subqueries are really inefficient especially on a delete statement.
This commit is contained in:
Rebecca Law
2020-03-20 07:35:15 +00:00
parent 3a61a84ca3
commit 4dc1a48464
2 changed files with 2 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
app/dao/notifications_dao.py
View File

@@ -342,40 +342,12 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim
def _delete_notifications(notification_type, date_to_delete_from, service_id, query_limit):
subquery = db.session.query(
Notification.id
).join(NotificationHistory, NotificationHistory.id == Notification.id).filter(
deleted = Notification.query.filter(
Notification.notification_type == notification_type,
Notification.service_id == service_id,
Notification.created_at < date_to_delete_from,
).limit(query_limit).subquery()
).delete()
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,
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
db.session.commit()
while number_deleted > 0:
number_deleted = db.session.query(Notification).filter(
Notification.id.in_(subquery)).delete(synchronize_session='fetch')
deleted += number_deleted
db.session.commit()
return deleted

15
tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py
View File

@@ -227,21 +227,6 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee
mock_get_s3.assert_not_called()
@freeze_time("2016-01-10 12:00:00.000000")
def test_should_not_delete_notification_if_history_does_not_exist(sample_service, mocker):
mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
mocker.patch("app.dao.notifications_dao.insert_update_notification_history")
with freeze_time('2016-01-01 12:00'):
email_template, letter_template, sms_template = _create_templates(sample_service)
create_notification(template=email_template, status='permanent-failure')
create_notification(template=sms_template, status='delivered')
create_notification(template=letter_template, status='temporary-failure')
assert Notification.query.count() == 3
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 3
assert NotificationHistory.query.count() == 0
def test_delete_notifications_calls_subquery_multiple_times(sample_template):
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))