From 50a5bedcbfb6ba983871bc7223675effb4ba4d83 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 7 Apr 2017 10:59:12 +0100 Subject: [PATCH] Refactor update to notifcations to be a bulk update. This is much better for performance. --- app/celery/tasks.py | 14 +++++------ app/dao/notifications_dao.py | 15 ++++++++++++ tests/app/dao/test_notification_dao.py | 34 ++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 7d15c6c75..aa9b37d28 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -24,7 +24,8 @@ from app.dao.jobs_dao import ( all_notifications_are_created_for_job, dao_get_all_notifications_for_job, dao_update_job_status) -from app.dao.notifications_dao import get_notification_by_id, dao_update_notification +from app.dao.notifications_dao import get_notification_by_id, dao_update_notification, \ + dao_update_notifications_sent_to_dvla from app.dao.provider_details_dao import get_current_provider from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count from app.dao.templates_dao import dao_get_template_by_id @@ -38,7 +39,7 @@ from app.models import ( JOB_STATUS_IN_PROGRESS, JOB_STATUS_FINISHED, JOB_STATUS_READY_TO_SEND, - JOB_STATUS_SENT_TO_DVLA, NOTIFICATION_SENDING) + JOB_STATUS_SENT_TO_DVLA, NOTIFICATION_SENDING, Notification) from app.notifications.process_notifications import persist_notification from app.service.utils import service_allowed_to_send_to from app.statsd_decorators import statsd @@ -298,14 +299,13 @@ def update_job_to_sent_to_dvla(self, job_id): # This task will be called by the FTP app to update the job to sent to dvla # and update all notifications for this job to sending, provider = DVLA provider = get_current_provider(LETTER_TYPE) - notifications = dao_get_all_notifications_for_job(job_id) - for n in notifications: - n.status = NOTIFICATION_SENDING - n.sent_by = provider.identifier - dao_update_notification(n) + updated_count = dao_update_notifications_sent_to_dvla(job_id, provider.identifier) dao_update_job_status(job_id, JOB_STATUS_SENT_TO_DVLA) + current_app.logger.info("Updated {} letter notifications to sending. " + "Updated {} job to {}".format(updated_count, job_id, JOB_STATUS_SENT_TO_DVLA)) + def create_dvla_file_contents(job_id): file_contents = '\n'.join( diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4327ce690..bc617f224 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -439,3 +439,18 @@ def is_delivery_slow_for_provider( (Notification.updated_at - Notification.sent_at) >= delivery_time, ).count() return count >= threshold + + +@statsd(namespace="dao") +@transactional +def dao_update_notifications_sent_to_dvla(job_id, provider): + updated_count = db.session.query(Notification + ).filter(Notification.job_id == job_id + ).update({'status': 'sending', "sent_by": provider}) + # only update NotificationHistory if api key typ is not test + db.session.query(NotificationHistory + ).filter(NotificationHistory.job_id == job_id, + NotificationHistory.key_type != KEY_TYPE_TEST + ).update({'status': 'sending', "sent_by": provider}) + + return updated_count diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 170a9a7ea..7ffb895bc 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -41,8 +41,8 @@ from app.dao.notifications_dao import ( dao_timeout_notifications, get_financial_year, get_april_fools, - is_delivery_slow_for_provider -) + is_delivery_slow_for_provider, + dao_update_notifications_sent_to_dvla) from app.dao.services_dao import dao_update_service from tests.app.db import create_notification @@ -1551,3 +1551,33 @@ def test_slow_provider_delivery_does_not_return_for_standard_delivery_time( ) assert not slow_delivery + + +def test_dao_update_notifications_sent_to_dvla(notify_db, notify_db_session, sample_letter_template): + job = sample_job(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_letter_template) + notification = create_notification(template=sample_letter_template, job=job) + + updated_count = dao_update_notifications_sent_to_dvla(job_id=job.id, provider='some provider') + + assert updated_count == 1 + updated_notification = Notification.query.get(notification.id) + assert updated_notification.status == 'sending' + assert updated_notification.sent_by == 'some provider' + history = NotificationHistory.query.get(notification.id) + assert history.status == 'sending' + assert history.sent_by == 'some provider' + + +def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key( + notify_db, notify_db_session, sample_letter_template, sample_api_key): + job = sample_job(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_letter_template) + notification = create_notification(template=sample_letter_template, job=job, api_key_id=sample_api_key.id, + key_type='test') + + updated_count = dao_update_notifications_sent_to_dvla(job_id=job.id, provider='some provider') + + assert updated_count == 1 + updated_notification = Notification.query.get(notification.id) + assert updated_notification.status == 'sending' + assert updated_notification.sent_by == 'some provider' + assert not NotificationHistory.query.get(notification.id)