diff --git a/PKG-INFO b/PKG-INFO
index c2bef65e22d7f91c906010ecb2d76da286c3bd50..259442eab04824e650759bbc21bf11008cf63d95 100644
--- a/PKG-INFO
+++ b/PKG-INFO
@@ -1,6 +1,6 @@
Metadata-Version: 1.0
Name: swh.deposit
-Version: 0.0.54
+Version: 0.0.55
Summary: Software Heritage Deposit Server
Home-page: https://forge.softwareheritage.org/source/swh-deposit/
Author: Software Heritage developers
diff --git a/swh.deposit.egg-info/PKG-INFO b/swh.deposit.egg-info/PKG-INFO
index c2bef65e22d7f91c906010ecb2d76da286c3bd50..259442eab04824e650759bbc21bf11008cf63d95 100644
--- a/swh.deposit.egg-info/PKG-INFO
+++ b/swh.deposit.egg-info/PKG-INFO
@@ -1,6 +1,6 @@
Metadata-Version: 1.0
Name: swh.deposit
-Version: 0.0.54
+Version: 0.0.55
Summary: Software Heritage Deposit Server
Home-page: https://forge.softwareheritage.org/source/swh-deposit/
Author: Software Heritage developers
diff --git a/swh/deposit/api/deposit_status.py b/swh/deposit/api/deposit_status.py
index c75ffea6b664435089a710b49700e28f82f3c0ef..e4907607bd57d8c17bdd369d4eebfa78576239dc 100644
--- a/swh/deposit/api/deposit_status.py
+++ b/swh/deposit/api/deposit_status.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2017 The Software Heritage developers
+# Copyright (C) 2017-2018 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
@@ -12,6 +12,58 @@ from ..errors import make_error_response_from_dict
from ..models import DEPOSIT_STATUS_DETAIL, Deposit
+def convert_status_detail(status_detail):
+ """Given a status_detail dict, transforms it into a human readable
+ string.
+
+ Dict has the following form (all first level keys are optional):
+ {
+ 'url': {
+ 'summary': <summary-string>,
+ 'fields': <impacted-fields-list>
+ },
+ 'metadata': [{
+ 'summary': <summary-string>,
+ 'fields': <impacted-fields-list>,
+ }],
+ 'archive': {
+ 'summary': <summary-string>,
+ 'fields': [],
+ }
+
+
+ }
+
+ Args:
+ status_detail (dict):
+
+ Returns:
+ Status detail as inlined string.
+
+ """
+ if not status_detail:
+ return None
+
+ msg = []
+ if 'metadata' in status_detail:
+ for data in status_detail['metadata']:
+ fields = ', '.join(data['fields'])
+ msg.append('- %s (%s)\n' % (data['summary'], fields))
+
+ for key in ['url', 'archive']:
+ if key in status_detail:
+ _detail = status_detail[key]
+ fields = _detail.get('fields')
+ suffix_msg = ''
+ if fields:
+ suffix_msg = ' (%s)' % ', '.join(fields)
+ msg.append('- %s%s\n' % (_detail['summary'], suffix_msg))
+
+ if not msg:
+ return None
+ return ''.join(msg)
+
+
class SWHDepositStatus(SWHBaseDeposit):
"""Deposit status.
@@ -35,10 +87,14 @@ class SWHDepositStatus(SWHBaseDeposit):
'deposit %s does not belong to collection %s' % (
deposit_id, collection_name))
+ status_detail = convert_status_detail(deposit.status_detail)
+ if not status_detail:
+ status_detail = DEPOSIT_STATUS_DETAIL[deposit.status]
+
context = {
'deposit_id': deposit.id,
'status': deposit.status,
- 'status_detail': DEPOSIT_STATUS_DETAIL[deposit.status],
+ 'status_detail': status_detail,
'swh_id': None,
}
diff --git a/swh/deposit/api/private/deposit_check.py b/swh/deposit/api/private/deposit_check.py
index c0e92c13171ce76224acaaef447a105b04e3bcb1..686b6558bd9810aed94777493ec59b5fdd667869 100644
--- a/swh/deposit/api/private/deposit_check.py
+++ b/swh/deposit/api/private/deposit_check.py
@@ -15,6 +15,14 @@ from ...config import ARCHIVE_TYPE, METADATA_TYPE
from ...models import Deposit, DepositRequest
+MANDATORY_FIELDS_MISSING = 'Mandatory fields are missing'
+ALTERNATE_FIELDS_MISSING = 'Mandatory alternate fields are missing'
+
+MANDATORY_ARCHIVE_UNREADABLE = 'Deposit was rejected because at least one of its associated archives was not readable' # noqa
+MANDATORY_ARCHIVE_MISSING = 'Deposit without archive is rejected'
+INCOMPATIBLE_URL_FIELDS = "At least one url field must be compatible with the client's domain name" # noqa
+
+
class SWHChecksDeposit(SWHGetDepositAPI, SWHPrivateAPIView):
"""Dedicated class to read a deposit's raw archives content.
@@ -55,8 +63,7 @@ class SWHChecksDeposit(SWHGetDepositAPI, SWHPrivateAPIView):
if len(requests) == 0: # no associated archive is refused
return False, {
'archive': {
- 'summary': 'Deposit without archive is rejected.',
- 'id': deposit.id,
+ 'summary': MANDATORY_ARCHIVE_MISSING,
}
}
@@ -70,10 +77,8 @@ class SWHChecksDeposit(SWHGetDepositAPI, SWHPrivateAPIView):
if rejected_dr_ids:
return False, {
'archive': {
- 'summary': 'Following deposit request ids are rejected '
- 'because their associated archive is not '
- 'readable',
- 'ids': rejected_dr_ids,
+ 'summary': MANDATORY_ARCHIVE_UNREADABLE,
+ 'fields': rejected_dr_ids,
}}
return True, None
@@ -144,19 +149,19 @@ class SWHChecksDeposit(SWHGetDepositAPI, SWHPrivateAPIView):
mandatory_result = [k for k, v in required_fields.items() if not v]
optional_result = [
- k for k, v in alternate_fields.items() if not v]
+ ' 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 are missing',
+ 'summary': MANDATORY_FIELDS_MISSING,
'fields': mandatory_result
})
if optional_result != []:
detail.append({
- 'summary': 'Mandatory alternate fields are missing',
+ 'summary': ALTERNATE_FIELDS_MISSING,
'fields': optional_result,
})
return False, {
@@ -178,17 +183,19 @@ class SWHChecksDeposit(SWHGetDepositAPI, SWHPrivateAPIView):
"""
url_fields = []
for field in metadata:
- url_fields.append(field)
- if 'url' in field and client_domain in metadata[field]:
- return True, None
+ if 'url' in field:
+ if client_domain in metadata[field]:
+ return True, None
+ url_fields.append(field)
- return False, {
+ detail = {
'url': {
- 'summary': "At least one url field must be compatible with the"
- "client's domain name. The following url fields "
- "failed the check.",
- 'fields': url_fields,
- }}
+ 'summary': INCOMPATIBLE_URL_FIELDS,
+ }
+ }
+ if url_fields:
+ detail['url']['fields'] = url_fields
+ return False, detail
def process_get(self, req, collection_name, deposit_id):
"""Build a unique tarball from the multiple received and stream that
diff --git a/swh/deposit/tests/api/test_deposit_check.py b/swh/deposit/tests/api/test_deposit_check.py
index f1142b179775e0dc970b2004324143863ebf7201..46768ce2a489f8de63cb5ece9c2fc93846d8ec8e 100644
--- a/swh/deposit/tests/api/test_deposit_check.py
+++ b/swh/deposit/tests/api/test_deposit_check.py
@@ -12,12 +12,19 @@ from nose.plugins.attrib import attr
from rest_framework import status
from rest_framework.test import APITestCase
-from ...models import Deposit
-from ...config import DEPOSIT_STATUS_VERIFIED, PRIVATE_CHECK_DEPOSIT
-from ...config import DEPOSIT_STATUS_DEPOSITED, DEPOSIT_STATUS_REJECTED
+from swh.deposit.config import (
+ DEPOSIT_STATUS_VERIFIED, PRIVATE_CHECK_DEPOSIT,
+ DEPOSIT_STATUS_DEPOSITED, DEPOSIT_STATUS_REJECTED
+)
+from swh.deposit.api.private.deposit_check import (
+ SWHChecksDeposit,
+ MANDATORY_FIELDS_MISSING, INCOMPATIBLE_URL_FIELDS,
+ MANDATORY_ARCHIVE_UNREADABLE, ALTERNATE_FIELDS_MISSING
+)
+from swh.deposit.models import Deposit
+
from ..common import BasicTestCase, WithAuthTestCase, CommonCreationRoutine
from ..common import FileSystemCreationRoutine
-from ...api.private.deposit_check import SWHChecksDeposit
@attr('fs')
@@ -71,40 +78,22 @@ class CheckDepositTest(APITestCase, WithAuthTestCase,
self.assertEqual(response.status_code, status.HTTP_200_OK)
data = json.loads(response.content.decode('utf-8'))
self.assertEqual(data['status'], DEPOSIT_STATUS_REJECTED)
- expected_error = {
- 'metadata': [
- {
- 'fields': ['url', 'external_identifier', 'author'],
- 'summary': 'Mandatory fields are missing'
- },
- {
- 'fields': [['name', 'title']],
- 'summary': 'Mandatory alternate fields are missing'
- }],
- }
details = data['details']
# archive checks failure
- self.assertEqual(len(details['archive']['ids']), 1)
+ self.assertEqual(len(details['archive']['fields']), 1)
self.assertEqual(details['archive']['summary'],
- 'Following deposit request ids are '
- 'rejected because their associated archive'
- ' is not readable')
+ MANDATORY_ARCHIVE_UNREADABLE)
# metadata check failure
self.assertEqual(len(details['metadata']), 2)
mandatory = details['metadata'][0]
- self.assertEqual(mandatory['summary'], 'Mandatory fields are missing')
+ self.assertEqual(mandatory['summary'], MANDATORY_FIELDS_MISSING)
self.assertEqual(set(mandatory['fields']),
set(['url', 'external_identifier', 'author']))
alternate = details['metadata'][1]
- self.assertEqual(alternate['summary'],
- 'Mandatory alternate fields are missing')
- self.assertEqual(alternate['fields'], [['name', 'title']])
+ self.assertEqual(alternate['summary'], ALTERNATE_FIELDS_MISSING)
+ self.assertEqual(alternate['fields'], ['name or title'])
# url check failure
- self.assertEqual(details['url']['summary'],
- "At least one url field must be compatible with the"
- "client's domain name. The following url fields "
- "failed the check.")
- self.assertEqual(details['url']['fields'], [])
+ self.assertEqual(details['url']['summary'], INCOMPATIBLE_URL_FIELDS)
deposit = Deposit.objects.get(pk=deposit.id)
self.assertEquals(deposit.status, DEPOSIT_STATUS_REJECTED)
@@ -175,7 +164,7 @@ class CheckMetadata(unittest.TestCase, SWHChecksDeposit):
expected_error = {
'metadata': [{
'summary': 'Mandatory alternate fields are missing',
- 'fields': [('name', 'title')],
+ 'fields': ['name or title'],
}]
}
self.assertFalse(actual_check)
diff --git a/swh/deposit/tests/api/test_deposit_status.py b/swh/deposit/tests/api/test_deposit_status.py
index 71e95db4a6e5696d13446fe1f156c6887b3389e7..180eacb8a7908a491e90b4a2ef09e1d55b29944f 100644
--- a/swh/deposit/tests/api/test_deposit_status.py
+++ b/swh/deposit/tests/api/test_deposit_status.py
@@ -10,13 +10,15 @@ from nose.tools import istest
from rest_framework import status
from rest_framework.test import APITestCase
+from swh.deposit.api.deposit_status import convert_status_detail
+from swh.deposit.config import (COL_IRI, STATE_IRI, DEPOSIT_STATUS_DEPOSITED,
+ DEPOSIT_STATUS_REJECTED)
from swh.deposit.models import Deposit, DEPOSIT_STATUS_DETAIL
from swh.deposit.models import DEPOSIT_STATUS_LOAD_SUCCESS
from swh.deposit.parsers import parse_xml
from ..common import BasicTestCase, WithAuthTestCase, FileSystemCreationRoutine
from ..common import CommonCreationRoutine
-from ...config import COL_IRI, STATE_IRI, DEPOSIT_STATUS_DEPOSITED
class DepositStatusTestCase(APITestCase, WithAuthTestCase, BasicTestCase,
@@ -112,3 +114,105 @@ class DepositStatusTestCase(APITestCase, WithAuthTestCase, BasicTestCase,
HTTP_ACCEPT='text/html,application/xml;q=9,*/*,q=8')
self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+ @istest
+ def convert_status_detail_empty(self):
+ actual_status_detail = convert_status_detail({})
+ self.assertIsNone(actual_status_detail)
+
+ actual_status_detail = convert_status_detail({'dummy-keys': []})
+ self.assertIsNone(actual_status_detail)
+
+ actual_status_detail = convert_status_detail(None)
+ self.assertIsNone(actual_status_detail)
+
+ @istest
+ def convert_status_detail(self):
+ status_detail = {
+ 'url': {
+ 'summary': "At least one url field must be compatible with the client\'s domain name. The following url fields failed the check", # noqa
+ 'fields': ['blahurl', 'testurl'],
+ },
+ 'metadata': [
+ {
+ 'summary': 'Mandatory fields missing',
+ 'fields': ['url', 'title'],
+ },
+ {
+ 'summary': 'Alternate fields missing',
+ 'fields': ['name or title', 'url or badurl']
+ }
+ ],
+ 'archive': {
+ 'summary': 'Unreadable archive',
+ 'fields': ['1', '2'],
+ },
+ }
+
+ expected_status_detail = '''- Mandatory fields missing (url, title)
+- Alternate fields missing (name or title, url or badurl)
+- At least one url field must be compatible with the client's domain name. The following url fields failed the check (blahurl, testurl)
+- Unreadable archive (1, 2)
+''' # noqa
+
+ actual_status_detail = convert_status_detail(status_detail)
+
+ self.assertEqual(actual_status_detail, expected_status_detail)
+
+ @istest
+ def convert_status_detail_2(self):
+ status_detail = {
+ 'url': {
+ 'summary': 'At least one compatible url field. Failed',
+ 'fields': ['testurl'],
+ },
+ 'metadata': [
+ {
+ 'summary': 'Mandatory fields missing',
+ 'fields': ['name'],
+ },
+ ],
+ }
+
+ expected_status_detail = '''- Mandatory fields missing (name)
+- At least one compatible url field. Failed (testurl)
+'''
+
+ actual_status_detail = convert_status_detail(status_detail)
+
+ self.assertEqual(actual_status_detail, expected_status_detail)
+
+ @istest
+ def convert_status_detail_3(self):
+ status_detail = {
+ 'url': {
+ 'summary': 'At least one compatible url field',
+ },
+ }
+
+ expected_status_detail = '- At least one compatible url field\n'
+ actual_status_detail = convert_status_detail(status_detail)
+ self.assertEqual(actual_status_detail, expected_status_detail)
+
+ @istest
+ def status_on_deposit_rejected(self):
+ _status = DEPOSIT_STATUS_REJECTED
+ _swh_id = '548b3c0a2bb43e1fca191e24b5803ff6b3bc7c10'
+ _status_detail = {'url': {'summary': 'Wrong url'}}
+
+ # given
+ deposit_id = self.create_deposit_with_status(
+ status=_status, swh_id=_swh_id, status_detail=_status_detail)
+
+ url = reverse(STATE_IRI, args=[self.collection.name, deposit_id])
+
+ # when
+ status_response = self.client.get(url)
+
+ # then
+ self.assertEqual(status_response.status_code, status.HTTP_200_OK)
+ r = parse_xml(BytesIO(status_response.content))
+ self.assertEqual(int(r['deposit_id']), deposit_id)
+ self.assertEqual(r['deposit_status'], _status)
+ self.assertEqual(r['deposit_status_detail'], '- Wrong url')
+ self.assertEqual(r['deposit_swh_id'], _swh_id)
diff --git a/swh/deposit/tests/common.py b/swh/deposit/tests/common.py
index 0bfab3eb400cb5f76fa02fa924153461be4434d2..f4d66e6aaf66ea68274a56cf1ef1a7017778a418 100644
--- a/swh/deposit/tests/common.py
+++ b/swh/deposit/tests/common.py
@@ -18,6 +18,7 @@ from rest_framework import status
from swh.deposit.config import (COL_IRI, EM_IRI, EDIT_SE_IRI,
DEPOSIT_STATUS_PARTIAL,
DEPOSIT_STATUS_VERIFIED,
+ DEPOSIT_STATUS_REJECTED,
DEPOSIT_STATUS_DEPOSITED)
from swh.deposit.models import DepositClient, DepositCollection, Deposit
from swh.deposit.models import DepositRequest
@@ -353,7 +354,8 @@ xmlns:codemeta="https://doi.org/10.5063/SCHEMA/CODEMETA-2.0">
return deposit_id
def create_deposit_with_status(
- self, status, external_id='some-external-id-1', swh_id=None):
+ self, status,
+ external_id='some-external-id-1', swh_id=None, status_detail=None):
# create an invalid deposit which we will update further down the line
deposit_id = self.create_deposit_with_invalid_archive(external_id)
@@ -361,6 +363,8 @@ xmlns:codemeta="https://doi.org/10.5063/SCHEMA/CODEMETA-2.0">
# test context ('rejected' for example). Update in place the
# deposit with such status to permit some further tests.
deposit = Deposit.objects.get(pk=deposit_id)
+ if status == DEPOSIT_STATUS_REJECTED:
+ deposit.status_detail = status_detail
deposit.status = status
if swh_id:
deposit.swh_id = swh_id
diff --git a/version.txt b/version.txt
index 9c7699ebe60ec23e2500ddcbd119dea1f3597236..a0c31f0e782dc913bb674a2c8b829026c31f20ca 100644
--- a/version.txt
+++ b/version.txt
@@ -1 +1 @@
-v0.0.54-0-g95a7aaa
\ No newline at end of file
+v0.0.55-0-g6317a5a
\ No newline at end of file