diff --git a/CHANGES/1381.bugfix b/CHANGES/1381.bugfix new file mode 100644 index 000000000..20ee93d9a --- /dev/null +++ b/CHANGES/1381.bugfix @@ -0,0 +1 @@ +RSS feed GUIDs are now stable when additional platform wheels arrive for the same release. GUIDs only change when a new PEP 427 build tag is introduced, preventing feed readers from showing duplicate entries for multi-architecture builds. diff --git a/pulp_python/app/pypi/feeds.py b/pulp_python/app/pypi/feeds.py index 4be42e7a5..832555505 100644 --- a/pulp_python/app/pypi/feeds.py +++ b/pulp_python/app/pypi/feeds.py @@ -2,6 +2,7 @@ from email.utils import getaddresses from urllib.parse import urljoin +from django.contrib.postgres.aggregates import ArrayAgg from django.db.models import F, FilteredRelation, Max, Min, Q from django.http.response import HttpResponse, HttpResponseNotFound from django.utils.decorators import method_decorator @@ -15,6 +16,8 @@ from pulp_python.app.cache import PythonApiCache, find_base_path_cached from pulp_python.app.pypi.views import PyPIMixin, _etag_func +_WHEEL_BUILD_TAG_RE = re.compile(r"^.+?-.+?-(?P\d[^-]*?)-[^-]+-[^-]+-[^-]+\.whl$") + UPDATES_LIMIT = 500 PACKAGES_LIMIT = 40 PROJECT_RELEASES_LIMIT = 40 @@ -70,6 +73,7 @@ def iter_releases(content, repo_ver, name_normalized=None, limit=UPDATES_LIMIT): name=Min("name"), summary=Min("summary"), author_email=Min("author_email"), + filenames=ArrayAgg("filename", distinct=True, ordering="filename"), ) .order_by("-added_at", "name_normalized", "version")[:limit] ) @@ -91,14 +95,30 @@ def iter_projects(content, repo_ver, limit=PACKAGES_LIMIT): ) -def _item_dict(title, link, description, author_email, pubdate): +def _build_tag_fragment(filenames): + """Extract sorted distinct build tags from wheel filenames for GUID stability. + + Returns a fragment like ``#builds=1,2`` when build tags are present, + or an empty string for sdists and wheels without build tags. + """ + tags = set() + for fn in filenames or (): + m = _WHEEL_BUILD_TAG_RE.match(fn) + if m: + tags.add(m.group("build")) + if not tags: + return "" + return "#builds=" + ",".join(sorted(tags)) + + +def _item_dict(title, link, description, author_email, pubdate, filenames=()): return { "title": sanitize_xml_text(title), "link": link, "description": sanitize_xml_text(description), "author_email": format_author(author_email), "pubdate": pubdate, - "unique_id": f"{link}#{pubdate.isoformat()}", + "unique_id": f"{link}{_build_tag_fragment(filenames)}", } @@ -139,6 +159,7 @@ def render_updates_feed(index_url, releases): description=release["summary"], author_email=release["author_email"], pubdate=release["added_at"], + filenames=release.get("filenames", ()), ) for release in releases ] @@ -178,6 +199,7 @@ def render_project_releases_feed(index_url, project_name, releases): description=release["summary"], author_email=release["author_email"], pubdate=release["added_at"], + filenames=release.get("filenames", ()), ) for release in releases ] diff --git a/pulp_python/tests/functional/api/test_pypi_feeds.py b/pulp_python/tests/functional/api/test_pypi_feeds.py index 63786d774..e7e11763b 100644 --- a/pulp_python/tests/functional/api/test_pypi_feeds.py +++ b/pulp_python/tests/functional/api/test_pypi_feeds.py @@ -1,3 +1,5 @@ +import io +import zipfile from urllib.parse import urljoin, urlsplit from xml.etree import ElementTree as ET @@ -20,6 +22,23 @@ TWINE_500_WHEEL_URL = urljoin(urljoin(PYTHON_FIXTURES_URL, "packages/"), TWINE_500_WHEEL_FILENAME) +def _make_build_tagged_wheel(tmp_path, base_wheel_bytes, build_tag, platform="linux_x86_64"): + """Repackage a wheel with a PEP 427 build tag and platform in the filename.""" + filename = f"shelf_reader-0.1-{build_tag}-py2-none-{platform}.whl" + path = tmp_path / filename + with zipfile.ZipFile(io.BytesIO(base_wheel_bytes)) as src, zipfile.ZipFile(path, "w") as dst: + for item in src.infolist(): + dst.writestr(item, src.read(item.filename)) + # Pulp deduplicates content by sha256, so identical zip bytes with + # different filenames map to the same content unit. Inject a unique + # marker so each (build_tag, platform) combination gets its own sha256. + dst.writestr( + "shelf_reader/.build_marker", + f"build={build_tag} platform={platform}\n", + ) + return str(path), filename + + def _index_url(distro, bindings_cfg): """Build the index URL using the same origin the API client uses.""" path = urlsplit(distro.base_url).path @@ -125,7 +144,7 @@ def test_pinned_version_feeds( item = _parse_items(_get_feed(distro, "rss/updates.xml", bindings_cfg))[0] assert item.findtext("link").endswith("pypi/shelf-reader/0.1/json") - assert "pypi/shelf-reader/0.1/json#" in item.findtext("guid") + assert item.findtext("guid").endswith("pypi/shelf-reader/0.1/json") python_content_factory(TWINE_WHEEL_FILENAME, url=TWINE_WHEEL_URL, repository=repo) update_titles = _titles(_parse_items(_get_feed(distro, "rss/updates.xml", bindings_cfg))) @@ -133,10 +152,10 @@ def test_pinned_version_feeds( @pytest.mark.parallel -def test_new_file_for_existing_version_updates_guid( +def test_guid_stable_when_adding_file_without_new_build_tag( bindings_cfg, python_content_factory, python_empty_repo_distro ): - """Adding a new file for an existing (name, version) produces a new guid and updated date.""" + """Adding a file without a new build tag keeps the GUID stable but updates pubDate.""" repo, distro = python_empty_repo_distro() python_content_factory(PYTHON_EGG_FILENAME, url=PYTHON_EGG_URL, repository=repo) @@ -145,7 +164,7 @@ def test_new_file_for_existing_version_updates_guid( assert len(items) == 1 first_guid = items[0].findtext("guid") first_date = items[0].findtext("pubDate") - assert "pypi/shelf-reader/0.1/json" in first_guid + assert first_guid.endswith("pypi/shelf-reader/0.1/json") python_content_factory(PYTHON_WHEEL_FILENAME, url=PYTHON_WHEEL_URL, repository=repo) @@ -154,11 +173,60 @@ def test_new_file_for_existing_version_updates_guid( second_guid = items[0].findtext("guid") second_date = items[0].findtext("pubDate") - assert second_guid != first_guid + assert second_guid == first_guid assert second_date >= first_date release_items = _parse_items( _get_feed(distro, "rss/project/shelf-reader/releases.xml", bindings_cfg) ) assert len(release_items) == 1 - assert release_items[0].findtext("guid") == second_guid + assert release_items[0].findtext("guid") == first_guid + + +@pytest.mark.parallel +def test_new_build_tag_changes_guid( + tmp_path, bindings_cfg, python_bindings, monitor_task, python_empty_repo_distro +): + """A new build tag produces a new GUID; a new arch for the same tag does not.""" + repo, distro = python_empty_repo_distro() + + base_wheel = requests.get(PYTHON_WHEEL_URL, timeout=30).content + + # Upload build tag 1 (x86_64) + path_1, name_1 = _make_build_tagged_wheel(tmp_path, base_wheel, "1") + task = python_bindings.ContentPackagesApi.create( + relative_path=name_1, file=path_1, repository=repo.pulp_href + ).task + monitor_task(task) + + items = _parse_items(_get_feed(distro, "rss/updates.xml", bindings_cfg)) + assert len(items) == 1 + guid_after_build1 = items[0].findtext("guid") + assert "#builds=1" in guid_after_build1 + + # Upload build tag 2 (x86_64) -- different build, GUID must change + path_2, name_2 = _make_build_tagged_wheel(tmp_path, base_wheel, "2") + task = python_bindings.ContentPackagesApi.create( + relative_path=name_2, file=path_2, repository=repo.pulp_href + ).task + monitor_task(task) + + items = _parse_items(_get_feed(distro, "rss/updates.xml", bindings_cfg)) + assert len(items) == 1 + guid_after_build2 = items[0].findtext("guid") + assert guid_after_build2 != guid_after_build1 + assert "#builds=1,2" in guid_after_build2 + + # Upload build tag 2 again with a different arch -- same build tag, GUID must stay + path_2_arm, name_2_arm = _make_build_tagged_wheel( + tmp_path, base_wheel, "2", platform="linux_aarch64" + ) + task = python_bindings.ContentPackagesApi.create( + relative_path=name_2_arm, file=path_2_arm, repository=repo.pulp_href + ).task + monitor_task(task) + + items = _parse_items(_get_feed(distro, "rss/updates.xml", bindings_cfg)) + assert len(items) == 1 + guid_after_build2_arm = items[0].findtext("guid") + assert guid_after_build2_arm == guid_after_build2 diff --git a/pulp_python/tests/unit/test_feeds.py b/pulp_python/tests/unit/test_feeds.py new file mode 100644 index 000000000..e31396cad --- /dev/null +++ b/pulp_python/tests/unit/test_feeds.py @@ -0,0 +1,67 @@ +import re + +import pytest + +# Duplicated here to avoid importing feeds.py, which pulls in Django/DRF and +# requires a configured Django settings module that the unit test runner lacks. +_WHEEL_BUILD_TAG_RE = re.compile(r"^.+?-.+?-(?P\d[^-]*?)-[^-]+-[^-]+-[^-]+\.whl$") + + +def _build_tag_fragment(filenames): + tags = set() + for fn in filenames or (): + m = _WHEEL_BUILD_TAG_RE.match(fn) + if m: + tags.add(m.group("build")) + if not tags: + return "" + return "#builds=" + ",".join(sorted(tags)) + + +@pytest.mark.parametrize( + "filenames, expected", + [ + ([], ""), + (["shelf-reader-0.1.tar.gz"], ""), + (["shelf_reader-0.1-py2-none-any.whl"], ""), + ( + ["docling_parse-7.19.1-1-cp312-cp312-linux_x86_64.whl"], + "#builds=1", + ), + ( + [ + "docling_parse-7.19.1-1-cp312-cp312-linux_x86_64.whl", + "docling_parse-7.19.1-1-cp312-cp312-linux_aarch64.whl", + "docling_parse-7.19.1-1-cp312-cp312-linux_ppc64le.whl", + ], + "#builds=1", + ), + ( + [ + "ctranslate2-4.5.0-1-cp312-cp312-linux_x86_64.whl", + "ctranslate2-4.5.0-2-cp312-cp312-linux_x86_64.whl", + ], + "#builds=1,2", + ), + ( + [ + "foo-1.0-1-cp312-cp312-linux_x86_64.whl", + "foo-1.0.tar.gz", + ], + "#builds=1", + ), + (None, ""), + ], + ids=[ + "empty", + "sdist-only", + "wheel-no-build-tag", + "single-build-tag", + "same-build-tag-multi-arch", + "two-build-tags", + "mixed-sdist-and-tagged-wheel", + "none", + ], +) +def test_build_tag_fragment(filenames, expected): + assert _build_tag_fragment(filenames) == expected