diff --git a/CHANGES/+is_highest.removal b/CHANGES/+is_highest.removal new file mode 100644 index 000000000..f5dcd6bb2 --- /dev/null +++ b/CHANGES/+is_highest.removal @@ -0,0 +1 @@ +Removed the `is_highest` attribute on CollectionVersion. diff --git a/pulp_ansible/app/models.py b/pulp_ansible/app/models.py index 960bffd5c..ce437f8e2 100644 --- a/pulp_ansible/app/models.py +++ b/pulp_ansible/app/models.py @@ -153,6 +153,7 @@ class CollectionVersion(Content): is_highest (models.BooleanField): Indicates that the version is the highest one in the collection. Import and sync workflows update this field, which then triggers the database to [re]build the search_vector. + This field is Deprecated and scheduled for removal as soon as 0.24.0. Relations: @@ -185,7 +186,8 @@ class CollectionVersion(Content): version_patch = models.IntegerField() version_prerelease = models.CharField(max_length=128) - is_highest = models.BooleanField(editable=False, default=False) + # This field is deprecated. We keep it for some releases for 0-Downtime upgrades. + _is_highest = models.BooleanField(editable=False, default=False, db_column="is_highest") # Foreign Key Fields collection = models.ForeignKey( diff --git a/pulp_ansible/app/tasks/collections.py b/pulp_ansible/app/tasks/collections.py index 4df92b508..61ce7ed96 100644 --- a/pulp_ansible/app/tasks/collections.py +++ b/pulp_ansible/app/tasks/collections.py @@ -15,7 +15,7 @@ from async_lru import alru_cache from django.conf import settings from django.db import transaction -from django.db.models import F, Q +from django.db.models import Q from django.db.utils import IntegrityError from django.urls import reverse from django.utils.dateparse import parse_datetime @@ -353,7 +353,6 @@ def create_collection_from_importer(importer_result, metadata_only=False): collection_version.tags.add(tag) collection_version.save() # Save the FK updates - _update_highest_version(collection_version) return collection_version @@ -432,34 +431,6 @@ def _get_backend_storage_url(artifact_file): return url -def _update_highest_version(collection_version): - """ - Checks if this version is greater than the most highest one. - - TODO: This function violates content immutability. It must be deprecated. - """ - - qs = collection_version.collection.versions.annotate(prerelease=Q(version_prerelease__ne="")) - highest_subq = qs.order_by("prerelease", "-version")[0:1].values("pk") - update_qs = qs.annotate(new_is_highest=Q(pk=highest_subq)) - # To avoid ordering issues, set everything to False first, then update with proper value - for i in range(2): - try: - with transaction.atomic(): - update_qs.update(is_highest=False) - update_qs.update(is_highest=F("new_is_highest")) - except IntegrityError: - log.debug(f"Retrying update_highest_version {collection_version.relative_path}") - pass - else: - return - log.warning( - _("Failed to update is_highest for {}, potentially out of date").format( - collection_version.relative_path - ) - ) - - class AnsibleDeclarativeVersion(DeclarativeVersion): """ Subclassed Declarative version creates a custom pipeline for Ansible sync. @@ -1232,6 +1203,4 @@ def _post_save(self, batch): continue setattr(collection_version, attr_name, attr_value) - collection_version.is_highest = False collection_version.save() - _update_highest_version(collection_version) diff --git a/pulp_ansible/app/tasks/upload.py b/pulp_ansible/app/tasks/upload.py index d10278905..69ea3539c 100644 --- a/pulp_ansible/app/tasks/upload.py +++ b/pulp_ansible/app/tasks/upload.py @@ -74,9 +74,6 @@ def process_collection_artifact(artifact, namespace, name, version): def finish_collection_upload(collection_version, tags, origin_repository): """After CollectionVersion has been created update its tags and latest_version.""" - # Avoid circular import - from .collections import _update_highest_version - for name in tags: tag, created = Tag.objects.get_or_create(name=name) collection_version.tags.add(tag) @@ -85,4 +82,3 @@ def finish_collection_upload(collection_version, tags, origin_repository): if origin_repository is not None and collection_version.repository != origin_repository: collection_version.repository = origin_repository collection_version.save() - _update_highest_version(collection_version) diff --git a/pulp_ansible/app/viewsets.py b/pulp_ansible/app/viewsets.py index 04f41282a..bc4c080c4 100644 --- a/pulp_ansible/app/viewsets.py +++ b/pulp_ansible/app/viewsets.py @@ -159,7 +159,6 @@ class CollectionVersionFilter(ContentFilter): namespace = filters.CharFilter(field_name="namespace") name = filters.CharFilter(field_name="name") - is_highest = filters.BooleanFilter(field_name="is_highest") q = filters.CharFilter(field_name="q", method="filter_by_q") tags = filters.CharFilter( field_name="tags", @@ -209,7 +208,7 @@ def filter_by_tags(self, qs, name, value): class Meta: model = CollectionVersion - fields = ["namespace", "name", "version", "q", "is_highest", "tags"] + fields = ["namespace", "name", "version", "q", "tags"] class CollectionRemoteFilter(RemoteFilter): diff --git a/pulp_ansible/tests/unit/test_tasks.py b/pulp_ansible/tests/unit/test_tasks.py index d1e7cedfb..85e89e5e2 100644 --- a/pulp_ansible/tests/unit/test_tasks.py +++ b/pulp_ansible/tests/unit/test_tasks.py @@ -1,8 +1,6 @@ from unittest import mock 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 CollectionVersion @@ -10,7 +8,6 @@ from pulp_ansible.app.tasks.collections import ( _rebuild_collection_version_meta, rebuild_repository_collection_versions_metadata, - _update_highest_version, ) from .utils import build_cvs_from_specs @@ -119,230 +116,3 @@ def test_reimport_repository_rebuilds_contents(self): cv2 = cobject.cast() cv2.refresh_from_db() assert cv2.contents != ["a", "b", "c"] - - -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, build_artifacts=False) - 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_multple_stable_releases(self): - namespace = randstr() - name = randstr() - specs = [ - (namespace, name, "1.0.1"), - (namespace, name, "2.0.1"), - ] - - collection_versions = build_cvs_from_specs(specs, build_artifacts=False) - 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 False - ) - - assert ( - CollectionVersion.objects.filter(namespace=namespace, name=name, version="2.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, build_artifacts=False) - 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, build_artifacts=False) - 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_one_pass(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, build_artifacts=False) - _update_highest_version(collection_versions[1]) - - 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, build_artifacts=False) - 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, build_artifacts=False) - 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, build_artifacts=False) - 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 - )