From 8a6457eb6760ce416ad5b807bc6107e6308166e4 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 28 Jul 2026 13:59:08 -0400 Subject: [PATCH 1/2] fix: read Learn's Fastly service ID from MIT_LEARN_FASTLY_SERVICE_ID The surrogate key purge targets MIT Learn's Fastly service, because MIT Learn is what tags its product page responses with the mitxonline:* surrogate keys. The setting holding that service ID was named MITX_ONLINE_FASTLY_SERVICE_ID and read from a bare FASTLY_SERVICE_ID env var, which reads as "MITxOnline's own service" and inverts this repo's convention -- elsewhere the app-prefixed name is the env var (EMAIL_BACKEND is read from MITX_ONLINE_EMAIL_BACKEND, and 16 others). That inversion is how the env vars came to be renamed to the setting names in https://github.com/mitodl/ol-infrastructure/pull/5119, leaving both settings unset in production and the purge a silent no-op. Rename to MIT_LEARN_FASTLY_SERVICE_ID, matching the existing MIT_LEARN_* namespace, whose seven settings all use the env var name unchanged -- the prefix names the external system, so no app prefix applies. The token and API URL now follow the app-prefixed convention instead, reading MITX_ONLINE_FASTLY_AUTH_TOKEN and MITX_ONLINE_FASTLY_URL, which is what the deployment already sets. So the infra follow-up only has to add the service ID; nothing has to be reverted. Pass the service ID into queue_fastly_surrogate_key_purge explicitly rather than reading it from module state, so the target of a purge is visible at the call site instead of being an unqualified global in the MITxOnline repo. It defaults to None so messages enqueued by the previous release skip the purge rather than raising during a rolling deploy. Read the token and URL through django.conf.settings, removing the module-import-time coupling that made these values invisible to Django's settings fixture in tests. Adds cms/tasks_test.py and cms/signals_test.py; neither module had tests. Co-Authored-By: Claude Opus 5 (1M context) --- app.json | 20 ++++---- cms/signals.py | 7 ++- cms/signals_test.py | 67 ++++++++++++++++++++++++++ cms/tasks.py | 50 +++++++++++--------- cms/tasks_test.py | 102 ++++++++++++++++++++++++++++++++++++++++ courses/signals.py | 19 ++++++-- courses/signals_test.py | 32 ++++++++++--- main/settings.py | 28 ++++++++--- 8 files changed, 278 insertions(+), 47 deletions(-) create mode 100644 cms/signals_test.py create mode 100644 cms/tasks_test.py diff --git a/app.json b/app.json index 462723dc4c..0f79ecc68f 100644 --- a/app.json +++ b/app.json @@ -138,14 +138,6 @@ "description": "Expose the OIDC login functionality.", "required": false }, - "FASTLY_AUTH_TOKEN": { - "description": "Optional token for the Fastly purge API.", - "required": false - }, - "FASTLY_URL": { - "description": "The URL to the Fastly API.", - "required": false - }, "GA_TRACKING_ID": { "description": "Google analytics tracking ID", "required": false @@ -482,6 +474,14 @@ "description": "The execution environment that the app is in (e.g. dev, staging, prod)", "required": true }, + "MITX_ONLINE_FASTLY_AUTH_TOKEN": { + "description": "Optional token for the Fastly purge API.", + "required": false + }, + "MITX_ONLINE_FASTLY_URL": { + "description": "The URL to the Fastly API.", + "required": false + }, "MITX_ONLINE_FROM_EMAIL": { "description": "E-mail to use for the from field", "required": false @@ -546,6 +546,10 @@ "description": "Dashboard URL for UAI enrollment emails", "required": false }, + "MIT_LEARN_FASTLY_SERVICE_ID": { + "description": "Fastly service ID for the MIT Learn frontend, used for surrogate key (tag) purging.", + "required": false + }, "MIT_LEARN_FROM_EMAIL": { "description": "From email address for UAI enrollment emails", "required": false diff --git a/cms/signals.py b/cms/signals.py index 9bf4a5be8b..04312afa3b 100644 --- a/cms/signals.py +++ b/cms/signals.py @@ -1,5 +1,6 @@ import logging +from django.conf import settings from django.db import transaction from wagtail.signals import page_published @@ -56,7 +57,11 @@ def purge_fastly_cache_on_publish(sender, **kwargs): # noqa: ARG001 logger.info( "Scheduling Fastly surrogate key purge on page publish: %s", surrogate_key ) - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) page_published.connect(flex_pricing_field_check) diff --git a/cms/signals_test.py b/cms/signals_test.py new file mode 100644 index 0000000000..25de295e67 --- /dev/null +++ b/cms/signals_test.py @@ -0,0 +1,67 @@ +"""Tests for cms.signals""" + +from unittest.mock import patch + +import pytest + +from cms.factories import CoursePageFactory, ProgramPageFactory, ResourcePageFactory +from cms.signals import purge_fastly_cache_on_publish + +pytestmark = pytest.mark.django_db + +LEARN_SERVICE_ID = "test-learn-service-id" + + +@pytest.fixture +def learn_service_id(settings): + """Point the purge at a known MIT Learn Fastly service.""" + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + return settings.MIT_LEARN_FASTLY_SERVICE_ID + + +@patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) +@patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") +def test_purge_fastly_cache_on_publish_course_page( + mock_purge_delay, mock_on_commit, learn_service_id +): + """Publishing a CoursePage purges the key for its course.""" + course_page = CoursePageFactory.create() + # Building the page saves a Course, which fires its own post_save purge. + mock_purge_delay.reset_mock() + + purge_fastly_cache_on_publish(sender=None, instance=course_page) + + mock_purge_delay.assert_called_once_with( + f"mitxonline:course:{course_page.course.readable_id}", learn_service_id + ) + + +@patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) +@patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") +def test_purge_fastly_cache_on_publish_program_page( + mock_purge_delay, mock_on_commit, learn_service_id +): + """Publishing a ProgramPage purges the key for its program.""" + program_page = ProgramPageFactory.create() + # Building the page saves a Program, which fires its own post_save purge. + mock_purge_delay.reset_mock() + + purge_fastly_cache_on_publish(sender=None, instance=program_page) + + mock_purge_delay.assert_called_once_with( + f"mitxonline:program:{program_page.program.readable_id}", learn_service_id + ) + + +@patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) +@patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") +def test_purge_fastly_cache_on_publish_ignores_other_pages( + mock_purge_delay, mock_on_commit +): + """Publishing a page that is not a product page purges nothing.""" + resource_page = ResourcePageFactory.create() + mock_purge_delay.reset_mock() + + purge_fastly_cache_on_publish(sender=None, instance=resource_page) + + mock_purge_delay.assert_not_called() diff --git a/cms/tasks.py b/cms/tasks.py index cf23ce9104..d10e4505b0 100644 --- a/cms/tasks.py +++ b/cms/tasks.py @@ -2,17 +2,12 @@ from urllib.parse import urljoin, urlparse import requests +from django.conf import settings from mitol.common.decorators import single_task from cms.api import create_featured_items from cms.models import Page from main.celery import app -from main.settings import ( - MITX_ONLINE_FASTLY_AUTH_TOKEN, - MITX_ONLINE_FASTLY_SERVICE_ID, - MITX_ONLINE_FASTLY_URL, - SITE_BASE_URL, -) def call_fastly_purge_api(relative_url): @@ -21,23 +16,26 @@ def call_fastly_purge_api(relative_url): because it doesn't work for this - the version of it that works with the current API only allows you to purge *everything*, not individual pages.) + Purges by URL against MITxOnline's own site -- the target is identified by + the `host` header taken from SITE_BASE_URL, not by a Fastly service ID. + Args: - relative_url The relative URL to purge. Returns: - Dict of the response (resp.json), or False if there was an error. """ logger = logging.getLogger("fastly_purge") - netloc = urlparse(SITE_BASE_URL)[1] + netloc = urlparse(settings.SITE_BASE_URL)[1] headers = {"host": netloc} if relative_url != "*": headers["fastly-soft-purge"] = "1" - if MITX_ONLINE_FASTLY_AUTH_TOKEN: - headers["fastly-key"] = MITX_ONLINE_FASTLY_AUTH_TOKEN + if settings.FASTLY_AUTH_TOKEN: + headers["fastly-key"] = settings.FASTLY_AUTH_TOKEN - api_url = urljoin(MITX_ONLINE_FASTLY_URL, relative_url) + api_url = urljoin(settings.FASTLY_URL, relative_url) resp = requests.request("PURGE", api_url, headers=headers) # noqa: S113 @@ -98,47 +96,57 @@ def queue_fastly_full_purge(): @app.task -def queue_fastly_surrogate_key_purge(surrogate_key): +def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): """ Purges all Fastly cached responses tagged with the given surrogate key. Uses the Fastly purge-by-tag API: POST /service/{service_id}/purge/{surrogate_key} - This allows MIT Learn pages to declare which MITxOnline surrogate keys they - subscribe to (via the Surrogate-Key response header), and MITxOnline to - invalidate those pages when course/program data changes. + MIT Learn tags its product page responses with the MITxOnline surrogate keys + they depend on (via the Surrogate-Key response header), which lets MITxOnline + invalidate those pages when course/program data changes. The service is + therefore Learn's -- purging MITxOnline's own Fastly service would do nothing, + since it tags no responses with these keys. It is passed in rather than read + from settings so that the target is visible at the call site. Key format: mitxonline:course: or mitxonline:program: Args: surrogate_key (str): The surrogate key to purge, e.g. "mitxonline:course:course-v1:MITx+6.00.1x" + service_id (str): The Fastly service ID whose cache should be purged. + Callers pass settings.MIT_LEARN_FASTLY_SERVICE_ID. Defaults to None + so that messages enqueued by an older release, before this argument + existed, skip the purge rather than raising during a rolling deploy. """ logger = logging.getLogger("fastly_purge") - if not MITX_ONLINE_FASTLY_SERVICE_ID: + if not service_id: logger.warning( - "FASTLY_SERVICE_ID is not set; skipping surrogate key purge for %s", + "No Fastly service ID given; skipping surrogate key purge for %s. " + "Is MIT_LEARN_FASTLY_SERVICE_ID set?", surrogate_key, ) return False - if not MITX_ONLINE_FASTLY_AUTH_TOKEN: + if not settings.FASTLY_AUTH_TOKEN: logger.warning( "FASTLY_AUTH_TOKEN is not set; skipping surrogate key purge for %s", surrogate_key, ) return False - logger.info("Purging Fastly surrogate key: %s", surrogate_key) + logger.info( + "Purging Fastly surrogate key %s from service %s", surrogate_key, service_id + ) api_url = urljoin( - MITX_ONLINE_FASTLY_URL, - f"/service/{MITX_ONLINE_FASTLY_SERVICE_ID}/purge/{surrogate_key}", + settings.FASTLY_URL, + f"/service/{service_id}/purge/{surrogate_key}", ) headers = { - "Fastly-Key": MITX_ONLINE_FASTLY_AUTH_TOKEN, + "Fastly-Key": settings.FASTLY_AUTH_TOKEN, "fastly-soft-purge": "1", } diff --git a/cms/tasks_test.py b/cms/tasks_test.py new file mode 100644 index 0000000000..c4ff406caf --- /dev/null +++ b/cms/tasks_test.py @@ -0,0 +1,102 @@ +"""Tests for cms.tasks""" + +import pytest +import responses + +from cms.tasks import call_fastly_purge_api, queue_fastly_surrogate_key_purge + +# Deliberately not the production default (https://api.fastly.com), so that +# reading these settings at module import time rather than call time would fail +# to match the registered responses. +FASTLY_URL = "https://fastly.test" +SITE_BASE_URL = "https://mitxonline.test" +FASTLY_AUTH_TOKEN = "fastly-token" # noqa: S105 + +LEARN_SERVICE_ID = "test-learn-service-id" +SURROGATE_KEY = "mitxonline:course:course-v1:MITx+6.00.1x" + + +@pytest.fixture +def fastly_settings(settings): + """ + Configure the Fastly settings the purge tasks read. + + MIT_LEARN_FASTLY_SERVICE_ID is set to a decoy so that a task reading the + service from settings instead of its argument fails rather than passing on a + coincidence. + """ + settings.FASTLY_URL = FASTLY_URL + settings.FASTLY_AUTH_TOKEN = FASTLY_AUTH_TOKEN + settings.SITE_BASE_URL = SITE_BASE_URL + settings.MIT_LEARN_FASTLY_SERVICE_ID = "decoy-service-id-not-used" + return settings + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_targets_given_service(fastly_settings): + """The purge goes to the service ID passed in, not one read from settings.""" + purge = responses.add( + responses.POST, + f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", + json={"status": "ok"}, + status=200, + ) + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is True + + assert purge.call_count == 1 + assert responses.calls[0].request.headers["Fastly-Key"] == FASTLY_AUTH_TOKEN + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_skips_without_service_id(fastly_settings): + """ + A missing service ID skips the purge instead of calling Fastly. + + Called with the surrogate key alone, as a message enqueued by a release + predating the service_id argument would be during a rolling deploy. The task + must skip rather than raise or request `/service/None/purge/...`. + """ + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is False + assert not responses.calls + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_skips_without_auth_token(fastly_settings): + """A missing auth token skips the purge rather than sending it unauthenticated.""" + fastly_settings.FASTLY_AUTH_TOKEN = None + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is False + assert not responses.calls + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_returns_false_on_error(fastly_settings): + """A Fastly error response is reported as a failure rather than swallowed.""" + responses.add( + responses.POST, + f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", + status=503, + ) + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is False + + +@responses.activate +def test_call_fastly_purge_api_targets_mitxonline_by_host(fastly_settings): + """ + The URL purge identifies MITxOnline's own site by `host`, not by service ID. + + Covers the three settings this helper reads, none of which are exercised + elsewhere. + """ + purge = responses.add( + responses.Response(method="PURGE", url=f"{FASTLY_URL}/catalog/", json={}) + ) + + call_fastly_purge_api("/catalog/") + + assert purge.call_count == 1 + sent_headers = responses.calls[0].request.headers + assert sent_headers["host"] == "mitxonline.test" + assert sent_headers["fastly-key"] == FASTLY_AUTH_TOKEN diff --git a/courses/signals.py b/courses/signals.py index 232a4ae8cf..eeaf9d084d 100644 --- a/courses/signals.py +++ b/courses/signals.py @@ -2,6 +2,7 @@ Signals for mitxonline course certificates """ +from django.conf import settings from django.db import transaction from django.db.models.signals import post_save from django.dispatch import receiver @@ -90,7 +91,11 @@ def purge_fastly_cache_on_course_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:course:{instance.readable_id}" - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) @receiver(post_save, sender=CourseRun, dispatch_uid="courserun_post_save_fastly_purge") @@ -107,7 +112,11 @@ def purge_fastly_cache_on_course_run_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:course:{instance.course.readable_id}" - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) @receiver(post_save, sender=Program, dispatch_uid="program_post_save_fastly_purge") @@ -124,4 +133,8 @@ def purge_fastly_cache_on_program_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:program:{instance.readable_id}" - transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) + transaction.on_commit( + lambda: queue_fastly_surrogate_key_purge.delay( + surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID + ) + ) diff --git a/courses/signals_test.py b/courses/signals_test.py index 253ffc425b..232fae3366 100644 --- a/courses/signals_test.py +++ b/courses/signals_test.py @@ -123,41 +123,59 @@ def test_sync_program_certificate_with_hubspot_on_save( # Fastly surrogate-key purge signal tests # --------------------------------------------------------------------------- +LEARN_SERVICE_ID = "test-learn-service-id" + @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_course_save_update(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_course_save_update( + mock_purge_delay, mock_on_commit, settings +): """ Updating (re-saving) a Course enqueues a fresh purge each time. """ + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + course = CourseFactory.create() - mock_purge_delay.assert_called_with(f"mitxonline:course:{course.readable_id}") + mock_purge_delay.assert_called_with( + f"mitxonline:course:{course.readable_id}", LEARN_SERVICE_ID + ) course.title = "Updated Title" course.save() - mock_purge_delay.assert_called_with(f"mitxonline:course:{course.readable_id}") + mock_purge_delay.assert_called_with( + f"mitxonline:course:{course.readable_id}", LEARN_SERVICE_ID + ) @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_course_run_save(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_course_run_save( + mock_purge_delay, mock_on_commit, settings +): """ Saving a CourseRun enqueues a Fastly surrogate-key purge for the parent course: mitxonline:course:. """ + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + course_run = CourseRunFactory.create() mock_purge_delay.assert_called_with( - f"mitxonline:course:{course_run.course.readable_id}" + f"mitxonline:course:{course_run.course.readable_id}", LEARN_SERVICE_ID ) @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_program_save(mock_purge_delay, mock_on_commit): +def test_purge_fastly_cache_on_program_save(mock_purge_delay, mock_on_commit, settings): """ Saving a Program enqueues a Fastly surrogate-key purge for mitxonline:program:. """ + settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + program = ProgramFactory.create() - mock_purge_delay.assert_called_with(f"mitxonline:program:{program.readable_id}") + mock_purge_delay.assert_called_with( + f"mitxonline:program:{program.readable_id}", LEARN_SERVICE_ID + ) diff --git a/main/settings.py b/main/settings.py index cc8c7c3071..76b74a8c78 100644 --- a/main/settings.py +++ b/main/settings.py @@ -1350,22 +1350,36 @@ # Fastly configuration -MITX_ONLINE_FASTLY_AUTH_TOKEN = get_string( - name="FASTLY_AUTH_TOKEN", +# MITxOnline's own Fastly config follows the convention used throughout this file: +# the MITX_ONLINE_-prefixed name is the *env var*, the unprefixed one the setting +# (as EMAIL_BACKEND is read from MITX_ONLINE_EMAIL_BACKEND, and 16 others). +# These three previously inverted that -- prefixed settings reading bare env vars -- +# which is how the env vars came to be renamed to the setting names, breaking the +# read entirely: https://github.com/mitodl/ol-infrastructure/pull/5119 +FASTLY_AUTH_TOKEN = get_string( + name="MITX_ONLINE_FASTLY_AUTH_TOKEN", default=None, description="Optional token for the Fastly purge API.", ) -MITX_ONLINE_FASTLY_URL = get_string( - name="FASTLY_URL", +FASTLY_URL = get_string( + name="MITX_ONLINE_FASTLY_URL", default="https://api.fastly.com", description="The URL to the Fastly API.", ) -MITX_ONLINE_FASTLY_SERVICE_ID = get_string( - name="FASTLY_SERVICE_ID", +# Not MITX_ONLINE_-prefixed, because the value is not MITxOnline's: MIT_LEARN_* is +# this file's namespace for MIT Learn config, and those seven settings all use the +# env var name unchanged. +MIT_LEARN_FASTLY_SERVICE_ID = get_string( + name="MIT_LEARN_FASTLY_SERVICE_ID", default=None, - description="Fastly service ID used for surrogate key (tag) purging.", + description=( + "Fastly service ID for the MIT Learn frontend, used for surrogate key " + "(tag) purging. MIT Learn tags its product page responses with MITxOnline " + "surrogate keys, so this is Learn's service ID -- not the service ID of " + "MITxOnline's own Fastly service." + ), ) # Hubspot sync settings From 221cae937ecacbdb0f31fa041d9992cd680454ed Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 30 Jul 2026 10:56:47 -0400 Subject: [PATCH 2/2] fix: fall back to the Learn service ID setting instead of skipping Keeps the enqueued Celery message a single positional argument so a rolling deploy is safe in both directions: an old webapp's one-arg message runs on a new worker and still purges via the fallback, and a new webapp's one-arg message runs on an old worker unchanged. Passing service_id explicitly from the call sites is #3804, once every worker accepts the argument. Also drives cms/signals_test.py through Wagtail's real publish rather than calling the receiver directly, so the page_published.connect() wiring is covered. Co-Authored-By: Claude Opus 5 (1M context) --- cms/signals.py | 7 +------ cms/signals_test.py | 41 ++++++++++++++++++----------------------- cms/tasks.py | 9 ++++----- cms/tasks_test.py | 40 +++++++++++++++++++++++++++++++--------- courses/signals.py | 19 +++---------------- courses/signals_test.py | 32 +++++++------------------------- 6 files changed, 64 insertions(+), 84 deletions(-) diff --git a/cms/signals.py b/cms/signals.py index 04312afa3b..9bf4a5be8b 100644 --- a/cms/signals.py +++ b/cms/signals.py @@ -1,6 +1,5 @@ import logging -from django.conf import settings from django.db import transaction from wagtail.signals import page_published @@ -57,11 +56,7 @@ def purge_fastly_cache_on_publish(sender, **kwargs): # noqa: ARG001 logger.info( "Scheduling Fastly surrogate key purge on page publish: %s", surrogate_key ) - transaction.on_commit( - lambda: queue_fastly_surrogate_key_purge.delay( - surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID - ) - ) + transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) page_published.connect(flex_pricing_field_check) diff --git a/cms/signals_test.py b/cms/signals_test.py index 25de295e67..37684889a0 100644 --- a/cms/signals_test.py +++ b/cms/signals_test.py @@ -2,54 +2,46 @@ from unittest.mock import patch +import factory import pytest +from django.db.models.signals import post_save from cms.factories import CoursePageFactory, ProgramPageFactory, ResourcePageFactory -from cms.signals import purge_fastly_cache_on_publish pytestmark = pytest.mark.django_db -LEARN_SERVICE_ID = "test-learn-service-id" - - -@pytest.fixture -def learn_service_id(settings): - """Point the purge at a known MIT Learn Fastly service.""" - settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID - return settings.MIT_LEARN_FASTLY_SERVICE_ID - @patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_publish_course_page( - mock_purge_delay, mock_on_commit, learn_service_id -): +def test_purge_fastly_cache_on_publish_course_page(mock_purge_delay, mock_on_commit): """Publishing a CoursePage purges the key for its course.""" course_page = CoursePageFactory.create() - # Building the page saves a Course, which fires its own post_save purge. mock_purge_delay.reset_mock() - purge_fastly_cache_on_publish(sender=None, instance=course_page) + # Publishing saves the Course as well, which purges the same key via its own + # post_save receiver; mute post_save so only the publish path is counted. + # This mutes every post_save receiver, Wagtail's index updates included, so + # the publish is not a fully faithful one -- adequate for these assertions. + with factory.django.mute_signals(post_save): + course_page.save_revision().publish() mock_purge_delay.assert_called_once_with( - f"mitxonline:course:{course_page.course.readable_id}", learn_service_id + f"mitxonline:course:{course_page.course.readable_id}" ) @patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_publish_program_page( - mock_purge_delay, mock_on_commit, learn_service_id -): +def test_purge_fastly_cache_on_publish_program_page(mock_purge_delay, mock_on_commit): """Publishing a ProgramPage purges the key for its program.""" program_page = ProgramPageFactory.create() - # Building the page saves a Program, which fires its own post_save purge. mock_purge_delay.reset_mock() - purge_fastly_cache_on_publish(sender=None, instance=program_page) + with factory.django.mute_signals(post_save): + program_page.save_revision().publish() mock_purge_delay.assert_called_once_with( - f"mitxonline:program:{program_page.program.readable_id}", learn_service_id + f"mitxonline:program:{program_page.program.readable_id}" ) @@ -62,6 +54,9 @@ def test_purge_fastly_cache_on_publish_ignores_other_pages( resource_page = ResourcePageFactory.create() mock_purge_delay.reset_mock() - purge_fastly_cache_on_publish(sender=None, instance=resource_page) + # A ResourcePage has no Course or Program, so there is no post_save purge to + # suppress here; muted only to keep the three tests the same shape. + with factory.django.mute_signals(post_save): + resource_page.save_revision().publish() mock_purge_delay.assert_not_called() diff --git a/cms/tasks.py b/cms/tasks.py index d10e4505b0..6fbbf14c8b 100644 --- a/cms/tasks.py +++ b/cms/tasks.py @@ -107,8 +107,7 @@ def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): they depend on (via the Surrogate-Key response header), which lets MITxOnline invalidate those pages when course/program data changes. The service is therefore Learn's -- purging MITxOnline's own Fastly service would do nothing, - since it tags no responses with these keys. It is passed in rather than read - from settings so that the target is visible at the call site. + since it tags no responses with these keys. Key format: mitxonline:course: or mitxonline:program: @@ -116,12 +115,12 @@ def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): surrogate_key (str): The surrogate key to purge, e.g. "mitxonline:course:course-v1:MITx+6.00.1x" service_id (str): The Fastly service ID whose cache should be purged. - Callers pass settings.MIT_LEARN_FASTLY_SERVICE_ID. Defaults to None - so that messages enqueued by an older release, before this argument - existed, skip the purge rather than raising during a rolling deploy. + Falls back to settings.MIT_LEARN_FASTLY_SERVICE_ID when omitted. """ logger = logging.getLogger("fastly_purge") + service_id = service_id or settings.MIT_LEARN_FASTLY_SERVICE_ID + if not service_id: logger.warning( "No Fastly service ID given; skipping surrogate key purge for %s. " diff --git a/cms/tasks_test.py b/cms/tasks_test.py index c4ff406caf..9e3592ba8a 100644 --- a/cms/tasks_test.py +++ b/cms/tasks_test.py @@ -21,20 +21,20 @@ def fastly_settings(settings): """ Configure the Fastly settings the purge tasks read. - MIT_LEARN_FASTLY_SERVICE_ID is set to a decoy so that a task reading the - service from settings instead of its argument fails rather than passing on a - coincidence. + MIT_LEARN_FASTLY_SERVICE_ID is set to a value the tests never register a + response for, so that a test meaning to exercise an explicitly passed + service ID cannot pass by falling back to settings. """ settings.FASTLY_URL = FASTLY_URL settings.FASTLY_AUTH_TOKEN = FASTLY_AUTH_TOKEN settings.SITE_BASE_URL = SITE_BASE_URL - settings.MIT_LEARN_FASTLY_SERVICE_ID = "decoy-service-id-not-used" + settings.MIT_LEARN_FASTLY_SERVICE_ID = "unregistered-fallback-service-id" return settings @responses.activate def test_queue_fastly_surrogate_key_purge_targets_given_service(fastly_settings): - """The purge goes to the service ID passed in, not one read from settings.""" + """An explicitly passed service ID takes precedence over the setting.""" purge = responses.add( responses.POST, f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", @@ -48,15 +48,37 @@ def test_queue_fastly_surrogate_key_purge_targets_given_service(fastly_settings) assert responses.calls[0].request.headers["Fastly-Key"] == FASTLY_AUTH_TOKEN +@responses.activate +def test_queue_fastly_surrogate_key_purge_falls_back_to_settings(fastly_settings): + """ + Called with the surrogate key alone, the task purges Learn's service anyway. + + This is the shape of a message enqueued by a release that does not pass + service_id, so the fallback is what keeps purges working across a rolling + deploy rather than silently skipping them. + """ + fastly_settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + purge = responses.add( + responses.POST, + f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", + json={"status": "ok"}, + status=200, + ) + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is True + + assert purge.call_count == 1 + + @responses.activate def test_queue_fastly_surrogate_key_purge_skips_without_service_id(fastly_settings): """ - A missing service ID skips the purge instead of calling Fastly. + With no service ID passed and none configured, the purge is skipped. - Called with the surrogate key alone, as a message enqueued by a release - predating the service_id argument would be during a rolling deploy. The task - must skip rather than raise or request `/service/None/purge/...`. + It must skip rather than raise or request `/service/None/purge/...`. """ + fastly_settings.MIT_LEARN_FASTLY_SERVICE_ID = None + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is False assert not responses.calls diff --git a/courses/signals.py b/courses/signals.py index eeaf9d084d..232a4ae8cf 100644 --- a/courses/signals.py +++ b/courses/signals.py @@ -2,7 +2,6 @@ Signals for mitxonline course certificates """ -from django.conf import settings from django.db import transaction from django.db.models.signals import post_save from django.dispatch import receiver @@ -91,11 +90,7 @@ def purge_fastly_cache_on_course_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:course:{instance.readable_id}" - transaction.on_commit( - lambda: queue_fastly_surrogate_key_purge.delay( - surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID - ) - ) + transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) @receiver(post_save, sender=CourseRun, dispatch_uid="courserun_post_save_fastly_purge") @@ -112,11 +107,7 @@ def purge_fastly_cache_on_course_run_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:course:{instance.course.readable_id}" - transaction.on_commit( - lambda: queue_fastly_surrogate_key_purge.delay( - surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID - ) - ) + transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) @receiver(post_save, sender=Program, dispatch_uid="program_post_save_fastly_purge") @@ -133,8 +124,4 @@ def purge_fastly_cache_on_program_save( from cms.tasks import queue_fastly_surrogate_key_purge # noqa: PLC0415 surrogate_key = f"mitxonline:program:{instance.readable_id}" - transaction.on_commit( - lambda: queue_fastly_surrogate_key_purge.delay( - surrogate_key, settings.MIT_LEARN_FASTLY_SERVICE_ID - ) - ) + transaction.on_commit(lambda: queue_fastly_surrogate_key_purge.delay(surrogate_key)) diff --git a/courses/signals_test.py b/courses/signals_test.py index 232fae3366..253ffc425b 100644 --- a/courses/signals_test.py +++ b/courses/signals_test.py @@ -123,59 +123,41 @@ def test_sync_program_certificate_with_hubspot_on_save( # Fastly surrogate-key purge signal tests # --------------------------------------------------------------------------- -LEARN_SERVICE_ID = "test-learn-service-id" - @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_course_save_update( - mock_purge_delay, mock_on_commit, settings -): +def test_purge_fastly_cache_on_course_save_update(mock_purge_delay, mock_on_commit): """ Updating (re-saving) a Course enqueues a fresh purge each time. """ - settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID - course = CourseFactory.create() - mock_purge_delay.assert_called_with( - f"mitxonline:course:{course.readable_id}", LEARN_SERVICE_ID - ) + mock_purge_delay.assert_called_with(f"mitxonline:course:{course.readable_id}") course.title = "Updated Title" course.save() - mock_purge_delay.assert_called_with( - f"mitxonline:course:{course.readable_id}", LEARN_SERVICE_ID - ) + mock_purge_delay.assert_called_with(f"mitxonline:course:{course.readable_id}") @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_course_run_save( - mock_purge_delay, mock_on_commit, settings -): +def test_purge_fastly_cache_on_course_run_save(mock_purge_delay, mock_on_commit): """ Saving a CourseRun enqueues a Fastly surrogate-key purge for the parent course: mitxonline:course:. """ - settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID - course_run = CourseRunFactory.create() mock_purge_delay.assert_called_with( - f"mitxonline:course:{course_run.course.readable_id}", LEARN_SERVICE_ID + f"mitxonline:course:{course_run.course.readable_id}" ) @patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback()) @patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") -def test_purge_fastly_cache_on_program_save(mock_purge_delay, mock_on_commit, settings): +def test_purge_fastly_cache_on_program_save(mock_purge_delay, mock_on_commit): """ Saving a Program enqueues a Fastly surrogate-key purge for mitxonline:program:. """ - settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID - program = ProgramFactory.create() - mock_purge_delay.assert_called_with( - f"mitxonline:program:{program.readable_id}", LEARN_SERVICE_ID - ) + mock_purge_delay.assert_called_with(f"mitxonline:program:{program.readable_id}")