From 4faafe01d9329acfdfc1d1c38b6c3e56f689ca62 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 9 May 2017 14:41:29 +0100 Subject: [PATCH] Tightened the error checking. - Looks now for a specific Integrity error, not a generic SQLAlchemy error when checking for an insert failure - If the update fails post an insert error, then there is an issue so raise an exception so tasks can be retried. --- app/dao/statistics_dao.py | 9 +++++---- tests/app/dao/test_statistics_dao.py | 18 +++++++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app/dao/statistics_dao.py b/app/dao/statistics_dao.py index a2fca3965..11c0c2e79 100644 --- a/app/dao/statistics_dao.py +++ b/app/dao/statistics_dao.py @@ -1,5 +1,5 @@ from flask import current_app -from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.exc import IntegrityError, SQLAlchemyError from app import db from app.dao.dao_utils import transactional @@ -15,14 +15,14 @@ from app.statsd_decorators import statsd @statsd(namespace="dao") -@transactional def create_or_update_job_sending_statistics(notification): if __update_job_stats_sent_count(notification) == 0: try: __insert_job_stats(notification) - except SQLAlchemyError as e: + except IntegrityError as e: current_app.logger.exception(e) - __update_job_stats_sent_count(notification) + if __update_job_stats_sent_count(notification) == 0: + raise SQLAlchemyError("Failed to create job statistics for {}".format(notification.job_id)) def __update_job_stats_sent_count(notification): @@ -39,6 +39,7 @@ def __update_job_stats_sent_count(notification): ).update(update) +@transactional def __insert_job_stats(notification): stats = JobStatistics( diff --git a/tests/app/dao/test_statistics_dao.py b/tests/app/dao/test_statistics_dao.py index c1e80208f..21979ba1a 100644 --- a/tests/app/dao/test_statistics_dao.py +++ b/tests/app/dao/test_statistics_dao.py @@ -1,7 +1,7 @@ from unittest.mock import call import pytest -from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.exc import IntegrityError, SQLAlchemyError from app.dao.statistics_dao import ( create_or_update_job_sending_statistics, @@ -126,18 +126,22 @@ def test_should_update_a_stats_entry_for_a_job( assert stat.letters_failed == 0 -def test_should_handle_case_where_stats_row_created_by_another_thread( +def test_should_handle_error_conditions( notify_db, notify_db_session, - sample_notification, + sample_job, mocker): - create_mock = mocker.patch("app.dao.statistics_dao.__insert_job_stats", side_effect=SQLAlchemyError("beaten to it")) + create_mock = mocker.patch("app.dao.statistics_dao.__insert_job_stats", side_effect=IntegrityError("1", "2", "3")) update_mock = mocker.patch("app.dao.statistics_dao.__update_job_stats_sent_count", return_value=0) - create_or_update_job_sending_statistics(sample_notification) + notification = sample_notification(notify_db, notify_db_session, job=sample_job) - update_mock.assert_has_calls([call(sample_notification), call(sample_notification)]) - create_mock.assert_called_once_with(sample_notification) + with pytest.raises(SQLAlchemyError) as e: + create_or_update_job_sending_statistics(notification) + assert 'Failed to create job statistics for {}'.format(sample_job.id) in str(e.value) + + update_mock.assert_has_calls([call(notification), call(notification)]) + create_mock.assert_called_once_with(notification) @pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count', [