Skip to content

Commit 5ebca12

Browse files
committed
Cache Simple API responses
closes #1254 Assisted By: Claude Opus 4.6
1 parent f168ae8 commit 5ebca12

4 files changed

Lines changed: 184 additions & 5 deletions

File tree

CHANGES/1254.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Added server-side caching and HTTP cache headers (`Cache-Control`, `ETag`) to Simple API responses.

pulp_python/app/cache.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
from django.core.exceptions import ObjectDoesNotExist
2+
from django.http import Http404
3+
4+
from pulpcore.plugin.cache import CacheKeys, SyncContentCache
5+
from pulpcore.plugin.util import cache_key, get_domain
6+
7+
from pulp_python.app.models import PythonDistribution
8+
9+
ACCEPT_HEADER_KEY = "accept_header"
10+
11+
12+
class PythonApiCache(SyncContentCache):
13+
"""
14+
Cache for the Simple API.
15+
16+
Adds Accept header to the cache key so HTML and JSON responses are cached separately.
17+
"""
18+
19+
def __init__(self, base_key=None):
20+
keys = (CacheKeys.path, CacheKeys.method, ACCEPT_HEADER_KEY)
21+
super().__init__(base_key=base_key, keys=keys)
22+
23+
def make_key(self, request):
24+
all_keys = {
25+
CacheKeys.path: request.path,
26+
CacheKeys.method: request.method,
27+
ACCEPT_HEADER_KEY: request.headers.get("accept", ""),
28+
}
29+
return ":".join(all_keys[k] for k in self.keys)
30+
31+
32+
def find_base_path_cached(request, cached):
33+
"""
34+
Resolve the distribution base_path for use as the Redis cache base_key.
35+
"""
36+
path = request.resolver_match.kwargs["path"]
37+
base_key = cache_key(path)
38+
if cached.exists(base_key=base_key):
39+
return base_key
40+
try:
41+
distro = PythonDistribution.objects.get(base_path=path, pulp_domain=get_domain())
42+
except ObjectDoesNotExist:
43+
raise Http404(f"No PythonDistribution found for base_path {path}")
44+
return cache_key(distro.base_path)

pulp_python/app/pypi/views.py

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import hashlib
12
import logging
23
from datetime import datetime, timedelta, timezone
34
from itertools import chain
@@ -15,9 +16,10 @@
1516
HttpResponseBadRequest,
1617
HttpResponseForbidden,
1718
HttpResponseNotFound,
18-
StreamingHttpResponse,
1919
)
2020
from django.shortcuts import redirect
21+
from django.utils.decorators import method_decorator
22+
from django.views.decorators.http import condition
2123
from drf_spectacular.utils import extend_schema
2224
from dynaconf import settings
2325
from packaging.utils import canonicalize_name
@@ -31,6 +33,7 @@
3133
from pulpcore.plugin.viewsets import OperationPostponedResponse
3234

3335
from pulp_python.app import tasks
36+
from pulp_python.app.cache import PythonApiCache, find_base_path_cached
3437
from pulp_python.app.models import (
3538
PackageProvenance,
3639
PythonDistribution,
@@ -63,6 +66,30 @@
6366

6467
PYPI_SIMPLE_V1_HTML = "application/vnd.pypi.simple.v1+html"
6568
PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json"
69+
CACHE_CONTROL = "max-age=600, public"
70+
71+
72+
def _add_cache_control_to_304(func):
73+
"""Ensure 304 responses include Cache-Control."""
74+
75+
def wrapper(*args, **kwargs):
76+
response = func(*args, **kwargs)
77+
if response.status_code == 304:
78+
response["Cache-Control"] = CACHE_CONTROL
79+
return response
80+
81+
return wrapper
82+
83+
84+
def _etag_func(request, path, **kwargs):
85+
"""Compute unquoted ETag for the condition decorator. Returns None if no repo."""
86+
distro = PyPIMixin.get_distribution(path)
87+
try:
88+
repo_ver = PyPIMixin.get_repository_version(distro)
89+
except Http404:
90+
return None
91+
raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}"
92+
return hashlib.sha256(raw.encode()).hexdigest()[:16]
6693

6794

6895
class PyPISimpleHTMLRenderer(TemplateHTMLRenderer):
@@ -298,6 +325,9 @@ def get_provenance_url(self, package, version, filename):
298325
)
299326

