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..f67c5c9e --- /dev/null +++ b/pulp_python/app/cache.py @@ -0,0 +1,32 @@ +from pulpcore.plugin.cache import CacheKeys, SyncContentCache +from pulpcore.plugin.util import cache_key + +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. + """ + + def __init__(self, base_key=None): + keys = (CacheKeys.path, CacheKeys.method, ACCEPT_HEADER_KEY) + super().__init__(base_key=base_key, keys=keys) + + 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"] + return cache_key(path) diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index e228d88d..102749bd 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,9 +16,11 @@ HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound, - StreamingHttpResponse, ) from django.shortcuts import redirect +from django.utils.decorators import method_decorator +from django.views.decorators.cache import cache_control +from django.views.decorators.http import condition from drf_spectacular.utils import extend_schema from dynaconf import settings from packaging.utils import canonicalize_name @@ -31,6 +34,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, @@ -65,6 +69,17 @@ PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json" +def _etag_func(request, path, **kwargs): + """Compute unquoted ETag for the condition decorator. Returns None if no repo.""" + try: + distro = PyPIMixin.get_distribution(path) + repo_ver = PyPIMixin.get_repository_version(distro) + except Http404: + return None + raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}" + return hashlib.sha256(raw.encode()).hexdigest()[:16] + + class PyPISimpleHTMLRenderer(TemplateHTMLRenderer): media_type = PYPI_SIMPLE_V1_HTML @@ -298,6 +313,9 @@ def get_provenance_url(self, package, version, filename): ) @extend_schema(summary="Get index simple page") + @method_decorator(cache_control(max_age=600, public=True)) + @method_decorator(condition(etag_func=_etag_func)) + @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() @@ -316,9 +334,9 @@ def list(self, request, path): 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,6 +373,9 @@ def parse_package(release_package): } @extend_schema(operation_id="pypi_simple_package_read", summary="Get package simple page") + @method_decorator(cache_control(max_age=600, public=True)) + @method_decorator(condition(etag_func=_etag_func)) + @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() 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..f2123e61 --- /dev/null +++ b/pulp_python/tests/functional/api/test_simple_cache.py @@ -0,0 +1,101 @@ +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"] + cache_control = r1.headers["Cache-Control"] + + r2 = requests.get(url, headers={"If-None-Match": etag}) + assert r2.status_code == 304 + assert r2.headers["ETag"] == etag + assert r2.headers["Cache-Control"] == cache_control + assert "X-PULP-CACHE" not in r2.headers + 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 r3.headers["Cache-Control"] == cache_control + assert r3.headers["X-PULP-CACHE"] == "HIT" + assert len(r3.content) > 0