Fix constraint violation in is_latest

This is the backported version of the fix. A different strategy was
pursued.

fixes #1571

CHANGES/1571.bugfix

@@ -0,0 +1 @@
+Prevented a race condition that lead to contraint violations when calculating the highest version of a collection.

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 Q
+from django.db.models import F, Q
 from django.db.utils import IntegrityError
 from django.urls import reverse
 from django.utils.dateparse import parse_datetime
@@ -344,9 +344,8 @@ def create_collection_from_importer(importer_result, metadata_only=False):
             tag, created = Tag.objects.get_or_create(name=name)
             collection_version.tags.add(tag)
 
-        _update_highest_version(collection_version)
-
         collection_version.save()  # Save the FK updates
+        _update_highest_version(collection_version)
     return collection_version
 
 
@@ -421,53 +420,22 @@ def _get_backend_storage_url(artifact_file):
     return url
 
 
-def _update_highest_version(collection_version, save=False):
+def _update_highest_version(collection_version):
     """
     Checks if this version is greater than the most highest one.
 
-    If this version is the first version in collection, is_highest is set to True.
-    If this version is greater than the highest version in collection, set is_highest
-    equals False on the last highest version and True on this version.
-    Otherwise does nothing.
+    TODO: This function violates content immutability. It must be deprecated.
     """
 
-    def is_new_highest(new, old):
-        if bool(new.prerelease) == bool(old.prerelease):
-            return new > old
-        return bool(old.prerelease)
-
-    # did we have one set previously for this collection?
-    last_highest = collection_version.collection.versions.filter(is_highest=True).first()
-
-    if not last_highest:
-        # we only have one version, so mark it as the highest
-        if collection_version.collection.versions.count() == 1:
-            collection_version.is_highest = True
-            if save:
-                collection_version.save(update_fields=["is_highest"])
-            return
-
-        # 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 is_new_highest(sv, highest[0]):
-                highest = (sv, cv)
-
-        highest[1].is_highest = True
-        if highest[1] != collection_version or save:
-            highest[1].save()
-        return
-
-    # exit if the new CV is not higher
-    if not is_new_highest(Version(collection_version.version), Version(last_highest.version)):
-        return
-
-    last_highest.is_highest = False
-    last_highest.save(update_fields=["is_highest"])
-    collection_version.is_highest = True
-    if save:
-        collection_version.save(update_fields=["is_highest"])
+    qs = collection_version.collection.versions.annotate(prerelease=Q(version_prerelease__ne=""))
+    highest_subq = qs.order_by(
+        "prerelease", "-version_major", "-version_minor", "-version_patch", "-version_prerelease"
+    )[0:1].values("pk")
+    # Order them in such a way, that only the latest updated record will recieve true.
+    # This should prevent hitting the uniquenes constraint.
+    qs.annotate(new_is_highest=Q(pk=highest_subq)).order_by(
+        "-prerelease", "version_major", "version_minor", "version_patch", "version_prerelease"
+    ).update(is_highest=F("new_is_highest"))
 
 
 class AnsibleDeclarativeVersion(DeclarativeVersion):
@@ -1209,6 +1177,5 @@ def _post_save(self, batch):
                         continue
                     setattr(collection_version, attr_name, attr_value)
 
-                _update_highest_version(collection_version)
-
                 collection_version.save()
+                _update_highest_version(collection_version)

pulp_ansible/tests/unit/test_tasks.py

@@ -114,7 +114,7 @@ def test_is_highest_set(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1")
@@ -133,7 +133,7 @@ def test_is_highest_set_on_multple_stable_releases(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1")
@@ -158,7 +158,7 @@ def test_is_highest_set_on_single_prerelease(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc2")
@@ -178,7 +178,7 @@ def test_is_highest_set_on_multiple_prereleases(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc1")
@@ -211,7 +211,7 @@ def test_is_highest_set_on_multiple_prereleases_one_pass(self):
         ]
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
-        _update_highest_version(collection_versions[1], save=True)
+        _update_highest_version(collection_versions[1])
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc1")
@@ -246,7 +246,7 @@ def test_is_highest_set_on_multiple_prereleases_backwards_order(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions[::-1]:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1-rc1")
@@ -286,7 +286,7 @@ def test_prerelease_not_higher_than_stable(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="1.0.1")
@@ -312,7 +312,7 @@ def test_prerelease_matches_stable(self):
 
         collection_versions = build_cvs_from_specs(specs, build_artifacts=False)
         for cv in collection_versions:
-            _update_highest_version(cv, save=True)
+            _update_highest_version(cv)
 
         assert (
             CollectionVersion.objects.filter(namespace=namespace, name=name, version="2.0.0-rc1")