Fix duplicate collection upload error msg & tests

fixes: pulp#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/galaxy/v3/views.py

@@ -27,7 +27,7 @@
 from rest_framework import status
 
 from pulpcore.plugin.exceptions import DigestValidationError
-from pulpcore.plugin.models import PulpTemporaryFile, Content
+from pulpcore.plugin.models import PulpTemporaryFile, Artifact, Content
 from pulpcore.plugin.serializers import AsyncOperationResponseSerializer
 from pulpcore.plugin.viewsets import BaseFilterSet, OperationPostponedResponse
 from pulpcore.plugin.tasking import add_and_remove, dispatch

pulp_ansible/app/galaxy/views.py

@@ -2,11 +2,11 @@
 
 from django.conf import settings
 from django.shortcuts import get_object_or_404, HttpResponse
-from rest_framework import generics, pagination, response, views
+from rest_framework import generics, pagination, response, views, serializers
 
 from pulpcore.plugin.models import PulpTemporaryFile
 from pulpcore.plugin.viewsets import OperationPostponedResponse
-from pulpcore.plugin.models import ContentArtifact
+from pulpcore.plugin.models import ContentArtifact, Artifact
 
 from pulp_ansible.app.galaxy.mixins import UploadGalaxyCollectionMixin
 from pulp_ansible.app.models import AnsibleDistribution, Collection, CollectionVersion, Role
@@ -19,6 +19,8 @@
     GalaxyRoleVersionSerializer,
 )
 
+from gettext import gettext as _
+
 
 class DistributionMixin:
     """

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/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",
+    }