From 57c734ec69e8aafbac14e428d3cf4c21cb8fefbc Mon Sep 17 00:00:00 2001 From: Anna Trzcinska Date: Wed, 23 Oct 2019 16:36:01 +0200 Subject: [PATCH 1/2] deposits: change /deposits/{pid}/files endpoints * download a file on /deposits/{pid}/files/{file_key} [GET] * add POST, PUT, DELETE Signed-off-by: Anna Trzcinska --- cap/config.py | 3 - cap/modules/deposit/ext.py | 8 +- cap/modules/deposit/serializers/__init__.py | 7 +- cap/modules/deposit/views.py | 86 +++++ setup.py | 16 +- tests/integration/test_files_for_deposit.py | 400 +++++--------------- tests/integration/test_files_workflow.py | 52 +-- 7 files changed, 231 insertions(+), 341 deletions(-) create mode 100644 cap/modules/deposit/views.py diff --git a/cap/config.py b/cap/config.py index 9f377b6e95..03bb3a044e 100644 --- a/cap/config.py +++ b/cap/config.py @@ -536,9 +536,6 @@ def _(x): 'search_class': 'cap.modules.deposit.search:CAPDepositSearch', 'search_factory_imp': 'cap.modules.search.query:cap_search_factory', 'item_route': '/deposits/<{0}:pid_value>'.format(_PID), - 'file_list_route': '/deposits/<{0}:pid_value>/files'.format(_PID), - 'file_item_route': '/deposits/<{0}:pid_value>/files/'.format( - _PID), 'create_permission_factory_imp': check_oauth2_scope( lambda record: CreateDepositPermission(record).can(), write_scope.id), 'read_permission_factory_imp': check_oauth2_scope( diff --git a/cap/modules/deposit/ext.py b/cap/modules/deposit/ext.py index a91b0fe9d3..0d545c4333 100644 --- a/cap/modules/deposit/ext.py +++ b/cap/modules/deposit/ext.py @@ -1,13 +1,16 @@ """Initialize extension.""" from __future__ import absolute_import, print_function -from cap.modules.schemas.models import Schema + from invenio_search import current_search +from cap.modules.schemas.models import Schema + +from .views import create_deposit_files_blueprint + class CAPDeposit(object): """CAPDeposit extension.""" - def __init__(self, app=None): """Extension initialization.""" if app: @@ -16,3 +19,4 @@ def __init__(self, app=None): def init_app(self, app): """Flask application initialization.""" app.extensions['cap_deposit'] = self + app.register_blueprint(create_deposit_files_blueprint(app)) diff --git a/cap/modules/deposit/serializers/__init__.py b/cap/modules/deposit/serializers/__init__.py index 3daa441c10..7d069cc89d 100644 --- a/cap/modules/deposit/serializers/__init__.py +++ b/cap/modules/deposit/serializers/__init__.py @@ -25,12 +25,11 @@ from __future__ import absolute_import, print_function -from invenio_deposit.serializers import json_file_response from invenio_records_rest.serializers.response import (record_responsify, search_responsify) from .json import DepositSerializer -from .schemas.json import DepositSchema, DepositFormSchema +from .schemas.json import DepositFormSchema, DepositSchema # Serializers # =========== @@ -46,7 +45,3 @@ deposit_form_json_v1_response = record_responsify(deposit_form_json_v1, 'application/json') deposit_json_v1_search = search_responsify(deposit_json_v1, 'application/json') - -# Files-REST serializers -# JSON Files serializers for deposit files -files_response = json_file_response diff --git a/cap/modules/deposit/views.py b/cap/modules/deposit/views.py new file mode 100644 index 0000000000..f31355aeb3 --- /dev/null +++ b/cap/modules/deposit/views.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- +# +# This file is part of CERN Analysis Preservation Framework. +# Copyright (C) 2016 CERN. +# +# CERN Analysis Preservation Framework is free software; you can redistribute +# it and/or modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of the +# License, or (at your option) any later version. +# +# CERN Analysis Preservation Framework is distributed in the hope that it will +# be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with CERN Analysis Preservation Framework; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307, USA. +# +# In applying this license, CERN does not +# waive the privileges and immunities granted to it by virtue of its status +# as an Intergovernmental Organization or submit itself to any jurisdiction. +"""Extra deposit views.""" + +from __future__ import absolute_import, print_function + +from flask import Blueprint, g +from invenio_files_rest.views import bucket_view, object_view +from sqlalchemy.orm.exc import NoResultFound + +from .api import CAPDeposit + + +def create_deposit_files_blueprint(app): + """Create blueprint from a Flask application. + :params app: A Flask application. + :returns: Configured blueprint. + """ + cap_deposit_files_bp = Blueprint('cap_deposit_files', + __name__, + url_prefix='') + + deposit_item_path = app.config['DEPOSIT_REST_ENDPOINTS']['depid'][ + 'item_route'] + cap_deposit_files_bp.add_url_rule('{}/files'.format(deposit_item_path), + view_func=bucket_view) + cap_deposit_files_bp.add_url_rule( + '{}/files/'.format(deposit_item_path), view_func=object_view) + + @cap_deposit_files_bp.url_value_preprocessor + def resolve_pid_to_bucket_id(endpoint, values): + """Flask URL preprocessor to resolve pid to Bucket ID. + In the ``cap_deposit_bp`` we are gluing together Records-REST + and Files-REST APIs. Records-REST knows about PIDs but Files-REST does + not, this function will pre-process the URL so the PID is removed from + the URL and resolved to bucket ID which is injected into Files-REST + view calls: + ``/api///files/`` -> + ``/files//``. + """ + # Remove the 'pid_value' in order to match the Files-REST bucket view + # signature. Store the value in Flask global request object for later + # usage. + g.pid = values.pop('pid_value') + pid, _ = g.pid.data + + try: + deposit = CAPDeposit.get_record(pid.object_uuid) + values['bucket_id'] = str(deposit.files.bucket) + except (AttributeError, NoResultFound): + # Hack, to make invenio_files_rest.views.as_uuid throw a + # ValueError instead of a TypeError if we set the value to None. + values['bucket_id'] = '' + + @cap_deposit_files_bp.url_defaults + def restore_pid_to_url(endpoint, values): + """Put ``pid_value`` back to the URL after matching Files-REST views. + Since we are computing the URL more than one times, we need the + original values of the request to be unchanged so that it can be + reproduced. + """ + # Value here has been saved in above method (resolve_pid_to_bucket_id) + values['pid_value'] = g.pid + + return cap_deposit_files_bp diff --git a/setup.py b/setup.py index 1c82db6f1e..355f2dda97 100644 --- a/setup.py +++ b/setup.py @@ -15,7 +15,7 @@ DATABASE = "postgresql" ELASTICSEARCH = "elasticsearch5" -INVENIO_VERSION = '3.0.0' # "3.0.0rc2" +INVENIO_VERSION = '3.0.0' # "3.0.0rc2" tests_require = [ 'check-manifest>=0.35', @@ -44,9 +44,7 @@ for reqs in extras_require.values(): extras_require['all'].extend(reqs) -extras_require['ldap'] = [ - 'python-ldap>=2.4.39' -] +extras_require['ldap'] = ['python-ldap>=2.4.39'] # Do not include in all requirement extras_require['xrootd'] = [ @@ -72,7 +70,7 @@ 'cachetools==3.1.0', # Pinned libraries - 'celery==4.1.1', # temporary fix + 'celery==4.1.1', # temporary fix # temporary pinned since there are 'connection closed' issues # on production server 'urllib3[secure]==1.22', @@ -95,7 +93,6 @@ # "raven" versions needed till we FIX dependecies on installation 'raven[flask]>=5.0.0,<5.5', 'invenio-logging[sentry]>=1.0.0b1', - 'uWSGI==2.0.17', 'uwsgi-tools==1.1.1', 'uwsgitop==0.10', @@ -103,11 +100,10 @@ packages = find_packages() - # Get the version string. Cannot be done with import! g = {} with open(os.path.join('cap', 'version.py'), 'rt') as fp: - exec(fp.read(), g) + exec (fp.read(), g) version = g['__version__'] setup( @@ -119,7 +115,8 @@ license='MIT', author='CERN', author_email='analysis-preservation-support@cern.ch', - url='https://github.com/cernanalysispreservation/analysispreservation.cern.ch', # noqa + url= + 'https://github.com/cernanalysispreservation/analysispreservation.cern.ch', # noqa packages=packages, zip_safe=False, include_package_data=True, @@ -175,7 +172,6 @@ 'cap.modules.records.minters:cap_record_minter', 'cap_deposit_minter = ' 'cap.modules.deposit.minters:cap_deposit_minter', - ], 'invenio_pidstore.fetchers': [ 'cap_record_fetcher = ' diff --git a/tests/integration/test_files_for_deposit.py b/tests/integration/test_files_for_deposit.py index 6d73b97eb0..39f7826cef 100644 --- a/tests/integration/test_files_for_deposit.py +++ b/tests/integration/test_files_for_deposit.py @@ -27,7 +27,6 @@ import json from flask import current_app -from mock import patch from pytest import mark from six import BytesIO @@ -46,21 +45,14 @@ def test_deposit_files_when_owner_can_see_files_list(client, users, headers=auth_headers_for_user(owner)) assert resp.status_code == 200 + bucket = resp.json - -def test_deposit_files_when_superuser_can_see_files_list( - client, users, auth_headers_for_superuser, create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - pid = deposit['_deposit']['id'] - - resp = client.get('/deposits/{}/files'.format(pid), - headers=auth_headers_for_superuser) - - assert resp.status_code == 200 + assert bucket['id'] == str(deposit.files.bucket) + assert bucket['locked'] is False + assert bucket['contents'] == [] -def test_deposit_files_when_other_user_returns_403(client, users, +def test_deposit_files_when_other_user_returns_404(client, users, auth_headers_for_user, create_deposit): owner, other_user = users['cms_user'], users['cms_user2'] @@ -70,7 +62,7 @@ def test_deposit_files_when_other_user_returns_403(client, users, resp = client.get('/deposits/{}/files'.format(pid), headers=auth_headers_for_user(other_user)) - assert resp.status_code == 403 + assert resp.status_code == 404 @mark.parametrize("action", [ @@ -100,130 +92,18 @@ def test_deposit_files_when_user_has_read_or_admin_permission_can_see_files_list assert resp.status_code == 200 -def test_deposit_files_when_no_files_returns_empty_list( - client, users, auth_headers_for_user, create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - pid = deposit['_deposit']['id'] - - resp = client.get('/deposits/{}/files'.format(pid), - headers=auth_headers_for_user(owner)) - - assert resp.json == [] - - -def test_deposit_files_returns_list_with_files_info(client, users, - auth_headers_for_user, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - deposit.files['file_2.txt'] = BytesIO(b'Hello brave new world!') - pid = deposit['_deposit']['id'] - - resp = client.get('/deposits/{}/files'.format(pid), - headers=auth_headers_for_user(owner)) - files = resp.json - - assert files[0]["filename"] == 'file_1.txt' - assert files[0]["filesize"] == 12 - - assert files[1]["filename"] == 'file_2.txt' - assert files[1]["filesize"] == 22 - - -######################################### -# api/files/{bucket_id} [GET] -######################################### -def test_bucket_read_when_nonexisting_bucket_returns_404( - client, users, auth_headers_for_user, create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - - resp = client.get('/files/nonexistingbucketid', - headers=auth_headers_for_user(owner)) - - assert resp.status_code == 404 - - -def test_bucket_read_when_owner_of_deposit_can_access(client, users, - auth_headers_for_user, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket - - resp = client.get('/files/{}'.format(bucket), - headers=auth_headers_for_user(owner)) - - assert resp.status_code == 200 - - -def test_bucket_read_when_superuser_can_access(client, users, - auth_headers_for_superuser, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket - - resp = client.get('/files/{}'.format(bucket), - headers=auth_headers_for_superuser) - - assert resp.status_code == 200 - - -def test_bucket_read_when_other_user_returns_404(client, users, - auth_headers_for_user, - create_deposit): - owner, other_user = users['cms_user'], users['cms_user2'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket - - resp = client.get('/files/{}'.format(bucket), - headers=auth_headers_for_user(other_user)) - - assert resp.status_code == 404 - - -@mark.parametrize("action", [ - ("deposit-read"), - ("deposit-update"), - ("deposit-admin"), -]) -def test_bucket_read_when_user_has_read_update_or_admin_access_can_access( - action, client, users, auth_headers_for_user, create_deposit, - json_headers): - owner, other_user = users['cms_user'], users['cms_user2'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - pid = deposit['_deposit']['id'] - bucket = deposit.files.bucket - permissions = [{ - 'email': other_user.email, - 'type': 'user', - 'op': 'add', - 'action': action - }] - - client.post('/deposits/{}/actions/permissions'.format(pid), - headers=auth_headers_for_user(owner) + json_headers, - data=json.dumps(permissions)) - - resp = client.get('/files/{}'.format(bucket), - headers=auth_headers_for_user(other_user)) - - assert resp.status_code == 200 - - ######################################### -# api/files/{bucket_id}/{filekey} [GET] +# api/deposits/{pid}/files/{filekey} [GET] ######################################### def test_file_read_when_given_nonexisting_filekey_returns_404( client, users, auth_headers_for_user, create_deposit): owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket + deposit = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!')}) + pid = deposit['_deposit']['id'] - resp = client.get('/files/{}/non-existing.txt'.format(bucket), + resp = client.get('/deposits/{}/files/non-existing.txt'.format(pid), headers=auth_headers_for_user(owner)) assert resp.status_code == 404 @@ -232,28 +112,16 @@ def test_file_read_when_given_nonexisting_filekey_returns_404( def test_file_read_when_owner_of_deposit_can_access_file( client, users, auth_headers_for_user, create_deposit): owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket + deposit = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!')}) + pid = deposit['_deposit']['id'] - resp = client.get('/files/{}/file_1.txt'.format(bucket), + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers_for_user(owner)) assert resp.status_code == 200 - - -def test_file_read_when_superuser_can_access_file(client, users, - auth_headers_for_superuser, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket - - resp = client.get('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_superuser) - - assert resp.status_code == 200 + assert resp.data == 'Hello world!' def test_file_read_when_other_user_returns_404(client, users, @@ -301,66 +169,59 @@ def test_file_read_when_user_has_read_update_or_admin_access_can_access( ######################################### -# api/files/{bucket_id}/{filekey} [PUT] +# api/deposits/{pid}/files/{filekey} [PUT] ######################################### -@mark.skip('This should be possible? or we solve problem from UI/client side') def test_file_upload_when_key_already_exists_returns_400( client, users, auth_headers_for_user, create_deposit): owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket - - resp = client.put('/files/{}/file_1.txt'.format(bucket), - input_stream=BytesIO(b'Hello world!'), - headers=auth_headers_for_user(owner)) - - assert resp.status_code == 400 - - -def test_file_upload_when_owner_of_deposit_can_upload(client, users, - auth_headers_for_user, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket + pid = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!') + })['_deposit']['id'] - resp = client.put('/files/{}/file_1.txt'.format(bucket), - input_stream=BytesIO(b'Hello world!'), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), + input_stream=BytesIO(b'Updated Hello world!'), headers=auth_headers_for_user(owner)) assert resp.status_code == 200 - -def test_file_upload_when_superuser_can_upload(client, users, - auth_headers_for_superuser, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket - - resp = client.put('/files/{}/file_1.txt'.format(bucket), - input_stream=BytesIO(b'Hello world!'), - headers=auth_headers_for_superuser) + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), + headers=auth_headers_for_user(owner)) assert resp.status_code == 200 + assert resp.data == 'Updated Hello world!' def test_file_upload_when_other_user_returns_404(client, users, auth_headers_for_user, create_deposit): owner, other_user = users['cms_user'], users['cms_user2'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket + pid = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!') + })['_deposit']['id'] - resp = client.put('/files/{}/file_1.txt'.format(bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Hello world!'), headers=auth_headers_for_user(other_user)) assert resp.status_code == 404 +def test_file_upload_when_owner_of_deposit_can_upload(client, users, + auth_headers_for_user, + create_deposit): + owner = users['cms_user'] + deposit = create_deposit(owner, 'test-analysis-v0.0.1') + pid = deposit['_deposit']['id'] + + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), + input_stream=BytesIO(b'Hello world!'), + headers=auth_headers_for_user(owner)) + + assert resp.status_code == 200 + + @mark.parametrize("action", [ ("deposit-update"), ("deposit-admin"), @@ -371,7 +232,6 @@ def test_file_upload_when_user_has_update_or_admin_access_can_upload( owner, other_user = users['cms_user'], users['cms_user2'] deposit = create_deposit(owner, 'test-analysis-v0.0.1') pid = deposit['_deposit']['id'] - bucket = deposit.files.bucket permissions = [{ 'email': other_user.email, 'type': 'user', @@ -383,7 +243,7 @@ def test_file_upload_when_user_has_update_or_admin_access_can_upload( headers=auth_headers_for_user(owner) + json_headers, data=json.dumps(permissions)) - resp = client.put('/files/{}/file_1.txt'.format(bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Hello world!'), headers=auth_headers_for_user(other_user)) @@ -395,7 +255,6 @@ def test_file_upload_when_user_has_only_read_access_returns_404( owner, other_user = users['cms_user'], users['cms_user2'] deposit = create_deposit(owner, 'test-analysis-v0.0.1') pid = deposit['_deposit']['id'] - bucket = deposit.files.bucket permissions = [{ 'email': other_user.email, 'type': 'user', @@ -407,33 +266,15 @@ def test_file_upload_when_user_has_only_read_access_returns_404( headers=auth_headers_for_user(owner) + json_headers, data=json.dumps(permissions)) - resp = client.put('/files/{}/file_1.txt'.format(bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Hello world!'), headers=auth_headers_for_user(other_user)) assert resp.status_code == 404 -def test_file_upload_uploads_successfully(client, users, auth_headers_for_user, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket - - client.put('/files/{}/file_1.txt'.format(bucket), - input_stream=BytesIO(b'Hello world!'), - headers=auth_headers_for_user(owner)) - - resp = client.get('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_user(owner)) - - assert resp.status_code == 200 - assert resp.data == 'Hello world!' - - def test_put_header_tags(client, users, auth_headers_for_user, create_deposit): """Test upload of an object with tags in the headers.""" - key = 'test.txt' headers = [ (current_app.config['FILES_REST_FILE_TAGS_HEADER'], 'key1=val1;key2=val2;key3=val3'), @@ -441,10 +282,10 @@ def test_put_header_tags(client, users, auth_headers_for_user, create_deposit): owner = users['cms_user'] deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket + pid = deposit['_deposit']['id'] resp = client.put( - '/files/{}/{}'.format(bucket, key), + '/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'updated_content'), headers=headers + auth_headers_for_user(owner), ) @@ -452,7 +293,7 @@ def test_put_header_tags(client, users, auth_headers_for_user, create_deposit): assert resp.status_code == 200 resp = client.get( - '/files/{}'.format(bucket), + 'deposits/{}/files/'.format(pid), headers=auth_headers_for_user(owner), ) @@ -466,22 +307,21 @@ def test_put_header_tags(client, users, auth_headers_for_user, create_deposit): def test_put_header_invalid_tags(client, users, auth_headers_for_user, create_deposit): """Test upload of an object with tags in the headers.""" - key = 'test.txt' header_name = current_app.config['FILES_REST_FILE_TAGS_HEADER'] + # We don't test zero-length values/keys, because they are filtered out + # from parse_qsl invalid = [ - # We don't test zero-length values/keys, because they are filtered out - # from parse_qsl ('a' * 256, 'valid'), ('valid', 'b' * 256), ] owner = users['cms_user'] deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket + pid = deposit['_deposit']['id'] # Invalid key or values for k, v in invalid: - resp = client.put('/files/{}/{}'.format(bucket, key), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'updated_content'), headers=[(header_name, '{}={}'.format(k, v))] + auth_headers_for_user(owner)) @@ -489,7 +329,7 @@ def test_put_header_invalid_tags(client, users, auth_headers_for_user, assert resp.status_code == 400 # Duplicate key - resp = client.put('/files/{}/{}'.format(bucket, key), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'updated_content'), headers=[(header_name, 'a=1&a=2')] + auth_headers_for_user(owner)) @@ -498,16 +338,15 @@ def test_put_header_invalid_tags(client, users, auth_headers_for_user, ######################################### -# api/files/{bucket_id}/{filekey} [DELETE] +# api/deposits/{pid}/files/{filekey} [DELETE] ######################################### def test_file_delete_when_key_doesnt_exist_returns_404(client, users, auth_headers_for_user, create_deposit): owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - bucket = deposit.files.bucket + pid = create_deposit(owner, 'test-analysis-v0.0.1')['_deposit']['id'] - resp = client.delete('/files/{}/nonexisting.txt'.format(bucket), + resp = client.delete('/deposits/{}/files/nonexisting.txt'.format(pid), headers=auth_headers_for_user(owner)) assert resp.status_code == 404 @@ -517,56 +356,30 @@ def test_file_delete_when_owner_of_deposit_can_delete(client, users, auth_headers_for_user, create_deposit): owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket + pid = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!') + })['_deposit']['id'] - resp = client.delete('/files/{}/file_1.txt'.format(bucket), + resp = client.delete('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers_for_user(owner)) assert resp.status_code == 204 -def test_file_delete_when_superuser_can_delete(client, users, - auth_headers_for_superuser, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket - - resp = client.delete('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_superuser) - - assert resp.status_code == 204 - - -def test_file_delete_when_other_user_returns_404(client, users, - auth_headers_for_user, - json_headers, create_deposit): - owner, other_user = users['cms_user'], users['cms_user2'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket - - resp = client.delete('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_user(other_user)) - - assert resp.status_code == 404 - - -@mark.parametrize("action", [ - ("deposit-update"), - ("deposit-admin"), +@mark.parametrize("action,status_code", [ + ("deposit-update", 204), + ("deposit-admin", 204), + ("deposit-read", 403), ]) -def test_file_delete_when_user_has_update_or_admin_access_can_delete( - action, client, users, auth_headers_for_user, create_deposit, - json_headers): +def test_file_delete_when_user_has_update_admin_or_read_access( + action, status_code, client, users, auth_headers_for_user, + create_deposit, json_headers): owner, other_user = users['cms_user'], users['cms_user2'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - pid = deposit['_deposit']['id'] - bucket = deposit.files.bucket + pid = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!') + })['_deposit']['id'] permissions = [{ 'email': other_user.email, 'type': 'user', @@ -578,47 +391,44 @@ def test_file_delete_when_user_has_update_or_admin_access_can_delete( headers=auth_headers_for_user(owner) + json_headers, data=json.dumps(permissions)) - resp = client.delete('/files/{}/file_1.txt'.format(bucket), + resp = client.delete('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers_for_user(other_user)) - assert resp.status_code == 204 + assert resp.status_code == status_code -def test_file_delete_when_user_has_only_read_access_returns_403( - client, users, auth_headers_for_user, create_deposit, json_headers): - owner, other_user = users['cms_user'], users['cms_user2'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - pid = deposit['_deposit']['id'] - bucket = deposit.files.bucket - permissions = [{ - 'email': other_user.email, - 'type': 'user', - 'op': 'add', - 'action': 'deposit-read' - }] +def test_file_delete_deletes_successfully(client, users, auth_headers_for_user, + create_deposit): + owner = users['cms_user'] + pid = create_deposit(owner, + 'test-analysis-v0.0.1', + files={'file_1.txt': BytesIO(b'Hello world!') + })['_deposit']['id'] - client.post('/deposits/{}/actions/permissions'.format(pid), - headers=auth_headers_for_user(owner) + json_headers, - data=json.dumps(permissions)) + resp = client.delete('/deposits/{}/files/file_1.txt'.format(pid), + headers=auth_headers_for_user(owner)) - resp = client.delete('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_user(other_user)) + assert resp.status_code == 204 - assert resp.status_code == 403 + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), + headers=auth_headers_for_user(owner)) + assert resp.status_code == 404 -def test_file_delete_delets_successfully(client, users, auth_headers_for_user, - create_deposit): - owner = users['cms_user'] - deposit = create_deposit(owner, 'test-analysis-v0.0.1') - deposit.files['file_1.txt'] = BytesIO(b'Hello world!') - bucket = deposit.files.bucket - client.delete('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_user(owner)) +######################################### +# /api/deposits/${draft_id}/actions/upload [POST] +######################################### +@mark.parametrize("type", ["url", "repo"]) +def test_upload_file_in_deposit_via_external_url_returns_400_when_url_is_not_correct( + type, client, users, auth_headers_for_user, json_headers, + create_deposit): + owner = users['cms_user'] + pid = create_deposit(owner, 'test-analysis-v0.0.1')['_deposit']['id'] + data = {'url': 'https://dream.team', 'type': type} - resp = client.get('/files/{}/file_1.txt'.format(bucket), - headers=auth_headers_for_user(owner)) + resp = client.post('/deposits/{}/actions/upload'.format(pid), + headers=auth_headers_for_user(owner) + json_headers, + data=json.dumps(data)) - assert resp.status_code == 404 + assert resp.status_code == 400 diff --git a/tests/integration/test_files_workflow.py b/tests/integration/test_files_workflow.py index 8edc20470b..01a1dc65fb 100644 --- a/tests/integration/test_files_workflow.py +++ b/tests/integration/test_files_workflow.py @@ -43,44 +43,44 @@ def test_files_workflow(client, users, auth_headers_for_user, create_deposit): assert deposit_bucket.locked is False # user can add new files - resp = client.put('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Original Hello world!'), headers=auth_headers) assert resp.status_code == 200 # and access them - resp = client.get('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers) assert resp.status_code == 200 assert resp.data == 'Original Hello world!' # member of collaboration cannot access file - resp = client.get('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers_for_user(member_of_collaboration)) - assert resp.status_code == 404 # TOFIX shouldnt be 403? + assert resp.status_code == 404 # user can update and delete a file - resp = client.put('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Updated Hello world!'), headers=auth_headers) assert resp.status_code == 200 - resp = client.get('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers) assert resp.status_code == 200 assert resp.data == 'Updated Hello world!' - resp = client.delete('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.delete('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers) assert resp.status_code == 204 - resp = client.put('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Original Hello world!'), headers=auth_headers) @@ -99,46 +99,48 @@ def test_files_workflow(client, users, auth_headers_for_user, create_deposit): # record has a differrent bucket, that is also locked _, record = deposit.fetch_published() + record_pid = record['control_number'] record_bucket = record.files.bucket + assert record_bucket != deposit_bucket assert record_bucket.locked is True # user and member of collaboration can access file for the published record - resp = client.get('/files/{}/file_1.txt'.format(record_bucket), + resp = client.get('/records/{}/files/file_1.txt'.format(record_pid), headers=auth_headers) assert resp.status_code == 200 - resp = client.get('/files/{}/file_1.txt'.format(record_bucket), + resp = client.get('/records/{}/files/file_1.txt'.format(record_pid), headers=auth_headers_for_user(member_of_collaboration)) assert resp.status_code == 200 # user outside of collaboration cant access file for the published record resp = client.get( - '/files/{}/file_1.txt'.format(record_bucket), + '/records/{}/files/file_1.txt'.format(record_pid), headers=auth_headers_for_user(non_member_of_collaboration)) assert resp.status_code == 404 - resp = client.get('/files/{}/file_1.txt'.format(record_bucket), + resp = client.get('/records/{}/files/file_1.txt'.format(record_pid), headers=auth_headers_for_user(member_of_collaboration)) assert resp.status_code == 200 # user cannot add/update/delete a file for deposit - resp = client.put('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'Try another Hello world!'), headers=auth_headers) assert resp.status_code == 403 - resp = client.delete('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.delete('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers) assert resp.status_code == 403 - resp = client.put('/files/{}/file_2.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_2.txt'.format(pid), input_stream=BytesIO(b'Try another Hello world!'), headers=auth_headers) @@ -153,37 +155,37 @@ def test_files_workflow(client, users, auth_headers_for_user, create_deposit): # after edit, deposit deposit_bucket is unlocked again, # so he can add new files - resp = client.put('/files/{}/file_2.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_2.txt'.format(pid), input_stream=BytesIO(b'Hello new world!'), headers=auth_headers) assert resp.status_code == 200 - resp = client.get('/files/{}/file_2.txt'.format(deposit_bucket), + resp = client.get('/deposits/{}/files/file_2.txt'.format(pid), headers=auth_headers) assert resp.status_code == 200 assert resp.data == 'Hello new world!' # user can upload a new version of file - resp = client.put('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_1.txt'.format(pid), input_stream=BytesIO(b'After edit Hello world!'), headers=auth_headers) assert resp.status_code == 200 - resp = client.get('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.get('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers) assert resp.data == 'After edit Hello world!' # even delete it - resp = client.delete('/files/{}/file_1.txt'.format(deposit_bucket), + resp = client.delete('/deposits/{}/files/file_1.txt'.format(pid), headers=auth_headers) assert resp.status_code == 204 # user can upload a new files - resp = client.put('/files/{}/file_3.txt'.format(deposit_bucket), + resp = client.put('/deposits/{}/files/file_3.txt'.format(pid), input_stream=BytesIO(b'Hello world!'), headers=auth_headers) @@ -191,23 +193,23 @@ def test_files_workflow(client, users, auth_headers_for_user, create_deposit): # original file with this name, referenced by bucket of published record, # remains unchanged - resp = client.get('/files/{}/file_1.txt'.format(record_bucket), + resp = client.get('/records/{}/files/file_1.txt'.format(record_pid), headers=auth_headers) assert resp.data == 'Original Hello world!' # user cannot modify(add, update, delete) a record bucket - resp = client.put('/files/{}/file_1.txt'.format(record_bucket), + resp = client.put('/records/{}/files/file_1.txt'.format(record_pid), input_stream=BytesIO(b'Updated Updated Hello world!'), headers=auth_headers) assert resp.status_code == 403 - resp = client.delete('/files/{}/file_1.txt'.format(record_bucket), + resp = client.delete('/records/{}/files/file_1.txt'.format(record_pid), headers=auth_headers) assert resp.status_code == 403 - resp = client.put('/files/{}/file_2.txt'.format(record_bucket), + resp = client.put('/records/{}/files/file_2.txt'.format(record_pid), input_stream=BytesIO(b'Hello new world!'), headers=auth_headers) From 223709b5f30e407fe7338725264ce13b83d38945 Mon Sep 17 00:00:00 2001 From: Anna Trzcinska Date: Wed, 23 Oct 2019 16:37:56 +0200 Subject: [PATCH 2/2] records: add /records/{pid}/files endpoints Signed-off-by: Anna Trzcinska --- cap/config.py | 3 -- cap/modules/records/views.py | 84 ++++++++++++++++++++++++++++++++++++ setup.py | 23 ++++------ 3 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 cap/modules/records/views.py diff --git a/cap/config.py b/cap/config.py index 03bb3a044e..a5a0c1cb6d 100644 --- a/cap/config.py +++ b/cap/config.py @@ -530,9 +530,6 @@ def _(x): 'application/basic+json': ('cap.modules.records.serializers' ':basic_json_v1_search') }, - 'files_serializers': { - 'application/json': ('cap.modules.deposit.serializers:files_response'), - }, 'search_class': 'cap.modules.deposit.search:CAPDepositSearch', 'search_factory_imp': 'cap.modules.search.query:cap_search_factory', 'item_route': '/deposits/<{0}:pid_value>'.format(_PID), diff --git a/cap/modules/records/views.py b/cap/modules/records/views.py new file mode 100644 index 0000000000..964541b37d --- /dev/null +++ b/cap/modules/records/views.py @@ -0,0 +1,84 @@ +# -*- coding: utf-8 -*- +# +# This file is part of CERN Analysis Preservation Framework. +# Copyright (C) 2016 CERN. +# +# CERN Analysis Preservation Framework is free software; you can redistribute +# it and/or modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of the +# License, or (at your option) any later version. +# +# CERN Analysis Preservation Framework is distributed in the hope that it will +# be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with CERN Analysis Preservation Framework; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307, USA. +# +# In applying this license, CERN does not +# waive the privileges and immunities granted to it by virtue of its status +# as an Intergovernmental Organization or submit itself to any jurisdiction. +"""Extra records views.""" + +from __future__ import absolute_import, print_function + +from flask import Blueprint, g +from invenio_files_rest.views import bucket_view, object_view +from sqlalchemy.orm.exc import NoResultFound + +from .api import CAPRecord + + +def create_cap_record_blueprint(app): + """Create blueprint from a Flask application. + :params app: A Flask application. + :returns: Configured blueprint. + """ + cap_records_bp = Blueprint('cap_records_files', __name__, url_prefix='') + + record_item_path = app.config['RECORDS_REST_ENDPOINTS']['recid'][ + 'item_route'] + cap_records_bp.add_url_rule('{}/files'.format(record_item_path), + view_func=bucket_view) + cap_records_bp.add_url_rule('{}/files/'.format(record_item_path), + view_func=object_view) + + @cap_records_bp.url_value_preprocessor + def resolve_pid_to_bucket_id(endpoint, values): + """Flask URL preprocessor to resolve pid to Bucket ID. + In the ``cap_records_bp`` we are gluing together Records-REST + and Files-REST APIs. Records-REST knows about PIDs but Files-REST does + not, this function will pre-process the URL so the PID is removed from + the URL and resolved to bucket ID which is injected into Files-REST + view calls: + ``/api///files/`` -> + ``/files//``. + """ + # Remove the 'pid_value' in order to match the Files-REST bucket view + # signature. Store the value in Flask global request object for later + # usage. + g.pid = values.pop('pid_value') + pid, _ = g.pid.data + + try: + record = CAPRecord.get_record(pid.object_uuid) + values['bucket_id'] = str(record.files.bucket) + except (AttributeError, NoResultFound): + # Hack, to make invenio_files_rest.views.as_uuid throw a + # ValueError instead of a TypeError if we set the value to None. + values['bucket_id'] = '' + + @cap_records_bp.url_defaults + def restore_pid_to_url(endpoint, values): + """Put ``pid_value`` back to the URL after matching Files-REST views. + Since we are computing the URL more than one times, we need the + original values of the request to be unchanged so that it can be + reproduced. + """ + # Value here has been saved in above method (resolve_pid_to_bucket_id) + values['pid_value'] = g.pid + + return cap_records_bp diff --git a/setup.py b/setup.py index 355f2dda97..3e41a9180b 100644 --- a/setup.py +++ b/setup.py @@ -15,7 +15,7 @@ DATABASE = "postgresql" ELASTICSEARCH = "elasticsearch5" -INVENIO_VERSION = '3.0.0' # "3.0.0rc2" +INVENIO_VERSION = '3.0.0' # "3.0.0rc2" tests_require = [ 'check-manifest>=0.35', @@ -34,9 +34,7 @@ ] extras_require = { - 'docs': [ - 'Sphinx>=1.5.1', - ], + 'docs': ['Sphinx>=1.5.1', ], 'tests': tests_require, } @@ -70,7 +68,7 @@ 'cachetools==3.1.0', # Pinned libraries - 'celery==4.1.1', # temporary fix + 'celery==4.1.1', # temporary fix # temporary pinned since there are 'connection closed' issues # on production server 'urllib3[secure]==1.22', @@ -116,15 +114,13 @@ author='CERN', author_email='analysis-preservation-support@cern.ch', url= - 'https://github.com/cernanalysispreservation/analysispreservation.cern.ch', # noqa + 'https://github.com/cernanalysispreservation/analysispreservation.cern.ch', # noqa packages=packages, zip_safe=False, include_package_data=True, platforms='any', entry_points={ - 'console_scripts': [ - 'cap = cap.cli:cli', - ], + 'console_scripts': ['cap = cap.cli:cli', ], 'invenio_access.actions': [ 'cms_access = ' 'cap.modules.experiments.permissions:cms_access_action', @@ -159,6 +155,7 @@ 'cap_schemas = cap.modules.schemas.views:blueprint', 'cap_auth = cap.modules.auth.views:blueprint', 'cap_repos = cap.modules.repoimporter.views:repos_bp', + 'cap_records = cap.modules.records.views:create_cap_record_blueprint', # noqa 'invenio_oauthclient = invenio_oauthclient.views.client:blueprint', ], 'invenio_celery.tasks': [ @@ -189,12 +186,8 @@ 'auth = cap.modules.auth.models', 'git_model = cap.modules.repoimporter.models', ], - 'invenio_db.alembic': [ - 'cap = cap:alembic', - ], - 'invenio_config.module': [ - 'cap = cap.config' - ] + 'invenio_db.alembic': ['cap = cap:alembic', ], + 'invenio_config.module': ['cap = cap.config'] }, extras_require=extras_require, install_requires=install_requires,