fix(pulpcore): include query string in content-app cache keys

Paginated JSON listings for the same path were sharing one cache entry,
so page N was served from page 0 when Redis was enabled.

ref #7887

Assisted-By: Cursor

Author: TenSt

pulpcore/cache/cache.py

@@ -30,6 +30,7 @@ class CacheKeys(enum.Enum):
     host = "host"
     method = "method"
     format = "format"
+    query = "query"
 
 
 def accept_prefers_json(accept_header):
@@ -373,7 +374,8 @@ def __init__(self, base_key=None, expires_ttl=None, keys=None, auth=None):
             keys: a list of CacheKeys to use for key creation upon entry placement,
                 (path, method) is default. Pass CacheKeys.format if responses for the same
                 path/method can differ based on the request's Accept header (e.g. JSON vs.
-                HTML), so that they are cached under separate keys.
+                HTML). Pass CacheKeys.query if the query string can change the response
+                (e.g. paginated JSON listings).
             auth: a callable to check authorization of the request; takes the request, cache
                   instance, and base_key as arguments.
         """
@@ -494,13 +496,17 @@ async def make_entry(self, key, base_key, handler, args, kwargs, expires=DEFAULT
     def make_key(self, request):
         """Makes the key based off the request"""
         # Might potentially have to make this async if keys require async data from request
+        query_string = getattr(request, "query_string", "")
+        if not isinstance(query_string, str):
+            query_string = ""
         all_keys = {
             CacheKeys.path: request.path,
             CacheKeys.method: request.method,
             CacheKeys.host: request.url.host,
             CacheKeys.format: (
                 "json" if accept_prefers_json(request.headers.get("Accept")) else "other"
             ),
+            CacheKeys.query: query_string,
         }
         key = ":".join(all_keys[k] for k in self.keys)
         return key

pulpcore/content/handler.py

@@ -281,7 +281,9 @@ async def auth_cached(cls, request, cached, base_key):
         # header (JSON vs. HTML/binary), so CacheKeys.format must be part of the cache key.
         # Without it, a JSON response could be cached and served back for an HTML request
         # (or vice versa). See pulpcore.cache.accept_prefers_json.
-        keys=(CacheKeys.path, CacheKeys.method, CacheKeys.format),
+        # JSON listings are also paginated via ?limit=&offset=, so CacheKeys.query is
+        # required or page N would be served from page 0's cache entry.
+        keys=(CacheKeys.path, CacheKeys.method, CacheKeys.format, CacheKeys.query),
     )
     async def stream_content(self, request):
         """

pulpcore/tests/unit/test_cache.py

@@ -160,3 +160,30 @@ def make_request(accept):
     assert default_key == html_key
     assert star_key == html_key
     assert json_key != html_key
+
+
+def test_content_cache_key_varies_by_query(monkeypatch):
+    """Paginated JSON listings for the same path must not share a cache key."""
+    monkeypatch.setattr("pulpcore.cache.cache.get_async_redis_connection", lambda: None)
+    cache = AsyncContentCache(
+        keys=(CacheKeys.path, CacheKeys.method, CacheKeys.format, CacheKeys.query)
+    )
+
+    def make_request(accept, query=""):
+        request = Mock()
+        request.path = "/pulp/content/foo/"
+        request.method = "GET"
+        request.url.host = "example.com"
+        request.headers = {"Accept": accept}
+        request.query_string = query
+        return request
+
+    page0 = cache.make_key(make_request("application/json", "limit=1&offset=0"))
+    page1 = cache.make_key(make_request("application/json", "limit=1&offset=1"))
+    no_query = cache.make_key(make_request("application/json", ""))
+
+    assert page0 == "/pulp/content/foo/:GET:json:limit=1&offset=0"
+    assert page1 == "/pulp/content/foo/:GET:json:limit=1&offset=1"
+    assert no_query == "/pulp/content/foo/:GET:json:"
+    assert page0 != page1
+    assert page0 != no_query