From a70fe1d0641ed363f70b9e71a86ac850f6dc0cc9 Mon Sep 17 00:00:00 2001 From: Jitka Halova Date: Tue, 14 Jul 2026 16:53:36 +0200 Subject: [PATCH] Cache Simple API responses closes #1254 Assisted By: Claude Opus 4.6 --- CHANGES/1254.feature | 1 + pulp_python/app/cache.py | 65 +++++++++++++ pulp_python/app/pypi/views.py | 31 +++++- .../tests/functional/api/test_simple_cache.py | 96 +++++++++++++++++++ 4 files changed, 188 insertions(+), 5 deletions(-) create mode 100644 CHANGES/1254.feature create mode 100644 pulp_python/app/cache.py create mode 100644 pulp_python/tests/functional/api/test_simple_cache.py diff --git a/CHANGES/1254.feature b/CHANGES/1254.feature new file mode 100644 index 00000000..48ec3b1c --- /dev/null +++ b/CHANGES/1254.feature @@ -0,0 +1 @@ +Added server-side caching and HTTP cache headers (`Cache-Control`, `ETag`) to Simple API responses. diff --git a/pulp_python/app/cache.py b/pulp_python/app/cache.py new file mode 100644 index 00000000..1aa53a1b --- /dev/null +++ b/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) diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index e228d88d..1b69975e 100644 --- a/pulp_python/app/pypi/views.py +++ b/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()) diff --git a/pulp_python/tests/functional/api/test_simple_cache.py b/pulp_python/tests/functional/api/test_simple_cache.py new file mode 100644 index 00000000..904bbca0 --- /dev/null +++ b/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