diff --git a/CHANGES/1232.bugfix b/CHANGES/1232.bugfix new file mode 100644 index 000000000..d6eabc4a5 --- /dev/null +++ b/CHANGES/1232.bugfix @@ -0,0 +1 @@ +Fixed `upload_time` in Simple and JSON APIs to reflect repository addition time instead of content creation time. diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index 04bcc1653..b8585df7a 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -7,6 +7,7 @@ from django.contrib.sessions.models import Session from django.core.exceptions import ObjectDoesNotExist from django.db import transaction +from django.db.models import OuterRef, Subquery from django.db.utils import DatabaseError from django.http.response import ( Http404, @@ -25,6 +26,7 @@ from rest_framework.response import Response from rest_framework.viewsets import ViewSet +from pulpcore.plugin.models import RepositoryContent from pulpcore.plugin.tasking import dispatch from pulpcore.plugin.util import get_domain, get_url from pulpcore.plugin.viewsets import OperationPostponedResponse @@ -366,13 +368,20 @@ def retrieve(self, request, path, package): return redirect(urljoin(self.base_content_url, f"{path}/simple/{normalized}/")) if content is not None: local_packages = content.filter(name_normalized=normalized) - packages = local_packages.values( + repo_added_subquery = RepositoryContent.objects.filter( + content_id=OuterRef("pk"), + repository=repo_ver.repository, + version_removed=None, + ).values("pulp_created")[:1] + packages = local_packages.annotate( + repo_added_time=Subquery(repo_added_subquery) + ).values( "filename", "sha256", "metadata_sha256", "requires_python", "size", - "pulp_created", + "repo_added_time", "version", ) provenances = PackageProvenance.objects.filter(package__in=local_packages).values_list( @@ -382,7 +391,7 @@ def retrieve(self, request, path, package): p["filename"]: { **p, "url": urljoin(self.base_content_url, f"{path}/{p['filename']}"), - "upload_time": p["pulp_created"], + "upload_time": p["repo_added_time"], "provenance": ( self.get_provenance_url(normalized, p["version"], p["filename"]) if p["filename"] in provenances @@ -464,7 +473,11 @@ def retrieve(self, request, path, meta): if settings.DOMAIN_ENABLED: domain = get_domain() json_body = python_content_to_json( - path, package_content, version=version, domain=domain + path, + package_content, + version=version, + domain=domain, + repository_version=repo_ver, ) if json_body: return Response(data=json_body, headers=headers) diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index 203a15300..1b30e82a4 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -11,6 +11,7 @@ import pkginfo from aiohttp.client_exceptions import ClientError from django.conf import settings +from django.db.models import OuterRef, Subquery from django.db.utils import IntegrityError from jinja2 import Template from packaging.requirements import Requirement @@ -19,7 +20,7 @@ from pypi_simple import ACCEPT_JSON_PREFERRED, ProjectPage from pulpcore.plugin.exceptions import TimeoutException -from pulpcore.plugin.models import Artifact, Remote +from pulpcore.plugin.models import Artifact, Remote, RepositoryContent from pulpcore.plugin.util import get_domain log = logging.getLogger(__name__) @@ -359,7 +360,9 @@ def fetch_json_release_metadata(name: str, version: str, remotes: set[Remote]) - raise Exception(f"Failed to fetch {url} from any remote.") -def python_content_to_json(base_path, content_query, version=None, domain=None): +def python_content_to_json( + base_path, content_query, version=None, domain=None, repository_version=None +): """ Converts a QuerySet of PythonPackageContent into the PyPi JSON format https://www.python.org/dev/peps/pep-0566/ @@ -371,6 +374,13 @@ def python_content_to_json(base_path, content_query, version=None, domain=None): Returns None if version is specified but not found within content_query """ + if repository_version: + repo_added_subquery = RepositoryContent.objects.filter( + content_id=OuterRef("pk"), + repository=repository_version.repository, + version_removed=None, + ).values("pulp_created")[:1] + content_query = content_query.annotate(repo_added_time=Subquery(repo_added_subquery)) full_metadata = {"last_serial": 0} # For now the serial field isn't supported by Pulp latest_content = latest_content_version(content_query, version) if not latest_content: @@ -515,8 +525,10 @@ def find_artifact(): "python_version": content.python_version, "requires_python": content.requires_python or None, "size": content.size, - "upload_time": str(content.pulp_created), - "upload_time_iso_8601": str(content.pulp_created.isoformat()), + "upload_time": str(getattr(content, "repo_added_time", None) or content.pulp_created), + "upload_time_iso_8601": str( + (getattr(content, "repo_added_time", None) or content.pulp_created).isoformat() + ), "url": url, "yanked": False, "yanked_reason": None, diff --git a/pulp_python/tests/functional/api/test_pypi_apis.py b/pulp_python/tests/functional/api/test_pypi_apis.py index 4934d3a98..48a3179d2 100644 --- a/pulp_python/tests/functional/api/test_pypi_apis.py +++ b/pulp_python/tests/functional/api/test_pypi_apis.py @@ -1,4 +1,6 @@ import subprocess +import time +from datetime import datetime from urllib.parse import urljoin import pytest @@ -6,6 +8,7 @@ from pulp_python.tests.functional.constants import ( PYPI_SERIAL_CONSTANT, + PYPI_SIMPLE_V1_JSON, PYTHON_EGG_FILENAME, PYTHON_EGG_SHA256, PYTHON_MD_PROJECT_SPECIFIER, @@ -13,6 +16,8 @@ PYTHON_WHEEL_FILENAME, PYTHON_WHEEL_SHA256, SHELF_PYTHON_JSON, + TWINE_WHEEL_FILENAME, + TWINE_WHEEL_URL, ) from pulp_python.tests.functional.utils import ensure_metadata @@ -328,3 +333,41 @@ def assert_download_info(expected, received, message="Failed to match"): matched = True break assert matched is True, message + + +@pytest.mark.parallel +def test_upload_time_reflects_repo_addition( + monitor_task, + python_bindings, + python_content_factory, + python_distribution_factory, + python_repo_factory, +): + """ + Test that upload_time reflects repository addition time instead of content creation time. + Checks both Simple and JSON APIs. + """ + content = python_content_factory(TWINE_WHEEL_FILENAME, url=TWINE_WHEEL_URL) + content_created = datetime.fromisoformat( + str(python_bindings.ContentPackagesApi.read(content.pulp_href).pulp_created) + ) + time.sleep(2) + + repo = python_repo_factory() + body = {"add_content_units": [content.pulp_href]} + monitor_task(python_bindings.RepositoriesPythonApi.modify(repo.pulp_href, body).task) + distro = python_distribution_factory(repository=repo) + + # Simple API + headers = {"Accept": PYPI_SIMPLE_V1_JSON} + resp = requests.get(f"{urljoin(distro.base_url, 'simple/')}twine", headers=headers) + assert resp.status_code == 200 + simple_time = datetime.fromisoformat(resp.json()["files"][0]["upload-time"]) + assert simple_time > content_created + + # JSON API + json_resp = requests.get(urljoin(distro.base_url, "pypi/twine/json")) + assert json_resp.status_code == 200 + json_time = datetime.fromisoformat(json_resp.json()["urls"][0]["upload_time"]) + assert json_time > content_created + assert json_time == simple_time diff --git a/pulp_python/tests/functional/api/test_pypi_simple_api.py b/pulp_python/tests/functional/api/test_pypi_simple_api.py index c9e6b7dd8..7a7ff8660 100644 --- a/pulp_python/tests/functional/api/test_pypi_simple_api.py +++ b/pulp_python/tests/functional/api/test_pypi_simple_api.py @@ -5,6 +5,9 @@ from pulp_python.tests.functional.constants import ( PYPI_SERIAL_CONSTANT, + PYPI_SIMPLE_V1_HTML, + PYPI_SIMPLE_V1_JSON, + PYPI_TEXT_HTML, PYTHON_SM_FIXTURE_CHECKSUMS, PYTHON_SM_FIXTURE_RELEASES, PYTHON_SM_PROJECT_SPECIFIER, @@ -27,10 +30,6 @@ API_VERSION = "1.1" -PYPI_TEXT_HTML = "text/html" -PYPI_SIMPLE_V1_HTML = "application/vnd.pypi.simple.v1+html" -PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json" - @pytest.mark.parallel def test_simple_html_index_api( diff --git a/pulp_python/tests/functional/constants.py b/pulp_python/tests/functional/constants.py index d1b54ac2e..9cd052250 100644 --- a/pulp_python/tests/functional/constants.py +++ b/pulp_python/tests/functional/constants.py @@ -382,3 +382,7 @@ ] PYPI_SERIAL_CONSTANT = 1000000000 + +PYPI_TEXT_HTML = "text/html" +PYPI_SIMPLE_V1_HTML = "application/vnd.pypi.simple.v1+html" +PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json"