[PULP-2012] Cache Simple API responses

CHANGES/1254.feature

@@ -0,0 +1 @@
+Added server-side caching and HTTP cache headers (`Cache-Control`, `ETag`) to Simple API responses.

pulp_python/app/cache.py

@@ -0,0 +1,65 @@
+from django.core.exceptions import ObjectDoesNotExist
+from django.http import Http404, HttpResponseNotModified
+
+from pulpcore.plugin.cache import CacheKeys, SyncContentCache
+from pulpcore.plugin.util import cache_key, get_domain
+
+from pulp_python.app.models import PythonDistribution
+
+ACCEPT_HEADER_KEY = "accept_header"
+
+
+class PythonApiCache(SyncContentCache):
+    """
+    Cache for the Simple API.
+
+    Adds Accept header to the cache key so HTML and JSON responses are cached separately.
+    Also handles ETag-based conditional requests (304 Not Modified).
+    """
+
+    def __init__(self, base_key=None):
+        keys = (CacheKeys.path, CacheKeys.method, ACCEPT_HEADER_KEY)
+        super().__init__(base_key=base_key, keys=keys)
+
+    def __call__(self, func):
+        original = super().__call__(func)
+
+        def with_conditional_get(*args, **kwargs):
+            response = original(*args, **kwargs)
+            etag = response.get("ETag")
+            if etag:
+                request = self.get_request_from_args(args)
+                if request and request.META.get("HTTP_IF_NONE_MATCH") == etag:
+                    not_modified = HttpResponseNotModified()
+                    not_modified["ETag"] = etag
+                    if cache_control := response.get("Cache-Control"):
+                        not_modified["Cache-Control"] = cache_control
+                    if x_pulp_cache := response.get("X-PULP-CACHE"):
+                        not_modified["X-PULP-CACHE"] = x_pulp_cache
+                    return not_modified
+            return response
+
+        return with_conditional_get
+
+    def make_key(self, request):
+        all_keys = {
+            CacheKeys.path: request.path,
+            CacheKeys.method: request.method,
+            ACCEPT_HEADER_KEY: request.headers.get("accept", ""),
+        }
+        return ":".join(all_keys[k] for k in self.keys)
+
+
+def find_base_path_cached(request, cached):
+    """
+    Resolve the distribution base_path for use as the Redis cache base_key.
+    """
+    path = request.resolver_match.kwargs["path"]
+    base_key = cache_key(path)
+    if cached.exists(base_key=base_key):
+        return base_key
+    try:
+        distro = PythonDistribution.objects.get(base_path=path, pulp_domain=get_domain())
+    except ObjectDoesNotExist:
+        raise Http404(f"No PythonDistribution found for base_path {path}")
+    return cache_key(distro.base_path)

pulp_python/app/pypi/views.py

@@ -1,3 +1,4 @@
+import hashlib
 import logging
 from datetime import datetime, timedelta, timezone
 from itertools import chain
@@ -15,7 +16,6 @@
     HttpResponseBadRequest,
     HttpResponseForbidden,
     HttpResponseNotFound,
-    StreamingHttpResponse,
 )
 from django.shortcuts import redirect
 from drf_spectacular.utils import extend_schema
@@ -31,6 +31,7 @@
 from pulpcore.plugin.viewsets import OperationPostponedResponse
 
 from pulp_python.app import tasks
+from pulp_python.app.cache import PythonApiCache, find_base_path_cached
 from pulp_python.app.models import (
     PackageProvenance,
     PythonDistribution,
@@ -63,6 +64,7 @@
 
 PYPI_SIMPLE_V1_HTML = "application/vnd.pypi.simple.v1+html"
 PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json"
+CACHE_CONTROL = "max-age=600, public"
 
 
 class PyPISimpleHTMLRenderer(TemplateHTMLRenderer):
@@ -297,7 +299,14 @@ def get_provenance_url(self, package, version, filename):
             self.base_api_url, f"{base_path}/integrity/{package}/{version}/{filename}/provenance/"
         )
 
+    @staticmethod
+    def _make_etag(repo_version):
+        """Builds a quoted ETag from the repo version. Changes when content changes."""
+        raw = f"{repo_version.number}:{repo_version.pulp_created.isoformat()}"
+        return f'"{hashlib.sha256(raw.encode()).hexdigest()[:16]}"'
+
     @extend_schema(summary="Get index simple page")
+    @PythonApiCache(base_key=find_base_path_cached)
     def list(self, request, path):
         """Gets the simple api html page for the index."""
         repo_version, content = self.get_rvc()
@@ -310,15 +319,19 @@ def list(self, request, path):
             .iterator()
         )
         media_type = request.accepted_renderer.media_type
-        headers = {"X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT)}
+        headers = {
+            "X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT),
+            "Cache-Control": CACHE_CONTROL,
+            "ETag": self._make_etag(repo_version),
+        }
 
         if media_type == PYPI_SIMPLE_V1_JSON:
             index_data = write_simple_index_json(names)
             return Response(index_data, headers=headers)
         else:
-            index_data = write_simple_index(names, streamed=True)
+            index_data = write_simple_index(names)
             kwargs = {"content_type": media_type, "headers": headers}
-            return StreamingHttpResponse(index_data, **kwargs)
+            return HttpResponse(index_data, **kwargs)
 
     def pull_through_package_simple(self, package, path, remote):
         """Gets the package's simple page from remote."""
@@ -355,14 +368,17 @@ def parse_package(release_package):
         }
 
     @extend_schema(operation_id="pypi_simple_package_read", summary="Get package simple page")
