Optimizing polling (#2946)

* Optimizing polling

* Fixed formatting issue
This commit is contained in:
Alex Janousek
2025-09-26 06:57:18 -04:00
committed by GitHub
parent 5a9c9824aa
commit a40c8861bf
15 changed files with 528 additions and 163 deletions

View File

@@ -161,7 +161,7 @@
"filename": "app/config.py",
"hashed_secret": "577a4c667e4af8682ca431857214b3a920883efc",
"is_verified": false,
"line_number": 124,
"line_number": 122,
"is_secret": false
}
],
@@ -634,5 +634,5 @@
}
]
},
"generated_at": "2025-09-24T22:03:22Z"
"generated_at": "2025-09-25T20:25:57Z"
}

1
.gitignore vendored
View File

@@ -85,6 +85,7 @@ coverage/
coverage.xml
test_results.xml
*,cover
.hypothesis/
# Translations
*.mo

View File

@@ -0,0 +1,186 @@
document.addEventListener('DOMContentLoaded', function () {
// Verify we are on the job page
const isJobPage = window.location.pathname.includes('/jobs/');
if (!isJobPage) return;
// Check if polling elements exist
const hasPollingElements = document.querySelector('[data-key="counts"]');
if (!hasPollingElements) return;
// Extract job info from URL path: /services/{serviceId}/jobs/{jobId}
const pathParts = window.location.pathname.split('/');
if (pathParts.length < 5 || pathParts[1] !== 'services' || pathParts[3] !== 'jobs') return;
const serviceId = pathParts[2];
const jobId = pathParts[4];
// Validate service and job IDs to prevent path injection
function isValidUuid(id) {
const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
return uuidRegex.test(id);
}
// Validate both IDs are UUIDs to prevent path injection attacks
if (!isValidUuid(serviceId) || !isValidUuid(jobId)) {
console.warn('Invalid service or job ID format detected');
return;
}
const DEFAULT_INTERVAL_MS = 10000;
const MIN_INTERVAL_MS = 1000;
const MAX_INTERVAL_MS = 30000;
let pollInterval;
let currentInterval = DEFAULT_INTERVAL_MS;
let isPolling = false;
let lastProcessedCount = 0;
function calculateBackoff(responseTime) {
return Math.min(
MAX_INTERVAL_MS,
Math.max(
MIN_INTERVAL_MS,
Math.floor((250 * Math.sqrt(responseTime)) - 1000)
)
);
}
async function updateNotifications() {
const notificationsUrl = `/services/${serviceId}/jobs/${jobId}.json`;
try {
const response = await fetch(notificationsUrl);
if (!response.ok) {
throw new Error(`Failed to fetch notifications: ${response.status}`);
}
const data = await response.json();
// Update notifications container if it exists
const notificationsContainer = document.querySelector('[data-key="notifications"]');
if (notificationsContainer && data.notifications) {
notificationsContainer.innerHTML = data.notifications;
}
} catch (error) {
console.warn('Failed to update notifications:', error.message);
}
}
async function updateAllJobSections(retryCount = 0) {
if (isPolling || document.hidden) {
return;
}
isPolling = true;
const pollStatusUrl = `/services/${serviceId}/jobs/${jobId}/status.json`;
try {
const response = await fetch(pollStatusUrl);
if (!response.ok) {
throw new Error(`HTTP ${response.status}: ${response.statusText}`);
}
const data = await response.json();
const countsContainer = document.querySelector('[data-key="counts"]');
if (countsContainer) {
// Get all big-number elements in order: total, pending, delivered, failed
const countElements = countsContainer.querySelectorAll('.big-number-number');
if (countElements.length >= 4) {
if (data.total_count !== undefined) {
countElements[0].textContent = data.total_count.toLocaleString();
}
if (data.pending_count !== undefined) {
countElements[1].textContent = data.pending_count.toLocaleString();
}
if (data.sent_count !== undefined) {
countElements[2].textContent = data.sent_count.toLocaleString();
}
if (data.failed_count !== undefined) {
countElements[3].textContent = data.failed_count.toLocaleString();
}
}
}
currentInterval = calculateBackoff(DEFAULT_INTERVAL_MS);
// Calculate how many messages have been processed
const processedCount = (data.sent_count || 0) + (data.failed_count || 0);
// Update notifications conditionally:
// 1. If we have new messages and still under 50 total
// 2. Always when job is finished
if (processedCount > lastProcessedCount && processedCount <= 50 && !data.finished) {
// Update notifications for first 50 messages to show early results
await updateNotifications();
lastProcessedCount = processedCount;
}
if (data.finished === true) {
await updateNotifications();
stopPolling();
}
} catch (error) {
if (retryCount < 3) {
console.debug(`Job polling retry ${retryCount}`, error.message);
isPolling = false;
const retryDelay = Math.pow(2, retryCount) * 1000;
setTimeout(() => {
updateAllJobSections(retryCount + 1);
}, retryDelay);
return;
}
console.warn('Job polling failed after 3 retries:', {
error: error.message,
url: pollStatusUrl,
jobId: jobId,
timestamp: new Date().toISOString()
});
currentInterval = Math.min(currentInterval * 2, MAX_INTERVAL_MS);
} finally {
isPolling = false;
}
}
function startPolling() {
updateAllJobSections();
function scheduleNext() {
if (pollInterval) clearTimeout(pollInterval);
pollInterval = setTimeout(() => {
updateAllJobSections();
scheduleNext();
}, currentInterval);
}
scheduleNext();
}
function stopPolling() {
if (pollInterval) {
clearTimeout(pollInterval);
pollInterval = null;
}
}
document.addEventListener('visibilitychange', () => {
if (document.hidden) {
stopPolling();
} else {
startPolling();
}
});
window.addEventListener('beforeunload', stopPolling);
startPolling();
});

