diff --git a/NEWS.md b/NEWS.md index 4fc987249..40082fed5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,13 @@ width: 128px; border-radius: 128px; " /> +## v2.2.9 + +- Fixes + - OPDS responses are cached per client User-Agent. Clients that support + different features no longer receive each other's cached feeds for a + minute. + ## v2.2.8 - Fixes diff --git a/codex/urls/opds/__init__.py b/codex/urls/opds/__init__.py index b375bd224..7cdfc896b 100644 --- a/codex/urls/opds/__init__.py +++ b/codex/urls/opds/__init__.py @@ -15,8 +15,19 @@ def opds_cached(view): across users / auth schemes (mirrors ``codex/urls/opds/binary.py``'s cover-route composition; sub-plan 01 #1). + Feed bodies also vary by client User-Agent: ``UserAgentNames`` + switches facet emission (FACET_SUPPORT), download mime types + (SIMPLE_DOWNLOAD_MIME_TYPES), order facet suppression + (CLIENT_REORDERS) and absolute hrefs (REQUIRE_ABSOLUTE_URL). Without + User-Agent in the key, a facet-capable client and a facet-blind one + served the same URL within the timeout get each other's variant + (#811). Varying also tells intermediary caches the same. + Used by ``v1.py``, ``v2.py``, and ``root.py``'s ``/opds/v2.0`` entry. Progression and binary routes are NOT wrapped — see their respective - modules for the rationale. + modules for the rationale. Covers don't vary by User-Agent, so the + binary routes keep the narrower Cookie/Authorization vary. """ - return cache_page(OPDS_TIMEOUT)(vary_on_headers("Cookie", "Authorization")(view)) + return cache_page(OPDS_TIMEOUT)( + vary_on_headers("Cookie", "Authorization", "User-Agent")(view) + ) diff --git a/tests/test_opds_cache.py b/tests/test_opds_cache.py new file mode 100644 index 000000000..b2ee4dbe2 --- /dev/null +++ b/tests/test_opds_cache.py @@ -0,0 +1,55 @@ +""" +OPDS response caching. + +``opds_cached`` wraps every feed route in ``cache_page``, but OPDS feed +bodies depend on the client: ``UserAgentNames`` switches facet emission, +download mime types and href absoluteness. These pin that the cache key +and the ``Vary`` header both account for the User-Agent, so one client is +never served a variant rendered for another. +""" + +from typing import Final + +from django.test import TestCase + +from tests.test_opds_feed import ( + _HTTP_OK, + _V1_START, + _OPDSFixtureMixin, +) + +_YAR_UA: Final = "yar/1.0" # kybooks, a facet capable client +_FEED: Final = f"{_V1_START}publishers" + +# Only ever appears as the opds:facetGroup attribute name, so a plain +# containment check is unambiguous. The internal param names it can +# carry ("orderBy") also appear in facet hrefs, hence matching the +# attribute name rather than its value. +_FACET_MARKER: Final = b"facetGroup" + + +class OPDSCacheTestCase(_OPDSFixtureMixin, TestCase): + """The OPDS response cache accounts for the client User-Agent.""" + + def test_feed_varies_on_user_agent(self) -> None: + """The Vary header lets caches key on the client.""" + response = self.client.get(_FEED, headers={"user-agent": _YAR_UA}) + assert response.status_code == _HTTP_OK + vary = {part.strip().lower() for part in response.headers["Vary"].split(",")} + assert "user-agent" in vary, response.headers["Vary"] + + def test_cache_not_served_across_user_agents(self) -> None: + """A feed rendered for one client is never replayed to another.""" + # A facet capable client primes the cache for this URL. The + # session cookie is shared with the request below, so a key + # blind to User-Agent matches both. + first = self.client.get(_FEED, headers={"user-agent": _YAR_UA}) + assert first.status_code == _HTTP_OK + assert _FACET_MARKER in first.content + + # Well inside OPDS_TIMEOUT, so a stale entry would still be + # live. This client can't read facets and must not get them. + second = self.client.get(_FEED) + assert second.status_code == _HTTP_OK + assert second.content != first.content + assert _FACET_MARKER not in second.content