From cc185385f72a7fefe4a883f3731fc8d6caea7388 Mon Sep 17 00:00:00 2001 From: Robert Scott Date: Wed, 18 Dec 2024 21:52:37 +0000 Subject: [PATCH] get_paginated_jobs, get_paginated_uploads: merge, cache job outcome stat processing the logic is effectively repeated between the two functions - take advantage of this to make a common implementation that conditionally redis-caches the result. the thinking here being that once all a job's notifications are in a "completed" status they are unlikely to change again and we can save ourselves from scanning the notifications table again for up to 100k rows per job. being able to share this cache between the get_paginated_jobs and get_paginated_uploads is a bonus. --- app/dao/jobs_dao.py | 32 +++++- app/job/rest.py | 18 +-- app/upload/rest.py | 23 +--- tests/app/dao/test_jobs_dao.py | 196 +++++++++++++++++++++++++++++++++ tests/app/job/test_rest.py | 83 ++++++++++++-- tests/app/upload/test_rest.py | 53 ++++++++- 6 files changed, 356 insertions(+), 49 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 6595711209..7430ed2d62 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -2,13 +2,14 @@ from datetime import datetime, timedelta from flask import current_app +from notifications_utils.clients.redis import RequestCache from notifications_utils.letter_timings import ( CANCELLABLE_JOB_LETTER_STATUSES, letter_can_be_cancelled, ) from sqlalchemy import and_, asc, desc, func -from app import db +from app import db, redis_store from app.constants import ( JOB_STATUS_CANCELLED, JOB_STATUS_FINISHED, @@ -17,8 +18,10 @@ LETTER_TYPE, NOTIFICATION_CANCELLED, NOTIFICATION_CREATED, + NOTIFICATION_STATUS_TYPES_COMPLETED, ) from app.dao.dao_utils import autocommit +from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job from app.dao.templates_dao import dao_get_template_by_id from app.models import ( FactNotificationStatus, @@ -258,3 +261,30 @@ def find_missing_row_for_job(job_id, job_size): .filter(Notification.job_row_number == None) # noqa ) return query.all() + + +redis_cache = RequestCache(redis_store) + + +@redis_cache.set("job-{job_id}-notification-outcomes", ttl_in_seconds=timedelta(days=1).total_seconds()) +def get_possibly_cached_notification_outcomes_for_job( + job_id: int, notification_count: int | None, processing_started: datetime | None +): + if processing_started is None: + statuses = [] + elif processing_started.replace(tzinfo=None) < midnight_n_days_ago(3): + # ft_notification_status table + statuses = fetch_notification_statuses_for_job(job_id) + else: + # notifications table + statuses = dao_get_notification_outcomes_for_job(job_id) + + return RequestCache.CacheResultWrapper( + value=[{"status": status.status, "count": status.count} for status in statuses], + # cache if all rows of the job are accounted for and no + # notifications are in a state still likely to change + cache_decision=bool( + sum(status.count for status in statuses) == notification_count + and all(status.status in NOTIFICATION_STATUS_TYPES_COMPLETED for status in statuses) + ), + ) diff --git a/app/job/rest.py b/app/job/rest.py index e2a509cb77..0c62b3a517 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -11,9 +11,6 @@ JOB_STATUS_SCHEDULED, LETTER_TYPE, ) -from app.dao.fact_notification_status_dao import ( - fetch_notification_statuses_for_job, -) from app.dao.jobs_dao import ( can_letter_job_be_cancelled, dao_cancel_letter_job, @@ -24,6 +21,7 @@ dao_get_scheduled_job_by_id_and_service_id, dao_get_scheduled_job_stats, dao_update_job, + get_possibly_cached_notification_outcomes_for_job, ) from app.dao.notifications_dao import ( dao_get_notification_count_for_job_id, @@ -39,7 +37,7 @@ notifications_filter_schema, unarchived_template_schema, ) -from app.utils import midnight_n_days_ago, pagination_links +from app.utils import pagination_links job_blueprint = Blueprint("job", __name__, url_prefix="/service//job") @@ -220,15 +218,9 @@ def get_paginated_jobs( start = job_data["processing_started"] start = dateutil.parser.parse(start).replace(tzinfo=None) if start else None - if start is None: - statistics = [] - elif start.replace(tzinfo=None) < midnight_n_days_ago(3): - # ft_notification_status table - statistics = fetch_notification_statuses_for_job(job_data["id"]) - else: - # notifications table - statistics = dao_get_notification_outcomes_for_job(job_data["id"]) - job_data["statistics"] = [{"status": statistic.status, "count": statistic.count} for statistic in statistics] + job_data["statistics"] = get_possibly_cached_notification_outcomes_for_job( + job_data["id"], job_data["notification_count"], start + ) return { "data": data, diff --git a/app/upload/rest.py b/app/upload/rest.py index 65103c1640..91995ccb78 100644 --- a/app/upload/rest.py +++ b/app/upload/rest.py @@ -2,17 +2,14 @@ from flask import Blueprint, abort, current_app, jsonify, request -from app.dao.fact_notification_status_dao import ( - fetch_notification_statuses_for_job, -) -from app.dao.jobs_dao import dao_get_notification_outcomes_for_job +from app.dao.jobs_dao import get_possibly_cached_notification_outcomes_for_job from app.dao.uploads_dao import ( dao_get_uploaded_letters_by_print_date, dao_get_uploads_by_service_id, ) from app.errors import register_errors from app.schemas import notification_with_template_schema -from app.utils import midnight_n_days_ago, pagination_links +from app.utils import pagination_links upload_blueprint = Blueprint("upload", __name__, url_prefix="/service//upload") @@ -49,19 +46,9 @@ def get_paginated_uploads(service_id, limit_days, page): "recipient": upload.recipient, } if upload.upload_type == "job": - start = upload.processing_started - - if start is None: - statistics = [] - elif start.replace(tzinfo=None) < midnight_n_days_ago(3): - # ft_notification_status table - statistics = fetch_notification_statuses_for_job(upload.id) - else: - # notifications table - statistics = dao_get_notification_outcomes_for_job(upload.id) - upload_dict["statistics"] = [ - {"status": statistic.status, "count": statistic.count} for statistic in statistics - ] + upload_dict["statistics"] = get_possibly_cached_notification_outcomes_for_job( + upload.id, upload.notification_count, upload.processing_started + ) else: upload_dict["statistics"] = [] data.append(upload_dict) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 72e27a453f..de23452f45 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -1,4 +1,6 @@ +import json import uuid +from collections import Counter from datetime import datetime, timedelta from functools import partial @@ -20,9 +22,11 @@ dao_update_job, find_jobs_with_missing_rows, find_missing_row_for_job, + get_possibly_cached_notification_outcomes_for_job, ) from app.models import Job from tests.app.db import ( + create_ft_notification_status, create_job, create_notification, create_service, @@ -566,3 +570,195 @@ def test_unique_key_on_job_id_and_job_row_number_no_error_if_row_number_for_diff job_2 = create_job(template=sample_email_template) create_notification(job=job_1, job_row_number=0) create_notification(job=job_2, job_row_number=0) + + +@pytest.mark.parametrize( + [ + "notification_statuses", + "notification_count", + "processing_started", + "create_notifications", + "expect_redis_set", + "expected_retval", + ], + ( + ( + ( + "permanent-failure", + "delivered", + "permanent-failure", + "technical-failure", + "technical-failure", + "permanent-failure", + ), + 6, + datetime.fromisoformat("2020-02-10T09:00:00"), + True, + True, + [ + {"status": "delivered", "count": 1}, + {"status": "permanent-failure", "count": 3}, + {"status": "technical-failure", "count": 2}, + ], + ), + ( + ( + "sent", + "delivered", + "delivered", + "delivered", + ), + 4, + datetime.fromisoformat("2020-02-09T09:00:00"), + True, + True, + [ + {"status": "delivered", "count": 3}, + {"status": "sent", "count": 1}, + ], + ), + ( + ( + "technical-failure", + "delivered", + "technical-failure", + ), + 3, + datetime.fromisoformat("2020-02-04T09:00:00"), # so from ft_notification_status + False, + True, + [ + {"status": "technical-failure", "count": 2}, + {"status": "delivered", "count": 1}, + ], + ), + ( + ( + "technical-failure", + "delivered", + "created", + "technical-failure", + ), + 4, + datetime.fromisoformat("2020-02-09T23:59:58"), + True, + False, # because non-complete status + [ + {"status": "technical-failure", "count": 2}, + {"status": "created", "count": 1}, + {"status": "delivered", "count": 1}, + ], + ), + ( + ( + "sent", + "delivered", + "delivered", + ), + 4, + datetime.fromisoformat("2020-02-10T09:00:00"), + True, + False, # because missing rows + [ + {"status": "delivered", "count": 2}, + {"status": "sent", "count": 1}, + ], + ), + ( + ( + "delivered", + "created", + "delivered", + ), + 3, + None, + True, + False, # because non-complete status + [], + ), + ), +) +def test_get_possibly_cached_notification_outcomes_for_job_empty_cache( + sample_email_template, + notification_statuses, + notification_count, + processing_started, + create_notifications, + expect_redis_set, + expected_retval, + mocker, +): + call_datetime = datetime.fromisoformat("2020-02-10T10:00:00") + ft_status_bins = Counter() + + with freeze_time(processing_started) as frozen_time: + job = create_job(template=sample_email_template, processing_started=processing_started) + for i, status in enumerate(notification_statuses): + frozen_time.tick() + + if create_notifications: + create_notification(job=job, status=status, job_row_number=i) + + d = datetime.now().date() + if d != call_datetime.date(): + ft_status_bins[d, status] += 1 + + for (d, status), count in ft_status_bins.items(): + create_ft_notification_status( + bst_date=d, + notification_type="email", + service=job.service, + job=job, + template=job.template, + key_type="normal", + notification_status=status, + count=count, + ) + + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mock_redis_set = mocker.patch("app.redis_store.set", return_value=None) + + with freeze_time(call_datetime): + retval = get_possibly_cached_notification_outcomes_for_job(job.id, notification_count, processing_started) + + assert sorted(retval, key=lambda x: x["status"]) == sorted(expected_retval, key=lambda x: x["status"]) + + if expect_redis_set: + assert mock_redis_set.mock_calls == [ + mocker.call( + f"job-{job.id}-notification-outcomes", json.dumps(retval), ex=timedelta(days=1).total_seconds() + ), + ] + else: + assert not mock_redis_set.mock_calls + + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job.id}-notification-outcomes"), + ] + + +def test_get_possibly_cached_notification_outcomes_for_job_present_cache( + fake_uuid, + mocker, +): + mocker.patch( + "app.dao.jobs_dao.fetch_notification_statuses_for_job", + side_effect=AssertionError("fetch_notification_statuses_for_job call not expected"), + ) + mocker.patch( + "app.dao.jobs_dao.dao_get_notification_outcomes_for_job", + side_effect=AssertionError("dao_get_notification_outcomes_for_job call not expected"), + ) + + mock_redis_get = mocker.patch( + "app.redis_store.get", return_value=b'[{"status": "delivered", "count": 12}, {"status": "sent", "count": 34}]' + ) + mocker.patch("app.redis_store.set", return_value=AssertionError("redis set call not expected")) + + retval = get_possibly_cached_notification_outcomes_for_job(fake_uuid, 46, datetime.now()) + + assert retval == [{"status": "delivered", "count": 12}, {"status": "sent", "count": 34}] + + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{fake_uuid}-notification-outcomes"), + ] diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 46886df5af..d6b5b08362 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -700,9 +700,14 @@ def __create_ft_status(job, status, count): @freeze_time("2017-07-17 07:17") -def test_get_jobs(admin_request, sample_template): +def test_get_jobs(admin_request, sample_template, mocker): _setup_jobs(sample_template) + mock_redis_get = mocker.patch( + "app.redis_store.get", + return_value=None, + ) + service_id = sample_template.service.id resp_json = admin_request.get("job.get_jobs_by_service", service_id=service_id) @@ -731,27 +736,39 @@ def test_get_jobs(admin_request, sample_template): "updated_at": None, "contact_list_id": None, } + assert len(mock_redis_get.mock_calls) == 5 -def test_get_jobs_with_limit_days(admin_request, sample_template): +def test_get_jobs_with_limit_days(admin_request, sample_template, mocker): + jobs = [] for time in [ "Sunday 1st July 2018 22:59", "Sunday 2nd July 2018 23:00", # beginning of monday morning "Monday 3rd July 2018 12:00", ]: with freeze_time(time): - create_job(template=sample_template) + jobs.append(create_job(template=sample_template)) + + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) with freeze_time("Monday 9th July 2018 12:00"): resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id, limit_days=7) assert len(resp_json["data"]) == 2 + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{jobs[2].id}-notification-outcomes"), + mocker.call(f"job-{jobs[1].id}-notification-outcomes"), + ] -def test_get_jobs_by_contact_list(admin_request, sample_template): +def test_get_jobs_by_contact_list(admin_request, sample_template, mocker): contact_list = create_service_contact_list() - create_job(template=sample_template) - create_job(template=sample_template, contact_list_id=contact_list.id) + job_1 = create_job(template=sample_template) # noqa + job_2 = create_job(template=sample_template, contact_list_id=contact_list.id) + + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) resp_json = admin_request.get( "job.get_jobs_by_service", @@ -760,9 +777,12 @@ def test_get_jobs_by_contact_list(admin_request, sample_template): ) assert len(resp_json["data"]) == 1 + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_2.id}-notification-outcomes"), + ] -def test_get_jobs_should_return_statistics(admin_request, sample_template): +def test_get_jobs_should_return_statistics(admin_request, sample_template, mocker): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(sample_template, processing_started=earlier) @@ -774,6 +794,9 @@ def test_get_jobs_should_return_statistics(admin_request, sample_template): create_notification(job=job_2, status="sending") create_notification(job=job_2, status="sending") + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) assert len(resp_json["data"]) == 2 @@ -781,14 +804,21 @@ def test_get_jobs_should_return_statistics(admin_request, sample_template): assert {"status": "sending", "count": 3} in resp_json["data"][0]["statistics"] assert resp_json["data"][1]["id"] == str(job_1.id) assert {"status": "created", "count": 3} in resp_json["data"][1]["statistics"] + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_2.id}-notification-outcomes"), + mocker.call(f"job-{job_1.id}-notification-outcomes"), + ] -def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_request, sample_template): +def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_request, sample_template, mocker): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(sample_template, created_at=earlier) job_2 = create_job(sample_template, created_at=now) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) assert len(resp_json["data"]) == 2 @@ -796,11 +826,18 @@ def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_reque assert resp_json["data"][0]["statistics"] == [] assert resp_json["data"][1]["id"] == str(job_1.id) assert resp_json["data"][1]["statistics"] == [] + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_2.id}-notification-outcomes"), + mocker.call(f"job-{job_1.id}-notification-outcomes"), + ] -def test_get_jobs_should_paginate(admin_request, sample_template): +def test_get_jobs_should_paginate(admin_request, sample_template, mocker): create_10_jobs(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + with set_config(admin_request.app, "PAGE_SIZE", 2): resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) @@ -810,11 +847,15 @@ def test_get_jobs_should_paginate(admin_request, sample_template): assert resp_json["total"] == 10 assert "links" in resp_json assert set(resp_json["links"].keys()) == {"next", "last"} + assert len(mock_redis_get.mock_calls) == 2 -def test_get_jobs_accepts_page_parameter(admin_request, sample_template): +def test_get_jobs_accepts_page_parameter(admin_request, sample_template, mocker): create_10_jobs(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + with set_config(admin_request.app, "PAGE_SIZE", 2): resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id, page=2) @@ -824,6 +865,10 @@ def test_get_jobs_accepts_page_parameter(admin_request, sample_template): assert resp_json["total"] == 10 assert "links" in resp_json assert set(resp_json["links"].keys()) == {"prev", "next", "last"} + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + ] @pytest.mark.parametrize( @@ -839,7 +884,7 @@ def test_get_jobs_accepts_page_parameter(admin_request, sample_template): ("foo", []), ], ) -def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuses_filter, expected_statuses): +def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuses_filter, expected_statuses, mocker): create_job(sample_template, job_status="pending") create_job(sample_template, job_status="in progress") create_job(sample_template, job_status="finished") @@ -850,11 +895,15 @@ def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuse create_job(sample_template, job_status="sent to dvla") create_job(sample_template, job_status="error") + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get( "job.get_jobs_by_service", service_id=sample_template.service_id, statuses=statuses_filter ) assert {x["job_status"] for x in resp_json["data"]} == set(expected_statuses) + assert len(mock_redis_get.mock_calls) == len(expected_statuses) def create_10_jobs(template): @@ -890,7 +939,7 @@ def test_get_all_notifications_for_job_returns_csv_format(admin_request, sample_ @freeze_time("2017-06-10 12:00") -def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template): +def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template, mocker): # it's the 10th today, so 3 days should include all of 7th, 8th, 9th, and some of 10th. just_three_days_ago = datetime(2017, 6, 6, 22, 59, 59) not_quite_three_days_ago = just_three_days_ago + timedelta(seconds=1) @@ -914,6 +963,11 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin # this isn't picked up because we're using the ft status table for job_1 as it's old create_notification(job=job_1, status="created", created_at=not_quite_three_days_ago) + mock_redis_get = mocker.patch( + "app.redis_store.get", + return_value=None, + ) + resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) assert resp_json["data"][0]["id"] == str(job_3.id) @@ -922,6 +976,11 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin assert resp_json["data"][1]["statistics"] == [{"status": "created", "count": 1}] assert resp_json["data"][2]["id"] == str(job_1.id) assert resp_json["data"][2]["statistics"] == [{"status": "delivered", "count": 6}] + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][2]["id"]}-notification-outcomes'), + ] @freeze_time("2017-07-17 07:17") diff --git a/tests/app/upload/test_rest.py b/tests/app/upload/test_rest.py index f2bcc53373..79e2d20bcf 100644 --- a/tests/app/upload/test_rest.py +++ b/tests/app/upload/test_rest.py @@ -38,7 +38,7 @@ def create_precompiled_template(service): @freeze_time("2020-02-02 14:00") -def test_get_uploads(admin_request, sample_template): +def test_get_uploads(admin_request, sample_template, mocker): letter_template = create_precompiled_template(sample_template.service) create_uploaded_letter( @@ -69,6 +69,9 @@ def test_get_uploads(admin_request, sample_template): service_id = sample_template.service.id + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("upload.get_uploads_by_service", service_id=service_id) data = resp_json["data"] assert len(data) == 4 @@ -103,8 +106,14 @@ def test_get_uploads(admin_request, sample_template): "upload_type": "job", } + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{upload_5.id}-notification-outcomes"), + mocker.call(f"job-{upload_4.id}-notification-outcomes"), + mocker.call(f"job-{upload_2.id}-notification-outcomes"), + ] + -def test_get_uploads_should_return_statistics(admin_request, sample_template): +def test_get_uploads_should_return_statistics(admin_request, sample_template, mocker): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(template=sample_template, job_status="pending") @@ -121,6 +130,9 @@ def test_get_uploads_should_return_statistics(admin_request, sample_template): letter_template, sample_template.service, status="delivered", created_at=datetime.utcnow() - timedelta(days=3) ) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id)["data"] assert len(resp_json) == 4 assert resp_json[0]["id"] == str(job_1.id) @@ -132,11 +144,20 @@ def test_get_uploads_should_return_statistics(admin_request, sample_template): assert resp_json[3]["id"] is None assert resp_json[3]["statistics"] == [] + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_1.id}-notification-outcomes"), + mocker.call(f"job-{job_3.id}-notification-outcomes"), + mocker.call(f"job-{job_2.id}-notification-outcomes"), + ] -def test_get_uploads_should_paginate(admin_request, sample_template): + +def test_get_uploads_should_paginate(admin_request, sample_template, mocker): for _ in range(10): create_job(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + with set_config(admin_request.app, "PAGE_SIZE", 2): resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id) @@ -146,11 +167,19 @@ def test_get_uploads_should_paginate(admin_request, sample_template): assert "links" in resp_json assert set(resp_json["links"].keys()) == {"next", "last"} + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + ] -def test_get_uploads_accepts_page_parameter(admin_request, sample_template): + +def test_get_uploads_accepts_page_parameter(admin_request, sample_template, mocker): for _ in range(10): create_job(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + with set_config(admin_request.app, "PAGE_SIZE", 2): resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id, page=2) @@ -160,9 +189,14 @@ def test_get_uploads_accepts_page_parameter(admin_request, sample_template): assert "links" in resp_json assert set(resp_json["links"].keys()) == {"prev", "next", "last"} + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + ] + @freeze_time("2017-06-10 12:00") -def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template): +def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template, mocker): # it's the 10th today, so 3 days should include all of 7th, 8th, 9th, and some of 10th. just_three_days_ago = datetime(2017, 6, 6, 22, 59, 59) not_quite_three_days_ago = just_three_days_ago + timedelta(seconds=1) @@ -186,6 +220,9 @@ def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(ad # this isn't picked up because we're using the ft status table for job_1 as it's old create_notification(job=job_1, status="created", created_at=not_quite_three_days_ago) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id)["data"] assert resp_json[0]["id"] == str(job_3.id) @@ -195,6 +232,12 @@ def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(ad assert resp_json[2]["id"] == str(job_1.id) assert resp_json[2]["statistics"] == [{"status": "delivered", "count": 6}] + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json[0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json[1]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json[2]["id"]}-notification-outcomes'), + ] + @freeze_time("2020-02-02 14:00") def test_get_uploaded_letters_by_print_date(admin_request, sample_template):