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 @@ -12,7 +12,7 @@ from django.utils import timezone from rest_framework import status from rest_framework.authentication import BasicAuthentication -from rest_framework.permissions import IsAuthenticated, AllowAny +from rest_framework.permissions import IsAuthenticated from rest_framework.views import APIView from swh.model import hashutil @@ -50,15 +50,6 @@ permission_classes = (IsAuthenticated, ) -class SWHPrivateAPIView(SWHAPIView): - """Mixin intended as private api (so no authentication) based API view - (for the private ones). - - """ - authentication_classes = () - permission_classes = (AllowAny, ) - - class SWHBaseDeposit(SWHDefaultConfig, SWHAPIView, metaclass=ABCMeta): """Base deposit request class sharing multiple common behaviors. diff --git a/swh/deposit/api/private/__init__.py b/swh/deposit/api/private/__init__.py --- a/swh/deposit/api/private/__init__.py +++ b/swh/deposit/api/private/__init__.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2018 The Software Heritage developers +# Copyright (C) 2017-2019 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 @@ -8,6 +8,11 @@ from ...config import METADATA_TYPE from ...models import DepositRequest, Deposit +from rest_framework.permissions import AllowAny + +from swh.deposit.api.common import SWHAPIView +from swh.deposit.errors import make_error_dict, NOT_FOUND + class DepositReadMixin: """Deposit Read mixin @@ -49,3 +54,39 @@ metadata = (m.metadata for m in self._deposit_requests( deposit, request_type=METADATA_TYPE)) return utils.merge(*metadata) + + +class SWHPrivateAPIView(SWHAPIView): + """Mixin intended as private api (so no authentication) based API view + (for the private ones). + + """ + authentication_classes = () + permission_classes = (AllowAny, ) + + def checks(self, req, collection_name, deposit_id=None): + """Override default checks implementation to allow empty collection. + + """ + if deposit_id: + try: + Deposit.objects.get(pk=deposit_id) + except Deposit.DoesNotExist: + return make_error_dict( + NOT_FOUND, + 'Deposit with id %s does not exist' % + deposit_id) + + headers = self._read_headers(req) + checks = self.additional_checks( + req, headers, collection_name, deposit_id) + if 'error' in checks: + return checks + + return {'headers': headers} + + def get(self, req, collection_name=None, deposit_id=None, format=None): + return super().get(req, collection_name, deposit_id, format) + + def put(self, req, collection_name=None, deposit_id=None, format=None): + return super().put(req, collection_name, deposit_id, format) 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 @@ -11,8 +11,8 @@ from rest_framework import status -from . import DepositReadMixin -from ..common import SWHGetDepositAPI, SWHPrivateAPIView +from . import DepositReadMixin, SWHPrivateAPIView +from ..common import SWHGetDepositAPI from ...config import DEPOSIT_STATUS_VERIFIED, DEPOSIT_STATUS_REJECTED from ...config import ARCHIVE_TYPE from ...models import Deposit @@ -33,7 +33,7 @@ r'.*\.(%s)$' % '|'.join(ARCHIVE_EXTENSIONS)) -class SWHChecksDeposit(SWHGetDepositAPI, SWHPrivateAPIView, DepositReadMixin): +class SWHChecksDeposit(SWHPrivateAPIView, SWHGetDepositAPI, DepositReadMixin): """Dedicated class to read a deposit's raw archives content. Only GET is supported. diff --git a/swh/deposit/api/private/deposit_list.py b/swh/deposit/api/private/deposit_list.py --- a/swh/deposit/api/private/deposit_list.py +++ b/swh/deposit/api/private/deposit_list.py @@ -1,4 +1,4 @@ -# Copyright (C) 2018 The Software Heritage developers +# Copyright (C) 2018-2019 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 @@ -9,7 +9,7 @@ from rest_framework.pagination import PageNumberPagination from rest_framework import serializers -from ..common import SWHPrivateAPIView +from . import SWHPrivateAPIView from ..converters import convert_status_detail from ...models import Deposit diff --git a/swh/deposit/api/private/deposit_read.py b/swh/deposit/api/private/deposit_read.py --- a/swh/deposit/api/private/deposit_read.py +++ b/swh/deposit/api/private/deposit_read.py @@ -17,9 +17,9 @@ from swh.deposit.utils import normalize_date from swh.deposit import utils -from . import DepositReadMixin +from . import DepositReadMixin, SWHPrivateAPIView from ...config import SWH_PERSON, ARCHIVE_TYPE -from ..common import SWHGetDepositAPI, SWHPrivateAPIView +from ..common import SWHGetDepositAPI from ...models import Deposit @@ -67,7 +67,7 @@ yield archive_paths[0] -class SWHDepositReadArchives(SWHGetDepositAPI, SWHPrivateAPIView, +class SWHDepositReadArchives(SWHPrivateAPIView, SWHGetDepositAPI, DepositReadMixin): """Dedicated class to read a deposit's raw archives content. @@ -105,7 +105,7 @@ content_type='application/octet-stream') -class SWHDepositReadMetadata(SWHGetDepositAPI, SWHPrivateAPIView, +class SWHDepositReadMetadata(SWHPrivateAPIView, SWHGetDepositAPI, DepositReadMixin): """Class in charge of aggregating metadata on a deposit. diff --git a/swh/deposit/api/private/deposit_update_status.py b/swh/deposit/api/private/deposit_update_status.py --- a/swh/deposit/api/private/deposit_update_status.py +++ b/swh/deposit/api/private/deposit_update_status.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2018 The Software Heritage developers +# Copyright (C) 2017-2019 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 @@ -9,13 +9,14 @@ persistent_identifier, REVISION, DIRECTORY ) -from ..common import SWHPutDepositAPI, SWHPrivateAPIView +from . import SWHPrivateAPIView +from ..common import SWHPutDepositAPI from ...errors import make_error_dict, BAD_REQUEST from ...models import Deposit, DEPOSIT_STATUS_DETAIL from ...models import DEPOSIT_STATUS_LOAD_SUCCESS -class SWHUpdateStatusDeposit(SWHPutDepositAPI, SWHPrivateAPIView): +class SWHUpdateStatusDeposit(SWHPrivateAPIView, SWHPutDepositAPI): """Deposit request class to update the deposit's status. HTTP verbs supported: PUT @@ -52,13 +53,6 @@ return {} - def restrict_access(self, req, deposit=None): - """Remove restriction modification to 'partial' deposit. - Update is possible regardless of the existing status. - - """ - return None - def process_put(self, req, headers, collection_name, deposit_id): """Update the deposit's status diff --git a/swh/deposit/api/private/urls.py b/swh/deposit/api/private/urls.py --- a/swh/deposit/api/private/urls.py +++ b/swh/deposit/api/private/urls.py @@ -36,6 +36,27 @@ url(r'^(?P[^/]+)/(?P[^/]+)/check/$', SWHChecksDeposit.as_view(), name=PRIVATE_CHECK_DEPOSIT), + # Retrieve deposit's raw archives' content + # -> GET + url(r'^(?P[^/]+)/raw/$', + SWHDepositReadArchives.as_view(), + name=PRIVATE_GET_RAW_CONTENT+'-nc'), + # Update deposit's status + # -> PUT + url(r'^(?P[^/]+)/update/$', + SWHUpdateStatusDeposit.as_view(), + name=PRIVATE_PUT_DEPOSIT+'-nc'), + # Retrieve metadata information on a specific deposit + # -> GET + url(r'^(?P[^/]+)/meta/$', + SWHDepositReadMetadata.as_view(), + name=PRIVATE_GET_DEPOSIT_METADATA+'-nc'), + # Check archive and metadata information on a specific deposit + # -> GET + url(r'^(?P[^/]+)/check/$', + SWHChecksDeposit.as_view(), + name=PRIVATE_CHECK_DEPOSIT+'-nc'), + url(r'^deposits/$', DepositList.as_view(), name=PRIVATE_LIST_DEPOSITS) ] diff --git a/swh/deposit/tests/api/test_deposit_check.py b/swh/deposit/tests/api/test_deposit_check.py --- a/swh/deposit/tests/api/test_deposit_check.py +++ b/swh/deposit/tests/api/test_deposit_check.py @@ -36,6 +36,10 @@ def setUp(self): super().setUp() + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_CHECK_DEPOSIT, + args=[self.collection.name, deposit_id]) + def test_deposit_ok(self): """Proper deposit should succeed the checks (-> status ready) @@ -47,9 +51,7 @@ deposit = Deposit.objects.get(pk=deposit_id) self.assertEqual(deposit.status, DEPOSIT_STATUS_DEPOSITED) - url = reverse(PRIVATE_CHECK_DEPOSIT, - args=[self.collection.name, deposit.id]) - + url = self.private_deposit_url(deposit.id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -69,9 +71,7 @@ deposit = Deposit.objects.get(pk=deposit_id) self.assertEqual(DEPOSIT_STATUS_DEPOSITED, deposit.status) - url = reverse(PRIVATE_CHECK_DEPOSIT, - args=[self.collection.name, deposit.id]) - + url = self.private_deposit_url(deposit.id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -94,9 +94,7 @@ deposit = Deposit.objects.get(pk=deposit_id) self.assertEqual(DEPOSIT_STATUS_DEPOSITED, deposit.status) - url = reverse(PRIVATE_CHECK_DEPOSIT, - args=[self.collection.name, deposit.id]) - + url = self.private_deposit_url(deposit.id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -119,9 +117,7 @@ deposit = Deposit.objects.get(pk=deposit_id) self.assertEqual(DEPOSIT_STATUS_DEPOSITED, deposit.status) - url = reverse(PRIVATE_CHECK_DEPOSIT, - args=[self.collection.name, deposit.id]) - + url = self.private_deposit_url(deposit.id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -158,8 +154,7 @@ deposit = Deposit.objects.get(pk=deposit_id) self.assertEqual(deposit.status, DEPOSIT_STATUS_DEPOSITED) - url = reverse(PRIVATE_CHECK_DEPOSIT, - args=[self.collection.name, deposit.id]) + url = self.private_deposit_url(deposit.id) response = self.client.get(url) @@ -171,6 +166,13 @@ self.assertEqual(deposit.status, DEPOSIT_STATUS_VERIFIED) +@pytest.mark.fs +class CheckDepositTest2(CheckDepositTest): + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_CHECK_DEPOSIT+'-nc', + args=[deposit_id]) + + class CheckMetadata(unittest.TestCase, SWHChecksDeposit): def test_check_metadata_ok(self): actual_check, detail = self._check_metadata({ diff --git a/swh/deposit/tests/api/test_deposit_read_archive.py b/swh/deposit/tests/api/test_deposit_read_archive.py --- a/swh/deposit/tests/api/test_deposit_read_archive.py +++ b/swh/deposit/tests/api/test_deposit_read_archive.py @@ -30,15 +30,17 @@ self.root_path, 'archive2', 'file2', b'some other content in file') self.workdir = os.path.join(self.root_path, 'workdir') + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_GET_RAW_CONTENT, + args=[self.collection.name, deposit_id]) + def test_access_to_existing_deposit_with_one_archive(self): """Access to deposit should stream a 200 response with its raw content """ deposit_id = self.create_simple_binary_deposit() - url = reverse(PRIVATE_GET_RAW_CONTENT, - args=[self.collection.name, deposit_id]) - + url = self.private_deposit_url(deposit_id) r = self.client.get(url) self.assertEqual(r.status_code, status.HTTP_200_OK) @@ -74,10 +76,7 @@ """ deposit_id = self.create_complex_binary_deposit() - - url = reverse(PRIVATE_GET_RAW_CONTENT, - args=[self.collection.name, deposit_id]) - + url = self.private_deposit_url(deposit_id) r = self.client.get(url) self.assertEqual(r.status_code, status.HTTP_200_OK) @@ -93,33 +92,7 @@ self.assertEqual(os.listdir(TEST_CONFIG['extraction_dir']), []) -class DepositReadArchivesFailureTest(APITestCase, WithAuthTestCase, - BasicTestCase, CommonCreationRoutine): - def test_access_to_nonexisting_deposit_returns_404_response(self): - """Read unknown collection should return a 404 response - - """ - unknown_id = '999' - url = reverse(PRIVATE_GET_RAW_CONTENT, - args=[self.collection.name, unknown_id]) - - response = self.client.get(url) - self.assertEqual(response.status_code, - status.HTTP_404_NOT_FOUND) - self.assertIn('Deposit with id %s does not exist' % unknown_id, - response.content.decode('utf-8')) - - def test_access_to_nonexisting_collection_returns_404_response(self): - """Read unknown deposit should return a 404 response - - """ - collection_name = 'non-existing' - deposit_id = self.create_deposit_partial() - url = reverse(PRIVATE_GET_RAW_CONTENT, - args=[collection_name, deposit_id]) - - response = self.client.get(url) - self.assertEqual(response.status_code, - status.HTTP_404_NOT_FOUND) - self.assertIn('Unknown collection name %s' % collection_name, - response.content.decode('utf-8')) +@pytest.mark.fs +class DepositReadArchivesTest2(DepositReadArchivesTest): + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_GET_RAW_CONTENT+'-nc', args=[deposit_id]) diff --git a/swh/deposit/tests/api/test_deposit_read_metadata.py b/swh/deposit/tests/api/test_deposit_read_metadata.py --- a/swh/deposit/tests/api/test_deposit_read_metadata.py +++ b/swh/deposit/tests/api/test_deposit_read_metadata.py @@ -60,14 +60,17 @@ %s """ + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_GET_DEPOSIT_METADATA, + args=[self.collection.name, deposit_id]) + def test_read_metadata(self): """Private metadata read api to existing deposit should return metadata """ deposit_id = self.create_deposit_partial() - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[self.collection.name, deposit_id]) + url = self.private_deposit_url(deposit_id) response = self.client.get(url) @@ -167,8 +170,7 @@ self.assertEqual(deposit.parent, deposit_parent) self.assertEqual(deposit.status, DEPOSIT_STATUS_PARTIAL) - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[self.collection.name, deposit_id]) + url = self.private_deposit_url(deposit_id) response = self.client.get(url) @@ -256,10 +258,7 @@ deposit_id = self.create_deposit_partial_with_data_in_args( codemeta_entry_data) - - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[self.collection.name, deposit_id]) - + url = self.private_deposit_url(deposit_id) response = self.client.get(url) self.assertEqual(response.status_code, @@ -383,9 +382,7 @@ deposit.complete_date = '2016-04-06' deposit.save() - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[self.collection.name, deposit_id]) - + url = self.private_deposit_url(deposit_id) response = self.client.get(url) self.assertEqual(response.status_code, @@ -512,10 +509,7 @@ deposit_id = self.create_deposit_partial_with_data_in_args( codemeta_entry_data) - - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[self.collection.name, deposit_id]) - + url = self.private_deposit_url(deposit_id) response = self.client.get(url) self.assertEqual(response.status_code, @@ -636,26 +630,15 @@ """ unknown_id = '999' - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[self.collection.name, unknown_id]) - + url = self.private_deposit_url(unknown_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.assertIn('Deposit with id %s does not exist' % unknown_id, response.content.decode('utf-8')) - def test_access_to_nonexisting_collection_returns_404_response(self): - """Read unknown deposit should return a 404 response - """ - collection_name = 'non-existing' - deposit_id = self.create_deposit_partial() - url = reverse(PRIVATE_GET_DEPOSIT_METADATA, - args=[collection_name, deposit_id]) - - response = self.client.get(url) - self.assertEqual(response.status_code, - status.HTTP_404_NOT_FOUND) - self.assertIn('Unknown collection name %s' % collection_name, - response.content.decode('utf-8'),) +class DepositReadMetadataTest2(DepositReadMetadataTest): + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_GET_DEPOSIT_METADATA+'-nc', + args=[deposit_id]) diff --git a/swh/deposit/tests/api/test_deposit_update_status.py b/swh/deposit/tests/api/test_deposit_update_status.py --- a/swh/deposit/tests/api/test_deposit_update_status.py +++ b/swh/deposit/tests/api/test_deposit_update_status.py @@ -28,12 +28,15 @@ self.deposit = Deposit.objects.get(pk=deposit.id) assert self.deposit.status == DEPOSIT_STATUS_VERIFIED + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_PUT_DEPOSIT, + args=[self.collection.name, deposit_id]) + def test_update_deposit_status(self): """Existing status for update should return a 204 response """ - url = reverse(PRIVATE_PUT_DEPOSIT, - args=[self.collection.name, self.deposit.id]) + url = self.private_deposit_url(self.deposit.id) possible_status = set(DEPOSIT_STATUS_DETAIL.keys()) - set( [DEPOSIT_STATUS_LOAD_SUCCESS]) @@ -53,8 +56,7 @@ """Existing status for update with info should return a 204 response """ - url = reverse(PRIVATE_PUT_DEPOSIT, - args=[self.collection.name, self.deposit.id]) + url = self.private_deposit_url(self.deposit.id) expected_status = DEPOSIT_STATUS_LOAD_SUCCESS origin_url = 'something' @@ -91,8 +93,7 @@ """Unknown status for update should return a 400 response """ - url = reverse(PRIVATE_PUT_DEPOSIT, - args=[self.collection.name, self.deposit.id]) + url = self.private_deposit_url(self.deposit.id) response = self.client.put( url, @@ -105,8 +106,7 @@ """No status provided for update should return a 400 response """ - url = reverse(PRIVATE_PUT_DEPOSIT, - args=[self.collection.name, self.deposit.id]) + url = self.private_deposit_url(self.deposit.id) response = self.client.put( url, @@ -119,8 +119,7 @@ """Providing successful status without swh_id should return a 400 """ - url = reverse(PRIVATE_PUT_DEPOSIT, - args=[self.collection.name, self.deposit.id]) + url = self.private_deposit_url(self.deposit.id) response = self.client.put( url, @@ -128,3 +127,8 @@ data=json.dumps({'status': DEPOSIT_STATUS_LOAD_SUCCESS})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + +class UpdateDepositStatusTest2(UpdateDepositStatusTest): + def private_deposit_url(self, deposit_id): + return reverse(PRIVATE_PUT_DEPOSIT+'-nc', args=[deposit_id])