Add conditional-request support to PyPI metadata APIs

CHANGES/1338.feature

@@ -0,0 +1 @@
+Added conditional request support (`Last-Modified` / `If-Modified-Since` and `ETag` / `If-None-Match`) to the Simple API and JSON Metadata API for improved cache efficiency.

pulp_python/app/pypi/views.py

@@ -72,17 +72,32 @@
 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."""
+def _get_repo_version(path):
+    """Resolve path to a RepositoryVersion, or None if not found."""
     try:
         distro = PyPIMixin.get_distribution(path)
-        repo_ver = PyPIMixin.get_repository_version(distro)
+        return PyPIMixin.get_repository_version(distro)
     except Http404:
         return None
+
+
+def _etag_func(request, path, **kwargs):
+    """Compute unquoted ETag for the condition decorator. Returns None if no repo."""
+    repo_ver = _get_repo_version(path)
+    if repo_ver is None:
+        return None
     raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}"
     return hashlib.sha256(raw.encode()).hexdigest()[:16]
 
 
+def _last_modified_func(request, path, **kwargs):
+    """Return the repository version creation timestamp for Last-Modified."""
+    repo_ver = _get_repo_version(path)
+    if repo_ver is None:
+        return None
+    return repo_ver.pulp_created
+
+
 class PyPISimpleHTMLRenderer(TemplateHTMLRenderer):
     media_type = PYPI_SIMPLE_V1_HTML
 
@@ -317,7 +332,7 @@ 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))
+    @method_decorator(condition(etag_func=_etag_func, last_modified_func=_last_modified_func))
     @PythonApiCache(base_key=find_base_path_cached)
     def list(self, request, path):
         """Gets the simple api html page for the index."""
@@ -379,7 +394,7 @@ 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))
+    @method_decorator(condition(etag_func=_etag_func, last_modified_func=_last_modified_func))
     @PythonApiCache(base_key=find_base_path_cached)
     def retrieve(self, request, path, package):
         """Retrieves the simple api html/json page for a package."""
@@ -482,6 +497,8 @@ class MetadataView(PyPIMixin, ViewSet):
         responses={200: PackageMetadataSerializer},
         summary="Get package metadata",
     )
+    @method_decorator(cache_control(max_age=900, public=True))
+    @method_decorator(condition(etag_func=_etag_func, last_modified_func=_last_modified_func))
     def retrieve(self, request, path, meta):
         """
         Retrieves the package's core-metadata specified by

pulp_python/tests/functional/api/test_simple_cache.py

@@ -1,3 +1,4 @@
+from email.utils import parsedate_to_datetime
 from urllib.parse import urljoin
 
 import pytest
