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/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
@@ -496,6 +496,10 @@ def create(self, request, distro_base_path):
             raise serializers.ValidationError(
                 _("The provided sha256 value does not match the sha256 of the uploaded file.")
             )
+        sha256 = serializer.validated_data["file"].hashers["sha256"].hexdigest()
+        artifact = Artifact.objects.filter(sha256=sha256).first()
+        if artifact:
+            raise serializers.ValidationError(_("Artifact already exists"))
 
         temp_file.save()
 

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:
     """
@@ -180,6 +182,11 @@ def post(self, request, path):
         )
         serializer.is_valid(raise_exception=True)
 
+        sha256 = serializer.validated_data["file"].hashers["sha256"].hexdigest()
+        artifact = Artifact.objects.filter(sha256=sha256).first()
+        if artifact:
+            raise serializers.ValidationError(_("Artifact already exists"))
+
         temp_file = PulpTemporaryFile.init_and_validate(serializer.validated_data["file"])
         temp_file.save()
 

pulp_ansible/app/serializers.py

@@ -383,7 +383,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
 
@@ -86,16 +85,18 @@ 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 +105,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 +193,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 +264,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 +307,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",
+    }