diff --git a/swh/deposit/api/converters.py b/swh/deposit/api/converters.py new file mode 100644 index 00000000..928fb248 --- /dev/null +++ b/swh/deposit/api/converters.py @@ -0,0 +1,58 @@ +# 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 + + +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': , + 'fields': + }, + 'metadata': [{ + 'summary': , + 'fields': , + }], + 'archive': [{ + 'summary': , + 'fields': , + }] + + + } + + Args: + status_detail (dict): + + Returns: + Status detail as inlined string. + + """ + if not status_detail: + return None + + def _str_fields(data): + fields = data.get('fields') + if not fields: + return '' + return ' (%s)' % ', '.join(map(str, fields)) + + msg = [] + for key in ['metadata', 'archive']: + _detail = status_detail.get(key) + if _detail: + for data in _detail: + msg.append('- %s%s\n' % (data['summary'], _str_fields(data))) + + _detail = status_detail.get('url') + if _detail: + msg.append('- %s%s\n' % (_detail['summary'], _str_fields(_detail))) + + if not msg: + return None + return ''.join(msg) diff --git a/swh/deposit/api/deposit_status.py b/swh/deposit/api/deposit_status.py index 31e5b337..240db47d 100644 --- a/swh/deposit/api/deposit_status.py +++ b/swh/deposit/api/deposit_status.py @@ -1,118 +1,65 @@ # 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 from django.shortcuts import render from rest_framework import status from .common import SWHBaseDeposit +from .converters import convert_status_detail from ..errors import NOT_FOUND, make_error_response 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': , - 'fields': - }, - 'metadata': [{ - 'summary': , - 'fields': , - }], - 'archive': [{ - 'summary': , - 'fields': , - }] - - - } - - Args: - status_detail (dict): - - Returns: - Status detail as inlined string. - - """ - if not status_detail: - return None - - def _str_fields(data): - fields = data.get('fields') - if not fields: - return '' - return ' (%s)' % ', '.join(map(str, fields)) - - msg = [] - for key in ['metadata', 'archive']: - _detail = status_detail.get(key) - if _detail: - for data in _detail: - msg.append('- %s%s\n' % (data['summary'], _str_fields(data))) - - _detail = status_detail.get('url') - if _detail: - msg.append('- %s%s\n' % (_detail['summary'], _str_fields(_detail))) - - if not msg: - return None - return ''.join(msg) - - class SWHDepositStatus(SWHBaseDeposit): """Deposit status. What's known as 'State IRI' in the sword specification. HTTP verbs supported: GET """ def get(self, req, collection_name, deposit_id, format=None): checks = self.checks(req, collection_name, deposit_id) if 'error' in checks: return make_error_response_from_dict(req, checks['error']) try: deposit = Deposit.objects.get(pk=deposit_id) if deposit.collection.name != collection_name: raise Deposit.DoesNotExist except Deposit.DoesNotExist: return make_error_response( req, NOT_FOUND, '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': status_detail, 'swh_id': None, 'swh_id_context': None, 'swh_anchor_id': None, 'swh_anchor_id_context': None, } if deposit.swh_id: context['swh_id'] = deposit.swh_id if deposit.swh_id_context: context['swh_id_context'] = deposit.swh_id_context if deposit.swh_anchor_id: context['swh_anchor_id'] = deposit.swh_anchor_id if deposit.swh_anchor_id_context: context['swh_anchor_id_context'] = deposit.swh_anchor_id_context return render(req, 'deposit/status.xml', context=context, content_type='application/xml', status=status.HTTP_200_OK) diff --git a/swh/deposit/api/private/deposit_list.py b/swh/deposit/api/private/deposit_list.py index bdc7ed12..a03d5a1a 100644 --- a/swh/deposit/api/private/deposit_list.py +++ b/swh/deposit/api/private/deposit_list.py @@ -1,48 +1,48 @@ # Copyright (C) 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 from rest_framework.fields import _UnvalidatedField from rest_framework.generics import ListAPIView from rest_framework.pagination import PageNumberPagination from rest_framework import serializers from ..common import SWHPrivateAPIView -from ..deposit_status import convert_status_detail +from ..converters import convert_status_detail from ...models import Deposit class DefaultPagination(PageNumberPagination): page_size = 100 page_size_query_param = 'page_size' class StatusDetailField(_UnvalidatedField): """status_detail field is a dict, we want a simple message instead. So, we reuse the convert_status_detail from deposit_status endpoint to that effect. """ def to_representation(self, value): return convert_status_detail(value) class DepositSerializer(serializers.ModelSerializer): status_detail = StatusDetailField() class Meta: model = Deposit fields = '__all__' class DepositList(ListAPIView, SWHPrivateAPIView): """Deposit request class to list the deposit's status per page. HTTP verbs supported: GET """ queryset = Deposit.objects.all().order_by('id') serializer_class = DepositSerializer pagination_class = DefaultPagination diff --git a/swh/deposit/tests/api/test_converters.py b/swh/deposit/tests/api/test_converters.py new file mode 100644 index 00000000..f02b43a2 --- /dev/null +++ b/swh/deposit/tests/api/test_converters.py @@ -0,0 +1,138 @@ +# 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 + +from nose.tools import istest +from rest_framework.test import APITestCase + +from swh.deposit.api.converters import convert_status_detail + + +class ConvertersTestCase(APITestCase): + + @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'], + }], + } + + expected_status_detail = '''- Mandatory fields missing (url, title) +- Alternate fields missing (name or title, url or badurl) +- Unreadable archive (1) +- At least one url field must be compatible with the client's domain name. The following url fields failed the check (blahurl, testurl) +''' # 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'], + }, + ], + 'archive': [ + { + 'summary': 'Invalid archive', + 'fields': ['2'], + }, + { + 'summary': 'Unsupported archive', + 'fields': ['1'], + } + ], + } + + expected_status_detail = '''- Mandatory fields missing (name) +- Invalid archive (2) +- Unsupported archive (1) +- 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 convert_status_detail_edge_case(self): + status_detail = { + 'url': { + 'summary': 'At least one compatible url field. Failed', + 'fields': ['testurl'], + }, + 'metadata': [ + { + 'summary': 'Mandatory fields missing', + 'fields': ['9', 10, 1.212], + }, + ], + 'archive': [ + { + 'summary': 'Invalid archive', + 'fields': ['3'], + }, + { + 'summary': 'Unsupported archive', + 'fields': [2], + } + ], + } + + expected_status_detail = '''- Mandatory fields missing (9, 10, 1.212) +- Invalid archive (3) +- Unsupported archive (2) +- At least one compatible url field. Failed (testurl) +''' + + actual_status_detail = convert_status_detail(status_detail) + + self.assertEqual(actual_status_detail, expected_status_detail) diff --git a/swh/deposit/tests/api/test_deposit_list.py b/swh/deposit/tests/api/test_deposit_list.py index b0e22431..fe05ff46 100644 --- a/swh/deposit/tests/api/test_deposit_list.py +++ b/swh/deposit/tests/api/test_deposit_list.py @@ -1,96 +1,96 @@ # 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 from django.core.urlresolvers import reverse from nose.tools import istest from nose.plugins.attrib import attr from rest_framework import status from rest_framework.test import APITestCase -from swh.deposit.api.deposit_status import convert_status_detail +from swh.deposit.api.converters import convert_status_detail from ...config import DEPOSIT_STATUS_PARTIAL, PRIVATE_LIST_DEPOSITS from ..common import BasicTestCase, WithAuthTestCase, CommonCreationRoutine from ...models import Deposit @attr('fs') class CheckDepositListTest(APITestCase, WithAuthTestCase, BasicTestCase, CommonCreationRoutine): """Check deposit list endpoints. """ def setUp(self): super().setUp() @istest def deposit_list(self): """Deposit list api should return the deposits """ deposit_id = self.create_deposit_partial() # amend the deposit with a status_detail deposit = Deposit.objects.get(pk=deposit_id) status_detail = { 'url': { 'summary': 'At least one compatible url field. Failed', 'fields': ['testurl'], }, 'metadata': [ { 'summary': 'Mandatory fields missing', 'fields': ['9', 10, 1.212], }, ], 'archive': [ { 'summary': 'Invalid archive', 'fields': ['3'], }, { 'summary': 'Unsupported archive', 'fields': [2], } ], } deposit.status_detail = status_detail deposit.save() deposit_id2 = self.create_deposit_partial() # NOTE: does not work as documented # https://docs.djangoproject.com/en/1.11/ref/urlresolvers/#django.core.urlresolvers.reverse # noqa # url = reverse(PRIVATE_LIST_DEPOSITS, kwargs={'page_size': 1}) main_url = reverse(PRIVATE_LIST_DEPOSITS) url = '%s?page_size=1' % main_url response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(data['count'], 2) # 2 deposits expected_next = '%s?page=2&page_size=1' % main_url self.assertTrue(data['next'].endswith(expected_next)) self.assertIsNone(data['previous']) self.assertEqual(len(data['results']), 1) # page of size 1 deposit = data['results'][0] self.assertEquals(deposit['id'], deposit_id) self.assertEquals(deposit['status'], DEPOSIT_STATUS_PARTIAL) expected_status_detail = convert_status_detail(status_detail) self.assertEquals(deposit['status_detail'], expected_status_detail) # then 2nd page response2 = self.client.get(expected_next) self.assertEqual(response2.status_code, status.HTTP_200_OK) data2 = response2.json() self.assertEqual(data2['count'], 2) # still 2 deposits self.assertIsNone(data2['next']) expected_previous = '%s?page_size=1' % main_url self.assertTrue(data2['previous'].endswith(expected_previous)) self.assertEqual(len(data2['results']), 1) # page of size 1 deposit2 = data2['results'][0] self.assertEquals(deposit2['id'], deposit_id2) self.assertEquals(deposit2['status'], DEPOSIT_STATUS_PARTIAL) diff --git a/swh/deposit/tests/api/test_deposit_status.py b/swh/deposit/tests/api/test_deposit_status.py index fca43909..d2362449 100644 --- a/swh/deposit/tests/api/test_deposit_status.py +++ b/swh/deposit/tests/api/test_deposit_status.py @@ -1,277 +1,151 @@ # 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 from django.core.urlresolvers import reverse from io import BytesIO 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 class DepositStatusTestCase(APITestCase, WithAuthTestCase, BasicTestCase, FileSystemCreationRoutine, CommonCreationRoutine): """Status on deposit """ @istest def post_deposit_with_status_check(self): """Binary upload should be accepted """ # given url = reverse(COL_IRI, args=[self.collection.name]) external_id = 'some-external-id-1' # when response = self.client.post( url, content_type='application/zip', # as zip data=self.archive['data'], # + headers CONTENT_LENGTH=self.archive['length'], HTTP_SLUG=external_id, HTTP_CONTENT_MD5=self.archive['md5sum'], HTTP_PACKAGING='http://purl.org/net/sword/package/SimpleZip', HTTP_IN_PROGRESS='false', HTTP_CONTENT_DISPOSITION='attachment; filename=filename0') # then self.assertEqual(response.status_code, status.HTTP_201_CREATED) deposit = Deposit.objects.get(external_id=external_id) status_url = reverse(STATE_IRI, args=[self.collection.name, deposit.id]) # check status status_response = self.client.get(status_url) 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'], DEPOSIT_STATUS_DEPOSITED) self.assertEqual(r['deposit_status_detail'], DEPOSIT_STATUS_DETAIL[DEPOSIT_STATUS_DEPOSITED]) @istest def status_with_swh_information(self): _status = DEPOSIT_STATUS_LOAD_SUCCESS _context = 'https://hal.archives-ouvertes.fr/hal-01727745' _swh_id = 'swh:1:dir:42a13fc721c8716ff695d0d62fc851d641f3a12b' _swh_id_context = '%s;%s' % (_swh_id, _context) _swh_anchor_id = 'swh:rev:1:548b3c0a2bb43e1fca191e24b5803ff6b3bc7c10' _swh_anchor_id_context = '%s;%s' % (_swh_anchor_id, _context) # given deposit_id = self.create_deposit_with_status( status=_status, swh_id=_swh_id, swh_id_context=_swh_id_context, swh_anchor_id=_swh_anchor_id, swh_anchor_id_context=_swh_anchor_id_context ) 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'], DEPOSIT_STATUS_DETAIL[DEPOSIT_STATUS_LOAD_SUCCESS]) self.assertEqual(r['deposit_swh_id'], _swh_id) self.assertEqual(r['deposit_swh_id_context'], _swh_id_context) self.assertEqual(r['deposit_swh_anchor_id'], _swh_anchor_id) self.assertEqual(r['deposit_swh_anchor_id_context'], _swh_anchor_id_context) @istest def status_on_unknown_deposit(self): """Asking for the status of unknown deposit returns 404 response""" status_url = reverse(STATE_IRI, args=[self.collection.name, 999]) status_response = self.client.get(status_url) self.assertEqual(status_response.status_code, status.HTTP_404_NOT_FOUND) @istest def status_with_http_accept_header_should_not_break(self): """Asking deposit status with Accept header should return 200 """ deposit_id = self.create_deposit_partial() status_url = reverse(STATE_IRI, args=[ self.collection.name, deposit_id]) response = self.client.get( status_url, 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'], - }], - } - - expected_status_detail = '''- Mandatory fields missing (url, title) -- Alternate fields missing (name or title, url or badurl) -- Unreadable archive (1) -- At least one url field must be compatible with the client's domain name. The following url fields failed the check (blahurl, testurl) -''' # 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'], - }, - ], - 'archive': [ - { - 'summary': 'Invalid archive', - 'fields': ['2'], - }, - { - 'summary': 'Unsupported archive', - 'fields': ['1'], - } - ], - } - - expected_status_detail = '''- Mandatory fields missing (name) -- Invalid archive (2) -- Unsupported archive (1) -- 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 convert_status_detail_edge_case(self): - status_detail = { - 'url': { - 'summary': 'At least one compatible url field. Failed', - 'fields': ['testurl'], - }, - 'metadata': [ - { - 'summary': 'Mandatory fields missing', - 'fields': ['9', 10, 1.212], - }, - ], - 'archive': [ - { - 'summary': 'Invalid archive', - 'fields': ['3'], - }, - { - 'summary': 'Unsupported archive', - 'fields': [2], - } - ], - } - - expected_status_detail = '''- Mandatory fields missing (9, 10, 1.212) -- Invalid archive (3) -- Unsupported archive (2) -- 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 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)