@@ -35,6 +36,20 @@ def synced_distro(
     return python_distribution_factory(repository=repo)
 
 
+@pytest.fixture
+def synced_distro_no_cache(
+    python_remote_factory,
+    python_repo_with_sync,
+    python_distribution_factory,
+):
+    """
+    Sync a repo and create a distribution (no cache requirement).
+    """
+    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):
     """
@@ -139,3 +154,159 @@ def test_simple_cache_etag_conditional_request(synced_distro):
     assert r3.headers["Cache-Control"] == cache_control
     assert r3.headers["X-PULP-CACHE"] == "HIT"
     assert len(r3.content) > 0
+
+
+@pytest.mark.parallel
+def test_simple_last_modified_header(synced_distro_no_cache):
+    """Simple API responses include Last-Modified header."""
+    index_url = urljoin(synced_distro_no_cache.base_url, "simple/")
+    detail_url = f"{index_url}aiohttp"
+
+    for url in [index_url, detail_url]:
+        r = requests.get(url)
+        assert r.status_code == 200
+        assert "Last-Modified" in r.headers
+        parsedate_to_datetime(r.headers["Last-Modified"])
+
+
+@pytest.mark.parallel
+def test_simple_if_modified_since_304(synced_distro_no_cache):
+    """If-Modified-Since with matching timestamp returns 304."""
+    url = urljoin(synced_distro_no_cache.base_url, "simple/")
+
+    r1 = requests.get(url)
+    assert r1.status_code == 200
+    last_modified = r1.headers["Last-Modified"]
+
+    r2 = requests.get(url, headers={"If-Modified-Since": last_modified})
+    assert r2.status_code == 304
+    assert len(r2.content) == 0
+
+
+@pytest.mark.parallel
+def test_simple_if_modified_since_old_timestamp_200(synced_distro_no_cache):
+    """If-Modified-Since with old timestamp returns 200 with content."""
+    url = urljoin(synced_distro_no_cache.base_url, "simple/")
+
+    r1 = requests.get(url)
+    assert r1.status_code == 200
+
+    r2 = requests.get(url, headers={"If-Modified-Since": "Thu, 01 Jan 2009 00:00:00 GMT"})
+    assert r2.status_code == 200
+    assert len(r2.content) > 0
+
+
+@pytest.mark.parallel
+def test_metadata_conditional_request_headers(synced_distro_no_cache):
+    """JSON metadata responses include ETag, Last-Modified, and Cache-Control headers."""
+    url = urljoin(synced_distro_no_cache.base_url, "pypi/aiohttp/json/")
+
+    r = requests.get(url)
+    assert r.status_code == 200
+    assert r.headers["Cache-Control"] == "max-age=900, public"
+    assert "ETag" in r.headers
+    assert r.headers["ETag"].startswith('"') and r.headers["ETag"].endswith('"')
+    assert "Last-Modified" in r.headers
+    parsedate_to_datetime(r.headers["Last-Modified"])
+
+
+@pytest.mark.parallel
+def test_metadata_etag_conditional_request(synced_distro_no_cache):
+    """JSON metadata: matching If-None-Match returns 304, non-matching returns 200."""
+    url = urljoin(synced_distro_no_cache.base_url, "pypi/aiohttp/json/")
+
+    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 len(r2.content) == 0
+
+    r3 = requests.get(url, headers={"If-None-Match": '"old"'})
+    assert r3.status_code == 200
+    assert r3.headers["ETag"] == etag
+
+
+@pytest.mark.parallel
+def test_metadata_if_modified_since_304(synced_distro_no_cache):
+    """JSON metadata: If-Modified-Since with matching timestamp returns 304."""
+    url = urljoin(synced_distro_no_cache.base_url, "pypi/aiohttp/json/")
+
+    r1 = requests.get(url)
+    assert r1.status_code == 200
+    last_modified = r1.headers["Last-Modified"]
+
+    r2 = requests.get(url, headers={"If-Modified-Since": last_modified})
+    assert r2.status_code == 304
+    assert len(r2.content) == 0
+
+
+@pytest.mark.parallel
+def test_metadata_if_modified_since_old_timestamp_200(synced_distro_no_cache):
+    """JSON metadata: If-Modified-Since with old timestamp returns 200."""
+    url = urljoin(synced_distro_no_cache.base_url, "pypi/aiohttp/json/")
+
+    r1 = requests.get(url)
+    assert r1.status_code == 200
+
+    r2 = requests.get(url, headers={"If-Modified-Since": "Thu, 01 Jan 2009 00:00:00 GMT"})
+    assert r2.status_code == 200
+    assert len(r2.content) > 0
+
+
+def test_unauthorized_gets_403_not_304(synced_distro_no_cache, pulpcore_bindings, bindings_cfg):
+    """Unauthorized client gets 403, not 304, even with conditional request headers."""
+    admin_auth = (bindings_cfg.username, bindings_cfg.password)
+    simple_url = urljoin(synced_distro_no_cache.base_url, "simple/")
+
+    r1 = requests.get(simple_url, auth=admin_auth)
+    assert r1.status_code == 200
+    last_modified = r1.headers["Last-Modified"]
+    etag = r1.headers["ETag"]
+
+    ap_response = pulpcore_bindings.AccessPoliciesApi.list(viewset_name="pypi/simple")
+    assert ap_response.count == 1
+    ap_href = ap_response.results[0].pulp_href
+
+    anon = requests.Session()
+    anon.trust_env = False
+    anon.verify = False
+
+
+    try:
+        pulpcore_bindings.AccessPoliciesApi.partial_update(
+            ap_href,
+            {
+                "statements": [
+                    {
+                        "action": ["list", "retrieve"],
+                        "principal": "authenticated",
+                        "effect": "allow",
+                    },
+                    {
+                        "action": ["create"],
+                        "principal": "authenticated",
+                        "effect": "allow",
+                        "condition": "index_has_repo_perm:python.modify_pythonrepository",
+                    },
+                ],
+            },
+        )
+
+        r_ims = anon.get(simple_url, headers={"If-Modified-Since": last_modified})
+        assert r_ims.status_code == 403, (
+            f"Expected 403 for unauthorized If-Modified-Since, got {r_ims.status_code}"
+        )
+
+        r_inm = anon.get(simple_url, headers={"If-None-Match": etag})
+        assert r_inm.status_code == 403, (
+            f"Expected 403 for unauthorized If-None-Match, got {r_inm.status_code}"
+        )
+
+        r_authed = requests.get(
+            simple_url, auth=admin_auth, headers={"If-Modified-Since": last_modified}
+        )
+        assert r_authed.status_code == 304
+    finally:
+        pulpcore_bindings.AccessPoliciesApi.reset(ap_href)