diff --git a/swh/deposit/api/common.py b/swh/deposit/api/common.py --- a/swh/deposit/api/common.py +++ b/swh/deposit/api/common.py @@ -64,6 +64,7 @@ make_error_dict, make_error_response, make_error_response_from_dict, + make_missing_slug_error, ) from ..models import DepositClient, DepositCollection, DepositRequest from ..parsers import parse_swh_reference, parse_xml @@ -195,15 +196,14 @@ ) .order_by("-id")[0:1] .get() - ) # noqa + ) except Deposit.DoesNotExist: # then no parent for that deposit, deposit_parent already None pass - assert external_id is not None deposit = Deposit( collection=self._collection, - external_id=external_id, + external_id=external_id or "", complete_date=complete_date, status=status_type, client=self._client, @@ -383,6 +383,7 @@ deposit_id: Optional[int] = None, replace_metadata: bool = False, replace_archives: bool = False, + check_slug_is_present: bool = False, ) -> Dict[str, Any]: """Binary upload routine. @@ -401,6 +402,8 @@ deposit. If False (default), this adds new archive request to existing ones. Otherwise, this will replace existing archives. ones. + check_slug_is_present: Check for the slug header if True and raise + if not present Returns: In the optimal case a dict with the following keys: @@ -453,7 +456,9 @@ if precondition_status_response: return precondition_status_response - external_id = headers["slug"] + slug = headers.get("slug") + if check_slug_is_present and not slug: + return make_missing_slug_error() # actual storage of data archive_metadata = filehandler @@ -461,7 +466,7 @@ request, deposit_id=deposit_id, in_progress=headers["in-progress"], - external_id=external_id, + external_id=slug, ) self._deposit_request_put( deposit, @@ -494,6 +499,7 @@ deposit_id: Optional[int] = None, replace_metadata: bool = False, replace_archives: bool = False, + check_slug_is_present: bool = False, ) -> Dict: """Multipart upload supported with exactly: - 1 archive (zip) @@ -514,6 +520,8 @@ deposit. If False (default), this adds new archive request to existing ones. Otherwise, this will replace existing archives. ones. + check_slug_is_present: Check for the slug header if True and raise + if not present Returns: In the optimal case a dict with the following keys: @@ -533,7 +541,9 @@ - 415 (unsupported media type) if a wrong media type is provided """ - external_id = headers["slug"] + slug = headers.get("slug") + if check_slug_is_present and not slug: + return make_missing_slug_error() content_types_present = set() @@ -597,7 +607,7 @@ request, deposit_id=deposit_id, in_progress=headers["in-progress"], - external_id=external_id, + external_id=slug, ) deposit_request_data = { ARCHIVE_KEY: filehandler, @@ -716,6 +726,7 @@ deposit_id: Optional[int] = None, replace_metadata: bool = False, replace_archives: bool = False, + check_slug_is_present: bool = False, ) -> Dict[str, Any]: """Atom entry deposit. @@ -732,6 +743,8 @@ deposit. If False (default), this adds new archive request to existing ones. Otherwise, this will replace existing archives. ones. + check_slug_is_present: Check for the slug header if True and raise + if not present Returns: In the optimal case a dict with the following keys: @@ -773,7 +786,14 @@ except ValidationError as e: return make_error_dict(PARSING_ERROR, "Invalid SWHID reference", str(e),) - external_id = metadata.get("external_identifier", headers["slug"]) + if swhid is not None: + external_id = metadata.get("external_identifier", headers.get("slug")) + else: + slug = headers.get("slug") + if check_slug_is_present and not slug: + return make_missing_slug_error() + + external_id = metadata.get("external_identifier", slug) deposit = self._deposit_put( request, diff --git a/swh/deposit/api/deposit.py b/swh/deposit/api/deposit.py --- a/swh/deposit/api/deposit.py +++ b/swh/deposit/api/deposit.py @@ -8,7 +8,6 @@ from rest_framework import status from ..config import EDIT_SE_IRI -from ..errors import BAD_REQUEST, make_error_dict from ..parsers import ( SWHAtomEntryParser, SWHFileUploadTarParser, @@ -34,21 +33,6 @@ SWHAtomEntryParser, ) - def additional_checks( - self, - req, - headers: Dict[str, Any], - collection_name: str, - deposit_id: Optional[int] = None, - ) -> Dict[str, Any]: - slug = headers["slug"] - if not slug: - msg = "Missing SLUG header in request" - verbose_description = "Provide in the SLUG header one identifier, for example the url pointing to the resource you are depositing." # noqa - return make_error_dict(BAD_REQUEST, msg, verbose_description) - - return {} - def process_post( self, req, @@ -103,10 +87,16 @@ """ assert deposit_id is None if req.content_type in ACCEPT_ARCHIVE_CONTENT_TYPES: - data = self._binary_upload(req, headers, collection_name) + data = self._binary_upload( + req, headers, collection_name, check_slug_is_present=True + ) elif req.content_type.startswith("multipart/"): - data = self._multipart_upload(req, headers, collection_name) + data = self._multipart_upload( + req, headers, collection_name, check_slug_is_present=True + ) else: - data = self._atom_entry(req, headers, collection_name) + data = self._atom_entry( + req, headers, collection_name, check_slug_is_present=True + ) return status.HTTP_201_CREATED, EDIT_SE_IRI, data diff --git a/swh/deposit/errors.py b/swh/deposit/errors.py --- a/swh/deposit/errors.py +++ b/swh/deposit/errors.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2019 The Software Heritage developers +# Copyright (C) 2017-2020 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -7,6 +7,8 @@ """ +from typing import Any, Dict + from django.shortcuts import render from rest_framework import status @@ -150,6 +152,20 @@ return make_error_response_from_dict(req, error["error"]) +def make_missing_slug_error() -> Dict[str, Any]: + """Returns a missing slug header error dict + + """ + return make_error_dict( + BAD_REQUEST, + "Missing SLUG header", + verbose_description=( + "Provide in the SLUG header one identifier, for example the " + "url pointing to the resource you are depositing." + ), + ) + + class BadRequestError(ValueError): """Represents a bad input from the deposit client diff --git a/swh/deposit/tests/api/test_deposit_metadata.py b/swh/deposit/tests/api/test_deposit_metadata.py --- a/swh/deposit/tests/api/test_deposit_metadata.py +++ b/swh/deposit/tests/api/test_deposit_metadata.py @@ -38,7 +38,6 @@ reverse(COL_IRI, args=[deposit_collection.name]), content_type="application/atom+xml;type=entry", data=xml_data, - HTTP_SLUG="external-id", ) assert response.status_code == status.HTTP_400_BAD_REQUEST assert b"Invalid SWHID reference" in response.content @@ -59,7 +58,6 @@ reverse(COL_IRI, args=[deposit_collection.name]), content_type="application/atom+xml;type=entry", data=invalid_xml_data, - HTTP_SLUG="external-id", ) assert response.status_code == status.HTTP_400_BAD_REQUEST assert b"Functional metadata checks failure" in response.content @@ -131,7 +129,6 @@ reverse(COL_IRI, args=[deposit_collection.name]), content_type="application/atom+xml;type=entry", data=xml_data, - HTTP_SLUG="external-id", ) assert response.status_code == status.HTTP_201_CREATED @@ -213,7 +210,6 @@ reverse(COL_IRI, args=[deposit_collection.name]), content_type="application/atom+xml;type=entry", data=xml_data, - HTTP_SLUG="external-id", ) assert response.status_code == status.HTTP_201_CREATED