Fix duplicate collection upload error msg & tests

fixes: #1175

CHANGES/1175.bugfix

@@ -0,0 +1 @@
+Properly return 400 error when trying to upload a duplicate Collection.

pulp_ansible/app/galaxy/serializers.py

@@ -6,6 +6,7 @@
 from rest_framework.reverse import reverse
 from rest_framework import serializers
 
+from pulpcore.plugin.models import Artifact
 from pulp_ansible.app.models import Collection, CollectionVersion, Role
 from pulp_ansible.app.galaxy.v3.serializers import CollectionMetadataSerializer
 
@@ -187,3 +188,10 @@ class GalaxyCollectionUploadSerializer(serializers.Serializer):
     file = serializers.FileField(
         help_text=_("The file containing the Artifact binary data."), required=True
     )
+
+    def validate(self, data):
+        """Ensure duplicate artifact isn't uploaded."""
+        sha256 = data["file"].hashers["sha256"].hexdigest()
+        artifact = Artifact.objects.filter(sha256=sha256).first()
+        if artifact:
+            raise serializers.ValidationError(_("Artifact already exists"))

pulp_ansible/app/serializers.py

@@ -285,6 +285,15 @@ 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):
     """
@@ -383,7 +392,7 @@ def validate(self, data):
         sha256 = data["file"].hashers["sha256"].hexdigest()
         artifact = Artifact.objects.filter(sha256=sha256).first()
         if artifact:
-            ValidationError(_("Artifact already exists"))
+            raise ValidationError(_("Artifact already exists"))
         temp_file = PulpTemporaryFile.init_and_validate(data.pop("file"))
         temp_file.save()
         data["temp_file_pk"] = str(temp_file.pk)

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

@@ -2,10 +2,14 @@
 import hashlib
 from tempfile import NamedTemporaryFile
 
-from pulp_smash.pulp3.bindings import delete_orphans, monitor_task, PulpTestCase, PulpTaskError
+from pulp_smash.pulp3.bindings import delete_orphans, monitor_task, PulpTestCase
 from pulp_smash.utils import http_get
 
-from pulpcore.client.pulp_ansible import AnsibleCollectionsApi, ContentCollectionVersionsApi
+from pulpcore.client.pulp_ansible import (
+    AnsibleCollectionsApi,
+    ContentCollectionVersionsApi,
+    ApiException,
+)
 
 from pulp_ansible.tests.functional.utils import gen_ansible_client
 from pulp_ansible.tests.functional.utils import set_up_module as setUpModule  # noqa:F401
@@ -51,13 +55,10 @@ def test_collection_upload(self):
 
         self.assertEqual(response.sha256, self.collection_sha256, response)
 
-        with self.assertRaises(PulpTaskError) as exc:
+        with self.assertRaises(ApiException) as exc:
             self.upload_collection()
 
-        task_result = exc.exception.task.to_dict()
-        self.assertEqual(task_result["state"], "failed")
-        error = task_result["error"]
-        for key in ("artifact", "already", "exists"):
-            self.assertIn(key, task_result["error"]["description"].lower(), error)
+        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

@@ -20,7 +20,6 @@
     ANSIBLE_COLLECTION_UPLOAD_FIXTURE_URL,
     ANSIBLE_DISTRIBUTION_PATH,
     ANSIBLE_REPO_PATH,
-    COLLECTION_METADATA,
 )
 from pulp_ansible.tests.functional.utils import set_up_module as setUpModule  # noqa:F401
 
@@ -87,15 +86,19 @@ def get_metadata_published(pulp_client, pulp_dist):
     return datetime.strptime(metadata["published"], "%Y-%m-%dT%H:%M:%S.%fZ")
 
 
+def upload_collection(client, filename, base_path):
+    """Helper to upload collections to pulp_ansible/galaxy."""
+    UPLOAD_PATH = get_galaxy_url(base_path, "/v3/artifacts/collections/")
+    collection = {"file": (open(filename, "rb"))}
+
+    return client.using_handler(upload_handler).post(UPLOAD_PATH, files=collection)
+
+
 @pytest.fixture(scope="session")
 def collection_upload(pulp_client, artifact, pulp_dist):
     """Publish a new collection and return the processed response data."""
-    UPLOAD_PATH = get_galaxy_url(pulp_dist["base_path"], "/v3/artifacts/collections/")
     published_before_upload = get_metadata_published(pulp_client, pulp_dist)
-    logging.info(f"Uploading collection to '{UPLOAD_PATH}'...")
-    collection = {"file": (ANSIBLE_COLLECTION_FILE_NAME, open(artifact.filename, "rb"))}
-
-    response = pulp_client.using_handler(upload_handler).post(UPLOAD_PATH, files=collection)
+    response = upload_collection(pulp_client, artifact.filename, pulp_dist["base_path"])
     published_after_upload = get_metadata_published(pulp_client, pulp_dist)
     assert published_after_upload > published_before_upload
     return response
@@ -104,12 +107,8 @@ def collection_upload(pulp_client, artifact, pulp_dist):
 @pytest.fixture(scope="session")
 def collection_upload2(pulp_client, artifact2, pulp_dist):
     """Publish the second new collection and return the processed response data."""
-    UPLOAD_PATH = get_galaxy_url(pulp_dist["base_path"], "/v3/artifacts/collections/")
     published_before_upload = get_metadata_published(pulp_client, pulp_dist)
-    logging.info(f"Uploading collection to '{UPLOAD_PATH}'...")
-    collection = {"file": (open(artifact2.filename, "rb"))}
-
-    response = pulp_client.using_handler(upload_handler).post(UPLOAD_PATH, files=collection)
+    response = upload_collection(pulp_client, artifact2.filename, pulp_dist["base_path"])
     published_after_upload = get_metadata_published(pulp_client, pulp_dist)
     assert published_after_upload > published_before_upload
     return response
@@ -196,9 +195,10 @@ def test_collection_upload(collection_upload):
     assert "finished_at" in collection_upload
     assert "messages" in collection_upload
 
-    for key, value in collection_upload.items():
-        if key in COLLECTION_METADATA.keys():
-            assert COLLECTION_METADATA[key] == value, collection_upload
+    # TODO: Add this back when namespace, name, and version are apart of the CollectionImport
+    # for key, value in collection_upload.items():
+    #     if key in COLLECTION_METADATA.keys():
+    #         assert COLLECTION_METADATA[key] == value, collection_upload
 
 
 def test_collection_list(
@@ -266,7 +266,7 @@ def test_collection_version(artifact, pulp_client, collection_detail):
     assert version["artifact"]["sha256"] == hashlib.sha256(tarball).hexdigest()
     assert version["artifact"]["size"] == len(tarball)
 
-    # assert version['artifact']['filename'] == artifact.filename
+    assert version["artifact"]["filename"] == artifact.filename.strip("/tmp/")
 
     assert "updated_at" in version
     assert "created_at" in version
@@ -309,27 +309,17 @@ def test_collection_download(artifact, pulp_client, collection_detail):
     assert f.content == tarball
 
 
-def test_collection_upload_repeat(pulp_client, known_collection, pulp_dist):
+def test_collection_upload_repeat(pulp_client, artifact, pulp_dist, collection_upload):
     """Upload a duplicate collection.
 
     Should fail, because of the conflict of collection name and version.
     """
-    cfg = config.get_config()
-    url = urljoin(cfg.get_base_url(), f"api/{pulp_dist['base_path']}/v3/artifacts/collections/")
-
     with pytest.raises(HTTPError) as ctx:
-        response = pulp_client.post(url, files=known_collection)
-
-        assert ctx.exception.response.json()["errors"][0] == {
-            "status": "400",
-            "code": "invalid",
-            "title": "Invalid input.",
-            "detail": "Artifact already exists.",
-        }
+        upload_collection(pulp_client, artifact.filename, pulp_dist["base_path"])
 
-        for key, value in collection_upload.items():
-            if key in COLLECTION_METADATA.keys():
-                assert COLLECTION_METADATA[key] == value, response
-
-        collection_sha256 = hashlib.sha256(known_collection["files"][1]).hexdigest()
-        assert response["sha256"] == collection_sha256, response
+    assert ctx.value.response.json()["errors"][0] == {
+        "status": "400",
+        "code": "invalid",
+        "title": "Invalid input.",
+        "detail": "Artifact already exists",
+    }