Page MenuHomeSoftware Heritage
Files F7123955

D4047.diff
No OneTemporary
Actions

D4047.diff
View Options

diff --git a/MANIFEST.in b/MANIFEST.in
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -4,6 +4,7 @@
recursive-include swh/deposit/static *
recursive-include swh/deposit/fixtures *
recursive-include swh/deposit/templates *
+recursive-include swh/deposit/tests/data *
recursive-include swh/deposit/tests/*/data *
recursive-include swh py.typed
include tox.ini
diff --git a/swh/deposit/api/checks.py b/swh/deposit/api/checks.py
new file mode 100644
--- /dev/null
+++ b/swh/deposit/api/checks.py
@@ -0,0 +1,54 @@
+# 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
+
+from typing import Dict, Optional, Tuple
+
+MANDATORY_FIELDS_MISSING = "Mandatory fields are missing"
+ALTERNATE_FIELDS_MISSING = "Mandatory alternate fields are missing"
+
+
+def check_metadata(metadata: Dict) -> Tuple[bool, Optional[Dict]]:
+ """Check metadata for mandatory field presence.
+
+ Args:
+ metadata: Metadata dictionary to check for mandatory fields
+
+ Returns:
+ tuple (status, error_detail): True, None if metadata are
+ ok (False, <detailed-error>) otherwise.
+
+ """
+ required_fields = {
+ "author": False,
+ }
+ alternate_fields = {
+ ("name", "title"): False, # alternate field, at least one
+ # of them must be present
+ }
+
+ for field, value in metadata.items():
+ for name in required_fields:
+ if name in field:
+ required_fields[name] = True
+
+ for possible_names in alternate_fields:
+ for possible_name in possible_names:
+ if possible_name in field:
+ alternate_fields[possible_names] = True
+ continue
+
+ mandatory_result = [k for k, v in required_fields.items() if not v]
+ optional_result = [" or ".join(k) for k, v in alternate_fields.items() if not v]
+
+ if mandatory_result == [] and optional_result == []:
+ return True, None
+ detail = []
+ if mandatory_result != []:
+ detail.append({"summary": MANDATORY_FIELDS_MISSING, "fields": mandatory_result})
+ if optional_result != []:
+ detail.append(
+ {"summary": ALTERNATE_FIELDS_MISSING, "fields": optional_result,}
+ )
+ return False, {"metadata": detail}
diff --git a/swh/deposit/api/deposit_update.py b/swh/deposit/api/deposit_update.py
--- a/swh/deposit/api/deposit_update.py
+++ b/swh/deposit/api/deposit_update.py
@@ -8,6 +8,8 @@
from rest_framework import status
from rest_framework.request import Request
+from swh.deposit.api.checks import check_metadata
+from swh.deposit.api.converters import convert_status_detail
from swh.deposit.models import Deposit
from swh.model.hashutil import hash_to_bytes
from swh.model.identifiers import parse_swhid
@@ -228,6 +230,15 @@
"If the body is empty, there is no metadata.",
)
+ metadata_ok, error_details = check_metadata(metadata)
+ if not metadata_ok:
+ assert error_details, "Details should be set when a failure occurs"
+ return make_error_dict(
+ BAD_REQUEST,
+ "Functional metadata checks failure",
+ convert_status_detail(error_details),
+ )
+
metadata_authority = MetadataAuthority(
type=MetadataAuthorityType.DEPOSIT_CLIENT,
url=deposit.client.provider_url,
diff --git a/swh/deposit/api/private/deposit_check.py b/swh/deposit/api/private/deposit_check.py
--- a/swh/deposit/api/private/deposit_check.py
+++ b/swh/deposit/api/private/deposit_check.py
@@ -17,10 +17,9 @@
from . import APIPrivateView, DepositReadMixin
from ...config import ARCHIVE_TYPE, DEPOSIT_STATUS_REJECTED, DEPOSIT_STATUS_VERIFIED
from ...models import Deposit, DepositRequest
+from ..checks import check_metadata
from ..common import APIGet
-MANDATORY_FIELDS_MISSING = "Mandatory fields are missing"
-ALTERNATE_FIELDS_MISSING = "Mandatory alternate fields are missing"
MANDATORY_ARCHIVE_UNREADABLE = (
"At least one of its associated archives is not readable" # noqa
)
@@ -130,52 +129,6 @@
return False, MANDATORY_ARCHIVE_INVALID
return True, None
- def _check_metadata(self, metadata: Dict) -> Tuple[bool, Optional[Dict]]:
- """Check to execute on all metadata for mandatory field presence.
-
- Args:
- metadata (dict): Metadata dictionary to check for mandatory fields
-
- Returns:
- tuple (status, error_detail): True, None if metadata are
- ok (False, <detailed-error>) otherwise.
-
- """
- required_fields = {
- "author": False,
- }
- alternate_fields = {
- ("name", "title"): False, # alternate field, at least one
- # of them must be present
- }
-
- for field, value in metadata.items():
- for name in required_fields:
- if name in field:
- required_fields[name] = True
-
- for possible_names in alternate_fields:
- for possible_name in possible_names:
- if possible_name in field:
- alternate_fields[possible_names] = True
- continue
-
- mandatory_result = [k for k, v in required_fields.items() if not v]
- optional_result = [" or ".join(k) for k, v in alternate_fields.items() if not v]
-
- if mandatory_result == [] and optional_result == []:
- return True, None
- detail = []
- if mandatory_result != []:
- detail.append(
- {"summary": MANDATORY_FIELDS_MISSING, "fields": mandatory_result}
- )
- if optional_result != []:
- detail.append(
- {"summary": ALTERNATE_FIELDS_MISSING, "fields": optional_result,}
- )
- return False, {"metadata": detail}
-
def process_get(
self, req, collection_name: str, deposit_id: int
) -> Tuple[int, Dict, str]:
@@ -201,7 +154,7 @@
assert error_detail is not None
problems.update(error_detail)
- metadata_status, error_detail = self._check_metadata(metadata)
+ metadata_status, error_detail = check_metadata(metadata)
if not metadata_status:
assert error_detail is not None
problems.update(error_detail)
diff --git a/swh/deposit/tests/api/test_checks.py b/swh/deposit/tests/api/test_checks.py
new file mode 100644
--- /dev/null
+++ b/swh/deposit/tests/api/test_checks.py
@@ -0,0 +1,78 @@
+# 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
+
+from swh.deposit.api.checks import check_metadata
+
+
+def test_api_checks_check_metadata_ok(swh_checks_deposit):
+ actual_check, detail = check_metadata(
+ {
+ "url": "something",
+ "external_identifier": "something-else",
+ "name": "foo",
+ "author": "someone",
+ }
+ )
+
+ assert actual_check is True
+ assert detail is None
+
+
+def test_api_checks_check_metadata_ok2(swh_checks_deposit):
+ actual_check, detail = check_metadata(
+ {
+ "url": "something",
+ "external_identifier": "something-else",
+ "title": "bar",
+ "author": "someone",
+ }
+ )
+
+ assert actual_check is True
+ assert detail is None
+
+
+def test_api_checks_check_metadata_ko(swh_checks_deposit):
+ """Missing optional field should be caught
+
+ """
+ actual_check, error_detail = check_metadata(
+ {
+ "url": "something",
+ "external_identifier": "something-else",
+ "author": "someone",
+ }
+ )
+
+ expected_error = {
+ "metadata": [
+ {
+ "summary": "Mandatory alternate fields are missing",
+ "fields": ["name or title"],
+ }
+ ]
+ }
+ assert actual_check is False
+ assert error_detail == expected_error
+
+
+def test_api_checks_check_metadata_ko2(swh_checks_deposit):
+ """Missing mandatory fields should be caught
+
+ """
+ actual_check, error_detail = check_metadata(
+ {
+ "url": "something",
+ "external_identifier": "something-else",
+ "title": "foobar",
+ }
+ )
+
+ expected_error = {
+ "metadata": [{"summary": "Mandatory fields are missing", "fields": ["author"],}]
+ }
+
+ assert actual_check is False
+ assert error_detail == expected_error
diff --git a/swh/deposit/tests/api/test_deposit_private_check.py b/swh/deposit/tests/api/test_deposit_private_check.py
--- a/swh/deposit/tests/api/test_deposit_private_check.py
+++ b/swh/deposit/tests/api/test_deposit_private_check.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,12 +7,11 @@
import pytest
from rest_framework import status
+from swh.deposit.api.checks import ALTERNATE_FIELDS_MISSING, MANDATORY_FIELDS_MISSING
from swh.deposit.api.private.deposit_check import (
- ALTERNATE_FIELDS_MISSING,
MANDATORY_ARCHIVE_INVALID,
MANDATORY_ARCHIVE_MISSING,
MANDATORY_ARCHIVE_UNSUPPORTED,
- MANDATORY_FIELDS_MISSING,
)
from swh.deposit.config import (
COL_IRI,
@@ -171,78 +170,6 @@
deposit.save()
-def test_check_metadata_ok(swh_checks_deposit):
- actual_check, detail = swh_checks_deposit._check_metadata(
- {
- "url": "something",
- "external_identifier": "something-else",
- "name": "foo",
- "author": "someone",
- }
- )
-
- assert actual_check is True
- assert detail is None
-
-
-def test_check_metadata_ok2(swh_checks_deposit):
- actual_check, detail = swh_checks_deposit._check_metadata(
- {
- "url": "something",
- "external_identifier": "something-else",
- "title": "bar",
- "author": "someone",
- }
- )
-
- assert actual_check is True
- assert detail is None
-
-
-def test_check_metadata_ko(swh_checks_deposit):
- """Missing optional field should be caught
-
- """
- actual_check, error_detail = swh_checks_deposit._check_metadata(
- {
- "url": "something",
- "external_identifier": "something-else",
- "author": "someone",
- }
- )
-
- expected_error = {
- "metadata": [
- {
- "summary": "Mandatory alternate fields are missing",
- "fields": ["name or title"],
- }
- ]
- }
- assert actual_check is False
- assert error_detail == expected_error
-
-
-def test_check_metadata_ko2(swh_checks_deposit):
- """Missing mandatory fields should be caught
-
- """
- actual_check, error_detail = swh_checks_deposit._check_metadata(
- {
- "url": "something",
- "external_identifier": "something-else",
- "title": "foobar",
- }
- )
-
- expected_error = {
- "metadata": [{"summary": "Mandatory fields are missing", "fields": ["author"],}]
- }
-
- assert actual_check is False
- assert error_detail == expected_error
-
-
def create_deposit_archive_with_archive(
root_path, archive_extension, client, collection_name
):
diff --git a/swh/deposit/tests/api/test_deposit_update.py b/swh/deposit/tests/api/test_deposit_update.py
--- a/swh/deposit/tests/api/test_deposit_update.py
+++ b/swh/deposit/tests/api/test_deposit_update.py
@@ -759,3 +759,37 @@
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert b"Empty body request is not supported" in response.content
+
+
+def test_put_update_metadata_done_deposit_failure_functional_checks(
+ tmp_path,
+ authenticated_client,
+ complete_deposit,
+ deposit_collection,
+ atom_dataset,
+ swh_storage,
+):
+ """failure: client updates metadata on deposit done without required incomplete metadata
+
+ Response: 400
+
+ """
+ update_uri = reverse(
+ EDIT_SE_IRI, args=[deposit_collection.name, complete_deposit.id]
+ )
+
+ response = authenticated_client.put(
+ update_uri,
+ content_type="application/atom+xml;type=entry",
+ # no title, nor author, nor name fields
+ data=atom_dataset["entry-data-fail-metadata-functional-checks"],
+ HTTP_X_CHECK_SWHID=complete_deposit.swh_id,
+ )
+
+ assert response.status_code == status.HTTP_400_BAD_REQUEST
+ assert b"Functional metadata checks failure" in response.content
+ # detail on the errors
+ assert b"- Mandatory fields are missing (author)" in response.content
+ assert (
+ b"- Mandatory alternate fields are missing (name or title)" in response.content
+ )
diff --git a/swh/deposit/tests/data/atom/entry-data-fail-metadata-functional-checks.xml b/swh/deposit/tests/data/atom/entry-data-fail-metadata-functional-checks.xml
new file mode 100644
--- /dev/null
+++ b/swh/deposit/tests/data/atom/entry-data-fail-metadata-functional-checks.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0"?>
+<entry xmlns="http://www.w3.org/2005/Atom">
+ <!-- no author nor title nor name -->
+ <client>hal</client>
+ <id>urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a</id>
+ <updated>2017-10-07T15:17:08Z</updated>
+</entry>

File Metadata

Mime Type
text/plain
Expires
Dec 20 2024, 7:13 AM (11 w, 3 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
3220233

Event Timeline

About · Developer information · Legal