diff --git a/api/api.py b/api/api.py index d986423ef..36f609f98 100644 --- a/api/api.py +++ b/api/api.py @@ -243,8 +243,9 @@ def prefix(path, routes): route('/packfile', FileListHandler, h='packfile', m=['POST']), route('/packfile-end', FileListHandler, h='packfile_end'), route('/', FileListHandler, m=['POST']), - route('//', FileListHandler, m=['GET', 'DELETE']), + route('//', FileListHandler, m=['GET', 'PUT', 'DELETE']), route('///info', FileListHandler, h='get_info', m=['GET']), + route('///info', FileListHandler, h='modify_info', m=['POST']), route( '/analyses', AnalysesHandler, m=['POST']), prefix('/analyses', [ diff --git a/api/config.py b/api/config.py index c1fd29cc3..a04e08ee4 100644 --- a/api/config.py +++ b/api/config.py @@ -159,8 +159,10 @@ def apply_env_variables(config): 'container.json', 'device.json', 'file.json', + 'file-update.json', 'group-new.json', 'group-update.json', + 'info_update.json', 'note.json', 'packfile.json', 'permission.json', diff --git a/api/dao/liststorage.py b/api/dao/liststorage.py index 0720ec26b..027d783b6 100644 --- a/api/dao/liststorage.py +++ b/api/dao/liststorage.py @@ -1,10 +1,13 @@ import bson.errors import bson.objectid import datetime +import pymongo -from . import APIStorageException, APIConflictException +from . import APIStorageException, APIConflictException, APINotFoundException from . import consistencychecker from .. import config +from .. import util +from ..jobs import rules from .containerstorage import SessionStorage, AcquisitionStorage log = config.log @@ -41,7 +44,6 @@ def get_container(self, _id, query_params=None): '$elemMatch': query_params } projection = {self.list_name + '.$': 1, 'permissions': 1, 'public': 1} - log.debug('query {}'.format(query)) return self.dbc.find_one(query, projection) def exec_op(self, action, _id=None, query_params=None, payload=None, exclude_params=None): @@ -67,7 +69,6 @@ def exec_op(self, action, _id=None, query_params=None, payload=None, exclude_par raise ValueError('action should be one of GET, POST, PUT, DELETE') def _create_el(self, _id, payload, exclude_params): - log.debug('payload {}'.format(payload)) query = {'_id': _id} if exclude_params: query[self.list_name] = {'$not': {'$elemMatch': exclude_params} } @@ -75,16 +76,12 @@ def _create_el(self, _id, payload, exclude_params): '$push': {self.list_name: payload}, '$set': {'modified': datetime.datetime.utcnow()} } - log.debug('query {}'.format(query)) - log.debug('update {}'.format(update)) result = self.dbc.update_one(query, update) if result.matched_count < 1: raise APIConflictException('Item already exists in list.') return result def _update_el(self, _id, query_params, payload, exclude_params): - log.debug('query_params {}'.format(query_params)) - log.debug('payload {}'.format(payload)) mod_elem = {} for k,v in payload.items(): mod_elem[self.list_name + '.$.' + k] = v @@ -100,19 +97,14 @@ def _update_el(self, _id, query_params, payload, exclude_params): update = { '$set': mod_elem } - log.debug('query {}'.format(query)) - log.debug('update {}'.format(update)) return self.dbc.update_one(query, update) def _delete_el(self, _id, query_params): - log.debug('query_params {}'.format(query_params)) query = {'_id': _id} update = { '$pull': {self.list_name: query_params}, '$set': { 'modified': datetime.datetime.utcnow()} } - log.debug('query {}'.format(query)) - log.debug('update {}'.format(update)) result = self.dbc.update_one(query, update) if self.list_name is 'files' and self.cont_name in ['sessions', 'acquisitions']: if self.cont_name == 'sessions': @@ -123,16 +115,93 @@ def _delete_el(self, _id, query_params): return result def _get_el(self, _id, query_params): - log.debug('query_params {}'.format(query_params)) query = {'_id': _id, self.list_name: {'$elemMatch': query_params}} projection = {self.list_name + '.$': 1} - log.debug('query {}'.format(query)) - log.debug('projection {}'.format(projection)) result = self.dbc.find_one(query, projection) if result and result.get(self.list_name): return result.get(self.list_name)[0] +class FileStorage(ListStorage): + + def __init__(self, cont_name): + super(FileStorage,self).__init__(cont_name, 'files', use_object_id=True) + + + def _update_el(self, _id, query_params, payload, exclude_params): + container_before = self.get_container(_id) + if not container_before: + raise APINotFoundException('Could not find {} {}, file not updated.'.format( + _id, self.cont_name + )) + + mod_elem = {} + for k,v in payload.items(): + mod_elem[self.list_name + '.$.' + k] = v + query = {'_id': _id } + if exclude_params is None: + query[self.list_name] = {'$elemMatch': query_params} + else: + query['$and'] = [ + {self.list_name: {'$elemMatch': query_params}}, + {self.list_name: {'$not': {'$elemMatch': exclude_params} }} + ] + mod_elem['modified'] = datetime.datetime.utcnow() + update = { + '$set': mod_elem + } + + container_after = self.dbc.find_one_and_update(query, update, return_document=pymongo.collection.ReturnDocument.AFTER) + if not container_after: + raise APINotFoundException('Could not find and modify {} {}. file not updated'.format(_id, self.cont_name)) + + jobs_spawned = rules.create_jobs(config.db, container_before, container_after, self.cont_name) + + return { + 'modified': 1, + 'jobs_triggered': len(jobs_spawned) + } + + + def modify_info(self, _id, query_params, payload): + update = {} + set_payload = payload.get('set') + delete_payload = payload.get('delete') + replace_payload = payload.get('replace') + + if (set_payload or delete_payload) and replace_payload is not None: + raise APIStorageException('Cannot set or delete AND replace info fields.') + + if replace_payload is not None: + update = { + '$set': { + self.list_name + '.$.info': util.mongo_sanitize_fields(replace_payload) + } + } + + else: + if set_payload: + update['$set'] = {} + for k,v in set_payload.items(): + update['$set'][self.list_name + '.$.info.' + k] = util.mongo_sanitize_fields(v) + if delete_payload: + update['$unset'] = {} + for k in delete_payload: + update['$unset'][self.list_name + '.$.info.' + k] = '' + + if self.use_object_id: + _id = bson.objectid.ObjectId(_id) + query = {'_id': _id } + query[self.list_name] = {'$elemMatch': query_params} + + if not update.get('$set'): + update['$set'] = {'modified': datetime.datetime.utcnow()} + else: + update['$set']['modified'] = datetime.datetime.utcnow() + + return self.dbc.update_one(query, update) + + class StringListStorage(ListStorage): """ This class provides access to string sublists of a mongodb collections elements (called containers). diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index 282fb347b..29fbe1196 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -37,7 +37,7 @@ def initialize_list_configurations(): 'input_schema_file': 'tag.json' }, 'files': { - 'storage': liststorage.ListStorage, + 'storage': liststorage.FileStorage, 'permchecker': listauth.default_sublist, 'use_object_id': True, 'storage_schema_file': 'file.json', @@ -87,11 +87,14 @@ def initialize_list_configurations(): for cont_name, cont_config in list_container_configurations.iteritems(): for list_name, list_config in cont_config.iteritems(): storage_class = list_config['storage'] - storage = storage_class( - cont_name, - list_name, - use_object_id=list_config.get('use_object_id', False) - ) + if list_name == 'files': + storage = storage_class(cont_name) + else: + storage = storage_class( + cont_name, + list_name, + use_object_id=list_config.get('use_object_id', False) + ) list_config['storage'] = storage return list_container_configurations @@ -501,6 +504,25 @@ def get(self, cont_name, list_name, **kwargs): def get_info(self, cont_name, list_name, **kwargs): return super(FileListHandler,self).get(cont_name, list_name, **kwargs) + def modify_info(self, cont_name, list_name, **kwargs): + _id = kwargs.pop('cid') + permchecker, storage, _, _, _ = self._initialize_request(cont_name, list_name, _id, query_params=kwargs) + + payload = self.request.json_body + + validators.validate_data(payload, 'info_update.json', 'input', 'POST') + + try: + permchecker(noop)('PUT', _id=_id, query_params=kwargs, payload=payload) + result = storage.modify_info(_id, kwargs, payload) + except APIStorageException as e: + self.abort(400, e.message) + # abort if the query of the update wasn't able to find any matching documents + if result.matched_count == 0: + self.abort(404, 'Element not updated in list {} of container {} {}'.format(storage.list_name, storage.cont_name, _id)) + else: + return {'modified':result.modified_count} + def post(self, cont_name, list_name, **kwargs): _id = kwargs.pop('cid') @@ -517,6 +539,16 @@ def post(self, cont_name, list_name, **kwargs): return upload.process_upload(self.request, upload.Strategy.targeted, container_type=cont_name, id_=_id, origin=self.origin) + def put(self, cont_name, list_name, **kwargs): + _id = kwargs.pop('cid') + permchecker, storage, _, _, _ = self._initialize_request(cont_name, list_name, _id, query_params=kwargs) + + payload = self.request.json_body + validators.validate_data(payload, 'file-update.json', 'input', 'PUT') + + result = permchecker(storage.exec_op)('PUT', _id=_id, query_params=kwargs, payload=payload) + return result + def delete(self, cont_name, list_name, **kwargs): # Overriding base class delete to audit action before completion _id = kwargs.pop('cid') diff --git a/raml/schemas/definitions/file.json b/raml/schemas/definitions/file.json index 10e4bec13..73d9b1eb6 100644 --- a/raml/schemas/definitions/file.json +++ b/raml/schemas/definitions/file.json @@ -43,6 +43,15 @@ }, "additionalProperties": false }, + "file-update":{ + "type": "object", + "properties": { + "type": {"$ref":"#/definitions/file-type"}, + "modality": {"$ref":"#/definitions/modality"}, + "measurements": {"$ref":"#/definitions/measurements"} + }, + "additionalProperties": false + }, "file-output":{ "type": "object", "properties": { diff --git a/raml/schemas/input/file-update.json b/raml/schemas/input/file-update.json new file mode 100644 index 000000000..698750c20 --- /dev/null +++ b/raml/schemas/input/file-update.json @@ -0,0 +1,4 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf":[{"$ref":"../definitions/file.json#/definitions/file-update"}] +} diff --git a/raml/schemas/input/info_update.json b/raml/schemas/input/info_update.json new file mode 100644 index 000000000..353780b0d --- /dev/null +++ b/raml/schemas/input/info_update.json @@ -0,0 +1,25 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "Helper endpoint for editing an object's info key", + "type": "object", + "oneOf": [ + { + "properties": { + "set": {"type": "object", "minProperties": 1}, + "delete": { + "type": "array", + "uniqueItems": true, + "minItems": 1, + "items": { + "type": "string" + } + } + }, "additionalProperties": false + }, + { + "properties": { + "replace": {"type": "object"} + }, "additionalProperties": false + } + ] +} diff --git a/test/integration_tests/python/test_containers.py b/test/integration_tests/python/test_containers.py index c7d45ac2c..f57942d1f 100644 --- a/test/integration_tests/python/test_containers.py +++ b/test/integration_tests/python/test_containers.py @@ -534,3 +534,162 @@ def test_analysis_put(data_builder, as_admin): r = as_admin.put('/sessions/'+session + '/analyses/' + analysis) assert r.status_code == 400 +def test_edit_file_attributes(data_builder, as_admin, file_form): + project = data_builder.create_project() + file_name = 'test_file.txt' + + assert as_admin.post('/projects/' + project + '/files', files=file_form(file_name)).ok + + payload = { + 'type': 'new type', + 'modality': 'new modality', + 'measurements': ['measurement'] + } + + assert as_admin.put('/projects/' + project + '/files/' + file_name, json=payload).ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + + file_object = r.json() + assert file_object['type'] == payload['type'] + assert file_object['measurements'] == payload['measurements'] + assert file_object['modality'] == payload['modality'] + + + # Attempt to set forbidden fields + payload = { + 'name': 'new_file_name.txt' + } + r = as_admin.put('/projects/' + project + '/files/' + file_name, json=payload) + assert r.status_code == 400 + + payload = { + 'info': {} + } + r = as_admin.put('/projects/' + project + '/files/' + file_name, json=payload) + assert r.status_code == 400 + + payload = { + 'mimetype': 'bad data' + } + r = as_admin.put('/projects/' + project + '/files/' + file_name, json=payload) + assert r.status_code == 400 + + +def test_edit_file_info(data_builder, as_admin, file_form): + project = data_builder.create_project() + file_name = 'test_file.txt' + + r = as_admin.post('/projects/' + project + '/files', files=file_form(file_name)) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == {} + + # Send improper payload + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'delete': ['map'], + 'replace': {'not_going': 'to_happen'} + }) + assert r.status_code == 400 + + # Send improper payload + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'delete': {'a': 'map'}, + }) + assert r.status_code == 400 + + # Send improper payload + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'set': 'cannot do this', + }) + assert r.status_code == 400 + + # Attempt full replace of info + file_info = { + 'a': 'b', + 'test': 123, + 'map': { + 'a': 'b' + }, + 'list': [1,2,3] + } + + + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'replace': file_info + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == file_info + + + # Use 'set' to add new key + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'set': {'map': 'no longer a map'} + }) + assert r.ok + + file_info['map'] = 'no longer a map' + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == file_info + + + # Use 'set' to do full replace of "map" key + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'set': {'map': 'no longer a map'} + }) + assert r.ok + + file_info['map'] = 'no longer a map' + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == file_info + + + # Use 'delete' to unset "map" key + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'delete': ['map', 'a'] + }) + assert r.ok + + file_info.pop('map') + file_info.pop('a') + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == file_info + + + # Use 'delete' on keys that do not exist + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'delete': ['madeup', 'keys'] + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == file_info + + + # Use 'replace' to set file info to {} + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/info', json={ + 'replace': {} + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['info'] == {} + + + + + + + +