From 795a3814fe9fa17b29bc0bb9107128966d0ea968 Mon Sep 17 00:00:00 2001 From: TenSt Date: Tue, 6 Oct 2026 14:23:25 +0200 Subject: [PATCH 1/7] Add a paginated repository endpoint that lists Python packages. Clients can walk every package in a repository version. Each row is one package name, and each stored version is one entry. Closes #1358. Assisted-By: Cursor (cherry picked from commit bc2c3d881b9801dc5d6c3f21ef95dba0cc123660) --- .gitignore | 4 + CHANGES/1358.feature | 1 + docs/index.md | 4 +- docs/user/guides/_SUMMARY.md | 1 + docs/user/guides/catalog.md | 44 ++++ pulp_python/app/catalog.py | 186 +++++++++++++++ pulp_python/app/pypi/views.py | 2 +- pulp_python/app/serializers.py | 51 +++++ pulp_python/app/utils.py | 1 - pulp_python/app/viewsets.py | 97 +++++++- .../tests/functional/api/test_catalog.py | 211 ++++++++++++++++++ .../functional/api/test_crud_publications.py | 64 ++++-- .../tests/functional/api/test_domains.py | 34 ++- pulp_python/tests/unit/test_catalog.py | 125 +++++++++++ 14 files changed, 802 insertions(+), 23 deletions(-) create mode 100644 CHANGES/1358.feature create mode 100644 docs/user/guides/catalog.md create mode 100644 pulp_python/app/catalog.py create mode 100644 pulp_python/tests/functional/api/test_catalog.py create mode 100644 pulp_python/tests/unit/test_catalog.py diff --git a/.gitignore b/.gitignore index 29c94b654..444a89de7 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ __pycache__/ # Distribution / packaging .Python env/ +.venv/ build/ develop-eggs/ dist/ @@ -61,3 +62,6 @@ target/ # PyCharm .idea + +# VS Code / Cursor +.vscode/ diff --git a/CHANGES/1358.feature b/CHANGES/1358.feature new file mode 100644 index 000000000..5cb68bb99 --- /dev/null +++ b/CHANGES/1358.feature @@ -0,0 +1 @@ +Added a repository endpoint that lists Python packages with limit/offset pagination. diff --git a/docs/index.md b/docs/index.md index 80c26e434..ff9bb16d1 100644 --- a/docs/index.md +++ b/docs/index.md @@ -12,6 +12,7 @@ The REST API documentation for `pulp_python` is available [here](site:pulp_pytho - [Create local mirrors of PyPI](site:pulp_python/docs/user/guides/sync/) that you have full control over - [Upload your own Python packages](site:pulp_python/docs/user/guides/upload/) +- [Browse the package catalog](site:pulp_python/docs/user/guides/catalog/) over the REST API - [Perform pip install](site:pulp_python/docs/user/guides/host/) from your Pulp Python repositories - Download packages on-demand to reduce disk usage - Every operation creates a restorable snapshot with Versioned Repositories @@ -34,5 +35,4 @@ Users may also find pulpcore’s conceptual docs useful. This documentation falls into two main categories: 1. `How-to Guides` shows the **major features** of the Python plugin, with links to reference docs. -2. The [REST API Docs](site:pulp_python/restapi/) are automatically generated and provide more detailed information for each -minor feature, including all fields and options. +2. The [REST API Docs](site:pulp_python/restapi/) are automatically generated and provide more detailed information for each minor feature, including all fields and options. diff --git a/docs/user/guides/_SUMMARY.md b/docs/user/guides/_SUMMARY.md index d9c01bdb2..36699ede5 100644 --- a/docs/user/guides/_SUMMARY.md +++ b/docs/user/guides/_SUMMARY.md @@ -1,6 +1,7 @@ * [Set up your own PyPI](pypi.md) * [Sync from Remote Repositories](sync.md) * [Upload and Manage Content](upload.md) +* [Browse the package catalog](catalog.md) * [Host Python Content](host.md) * [Vulnerability Report](vulnerability_report.md) * [Attestation Hosting](attestation.md) diff --git a/docs/user/guides/catalog.md b/docs/user/guides/catalog.md new file mode 100644 index 000000000..1632c7a7d --- /dev/null +++ b/docs/user/guides/catalog.md @@ -0,0 +1,44 @@ +# Browse the package catalog + +The package list returns **one row per package name**. It defaults to the +repository's latest complete version. Pass `repository_version` (HREF or PRN) +to read an older snapshot. `{pulp_id}` is the repository UUID. + +## List packages + +```bash +http GET "${BASE_ADDR}/pulp/api/v3/repositories/python/python/${REPO_PK}/packages/?limit=10" +``` + +`count` is the number of distinct packages, not files. Each row is one package +name. `summary`, `description`, `author`, `author_email`, and `license` come +from the newest version. `versions` has one entry per stored version string, +newest first, with that version's `license_expression`. A wheel and an sdist +of the same version are one entry. A rebuild such as `1.0.0+test.1` stays its +own entry because the stored version differs. + +```json +{ + "count": 1, + "next": null, + "previous": null, + "results": [ + { + "name": "shelf-reader", + "name_normalized": "shelf-reader", + "summary": "A small example package", + "description": "", + "author": "", + "author_email": "", + "license": "", + "versions": [ + {"version": "0.1", "license_expression": ""} + ] + } + ] +} +``` + +Results are ordered by `name_normalized`. Within a package, versions are newest +first. Page with `limit` and `offset` to read the whole catalog. Pass +`repository_version` when the export must stay on one snapshot. diff --git a/pulp_python/app/catalog.py b/pulp_python/app/catalog.py new file mode 100644 index 000000000..5ec58b575 --- /dev/null +++ b/pulp_python/app/catalog.py @@ -0,0 +1,186 @@ +"""Helpers for the repository package list.""" + +from collections import defaultdict +from urllib.parse import urlparse + +from django.core.exceptions import FieldError +from django.core.exceptions import ValidationError as DjangoValidationError +from django.db.models import Case, IntegerField, Q, Value, When +from django.urls import Resolver404, resolve +from packaging.version import InvalidVersion, Version + +from pulpcore.plugin.models import Repository, RepositoryContent, RepositoryVersion +from pulpcore.plugin.util import resolve_prn + +from pulp_python.app.models import PythonPackageContent + + +def _version_sort_key(version): + """PEP 440 sort key. Use with ``reverse=True`` for newest first. + + Invalid versions sort after all valid ones when ``reverse=True``. + """ + if not version: + return (-1, "") + try: + return (0, Version(version)) + except InvalidVersion: + return (-1, str(version)) + + +def _in_version_bounds_q(repository_version, prefix=""): + """Version-added/removed bounds for rows present in ``repository_version``. + + ``prefix`` is empty for ``RepositoryContent`` and ``in_repo__`` for the + filtered membership join. Both catalog paths use this helper. + """ + return Q(**{f"{prefix}version_added__number__lte": repository_version.number}) & ( + Q(**{f"{prefix}version_removed__isnull": True}) + | Q(**{f"{prefix}version_removed__number__gt": repository_version.number}) + ) + + +def memberships_in_version(repository_version): + """RepositoryContent rows contained in ``repository_version``. + + A subquery against this queryset keeps the content-id list in the database. + ``RepositoryVersion.content`` inlines ``content_ids`` as one bound UUID per + content unit whenever that array is shorter than 65535. + """ + return RepositoryContent.objects.filter( + repository_id=repository_version.repository_id, + ).filter(_in_version_bounds_q(repository_version)) + + +def python_packages_in_version(repository_version): + """Python package content contained in ``repository_version``.""" + if repository_version is None: + return PythonPackageContent.objects.none() + content_ids = ( + memberships_in_version(repository_version) + .filter(content__pulp_type=PythonPackageContent.get_pulp_type()) + .order_by() + .values("content_id") + ) + return PythonPackageContent.objects.filter(pk__in=content_ids) + + +def resolve_repository_version(href): + """Load a repository version without selecting ``content_ids``. + + Accepts a repository-version or repository HREF/PRN. Domain-prefixed hrefs + include ``pulp_domain`` in the resolved kwargs; that field lives on the + repository, not ``RepositoryVersion``. + """ + if not href: + raise ValueError("No value supplied for repository version.") + + if href.startswith("prn:"): + model, pk = resolve_prn(href) + found_kwargs = {"pk": pk} + else: + try: + match = resolve(urlparse(href).path) + except Resolver404: + raise ValueError(f"URI not valid: {href}") from None + model = match.func.cls.queryset.model + found_kwargs = match.kwargs + + if "pk" in found_kwargs: + lookup = {"pk": found_kwargs["pk"]} + else: + lookup = {} + for key, value in found_kwargs.items(): + if key.endswith("_pk"): + lookup[f"{key[:-3]}__pk"] = value + elif key == "pulp_domain": + if hasattr(model, "pulp_domain"): + lookup["pulp_domain__name"] = value + elif key in ("api_root", "version"): + continue + else: + lookup[key] = value + + qs = model.objects.all() + if issubclass(model, RepositoryVersion): + qs = qs.defer("content_ids") + try: + obj = qs.get(**lookup) + except model.DoesNotExist: + raise ValueError(f"URI {href} not found.") from None + except (model.MultipleObjectsReturned, DjangoValidationError, FieldError): + raise ValueError(f"URI {href} is not a valid repository version.") from None + + if isinstance(obj, RepositoryVersion): + return obj + if isinstance(obj, Repository): + try: + return obj.versions.complete().defer("content_ids").latest() + except RepositoryVersion.DoesNotExist: + return None + raise ValueError("Must be a repository version.") + + +def distinct_package_names_qs(content_qs): + """One row per distinct ``name_normalized``, ordered for stable pagination.""" + return content_qs.order_by().values("name_normalized").distinct().order_by("name_normalized") + + +def assemble_package_index(content_qs, name_rows, repository_version): + """One catalog row per package name, with one content unit per stored version. + + Wheel and sdist of the same ``(name_normalized, version)`` collapse to one + row. An sdist is preferred; otherwise the newest ``pulp_created`` is kept. + Version strings are the values stored on the content unit, newest first. + Summary, description, author, author email, and license are taken from the + newest version because they do not vary by version. + """ + if not name_rows or repository_version is None: + return [] + + names = [row["name_normalized"] for row in name_rows] + by_name = defaultdict(list) + # DISTINCT ON keeps the first row for each name and stored version. + units = ( + content_qs.filter(name_normalized__in=names) + .order_by( + "name_normalized", + "version", + Case( + When(packagetype="sdist", then=Value(0)), + default=Value(1), + output_field=IntegerField(), + ), + "-pulp_created", + "pk", + ) + .distinct("name_normalized", "version") + ) + for unit in units: + by_name[unit.name_normalized].append(unit) + + result = [] + for row in name_rows: + normalized = row["name_normalized"] + versions = sorted( + by_name.get(normalized, []), + key=lambda unit: (_version_sort_key(unit.version), unit.filename), + reverse=True, + ) + source = versions[0] if versions else None + result.append( + { + "name": source.name if source else normalized, + "name_normalized": normalized, + "summary": source.summary if source else "", + "description": source.description if source else "", + "author": source.author if source else "", + "author_email": source.author_email if source else "", + "license": source.license if source else "", + "versions": [ + {"version": unit.version, "license_expression": unit.license_expression} + for unit in versions + ], + } + ) + return result diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index 8d580a2f4..ae9748320 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -349,7 +349,7 @@ def list(self, request, path): if self.should_redirect(repo_version=repo_version): return redirect(urljoin(self.base_content_url, f"{path}/simple/")) names = ( - content.order_by("name_normalized") + content.order_by("name_normalized", "name") .values_list("name", flat=True) .distinct("name_normalized") .iterator() diff --git a/pulp_python/app/serializers.py b/pulp_python/app/serializers.py index c59a53980..f7fd7ee6d 100644 --- a/pulp_python/app/serializers.py +++ b/pulp_python/app/serializers.py @@ -641,6 +641,57 @@ class Meta: model = python_models.PythonPackageContent +class PythonRepositoryPackageVersionSerializer(serializers.Serializer): + """One stored version of a package.""" + + version = serializers.CharField(help_text=_("The package's version number.")) + license_expression = serializers.CharField( + allow_blank=True, + help_text=_("Text string that is a valid SPDX license expression."), + ) + + +class PythonRepositoryPackageSerializer(serializers.Serializer): + """One distinct package name and its stored versions.""" + + name = serializers.CharField( + help_text=_("A project name stored on one of this package's files."), + ) + name_normalized = serializers.CharField( + help_text=_("PEP 503 normalized package name. Index rows are unique on this field."), + ) + summary = serializers.CharField( + allow_blank=True, + help_text=_("A one-line summary of what the package does."), + ) + description = serializers.CharField( + allow_blank=True, + help_text=_("A longer description of the package that can run to several paragraphs."), + ) + author = serializers.CharField( + allow_blank=True, + help_text=_( + "Text containing the author's name. Contact information can also be added," + " separated with newlines." + ), + ) + author_email = serializers.CharField( + allow_blank=True, + help_text=_("The author's e-mail address."), + ) + license = serializers.CharField( + allow_blank=True, + help_text=_("Text indicating the license covering the distribution."), + ) + versions = PythonRepositoryPackageVersionSerializer( + many=True, + help_text=_( + "One entry per stored version, newest first. A wheel and an sdist of the " + "same version are one entry. Version strings are unchanged." + ), + ) + + class PackageProvenanceSerializer(core_serializers.NoArtifactContentUploadSerializer): """ A Serializer for PackageProvenance. diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index 1fd85e3eb..e3c05e354 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -28,7 +28,6 @@ log = logging.getLogger(__name__) - PYPI_LAST_SERIAL = "X-PYPI-LAST-SERIAL" """TODO This serial constant is temporary until Python repositories implements serials""" PYPI_SERIAL_CONSTANT = 1000000000 diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index 400c9398b..35f26a648 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -4,7 +4,8 @@ from django.db import transaction from django_filters import CharFilter from django_filters.rest_framework import filters as drf_filters -from drf_spectacular.utils import extend_schema, extend_schema_view +from drf_spectacular.types import OpenApiTypes +from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view from packaging.specifiers import InvalidSpecifier, SpecifierSet from packaging.utils import canonicalize_name from rest_framework import status @@ -32,6 +33,12 @@ from pulp_python.app import models as python_models from pulp_python.app import serializers as python_serializers from pulp_python.app import tasks +from pulp_python.app.catalog import ( + assemble_package_index, + distinct_package_names_qs, + python_packages_in_version, + resolve_repository_version, +) class PythonRepositoryViewSet( @@ -64,7 +71,7 @@ class PythonRepositoryViewSet( ], }, { - "action": ["retrieve"], + "action": ["retrieve", "packages"], "principal": "authenticated", "effect": "allow", "condition": "has_model_or_domain_or_obj_perms:python.view_pythonrepository", @@ -138,6 +145,46 @@ class PythonRepositoryViewSet( "python.pythonrepository_viewer": ["python.view_pythonrepository"], } + @property + def filter_backends(self): + """Repository filters apply to the repository list, not the package index. + + ``packages`` is a detail GET that returns a list. Spectacular treats that + as a list operation and would otherwise document ``RepositoryFilter``. + """ + if getattr(self, "action", None) == "packages": + return [] + return super().filter_backends + + def filter_queryset(self, queryset): + """Do not apply the repository FilterSet to package-index query params.""" + if getattr(self, "action", None) == "packages": + return queryset + return super().filter_queryset(queryset) + + def _repository_version_from_href(self, href): + """Load a repository version without selecting ``content_ids``.""" + try: + return resolve_repository_version(href) + except ValueError as exc: + raise ValidationError({"repository_version": str(exc)}) from exc + + def _requested_repository_version(self, repository): + """Resolve optional ``repository_version`` href/PRN, else latest complete version.""" + href = self.request.query_params.get("repository_version") + if not href: + try: + # content_ids is a UUID per content unit. The catalog never reads it. + return repository.versions.complete().defer("content_ids").latest() + except RepositoryVersion.DoesNotExist: + return None + repo_version = self._repository_version_from_href(href) + if repo_version is None: + return None + if repo_version.repository_id != repository.pk: + raise ValidationError({"repository_version": "Must be a version of this repository."}) + return repo_version + @extend_schema( description="Trigger an asynchronous task to create a new repository version.", summary="Modify Repository Content", @@ -247,6 +294,52 @@ def sync(self, request, pk, **kwargs): ) return core_viewsets.OperationPostponedResponse(result, request) + @extend_schema( + summary="List packages", + description=( + "Return one row per distinct package name in a repository version " + "(latest complete version if repository_version is omitted). " + "Each versions entry is the stored version and its license_expression " + "(wheel and sdist of the same version are a single entry). Results are " + "ordered by name_normalized. Pagination count is the number of " + "distinct packages, not files." + ), + parameters=[ + OpenApiParameter( + name="repository_version", + type=OpenApiTypes.URI, + location=OpenApiParameter.QUERY, + required=False, + description=( + "HREF or PRN of a version of this repository. " + "Defaults to the latest complete version." + ), + ), + ], + responses={200: python_serializers.PythonRepositoryPackageSerializer(many=True)}, + ) + @action( + detail=True, + methods=["get"], + serializer_class=python_serializers.PythonRepositoryPackageSerializer, + ) + def packages(self, request, pk, **kwargs): + """List distinct packages in a repository version.""" + repository = self.get_object() + repo_version = self._requested_repository_version(repository) + content_qs = python_packages_in_version(repo_version) + names_qs = distinct_package_names_qs(content_qs) + page = self.paginate_queryset(names_qs) + rows = assemble_package_index( + content_qs, + page if page is not None else list(names_qs), + repo_version, + ) + serializer = self.get_serializer(rows, many=True) + if page is not None: + return self.get_paginated_response(serializer.data) + return Response(serializer.data) + class PythonBlocklistEntryViewSet( core_viewsets.NamedModelViewSet, diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py new file mode 100644 index 000000000..6f5d167f7 --- /dev/null +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -0,0 +1,211 @@ +"""Catalog API tests. + +Generated client methods are unavailable until `oci-env generate-client` is rerun. +""" + +import io +import tarfile +import uuid +from urllib.parse import urljoin + +import pytest +import requests + +from pulp_python.tests.functional.constants import PYTHON_SM_PROJECT_SPECIFIER + + +def _api_get(bindings_cfg, path, **params): + url = urljoin(bindings_cfg.host + "/", path.lstrip("/")) + response = requests.get(url, params=params, auth=(bindings_cfg.username, bindings_cfg.password)) + assert response.status_code == 200, response.text + return response.json() + + +_PACKAGE_FIELDS = { + "name", + "name_normalized", + "summary", + "description", + "author", + "author_email", + "license", + "versions", +} + + +def _assert_package_row(pkg): + assert set(pkg) == _PACKAGE_FIELDS + assert pkg["name"] + assert pkg["name_normalized"] + versions = [item["version"] for item in pkg["versions"]] + assert versions + assert len(versions) == len(set(versions)) + for item in pkg["versions"]: + assert set(item) == {"version", "license_expression"} + assert item["version"] + + +def _write_sdist(directory, name, version): + """Write a minimal sdist whose PKG-INFO Name/Version pkginfo can read.""" + pkg_dir = f"{name}-{version}" + filename = f"{pkg_dir}.tar.gz" + path = directory / filename + pkg_info = f"Metadata-Version: 2.1\nName: {name}\nVersion: {version}\n".encode() + with tarfile.open(path, "w:gz") as tar: + info = tarfile.TarInfo(name=f"{pkg_dir}/PKG-INFO") + info.size = len(pkg_info) + tar.addfile(info, io.BytesIO(pkg_info)) + return filename, str(path) + + +def _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, version): + filename, path = _write_sdist(tmp_path, name, version) + python_content_factory(relative_path=filename, file=path, repository=repo) + return python_bindings.RepositoriesPythonApi.read(repo.pulp_href) + + +@pytest.fixture +def sm_repo(python_repo_with_sync, python_remote_factory): + remote = python_remote_factory(includes=PYTHON_SM_PROJECT_SPECIFIER) + return python_repo_with_sync(remote) + + +@pytest.mark.parallel +def test_package_list_grouping_and_pagination(bindings_cfg, sm_repo): + """Package index is one row per name, and count is distinct packages not files.""" + data = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", limit=1) + assert data["count"] == 3 + assert len(data["results"]) == 1 + _assert_package_row(data["results"][0]) + + page2 = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", limit=1, offset=1) + assert page2["count"] == 3 + assert page2["results"][0]["name_normalized"] != data["results"][0]["name_normalized"] + + all_rows = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", limit=100)["results"] + names = [pkg["name_normalized"] for pkg in all_rows] + assert names == sorted(names) + assert set(names) == {"aiohttp", "celery", "django"} + django = next(pkg for pkg in all_rows if pkg["name_normalized"] == "django") + assert {item["version"] for item in django["versions"]} >= { + "1.10.4", + "1.10.3", + "1.10.2", + "1.10.1", + } + assert all("+" not in item["version"] for item in django["versions"]) + + +@pytest.mark.parallel +def test_package_list_empty_repository(bindings_cfg, python_repo_factory): + repo = python_repo_factory() + data = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") + assert data["count"] == 0 + assert data["results"] == [] + + +@pytest.mark.parallel +def test_packages_repository_version(bindings_cfg, sm_repo, python_repo_factory): + """repository_version selects a snapshot; omitted uses the latest complete version.""" + latest_href = sm_repo.latest_version_href + v0_href = f"{sm_repo.pulp_href}versions/0/" + + default_pkgs = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/") + explicit_pkgs = _api_get( + bindings_cfg, f"{sm_repo.pulp_href}packages/", repository_version=latest_href + ) + assert default_pkgs["count"] == explicit_pkgs["count"] == 3 + + version = _api_get(bindings_cfg, latest_href) + prn_pkgs = _api_get( + bindings_cfg, f"{sm_repo.pulp_href}packages/", repository_version=version["prn"] + ) + assert prn_pkgs["count"] == 3 + assert version["prn"].startswith("prn:core.repositoryversion:") + + v0_pkgs = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", repository_version=v0_href) + assert v0_pkgs["count"] == 0 + assert v0_pkgs["results"] == [] + + other = python_repo_factory() + url = urljoin(bindings_cfg.host + "/", f"{sm_repo.pulp_href}packages/".lstrip("/")) + response = requests.get( + url, + params={"repository_version": other.latest_version_href}, + auth=(bindings_cfg.username, bindings_cfg.password), + ) + assert response.status_code == 400, response.text + + +@pytest.mark.parallel +def test_package_list_one_entry_per_stored_version( + bindings_cfg, python_bindings, python_repo_with_sync +): + """A wheel and an sdist of the same version are one version string.""" + repo = python_repo_with_sync() + files = python_bindings.ContentPackagesApi.list(repository_version=repo.latest_version_href) + assert files.count == 2 + assert {item.packagetype for item in files.results} == {"sdist", "bdist_wheel"} + + pkgs = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") + assert pkgs["count"] == 1 + row = pkgs["results"][0] + _assert_package_row(row) + assert row["name_normalized"] == "shelf-reader" + assert [item["version"] for item in row["versions"]] == ["0.1"] + + +@pytest.mark.parallel +def test_package_list_version_order( + bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path +): + """Stored versions are newest-first by PEP 440, not lexicographically.""" + repo = python_repo_factory() + name = f"ordered-{uuid.uuid4().hex[:8]}" + for version in ("1.10", "1.9", "1.2"): + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, version) + data = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") + assert data["count"] == 1 + pkg = data["results"][0] + _assert_package_row(pkg) + assert [item["version"] for item in pkg["versions"]] == ["1.10", "1.9", "1.2"] + + +@pytest.mark.parallel +def test_package_list_keeps_stored_versions( + bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path +): + """A rebuild stays its own file, with the version string stored in Pulp.""" + repo = python_repo_factory() + suffix = uuid.uuid4().hex[:8] + name = f"pkg-{suffix}" + + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "2.0.0") + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0.0+test.3") + row = _api_get(bindings_cfg, f"{repo.pulp_href}packages/")["results"][0] + _assert_package_row(row) + assert [item["version"] for item in row["versions"]] == ["2.0.0", "1.0.0+test.3"] + + +@pytest.mark.parallel +def test_package_list_keeps_rebuild_version_string( + bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path +): + """A rebuild is a separate entry because its stored version string differs.""" + repo = python_repo_factory() + name = f"rebuild-{uuid.uuid4().hex[:8]}" + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "5.3.17") + repo = _add_sdist( + python_content_factory, + python_bindings, + tmp_path, + repo, + name, + "5.3.17+test.1.n1", + ) + + pkgs = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") + assert pkgs["count"] == 1 + pkg = pkgs["results"][0] + _assert_package_row(pkg) + assert {item["version"] for item in pkg["versions"]} == {"5.3.17", "5.3.17+test.1.n1"} diff --git a/pulp_python/tests/functional/api/test_crud_publications.py b/pulp_python/tests/functional/api/test_crud_publications.py index e7941ac18..fb65cbd15 100644 --- a/pulp_python/tests/functional/api/test_crud_publications.py +++ b/pulp_python/tests/functional/api/test_crud_publications.py @@ -1,8 +1,10 @@ +import io import random +import tarfile +import zipfile from urllib.parse import urljoin import pytest -from pypi_simple import PyPISimple from pulp_python.tests.functional.constants import ( PYTHON_EGG_FILENAME, @@ -113,27 +115,57 @@ def test_new_content_is_published(python_publication_workflow, python_distributi assert proper is True, msgs +def _write_sdist_with_name(directory, name, version): + """Write a minimal sdist whose PKG-INFO Name differs from PEP 503 canonical form.""" + pkg_dir = f"{name}-{version}" + filename = f"{pkg_dir}.tar.gz" + path = directory / filename + pkg_info = f"Metadata-Version: 2.1\nName: {name}\nVersion: {version}\n".encode() + with tarfile.open(path, "w:gz") as tar: + info = tarfile.TarInfo(name=f"{pkg_dir}/PKG-INFO") + info.size = len(pkg_info) + tar.addfile(info, io.BytesIO(pkg_info)) + return filename, str(path) + + +def _write_wheel_with_name(directory, metadata_name, version, filename): + """Write a minimal wheel whose METADATA Name can differ from the sdist Name.""" + dist_info = f"{filename.split('-')[0]}-{version}.dist-info" + metadata = f"Metadata-Version: 2.1\nName: {metadata_name}\nVersion: {version}\n" + wheel = ( + "Wheel-Version: 1.0\nGenerator: pulp-python-test\nRoot-Is-Purelib: true\n" + "Tag: py2.py3-none-any\n" + ) + path = directory / filename + with zipfile.ZipFile(path, "w") as zf: + zf.writestr(f"{dist_info}/METADATA", metadata) + zf.writestr(f"{dist_info}/WHEEL", wheel) + return filename, str(path) + + @pytest.mark.parallel def test_non_matching_canonicalized_name( - python_repo, python_content_factory, python_publication_factory, python_distribution_factory + python_repo, + python_content_factory, + python_publication_factory, + python_distribution_factory, + tmp_path, ): """Ensures a package with dists that have non-matching canonicalized names is published.""" - packages = [] - filenames = ["msg_parser-1.0.0-py2.py3-none-any.whl", "msg_parser-1.0.0.tar.gz"] - with PyPISimple() as client: - page = client.get_project_page("msg-parser") - for pkg in page.packages: - if pkg.filename in filenames: - c = python_content_factory(pkg.filename, url=pkg.url, repository=python_repo) - if c.filename.endswith(".tar.gz"): - # The metadata name in the SDist is not the same as the Wheel's name - assert c.name == "msg_parser" - else: - assert c.name == "msg-parser" - packages.append(c) + version = "1.0.0" + wheel_filename = "msg_parser-1.0.0-py2.py3-none-any.whl" + sdist_filename, sdist_path = _write_sdist_with_name(tmp_path, "msg_parser", version) + _, wheel_path = _write_wheel_with_name(tmp_path, "msg-parser", version, wheel_filename) + + wheel = python_content_factory(wheel_filename, file=wheel_path, repository=python_repo) + sdist = python_content_factory(sdist_filename, file=sdist_path, repository=python_repo) + # The metadata name in the SDist is not the same as the Wheel's name + assert wheel.name == "msg-parser" + assert sdist.name == "msg_parser" + pub = python_publication_factory(repository=python_repo) distro = python_distribution_factory(publication=pub) url = urljoin(distro.base_url, "simple/") - proper, msgs = ensure_simple(url, {"msg-parser": filenames}) + proper, msgs = ensure_simple(url, {"msg-parser": [wheel_filename, sdist_filename]}) assert proper is True, msgs diff --git a/pulp_python/tests/functional/api/test_domains.py b/pulp_python/tests/functional/api/test_domains.py index 391af15c5..2a2e54865 100644 --- a/pulp_python/tests/functional/api/test_domains.py +++ b/pulp_python/tests/functional/api/test_domains.py @@ -1,9 +1,10 @@ import json import subprocess import uuid -from urllib.parse import urlsplit +from urllib.parse import urljoin, urlsplit import pytest +import requests from pulpcore.app import settings # noqa: TID251 @@ -83,6 +84,37 @@ def test_domain_object_creation( } +def _catalog_api_get(bindings_cfg, path, **params): + url = urljoin(bindings_cfg.host + "/", path.lstrip("/")) + response = requests.get(url, params=params, auth=(bindings_cfg.username, bindings_cfg.password)) + assert response.status_code == 200, response.text + return response.json() + + +@pytest.mark.parallel +def test_domain_catalog_packages(bindings_cfg, domain_factory, python_repo_factory): + """Domain-prefixed repository and repository_version hrefs, including PRN.""" + domain = domain_factory() + repo = python_repo_factory(pulp_domain=domain.name) + assert f"{domain.name}/api/" in repo.pulp_href + assert f"{domain.name}/api/" in repo.latest_version_href + + pkgs = _catalog_api_get(bindings_cfg, f"{repo.pulp_href}packages/") + assert pkgs["count"] == 0 + assert pkgs["results"] == [] + + by_href = _catalog_api_get( + bindings_cfg, f"{repo.pulp_href}packages/", repository_version=repo.latest_version_href + ) + assert by_href["count"] == 0 + version = _catalog_api_get(bindings_cfg, repo.latest_version_href) + by_prn = _catalog_api_get( + bindings_cfg, f"{repo.pulp_href}packages/", repository_version=version["prn"] + ) + assert by_prn["count"] == 0 + assert version["prn"].startswith("prn:core.repositoryversion:") + + @pytest.mark.parallel def test_domain_content_upload( domain_factory, diff --git a/pulp_python/tests/unit/test_catalog.py b/pulp_python/tests/unit/test_catalog.py new file mode 100644 index 000000000..a254853d3 --- /dev/null +++ b/pulp_python/tests/unit/test_catalog.py @@ -0,0 +1,125 @@ +"""Unit tests for the repository package index helpers.""" + +import re +import uuid + +import pytest +from django.db import connection + +from pulpcore.plugin.models import RepositoryContent + +from pulp_python.app.catalog import ( + assemble_package_index, + distinct_package_names_qs, + python_packages_in_version, +) +from pulp_python.app.models import PythonPackageContent, PythonRepository + +_UUID_RE = re.compile(r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}") + +_EMPTY_TEXT = ( + "author", + "author_email", + "description", + "home_page", + "keywords", + "license", + "metadata_version", + "platform", + "summary", + "download_url", + "supported_platform", + "maintainer", + "maintainer_email", + "project_url", + "requires_python", + "description_content_type", + "license_expression", + "python_version", +) + + +def _bound_id_count(sql): + """How many ids the statement binds, whether mogrified or left as ``%s``.""" + lowered = sql.lower() + return max(len(_UUID_RE.findall(lowered)), lowered.count("%s")) + + +def _create_package(name, version): + fields = {field: "" for field in _EMPTY_TEXT} + fields.update( + name=name, + version=version, + filename=f"{name}-{version}.tar.gz", + packagetype="sdist", + sha256=uuid.uuid4().hex + uuid.uuid4().hex, + ) + return PythonPackageContent.objects.create(**fields) + + +@pytest.mark.django_db +def test_package_list_sql_does_not_expand_content_ids(): + """The package index must filter through RepositoryContent, not content_ids. + + A paged list used to inline every content UUID in the version. The name + page is a distinct name_normalized query. File rows for that page are + loaded through the membership subquery. + """ + repository = PythonRepository.objects.create(name=str(uuid.uuid4())) + created = [ + _create_package(name, f"1.0.{number}") + for name in ("alpha", "beta", "gamma", "delta") + for number in range(5) + ] + with repository.new_version() as version: + version.add_content( + PythonPackageContent.objects.filter(pk__in=[item.pk for item in created]) + ) + + repo_version = repository.versions.complete().defer("content_ids").latest() + content_count = RepositoryContent.objects.filter( + repository_id=repository.pk, version_removed__isnull=True + ).count() + content_qs = python_packages_in_version(repo_version) + default_qs = distinct_package_names_qs(content_qs) + count_qs = content_qs.order_by().values("name_normalized").distinct() + + connection.force_debug_cursor = True + start = len(connection.queries) + + def _since(): + nonlocal start + sqls = [query["sql"] for query in connection.queries[start:]] + start = len(connection.queries) + return sqls + + try: + count_qs.count() + count_sqls = _since() + page = list(default_qs[:20]) + default_sqls = _since() + rows = assemble_package_index(content_qs, page, repo_version) + assembled = _since() + finally: + connection.force_debug_cursor = False + + assert rows + assert rows[0]["versions"] + assert content_count == len(created) + membership_sql = [ + sql + for sql in count_sqls + default_sqls + assembled + if "core_repositorycontent" in sql.lower() + ] + assert membership_sql + for sql in membership_sql: + lowered = sql.lower() + assert "content_id" in lowered, sql + assert _bound_id_count(sql) < content_count, sql + assert len(count_sqls) == 1, count_sqls + assert len(default_sqls) == 1, default_sqls + count_sql, default_sql = count_sqls[0], default_sqls[0] + assert "max(" not in count_sql.lower() + assert "max(" not in default_sql.lower() + assert "distinct" in default_sql.lower() + assert "limit" in default_sql.lower() From d96e96cdaeba50618057d57873b38a7e5dd7f96f Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 27 Oct 2026 14:23:54 -0400 Subject: [PATCH 2/7] Move the Python package catalog into a nested read-only viewset Give package listings their own serializer, name filters, ordering, and pagination while retaining the repository packages URL. Resolve snapshots with pulpcore's resource helpers and reject incomplete or unrelated versions. Use the repository-version content API instead of duplicating membership and URI resolution logic, and enforce the parent repository's domain. Add regression coverage for filtering, generated clients, snapshot selection, repository permissions, incomplete versions, and domain isolation. Remove unrelated workspace exclusions and whitespace changes from the original PR. Ref #1358. Generated-By: Codex --- .gitignore | 4 - AGENTS.md | 5 + docs/user/guides/catalog.md | 9 +- pulp_python/app/catalog.py | 118 ++------------- pulp_python/app/utils.py | 1 + pulp_python/app/viewsets.py | 135 +++++++++++------- .../tests/functional/api/test_catalog.py | 92 ++++++++++-- .../tests/functional/api/test_domains.py | 16 ++- pulp_python/tests/unit/test_catalog.py | 40 ++++-- 9 files changed, 237 insertions(+), 183 deletions(-) diff --git a/.gitignore b/.gitignore index 444a89de7..29c94b654 100644 --- a/.gitignore +++ b/.gitignore @@ -8,7 +8,6 @@ __pycache__/ # Distribution / packaging .Python env/ -.venv/ build/ develop-eggs/ dist/ @@ -62,6 +61,3 @@ target/ # PyCharm .idea - -# VS Code / Cursor -.vscode/ diff --git a/AGENTS.md b/AGENTS.md index 2b0005f9e..b93acdfe4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,11 @@ oci-env test --help # run the functional/unit tests oci-env pulpcore-manager # run any pulpcore or Django commands ``` +The package catalog viewset uses repository membership as its registered model. Registering a +second viewset for `PythonPackageContent` prevents pulpcore from resolving content URLs, because +its model-to-viewset lookup requires a unique viewset per model. Nested parent lookups also +need an explicit domain check when `get_queryset()` bypasses the default domain filtering. + ## Running/Writing tests Prefer writing functional tests for new changes/bugfixes and only fallback on unit tests when the change is not easily testable through the API. diff --git a/docs/user/guides/catalog.md b/docs/user/guides/catalog.md index 1632c7a7d..e7b5bcfc3 100644 --- a/docs/user/guides/catalog.md +++ b/docs/user/guides/catalog.md @@ -2,7 +2,7 @@ The package list returns **one row per package name**. It defaults to the repository's latest complete version. Pass `repository_version` (HREF or PRN) -to read an older snapshot. `{pulp_id}` is the repository UUID. +to read an older snapshot. `${REPO_PK}` is the repository UUID. ## List packages @@ -41,4 +41,9 @@ own entry because the stored version differs. Results are ordered by `name_normalized`. Within a package, versions are newest first. Page with `limit` and `offset` to read the whole catalog. Pass -`repository_version` when the export must stay on one snapshot. +`repository_version` when the export must stay on one snapshot. The selected version +must belong to this repository and be complete. + +Filter names with `name_normalized`, `name_normalized__icontains`, or +`name_normalized__istartswith`. Use `ordering=-name_normalized` to list names +in descending order. diff --git a/pulp_python/app/catalog.py b/pulp_python/app/catalog.py index 5ec58b575..a19c3d70a 100644 --- a/pulp_python/app/catalog.py +++ b/pulp_python/app/catalog.py @@ -1,24 +1,17 @@ """Helpers for the repository package list.""" from collections import defaultdict -from urllib.parse import urlparse -from django.core.exceptions import FieldError -from django.core.exceptions import ValidationError as DjangoValidationError -from django.db.models import Case, IntegerField, Q, Value, When -from django.urls import Resolver404, resolve +from django.db.models import Case, IntegerField, Value, When from packaging.version import InvalidVersion, Version -from pulpcore.plugin.models import Repository, RepositoryContent, RepositoryVersion -from pulpcore.plugin.util import resolve_prn - from pulp_python.app.models import PythonPackageContent def _version_sort_key(version): - """PEP 440 sort key. Use with ``reverse=True`` for newest first. + """PEP 440 sort key. Use with `reverse=True` for newest first. - Invalid versions sort after all valid ones when ``reverse=True``. + Invalid versions sort after all valid ones when `reverse=True`. """ if not version: return (-1, "") @@ -28,109 +21,28 @@ def _version_sort_key(version): return (-1, str(version)) -def _in_version_bounds_q(repository_version, prefix=""): - """Version-added/removed bounds for rows present in ``repository_version``. - - ``prefix`` is empty for ``RepositoryContent`` and ``in_repo__`` for the - filtered membership join. Both catalog paths use this helper. - """ - return Q(**{f"{prefix}version_added__number__lte": repository_version.number}) & ( - Q(**{f"{prefix}version_removed__isnull": True}) - | Q(**{f"{prefix}version_removed__number__gt": repository_version.number}) - ) - - -def memberships_in_version(repository_version): - """RepositoryContent rows contained in ``repository_version``. - - A subquery against this queryset keeps the content-id list in the database. - ``RepositoryVersion.content`` inlines ``content_ids`` as one bound UUID per - content unit whenever that array is shorter than 65535. - """ - return RepositoryContent.objects.filter( - repository_id=repository_version.repository_id, - ).filter(_in_version_bounds_q(repository_version)) - - def python_packages_in_version(repository_version): - """Python package content contained in ``repository_version``.""" + """Python package content contained in `repository_version`.""" + content = PythonPackageContent.objects.all() if repository_version is None: - return PythonPackageContent.objects.none() - content_ids = ( - memberships_in_version(repository_version) - .filter(content__pulp_type=PythonPackageContent.get_pulp_type()) - .order_by() - .values("content_id") - ) - return PythonPackageContent.objects.filter(pk__in=content_ids) - - -def resolve_repository_version(href): - """Load a repository version without selecting ``content_ids``. - - Accepts a repository-version or repository HREF/PRN. Domain-prefixed hrefs - include ``pulp_domain`` in the resolved kwargs; that field lives on the - repository, not ``RepositoryVersion``. - """ - if not href: - raise ValueError("No value supplied for repository version.") - - if href.startswith("prn:"): - model, pk = resolve_prn(href) - found_kwargs = {"pk": pk} - else: - try: - match = resolve(urlparse(href).path) - except Resolver404: - raise ValueError(f"URI not valid: {href}") from None - model = match.func.cls.queryset.model - found_kwargs = match.kwargs - - if "pk" in found_kwargs: - lookup = {"pk": found_kwargs["pk"]} - else: - lookup = {} - for key, value in found_kwargs.items(): - if key.endswith("_pk"): - lookup[f"{key[:-3]}__pk"] = value - elif key == "pulp_domain": - if hasattr(model, "pulp_domain"): - lookup["pulp_domain__name"] = value - elif key in ("api_root", "version"): - continue - else: - lookup[key] = value - - qs = model.objects.all() - if issubclass(model, RepositoryVersion): - qs = qs.defer("content_ids") - try: - obj = qs.get(**lookup) - except model.DoesNotExist: - raise ValueError(f"URI {href} not found.") from None - except (model.MultipleObjectsReturned, DjangoValidationError, FieldError): - raise ValueError(f"URI {href} is not a valid repository version.") from None - - if isinstance(obj, RepositoryVersion): - return obj - if isinstance(obj, Repository): - try: - return obj.versions.complete().defer("content_ids").latest() - except RepositoryVersion.DoesNotExist: - return None - raise ValueError("Must be a repository version.") + return content.none() + return repository_version.get_content(content) def distinct_package_names_qs(content_qs): - """One row per distinct ``name_normalized``, ordered for stable pagination.""" - return content_qs.order_by().values("name_normalized").distinct().order_by("name_normalized") + """One row per distinct `name_normalized`, ordered for stable pagination.""" + return ( + content_qs.values("name_normalized") + .distinct() + .order_by(*(content_qs.query.order_by or ("name_normalized",))) + ) def assemble_package_index(content_qs, name_rows, repository_version): """One catalog row per package name, with one content unit per stored version. - Wheel and sdist of the same ``(name_normalized, version)`` collapse to one - row. An sdist is preferred; otherwise the newest ``pulp_created`` is kept. + Wheel and sdist of the same `(name_normalized, version)` collapse to one + row. An sdist is preferred; otherwise the newest `pulp_created` is kept. Version strings are the values stored on the content unit, newest first. Summary, description, author, author email, and license are taken from the newest version because they do not vary by version. diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index e3c05e354..1fd85e3eb 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -28,6 +28,7 @@ log = logging.getLogger(__name__) + PYPI_LAST_SERIAL = "X-PYPI-LAST-SERIAL" """TODO This serial constant is temporary until Python repositories implements serials""" PYPI_SERIAL_CONSTANT = 1000000000 diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index 35f26a648..3cb3d93e9 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -2,7 +2,9 @@ from bandersnatch.configuration import BandersnatchConfig from django.db import transaction -from django_filters import CharFilter +from django.http import Http404 +from django_filters import CharFilter, OrderingFilter +from django_filters.rest_framework import DjangoFilterBackend, FilterSet from django_filters.rest_framework import filters as drf_filters from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view @@ -21,14 +23,14 @@ from pulpcore.plugin import viewsets as core_viewsets from pulpcore.plugin.actions import ModifyRepositoryActionMixin -from pulpcore.plugin.models import RepositoryVersion +from pulpcore.plugin.models import RepositoryContent, RepositoryVersion from pulpcore.plugin.serializers import ( AsyncOperationResponseSerializer, RepositoryAddRemoveContentSerializer, RepositorySyncURLSerializer, ) from pulpcore.plugin.tasking import dispatch -from pulpcore.plugin.util import extract_pk +from pulpcore.plugin.util import extract_pk, get_domain_pk from pulp_python.app import models as python_models from pulp_python.app import serializers as python_serializers @@ -37,7 +39,6 @@ assemble_package_index, distinct_package_names_qs, python_packages_in_version, - resolve_repository_version, ) @@ -71,7 +72,7 @@ class PythonRepositoryViewSet( ], }, { - "action": ["retrieve", "packages"], + "action": ["retrieve"], "principal": "authenticated", "effect": "allow", "condition": "has_model_or_domain_or_obj_perms:python.view_pythonrepository", @@ -145,46 +146,6 @@ class PythonRepositoryViewSet( "python.pythonrepository_viewer": ["python.view_pythonrepository"], } - @property - def filter_backends(self): - """Repository filters apply to the repository list, not the package index. - - ``packages`` is a detail GET that returns a list. Spectacular treats that - as a list operation and would otherwise document ``RepositoryFilter``. - """ - if getattr(self, "action", None) == "packages": - return [] - return super().filter_backends - - def filter_queryset(self, queryset): - """Do not apply the repository FilterSet to package-index query params.""" - if getattr(self, "action", None) == "packages": - return queryset - return super().filter_queryset(queryset) - - def _repository_version_from_href(self, href): - """Load a repository version without selecting ``content_ids``.""" - try: - return resolve_repository_version(href) - except ValueError as exc: - raise ValidationError({"repository_version": str(exc)}) from exc - - def _requested_repository_version(self, repository): - """Resolve optional ``repository_version`` href/PRN, else latest complete version.""" - href = self.request.query_params.get("repository_version") - if not href: - try: - # content_ids is a UUID per content unit. The catalog never reads it. - return repository.versions.complete().defer("content_ids").latest() - except RepositoryVersion.DoesNotExist: - return None - repo_version = self._repository_version_from_href(href) - if repo_version is None: - return None - if repo_version.repository_id != repository.pk: - raise ValidationError({"repository_version": "Must be a version of this repository."}) - return repo_version - @extend_schema( description="Trigger an asynchronous task to create a new repository version.", summary="Modify Repository Content", @@ -294,6 +255,77 @@ def sync(self, request, pk, **kwargs): ) return core_viewsets.OperationPostponedResponse(result, request) + +class PythonRepositoryPackageFilter(FilterSet): + """Filter catalog rows by package name and order distinct names.""" + + ordering = OrderingFilter(fields=("name_normalized",)) + + class Meta: + model = python_models.PythonPackageContent + fields = {"name_normalized": core_viewsets.NAME_FILTER_OPTIONS} + + +class PythonRepositoryPackageFilterBackend(DjangoFilterBackend): + """Apply package filters while the viewset is registered for repository membership.""" + + def get_filterset_class(self, view, queryset=None): + return view.filterset_class + + +class PythonRepositoryPackageViewSet(core_viewsets.NamedModelViewSet, ListModelMixin): + """Read-only package catalog for a Python repository.""" + + endpoint_name = "packages" + parent_viewset = PythonRepositoryViewSet + parent_lookup_kwargs = {"repository_pk": "repository__pk"} + serializer_class = python_serializers.PythonRepositoryPackageSerializer + # Catalog rows describe repository membership, not individual content resources. + queryset = RepositoryContent.objects.none() + filterset_class = PythonRepositoryPackageFilter + filter_backends = (PythonRepositoryPackageFilterBackend,) + + DEFAULT_ACCESS_POLICY = { + "statements": [ + { + "action": ["list"], + "principal": "authenticated", + "effect": "allow", + "condition": "has_repository_model_or_domain_or_obj_perms:python.view_pythonrepository", # noqa: E501 + }, + ], + } + + def get_parent_field_and_object(self): + """Require the parent repository to belong to the current domain.""" + field, repository = super().get_parent_field_and_object() + if repository.pulp_domain_id != get_domain_pk(): + raise Http404 + return field, repository + + def _requested_repository_version(self, repository): + """Resolve a complete snapshot belonging to the parent repository.""" + href = self.request.query_params.get("repository_version") + if not href: + return repository.latest_version() + try: + repo_version = self.get_resource(href, RepositoryVersion) + except ValidationError as exc: + raise ValidationError({"repository_version": exc.detail}) from exc + if repo_version.repository_id != repository.pk: + raise ValidationError({"repository_version": "Must be a version of this repository."}) + if not repo_version.complete: + raise ValidationError({"repository_version": "Must be a complete repository version."}) + return repo_version + + def get_queryset(self): + """Select package content through the requested repository snapshot.""" + if getattr(self, "swagger_fake_view", False): + return python_models.PythonPackageContent.objects.none() + repository = self.get_parent_object() + self.repository_version = self._requested_repository_version(repository) + return python_packages_in_version(self.repository_version) + @extend_schema( summary="List packages", description=( @@ -318,22 +350,15 @@ def sync(self, request, pk, **kwargs): ], responses={200: python_serializers.PythonRepositoryPackageSerializer(many=True)}, ) - @action( - detail=True, - methods=["get"], - serializer_class=python_serializers.PythonRepositoryPackageSerializer, - ) - def packages(self, request, pk, **kwargs): + def list(self, request, *args, **kwargs): """List distinct packages in a repository version.""" - repository = self.get_object() - repo_version = self._requested_repository_version(repository) - content_qs = python_packages_in_version(repo_version) + content_qs = self.filter_queryset(self.get_queryset()) names_qs = distinct_package_names_qs(content_qs) page = self.paginate_queryset(names_qs) rows = assemble_package_index( content_qs, page if page is not None else list(names_qs), - repo_version, + self.repository_version, ) serializer = self.get_serializer(rows, many=True) if page is not None: diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py index 6f5d167f7..4c6c9f53e 100644 --- a/pulp_python/tests/functional/api/test_catalog.py +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -1,7 +1,4 @@ -"""Catalog API tests. - -Generated client methods are unavailable until `oci-env generate-client` is rerun. -""" +"""Catalog API tests.""" import io import tarfile @@ -97,11 +94,11 @@ def test_package_list_grouping_and_pagination(bindings_cfg, sm_repo): @pytest.mark.parallel -def test_package_list_empty_repository(bindings_cfg, python_repo_factory): +def test_package_list_empty_repository(python_bindings, python_repo_factory): repo = python_repo_factory() - data = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") - assert data["count"] == 0 - assert data["results"] == [] + data = python_bindings.RepositoriesPythonPackagesApi.list(repo.pulp_href) + assert data.count == 0 + assert data.results == [] @pytest.mark.parallel @@ -181,10 +178,15 @@ def test_package_list_keeps_stored_versions( name = f"pkg-{suffix}" repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "2.0.0") + original_version = repo.latest_version_href repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0.0+test.3") row = _api_get(bindings_cfg, f"{repo.pulp_href}packages/")["results"][0] _assert_package_row(row) assert [item["version"] for item in row["versions"]] == ["2.0.0", "1.0.0+test.3"] + historical = _api_get( + bindings_cfg, f"{repo.pulp_href}packages/", repository_version=original_version + )["results"][0] + assert [item["version"] for item in historical["versions"]] == ["2.0.0"] @pytest.mark.parallel @@ -209,3 +211,77 @@ def test_package_list_keeps_rebuild_version_string( pkg = pkgs["results"][0] _assert_package_row(pkg) assert {item["version"] for item in pkg["versions"]} == {"5.3.17", "5.3.17+test.1.n1"} + + +@pytest.mark.parallel +def test_package_list_filters_and_ordering(bindings_cfg, sm_repo): + """Name filters and ordering apply to packages within the selected snapshot.""" + path = f"{sm_repo.pulp_href}packages/" + for params in ( + {"name_normalized": "django"}, + {"name_normalized__istartswith": "DJAN"}, + {"name_normalized__icontains": "JANG"}, + {"name_normalized__in": "django,absent"}, + ): + data = _api_get(bindings_cfg, path, **params) + assert data["count"] == 1 + assert [row["name_normalized"] for row in data["results"]] == ["django"] + data = _api_get(bindings_cfg, path, ordering="-name_normalized", limit=2) + assert data["count"] == 3 + assert [row["name_normalized"] for row in data["results"]] == ["django", "celery"] + data = _api_get(bindings_cfg, path, ordering="-name_normalized", limit=2, offset=2) + assert [row["name_normalized"] for row in data["results"]] == ["aiohttp"] + + +@pytest.mark.parallel +@pytest.mark.parametrize("selector", ["invalid", "repository", "missing", "wrong_type_prn"]) +def test_package_list_invalid_version(bindings_cfg, python_repo_factory, selector): + """Malformed, missing, and non-version resources return a validation error.""" + repo = python_repo_factory() + selectors = { + "invalid": "/not-a-pulp-resource/", + "repository": repo.pulp_href, + "missing": f"{repo.pulp_href}versions/999/", + "wrong_type_prn": repo.prn, + } + response = requests.get( + urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), + params={"repository_version": selectors[selector]}, + auth=(bindings_cfg.username, bindings_cfg.password), + ) + assert response.status_code == 400, response.text + assert "repository_version" in response.json() + + +@pytest.mark.parallel +def test_package_list_permissions(bindings_cfg, python_repo_factory, gen_user): + """Catalog access requires view permission on the parent repository.""" + repo = python_repo_factory() + other = python_repo_factory() + user = gen_user() + viewer = gen_user(object_roles=[("python.pythonrepository_viewer", repo.pulp_href)]) + model_viewer = gen_user(model_roles=["python.pythonrepository_viewer"]) + for actor, repository, status in ( + (user, repo, 403), + (viewer, repo, 200), + (viewer, other, 403), + (model_viewer, other, 200), + ): + response = requests.get( + urljoin(bindings_cfg.host, f"{repository.pulp_href}packages/"), + auth=(actor.username, actor.password), + ) + assert response.status_code == status, response.text + + response = requests.post( + urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), + json={}, + auth=(bindings_cfg.username, bindings_cfg.password), + ) + assert response.status_code == 405, response.text + + response = requests.get( + urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), + auth=("", ""), + ) + assert response.status_code == 401, response.text diff --git a/pulp_python/tests/functional/api/test_domains.py b/pulp_python/tests/functional/api/test_domains.py index 2a2e54865..070692479 100644 --- a/pulp_python/tests/functional/api/test_domains.py +++ b/pulp_python/tests/functional/api/test_domains.py @@ -92,7 +92,7 @@ def _catalog_api_get(bindings_cfg, path, **params): @pytest.mark.parallel -def test_domain_catalog_packages(bindings_cfg, domain_factory, python_repo_factory): +def test_domain_catalog_packages(bindings_cfg, domain_factory, python_repo_factory, gen_user): """Domain-prefixed repository and repository_version hrefs, including PRN.""" domain = domain_factory() repo = python_repo_factory(pulp_domain=domain.name) @@ -114,6 +114,20 @@ def test_domain_catalog_packages(bindings_cfg, domain_factory, python_repo_facto assert by_prn["count"] == 0 assert version["prn"].startswith("prn:core.repositoryversion:") + default_path = f"{repo.pulp_href}packages/".replace(f"/{domain.name}/api/", "/default/api/") + response = requests.get( + urljoin(bindings_cfg.host, default_path), + auth=(bindings_cfg.username, bindings_cfg.password), + ) + assert response.status_code == 404, response.text + + viewer = gen_user(domain_roles=[("python.pythonrepository_viewer", domain.pulp_href)]) + response = requests.get( + urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), + auth=(viewer.username, viewer.password), + ) + assert response.status_code == 200, response.text + @pytest.mark.parallel def test_domain_content_upload( diff --git a/pulp_python/tests/unit/test_catalog.py b/pulp_python/tests/unit/test_catalog.py index a254853d3..16eec2659 100644 --- a/pulp_python/tests/unit/test_catalog.py +++ b/pulp_python/tests/unit/test_catalog.py @@ -5,8 +5,9 @@ import pytest from django.db import connection +from rest_framework.test import APIRequestFactory, force_authenticate -from pulpcore.plugin.models import RepositoryContent +from pulpcore.plugin.models import RepositoryContent, RepositoryVersion from pulp_python.app.catalog import ( assemble_package_index, @@ -14,6 +15,7 @@ python_packages_in_version, ) from pulp_python.app.models import PythonPackageContent, PythonRepository +from pulp_python.app.viewsets import PythonRepositoryPackageViewSet _UUID_RE = re.compile(r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}") @@ -40,7 +42,7 @@ def _bound_id_count(sql): - """How many ids the statement binds, whether mogrified or left as ``%s``.""" + """How many ids the statement binds, whether mogrified or left as `%s`.""" lowered = sql.lower() return max(len(_UUID_RE.findall(lowered)), lowered.count("%s")) @@ -59,12 +61,7 @@ def _create_package(name, version): @pytest.mark.django_db def test_package_list_sql_does_not_expand_content_ids(): - """The package index must filter through RepositoryContent, not content_ids. - - A paged list used to inline every content UUID in the version. The name - page is a distinct name_normalized query. File rows for that page are - loaded through the membership subquery. - """ + """Keep snapshot content IDs in the database while paginating package names.""" repository = PythonRepository.objects.create(name=str(uuid.uuid4())) created = [ _create_package(name, f"1.0.{number}") @@ -76,7 +73,7 @@ def test_package_list_sql_does_not_expand_content_ids(): PythonPackageContent.objects.filter(pk__in=[item.pk for item in created]) ) - repo_version = repository.versions.complete().defer("content_ids").latest() + repo_version = repository.latest_version() content_count = RepositoryContent.objects.filter( repository_id=repository.pk, version_removed__isnull=True ).count() @@ -109,7 +106,7 @@ def _since(): membership_sql = [ sql for sql in count_sqls + default_sqls + assembled - if "core_repositorycontent" in sql.lower() + if "core_repositoryversion" in sql.lower() ] assert membership_sql for sql in membership_sql: @@ -123,3 +120,26 @@ def _since(): assert "max(" not in default_sql.lower() assert "distinct" in default_sql.lower() assert "limit" in default_sql.lower() + + +@pytest.mark.django_db +@pytest.mark.parametrize("explicit", [True, False]) +def test_package_list_incomplete_version(django_user_model, explicit): + """Reject an explicit incomplete snapshot and ignore it when selecting the latest.""" + repository = PythonRepository.objects.create(name=str(uuid.uuid4())) + incomplete = RepositoryVersion.objects.create(repository=repository, number=1, complete=False) + user = django_user_model.objects.create(username=str(uuid.uuid4()), is_superuser=True) + params = ( + {"repository_version": f"prn:core.repositoryversion:{incomplete.pk}"} if explicit else {} + ) + request = APIRequestFactory().get("/", params) + force_authenticate(request, user=user) + response = PythonRepositoryPackageViewSet.as_view({"get": "list"})( + request, repository_pk=str(repository.pk) + ) + assert response.status_code == (400 if explicit else 200) + if explicit: + assert "complete" in str(response.data["repository_version"]) + else: + assert response.data["count"] == 0 + assert response.data["results"] == [] From 1f36c54ae8c7fd9413b8beb9704e33b38a14e000 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 27 Oct 2026 14:48:26 -0400 Subject: [PATCH 3/7] Fix catalog tests across supported pulpcore versions Check distinct-name pagination, returned version metadata, and deferred column loading without assuming how pulpcore selects snapshot content. This fixes the lowerbounds failure on pulpcore 3.105.25 while keeping coverage for the catalog's query behavior. Defer unused large metadata fields and load descriptions only for each package's newest version. Remove the added AGENTS.md guidance as requested. Ref #1358. Generated-By: Codex --- AGENTS.md | 5 - docs/user/guides/catalog.md | 5 +- pulp_python/app/__init__.py | 18 +- pulp_python/app/catalog.py | 144 ++++++++-------- pulp_python/app/serializers.py | 12 ++ pulp_python/app/urls.py | 15 ++ pulp_python/app/viewsets.py | 143 ++++++---------- .../tests/functional/api/test_catalog.py | 38 ++++- pulp_python/tests/unit/test_catalog.py | 161 +++++++++++------- 9 files changed, 305 insertions(+), 236 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b93acdfe4..2b0005f9e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,11 +30,6 @@ oci-env test --help # run the functional/unit tests oci-env pulpcore-manager # run any pulpcore or Django commands ``` -The package catalog viewset uses repository membership as its registered model. Registering a -second viewset for `PythonPackageContent` prevents pulpcore from resolving content URLs, because -its model-to-viewset lookup requires a unique viewset per model. Nested parent lookups also -need an explicit domain check when `get_queryset()` bypasses the default domain filtering. - ## Running/Writing tests Prefer writing functional tests for new changes/bugfixes and only fallback on unit tests when the change is not easily testable through the API. diff --git a/docs/user/guides/catalog.md b/docs/user/guides/catalog.md index e7b5bcfc3..3904751e2 100644 --- a/docs/user/guides/catalog.md +++ b/docs/user/guides/catalog.md @@ -12,8 +12,9 @@ http GET "${BASE_ADDR}/pulp/api/v3/repositories/python/python/${REPO_PK}/package `count` is the number of distinct packages, not files. Each row is one package name. `summary`, `description`, `author`, `author_email`, and `license` come -from the newest version. `versions` has one entry per stored version string, -newest first, with that version's `license_expression`. A wheel and an sdist +from the highest version in PEP 440 order. For each version, an sdist supplies +the metadata when available; otherwise the most recently created content unit +is used. `versions` has one entry per stored version string, newest first, with that version's `license_expression`. A wheel and an sdist of the same version are one entry. A rebuild such as `1.0.0+test.1` stays its own entry because the stored version differs. diff --git a/pulp_python/app/__init__.py b/pulp_python/app/__init__.py index 3bca50680..508e3a6fd 100644 --- a/pulp_python/app/__init__.py +++ b/pulp_python/app/__init__.py @@ -17,19 +17,20 @@ class PulpPythonPluginAppConfig(PulpPluginAppConfig): domain_compatible = True def ready(self): - """Register PyPI access policy hook.""" + """Register access policies for explicitly routed endpoints.""" super().ready() post_migrate.connect( - _populate_pypi_access_policies, + _populate_extra_access_policies, sender=self, dispatch_uid="populate_pypi_access_policies_identifier", ) # TODO: Remove this when https://github.com/pulp/pulpcore/issues/5500 is resolved -def _populate_pypi_access_policies(sender, apps, verbosity, **kwargs): +def _populate_extra_access_policies(sender, apps, verbosity, **kwargs): from pulp_python.app.pypi.feeds import FeedView from pulp_python.app.pypi.views import MetadataView, PyPIView, SimpleView, UploadView + from pulp_python.app.viewsets import PythonRepositoryPackageViewSet try: AccessPolicy = apps.get_model("core", "AccessPolicy") @@ -38,10 +39,19 @@ def _populate_pypi_access_policies(sender, apps, verbosity, **kwargs): print(_("AccessPolicy model does not exist. Skipping initialization.")) return - for viewset in (PyPIView, SimpleView, UploadView, MetadataView, FeedView): + for viewset in ( + PyPIView, + SimpleView, + UploadView, + MetadataView, + FeedView, + PythonRepositoryPackageViewSet, + ): access_policy = getattr(viewset, "DEFAULT_ACCESS_POLICY", None) if access_policy is not None: viewset_name = viewset.urlpattern() + if parent := getattr(viewset, "parent_viewset", None): + viewset_name = f"{parent.urlpattern()}/{viewset_name}" db_access_policy, created = AccessPolicy.objects.get_or_create( viewset_name=viewset_name, defaults=access_policy ) diff --git a/pulp_python/app/catalog.py b/pulp_python/app/catalog.py index a19c3d70a..09a8c4a54 100644 --- a/pulp_python/app/catalog.py +++ b/pulp_python/app/catalog.py @@ -1,98 +1,104 @@ """Helpers for the repository package list.""" -from collections import defaultdict - -from django.db.models import Case, IntegerField, Value, When +from django import VERSION as DJANGO_VERSION +from django.contrib.postgres.aggregates import JSONBAgg +from django.db.models import Case, Func, IntegerField, JSONField, Value, When from packaging.version import InvalidVersion, Version from pulp_python.app.models import PythonPackageContent def _version_sort_key(version): - """PEP 440 sort key. Use with `reverse=True` for newest first. - - Invalid versions sort after all valid ones when `reverse=True`. - """ - if not version: - return (-1, "") + """Keep legacy invalid versions browsable, after valid PEP 440 versions.""" try: - return (0, Version(version)) + return (True, Version(version)) except InvalidVersion: - return (-1, str(version)) - + return (False, version) -def python_packages_in_version(repository_version): - """Python package content contained in `repository_version`.""" - content = PythonPackageContent.objects.all() - if repository_version is None: - return content.none() - return repository_version.get_content(content) +class PackageVersions(JSONBAgg): + """Map versions to [content ID, license expression], keeping the last file.""" -def distinct_package_names_qs(content_qs): - """One row per distinct `name_normalized`, ordered for stable pagination.""" - return ( - content_qs.values("name_normalized") - .distinct() - .order_by(*(content_qs.query.order_by or ("name_normalized",))) - ) + function = "JSONB_OBJECT_AGG" + arity = 2 -def assemble_package_index(content_qs, name_rows, repository_version): - """One catalog row per package name, with one content unit per stored version. +def package_catalog_queryset(content, ordering=("name_normalized",)): + """Group package names and load their representative versions in the same query. Wheel and sdist of the same `(name_normalized, version)` collapse to one row. An sdist is preferred; otherwise the newest `pulp_created` is kept. - Version strings are the values stored on the content unit, newest first. - Summary, description, author, author email, and license are taken from the - newest version because they do not vary by version. """ - if not name_rows or repository_version is None: - return [] - - names = [row["name_normalized"] for row in name_rows] - by_name = defaultdict(list) - # DISTINCT ON keeps the first row for each name and stored version. - units = ( - content_qs.filter(name_normalized__in=names) - .order_by( - "name_normalized", + # The aggregate keeps the last file for each stored version. + file_ordering = { + "order_by" if DJANGO_VERSION >= (5, 2) else "ordering": ( "version", Case( - When(packagetype="sdist", then=Value(0)), - default=Value(1), + When(packagetype="sdist", then=Value(1)), + default=Value(0), output_field=IntegerField(), ), - "-pulp_created", - "pk", + "pulp_created", + "-pk", + ) + } + return ( + content.order_by(*ordering) + .values("name_normalized") + .annotate( + versions=PackageVersions( + "version", + Func( + "pk", + "license_expression", + function="jsonb_build_array", + output_field=JSONField(), + ), + **file_ordering, + ) ) - .distinct("name_normalized", "version") ) - for unit in units: - by_name[unit.name_normalized].append(unit) - result = [] - for row in name_rows: - normalized = row["name_normalized"] - versions = sorted( - by_name.get(normalized, []), - key=lambda unit: (_version_sort_key(unit.version), unit.filename), + +def assemble_package_index(package_rows): + """Sort the page's versions and bulk-load metadata from their highest versions.""" + if not package_rows: + return [] + + for row in package_rows: + row["versions"] = sorted( + ( + {"version": version, "pk": unit[0], "license_expression": unit[1]} + for version, unit in row["versions"].items() + ), + key=lambda unit: (_version_sort_key(unit["version"]), unit["version"]), reverse=True, ) - source = versions[0] if versions else None - result.append( - { - "name": source.name if source else normalized, - "name_normalized": normalized, - "summary": source.summary if source else "", - "description": source.description if source else "", - "author": source.author if source else "", - "author_email": source.author_email if source else "", - "license": source.license if source else "", - "versions": [ - {"version": unit.version, "license_expression": unit.license_expression} - for unit in versions - ], - } + + fields = ( + "name", + "name_normalized", + "summary", + "description", + "author", + "author_email", + "license", + ) + metadata = { + str(unit["pk"]): {field: unit[field] for field in fields} + for unit in PythonPackageContent.objects.filter( + pk__in=[row["versions"][0]["pk"] for row in package_rows] ) - return result + .order_by() + .values("pk", *fields) + } + return [ + { + **metadata[row["versions"][0]["pk"]], + "versions": [ + {"version": unit["version"], "license_expression": unit["license_expression"]} + for unit in row["versions"] + ], + } + for row in package_rows + ] diff --git a/pulp_python/app/serializers.py b/pulp_python/app/serializers.py index f7fd7ee6d..8920de24b 100644 --- a/pulp_python/app/serializers.py +++ b/pulp_python/app/serializers.py @@ -19,6 +19,7 @@ from pulpcore.plugin.util import get_current_authenticated_user, get_domain, get_prn, reverse from pulp_python.app import models as python_models +from pulp_python.app.catalog import assemble_package_index from pulp_python.app.provenance import ( AnyPublisher, Attestation, @@ -651,6 +652,14 @@ class PythonRepositoryPackageVersionSerializer(serializers.Serializer): ) +class PythonRepositoryPackageListSerializer(serializers.ListSerializer): + """Assemble metadata only for the names selected by pagination.""" + + def to_representation(self, data): + rows = assemble_package_index(list(data)) + return super().to_representation(rows) + + class PythonRepositoryPackageSerializer(serializers.Serializer): """One distinct package name and its stored versions.""" @@ -691,6 +700,9 @@ class PythonRepositoryPackageSerializer(serializers.Serializer): ), ) + class Meta: + list_serializer_class = PythonRepositoryPackageListSerializer + class PackageProvenanceSerializer(core_serializers.NoArtifactContentUploadSerializer): """ diff --git a/pulp_python/app/urls.py b/pulp_python/app/urls.py index bf62db314..0554c7dfd 100644 --- a/pulp_python/app/urls.py +++ b/pulp_python/app/urls.py @@ -1,6 +1,8 @@ from django.conf import settings from django.urls import path +from pulpcore.plugin.find_url import find_api_root + from pulp_python.app.pypi.feeds import FeedView from pulp_python.app.pypi.views import ( MetadataView, @@ -10,6 +12,7 @@ UploadView, YankView, ) +from pulp_python.app.viewsets import PythonRepositoryPackageViewSet if settings.DOMAIN_ENABLED: PYPI_API_URL = "///" @@ -61,3 +64,15 @@ ), path(PYPI_API_URL, PyPIView.as_view({"get": "retrieve"}), name="pypi-detail"), ] + + +_, api_root = find_api_root( + lstrip=True, version="" if getattr(settings, "ENABLE_V4_API", False) else "v3" +) +urlpatterns.append( + path( + f"{api_root}repositories/python/python//packages/", + PythonRepositoryPackageViewSet.as_view({"get": "list"}), + name="python-repository-packages-list", + ) +) diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index 3cb3d93e9..d8128425d 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -2,12 +2,12 @@ from bandersnatch.configuration import BandersnatchConfig from django.db import transaction -from django.http import Http404 +from django.shortcuts import get_object_or_404 from django_filters import CharFilter, OrderingFilter -from django_filters.rest_framework import DjangoFilterBackend, FilterSet +from django_filters.rest_framework import FilterSet from django_filters.rest_framework import filters as drf_filters from drf_spectacular.types import OpenApiTypes -from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view +from drf_spectacular.utils import extend_schema, extend_schema_field, extend_schema_view from packaging.specifiers import InvalidSpecifier, SpecifierSet from packaging.utils import canonicalize_name from rest_framework import status @@ -20,10 +20,11 @@ ) from rest_framework.response import Response from rest_framework.serializers import ValidationError +from rest_framework.viewsets import GenericViewSet from pulpcore.plugin import viewsets as core_viewsets from pulpcore.plugin.actions import ModifyRepositoryActionMixin -from pulpcore.plugin.models import RepositoryContent, RepositoryVersion +from pulpcore.plugin.models import RepositoryVersion from pulpcore.plugin.serializers import ( AsyncOperationResponseSerializer, RepositoryAddRemoveContentSerializer, @@ -35,11 +36,7 @@ from pulp_python.app import models as python_models from pulp_python.app import serializers as python_serializers from pulp_python.app import tasks -from pulp_python.app.catalog import ( - assemble_package_index, - distinct_package_names_qs, - python_packages_in_version, -) +from pulp_python.app.catalog import package_catalog_queryset class PythonRepositoryViewSet( @@ -257,33 +254,53 @@ def sync(self, request, pk, **kwargs): class PythonRepositoryPackageFilter(FilterSet): - """Filter catalog rows by package name and order distinct names.""" + """Select a repository snapshot, filter names, and order packages.""" + repository_version = CharFilter( + method="filter_repository_version", + help_text="HREF or PRN of a complete version of this repository. Defaults to the latest.", + ) ordering = OrderingFilter(fields=("name_normalized",)) class Meta: model = python_models.PythonPackageContent fields = {"name_normalized": core_viewsets.NAME_FILTER_OPTIONS} - -class PythonRepositoryPackageFilterBackend(DjangoFilterBackend): - """Apply package filters while the viewset is registered for repository membership.""" - - def get_filterset_class(self, view, queryset=None): - return view.filterset_class + @extend_schema_field(OpenApiTypes.URI) + def filter_repository_version(self, queryset, name, value): + view = self.request.parser_context["view"] + repository = view.repository + if value: + try: + version = core_viewsets.NamedModelViewSet.get_resource(value, RepositoryVersion) + except ValidationError as exc: + raise ValidationError({name: exc.detail}) from exc + if version.repository_id != repository.pk: + raise ValidationError({name: "Must be a version of this repository."}) + if not version.complete: + raise ValidationError({name: "Must be a complete repository version."}) + else: + version = repository.latest_version() + return version.get_content(queryset) if version is not None else queryset.none() + + def filter_queryset(self, queryset): + # Method filters skip empty values, but an omitted snapshot selects the latest. + if not self.form.cleaned_data["repository_version"]: + queryset = self.filter_repository_version(queryset, "repository_version", None) + return package_catalog_queryset( + super().filter_queryset(queryset), + ordering=self.form.cleaned_data["ordering"] or ("name_normalized",), + ) -class PythonRepositoryPackageViewSet(core_viewsets.NamedModelViewSet, ListModelMixin): - """Read-only package catalog for a Python repository.""" +@extend_schema_view(list=extend_schema(summary="List packages")) +class PythonRepositoryPackageViewSet(GenericViewSet, ListModelMixin): + """Read-only package catalog, registered explicitly without a model resource.""" - endpoint_name = "packages" - parent_viewset = PythonRepositoryViewSet - parent_lookup_kwargs = {"repository_pk": "repository__pk"} + queryset = python_models.PythonPackageContent.objects.none() serializer_class = python_serializers.PythonRepositoryPackageSerializer - # Catalog rows describe repository membership, not individual content resources. - queryset = RepositoryContent.objects.none() filterset_class = PythonRepositoryPackageFilter - filter_backends = (PythonRepositoryPackageFilterBackend,) + parent_viewset = PythonRepositoryViewSet DEFAULT_ACCESS_POLICY = { "statements": [ @@ -296,74 +313,24 @@ class PythonRepositoryPackageViewSet(core_viewsets.NamedModelViewSet, ListModelM ], } - def get_parent_field_and_object(self): - """Require the parent repository to belong to the current domain.""" - field, repository = super().get_parent_field_and_object() - if repository.pulp_domain_id != get_domain_pk(): - raise Http404 - return field, repository - - def _requested_repository_version(self, repository): - """Resolve a complete snapshot belonging to the parent repository.""" - href = self.request.query_params.get("repository_version") - if not href: - return repository.latest_version() - try: - repo_version = self.get_resource(href, RepositoryVersion) - except ValidationError as exc: - raise ValidationError({"repository_version": exc.detail}) from exc - if repo_version.repository_id != repository.pk: - raise ValidationError({"repository_version": "Must be a version of this repository."}) - if not repo_version.complete: - raise ValidationError({"repository_version": "Must be a complete repository version."}) - return repo_version + @classmethod + def endpoint_pieces(cls): + return ["packages"] + + @classmethod + def urlpattern(cls): + return "packages" def get_queryset(self): - """Select package content through the requested repository snapshot.""" + """Validate the parent repository's domain before selecting catalog content.""" if getattr(self, "swagger_fake_view", False): - return python_models.PythonPackageContent.objects.none() - repository = self.get_parent_object() - self.repository_version = self._requested_repository_version(repository) - return python_packages_in_version(self.repository_version) - - @extend_schema( - summary="List packages", - description=( - "Return one row per distinct package name in a repository version " - "(latest complete version if repository_version is omitted). " - "Each versions entry is the stored version and its license_expression " - "(wheel and sdist of the same version are a single entry). Results are " - "ordered by name_normalized. Pagination count is the number of " - "distinct packages, not files." - ), - parameters=[ - OpenApiParameter( - name="repository_version", - type=OpenApiTypes.URI, - location=OpenApiParameter.QUERY, - required=False, - description=( - "HREF or PRN of a version of this repository. " - "Defaults to the latest complete version." - ), - ), - ], - responses={200: python_serializers.PythonRepositoryPackageSerializer(many=True)}, - ) - def list(self, request, *args, **kwargs): - """List distinct packages in a repository version.""" - content_qs = self.filter_queryset(self.get_queryset()) - names_qs = distinct_package_names_qs(content_qs) - page = self.paginate_queryset(names_qs) - rows = assemble_package_index( - content_qs, - page if page is not None else list(names_qs), - self.repository_version, + return self.queryset + self.repository = get_object_or_404( + python_models.PythonRepository, + pk=self.kwargs["repository_pk"], + pulp_domain_id=get_domain_pk(), ) - serializer = self.get_serializer(rows, many=True) - if page is not None: - return self.get_paginated_response(serializer.data) - return Response(serializer.data) + return python_models.PythonPackageContent.objects.all() class PythonBlocklistEntryViewSet( diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py index 4c6c9f53e..f012cf3cf 100644 --- a/pulp_python/tests/functional/api/test_catalog.py +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -42,12 +42,15 @@ def _assert_package_row(pkg): assert item["version"] -def _write_sdist(directory, name, version): +def _write_sdist(directory, name, version, license_expression=""): """Write a minimal sdist whose PKG-INFO Name/Version pkginfo can read.""" pkg_dir = f"{name}-{version}" filename = f"{pkg_dir}.tar.gz" path = directory / filename - pkg_info = f"Metadata-Version: 2.1\nName: {name}\nVersion: {version}\n".encode() + pkg_info = ( + f"Metadata-Version: 2.4\nName: {name}\nVersion: {version}\n" + f"License-Expression: {license_expression}\n\n{name} {version} description\n" + ).encode() with tarfile.open(path, "w:gz") as tar: info = tarfile.TarInfo(name=f"{pkg_dir}/PKG-INFO") info.size = len(pkg_info) @@ -55,8 +58,10 @@ def _write_sdist(directory, name, version): return filename, str(path) -def _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, version): - filename, path = _write_sdist(tmp_path, name, version) +def _add_sdist( + python_content_factory, python_bindings, tmp_path, repo, name, version, license_expression="" +): + filename, path = _write_sdist(tmp_path, name, version, license_expression) python_content_factory(relative_path=filename, file=path, repository=repo) return python_bindings.RepositoriesPythonApi.read(repo.pulp_href) @@ -168,6 +173,31 @@ def test_package_list_version_order( assert [item["version"] for item in pkg["versions"]] == ["1.10", "1.9", "1.2"] +@pytest.mark.parallel +def test_package_list_version_metadata( + python_bindings, python_content_factory, python_repo_factory, tmp_path +): + """Metadata follows version precedence and remains scoped to the selected snapshot.""" + repo = python_repo_factory() + name = f"metadata-{uuid.uuid4().hex[:8]}" + repo = _add_sdist( + python_content_factory, python_bindings, tmp_path, repo, name, "2.0", "Apache-2.0" + ) + historical_href = repo.latest_version_href + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0", "MIT") + packages = python_bindings.RepositoriesPythonPackagesApi + latest = packages.list(repo.pulp_href).results[0] + assert latest.description.strip() == f"{name} 2.0 description" + assert [(item.version, item.license_expression) for item in latest.versions] == [ + ("2.0", "Apache-2.0"), + ("1.0", "MIT"), + ] + historical = packages.list(repo.pulp_href, repository_version=historical_href).results[0] + assert [(item.version, item.license_expression) for item in historical.versions] == [ + ("2.0", "Apache-2.0"), + ] + + @pytest.mark.parallel def test_package_list_keeps_stored_versions( bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path diff --git a/pulp_python/tests/unit/test_catalog.py b/pulp_python/tests/unit/test_catalog.py index 16eec2659..502a82c88 100644 --- a/pulp_python/tests/unit/test_catalog.py +++ b/pulp_python/tests/unit/test_catalog.py @@ -1,23 +1,21 @@ """Unit tests for the repository package index helpers.""" -import re import uuid import pytest +from django.apps import apps from django.db import connection +from django.test.utils import CaptureQueriesContext from rest_framework.test import APIRequestFactory, force_authenticate -from pulpcore.plugin.models import RepositoryContent, RepositoryVersion +from pulpcore.plugin.models import RepositoryVersion -from pulp_python.app.catalog import ( - assemble_package_index, - distinct_package_names_qs, - python_packages_in_version, -) +from pulp_python.app.catalog import assemble_package_index, package_catalog_queryset from pulp_python.app.models import PythonPackageContent, PythonRepository -from pulp_python.app.viewsets import PythonRepositoryPackageViewSet - -_UUID_RE = re.compile(r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}") +from pulp_python.app.viewsets import ( + PythonPackageSingleArtifactContentUploadViewSet, + PythonRepositoryPackageViewSet, +) _EMPTY_TEXT = ( "author", @@ -41,13 +39,7 @@ ) -def _bound_id_count(sql): - """How many ids the statement binds, whether mogrified or left as `%s`.""" - lowered = sql.lower() - return max(len(_UUID_RE.findall(lowered)), lowered.count("%s")) - - -def _create_package(name, version): +def _create_package(name, version, **overrides): fields = {field: "" for field in _EMPTY_TEXT} fields.update( name=name, @@ -55,13 +47,16 @@ def _create_package(name, version): filename=f"{name}-{version}.tar.gz", packagetype="sdist", sha256=uuid.uuid4().hex + uuid.uuid4().hex, + description=f"{name} {version} description", ) + fields.update(overrides) return PythonPackageContent.objects.create(**fields) @pytest.mark.django_db -def test_package_list_sql_does_not_expand_content_ids(): - """Keep snapshot content IDs in the database while paginating package names.""" +@pytest.mark.parametrize("page_size", [1, 2, 4]) +def test_package_list_pagination_and_bulk_metadata(django_assert_num_queries, page_size): + """Page distinct names and load large metadata only for the newest version.""" repository = PythonRepository.objects.create(name=str(uuid.uuid4())) created = [ _create_package(name, f"1.0.{number}") @@ -74,52 +69,64 @@ def test_package_list_sql_does_not_expand_content_ids(): ) repo_version = repository.latest_version() - content_count = RepositoryContent.objects.filter( - repository_id=repository.pk, version_removed__isnull=True - ).count() - content_qs = python_packages_in_version(repo_version) - default_qs = distinct_package_names_qs(content_qs) - count_qs = content_qs.order_by().values("name_normalized").distinct() - - connection.force_debug_cursor = True - start = len(connection.queries) - - def _since(): - nonlocal start - sqls = [query["sql"] for query in connection.queries[start:]] - start = len(connection.queries) - return sqls - - try: - count_qs.count() - count_sqls = _since() - page = list(default_qs[:20]) - default_sqls = _since() - rows = assemble_package_index(content_qs, page, repo_version) - assembled = _since() - finally: - connection.force_debug_cursor = False - - assert rows - assert rows[0]["versions"] - assert content_count == len(created) - membership_sql = [ - sql - for sql in count_sqls + default_sqls + assembled - if "core_repositoryversion" in sql.lower() + content_qs = repo_version.get_content(PythonPackageContent.objects.all()) + packages = package_catalog_queryset(content_qs) + + with django_assert_num_queries(1): + assert packages.count() == 4 + with django_assert_num_queries(1), CaptureQueriesContext(connection) as page_queries: + page = list(packages[:page_size]) + with django_assert_num_queries(1), CaptureQueriesContext(connection) as queries: + rows = assemble_package_index(page) + + assert [row["name_normalized"] for row in rows] == ["alpha", "beta", "delta", "gamma"][ + :page_size + ] + for row in rows: + assert [item["version"] for item in row["versions"]] == [ + f"1.0.{number}" for number in reversed(range(5)) + ] + assert row["description"] == f"{row['name_normalized']} 1.0.4 description" + + for field in ("description", "classifiers", "requires_dist", "project_urls", "license_file"): + assert f'"{field}"' not in page_queries[0]["sql"] + description_queries = [ + query for query in queries if '"description"' in query["sql"].split(" FROM ", 1)[0] ] - assert membership_sql - for sql in membership_sql: - lowered = sql.lower() - assert "content_id" in lowered, sql - assert _bound_id_count(sql) < content_count, sql - assert len(count_sqls) == 1, count_sqls - assert len(default_sqls) == 1, default_sqls - count_sql, default_sql = count_sqls[0], default_sqls[0] - assert "max(" not in count_sql.lower() - assert "max(" not in default_sql.lower() - assert "distinct" in default_sql.lower() - assert "limit" in default_sql.lower() + assert len(description_queries) == 1 + + +@pytest.mark.django_db +@pytest.mark.parametrize("has_sdist", [True, False]) +def test_package_list_representative_file(has_sdist, django_assert_num_queries): + """Each version uses an sdist if available, otherwise its newest wheel.""" + repository = PythonRepository.objects.create(name=str(uuid.uuid4())) + units = [] + if has_sdist: + units.append(_create_package("alpha", "1.0", license_expression="MIT")) + for platform in ("linux", "macos"): + units.append( + _create_package( + "alpha", + "1.0", + filename=f"alpha-1.0-{platform}.whl", + packagetype="bdist_wheel", + license_expression="Apache-2.0", + description=f"{platform} wheel description", + ) + ) + with repository.new_version() as version: + version.add_content(PythonPackageContent.objects.filter(pk__in=[unit.pk for unit in units])) + content = repository.latest_version().get_content(PythonPackageContent.objects.all()) + + with django_assert_num_queries(2): + rows = assemble_package_index(list(package_catalog_queryset(content))) + assert rows[0]["versions"] == [ + {"version": "1.0", "license_expression": "MIT" if has_sdist else "Apache-2.0"} + ] + assert rows[0]["description"] == ( + "alpha 1.0 description" if has_sdist else "macos wheel description" + ) @pytest.mark.django_db @@ -143,3 +150,29 @@ def test_package_list_incomplete_version(django_user_model, explicit): else: assert response.data["count"] == 0 assert response.data["results"] == [] + + +def test_package_content_viewset_registration(): + """The catalog must not compete with the package content resource's viewset.""" + assert apps.get_app_config("python").named_viewsets[PythonPackageContent] == [ + PythonPackageSingleArtifactContentUploadViewSet + ] + + +@pytest.mark.django_db +def test_package_list_without_pagination(django_user_model): + """The standard list action also assembles rows when pagination is disabled.""" + repository = PythonRepository.objects.create(name=str(uuid.uuid4())) + package = _create_package("alpha", "1.0") + with repository.new_version() as version: + version.add_content(PythonPackageContent.objects.filter(pk=package.pk)) + user = django_user_model.objects.create(username=str(uuid.uuid4()), is_superuser=True) + request = APIRequestFactory().get("/") + force_authenticate(request, user=user) + response = PythonRepositoryPackageViewSet.as_view({"get": "list"}, pagination_class=None)( + request, repository_pk=str(repository.pk) + ) + assert response.status_code == 200 + assert len(response.data) == 1 + assert response.data[0]["description"] == "alpha 1.0 description" + assert response.data[0]["versions"] == [{"version": "1.0", "license_expression": ""}] From 3737d4bf2edd9cf1f272987e78b82607e26dbf62 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Wed, 11 Nov 2026 00:37:22 -0500 Subject: [PATCH 4/7] Share the Python package catalog across repository snapshots Expose packages through a shared read-only action on Python repositories and repository versions. Select snapshots from resource URLs and inherit the parent viewsets' permission checks instead of maintaining a separate endpoint. Return last_updated for each stored package version instead of license_expression. Compute update times across the snapshot's files independently from representative metadata, retaining bulk metadata loading. Update documentation and coverage for both URLs, generated clients, snapshot timestamps, schema, permissions, and domain isolation. Ref #1358. Generated-By: Codex --- CHANGES/1358.feature | 2 +- docs/user/guides/catalog.md | 32 ++-- pulp_python/app/__init__.py | 4 - pulp_python/app/catalog.py | 39 ++-- pulp_python/app/serializers.py | 9 +- pulp_python/app/urls.py | 15 -- pulp_python/app/viewsets.py | 169 ++++++++--------- .../tests/functional/api/test_catalog.py | 173 ++++++++++-------- .../tests/functional/api/test_domains.py | 46 ++--- pulp_python/tests/unit/test_catalog.py | 73 +++++--- 10 files changed, 291 insertions(+), 271 deletions(-) diff --git a/CHANGES/1358.feature b/CHANGES/1358.feature index 5cb68bb99..2d8c205a2 100644 --- a/CHANGES/1358.feature +++ b/CHANGES/1358.feature @@ -1 +1 @@ -Added a repository endpoint that lists Python packages with limit/offset pagination. +Added repository and repository-version endpoints that list Python packages and version update times with limit/offset pagination. diff --git a/docs/user/guides/catalog.md b/docs/user/guides/catalog.md index 3904751e2..07d6d8666 100644 --- a/docs/user/guides/catalog.md +++ b/docs/user/guides/catalog.md @@ -1,8 +1,8 @@ # Browse the package catalog -The package list returns **one row per package name**. It defaults to the -repository's latest complete version. Pass `repository_version` (HREF or PRN) -to read an older snapshot. `${REPO_PK}` is the repository UUID. +The package list returns **one row per package name**. Use the repository URL +to read its latest complete version, or a repository-version URL to read a +specific complete snapshot. `${REPO_PK}` is the repository UUID. ## List packages @@ -10,13 +10,21 @@ to read an older snapshot. `${REPO_PK}` is the repository UUID. http GET "${BASE_ADDR}/pulp/api/v3/repositories/python/python/${REPO_PK}/packages/?limit=10" ``` +For a specific snapshot, `${VERSION_NUMBER}` is its repository version number: + +```bash +http GET "${BASE_ADDR}/pulp/api/v3/repositories/python/python/${REPO_PK}/versions/${VERSION_NUMBER}/packages/?limit=10" +``` + `count` is the number of distinct packages, not files. Each row is one package name. `summary`, `description`, `author`, `author_email`, and `license` come -from the highest version in PEP 440 order. For each version, an sdist supplies -the metadata when available; otherwise the most recently created content unit -is used. `versions` has one entry per stored version string, newest first, with that version's `license_expression`. A wheel and an sdist -of the same version are one entry. A rebuild such as `1.0.0+test.1` stays its -own entry because the stored version differs. +from the highest version in PEP 440 order. An sdist supplies the metadata when +available; otherwise the most recently created content unit is used. +`versions` has one entry per stored version string, newest first, with a +`last_updated` timestamp for each version. This is the latest content creation +time among that version's files in the selected snapshot, not the upstream +release date. A wheel and an sdist of the same version are one entry. A rebuild +such as `1.0.0+test.1` stays its own entry because the stored version differs. ```json { @@ -33,7 +41,7 @@ own entry because the stored version differs. "author_email": "", "license": "", "versions": [ - {"version": "0.1", "license_expression": ""} + {"version": "0.1", "last_updated": "2026-11-11T12:00:00Z"} ] } ] @@ -41,9 +49,9 @@ own entry because the stored version differs. ``` Results are ordered by `name_normalized`. Within a package, versions are newest -first. Page with `limit` and `offset` to read the whole catalog. Pass -`repository_version` when the export must stay on one snapshot. The selected version -must belong to this repository and be complete. +first. Page with `limit` and `offset` to read the whole catalog. Use the +repository-version URL when the export must stay on one snapshot. Missing or +incomplete snapshots return HTTP 404. Filter names with `name_normalized`, `name_normalized__icontains`, or `name_normalized__istartswith`. Use `ordering=-name_normalized` to list names diff --git a/pulp_python/app/__init__.py b/pulp_python/app/__init__.py index 508e3a6fd..bb822f0ab 100644 --- a/pulp_python/app/__init__.py +++ b/pulp_python/app/__init__.py @@ -30,7 +30,6 @@ def ready(self): def _populate_extra_access_policies(sender, apps, verbosity, **kwargs): from pulp_python.app.pypi.feeds import FeedView from pulp_python.app.pypi.views import MetadataView, PyPIView, SimpleView, UploadView - from pulp_python.app.viewsets import PythonRepositoryPackageViewSet try: AccessPolicy = apps.get_model("core", "AccessPolicy") @@ -45,13 +44,10 @@ def _populate_extra_access_policies(sender, apps, verbosity, **kwargs): UploadView, MetadataView, FeedView, - PythonRepositoryPackageViewSet, ): access_policy = getattr(viewset, "DEFAULT_ACCESS_POLICY", None) if access_policy is not None: viewset_name = viewset.urlpattern() - if parent := getattr(viewset, "parent_viewset", None): - viewset_name = f"{parent.urlpattern()}/{viewset_name}" db_access_policy, created = AccessPolicy.objects.get_or_create( viewset_name=viewset_name, defaults=access_policy ) diff --git a/pulp_python/app/catalog.py b/pulp_python/app/catalog.py index 09a8c4a54..5cfe4138d 100644 --- a/pulp_python/app/catalog.py +++ b/pulp_python/app/catalog.py @@ -2,7 +2,7 @@ from django import VERSION as DJANGO_VERSION from django.contrib.postgres.aggregates import JSONBAgg -from django.db.models import Case, Func, IntegerField, JSONField, Value, When +from django.db.models import Case, IntegerField, Value, When from packaging.version import InvalidVersion, Version from pulp_python.app.models import PythonPackageContent @@ -17,7 +17,7 @@ def _version_sort_key(version): class PackageVersions(JSONBAgg): - """Map versions to [content ID, license expression], keeping the last file.""" + """Map versions to values, keeping the last file.""" function = "JSONB_OBJECT_AGG" arity = 2 @@ -27,11 +27,13 @@ def package_catalog_queryset(content, ordering=("name_normalized",)): """Group package names and load their representative versions in the same query. Wheel and sdist of the same `(name_normalized, version)` collapse to one - row. An sdist is preferred; otherwise the newest `pulp_created` is kept. + row. An sdist supplies metadata when available, while the newest file's + `pulp_created` supplies the update time. """ - # The aggregate keeps the last file for each stored version. + # Each aggregate keeps the last file for each stored version. + ordering_argument = "order_by" if DJANGO_VERSION >= (5, 2) else "ordering" file_ordering = { - "order_by" if DJANGO_VERSION >= (5, 2) else "ordering": ( + ordering_argument: ( "version", Case( When(packagetype="sdist", then=Value(1)), @@ -48,14 +50,14 @@ def package_catalog_queryset(content, ordering=("name_normalized",)): .annotate( versions=PackageVersions( "version", - Func( - "pk", - "license_expression", - function="jsonb_build_array", - output_field=JSONField(), - ), + "pulp_created", + **{ordering_argument: ("pulp_created",)}, + ), + metadata_ids=PackageVersions( + "version", + "pk", **file_ordering, - ) + ), ) ) @@ -68,8 +70,8 @@ def assemble_package_index(package_rows): for row in package_rows: row["versions"] = sorted( ( - {"version": version, "pk": unit[0], "license_expression": unit[1]} - for version, unit in row["versions"].items() + {"version": version, "last_updated": updated} + for version, updated in row["versions"].items() ), key=lambda unit: (_version_sort_key(unit["version"]), unit["version"]), reverse=True, @@ -87,18 +89,15 @@ def assemble_package_index(package_rows): metadata = { str(unit["pk"]): {field: unit[field] for field in fields} for unit in PythonPackageContent.objects.filter( - pk__in=[row["versions"][0]["pk"] for row in package_rows] + pk__in=[row["metadata_ids"][row["versions"][0]["version"]] for row in package_rows] ) .order_by() .values("pk", *fields) } return [ { - **metadata[row["versions"][0]["pk"]], - "versions": [ - {"version": unit["version"], "license_expression": unit["license_expression"]} - for unit in row["versions"] - ], + **metadata[row["metadata_ids"][row["versions"][0]["version"]]], + "versions": row["versions"], } for row in package_rows ] diff --git a/pulp_python/app/serializers.py b/pulp_python/app/serializers.py index 8920de24b..71dcdd839 100644 --- a/pulp_python/app/serializers.py +++ b/pulp_python/app/serializers.py @@ -646,9 +646,8 @@ class PythonRepositoryPackageVersionSerializer(serializers.Serializer): """One stored version of a package.""" version = serializers.CharField(help_text=_("The package's version number.")) - license_expression = serializers.CharField( - allow_blank=True, - help_text=_("Text string that is a valid SPDX license expression."), + last_updated = serializers.DateTimeField( + help_text=_("The latest content creation time among this version's files in the snapshot."), ) @@ -695,8 +694,8 @@ class PythonRepositoryPackageSerializer(serializers.Serializer): versions = PythonRepositoryPackageVersionSerializer( many=True, help_text=_( - "One entry per stored version, newest first. A wheel and an sdist of the " - "same version are one entry. Version strings are unchanged." + "One entry per stored version with its update time, newest version first. " + "A wheel and an sdist of the same version are one entry. Version strings are unchanged." ), ) diff --git a/pulp_python/app/urls.py b/pulp_python/app/urls.py index 0554c7dfd..bf62db314 100644 --- a/pulp_python/app/urls.py +++ b/pulp_python/app/urls.py @@ -1,8 +1,6 @@ from django.conf import settings from django.urls import path -from pulpcore.plugin.find_url import find_api_root - from pulp_python.app.pypi.feeds import FeedView from pulp_python.app.pypi.views import ( MetadataView, @@ -12,7 +10,6 @@ UploadView, YankView, ) -from pulp_python.app.viewsets import PythonRepositoryPackageViewSet if settings.DOMAIN_ENABLED: PYPI_API_URL = "///" @@ -64,15 +61,3 @@ ), path(PYPI_API_URL, PyPIView.as_view({"get": "retrieve"}), name="pypi-detail"), ] - - -_, api_root = find_api_root( - lstrip=True, version="" if getattr(settings, "ENABLE_V4_API", False) else "v3" -) -urlpatterns.append( - path( - f"{api_root}repositories/python/python//packages/", - PythonRepositoryPackageViewSet.as_view({"get": "list"}), - name="python-repository-packages-list", - ) -) diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index d8128425d..e483499d5 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -2,12 +2,11 @@ from bandersnatch.configuration import BandersnatchConfig from django.db import transaction -from django.shortcuts import get_object_or_404 +from django.http import Http404 from django_filters import CharFilter, OrderingFilter -from django_filters.rest_framework import FilterSet +from django_filters.rest_framework import DjangoFilterBackend, FilterSet from django_filters.rest_framework import filters as drf_filters -from drf_spectacular.types import OpenApiTypes -from drf_spectacular.utils import extend_schema, extend_schema_field, extend_schema_view +from drf_spectacular.utils import extend_schema, extend_schema_view from packaging.specifiers import InvalidSpecifier, SpecifierSet from packaging.utils import canonicalize_name from rest_framework import status @@ -20,7 +19,6 @@ ) from rest_framework.response import Response from rest_framework.serializers import ValidationError -from rest_framework.viewsets import GenericViewSet from pulpcore.plugin import viewsets as core_viewsets from pulpcore.plugin.actions import ModifyRepositoryActionMixin @@ -39,8 +37,79 @@ from pulp_python.app.catalog import package_catalog_queryset +class PythonRepositoryPackageFilter(FilterSet): + """Filter names and order packages in a repository snapshot.""" + + ordering = OrderingFilter(fields=("name_normalized",)) + + class Meta: + model = python_models.PythonPackageContent + fields = {"name_normalized": core_viewsets.NAME_FILTER_OPTIONS} + + def filter_queryset(self, queryset): + return package_catalog_queryset( + super().filter_queryset(queryset), + ordering=self.form.cleaned_data["ordering"] or ("name_normalized",), + ) + + +class PythonRepositoryPackageFilterBackend(DjangoFilterBackend): + """Apply content filters without filtering the parent resource lookup.""" + + def get_filterset_class(self, view, queryset=None): + return view.filterset_class + + def filter_queryset(self, request, queryset, view): + # Package filters apply to content, not the parent resource lookup. + if queryset.model is not python_models.PythonPackageContent: + return queryset + return super().filter_queryset(request, queryset, view) + + +class PythonRepositoryPackagesMixin: + """Browse packages on a repository or a complete repository version.""" + + @extend_schema( + summary="List packages", + description=( + "Return one row per distinct package name in this repository version, " + "or the latest complete version when called on a repository. " + "Pagination counts package names, not distribution files." + ), + responses={200: python_serializers.PythonRepositoryPackageSerializer(many=True)}, + filters=True, + ) + @action( + detail=True, + methods=["get"], + serializer_class=python_serializers.PythonRepositoryPackageSerializer, + filterset_class=PythonRepositoryPackageFilter, + filter_backends=(PythonRepositoryPackageFilterBackend,), + ) + def packages(self, request, **kwargs): + """List packages from the snapshot identified by the resource URL.""" + resource = self.get_object() + if isinstance(resource, RepositoryVersion): + if resource.repository.pulp_domain_id != get_domain_pk(): + raise Http404 + version = resource + else: + version = resource.latest_version() + content = python_models.PythonPackageContent.objects.all() + content = version.get_content(content) if version is not None else content.none() + queryset = self.filter_queryset(content) + page = self.paginate_queryset(queryset) + serializer = self.get_serializer(page if page is not None else queryset, many=True) + if page is not None: + return self.get_paginated_response(serializer.data) + return Response(serializer.data) + + class PythonRepositoryViewSet( - core_viewsets.RepositoryViewSet, ModifyRepositoryActionMixin, core_viewsets.RolesMixin + PythonRepositoryPackagesMixin, + core_viewsets.RepositoryViewSet, + ModifyRepositoryActionMixin, + core_viewsets.RolesMixin, ): """ PythonRepository represents a single Python repository, to which content can be @@ -69,7 +138,7 @@ class PythonRepositoryViewSet( ], }, { - "action": ["retrieve"], + "action": ["retrieve", "packages"], "principal": "authenticated", "effect": "allow", "condition": "has_model_or_domain_or_obj_perms:python.view_pythonrepository", @@ -253,86 +322,6 @@ def sync(self, request, pk, **kwargs): return core_viewsets.OperationPostponedResponse(result, request) -class PythonRepositoryPackageFilter(FilterSet): - """Select a repository snapshot, filter names, and order packages.""" - - repository_version = CharFilter( - method="filter_repository_version", - help_text="HREF or PRN of a complete version of this repository. Defaults to the latest.", - ) - ordering = OrderingFilter(fields=("name_normalized",)) - - class Meta: - model = python_models.PythonPackageContent - fields = {"name_normalized": core_viewsets.NAME_FILTER_OPTIONS} - - @extend_schema_field(OpenApiTypes.URI) - def filter_repository_version(self, queryset, name, value): - view = self.request.parser_context["view"] - repository = view.repository - if value: - try: - version = core_viewsets.NamedModelViewSet.get_resource(value, RepositoryVersion) - except ValidationError as exc: - raise ValidationError({name: exc.detail}) from exc - if version.repository_id != repository.pk: - raise ValidationError({name: "Must be a version of this repository."}) - if not version.complete: - raise ValidationError({name: "Must be a complete repository version."}) - else: - version = repository.latest_version() - return version.get_content(queryset) if version is not None else queryset.none() - - def filter_queryset(self, queryset): - # Method filters skip empty values, but an omitted snapshot selects the latest. - if not self.form.cleaned_data["repository_version"]: - queryset = self.filter_repository_version(queryset, "repository_version", None) - return package_catalog_queryset( - super().filter_queryset(queryset), - ordering=self.form.cleaned_data["ordering"] or ("name_normalized",), - ) - - -@extend_schema_view(list=extend_schema(summary="List packages")) -class PythonRepositoryPackageViewSet(GenericViewSet, ListModelMixin): - """Read-only package catalog, registered explicitly without a model resource.""" - - queryset = python_models.PythonPackageContent.objects.none() - serializer_class = python_serializers.PythonRepositoryPackageSerializer - filterset_class = PythonRepositoryPackageFilter - parent_viewset = PythonRepositoryViewSet - - DEFAULT_ACCESS_POLICY = { - "statements": [ - { - "action": ["list"], - "principal": "authenticated", - "effect": "allow", - "condition": "has_repository_model_or_domain_or_obj_perms:python.view_pythonrepository", # noqa: E501 - }, - ], - } - - @classmethod - def endpoint_pieces(cls): - return ["packages"] - - @classmethod - def urlpattern(cls): - return "packages" - - def get_queryset(self): - """Validate the parent repository's domain before selecting catalog content.""" - if getattr(self, "swagger_fake_view", False): - return self.queryset - self.repository = get_object_or_404( - python_models.PythonRepository, - pk=self.kwargs["repository_pk"], - pulp_domain_id=get_domain_pk(), - ) - return python_models.PythonPackageContent.objects.all() - - class PythonBlocklistEntryViewSet( core_viewsets.NamedModelViewSet, CreateModelMixin, @@ -388,7 +377,9 @@ def get_serializer_context(self): return context -class PythonRepositoryVersionViewSet(core_viewsets.RepositoryVersionViewSet): +class PythonRepositoryVersionViewSet( + PythonRepositoryPackagesMixin, core_viewsets.RepositoryVersionViewSet +): """ PythonRepositoryVersion represents a single Python repository version. """ @@ -398,7 +389,7 @@ class PythonRepositoryVersionViewSet(core_viewsets.RepositoryVersionViewSet): DEFAULT_ACCESS_POLICY = { "statements": [ { - "action": ["list", "retrieve"], + "action": ["list", "retrieve", "packages"], "principal": "authenticated", "effect": "allow", "condition": "has_repository_model_or_domain_or_obj_perms:python.view_pythonrepository", # noqa: E501 diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py index f012cf3cf..488819b19 100644 --- a/pulp_python/tests/functional/api/test_catalog.py +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -3,6 +3,7 @@ import io import tarfile import uuid +from datetime import datetime from urllib.parse import urljoin import pytest @@ -38,18 +39,18 @@ def _assert_package_row(pkg): assert versions assert len(versions) == len(set(versions)) for item in pkg["versions"]: - assert set(item) == {"version", "license_expression"} + assert set(item) == {"version", "last_updated"} assert item["version"] + assert datetime.fromisoformat(item["last_updated"]).tzinfo is not None -def _write_sdist(directory, name, version, license_expression=""): +def _write_sdist(directory, name, version): """Write a minimal sdist whose PKG-INFO Name/Version pkginfo can read.""" pkg_dir = f"{name}-{version}" filename = f"{pkg_dir}.tar.gz" path = directory / filename pkg_info = ( - f"Metadata-Version: 2.4\nName: {name}\nVersion: {version}\n" - f"License-Expression: {license_expression}\n\n{name} {version} description\n" + f"Metadata-Version: 2.4\nName: {name}\nVersion: {version}\n\n{name} {version} description\n" ).encode() with tarfile.open(path, "w:gz") as tar: info = tarfile.TarInfo(name=f"{pkg_dir}/PKG-INFO") @@ -58,10 +59,8 @@ def _write_sdist(directory, name, version, license_expression=""): return filename, str(path) -def _add_sdist( - python_content_factory, python_bindings, tmp_path, repo, name, version, license_expression="" -): - filename, path = _write_sdist(tmp_path, name, version, license_expression) +def _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, version): + filename, path = _write_sdist(tmp_path, name, version) python_content_factory(relative_path=filename, file=path, repository=repo) return python_bindings.RepositoriesPythonApi.read(repo.pulp_href) @@ -72,19 +71,53 @@ def sm_repo(python_repo_with_sync, python_remote_factory): return python_repo_with_sync(remote) +@pytest.fixture(params=[False, True], ids=["repository", "version"]) +def catalog_path(request): + def path(repo): + resource = repo.latest_version_href if request.param else repo.pulp_href + return f"{resource}packages/" + + return path + + +def test_package_list_schema(pulp_openapi_schema_pk_path_set): + """Both actions document package filters and paginated package responses.""" + schema = pulp_openapi_schema_pk_path_set + operations = [ + methods["get"] + for path, methods in schema["paths"].items() + if "/repositories/python/python/" in path and path.endswith("/packages/") + ] + assert len(operations) == 2 + for operation in operations: + query_params = {item["name"] for item in operation["parameters"] if item["in"] == "query"} + assert { + "name_normalized", + "name_normalized__istartswith", + "ordering", + "limit", + "offset", + } <= query_params + assert "repository_version" not in query_params + response = operation["responses"]["200"]["content"]["application/json"]["schema"] + component = response["$ref"].rsplit("/", 1)[-1] + assert "results" in schema["components"]["schemas"][component]["properties"] + + @pytest.mark.parallel -def test_package_list_grouping_and_pagination(bindings_cfg, sm_repo): +def test_package_list_grouping_and_pagination(bindings_cfg, sm_repo, catalog_path): """Package index is one row per name, and count is distinct packages not files.""" - data = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", limit=1) + path = catalog_path(sm_repo) + data = _api_get(bindings_cfg, path, limit=1) assert data["count"] == 3 assert len(data["results"]) == 1 _assert_package_row(data["results"][0]) - page2 = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", limit=1, offset=1) + page2 = _api_get(bindings_cfg, path, limit=1, offset=1) assert page2["count"] == 3 assert page2["results"][0]["name_normalized"] != data["results"][0]["name_normalized"] - all_rows = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", limit=100)["results"] + all_rows = _api_get(bindings_cfg, path, limit=100)["results"] names = [pkg["name_normalized"] for pkg in all_rows] assert names == sorted(names) assert set(names) == {"aiohttp", "celery", "django"} @@ -101,49 +134,39 @@ def test_package_list_grouping_and_pagination(bindings_cfg, sm_repo): @pytest.mark.parallel def test_package_list_empty_repository(python_bindings, python_repo_factory): repo = python_repo_factory() - data = python_bindings.RepositoriesPythonPackagesApi.list(repo.pulp_href) - assert data.count == 0 - assert data.results == [] + for api, href in ( + (python_bindings.RepositoriesPythonApi, repo.pulp_href), + (python_bindings.RepositoriesPythonVersionsApi, repo.latest_version_href), + ): + data = api.packages(href) + assert data.count == 0 + assert data.results == [] @pytest.mark.parallel def test_packages_repository_version(bindings_cfg, sm_repo, python_repo_factory): - """repository_version selects a snapshot; omitted uses the latest complete version.""" + """Version URLs select snapshots independently of the repository's latest version.""" latest_href = sm_repo.latest_version_href v0_href = f"{sm_repo.pulp_href}versions/0/" default_pkgs = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/") - explicit_pkgs = _api_get( - bindings_cfg, f"{sm_repo.pulp_href}packages/", repository_version=latest_href - ) - assert default_pkgs["count"] == explicit_pkgs["count"] == 3 - - version = _api_get(bindings_cfg, latest_href) - prn_pkgs = _api_get( - bindings_cfg, f"{sm_repo.pulp_href}packages/", repository_version=version["prn"] - ) - assert prn_pkgs["count"] == 3 - assert version["prn"].startswith("prn:core.repositoryversion:") + explicit_pkgs = _api_get(bindings_cfg, f"{latest_href}packages/") + assert default_pkgs == explicit_pkgs - v0_pkgs = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/", repository_version=v0_href) + v0_pkgs = _api_get(bindings_cfg, f"{v0_href}packages/") assert v0_pkgs["count"] == 0 assert v0_pkgs["results"] == [] other = python_repo_factory() - url = urljoin(bindings_cfg.host + "/", f"{sm_repo.pulp_href}packages/".lstrip("/")) - response = requests.get( - url, - params={"repository_version": other.latest_version_href}, - auth=(bindings_cfg.username, bindings_cfg.password), - ) - assert response.status_code == 400, response.text + other_pkgs = _api_get(bindings_cfg, f"{other.latest_version_href}packages/") + assert other_pkgs["results"] == [] @pytest.mark.parallel def test_package_list_one_entry_per_stored_version( - bindings_cfg, python_bindings, python_repo_with_sync + bindings_cfg, python_bindings, python_repo_with_sync, monitor_task ): - """A wheel and an sdist of the same version are one version string.""" + """Update times include all files in the selected snapshot.""" repo = python_repo_with_sync() files = python_bindings.ContentPackagesApi.list(repository_version=repo.latest_version_href) assert files.count == 2 @@ -155,6 +178,21 @@ def test_package_list_one_entry_per_stored_version( _assert_package_row(row) assert row["name_normalized"] == "shelf-reader" assert [item["version"] for item in row["versions"]] == ["0.1"] + assert datetime.fromisoformat(row["versions"][0]["last_updated"]) == max( + item.pulp_created for item in files.results + ) + + latest_file = max(files.results, key=lambda item: item.pulp_created) + remaining = next(item for item in files.results if item.pulp_href != latest_file.pulp_href) + monitor_task( + python_bindings.RepositoriesPythonApi.modify( + repo.pulp_href, {"remove_content_units": [latest_file.pulp_href]} + ).task + ) + current = _api_get(bindings_cfg, f"{repo.pulp_href}packages/")["results"][0] + assert datetime.fromisoformat(current["versions"][0]["last_updated"]) == remaining.pulp_created + historical = _api_get(bindings_cfg, f"{repo.latest_version_href}packages/")["results"][0] + assert historical == row @pytest.mark.parallel @@ -180,21 +218,20 @@ def test_package_list_version_metadata( """Metadata follows version precedence and remains scoped to the selected snapshot.""" repo = python_repo_factory() name = f"metadata-{uuid.uuid4().hex[:8]}" - repo = _add_sdist( - python_content_factory, python_bindings, tmp_path, repo, name, "2.0", "Apache-2.0" - ) + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "2.0") historical_href = repo.latest_version_href - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0", "MIT") - packages = python_bindings.RepositoriesPythonPackagesApi - latest = packages.list(repo.pulp_href).results[0] + repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0") + files = python_bindings.ContentPackagesApi.list(repository_version=repo.latest_version_href) + updated = {item.version: item.pulp_created for item in files.results} + latest = python_bindings.RepositoriesPythonApi.packages(repo.pulp_href).results[0] assert latest.description.strip() == f"{name} 2.0 description" - assert [(item.version, item.license_expression) for item in latest.versions] == [ - ("2.0", "Apache-2.0"), - ("1.0", "MIT"), + assert [(item.version, item.last_updated) for item in latest.versions] == [ + ("2.0", updated["2.0"]), + ("1.0", updated["1.0"]), ] - historical = packages.list(repo.pulp_href, repository_version=historical_href).results[0] - assert [(item.version, item.license_expression) for item in historical.versions] == [ - ("2.0", "Apache-2.0"), + historical = python_bindings.RepositoriesPythonVersionsApi.packages(historical_href).results[0] + assert [(item.version, item.last_updated) for item in historical.versions] == [ + ("2.0", updated["2.0"]), ] @@ -213,9 +250,7 @@ def test_package_list_keeps_stored_versions( row = _api_get(bindings_cfg, f"{repo.pulp_href}packages/")["results"][0] _assert_package_row(row) assert [item["version"] for item in row["versions"]] == ["2.0.0", "1.0.0+test.3"] - historical = _api_get( - bindings_cfg, f"{repo.pulp_href}packages/", repository_version=original_version - )["results"][0] + historical = _api_get(bindings_cfg, f"{original_version}packages/")["results"][0] assert [item["version"] for item in historical["versions"]] == ["2.0.0"] @@ -244,9 +279,9 @@ def test_package_list_keeps_rebuild_version_string( @pytest.mark.parallel -def test_package_list_filters_and_ordering(bindings_cfg, sm_repo): +def test_package_list_filters_and_ordering(bindings_cfg, sm_repo, catalog_path): """Name filters and ordering apply to packages within the selected snapshot.""" - path = f"{sm_repo.pulp_href}packages/" + path = catalog_path(sm_repo) for params in ( {"name_normalized": "django"}, {"name_normalized__istartswith": "DJAN"}, @@ -264,54 +299,46 @@ def test_package_list_filters_and_ordering(bindings_cfg, sm_repo): @pytest.mark.parallel -@pytest.mark.parametrize("selector", ["invalid", "repository", "missing", "wrong_type_prn"]) -def test_package_list_invalid_version(bindings_cfg, python_repo_factory, selector): - """Malformed, missing, and non-version resources return a validation error.""" +def test_package_list_missing_version(bindings_cfg, python_repo_factory): + """A missing repository snapshot returns 404.""" repo = python_repo_factory() - selectors = { - "invalid": "/not-a-pulp-resource/", - "repository": repo.pulp_href, - "missing": f"{repo.pulp_href}versions/999/", - "wrong_type_prn": repo.prn, - } response = requests.get( - urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), - params={"repository_version": selectors[selector]}, + urljoin(bindings_cfg.host, f"{repo.pulp_href}versions/999/packages/"), auth=(bindings_cfg.username, bindings_cfg.password), ) - assert response.status_code == 400, response.text - assert "repository_version" in response.json() + assert response.status_code == 404, response.text @pytest.mark.parallel -def test_package_list_permissions(bindings_cfg, python_repo_factory, gen_user): +def test_package_list_permissions(bindings_cfg, python_repo_factory, gen_user, catalog_path): """Catalog access requires view permission on the parent repository.""" repo = python_repo_factory() other = python_repo_factory() user = gen_user() viewer = gen_user(object_roles=[("python.pythonrepository_viewer", repo.pulp_href)]) model_viewer = gen_user(model_roles=["python.pythonrepository_viewer"]) + denied_status = 403 if "/versions/" in catalog_path(repo) else 404 for actor, repository, status in ( - (user, repo, 403), + (user, repo, denied_status), (viewer, repo, 200), - (viewer, other, 403), + (viewer, other, denied_status), (model_viewer, other, 200), ): response = requests.get( - urljoin(bindings_cfg.host, f"{repository.pulp_href}packages/"), + urljoin(bindings_cfg.host, catalog_path(repository)), auth=(actor.username, actor.password), ) assert response.status_code == status, response.text response = requests.post( - urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), + urljoin(bindings_cfg.host, catalog_path(repo)), json={}, auth=(bindings_cfg.username, bindings_cfg.password), ) assert response.status_code == 405, response.text response = requests.get( - urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), + urljoin(bindings_cfg.host, catalog_path(repo)), auth=("", ""), ) assert response.status_code == 401, response.text diff --git a/pulp_python/tests/functional/api/test_domains.py b/pulp_python/tests/functional/api/test_domains.py index 070692479..404c45f71 100644 --- a/pulp_python/tests/functional/api/test_domains.py +++ b/pulp_python/tests/functional/api/test_domains.py @@ -93,40 +93,30 @@ def _catalog_api_get(bindings_cfg, path, **params): @pytest.mark.parallel def test_domain_catalog_packages(bindings_cfg, domain_factory, python_repo_factory, gen_user): - """Domain-prefixed repository and repository_version hrefs, including PRN.""" + """Both catalog actions respect domain boundaries and repository permissions.""" domain = domain_factory() repo = python_repo_factory(pulp_domain=domain.name) assert f"{domain.name}/api/" in repo.pulp_href assert f"{domain.name}/api/" in repo.latest_version_href - pkgs = _catalog_api_get(bindings_cfg, f"{repo.pulp_href}packages/") - assert pkgs["count"] == 0 - assert pkgs["results"] == [] - - by_href = _catalog_api_get( - bindings_cfg, f"{repo.pulp_href}packages/", repository_version=repo.latest_version_href - ) - assert by_href["count"] == 0 - version = _catalog_api_get(bindings_cfg, repo.latest_version_href) - by_prn = _catalog_api_get( - bindings_cfg, f"{repo.pulp_href}packages/", repository_version=version["prn"] - ) - assert by_prn["count"] == 0 - assert version["prn"].startswith("prn:core.repositoryversion:") - - default_path = f"{repo.pulp_href}packages/".replace(f"/{domain.name}/api/", "/default/api/") - response = requests.get( - urljoin(bindings_cfg.host, default_path), - auth=(bindings_cfg.username, bindings_cfg.password), - ) - assert response.status_code == 404, response.text - viewer = gen_user(domain_roles=[("python.pythonrepository_viewer", domain.pulp_href)]) - response = requests.get( - urljoin(bindings_cfg.host, f"{repo.pulp_href}packages/"), - auth=(viewer.username, viewer.password), - ) - assert response.status_code == 200, response.text + for resource in (repo.pulp_href, repo.latest_version_href): + path = f"{resource}packages/" + pkgs = _catalog_api_get(bindings_cfg, path) + assert pkgs["count"] == 0 + assert pkgs["results"] == [] + + default_path = path.replace(f"/{domain.name}/api/", "/default/api/") + response = requests.get( + urljoin(bindings_cfg.host, default_path), + auth=(bindings_cfg.username, bindings_cfg.password), + ) + assert response.status_code == 404, response.text + + response = requests.get( + urljoin(bindings_cfg.host, path), auth=(viewer.username, viewer.password) + ) + assert response.status_code == 200, response.text @pytest.mark.parallel diff --git a/pulp_python/tests/unit/test_catalog.py b/pulp_python/tests/unit/test_catalog.py index 502a82c88..b1d456943 100644 --- a/pulp_python/tests/unit/test_catalog.py +++ b/pulp_python/tests/unit/test_catalog.py @@ -1,6 +1,7 @@ """Unit tests for the repository package index helpers.""" import uuid +from datetime import datetime, timedelta import pytest from django.apps import apps @@ -14,7 +15,8 @@ from pulp_python.app.models import PythonPackageContent, PythonRepository from pulp_python.app.viewsets import ( PythonPackageSingleArtifactContentUploadViewSet, - PythonRepositoryPackageViewSet, + PythonRepositoryVersionViewSet, + PythonRepositoryViewSet, ) _EMPTY_TEXT = ( @@ -88,7 +90,14 @@ def test_package_list_pagination_and_bulk_metadata(django_assert_num_queries, pa ] assert row["description"] == f"{row['name_normalized']} 1.0.4 description" - for field in ("description", "classifiers", "requires_dist", "project_urls", "license_file"): + for field in ( + "description", + "license_expression", + "classifiers", + "requires_dist", + "project_urls", + "license_file", + ): assert f'"{field}"' not in page_queries[0]["sql"] description_queries = [ query for query in queries if '"description"' in query["sql"].split(" FROM ", 1)[0] @@ -99,11 +108,11 @@ def test_package_list_pagination_and_bulk_metadata(django_assert_num_queries, pa @pytest.mark.django_db @pytest.mark.parametrize("has_sdist", [True, False]) def test_package_list_representative_file(has_sdist, django_assert_num_queries): - """Each version uses an sdist if available, otherwise its newest wheel.""" + """Metadata prefers an sdist, but update times use the newest file in the snapshot.""" repository = PythonRepository.objects.create(name=str(uuid.uuid4())) units = [] if has_sdist: - units.append(_create_package("alpha", "1.0", license_expression="MIT")) + units.append(_create_package("alpha", "1.0")) for platform in ("linux", "macos"): units.append( _create_package( @@ -111,19 +120,24 @@ def test_package_list_representative_file(has_sdist, django_assert_num_queries): "1.0", filename=f"alpha-1.0-{platform}.whl", packagetype="bdist_wheel", - license_expression="Apache-2.0", description=f"{platform} wheel description", ) ) + outside = _create_package("alpha", "1.0") + PythonPackageContent.objects.filter(pk=outside.pk).update( + pulp_created=units[-1].pulp_created + timedelta(days=1) + ) with repository.new_version() as version: version.add_content(PythonPackageContent.objects.filter(pk__in=[unit.pk for unit in units])) content = repository.latest_version().get_content(PythonPackageContent.objects.all()) with django_assert_num_queries(2): rows = assemble_package_index(list(package_catalog_queryset(content))) - assert rows[0]["versions"] == [ - {"version": "1.0", "license_expression": "MIT" if has_sdist else "Apache-2.0"} - ] + assert len(rows[0]["versions"]) == 1 + version = rows[0]["versions"][0] + assert set(version) == {"version", "last_updated"} + assert version["version"] == "1.0" + assert datetime.fromisoformat(version["last_updated"]) == units[-1].pulp_created assert rows[0]["description"] == ( "alpha 1.0 description" if has_sdist else "macos wheel description" ) @@ -132,22 +146,22 @@ def test_package_list_representative_file(has_sdist, django_assert_num_queries): @pytest.mark.django_db @pytest.mark.parametrize("explicit", [True, False]) def test_package_list_incomplete_version(django_user_model, explicit): - """Reject an explicit incomplete snapshot and ignore it when selecting the latest.""" + """An incomplete version returns 404 and is ignored by the repository action.""" repository = PythonRepository.objects.create(name=str(uuid.uuid4())) incomplete = RepositoryVersion.objects.create(repository=repository, number=1, complete=False) user = django_user_model.objects.create(username=str(uuid.uuid4()), is_superuser=True) - params = ( - {"repository_version": f"prn:core.repositoryversion:{incomplete.pk}"} if explicit else {} - ) - request = APIRequestFactory().get("/", params) + request = APIRequestFactory().get("/") + request.pulp_domain = repository.pulp_domain force_authenticate(request, user=user) - response = PythonRepositoryPackageViewSet.as_view({"get": "list"})( - request, repository_pk=str(repository.pk) + viewset = PythonRepositoryVersionViewSet if explicit else PythonRepositoryViewSet + kwargs = ( + {"repository_pk": str(repository.pk), "number": incomplete.number} + if explicit + else {"pk": str(repository.pk)} ) - assert response.status_code == (400 if explicit else 200) - if explicit: - assert "complete" in str(response.data["repository_version"]) - else: + response = viewset.as_view({"get": "packages"}, **viewset.packages.kwargs)(request, **kwargs) + assert response.status_code == (404 if explicit else 200) + if not explicit: assert response.data["count"] == 0 assert response.data["results"] == [] @@ -160,19 +174,30 @@ def test_package_content_viewset_registration(): @pytest.mark.django_db -def test_package_list_without_pagination(django_user_model): - """The standard list action also assembles rows when pagination is disabled.""" +@pytest.mark.parametrize("version_url", [False, True]) +def test_package_list_without_pagination(django_user_model, version_url): + """Both package actions assemble rows when pagination is disabled.""" repository = PythonRepository.objects.create(name=str(uuid.uuid4())) package = _create_package("alpha", "1.0") with repository.new_version() as version: version.add_content(PythonPackageContent.objects.filter(pk=package.pk)) user = django_user_model.objects.create(username=str(uuid.uuid4()), is_superuser=True) request = APIRequestFactory().get("/") + request.pulp_domain = repository.pulp_domain force_authenticate(request, user=user) - response = PythonRepositoryPackageViewSet.as_view({"get": "list"}, pagination_class=None)( - request, repository_pk=str(repository.pk) + viewset = PythonRepositoryVersionViewSet if version_url else PythonRepositoryViewSet + kwargs = ( + {"repository_pk": str(repository.pk), "number": repository.latest_version().number} + if version_url + else {"pk": str(repository.pk)} ) + response = viewset.as_view( + {"get": "packages"}, pagination_class=None, **viewset.packages.kwargs + )(request, **kwargs) assert response.status_code == 200 assert len(response.data) == 1 assert response.data[0]["description"] == "alpha 1.0 description" - assert response.data[0]["versions"] == [{"version": "1.0", "license_expression": ""}] + version = response.data[0]["versions"][0] + assert set(version) == {"version", "last_updated"} + assert version["version"] == "1.0" + assert datetime.fromisoformat(version["last_updated"]) == package.pulp_created From aeb1dc0976fb4ea0b3c982ff17b9ad3240fa7ce0 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Wed, 11 Nov 2026 00:52:32 -0500 Subject: [PATCH 5/7] Keep package catalog changes scoped to the catalog Restore the existing publication tests and PyPI access-policy initialization. Keep the catalog's domain isolation and permission checks alongside the other catalog tests instead of changing the existing domain test module. Ref #1358. Generated-By: Codex --- pulp_python/app/__init__.py | 14 ++-- .../tests/functional/api/test_catalog.py | 32 ++++++++++ .../functional/api/test_crud_publications.py | 64 +++++-------------- .../tests/functional/api/test_domains.py | 38 +---------- 4 files changed, 53 insertions(+), 95 deletions(-) diff --git a/pulp_python/app/__init__.py b/pulp_python/app/__init__.py index bb822f0ab..3bca50680 100644 --- a/pulp_python/app/__init__.py +++ b/pulp_python/app/__init__.py @@ -17,17 +17,17 @@ class PulpPythonPluginAppConfig(PulpPluginAppConfig): domain_compatible = True def ready(self): - """Register access policies for explicitly routed endpoints.""" + """Register PyPI access policy hook.""" super().ready() post_migrate.connect( - _populate_extra_access_policies, + _populate_pypi_access_policies, sender=self, dispatch_uid="populate_pypi_access_policies_identifier", ) # TODO: Remove this when https://github.com/pulp/pulpcore/issues/5500 is resolved -def _populate_extra_access_policies(sender, apps, verbosity, **kwargs): +def _populate_pypi_access_policies(sender, apps, verbosity, **kwargs): from pulp_python.app.pypi.feeds import FeedView from pulp_python.app.pypi.views import MetadataView, PyPIView, SimpleView, UploadView @@ -38,13 +38,7 @@ def _populate_extra_access_policies(sender, apps, verbosity, **kwargs): print(_("AccessPolicy model does not exist. Skipping initialization.")) return - for viewset in ( - PyPIView, - SimpleView, - UploadView, - MetadataView, - FeedView, - ): + for viewset in (PyPIView, SimpleView, UploadView, MetadataView, FeedView): access_policy = getattr(viewset, "DEFAULT_ACCESS_POLICY", None) if access_policy is not None: viewset_name = viewset.urlpattern() diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py index 488819b19..0c8594b15 100644 --- a/pulp_python/tests/functional/api/test_catalog.py +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -342,3 +342,35 @@ def test_package_list_permissions(bindings_cfg, python_repo_factory, gen_user, c auth=("", ""), ) assert response.status_code == 401, response.text + + +@pytest.mark.parallel +def test_domain_catalog_packages( + bindings_cfg, domain_factory, python_repo_factory, gen_user, pulp_domain_enabled +): + """Both catalog actions respect domain boundaries and repository permissions.""" + if not pulp_domain_enabled: + pytest.skip("Domain not enabled") + domain = domain_factory() + repo = python_repo_factory(pulp_domain=domain.name) + assert f"{domain.name}/api/" in repo.pulp_href + assert f"{domain.name}/api/" in repo.latest_version_href + + viewer = gen_user(domain_roles=[("python.pythonrepository_viewer", domain.pulp_href)]) + for resource in (repo.pulp_href, repo.latest_version_href): + path = f"{resource}packages/" + pkgs = _api_get(bindings_cfg, path) + assert pkgs["count"] == 0 + assert pkgs["results"] == [] + + default_path = path.replace(f"/{domain.name}/api/", "/default/api/") + response = requests.get( + urljoin(bindings_cfg.host, default_path), + auth=(bindings_cfg.username, bindings_cfg.password), + ) + assert response.status_code == 404, response.text + + response = requests.get( + urljoin(bindings_cfg.host, path), auth=(viewer.username, viewer.password) + ) + assert response.status_code == 200, response.text diff --git a/pulp_python/tests/functional/api/test_crud_publications.py b/pulp_python/tests/functional/api/test_crud_publications.py index fb65cbd15..e7941ac18 100644 --- a/pulp_python/tests/functional/api/test_crud_publications.py +++ b/pulp_python/tests/functional/api/test_crud_publications.py @@ -1,10 +1,8 @@ -import io import random -import tarfile -import zipfile from urllib.parse import urljoin import pytest +from pypi_simple import PyPISimple from pulp_python.tests.functional.constants import ( PYTHON_EGG_FILENAME, @@ -115,57 +113,27 @@ def test_new_content_is_published(python_publication_workflow, python_distributi assert proper is True, msgs -def _write_sdist_with_name(directory, name, version): - """Write a minimal sdist whose PKG-INFO Name differs from PEP 503 canonical form.""" - pkg_dir = f"{name}-{version}" - filename = f"{pkg_dir}.tar.gz" - path = directory / filename - pkg_info = f"Metadata-Version: 2.1\nName: {name}\nVersion: {version}\n".encode() - with tarfile.open(path, "w:gz") as tar: - info = tarfile.TarInfo(name=f"{pkg_dir}/PKG-INFO") - info.size = len(pkg_info) - tar.addfile(info, io.BytesIO(pkg_info)) - return filename, str(path) - - -def _write_wheel_with_name(directory, metadata_name, version, filename): - """Write a minimal wheel whose METADATA Name can differ from the sdist Name.""" - dist_info = f"{filename.split('-')[0]}-{version}.dist-info" - metadata = f"Metadata-Version: 2.1\nName: {metadata_name}\nVersion: {version}\n" - wheel = ( - "Wheel-Version: 1.0\nGenerator: pulp-python-test\nRoot-Is-Purelib: true\n" - "Tag: py2.py3-none-any\n" - ) - path = directory / filename - with zipfile.ZipFile(path, "w") as zf: - zf.writestr(f"{dist_info}/METADATA", metadata) - zf.writestr(f"{dist_info}/WHEEL", wheel) - return filename, str(path) - - @pytest.mark.parallel def test_non_matching_canonicalized_name( - python_repo, - python_content_factory, - python_publication_factory, - python_distribution_factory, - tmp_path, + python_repo, python_content_factory, python_publication_factory, python_distribution_factory ): """Ensures a package with dists that have non-matching canonicalized names is published.""" - version = "1.0.0" - wheel_filename = "msg_parser-1.0.0-py2.py3-none-any.whl" - sdist_filename, sdist_path = _write_sdist_with_name(tmp_path, "msg_parser", version) - _, wheel_path = _write_wheel_with_name(tmp_path, "msg-parser", version, wheel_filename) - - wheel = python_content_factory(wheel_filename, file=wheel_path, repository=python_repo) - sdist = python_content_factory(sdist_filename, file=sdist_path, repository=python_repo) - # The metadata name in the SDist is not the same as the Wheel's name - assert wheel.name == "msg-parser" - assert sdist.name == "msg_parser" - + packages = [] + filenames = ["msg_parser-1.0.0-py2.py3-none-any.whl", "msg_parser-1.0.0.tar.gz"] + with PyPISimple() as client: + page = client.get_project_page("msg-parser") + for pkg in page.packages: + if pkg.filename in filenames: + c = python_content_factory(pkg.filename, url=pkg.url, repository=python_repo) + if c.filename.endswith(".tar.gz"): + # The metadata name in the SDist is not the same as the Wheel's name + assert c.name == "msg_parser" + else: + assert c.name == "msg-parser" + packages.append(c) pub = python_publication_factory(repository=python_repo) distro = python_distribution_factory(publication=pub) url = urljoin(distro.base_url, "simple/") - proper, msgs = ensure_simple(url, {"msg-parser": [wheel_filename, sdist_filename]}) + proper, msgs = ensure_simple(url, {"msg-parser": filenames}) assert proper is True, msgs diff --git a/pulp_python/tests/functional/api/test_domains.py b/pulp_python/tests/functional/api/test_domains.py index 404c45f71..391af15c5 100644 --- a/pulp_python/tests/functional/api/test_domains.py +++ b/pulp_python/tests/functional/api/test_domains.py @@ -1,10 +1,9 @@ import json import subprocess import uuid -from urllib.parse import urljoin, urlsplit +from urllib.parse import urlsplit import pytest -import requests from pulpcore.app import settings # noqa: TID251 @@ -84,41 +83,6 @@ def test_domain_object_creation( } -def _catalog_api_get(bindings_cfg, path, **params): - url = urljoin(bindings_cfg.host + "/", path.lstrip("/")) - response = requests.get(url, params=params, auth=(bindings_cfg.username, bindings_cfg.password)) - assert response.status_code == 200, response.text - return response.json() - - -@pytest.mark.parallel -def test_domain_catalog_packages(bindings_cfg, domain_factory, python_repo_factory, gen_user): - """Both catalog actions respect domain boundaries and repository permissions.""" - domain = domain_factory() - repo = python_repo_factory(pulp_domain=domain.name) - assert f"{domain.name}/api/" in repo.pulp_href - assert f"{domain.name}/api/" in repo.latest_version_href - - viewer = gen_user(domain_roles=[("python.pythonrepository_viewer", domain.pulp_href)]) - for resource in (repo.pulp_href, repo.latest_version_href): - path = f"{resource}packages/" - pkgs = _catalog_api_get(bindings_cfg, path) - assert pkgs["count"] == 0 - assert pkgs["results"] == [] - - default_path = path.replace(f"/{domain.name}/api/", "/default/api/") - response = requests.get( - urljoin(bindings_cfg.host, default_path), - auth=(bindings_cfg.username, bindings_cfg.password), - ) - assert response.status_code == 404, response.text - - response = requests.get( - urljoin(bindings_cfg.host, path), auth=(viewer.username, viewer.password) - ) - assert response.status_code == 200, response.text - - @pytest.mark.parallel def test_domain_content_upload( domain_factory, From 12d265d71979e1789e32bf22c7909ef8c12d4155 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Wed, 11 Nov 2026 10:49:31 -0500 Subject: [PATCH 6/7] Simplify catalog coverage around functional API tests Remove the catalog unit tests and consolidate overlapping functional cases. Reuse repository, sync, content, and user fixtures with generated API clients, keeping coverage for pagination, filters, versions, snapshots, and permissions. Ref #1358. Generated-By: Codex --- .../tests/functional/api/test_catalog.py | 439 +++++------------- pulp_python/tests/unit/test_catalog.py | 203 -------- 2 files changed, 111 insertions(+), 531 deletions(-) delete mode 100644 pulp_python/tests/unit/test_catalog.py diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py index 0c8594b15..f8f95dab0 100644 --- a/pulp_python/tests/functional/api/test_catalog.py +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -2,375 +2,158 @@ import io import tarfile -import uuid -from datetime import datetime -from urllib.parse import urljoin import pytest -import requests from pulp_python.tests.functional.constants import PYTHON_SM_PROJECT_SPECIFIER -def _api_get(bindings_cfg, path, **params): - url = urljoin(bindings_cfg.host + "/", path.lstrip("/")) - response = requests.get(url, params=params, auth=(bindings_cfg.username, bindings_cfg.password)) - assert response.status_code == 200, response.text - return response.json() - - -_PACKAGE_FIELDS = { - "name", - "name_normalized", - "summary", - "description", - "author", - "author_email", - "license", - "versions", -} - - -def _assert_package_row(pkg): - assert set(pkg) == _PACKAGE_FIELDS - assert pkg["name"] - assert pkg["name_normalized"] - versions = [item["version"] for item in pkg["versions"]] - assert versions - assert len(versions) == len(set(versions)) - for item in pkg["versions"]: - assert set(item) == {"version", "last_updated"} - assert item["version"] - assert datetime.fromisoformat(item["last_updated"]).tzinfo is not None - - -def _write_sdist(directory, name, version): - """Write a minimal sdist whose PKG-INFO Name/Version pkginfo can read.""" - pkg_dir = f"{name}-{version}" - filename = f"{pkg_dir}.tar.gz" - path = directory / filename - pkg_info = ( - f"Metadata-Version: 2.4\nName: {name}\nVersion: {version}\n\n{name} {version} description\n" - ).encode() - with tarfile.open(path, "w:gz") as tar: - info = tarfile.TarInfo(name=f"{pkg_dir}/PKG-INFO") - info.size = len(pkg_info) - tar.addfile(info, io.BytesIO(pkg_info)) - return filename, str(path) - - -def _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, version): - filename, path = _write_sdist(tmp_path, name, version) - python_content_factory(relative_path=filename, file=path, repository=repo) - return python_bindings.RepositoriesPythonApi.read(repo.pulp_href) - - -@pytest.fixture -def sm_repo(python_repo_with_sync, python_remote_factory): - remote = python_remote_factory(includes=PYTHON_SM_PROJECT_SPECIFIER) - return python_repo_with_sync(remote) - +@pytest.fixture(params=["repository", "version"]) +def catalog(request, python_bindings): + """Call either catalog action through its generated client.""" + version = request.param == "version" + api = ( + python_bindings.RepositoriesPythonVersionsApi + if version + else python_bindings.RepositoriesPythonApi + ) -@pytest.fixture(params=[False, True], ids=["repository", "version"]) -def catalog_path(request): - def path(repo): - resource = repo.latest_version_href if request.param else repo.pulp_href - return f"{resource}packages/" + def packages(repo, **params): + href = repo.latest_version_href if version else repo.pulp_href + return api.packages(href, **params) - return path + return packages def test_package_list_schema(pulp_openapi_schema_pk_path_set): - """Both actions document package filters and paginated package responses.""" - schema = pulp_openapi_schema_pk_path_set + """Both actions expose package filters without a repository-version filter.""" operations = [ methods["get"] - for path, methods in schema["paths"].items() + for path, methods in pulp_openapi_schema_pk_path_set["paths"].items() if "/repositories/python/python/" in path and path.endswith("/packages/") ] assert len(operations) == 2 for operation in operations: - query_params = {item["name"] for item in operation["parameters"] if item["in"] == "query"} - assert { - "name_normalized", - "name_normalized__istartswith", - "ordering", - "limit", - "offset", - } <= query_params - assert "repository_version" not in query_params - response = operation["responses"]["200"]["content"]["application/json"]["schema"] - component = response["$ref"].rsplit("/", 1)[-1] - assert "results" in schema["components"]["schemas"][component]["properties"] - - -@pytest.mark.parallel -def test_package_list_grouping_and_pagination(bindings_cfg, sm_repo, catalog_path): - """Package index is one row per name, and count is distinct packages not files.""" - path = catalog_path(sm_repo) - data = _api_get(bindings_cfg, path, limit=1) - assert data["count"] == 3 - assert len(data["results"]) == 1 - _assert_package_row(data["results"][0]) - - page2 = _api_get(bindings_cfg, path, limit=1, offset=1) - assert page2["count"] == 3 - assert page2["results"][0]["name_normalized"] != data["results"][0]["name_normalized"] - - all_rows = _api_get(bindings_cfg, path, limit=100)["results"] - names = [pkg["name_normalized"] for pkg in all_rows] - assert names == sorted(names) - assert set(names) == {"aiohttp", "celery", "django"} - django = next(pkg for pkg in all_rows if pkg["name_normalized"] == "django") - assert {item["version"] for item in django["versions"]} >= { - "1.10.4", - "1.10.3", - "1.10.2", - "1.10.1", - } - assert all("+" not in item["version"] for item in django["versions"]) - - -@pytest.mark.parallel -def test_package_list_empty_repository(python_bindings, python_repo_factory): - repo = python_repo_factory() - for api, href in ( - (python_bindings.RepositoriesPythonApi, repo.pulp_href), - (python_bindings.RepositoriesPythonVersionsApi, repo.latest_version_href), - ): - data = api.packages(href) - assert data.count == 0 - assert data.results == [] + params = {item["name"] for item in operation["parameters"] if item["in"] == "query"} + assert {"name_normalized", "ordering", "limit", "offset"} <= params + assert "repository_version" not in params @pytest.mark.parallel -def test_packages_repository_version(bindings_cfg, sm_repo, python_repo_factory): - """Version URLs select snapshots independently of the repository's latest version.""" - latest_href = sm_repo.latest_version_href - v0_href = f"{sm_repo.pulp_href}versions/0/" - - default_pkgs = _api_get(bindings_cfg, f"{sm_repo.pulp_href}packages/") - explicit_pkgs = _api_get(bindings_cfg, f"{latest_href}packages/") - assert default_pkgs == explicit_pkgs - - v0_pkgs = _api_get(bindings_cfg, f"{v0_href}packages/") - assert v0_pkgs["count"] == 0 - assert v0_pkgs["results"] == [] - - other = python_repo_factory() - other_pkgs = _api_get(bindings_cfg, f"{other.latest_version_href}packages/") - assert other_pkgs["results"] == [] +def test_package_list(python_repo_with_sync, python_remote_factory, catalog): + """Group and page package names, with name filtering and ordering.""" + repo = python_repo_with_sync(python_remote_factory(includes=PYTHON_SM_PROJECT_SPECIFIER)) + first = catalog(repo, limit=2) + second = catalog(repo, limit=2, offset=2) + assert first.count == second.count == 3 + assert [pkg.name_normalized for pkg in first.results + second.results] == [ + "aiohttp", + "celery", + "django", + ] + django = second.results[0] + assert [item.version for item in django.versions] == ["1.10.4", "1.10.3", "1.10.2", "1.10.1"] + assert set(django.versions[0].to_dict()) == {"version", "last_updated"} + for params in ({"name_normalized": "django"}, {"name_normalized__istartswith": "DJAN"}): + filtered = catalog(repo, **params) + assert filtered.count == 1 + assert filtered.results[0].name_normalized == "django" + descending = catalog(repo, ordering=["-name_normalized"], limit=2) + assert [pkg.name_normalized for pkg in descending.results] == ["django", "celery"] @pytest.mark.parallel -def test_package_list_one_entry_per_stored_version( - bindings_cfg, python_bindings, python_repo_with_sync, monitor_task -): - """Update times include all files in the selected snapshot.""" +def test_package_list_snapshots(python_bindings, python_repo_with_sync, monitor_task): + """Metadata and update times use only files in the requested snapshot.""" repo = python_repo_with_sync() - files = python_bindings.ContentPackagesApi.list(repository_version=repo.latest_version_href) - assert files.count == 2 - assert {item.packagetype for item in files.results} == {"sdist", "bdist_wheel"} - - pkgs = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") - assert pkgs["count"] == 1 - row = pkgs["results"][0] - _assert_package_row(row) - assert row["name_normalized"] == "shelf-reader" - assert [item["version"] for item in row["versions"]] == ["0.1"] - assert datetime.fromisoformat(row["versions"][0]["last_updated"]) == max( - item.pulp_created for item in files.results - ) + repositories = python_bindings.RepositoriesPythonApi + versions = python_bindings.RepositoriesPythonVersionsApi + empty = versions.packages(f"{repo.pulp_href}versions/0/") + assert empty.count == 0 + assert empty.results == [] + + files = python_bindings.ContentPackagesApi.list( + repository_version=repo.latest_version_href + ).results + sdist = next(item for item in files if item.packagetype == "sdist") + newest = max(files, key=lambda item: item.pulp_created) + remaining = next(item for item in files if item.pulp_href != newest.pulp_href) + original = repositories.packages(repo.pulp_href) + assert original.count == 1 + row = original.results[0] + assert row.description == sdist.description + assert [(item.version, item.last_updated) for item in row.versions] == [ + ("0.1", newest.pulp_created) + ] - latest_file = max(files.results, key=lambda item: item.pulp_created) - remaining = next(item for item in files.results if item.pulp_href != latest_file.pulp_href) monitor_task( - python_bindings.RepositoriesPythonApi.modify( - repo.pulp_href, {"remove_content_units": [latest_file.pulp_href]} - ).task + repositories.modify(repo.pulp_href, {"remove_content_units": [newest.pulp_href]}).task ) - current = _api_get(bindings_cfg, f"{repo.pulp_href}packages/")["results"][0] - assert datetime.fromisoformat(current["versions"][0]["last_updated"]) == remaining.pulp_created - historical = _api_get(bindings_cfg, f"{repo.latest_version_href}packages/")["results"][0] - assert historical == row - - -@pytest.mark.parallel -def test_package_list_version_order( - bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path -): - """Stored versions are newest-first by PEP 440, not lexicographically.""" - repo = python_repo_factory() - name = f"ordered-{uuid.uuid4().hex[:8]}" - for version in ("1.10", "1.9", "1.2"): - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, version) - data = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") - assert data["count"] == 1 - pkg = data["results"][0] - _assert_package_row(pkg) - assert [item["version"] for item in pkg["versions"]] == ["1.10", "1.9", "1.2"] + current = repositories.packages(repo.pulp_href).results[0] + assert current.versions[0].last_updated == remaining.pulp_created + assert current.description == remaining.description + assert versions.packages(repo.latest_version_href).to_dict() == original.to_dict() + with pytest.raises(python_bindings.ApiException) as exc: + versions.packages(f"{repo.pulp_href}versions/999/") + assert exc.value.status == 404 @pytest.mark.parallel -def test_package_list_version_metadata( - python_bindings, python_content_factory, python_repo_factory, tmp_path -): - """Metadata follows version precedence and remains scoped to the selected snapshot.""" - repo = python_repo_factory() - name = f"metadata-{uuid.uuid4().hex[:8]}" - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "2.0") - historical_href = repo.latest_version_href - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0") - files = python_bindings.ContentPackagesApi.list(repository_version=repo.latest_version_href) - updated = {item.version: item.pulp_created for item in files.results} - latest = python_bindings.RepositoriesPythonApi.packages(repo.pulp_href).results[0] - assert latest.description.strip() == f"{name} 2.0 description" - assert [(item.version, item.last_updated) for item in latest.versions] == [ - ("2.0", updated["2.0"]), - ("1.0", updated["1.0"]), - ] - historical = python_bindings.RepositoriesPythonVersionsApi.packages(historical_href).results[0] - assert [(item.version, item.last_updated) for item in historical.versions] == [ - ("2.0", updated["2.0"]), +def test_package_list_versions(python_content_factory, python_repo, python_bindings, tmp_path): + """PEP 440 ordering selects metadata and preserves separate rebuild versions.""" + content = {} + for version in ("1.10", "1.9", "1.9+test.1"): + filename = f"catalog-test-{version}.tar.gz" + path = tmp_path / filename + metadata = ( + f"Metadata-Version: 2.4\nName: catalog-test\nVersion: {version}\n\nVersion {version}\n" + ).encode() + with tarfile.open(path, "w:gz") as archive: + info = tarfile.TarInfo(f"catalog-test-{version}/PKG-INFO") + info.size = len(metadata) + archive.addfile(info, io.BytesIO(metadata)) + content[version] = python_content_factory(filename, file=str(path), repository=python_repo) + + data = python_bindings.RepositoriesPythonApi.packages(python_repo.pulp_href) + assert data.count == 1 + row = data.results[0] + assert row.description.strip() == "Version 1.10" + assert [(item.version, item.last_updated) for item in row.versions] == [ + (version, content[version].pulp_created) for version in ("1.10", "1.9+test.1", "1.9") ] @pytest.mark.parallel -def test_package_list_keeps_stored_versions( - bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path +def test_package_list_permissions( + python_bindings, python_repo, python_repo_factory, gen_user, catalog ): - """A rebuild stays its own file, with the version string stored in Pulp.""" - repo = python_repo_factory() - suffix = uuid.uuid4().hex[:8] - name = f"pkg-{suffix}" - - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "2.0.0") - original_version = repo.latest_version_href - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "1.0.0+test.3") - row = _api_get(bindings_cfg, f"{repo.pulp_href}packages/")["results"][0] - _assert_package_row(row) - assert [item["version"] for item in row["versions"]] == ["2.0.0", "1.0.0+test.3"] - historical = _api_get(bindings_cfg, f"{original_version}packages/")["results"][0] - assert [item["version"] for item in historical["versions"]] == ["2.0.0"] - - -@pytest.mark.parallel -def test_package_list_keeps_rebuild_version_string( - bindings_cfg, python_bindings, python_content_factory, python_repo_factory, tmp_path -): - """A rebuild is a separate entry because its stored version string differs.""" - repo = python_repo_factory() - name = f"rebuild-{uuid.uuid4().hex[:8]}" - repo = _add_sdist(python_content_factory, python_bindings, tmp_path, repo, name, "5.3.17") - repo = _add_sdist( - python_content_factory, - python_bindings, - tmp_path, - repo, - name, - "5.3.17+test.1.n1", - ) - - pkgs = _api_get(bindings_cfg, f"{repo.pulp_href}packages/") - assert pkgs["count"] == 1 - pkg = pkgs["results"][0] - _assert_package_row(pkg) - assert {item["version"] for item in pkg["versions"]} == {"5.3.17", "5.3.17+test.1.n1"} - - -@pytest.mark.parallel -def test_package_list_filters_and_ordering(bindings_cfg, sm_repo, catalog_path): - """Name filters and ordering apply to packages within the selected snapshot.""" - path = catalog_path(sm_repo) - for params in ( - {"name_normalized": "django"}, - {"name_normalized__istartswith": "DJAN"}, - {"name_normalized__icontains": "JANG"}, - {"name_normalized__in": "django,absent"}, - ): - data = _api_get(bindings_cfg, path, **params) - assert data["count"] == 1 - assert [row["name_normalized"] for row in data["results"]] == ["django"] - data = _api_get(bindings_cfg, path, ordering="-name_normalized", limit=2) - assert data["count"] == 3 - assert [row["name_normalized"] for row in data["results"]] == ["django", "celery"] - data = _api_get(bindings_cfg, path, ordering="-name_normalized", limit=2, offset=2) - assert [row["name_normalized"] for row in data["results"]] == ["aiohttp"] - - -@pytest.mark.parallel -def test_package_list_missing_version(bindings_cfg, python_repo_factory): - """A missing repository snapshot returns 404.""" - repo = python_repo_factory() - response = requests.get( - urljoin(bindings_cfg.host, f"{repo.pulp_href}versions/999/packages/"), - auth=(bindings_cfg.username, bindings_cfg.password), - ) - assert response.status_code == 404, response.text - - -@pytest.mark.parallel -def test_package_list_permissions(bindings_cfg, python_repo_factory, gen_user, catalog_path): - """Catalog access requires view permission on the parent repository.""" - repo = python_repo_factory() + """Both actions require view permission on the parent repository.""" other = python_repo_factory() - user = gen_user() - viewer = gen_user(object_roles=[("python.pythonrepository_viewer", repo.pulp_href)]) - model_viewer = gen_user(model_roles=["python.pythonrepository_viewer"]) - denied_status = 403 if "/versions/" in catalog_path(repo) else 404 - for actor, repository, status in ( - (user, repo, denied_status), - (viewer, repo, 200), - (viewer, other, denied_status), - (model_viewer, other, 200), - ): - response = requests.get( - urljoin(bindings_cfg.host, catalog_path(repository)), - auth=(actor.username, actor.password), - ) - assert response.status_code == status, response.text - - response = requests.post( - urljoin(bindings_cfg.host, catalog_path(repo)), - json={}, - auth=(bindings_cfg.username, bindings_cfg.password), - ) - assert response.status_code == 405, response.text - - response = requests.get( - urljoin(bindings_cfg.host, catalog_path(repo)), - auth=("", ""), - ) - assert response.status_code == 401, response.text + viewer = gen_user(object_roles=[("python.pythonrepository_viewer", python_repo.pulp_href)]) + with viewer: + assert catalog(python_repo).results == [] + with pytest.raises(python_bindings.ApiException) as exc: + catalog(other) + assert exc.value.status in (403, 404) + with gen_user(model_roles=["python.pythonrepository_viewer"]): + assert catalog(other).results == [] @pytest.mark.parallel -def test_domain_catalog_packages( - bindings_cfg, domain_factory, python_repo_factory, gen_user, pulp_domain_enabled +def test_package_list_domains( + python_bindings, domain_factory, python_repo_factory, gen_user, pulp_domain_enabled, catalog ): - """Both catalog actions respect domain boundaries and repository permissions.""" + """Both actions enforce domain boundaries and accept domain viewer roles.""" if not pulp_domain_enabled: pytest.skip("Domain not enabled") domain = domain_factory() repo = python_repo_factory(pulp_domain=domain.name) - assert f"{domain.name}/api/" in repo.pulp_href - assert f"{domain.name}/api/" in repo.latest_version_href - - viewer = gen_user(domain_roles=[("python.pythonrepository_viewer", domain.pulp_href)]) - for resource in (repo.pulp_href, repo.latest_version_href): - path = f"{resource}packages/" - pkgs = _api_get(bindings_cfg, path) - assert pkgs["count"] == 0 - assert pkgs["results"] == [] - - default_path = path.replace(f"/{domain.name}/api/", "/default/api/") - response = requests.get( - urljoin(bindings_cfg.host, default_path), - auth=(bindings_cfg.username, bindings_cfg.password), - ) - assert response.status_code == 404, response.text - - response = requests.get( - urljoin(bindings_cfg.host, path), auth=(viewer.username, viewer.password) - ) - assert response.status_code == 200, response.text + with gen_user(domain_roles=[("python.pythonrepository_viewer", domain.pulp_href)]): + assert catalog(repo).results == [] + repo.pulp_href = repo.pulp_href.replace(f"/{domain.name}/api/", "/default/api/") + repo.latest_version_href = repo.latest_version_href.replace( + f"/{domain.name}/api/", "/default/api/" + ) + with pytest.raises(python_bindings.ApiException) as exc: + catalog(repo) + assert exc.value.status == 404 diff --git a/pulp_python/tests/unit/test_catalog.py b/pulp_python/tests/unit/test_catalog.py deleted file mode 100644 index b1d456943..000000000 --- a/pulp_python/tests/unit/test_catalog.py +++ /dev/null @@ -1,203 +0,0 @@ -"""Unit tests for the repository package index helpers.""" - -import uuid -from datetime import datetime, timedelta - -import pytest -from django.apps import apps -from django.db import connection -from django.test.utils import CaptureQueriesContext -from rest_framework.test import APIRequestFactory, force_authenticate - -from pulpcore.plugin.models import RepositoryVersion - -from pulp_python.app.catalog import assemble_package_index, package_catalog_queryset -from pulp_python.app.models import PythonPackageContent, PythonRepository -from pulp_python.app.viewsets import ( - PythonPackageSingleArtifactContentUploadViewSet, - PythonRepositoryVersionViewSet, - PythonRepositoryViewSet, -) - -_EMPTY_TEXT = ( - "author", - "author_email", - "description", - "home_page", - "keywords", - "license", - "metadata_version", - "platform", - "summary", - "download_url", - "supported_platform", - "maintainer", - "maintainer_email", - "project_url", - "requires_python", - "description_content_type", - "license_expression", - "python_version", -) - - -def _create_package(name, version, **overrides): - fields = {field: "" for field in _EMPTY_TEXT} - fields.update( - name=name, - version=version, - filename=f"{name}-{version}.tar.gz", - packagetype="sdist", - sha256=uuid.uuid4().hex + uuid.uuid4().hex, - description=f"{name} {version} description", - ) - fields.update(overrides) - return PythonPackageContent.objects.create(**fields) - - -@pytest.mark.django_db -@pytest.mark.parametrize("page_size", [1, 2, 4]) -def test_package_list_pagination_and_bulk_metadata(django_assert_num_queries, page_size): - """Page distinct names and load large metadata only for the newest version.""" - repository = PythonRepository.objects.create(name=str(uuid.uuid4())) - created = [ - _create_package(name, f"1.0.{number}") - for name in ("alpha", "beta", "gamma", "delta") - for number in range(5) - ] - with repository.new_version() as version: - version.add_content( - PythonPackageContent.objects.filter(pk__in=[item.pk for item in created]) - ) - - repo_version = repository.latest_version() - content_qs = repo_version.get_content(PythonPackageContent.objects.all()) - packages = package_catalog_queryset(content_qs) - - with django_assert_num_queries(1): - assert packages.count() == 4 - with django_assert_num_queries(1), CaptureQueriesContext(connection) as page_queries: - page = list(packages[:page_size]) - with django_assert_num_queries(1), CaptureQueriesContext(connection) as queries: - rows = assemble_package_index(page) - - assert [row["name_normalized"] for row in rows] == ["alpha", "beta", "delta", "gamma"][ - :page_size - ] - for row in rows: - assert [item["version"] for item in row["versions"]] == [ - f"1.0.{number}" for number in reversed(range(5)) - ] - assert row["description"] == f"{row['name_normalized']} 1.0.4 description" - - for field in ( - "description", - "license_expression", - "classifiers", - "requires_dist", - "project_urls", - "license_file", - ): - assert f'"{field}"' not in page_queries[0]["sql"] - description_queries = [ - query for query in queries if '"description"' in query["sql"].split(" FROM ", 1)[0] - ] - assert len(description_queries) == 1 - - -@pytest.mark.django_db -@pytest.mark.parametrize("has_sdist", [True, False]) -def test_package_list_representative_file(has_sdist, django_assert_num_queries): - """Metadata prefers an sdist, but update times use the newest file in the snapshot.""" - repository = PythonRepository.objects.create(name=str(uuid.uuid4())) - units = [] - if has_sdist: - units.append(_create_package("alpha", "1.0")) - for platform in ("linux", "macos"): - units.append( - _create_package( - "alpha", - "1.0", - filename=f"alpha-1.0-{platform}.whl", - packagetype="bdist_wheel", - description=f"{platform} wheel description", - ) - ) - outside = _create_package("alpha", "1.0") - PythonPackageContent.objects.filter(pk=outside.pk).update( - pulp_created=units[-1].pulp_created + timedelta(days=1) - ) - with repository.new_version() as version: - version.add_content(PythonPackageContent.objects.filter(pk__in=[unit.pk for unit in units])) - content = repository.latest_version().get_content(PythonPackageContent.objects.all()) - - with django_assert_num_queries(2): - rows = assemble_package_index(list(package_catalog_queryset(content))) - assert len(rows[0]["versions"]) == 1 - version = rows[0]["versions"][0] - assert set(version) == {"version", "last_updated"} - assert version["version"] == "1.0" - assert datetime.fromisoformat(version["last_updated"]) == units[-1].pulp_created - assert rows[0]["description"] == ( - "alpha 1.0 description" if has_sdist else "macos wheel description" - ) - - -@pytest.mark.django_db -@pytest.mark.parametrize("explicit", [True, False]) -def test_package_list_incomplete_version(django_user_model, explicit): - """An incomplete version returns 404 and is ignored by the repository action.""" - repository = PythonRepository.objects.create(name=str(uuid.uuid4())) - incomplete = RepositoryVersion.objects.create(repository=repository, number=1, complete=False) - user = django_user_model.objects.create(username=str(uuid.uuid4()), is_superuser=True) - request = APIRequestFactory().get("/") - request.pulp_domain = repository.pulp_domain - force_authenticate(request, user=user) - viewset = PythonRepositoryVersionViewSet if explicit else PythonRepositoryViewSet - kwargs = ( - {"repository_pk": str(repository.pk), "number": incomplete.number} - if explicit - else {"pk": str(repository.pk)} - ) - response = viewset.as_view({"get": "packages"}, **viewset.packages.kwargs)(request, **kwargs) - assert response.status_code == (404 if explicit else 200) - if not explicit: - assert response.data["count"] == 0 - assert response.data["results"] == [] - - -def test_package_content_viewset_registration(): - """The catalog must not compete with the package content resource's viewset.""" - assert apps.get_app_config("python").named_viewsets[PythonPackageContent] == [ - PythonPackageSingleArtifactContentUploadViewSet - ] - - -@pytest.mark.django_db -@pytest.mark.parametrize("version_url", [False, True]) -def test_package_list_without_pagination(django_user_model, version_url): - """Both package actions assemble rows when pagination is disabled.""" - repository = PythonRepository.objects.create(name=str(uuid.uuid4())) - package = _create_package("alpha", "1.0") - with repository.new_version() as version: - version.add_content(PythonPackageContent.objects.filter(pk=package.pk)) - user = django_user_model.objects.create(username=str(uuid.uuid4()), is_superuser=True) - request = APIRequestFactory().get("/") - request.pulp_domain = repository.pulp_domain - force_authenticate(request, user=user) - viewset = PythonRepositoryVersionViewSet if version_url else PythonRepositoryViewSet - kwargs = ( - {"repository_pk": str(repository.pk), "number": repository.latest_version().number} - if version_url - else {"pk": str(repository.pk)} - ) - response = viewset.as_view( - {"get": "packages"}, pagination_class=None, **viewset.packages.kwargs - )(request, **kwargs) - assert response.status_code == 200 - assert len(response.data) == 1 - assert response.data[0]["description"] == "alpha 1.0 description" - version = response.data[0]["versions"][0] - assert set(version) == {"version", "last_updated"} - assert version["version"] == "1.0" - assert datetime.fromisoformat(version["last_updated"]) == package.pulp_created From db4d7782a751c51ab766c25c75a2246a9600402a Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Wed, 11 Nov 2026 11:07:20 -0500 Subject: [PATCH 7/7] Clarify package catalog descriptions Shorten catalog field descriptions and remove repeated explanations from the user guide. Describe aggregation values and metadata selection precisely, and use repository version consistently in comments and documentation. Ref #1358. Generated-By: Codex --- docs/user/guides/catalog.md | 36 ++++++++++--------- pulp_python/app/catalog.py | 17 +++++---- pulp_python/app/serializers.py | 23 ++++++------ pulp_python/app/viewsets.py | 4 +-- .../tests/functional/api/test_catalog.py | 2 +- 5 files changed, 41 insertions(+), 41 deletions(-) diff --git a/docs/user/guides/catalog.md b/docs/user/guides/catalog.md index 07d6d8666..10025f254 100644 --- a/docs/user/guides/catalog.md +++ b/docs/user/guides/catalog.md @@ -1,8 +1,8 @@ # Browse the package catalog -The package list returns **one row per package name**. Use the repository URL -to read its latest complete version, or a repository-version URL to read a -specific complete snapshot. `${REPO_PK}` is the repository UUID. +The package catalog returns one row per normalized package name. Use the +repository URL to read its latest complete repository version, or a repository +version URL to read a specific complete version. `${REPO_PK}` is the repository UUID. ## List packages @@ -10,21 +10,24 @@ specific complete snapshot. `${REPO_PK}` is the repository UUID. http GET "${BASE_ADDR}/pulp/api/v3/repositories/python/python/${REPO_PK}/packages/?limit=10" ``` -For a specific snapshot, `${VERSION_NUMBER}` is its repository version number: +To read a specific repository version, set `${VERSION_NUMBER}` to its number: ```bash http GET "${BASE_ADDR}/pulp/api/v3/repositories/python/python/${REPO_PK}/versions/${VERSION_NUMBER}/packages/?limit=10" ``` -`count` is the number of distinct packages, not files. Each row is one package -name. `summary`, `description`, `author`, `author_email`, and `license` come -from the highest version in PEP 440 order. An sdist supplies the metadata when -available; otherwise the most recently created content unit is used. -`versions` has one entry per stored version string, newest first, with a -`last_updated` timestamp for each version. This is the latest content creation -time among that version's files in the selected snapshot, not the upstream -release date. A wheel and an sdist of the same version are one entry. A rebuild -such as `1.0.0+test.1` stays its own entry because the stored version differs. +`count` is the number of distinct packages, not files. `summary`, `description`, +`author`, `author_email`, and `license` come from the highest package version in +PEP 440 order. Metadata comes from an sdist of that version when available. +Otherwise, it comes from that version's newest file. + +`versions` has one entry per stored version string, in descending PEP 440 order. +A wheel and an sdist of the same version share one entry. Different version +strings, such as `1.0.0` and `1.0.0+test.1`, produce separate entries. + +Each version's `last_updated` is the latest `pulp_created` timestamp among its +files in the selected repository version. This records when Pulp created the +content, not the upstream release date. ```json { @@ -48,10 +51,9 @@ such as `1.0.0+test.1` stays its own entry because the stored version differs. } ``` -Results are ordered by `name_normalized`. Within a package, versions are newest -first. Page with `limit` and `offset` to read the whole catalog. Use the -repository-version URL when the export must stay on one snapshot. Missing or -incomplete snapshots return HTTP 404. +Results are ordered by `name_normalized`. Use `limit` and `offset` to page +through the catalog. Use a repository version URL to keep every page on the +same repository version. Missing or incomplete repository versions return HTTP 404. Filter names with `name_normalized`, `name_normalized__icontains`, or `name_normalized__istartswith`. Use `ordering=-name_normalized` to list names diff --git a/pulp_python/app/catalog.py b/pulp_python/app/catalog.py index 5cfe4138d..7f41d8687 100644 --- a/pulp_python/app/catalog.py +++ b/pulp_python/app/catalog.py @@ -1,4 +1,4 @@ -"""Helpers for the repository package list.""" +"""Helpers for the repository package catalog.""" from django import VERSION as DJANGO_VERSION from django.contrib.postgres.aggregates import JSONBAgg @@ -9,7 +9,7 @@ def _version_sort_key(version): - """Keep legacy invalid versions browsable, after valid PEP 440 versions.""" + """Sort invalid version strings after valid PEP 440 versions.""" try: return (True, Version(version)) except InvalidVersion: @@ -17,20 +17,19 @@ def _version_sort_key(version): class PackageVersions(JSONBAgg): - """Map versions to values, keeping the last file.""" + """Aggregate values by version, keeping the last value for duplicate keys.""" function = "JSONB_OBJECT_AGG" arity = 2 def package_catalog_queryset(content, ordering=("name_normalized",)): - """Group package names and load their representative versions in the same query. + """Collect version timestamps and metadata IDs for each normalized package name. - Wheel and sdist of the same `(name_normalized, version)` collapse to one - row. An sdist supplies metadata when available, while the newest file's - `pulp_created` supplies the update time. + Wheels and sdists with the same version share an aggregate entry. + Each timestamp is the newest file's `pulp_created`. Use sdist metadata when available. """ - # Each aggregate keeps the last file for each stored version. + # Aggregate ordering selects the newest timestamp and prefers sdist metadata. ordering_argument = "order_by" if DJANGO_VERSION >= (5, 2) else "ordering" file_ordering = { ordering_argument: ( @@ -63,7 +62,7 @@ def package_catalog_queryset(content, ordering=("name_normalized",)): def assemble_package_index(package_rows): - """Sort the page's versions and bulk-load metadata from their highest versions.""" + """Sort each package's versions and fetch metadata for its highest version in one query.""" if not package_rows: return [] diff --git a/pulp_python/app/serializers.py b/pulp_python/app/serializers.py index 71dcdd839..e6031ed7f 100644 --- a/pulp_python/app/serializers.py +++ b/pulp_python/app/serializers.py @@ -647,12 +647,15 @@ class PythonRepositoryPackageVersionSerializer(serializers.Serializer): version = serializers.CharField(help_text=_("The package's version number.")) last_updated = serializers.DateTimeField( - help_text=_("The latest content creation time among this version's files in the snapshot."), + help_text=_( + "The latest `pulp_created` timestamp for this version's files " + "in the selected repository version." + ), ) class PythonRepositoryPackageListSerializer(serializers.ListSerializer): - """Assemble metadata only for the names selected by pagination.""" + """Load package metadata for the current page.""" def to_representation(self, data): rows = assemble_package_index(list(data)) @@ -663,10 +666,10 @@ class PythonRepositoryPackageSerializer(serializers.Serializer): """One distinct package name and its stored versions.""" name = serializers.CharField( - help_text=_("A project name stored on one of this package's files."), + help_text=_("The package name from its metadata."), ) name_normalized = serializers.CharField( - help_text=_("PEP 503 normalized package name. Index rows are unique on this field."), + help_text=_("The package name normalized according to PEP 503."), ) summary = serializers.CharField( allow_blank=True, @@ -674,14 +677,11 @@ class PythonRepositoryPackageSerializer(serializers.Serializer): ) description = serializers.CharField( allow_blank=True, - help_text=_("A longer description of the package that can run to several paragraphs."), + help_text=_("The package description."), ) author = serializers.CharField( allow_blank=True, - help_text=_( - "Text containing the author's name. Contact information can also be added," - " separated with newlines." - ), + help_text=_("Author information from the package metadata."), ) author_email = serializers.CharField( allow_blank=True, @@ -689,13 +689,12 @@ class PythonRepositoryPackageSerializer(serializers.Serializer): ) license = serializers.CharField( allow_blank=True, - help_text=_("Text indicating the license covering the distribution."), + help_text=_("The license declared by the package."), ) versions = PythonRepositoryPackageVersionSerializer( many=True, help_text=_( - "One entry per stored version with its update time, newest version first. " - "A wheel and an sdist of the same version are one entry. Version strings are unchanged." + "Package versions in descending PEP 440 order, with an update time for each version." ), ) diff --git a/pulp_python/app/viewsets.py b/pulp_python/app/viewsets.py index e483499d5..cce1192d8 100644 --- a/pulp_python/app/viewsets.py +++ b/pulp_python/app/viewsets.py @@ -38,7 +38,7 @@ class PythonRepositoryPackageFilter(FilterSet): - """Filter names and order packages in a repository snapshot.""" + """Filter names and order packages in a repository version.""" ordering = OrderingFilter(fields=("name_normalized",)) @@ -87,7 +87,7 @@ class PythonRepositoryPackagesMixin: filter_backends=(PythonRepositoryPackageFilterBackend,), ) def packages(self, request, **kwargs): - """List packages from the snapshot identified by the resource URL.""" + """List packages from the repository version identified by the resource URL.""" resource = self.get_object() if isinstance(resource, RepositoryVersion): if resource.repository.pulp_domain_id != get_domain_pk(): diff --git a/pulp_python/tests/functional/api/test_catalog.py b/pulp_python/tests/functional/api/test_catalog.py index f8f95dab0..bed4168a2 100644 --- a/pulp_python/tests/functional/api/test_catalog.py +++ b/pulp_python/tests/functional/api/test_catalog.py @@ -64,7 +64,7 @@ def test_package_list(python_repo_with_sync, python_remote_factory, catalog): @pytest.mark.parallel def test_package_list_snapshots(python_bindings, python_repo_with_sync, monitor_task): - """Metadata and update times use only files in the requested snapshot.""" + """Metadata and update times use only files in the requested repository version.""" repo = python_repo_with_sync() repositories = python_bindings.RepositoriesPythonApi versions = python_bindings.RepositoriesPythonVersionsApi