+    @PythonApiCache(base_key=find_base_path_cached)
     def retrieve(self, request, path, package):
         """Retrieves the simple api html/json page for a package."""
         repo_ver, content = self.get_rvc()
         # Should I redirect if the normalized name is different?
         normalized = canonicalize_name(package)
         releases = {}
+        is_pull_through = False
         if self.distribution.remote:
             releases = self.pull_through_package_simple(normalized, path, self.distribution.remote)
+            is_pull_through = True
         elif self.should_redirect(repo_version=repo_ver):
             return redirect(urljoin(self.base_content_url, f"{path}/simple/{normalized}/"))
         if content is not None:
@@ -405,7 +421,12 @@ def retrieve(self, request, path, package):
             return HttpResponseNotFound(f"{normalized} does not exist.")
 
         media_type = request.accepted_renderer.media_type
-        headers = {"X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT)}
+        headers = {
+            "X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT),
+            "Cache-Control": CACHE_CONTROL,
+        }
+        if not is_pull_through:
+            headers["ETag"] = self._make_etag(repo_ver)
 
         if media_type == PYPI_SIMPLE_V1_JSON:
             detail_data = write_simple_detail_json(normalized, releases.values())

pulp_python/tests/functional/api/test_simple_cache.py

@@ -0,0 +1,96 @@
+from urllib.parse import urljoin
+
+import pytest
+import requests
+
+from pulp_python.tests.functional.constants import (
+    PYPI_SIMPLE_V1_HTML,
+    PYPI_SIMPLE_V1_JSON,
+    PYTHON_SM_PROJECT_SPECIFIER,
+)
+
+
+@pytest.fixture
+def skip_without_cache(pulp_settings):
+    """
+    Skip test if server-side caching is not enabled.
+    """
+    if not pulp_settings.CACHE_ENABLED:
+        pytest.skip("CACHE_ENABLED is not set")
+
+
+@pytest.fixture
+def synced_distro(
+    skip_without_cache,
+    python_remote_factory,
+    python_repo_with_sync,
+    python_distribution_factory,
+):
+    """
+    Sync a repo and create a distribution for cache tests.
+    """
+    remote = python_remote_factory(includes=PYTHON_SM_PROJECT_SPECIFIER)
+    repo = python_repo_with_sync(remote)
+    return python_distribution_factory(repository=repo)
+
+
+@pytest.mark.parallel
+def test_simple_cache_hit_miss_and_headers(synced_distro):
+    """
+    First request is a MISS, second is a HIT. Cache headers are present and stable.
+    """
+    index_url = urljoin(synced_distro.base_url, "simple/")
+    detail_url = f"{index_url}aiohttp"
+
+    for url in [index_url, detail_url]:
+        r1 = requests.get(url)
+        assert r1.status_code == 200
+        assert r1.headers["X-PULP-CACHE"] == "MISS"
+        assert r1.headers["Cache-Control"] == "max-age=600, public"
+        assert r1.headers["ETag"].startswith('"') and r1.headers["ETag"].endswith('"')
+
+        r2 = requests.get(url)
+        assert r2.status_code == 200
+        assert r2.headers["X-PULP-CACHE"] == "HIT"
+        assert r2.headers["Cache-Control"] == r1.headers["Cache-Control"]
+        assert r2.headers["ETag"] == r1.headers["ETag"]
+
+
+@pytest.mark.parallel
+def test_simple_cache_separate_accept_headers(synced_distro):
+    """
+    HTML and JSON responses are cached separately.
+    """
+    url = urljoin(synced_distro.base_url, "simple/")
+
+    for header in [PYPI_SIMPLE_V1_HTML, PYPI_SIMPLE_V1_JSON]:
+        r = requests.get(url, headers={"Accept": header})
+        assert r.status_code == 200
+        assert r.headers["X-PULP-CACHE"] == "MISS"
+
+    for header in [PYPI_SIMPLE_V1_HTML, PYPI_SIMPLE_V1_JSON]:
+        r = requests.get(url, headers={"Accept": header})
+        assert r.status_code == 200
+        assert r.headers["X-PULP-CACHE"] == "HIT"
+
+
+@pytest.mark.parallel
+def test_simple_cache_etag_conditional_request(synced_distro):
+    """
+    Matching If-None-Match returns 304, non-matching returns 200.
+    """
+    url = urljoin(synced_distro.base_url, "simple/")
+
+    r1 = requests.get(url)
+    assert r1.status_code == 200
+    etag = r1.headers["ETag"]
+
+    r2 = requests.get(url, headers={"If-None-Match": etag})
+    assert r2.status_code == 304
+    assert r2.headers["ETag"] == etag
+    assert len(r2.content) == 0
+
+    r3 = requests.get(url, headers={"If-None-Match": '"old"'})
+    assert r3.status_code == 200
+    assert r3.headers["ETag"] == etag
+    assert len(r3.content) > 0