From d7d9b9132b2bad81b784061830eefa3f13c979ea Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Wed, 17 Aug 2022 18:07:18 -0400 Subject: [PATCH] Fix duplicate collection upload error msg & tests fixes: #1175 --- CHANGES/1175.bugfix | 1 + pulp_ansible/app/galaxy/v3/views.py | 6 +- pulp_ansible/app/galaxy/views.py | 11 +++- pulp_ansible/app/serializers.py | 2 +- .../api/collection/v3/test_collection.py | 56 ++++++++----------- 5 files changed, 39 insertions(+), 37 deletions(-) create mode 100644 CHANGES/1175.bugfix diff --git a/CHANGES/1175.bugfix b/CHANGES/1175.bugfix new file mode 100644 index 000000000..20055fc36 --- /dev/null +++ b/CHANGES/1175.bugfix @@ -0,0 +1 @@ +Properly return 400 error when trying to upload a duplicate Collection. diff --git a/pulp_ansible/app/galaxy/v3/views.py b/pulp_ansible/app/galaxy/v3/views.py index 18b0d2097..8d3aa4c5d 100644 --- a/pulp_ansible/app/galaxy/v3/views.py +++ b/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() diff --git a/pulp_ansible/app/galaxy/views.py b/pulp_ansible/app/galaxy/views.py index 3148ecadc..ac64faed2 100644 --- a/pulp_ansible/app/galaxy/views.py +++ b/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() diff --git a/pulp_ansible/app/serializers.py b/pulp_ansible/app/serializers.py index 026b7a369..8d8f8ff37 100644 --- a/pulp_ansible/app/serializers.py +++ b/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) diff --git a/pulp_ansible/tests/functional/api/collection/v3/test_collection.py b/pulp_ansible/tests/functional/api/collection/v3/test_collection.py index 1ba00b77d..3ba7bc64e 100644 --- a/pulp_ansible/tests/functional/api/collection/v3/test_collection.py +++ b/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", + }