-
Notifications
You must be signed in to change notification settings - Fork 2
fix: read Learn's Fastly service ID from MIT_LEARN_FASTLY_SERVICE_ID #3794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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:<readable_id> or mitxonline:program:<readable_id> | ||
|
|
||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fix is one sided and will always work if the celery is the one to be deployed first.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I do want to keep the service id passing explicit, since i think it's somewhat unexpected that mitxonline's |
||
| """ | ||
| 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", | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Uh oh!
There was an error while loading. Please reload this page.