View File

@@ -1,132 +0,0 @@
document.addEventListener('DOMContentLoaded', function () {
const isJobPage = window.location.pathname.includes('/jobs/');
if (!isJobPage) return;
const jobEl = document.querySelector('[data-job-id]');
const jobId = jobEl?.dataset?.jobId;
const featureEnabled = jobEl?.dataset?.feature === 'true';
const apiHost = jobEl?.dataset?.host;
if (!jobId || !featureEnabled) return;
const DEFAULT_INTERVAL_MS = 10000;
const MIN_INTERVAL_MS = 1000;
const MAX_INTERVAL_MS = 30000;
let pollInterval;
let currentInterval = DEFAULT_INTERVAL_MS;
let isPolling = false;
function calculateBackoff(responseTime) {
return Math.min(
MAX_INTERVAL_MS,
Math.max(
MIN_INTERVAL_MS,
Math.floor((250 * Math.sqrt(responseTime)) - 1000)
)
);
}
async function updateAllJobSections(retryCount = 0) {
if (isPolling || document.hidden) {
return;
}
isPolling = true;
const startTime = Date.now();
const resourceEl = document.querySelector('[data-socket-update="status"]');
const url = resourceEl?.dataset?.resource;
if (!url) {
isPolling = false;
return;
}
try {
const response = await fetch(url);
if (!response.ok) {
throw new Error(`HTTP ${response.status}: ${response.statusText}`);
}
const data = await response.json();
const sections = {
status: document.querySelector('[data-socket-update="status"]'),
counts: document.querySelector('[data-socket-update="counts"]'),
notifications: document.querySelector('[data-socket-update="notifications"]'),
};
if (data.status && sections.status) {
sections.status.innerHTML = data.status;
}
if (data.counts && sections.counts) {
sections.counts.innerHTML = data.counts;
}
if (data.notifications && sections.notifications) {
sections.notifications.innerHTML = data.notifications;
}
const responseTime = Date.now() - startTime;
currentInterval = calculateBackoff(responseTime);
if (data.finished === true) {
stopPolling();
}
} catch (error) {
if (retryCount < 3) {
console.debug(`Job polling retry ${retryCount}`, error.message);
isPolling = false;
const retryDelay = Math.pow(2, retryCount) * 1000;
setTimeout(() => {
updateAllJobSections(retryCount + 1);
}, retryDelay);
return;
}
console.warn('Job polling failed after 3 retries:', {
error: error.message,
url: url,
jobId: jobId,
timestamp: new Date().toISOString()
});
currentInterval = Math.min(currentInterval * 2, MAX_INTERVAL_MS);
} finally {
isPolling = false;
}
}
function startPolling() {
updateAllJobSections();
function scheduleNext() {
if (pollInterval) clearTimeout(pollInterval);
pollInterval = setTimeout(() => {
updateAllJobSections();
scheduleNext();
}, currentInterval);
}
scheduleNext();
}
function stopPolling() {
if (pollInterval) {
clearTimeout(pollInterval);
pollInterval = null;
}
}
document.addEventListener('visibilitychange', () => {
if (document.hidden) {
stopPolling();
} else {
startPolling();
}
});
window.addEventListener('beforeunload', stopPolling);
startPolling();
});

