Remove CollectionVersion is_highest

pulp · Oct 8, 2024 · e4d6a55 · e4d6a55
1 parent 4c57cc0
commit e4d6a55
Show file tree

Hide file tree

Showing 6 changed files with 6 additions and 269 deletions.
diff --git a/CHANGES/+is_highest.removal 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,16 +1,13 @@
 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
 
 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
-        )