300327
@extend_schema(summary="Get index simple page")
328+
@method_decorator(_add_cache_control_to_304)
329+
@method_decorator(condition(etag_func=_etag_func))
330+
@PythonApiCache(base_key=find_base_path_cached)
301331
def list(self, request, path):
302332
"""Gets the simple api html page for the index."""
303333
repo_version, content = self.get_rvc()
@@ -310,15 +340,15 @@ def list(self, request, path):
310340
.iterator()
311341
)
312342
media_type = request.accepted_renderer.media_type
313-
headers = {"X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT)}
343+
headers = {"X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT), "Cache-Control": CACHE_CONTROL}
314344

315345
if media_type == PYPI_SIMPLE_V1_JSON:
316346
index_data = write_simple_index_json(names)
317347
return Response(index_data, headers=headers)
318348
else:
319-
index_data = write_simple_index(names, streamed=True)
349+
index_data = write_simple_index(names)
320350
kwargs = {"content_type": media_type, "headers": headers}
321-
return StreamingHttpResponse(index_data, **kwargs)
351+
return HttpResponse(index_data, **kwargs)
322352

323353
def pull_through_package_simple(self, package, path, remote):
324354
"""Gets the package's simple page from remote."""
@@ -355,6 +385,9 @@ def parse_package(release_package):
355385
}
356386

357387
@extend_schema(operation_id="pypi_simple_package_read", summary="Get package simple page")
388+
@method_decorator(_add_cache_control_to_304)
389+
@method_decorator(condition(etag_func=_etag_func))
390+
@PythonApiCache(base_key=find_base_path_cached)
358391
def retrieve(self, request, path, package):
359392
"""Retrieves the simple api html/json page for a package."""
360393
repo_ver, content = self.get_rvc()
@@ -405,7 +438,7 @@ def retrieve(self, request, path, package):
405438
return HttpResponseNotFound(f"{normalized} does not exist.")
406439

407440
media_type = request.accepted_renderer.media_type
408-
headers = {"X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT)}
441+
headers = {"X-PyPI-Last-Serial": str(PYPI_SERIAL_CONSTANT), "Cache-Control": CACHE_CONTROL}
409442

410443
if media_type == PYPI_SIMPLE_V1_JSON:
411444
detail_data = write_simple_detail_json(normalized, releases.values())
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
from urllib.parse import urljoin
2+
3+
import pytest
4+
import requests
5+
6+
from pulp_python.tests.functional.constants import (
7+
PYPI_SIMPLE_V1_HTML,
8+
PYPI_SIMPLE_V1_JSON,
9+
PYTHON_SM_PROJECT_SPECIFIER,
10+
)
11+
12+
13+
@pytest.fixture
14+
def skip_without_cache(pulp_settings):
15+
"""
16+
Skip test if server-side caching is not enabled.
17+
"""
18+
if not pulp_settings.CACHE_ENABLED:
19+
pytest.skip("CACHE_ENABLED is not set")
20+
21+
22+
@pytest.fixture
23+
def synced_distro(
24+
skip_without_cache,
25+
python_remote_factory,
26+
python_repo_with_sync,
27+
python_distribution_factory,
28+
):
29+
"""
30+
Sync a repo and create a distribution for cache tests.
31+
"""
32+
remote = python_remote_factory(includes=PYTHON_SM_PROJECT_SPECIFIER)
33+
repo = python_repo_with_sync(remote)
34+
return python_distribution_factory(repository=repo)
35+
36+
37+
@pytest.mark.parallel
38+
def test_simple_cache_hit_miss_and_headers(synced_distro):
39+
"""
40+
First request is a MISS, second is a HIT. Cache headers are present and stable.
41+
"""
42+
index_url = urljoin(synced_distro.base_url, "simple/")
43+
detail_url = f"{index_url}aiohttp"
44+
45+
for url in [index_url, detail_url]:
46+
r1 = requests.get(url)
47+
assert r1.status_code == 200
48+
assert r1.headers["X-PULP-CACHE"] == "MISS"
49+
assert r1.headers["Cache-Control"] == "max-age=600, public"
50+
assert r1.headers["ETag"].startswith('"') and r1.headers["ETag"].endswith('"')
51+
52+
r2 = requests.get(url)
53+
assert r2.status_code == 200
54+
assert r2.headers["X-PULP-CACHE"] == "HIT"
55+
assert r2.headers["Cache-Control"] == r1.headers["Cache-Control"]
56+
assert r2.headers["ETag"] == r1.headers["ETag"]
57+
58+
59+
@pytest.mark.parallel
60+
def test_simple_cache_separate_accept_headers(synced_distro):
61+
"""
62+
HTML and JSON responses are cached separately.
63+
"""
64+
url = urljoin(synced_distro.base_url, "simple/")
65+
66+
for header in [PYPI_SIMPLE_V1_HTML, PYPI_SIMPLE_V1_JSON]:
67+
r = requests.get(url, headers={"Accept": header})
68+
assert r.status_code == 200
69+
assert r.headers["X-PULP-CACHE"] == "MISS"
70+
71+
for header in [PYPI_SIMPLE_V1_HTML, PYPI_SIMPLE_V1_JSON]:
72+
r = requests.get(url, headers={"Accept": header})
73+
assert r.status_code == 200
74+
assert r.headers["X-PULP-CACHE"] == "HIT"
75+
76+
77+
@pytest.mark.parallel
78+
def test_simple_cache_etag_conditional_request(synced_distro):
79+
"""
80+
Matching If-None-Match returns 304, non-matching returns 200.
81+
"""
82+
url = urljoin(synced_distro.base_url, "simple/")
83+
84+
r1 = requests.get(url)
85+
assert r1.status_code == 200
86+
etag = r1.headers["ETag"]
87+
cache_control = r1.headers["Cache-Control"]
88+
89+
r2 = requests.get(url, headers={"If-None-Match": etag})
90+
assert r2.status_code == 304
91+
assert r2.headers["ETag"] == etag
92+
assert r2.headers["Cache-Control"] == cache_control
93+
assert "X-PULP-CACHE" not in r2.headers
94+
assert len(r2.content) == 0
95+
96+
r3 = requests.get(url, headers={"If-None-Match": '"old"'})
97+
assert r3.status_code == 200
98+
assert r3.headers["ETag"] == etag
99+
assert r3.headers["Cache-Control"] == cache_control
100+
assert r3.headers["X-PULP-CACHE"] == "HIT"
101+
assert len(r3.content) > 0

0 commit comments

Comments
 (0)