From 822cf5aa6846f4540ccea5fb9cb308779c56332f Mon Sep 17 00:00:00 2001 From: Alex Janousek Date: Wed, 1 Oct 2025 11:42:46 -0400 Subject: [PATCH] Added light polling endpoint (#2006) * Added light polling endpoint * Import linting * CICD checks updated --- .gitignore | 1 + app/job/rest.py | 82 ++++++----------------- tests/app/job/test_rest.py | 130 ++++++++++++++++++++++++++++--------- 3 files changed, 120 insertions(+), 93 deletions(-) diff --git a/.gitignore b/.gitignore index cf35582a6..f433ea7d4 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,7 @@ htmlcov/ .coverage.* .cache .pytest_cache +.hypothesis/ coverage.xml test_results.xml *,cover diff --git a/app/job/rest.py b/app/job/rest.py index fa3b810c8..45207147c 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -1,10 +1,9 @@ -import json from zoneinfo import ZoneInfo import dateutil from flask import Blueprint, current_app, jsonify, request -from app import db, redis_store +from app import db from app.aws.s3 import ( extract_personalisation, extract_phones, @@ -32,7 +31,7 @@ from app.dao.notifications_dao import ( ) from app.dao.services_dao import dao_fetch_service_by_id from app.dao.templates_dao import dao_get_template_by_id -from app.enums import JobStatus +from app.enums import JobStatus, NotificationStatus from app.errors import InvalidRequest, register_errors from app.schemas import ( JobSchema, @@ -65,78 +64,35 @@ def get_job_by_service_and_job_id(service_id, job_id): @job_blueprint.route("//status", methods=["GET"]) def get_job_status(service_id, job_id): - """Lightweight endpoint for polling job status with aggressive caching.""" - current_app.logger.info( - f"Getting job status for service_id={service_id}, job_id={job_id}" - ) + """Fast job status endpoint for real-time polling. No S3 calls, no caching.""" check_suspicious_id(service_id, job_id) - # Check cache first (10 second TTL) - cache_key = f"job_status:{service_id}:{job_id}" - cached_status = redis_store.get(cache_key) - - if cached_status: - current_app.logger.info(f"Returning cached status for job {job_id}") - return jsonify(json.loads(cached_status)) - job = dao_get_job_by_service_id_and_job_id(service_id, job_id) - statistics = dao_get_notification_outcomes_for_job(service_id, job_id) - sent_count = 0 - failed_count = 0 - pending_count = 0 - - status_mapping = { - "sent": ["sent"], - "delivered": ["sent"], - "failed": [ - "failed", - "permanent-failure", - "temporary-failure", - "technical-failure", - "virus-scan-failed", - ], - "pending": ["created", "sending", "pending", "pending-virus-check"], - } + delivered_statuses = (NotificationStatus.DELIVERED, NotificationStatus.SENT) + failed_statuses = (NotificationStatus.FAILED,) + NotificationStatus.failed_types() + delivered_count = failed_count = 0 for stat in statistics: - count = stat[0] if isinstance(stat, tuple) else stat.count - status = stat[1] if isinstance(stat, tuple) else stat.status + if stat.status in delivered_statuses: + delivered_count += stat.count + elif stat.status in failed_statuses: + failed_count += stat.count - if status in status_mapping.get( - "delivered", [] - ) or status in status_mapping.get("sent", []): - sent_count += count - elif status in status_mapping.get("failed", []): - failed_count += count - elif status in status_mapping.get("pending", []): - pending_count += count - else: - if job.processing_finished: - sent_count += count - else: - pending_count += count + total_count = job.notification_count or 0 + pending_calculated = max(0, total_count - delivered_count - failed_count) + + is_finished = job.processing_finished is not None and pending_calculated == 0 response_data = { - "sent_count": sent_count, - "failed_count": failed_count, - "pending_count": pending_count, - "total_count": job.notification_count, - "job_status": job.job_status, - "processing_finished": job.processing_finished is not None, + "total": total_count, + "delivered": delivered_count, + "failed": failed_count, + "pending": pending_calculated, + "finished": is_finished, } - if job.processing_finished: - # Finished jobs can be cached for 5 minutes since they won't change - cache_ttl = 300 - else: - # Active jobs cached for 10 seconds - cache_ttl = 10 - - redis_store.set(cache_key, json.dumps(response_data), ex=cache_ttl) - current_app.logger.info(f"Cached status for job {job_id} with TTL={cache_ttl}s") - return jsonify(response_data) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 7f1552964..595bffabc 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -1215,16 +1215,16 @@ def test_get_scheduled_job_stats(admin_request): } -def test_get_job_status_returns_lightweight_response(admin_request, sample_job): - """Test that the new status endpoint returns only essential fields.""" +def test_get_job_status_returns_light_response(admin_request, sample_job): + """Test that the status endpoint returns only required fields.""" job_id = str(sample_job.id) service_id = sample_job.service.id + sample_job.notification_count = 5 + create_notification(job=sample_job, status=NotificationStatus.SENT) - create_notification(job=sample_job, status=NotificationStatus.SENT) + create_notification(job=sample_job, status=NotificationStatus.DELIVERED) create_notification(job=sample_job, status=NotificationStatus.FAILED) - create_notification(job=sample_job, status=NotificationStatus.PENDING) - create_notification(job=sample_job, status=NotificationStatus.CREATED) resp_json = admin_request.get( "job.get_job_status", @@ -1233,42 +1233,112 @@ def test_get_job_status_returns_lightweight_response(admin_request, sample_job): ) assert set(resp_json.keys()) == { - "sent_count", - "failed_count", - "pending_count", - "total_count", - "job_status", - "processing_finished", + "total", + "delivered", + "failed", + "pending", + "finished", } - # Verify counts are correct - assert resp_json["sent_count"] == 2 - assert resp_json["failed_count"] == 1 - assert resp_json["pending_count"] == 2 - assert resp_json["total_count"] == sample_job.notification_count - assert resp_json["job_status"] == sample_job.job_status - assert resp_json["processing_finished"] == ( - sample_job.processing_finished is not None - ) + assert resp_json["total"] == 5 + assert resp_json["delivered"] == 2 # sent + delivered + assert resp_json["failed"] == 1 + assert resp_json["pending"] == 2 # total - delivered - failed + assert resp_json["finished"] is False -def test_get_job_status_caches_response(admin_request, sample_job, mocker): - """Test that the status endpoint uses caching.""" +def test_get_job_status_counts_all_delivered_statuses(admin_request, sample_job): + """Test that delivered count includes both 'delivered' and 'sent' statuses.""" job_id = str(sample_job.id) service_id = sample_job.service.id + sample_job.notification_count = 4 + create_notification(job=sample_job, status=NotificationStatus.SENT) + create_notification(job=sample_job, status=NotificationStatus.SENT) + create_notification(job=sample_job, status=NotificationStatus.DELIVERED) + create_notification(job=sample_job, status=NotificationStatus.DELIVERED) - mock_redis_get = mocker.patch("app.job.rest.redis_store.get", return_value=None) - mock_redis_set = mocker.patch("app.job.rest.redis_store.set") - - # First request should cache the result - admin_request.get( + resp_json = admin_request.get( "job.get_job_status", service_id=service_id, job_id=job_id, ) - cache_key = f"job_status:{service_id}:{job_id}" - mock_redis_get.assert_called_once_with(cache_key) - mock_redis_set.assert_called_once() + assert resp_json["delivered"] == 4 + assert resp_json["failed"] == 0 + assert resp_json["pending"] == 0 + + +def test_get_job_status_counts_all_failed_statuses(admin_request, sample_job): + """Test that failed count includes all failure status types.""" + job_id = str(sample_job.id) + service_id = sample_job.service.id + + sample_job.notification_count = 6 + + create_notification(job=sample_job, status=NotificationStatus.FAILED) + create_notification(job=sample_job, status=NotificationStatus.TECHNICAL_FAILURE) + create_notification(job=sample_job, status=NotificationStatus.TEMPORARY_FAILURE) + create_notification(job=sample_job, status=NotificationStatus.PERMANENT_FAILURE) + create_notification(job=sample_job, status=NotificationStatus.VALIDATION_FAILED) + create_notification(job=sample_job, status=NotificationStatus.VIRUS_SCAN_FAILED) + + resp_json = admin_request.get( + "job.get_job_status", + service_id=service_id, + job_id=job_id, + ) + + assert resp_json["delivered"] == 0 + assert resp_json["failed"] == 6 + assert resp_json["pending"] == 0 + + +def test_get_job_status_finished_when_processing_complete_and_no_pending( + admin_request, sample_job +): + """Test that finished is True only when processing_finished is set and pending is 0.""" + from app.utils import utc_now + + job_id = str(sample_job.id) + service_id = sample_job.service.id + + sample_job.notification_count = 2 + sample_job.processing_finished = utc_now() + + create_notification(job=sample_job, status=NotificationStatus.DELIVERED) + create_notification(job=sample_job, status=NotificationStatus.DELIVERED) + + resp_json = admin_request.get( + "job.get_job_status", + service_id=service_id, + job_id=job_id, + ) + + assert resp_json["pending"] == 0 + assert resp_json["finished"] is True + + +def test_get_job_status_not_finished_when_pending_exists(admin_request, sample_job): + """Test that finished is False when there are still pending notifications.""" + from app.utils import utc_now + + job_id = str(sample_job.id) + service_id = sample_job.service.id + + sample_job.notification_count = 5 + sample_job.processing_finished = utc_now() + + create_notification(job=sample_job, status=NotificationStatus.DELIVERED) + + resp_json = admin_request.get( + "job.get_job_status", + service_id=service_id, + job_id=job_id, + ) + + assert resp_json["pending"] == 4 + assert ( + resp_json["finished"] is False + ) # Still has pending even though processing_finished is set