From 15065c4bc72719a673e1241071898fc362126019 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 9 May 2017 14:06:27 +0100 Subject: [PATCH] Added DAO methods to add and update the stats table for JOBS - create_or_update_job_sending_statistics This will try and update an existing row. if this fails as it hasn't been created, then it will insert a row. If this fails as another process has got there first then it should try and update again. This is a code version of upset - update_job_stats_outcome_count Will update the outcome states. Uses the NOTIFICATION_STATUS_TYPES_FAILED to determine if the notification failed. Any thing in DELIVERED will be marked as delivered. Statues not in the FAILED array or delivered provoke no update to the table. --- app/dao/statistics_dao.py | 79 ++++++- tests/app/dao/test_statistics_dao.py | 333 ++++++++++++++++++++++++++- 2 files changed, 404 insertions(+), 8 deletions(-) diff --git a/app/dao/statistics_dao.py b/app/dao/statistics_dao.py index 02ed25e78..a2fca3965 100644 --- a/app/dao/statistics_dao.py +++ b/app/dao/statistics_dao.py @@ -1,4 +1,79 @@ +from flask import current_app +from sqlalchemy.exc import SQLAlchemyError + +from app import db +from app.dao.dao_utils import transactional +from app.models import ( + JobStatistics, + EMAIL_TYPE, + SMS_TYPE, + LETTER_TYPE, + NOTIFICATION_STATUS_TYPES_FAILED, + NOTIFICATION_DELIVERED +) +from app.statsd_decorators import statsd -def persist_initial_job_statistics(notification): - pass +@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: + current_app.logger.exception(e) + __update_job_stats_sent_count(notification) + + +def __update_job_stats_sent_count(notification): + update = { + JobStatistics.emails_sent: + JobStatistics.emails_sent + 1 if notification.notification_type == EMAIL_TYPE else 0, + JobStatistics.sms_sent: + JobStatistics.sms_sent + 1 if notification.notification_type == SMS_TYPE else 0, + JobStatistics.letters_sent: + JobStatistics.letters_sent + 1 if notification.notification_type == LETTER_TYPE else 0 + } + return db.session.query(JobStatistics).filter_by( + job_id=notification.job_id, + ).update(update) + + +def __insert_job_stats(notification): + + stats = JobStatistics( + job_id=notification.job_id, + emails_sent=1 if notification.notification_type == EMAIL_TYPE else 0, + sms_sent=1 if notification.notification_type == SMS_TYPE else 0, + letters_sent=1 if notification.notification_type == LETTER_TYPE else 0 + ) + db.session.add(stats) + + +def update_job_stats_outcome_count(notification): + update = None + + if notification.status in NOTIFICATION_STATUS_TYPES_FAILED: + update = { + JobStatistics.emails_failed: + JobStatistics.emails_failed + 1 if notification.notification_type == EMAIL_TYPE else 0, + JobStatistics.sms_failed: + JobStatistics.sms_failed + 1 if notification.notification_type == SMS_TYPE else 0, + JobStatistics.letters_failed: + JobStatistics.letters_failed + 1 if notification.notification_type == LETTER_TYPE else 0 + } + + elif notification.status == NOTIFICATION_DELIVERED and notification.notification_type != LETTER_TYPE: + update = { + JobStatistics.emails_delivered: + JobStatistics.emails_delivered + 1 if notification.notification_type == EMAIL_TYPE else 0, + JobStatistics.sms_delivered: + JobStatistics.sms_delivered + 1 if notification.notification_type == SMS_TYPE else 0 + } + + if update: + return db.session.query(JobStatistics).filter_by( + job_id=notification.job_id, + ).update(update) + else: + return 0 diff --git a/tests/app/dao/test_statistics_dao.py b/tests/app/dao/test_statistics_dao.py index a66dfbe42..c1e80208f 100644 --- a/tests/app/dao/test_statistics_dao.py +++ b/tests/app/dao/test_statistics_dao.py @@ -1,19 +1,340 @@ -from app.dao.statistics_dao import persist_initial_job_statistics -from app.models import JobStatistics -from tests.app.conftest import sample_notification +from unittest.mock import call + +import pytest +from sqlalchemy.exc import SQLAlchemyError + +from app.dao.statistics_dao import ( + create_or_update_job_sending_statistics, + update_job_stats_outcome_count +) +from app.models import ( + JobStatistics, + SMS_TYPE, + EMAIL_TYPE, + LETTER_TYPE, + NOTIFICATION_DELIVERED, + NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_TEMPORARY_FAILURE, + NOTIFICATION_PERMANENT_FAILURE, + NOTIFICATION_PENDING, NOTIFICATION_CREATED, NOTIFICATION_FAILED, NOTIFICATION_SENT, NOTIFICATION_SENDING) +from tests.app.conftest import sample_notification, sample_email_template, sample_template +@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count', [ + (SMS_TYPE, 1, 0, 0), + (EMAIL_TYPE, 0, 1, 0), + (LETTER_TYPE, 0, 0, 1) +]) def test_should_create_a_stats_entry_for_a_job( - notify_db, notify_db_session, sample_service, sample_template, sample_job + notify_db, + notify_db_session, + sample_job, + sample_letter_template, + notification_type, + sms_count, + email_count, + letter_count ): assert not len(JobStatistics.query.all()) + template = None + + if notification_type == SMS_TYPE: + template = sample_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == EMAIL_TYPE: + template = sample_email_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == LETTER_TYPE: + template = sample_letter_template + notification = sample_notification( - notify_db, notify_db_session, service=sample_service, template=sample_template, job=sample_job + notify_db, notify_db_session, service=sample_job.service, template=template, job=sample_job ) - persist_initial_job_statistics(notification) + create_or_update_job_sending_statistics(notification) stats = JobStatistics.query.all() assert len(stats) == 1 + + stat = stats[0] + assert stat.job_id == sample_job.id + + assert stat.emails_sent == email_count + assert stat.sms_sent == sms_count + assert stat.letters_sent == letter_count + + assert stat.emails_delivered == 0 + assert stat.emails_failed == 0 + assert stat.sms_delivered == 0 + assert stat.sms_failed == 0 + assert stat.letters_failed == 0 + + +@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count', [ + (SMS_TYPE, 2, 0, 0), + (EMAIL_TYPE, 0, 2, 0), + (LETTER_TYPE, 0, 0, 2) +]) +def test_should_update_a_stats_entry_for_a_job( + notify_db, + notify_db_session, + sample_job, + sample_letter_template, + notification_type, + sms_count, + email_count, + letter_count +): + assert not len(JobStatistics.query.all()) + + template = None + + if notification_type == SMS_TYPE: + template = sample_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == EMAIL_TYPE: + template = sample_email_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == LETTER_TYPE: + template = sample_letter_template + + notification = sample_notification( + notify_db, notify_db_session, service=sample_job.service, template=template, job=sample_job + ) + + create_or_update_job_sending_statistics(notification) + + stats = JobStatistics.query.all() + + assert len(stats) == 1 + + create_or_update_job_sending_statistics(notification) + + stat = stats[0] + assert stat.job_id == sample_job.id + + assert stat.emails_sent == email_count + assert stat.sms_sent == sms_count + assert stat.letters_sent == letter_count + + assert stat.emails_delivered == 0 + assert stat.emails_failed == 0 + assert stat.sms_delivered == 0 + assert stat.sms_failed == 0 + assert stat.letters_failed == 0 + + +def test_should_handle_case_where_stats_row_created_by_another_thread( + notify_db, + notify_db_session, + sample_notification, + mocker): + create_mock = mocker.patch("app.dao.statistics_dao.__insert_job_stats", side_effect=SQLAlchemyError("beaten to it")) + update_mock = mocker.patch("app.dao.statistics_dao.__update_job_stats_sent_count", return_value=0) + + create_or_update_job_sending_statistics(sample_notification) + + update_mock.assert_has_calls([call(sample_notification), call(sample_notification)]) + create_mock.assert_called_once_with(sample_notification) + + +@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count', [ + (SMS_TYPE, 1, 0, 0), + (EMAIL_TYPE, 0, 1, 0), + (LETTER_TYPE, 0, 0, 1) +]) +def test_should_update_a_stats_entry_with_its_success_outcome_for_a_job( + notify_db, + notify_db_session, + sample_job, + sample_letter_template, + notification_type, + sms_count, + email_count, + letter_count +): + assert not len(JobStatistics.query.all()) + + template = None + + if notification_type == SMS_TYPE: + template = sample_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == EMAIL_TYPE: + template = sample_email_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == LETTER_TYPE: + template = sample_letter_template + + notification = sample_notification( + notify_db, + notify_db_session, + service=sample_job.service, + template=template, + job=sample_job, + status=NOTIFICATION_DELIVERED + ) + + create_or_update_job_sending_statistics(notification) + + stats = JobStatistics.query.all() + + assert len(stats) == 1 + + update_job_stats_outcome_count(notification) + + stat = stats[0] + assert stat.job_id == sample_job.id + + assert stat.emails_sent == email_count + assert stat.sms_sent == sms_count + assert stat.letters_sent == letter_count + + assert stat.emails_delivered == email_count + assert stat.sms_delivered == sms_count + + assert stat.emails_failed == 0 + assert stat.sms_failed == 0 + assert stat.letters_failed == 0 + + +@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [ + (SMS_TYPE, 1, 0, 0, NOTIFICATION_TECHNICAL_FAILURE), + (SMS_TYPE, 1, 0, 0, NOTIFICATION_TEMPORARY_FAILURE), + (SMS_TYPE, 1, 0, 0, NOTIFICATION_PERMANENT_FAILURE), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_TECHNICAL_FAILURE), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_PERMANENT_FAILURE), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_TEMPORARY_FAILURE), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_PERMANENT_FAILURE), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_TEMPORARY_FAILURE), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_TECHNICAL_FAILURE) +]) +def test_should_update_a_stats_entry_with_its_error_outcomes_for_a_job( + notify_db, + notify_db_session, + sample_job, + sample_letter_template, + notification_type, + sms_count, + email_count, + letter_count, + status +): + assert not len(JobStatistics.query.all()) + + template = None + + if notification_type == SMS_TYPE: + template = sample_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == EMAIL_TYPE: + template = sample_email_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == LETTER_TYPE: + template = sample_letter_template + + notification = sample_notification( + notify_db, + notify_db_session, + service=sample_job.service, + template=template, + job=sample_job, + status=status + ) + + create_or_update_job_sending_statistics(notification) + + stats = JobStatistics.query.all() + + assert len(stats) == 1 + + update_job_stats_outcome_count(notification) + + stat = stats[0] + assert stat.job_id == sample_job.id + + assert stat.emails_sent == email_count + assert stat.sms_sent == sms_count + assert stat.letters_sent == letter_count + + assert stat.emails_failed == email_count + assert stat.letters_failed == letter_count + assert stat.sms_failed == sms_count + + assert stat.emails_delivered == 0 + assert stat.sms_delivered == 0 + + +@pytest.mark.parametrize('notification_type, sms_count, email_count, letter_count, status', [ + (SMS_TYPE, 1, 0, 0, NOTIFICATION_PENDING), + (SMS_TYPE, 1, 0, 0, NOTIFICATION_CREATED), + (SMS_TYPE, 1, 0, 0, NOTIFICATION_FAILED), + (SMS_TYPE, 1, 0, 0, NOTIFICATION_SENT), + (SMS_TYPE, 1, 0, 0, NOTIFICATION_SENDING), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_PENDING), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_CREATED), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_FAILED), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_SENT), + (EMAIL_TYPE, 0, 1, 0, NOTIFICATION_SENDING), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_PENDING), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_CREATED), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_FAILED), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_SENT), + (LETTER_TYPE, 0, 0, 1, NOTIFICATION_SENDING) +]) +def test_should_not_update_job_stats_if_irrelevant_status( + notify_db, + notify_db_session, + sample_job, + sample_letter_template, + notification_type, + sms_count, + email_count, + letter_count, + status +): + assert not len(JobStatistics.query.all()) + + template = None + + if notification_type == SMS_TYPE: + template = sample_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == EMAIL_TYPE: + template = sample_email_template(notify_db, notify_db_session, service=sample_job.service) + + if notification_type == LETTER_TYPE: + template = sample_letter_template + + notification = sample_notification( + notify_db, + notify_db_session, + service=sample_job.service, + template=template, + job=sample_job, + status=status + ) + + create_or_update_job_sending_statistics(notification) + + stats = JobStatistics.query.all() + + assert len(stats) == 1 + + update_job_stats_outcome_count(notification) + + stat = stats[0] + assert stat.job_id == sample_job.id + + assert stat.emails_sent == email_count + assert stat.sms_sent == sms_count + assert stat.letters_sent == letter_count + + assert stat.emails_failed == 0 + assert stat.letters_failed == 0 + assert stat.sms_failed == 0 + + assert stat.emails_delivered == 0 + assert stat.sms_delivered == 0