Change duplicate collection uploads to use Pulp retrieval logic

fixes: #1691
(cherry picked from commit 076a9bb)

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)
 
@@ -518,9 +502,31 @@ def deferred_validate(self, data):
         # repository field clashes
         collection_info["origin_repository"] = collection_info.pop("repository", None)
         data.update(collection_info)
+        # `retrieve` needs artifact, but it won't be in validated_data because of `get_artifacts`
+        self.context["artifact"] = artifact
 
         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 = self.context["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,8 @@ 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:
+    # Workaround for ColVersion's `repository` field name clashing with upload serializer's field
+    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/tests/functional/api/collection/test_crud_collection_versions.py

@@ -4,7 +4,6 @@
 from pulp_smash.utils import http_get
 
 from pulpcore.client.pulp_ansible import ContentCollectionVersionsApi
-from pulpcore.client.pulp_ansible.exceptions import ApiException
 
 
 from pulp_ansible.tests.functional.constants import (
@@ -159,9 +158,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,6 @@ 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
-
         delete_orphans()

pulp_ansible/tests/functional/api/collection/v3/test_collection.py

@@ -13,8 +13,6 @@
 from pulp_smash.pulp3.utils import gen_distribution, gen_repo
 from pulp_smash.utils import http_get
 
-from requests.exceptions import HTTPError
-
 from pulp_ansible.tests.functional.constants import (
     ANSIBLE_COLLECTION_FILE_NAME,
     ANSIBLE_COLLECTION_UPLOAD_FIXTURE_URL,
@@ -356,16 +354,10 @@ def test_collection_download(collection_artifact, pulp_client, collection_detail
 
 
 def test_collection_upload_repeat(pulp_client, collection_artifact, pulp_dist, collection_upload):
-    """Upload a duplicate collection.
-
-    Should fail, because of the conflict of collection name and version.
     """
-    with pytest.raises(HTTPError) as ctx:
-        upload_collection(pulp_client, collection_artifact.filename, pulp_dist["base_path"])
-
-    assert ctx.value.response.json()["errors"][0] == {
-        "status": "400",
-        "code": "invalid",
-        "title": "Invalid input.",
-        "detail": "Artifact already exists",
-    }
+    Upload a duplicate collection.
+    """
+    response = upload_collection(pulp_client, collection_artifact.filename, pulp_dist["base_path"])
+    assert response["error"] is None
+    assert response["state"] == "completed"
+    assert re.match(r"[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}", response["id"])