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 index 37684889a0..823e813d6d 100644 --- a/cms/signals_test.py +++ b/cms/signals_test.py @@ -10,10 +10,21 @@ 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): +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() mock_purge_delay.reset_mock() @@ -26,13 +37,15 @@ def test_purge_fastly_cache_on_publish_course_page(mock_purge_delay, mock_on_com course_page.save_revision().publish() mock_purge_delay.assert_called_once_with( - f"mitxonline:course:{course_page.course.readable_id}" + 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): +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() mock_purge_delay.reset_mock() @@ -41,7 +54,7 @@ def test_purge_fastly_cache_on_publish_program_page(mock_purge_delay, mock_on_co program_page.save_revision().publish() mock_purge_delay.assert_called_once_with( - f"mitxonline:program:{program_page.program.readable_id}" + f"mitxonline:program:{program_page.program.readable_id}", learn_service_id ) diff --git a/cms/tasks.py b/cms/tasks.py index 6fbbf14c8b..c3af5b9512 100644 --- a/cms/tasks.py +++ b/cms/tasks.py @@ -121,8 +121,13 @@ def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): service_id = service_id or settings.MIT_LEARN_FASTLY_SERVICE_ID + # Logged at error level, not warning: a missing setting disables cache + # invalidation entirely, and Sentry only raises an issue from ERROR and above + # (main.sentry passes no event_level, so it keeps the sentry_sdk default). + # A warning here is a breadcrumb, which is how the unset service ID went + # unnoticed until it was found by reading the code. if not service_id: - logger.warning( + logger.error( "No Fastly service ID given; skipping surrogate key purge for %s. " "Is MIT_LEARN_FASTLY_SERVICE_ID set?", surrogate_key, @@ -130,7 +135,7 @@ def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): return False if not settings.FASTLY_AUTH_TOKEN: - logger.warning( + logger.error( "FASTLY_AUTH_TOKEN is not set; skipping surrogate key purge for %s", surrogate_key, ) diff --git a/cms/tasks_test.py b/cms/tasks_test.py index 9e3592ba8a..82ebef40ff 100644 --- a/cms/tasks_test.py +++ b/cms/tasks_test.py @@ -1,5 +1,7 @@ """Tests for cms.tasks""" +import logging + import pytest import responses @@ -71,25 +73,33 @@ def test_queue_fastly_surrogate_key_purge_falls_back_to_settings(fastly_settings @responses.activate -def test_queue_fastly_surrogate_key_purge_skips_without_service_id(fastly_settings): +def test_queue_fastly_surrogate_key_purge_skips_without_service_id( + fastly_settings, caplog +): """ With no service ID passed and none configured, the purge is skipped. - It must skip rather than raise or request `/service/None/purge/...`. + It must skip rather than raise or request `/service/None/purge/...`, and it + must say so at error level -- an unconfigured service ID disables cache + invalidation, and only ERROR and above reaches Sentry as an issue. """ fastly_settings.MIT_LEARN_FASTLY_SERVICE_ID = None assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is False assert not responses.calls + assert [record.levelno for record in caplog.records] == [logging.ERROR] @responses.activate -def test_queue_fastly_surrogate_key_purge_skips_without_auth_token(fastly_settings): +def test_queue_fastly_surrogate_key_purge_skips_without_auth_token( + fastly_settings, caplog +): """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 + assert [record.levelno for record in caplog.records] == [logging.ERROR] @responses.activate 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 + )