View File

@@ -90,9 +90,7 @@ class Config(object):
],
}
# TODO FIX!!!
# FEATURE_SOCKET_ENABLED = getenv("FEATURE_SOCKET_ENABLED", "true") == "true"
FEATURE_SOCKET_ENABLED = False
FEATURE_SOCKET_ENABLED = getenv("FEATURE_SOCKET_ENABLED", "true") == "true"
def _s3_credentials_from_env(bucket_prefix):

View File

@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
import json
import os
from functools import partial
@@ -67,12 +68,6 @@ def view_job(service_id, job_id):
FEATURE_SOCKET_ENABLED=current_app.config["FEATURE_SOCKET_ENABLED"],
job=job,
status=request.args.get("status", ""),
updates_url=url_for(
".view_job_updates",
service_id=service_id,
job_id=job.id,
status=request.args.get("status", ""),
),
partials=get_job_partials(job),
)
@@ -112,11 +107,48 @@ def cancel_job(service_id, job_id):
return redirect(url_for("main.service_dashboard", service_id=service_id))
@main.route("/services/<uuid:service_id>/jobs/<uuid:job_id>/status.json")
@user_has_permissions()
def view_job_status_poll(service_id, job_id):
"""
Poll status endpoint that only queries jobs table.
Returns minimal data needed for polling.
"""
import time
start_time = time.time()
job = Job.from_id(job_id, service_id=service_id)
processed_count = job.notifications_delivered + job.notifications_failed
total_count = job.notification_count
response_data = {
"sent_count": job.notifications_delivered,
"failed_count": job.notifications_failed,
"pending_count": job.notifications_sending,
"total_count": total_count,
"finished": job.finished_processing,
}
response_time_ms = round((time.time() - start_time) * 1000, 2)
response_json = json.dumps(response_data)
response_size_bytes = len(response_json.encode("utf-8"))
current_app.logger.info(
f"Poll status request - job_id={job_id[:8]} "
f"response_size={response_size_bytes}b "
f"response_time={response_time_ms}ms "
f"progress={processed_count}/{total_count}"
)
return jsonify(response_data)
@main.route("/services/<uuid:service_id>/jobs/<uuid:job_id>.json")
@user_has_permissions()
def view_job_updates(service_id, job_id):
job = Job.from_id(job_id, service_id=service_id)
return jsonify(**get_job_partials(job))

View File

@@ -92,7 +92,11 @@ def organization_dashboard(org_id):
def download_organization_usage_report(org_id):
selected_year_input = request.args.get("selected_year")
# Validate selected_year to prevent header injection
if selected_year_input and selected_year_input.isdigit() and len(selected_year_input) == 4:
if (
selected_year_input
and selected_year_input.isdigit()
and len(selected_year_input) == 4
):
selected_year = selected_year_input
else:
selected_year = str(datetime.now().year)
@@ -128,8 +132,9 @@ def download_organization_usage_report(org_id):
# Sanitize organization name for filename to prevent header injection
import re
safe_org_name = re.sub(r'[^\w\s-]', '', current_organization.name).strip()
safe_org_name = re.sub(r'[-\s]+', '-', safe_org_name)
safe_org_name = re.sub(r"[^\w\s-]", "", current_organization.name).strip()
safe_org_name = re.sub(r"[-\s]+", "-", safe_org_name)
return (
Spreadsheet.from_rows(org_usage_data).as_csv_data,

View File

@@ -198,16 +198,14 @@ def process_folder_management_form(form, current_folder_id):
# Use request.full_path which includes query string but not host
# This avoids host header injection while preserving all parameters
# Hardened redirect: only allow relative URLs, and strip any backslashes
target = request.full_path.replace('\\', '')
target = request.full_path.replace("\\", "")
parts = urlparse(target)
if not parts.scheme and not parts.netloc and target.startswith('/'):
if not parts.scheme and not parts.netloc and target.startswith("/"):
return redirect(target)
# Fallback to main template list for this service
return redirect(url_for(
'.choose_template',
service_id=current_service.id,
template_type='all'
))
return redirect(
url_for(".choose_template", service_id=current_service.id, template_type="all")
)
def get_template_nav_label(value):

View File

@@ -14,10 +14,9 @@
{% if not job.finished_processing %}
<p class="max-width-full">This page refreshes automatically to show the latest message activity delivery rates, details, and reports.<br>You can watch it in progress or check back later.</p>
{% endif %}
<div data-job-id="{{ job.id }}" data-feature="{{FEATURE_SOCKET_ENABLED | lower}}" data-host="{{ api_public_url }}">
<div data-job-id="{{ job.id }}" data-host="{{ api_public_url }}">
{% if not job.finished_processing and FEATURE_SOCKET_ENABLED %}
<div
data-resource="{{ updates_url }}"
data-socket-update="status"
data-key="status"
data-form=""
@@ -29,7 +28,6 @@
{% endif %}
{% if not finished and FEATURE_SOCKET_ENABLED %}
<div
data-resource="{{ updates_url }}"
data-socket-update="counts"
data-key="counts"
data-form=""
@@ -45,7 +43,6 @@
</p>
{% if not job.processing_finished and FEATURE_SOCKET_ENABLED %}
<div
data-resource="{{ updates_url }}"
data-socket-update="notifications"
data-key="notifications"
data-form=""

View File

@@ -58,7 +58,7 @@ def is_safe_redirect_url(target):
if not target:
return False
target = target.replace('\\', '')
target = target.replace("\\", "")
parsed = urlparse(target)

View File

@@ -79,7 +79,7 @@ const javascripts = () => {
paths.src + 'javascripts/activityChart.js',
paths.src + 'javascripts/sidenav.js',
paths.src + 'javascripts/validation.js',
paths.src + 'javascripts/socketio.js',
paths.src + 'javascripts/job-status-polling.js',
paths.src + 'javascripts/scrollPosition.js',
])
.pipe(plugins.prettyerror())

