From f948555ca8eab0229619aa199cd59f70ddc2f643 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Tue, 24 Dec 2019 09:29:27 +0000 Subject: [PATCH] Do nothing on db conflict For notification and notification_history we do an upsert. Here, as the inbound_sms table is never updated, only inserted to once (signified by lack of updated_at field), an upsert would be unnecessary. Therefore, if for some reason the delete statement failed as part of moving data into the inbound_sms_history table, we can simply just ignore any db conflicts raised by a rerun of `delete_inbound_sms_older_than_retention`. --- app/dao/inbound_sms_dao.py | 15 +++------- tests/app/dao/test_inbound_sms_dao.py | 40 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/app/dao/inbound_sms_dao.py b/app/dao/inbound_sms_dao.py index 40ef356b2..ba17ddd26 100644 --- a/app/dao/inbound_sms_dao.py +++ b/app/dao/inbound_sms_dao.py @@ -63,7 +63,7 @@ def dao_count_inbound_sms_for_service(service_id, limit_days): ).count() -def _insert_update_inbound_sms_history(subquery, query_limit=10000): +def _insert_inbound_sms_history(subquery, query_limit=10000): offset = 0 inbound_sms_query = db.session.query( *[x.name for x in InboundSmsHistory.__table__.c] @@ -76,15 +76,8 @@ def _insert_update_inbound_sms_history(subquery, query_limit=10000): inbound_sms_query.limit(query_limit).offset(offset) ) - statement = statement.on_conflict_do_update( - constraint="inbound_sms_history_pkey", - set_={ - "service_id": statement.excluded.service_id, - "notify_number": statement.excluded.notify_number, - "provider_date": statement.excluded.provider_date, - "provider_reference": statement.excluded.provider_reference, - "provider": statement.excluded.provider - } + statement = statement.on_conflict_do_nothing( + constraint="inbound_sms_history_pkey" ) db.session.connection().execute(statement) @@ -107,7 +100,7 @@ def _delete_inbound_sms(datetime_to_delete_from, query_filter): # set to nonzero just to enter the loop number_deleted = 1 while number_deleted > 0: - _insert_update_inbound_sms_history(subquery, query_limit=query_limit) + _insert_inbound_sms_history(subquery, query_limit=query_limit) number_deleted = InboundSms.query.filter(InboundSms.id.in_(subquery)).delete(synchronize_session='fetch') deleted += number_deleted diff --git a/tests/app/dao/test_inbound_sms_dao.py b/tests/app/dao/test_inbound_sms_dao.py index cd8daea7f..b3b3f00db 100644 --- a/tests/app/dao/test_inbound_sms_dao.py +++ b/tests/app/dao/test_inbound_sms_dao.py @@ -13,6 +13,7 @@ from app.dao.inbound_sms_dao import ( ) from app.models import InboundSmsHistory +from app import db from tests.conftest import set_config from tests.app.db import create_inbound_sms, create_service, create_service_data_retention @@ -163,6 +164,45 @@ def test_insert_into_inbound_sms_history_when_deleting_inbound_sms(sample_servic assert history[0].created_at == datetime(2019, 12, 12, 20, 20) +@freeze_time("2019-12-20 12:00:00") +def test_delete_inbound_sms_older_than_retention_does_nothing_when_database_conflict_raised(sample_service): + inbound_sms = create_inbound_sms( + sample_service, created_at=datetime(2019, 12, 12, 20, 20), + notify_number='07700900100', + provider_date=datetime(2019, 12, 12, 20, 19), + provider_reference='from daisy pie', + provider='unicorn' + ) + inbound_sms_id = inbound_sms.id + + # Insert data directly in to inbound_sms_history to mimic if we had run `delete_inbound_sms_older_than_retention` + # before but for some reason the delete statement had failed + conflict_creating_row = InboundSmsHistory( + id=inbound_sms.id, + service_id=inbound_sms.service.id, + created_at=inbound_sms.created_at, + notify_number=inbound_sms.notify_number, + provider_date=inbound_sms.provider_date, + provider_reference=inbound_sms.provider_reference, + provider=inbound_sms.provider, + ) + db.session.add(conflict_creating_row) + db.session.commit() + assert conflict_creating_row.id + + delete_inbound_sms_older_than_retention() + + history = InboundSmsHistory.query.all() + assert len(history) == 1 + + assert history[0].id == inbound_sms_id + assert history[0].notify_number == '07700900100' + assert history[0].provider_date == datetime(2019, 12, 12, 20, 19) + assert history[0].provider_reference == 'from daisy pie' + assert history[0].provider == 'unicorn' + assert history[0].created_at == datetime(2019, 12, 12, 20, 20) + + def test_get_inbound_sms_by_id_returns(sample_service): inbound_sms = create_inbound_sms(service=sample_service) inbound_from_db = dao_get_inbound_sms_by_id(inbound_sms.service.id, inbound_sms.id)