Change duplicate collection uploads to use Pulp retrieval logic

fixes: #1691

CHANGES/1691.bugfix

@@ -0,0 +1 @@
+Duplicate Collection uploads no longer return 400s.

pulp_ansible/app/serializers.py

@@ -335,15 +335,6 @@ class CollectionOneShotSerializer(serializers.Serializer):
         default=None,
     )
 
-    def validate(self, data):
-        """Ensure duplicate artifact isn't uploaded."""
-        data = super().validate(data)
-        sha256 = data["file"].hashers["sha256"].hexdigest()
-        artifact = Artifact.objects.filter(sha256=sha256).first()
-        if artifact:
-            raise ValidationError(_("Artifact already exists"))
-        return data
-
 
 class AnsibleDistributionSerializer(DistributionSerializer):
     """
@@ -491,13 +482,6 @@ def validate(self, data):
             data["expected_name"] = collection.name
             data["expected_version"] = collection.version
 
-        if CollectionVersion.objects.filter(**{f: data[f"expected_{f}"] for f in fields}).exists():
-            raise ValidationError(
-                _("Collection {}.{}-{} already exists").format(
-                    data["expected_namespace"], data["expected_name"], data["expected_version"]
-                )
-            )
-
         # Super will call deferred_validate on second call in task context
         return super().validate(data)
 
@@ -521,6 +505,26 @@ def deferred_validate(self, data):
 
         return data
 
+    def retrieve(self, validated_data):
+        """Reuse existing CollectionVersion if provided artifact matches."""
+        namespace = validated_data["namespace"]
+        name = validated_data["name"]
+        version = validated_data["version"]
+        artifact = validated_data["artifact"]
+        # TODO switch this check to use digest when ColVersion uniqueness constraint is changed
+        col = CollectionVersion.objects.filter(
+            namespace=namespace, name=name, version=version
+        ).first()
+        if col:
+            if col._artifacts.get() != artifact:
+                raise ValidationError(
+                    _("Collection {}.{}-{} already exists with a different artifact").format(
+                        namespace, name, version
+                    )
+                )
+
+        return col
+
     def create(self, validated_data):
         """Final step in creating the CollectionVersion."""
         tags = validated_data.pop("tags")

pulp_ansible/app/tasks/upload.py

@@ -81,7 +81,7 @@ def finish_collection_upload(collection_version, tags, origin_repository):
         tag, created = Tag.objects.get_or_create(name=name)
         collection_version.tags.add(tag)
 
-    if origin_repository is not None:
+    if origin_repository is not None and collection_version.repository != origin_repository:
         collection_version.repository = origin_repository
-    collection_version.save()
+        collection_version.save()
     _update_highest_version(collection_version)

pulp_ansible/app/viewsets.py

@@ -1095,26 +1095,7 @@ def create(self, request):
         """
         Dispatch a Collection creation task.
         """
-        serializer = CollectionOneShotSerializer(data=request.data, context={"request": request})
-        serializer.is_valid(raise_exception=True)
-
-        expected_digests = {}
-        if serializer.validated_data["sha256"]:
-            expected_digests["sha256"] = serializer.validated_data["sha256"]
-        try:
-            temp_file = PulpTemporaryFile.init_and_validate(
-                serializer.validated_data["file"],
-                expected_digests=expected_digests,
-            )
-        except DigestValidationError:
-            raise serializers.ValidationError(
-                _("The provided sha256 value does not match the sha256 of the uploaded file.")
-            )
-
-        temp_file.save()
-        async_result = self._dispatch_import_collection_task(temp_file.pk)
-
-        return OperationPostponedResponse(async_result, request)
+        return CollectionVersionViewSet().create(request)
 
 
 class AnsibleDistributionViewSet(DistributionViewSet, RolesMixin):

pulp_ansible/tests/functional/api/collection/test_crud_collection_versions.py

@@ -159,9 +159,9 @@ def test_04_delete(self):
         self.assertIn(msg, exc.exception.args[0])
 
     @skip_if(bool, "content_unit", False)
-    def test_05_duplicate_raise_error(self):
+    def test_05_duplicate_no_error(self):
         """Attempt to create duplicate collection."""
         attrs = dict(namespace="pulp", name="squeezer", version="0.0.9")
-        with self.assertRaises(ApiException) as ctx:
-            self.upload_collection(**attrs)
-        self.assertIn("Collection pulp.squeezer-0.0.9 already exists", ctx.exception.body)
+        response = self.upload_collection(**attrs)
+        created_resources = monitor_task(response.task).created_resources
+        self.assertEqual(created_resources[0], self.content_unit["pulp_href"])

pulp_ansible/tests/functional/api/collection/v2/test_upload.py

@@ -8,7 +8,6 @@
 from pulpcore.client.pulp_ansible import (
     AnsibleCollectionsApi,
     ContentCollectionVersionsApi,
-    ApiException,
 )
 
 from pulp_ansible.tests.functional.utils import gen_ansible_client
@@ -53,13 +52,9 @@ def test_collection_upload(self):
         * `Pulp #5262 <https://pulp.plan.io/issues/5262>`_
         """
         response = self.upload_collection()
-
         self.assertEqual(response.sha256, self.collection_sha256, response)
 
-        with self.assertRaises(ApiException) as exc:
-            self.upload_collection()
-
-        assert exc.exception.status == 400
-        assert "Artifact already exists" in exc.exception.body
+        response2 = self.upload_collection()
+        self.assertEqual(response, response2)
 
         delete_orphans()