View File

@@ -0,0 +1,141 @@
"""
Tests for job notification update logic during polling.
These tests verify the poll status endpoint behavior and document
the JavaScript notification refresh logic:
1. Notifications update for first 50 messages
2. Notifications stop updating after 50 messages (to prevent performance issues)
3. Notifications always update when job finishes
"""
import json
import pytest
from tests import job_json, user_json
@pytest.mark.parametrize(
("delivered", "failed", "pending", "finished", "js_should_update_notifications", "reason"),
[
(20, 10, 70, False, True, "30 messages processed (≤50 threshold)"),
(40, 10, 50, False, True, "50 messages processed (exactly at threshold)"),
(45, 15, 40, False, False, "60 messages processed (>50 threshold)"),
(450, 50, 0, True, True, "500 messages but job finished (always updates)"),
],
)
def test_poll_status_notification_update_logic(
client_request,
service_one,
active_user_with_permissions,
mock_get_service_data_retention,
mocker,
fake_uuid,
delivered,
failed,
pending,
finished,
js_should_update_notifications,
reason,
):
"""
Test poll status endpoint for various scenarios.
The JavaScript updates notifications when:
processedCount ≤ 50 AND job not finished
job is finished (regardless of count)
"""
total = delivered + failed + pending
job_status = "finished" if finished else "sending"
mock_job = mocker.patch("app.job_api_client.get_job")
mock_job.return_value = {
"data": {
**job_json(
service_one["id"],
created_by=user_json(),
job_id=fake_uuid,
job_status=job_status,
notification_count=total,
notifications_requested=total,
),
"statistics": [
{"status": "delivered", "count": delivered},
{"status": "failed", "count": failed},
{"status": "pending", "count": pending},
],
}
}
response = client_request.get_response(
"main.view_job_status_poll",
service_id=service_one["id"],
job_id=fake_uuid,
)
assert response.status_code == 200
data = json.loads(response.get_data(as_text=True))
# Verify the response
assert data["sent_count"] == delivered
assert data["failed_count"] == failed
assert data["pending_count"] == pending
assert data["total_count"] == total
assert data["finished"] is finished
processed_count = delivered + failed
if js_should_update_notifications:
# JavaScript would call: await updateNotifications()
if finished:
assert finished, f"JS updates notifications: {reason}"
else:
assert processed_count <= 50, f"JS updates notifications: {reason}"
assert not finished, f"JS updates notifications: {reason}"
else:
# JavaScript would NOT update notifications
assert processed_count > 50, f"JS skips notification update: {reason}"
assert not finished, f"JS skips notification update: {reason}"
def test_poll_status_provides_required_fields(
client_request,
service_one,
active_user_with_permissions,
mock_get_service_data_retention,
mocker,
fake_uuid,
):
"""Verify poll status endpoint returns all fields needed for notification update logic."""
mock_job = mocker.patch("app.job_api_client.get_job")
mock_job.return_value = {
"data": {
**job_json(
service_one["id"],
created_by=user_json(),
job_id=fake_uuid,
job_status="sending",
notification_count=25,
notifications_requested=25,
),
"statistics": [
{"status": "delivered", "count": 15},
{"status": "failed", "count": 5},
{"status": "pending", "count": 5},
],
}
}
response = client_request.get_response(
"main.view_job_status_poll",
service_id=service_one["id"],
job_id=fake_uuid,
)
data = json.loads(response.get_data(as_text=True))
required_fields = {"sent_count", "failed_count", "finished", "pending_count", "total_count"}
assert set(data.keys()) == required_fields
response_size = len(response.get_data(as_text=True))
assert response_size < 200, f"Response too large: {response_size} bytes"

