From 1e866eeeb8889feebc783bc06c8e394e226c09aa Mon Sep 17 00:00:00 2001 From: James Tanner Date: Thu, 9 Mar 2023 16:29:25 -0500 Subject: [PATCH] Do not use pre-release versions as is_highest unless no stable versions exist. fixes: #1391 Signed-off-by: James Tanner --- CHANGES/1391.bugfix | 1 + pulp_ansible/app/tasks/collections.py | 24 ++- pulp_ansible/tests/unit/test_tasks.py | 230 +++++++++++++++++++++++--- unittest_requirements.txt | 1 + 4 files changed, 230 insertions(+), 26 deletions(-) create mode 100644 CHANGES/1391.bugfix diff --git a/CHANGES/1391.bugfix b/CHANGES/1391.bugfix new file mode 100644 index 000000000..9fe709d2c --- /dev/null +++ b/CHANGES/1391.bugfix @@ -0,0 +1 @@ +Pre-release collection versions should only be higher than stable releases when none exist. diff --git a/pulp_ansible/app/tasks/collections.py b/pulp_ansible/app/tasks/collections.py index a4f917a8d..0823eca47 100644 --- a/pulp_ansible/app/tasks/collections.py +++ b/pulp_ansible/app/tasks/collections.py @@ -430,10 +430,32 @@ def _update_highest_version(collection_version): equals False on the last highest version and True on this version. Otherwise does nothing. """ + last_highest = collection_version.collection.versions.filter(is_highest=True).first() if not last_highest: - collection_version.is_highest = True + # we only have one version, so mark it as the highest + if collection_version.collection.versions.count() == 1: + collection_version.is_highest = True + collection_version.save() + return None + + # compute highest from the whole list ... + highest = None + for cv in collection_version.collection.versions.all(): + sv = Version(cv.version) + if highest is None or (highest[0] < sv and not sv.prerelease): + highest = (sv, cv) + highest[1].is_highest = True + highest[1].save() + return None + + # don't use newer pre-releases if we have a stable version as is_highest + if ( + Version(collection_version.version).prerelease + and not Version(last_highest.version).prerelease + ): return None + if Version(collection_version.version) > Version(last_highest.version): last_highest.is_highest = False collection_version.is_highest = True diff --git a/pulp_ansible/tests/unit/test_tasks.py b/pulp_ansible/tests/unit/test_tasks.py index 552c44922..a4cf5bd44 100644 --- a/pulp_ansible/tests/unit/test_tasks.py +++ b/pulp_ansible/tests/unit/test_tasks.py @@ -7,6 +7,8 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase +from orionutils.generator import randstr + from pulp_ansible.app.models import AnsibleDistribution from pulp_ansible.app.models import AnsibleRepository from pulp_ansible.app.models import Collection @@ -14,8 +16,11 @@ from pulpcore.plugin.models import Artifact from pulpcore.plugin.models import ContentArtifact -from pulp_ansible.app.tasks.collections import _rebuild_collection_version_meta -from pulp_ansible.app.tasks.collections import rebuild_repository_collection_versions_metadata +from pulp_ansible.app.tasks.collections import ( + _rebuild_collection_version_meta, + rebuild_repository_collection_versions_metadata, + _update_highest_version, +) def make_cv_tarball(namespace, name, version): @@ -37,12 +42,40 @@ def make_cv_tarball(namespace, name, version): return tarfn +def build_cvs_from_specs(specs): + """Make CVs from namespace.name.version specs.""" + collection_versions = [] + for spec in specs: + tarfn = make_cv_tarball(spec[0], spec[1], None) + rawbin = open(tarfn, "rb").read() + artifact = Artifact.objects.create( + sha224=hashlib.sha224(rawbin).hexdigest(), + sha256=hashlib.sha256(rawbin).hexdigest(), + sha384=hashlib.sha384(rawbin).hexdigest(), + sha512=hashlib.sha512(rawbin).hexdigest(), + size=os.path.getsize(tarfn), + file=SimpleUploadedFile(tarfn, rawbin), + ) + artifact.save() + + col, _ = Collection.objects.get_or_create(name=spec[0]) + col.save() + cv = CollectionVersion(collection=col, namespace=spec[0], name=spec[1], version=spec[2]) + cv.save() + ca = ContentArtifact.objects.create( + artifact=artifact, content=cv, relative_path=cv.relative_path + ) + ca.save() + collection_versions.append(cv) + + return collection_versions + + class TestCollectionReImport(TestCase): """Test Collection Re-Import.""" def setUp(self): """Make all the test data.""" - self.collection_versions = [] specs = [ ("foo", "bar", "1.0.0"), @@ -50,28 +83,7 @@ def setUp(self): ("zip", "drive", "1.0.0"), ] - for spec in specs: - tarfn = make_cv_tarball(spec[0], spec[1], None) - rawbin = open(tarfn, "rb").read() - artifact = Artifact.objects.create( - sha224=hashlib.sha224(rawbin).hexdigest(), - sha256=hashlib.sha256(rawbin).hexdigest(), - sha384=hashlib.sha384(rawbin).hexdigest(), - sha512=hashlib.sha512(rawbin).hexdigest(), - size=os.path.getsize(tarfn), - file=SimpleUploadedFile(tarfn, rawbin), - ) - artifact.save() - - col, _ = Collection.objects.get_or_create(name=spec[0]) - col.save() - cv = CollectionVersion(collection=col, namespace=spec[0], name=spec[1], version=spec[2]) - cv.save() - ca = ContentArtifact.objects.create( - artifact=artifact, content=cv, relative_path=cv.relative_path - ) - ca.save() - self.collection_versions.append(cv) + self.collection_versions = build_cvs_from_specs(specs) # create a repository self.repo = AnsibleRepository(name="foorepo") @@ -143,3 +155,171 @@ def test_reimport_repository_rebuilds_name(self, mock_progress_report, mock_rebu call_pks = sorted([str(x.args[0].pk) for x in mock_rebuild_cv.mock_calls]) expected_pks = sorted([str(x.pk) for x in expected_cvs]) assert call_pks == expected_pks + + +class TestCollectionVersionHighest(TestCase): + """Test Collection Re-Import.""" + + def test_is_highest_set(self): + namespace = randstr() + name = randstr() + specs = [ + (namespace, name, "1.0.1"), + ] + + collection_versions = build_cvs_from_specs(specs) + for cv in collection_versions: + _update_highest_version(cv) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1") + .first() + .is_highest + is True + ) + + def test_is_highest_set_on_single_prerelease(self): + namespace = randstr() + name = randstr() + specs = [ + (namespace, name, "1.0.1-rc2"), + ] + + collection_versions = build_cvs_from_specs(specs) + for cv in collection_versions: + _update_highest_version(cv) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc2") + .first() + .is_highest + is True + ) + + def test_is_highest_set_on_multiple_prereleases(self): + namespace = randstr() + name = randstr() + specs = [ + (namespace, name, "1.0.1-rc1"), + (namespace, name, "1.0.1-rc2"), + (namespace, name, "1.0.1-rc3"), + ] + + collection_versions = build_cvs_from_specs(specs) + for cv in collection_versions: + _update_highest_version(cv) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc1") + .first() + .is_highest + is False + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc2") + .first() + .is_highest + is False + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc3") + .first() + .is_highest + is True + ) + + def test_is_highest_set_on_multiple_prereleases_backwards_order(self): + namespace = randstr() + name = randstr() + specs = [ + (namespace, name, "1.0.1-rc4"), + (namespace, name, "1.0.1-rc1"), + (namespace, name, "1.0.1-rc2"), + (namespace, name, "1.0.1-rc3"), + ] + + collection_versions = build_cvs_from_specs(specs) + for cv in collection_versions[::-1]: + _update_highest_version(cv) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc1") + .first() + .is_highest + is False + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc2") + .first() + .is_highest + is False + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc3") + .first() + .is_highest + is False + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc4") + .first() + .is_highest + is True + ) + + def test_prerelease_not_higher_than_stable(self): + namespace = randstr() + name = randstr() + specs = [ + (namespace, name, "1.0.1"), + (namespace, name, "2.0.0-rc1"), + ] + + collection_versions = build_cvs_from_specs(specs) + for cv in collection_versions: + _update_highest_version(cv) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1") + .first() + .is_highest + is True + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="2.0.0-rc1") + .first() + .is_highest + is False + ) + + def test_prerelease_matches_stable(self): + namespace = randstr() + name = randstr() + specs = [ + (namespace, name, "2.0.0-rc1"), + (namespace, name, "2.0.0"), + ] + + collection_versions = build_cvs_from_specs(specs) + for cv in collection_versions: + _update_highest_version(cv) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="2.0.0-rc1") + .first() + .is_highest + is False + ) + + assert ( + CollectionVersion.objects.filter(namespace=namespace, name=name, version="2.0.0") + .first() + .is_highest + is True + ) diff --git a/unittest_requirements.txt b/unittest_requirements.txt index 25154d957..6a378f0b0 100644 --- a/unittest_requirements.txt +++ b/unittest_requirements.txt @@ -1,3 +1,4 @@ mock pulp-smash @ git+https://github.com/pulp/pulp-smash.git pytest-django +orionutils