View File

@@ -499,3 +499,143 @@ def test_should_show_message_note(
'Messages are sent immediately to the cell phone carrier, but will remain in "pending" status until we hear '
"back from the carrier they have received it and attempted deliver. More information on delivery status."
)
def test_poll_status_endpoint(
client_request,
service_one,
active_user_with_permissions,
mock_get_service_data_retention,
mocker,
fake_uuid,
):
"""Test that the poll status endpoint returns only required data without notifications"""
mock_job = mocker.patch("app.job_api_client.get_job")
mock_job.return_value = {
"data": {
**job_json(
service_one["id"],
created_by=user_json(),
job_id=fake_uuid,
job_status="finished",
notification_count=100,
notifications_requested=100,
),
"statistics": [
{"status": "delivered", "count": 90},
{"status": "failed", "count": 10},
{"status": "pending", "count": 0},
],
}
}
response = client_request.get_response(
"main.view_job_status_poll",
service_id=service_one["id"],
job_id=fake_uuid,
)
assert response.status_code == 200
data = json.loads(response.get_data(as_text=True))
expected_keys = {
"sent_count",
"failed_count",
"pending_count",
"total_count",
"finished",
}
assert set(data.keys()) == expected_keys
assert data["sent_count"] == 90
assert data["failed_count"] == 10
assert data["pending_count"] == 0
assert data["total_count"] == 100
assert data["finished"] is True
def test_poll_status_with_zero_notifications(
client_request,
service_one,
active_user_with_permissions,
mock_get_service_data_retention,
mocker,
fake_uuid,
):
"""Test poll status endpoint handles edge case of no notifications"""
mock_job = mocker.patch("app.job_api_client.get_job")
mock_job.return_value = {
"data": {
**job_json(
service_one["id"],
created_by=user_json(),
job_id=fake_uuid,
job_status="pending",
notification_count=0,
notifications_requested=0,
),
"statistics": [],
}
}
response = client_request.get_response(
"main.view_job_status_poll",
service_id=service_one["id"],
job_id=fake_uuid,
)
assert response.status_code == 200
data = json.loads(response.get_data(as_text=True))
assert data["total_count"] == 0
assert (
data["finished"] is True
)
def test_poll_status_endpoint_does_not_query_notifications_table(
client_request,
service_one,
active_user_with_permissions,
mock_get_service_data_retention,
mocker,
fake_uuid,
):
"""Critical regression test: ensure poll status endpoint never queries notifications"""
mock_job = mocker.patch("app.job_api_client.get_job")
mock_job.return_value = {
"data": {
**job_json(
service_one["id"],
created_by=user_json(),
job_id=fake_uuid,
job_status="sending",
notification_count=500,
notifications_requested=500,
),
"statistics": [
{"status": "delivered", "count": 300},
{"status": "failed", "count": 50},
{"status": "pending", "count": 150},
],
}
}
mock_get_notifications = mocker.patch(
"app.notification_api_client.get_notifications_for_service"
)
response = client_request.get_response(
"main.view_job_status_poll",
service_id=service_one["id"],
job_id=fake_uuid,
)
assert response.status_code == 200
# Verify no notifications were fetched
mock_get_notifications.assert_not_called()
data = json.loads(response.get_data(as_text=True))
assert data["total_count"] == 500
assert data["sent_count"] == 300

View File

@@ -1533,9 +1533,7 @@ def test_should_be_able_to_move_to_new_folder(
],
},
_expected_status=302,
_expected_redirect=url_for(
"main.choose_template", service_id=SERVICE_ONE_ID
),
_expected_redirect=url_for("main.choose_template", service_id=SERVICE_ONE_ID),
)
mock_create_template_folder.assert_called_once_with(

View File

@@ -242,6 +242,7 @@ EXCLUDED_ENDPOINTS = tuple(
"verify_email",
"view_job",
"view_job_csv",
"view_job_status_poll",
"view_job_updates",
"view_jobs",
"view_notification",