diff --git a/api/api.py b/api/api.py index db7a9dc70..d6ee70b24 100644 --- a/api/api.py +++ b/api/api.py @@ -9,6 +9,7 @@ from .handlers.devicehandler import DeviceHandler from .handlers.grouphandler import GroupHandler from .handlers.listhandler import FileListHandler, NotesListHandler, PermissionsListHandler, TagsListHandler +from .handlers.modalityhandler import ModalityHandler from .handlers.refererhandler import AnalysesHandler from .handlers.reporthandler import ReportHandler from .handlers.resolvehandler import ResolveHandler @@ -177,6 +178,15 @@ def prefix(path, routes): ]), + # Modalities + + route( '/modalities', ModalityHandler, h='get_all', m=['GET']), + route( '/modalities', ModalityHandler, m=['POST']), + prefix('/modalities', [ + route('/', ModalityHandler, m=['GET', 'PUT', 'DELETE']), + ]), + + # Site route('//rules', RulesHandler, m=['GET', 'POST']), @@ -268,13 +278,14 @@ def prefix(path, routes): route('/', TagsListHandler, m=['POST']), route('//', TagsListHandler, m=['GET', 'PUT', 'DELETE']), - route('/packfile-start', FileListHandler, h='packfile_start', m=['POST']), - route('/packfile', FileListHandler, h='packfile', m=['POST']), - route('/packfile-end', FileListHandler, h='packfile_end'), - route('/', FileListHandler, m=['POST']), - route('//', FileListHandler, m=['GET', 'PUT', 'DELETE']), - route('///info', FileListHandler, h='get_info', m=['GET']), - route('///info', FileListHandler, h='modify_info', m=['POST']), + route('/packfile-start', FileListHandler, h='packfile_start', m=['POST']), + route('/packfile', FileListHandler, h='packfile', m=['POST']), + route('/packfile-end', FileListHandler, h='packfile_end'), + route('/', FileListHandler, m=['POST']), + route('//', FileListHandler, m=['GET', 'PUT', 'DELETE']), + route('///info', FileListHandler, h='get_info', m=['GET']), + route('///info', FileListHandler, h='modify_info', m=['POST']), + route('///classification', FileListHandler, h='modify_classification', m=['POST']), route( '//analyses', AnalysesHandler, h='get_all', m=['GET']), route( '/analyses', AnalysesHandler, h='get_all', m=['GET']), diff --git a/api/config.py b/api/config.py index c2bbaf696..999cc7767 100644 --- a/api/config.py +++ b/api/config.py @@ -1,6 +1,5 @@ import os import copy -import glob import json import logging import pymongo @@ -132,76 +131,6 @@ def apply_env_variables(config): # validate the lists of json schemas schema_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), '../swagger/schemas') -expected_mongo_schemas = set([ - 'acquisition.json', - 'analysis.json', - 'collection.json', - 'container.json', - 'file.json', - 'group.json', - 'note.json', - 'permission.json', - 'project.json', - 'session.json', - 'subject.json', - 'user.json', - 'avatars.json', - 'tag.json' -]) -expected_input_schemas = set([ - 'acquisition.json', - 'acquisition-update.json', - 'analysis.json', - 'analysis-job.json', - 'analysis-update.json', - 'avatars.json', - 'collection.json', - 'collection-update.json', - 'device.json', - 'file.json', - 'file-update.json', - 'group-new.json', - 'group-update.json', - 'info_update.json', - 'note.json', - 'packfile.json', - 'permission.json', - 'project.json', - 'project-template.json', - 'project-update.json', - 'rule-new.json', - 'rule-update.json', - 'session.json', - 'session-update.json', - 'subject.json', - 'user-new.json', - 'user-update.json', - 'download.json', - 'tag.json', - 'enginemetadata.json', - 'labelupload.json', - 'uidupload.json', - 'uidmatchupload.json' -]) -mongo_schemas = set() -input_schemas = set() - -# check that the lists of schemas are correct -for schema_filepath in glob.glob(schema_path + '/mongo/*.json'): - schema_file = os.path.basename(schema_filepath) - mongo_schemas.add(schema_file) - with open(schema_filepath, 'rU') as f: - pass - -assert mongo_schemas == expected_mongo_schemas, '{} is different from {}'.format(mongo_schemas, expected_mongo_schemas) - -for schema_filepath in glob.glob(schema_path + '/input/*.json'): - schema_file = os.path.basename(schema_filepath) - input_schemas.add(schema_file) - with open(schema_filepath, 'rU') as f: - pass - -assert input_schemas == expected_input_schemas, '{} is different from {}'.format(input_schemas, expected_input_schemas) def create_or_recreate_ttl_index(coll_name, index_name, ttl): if coll_name in db.collection_names(): diff --git a/api/dao/basecontainerstorage.py b/api/dao/basecontainerstorage.py index 9484833f4..8d3d260b3 100644 --- a/api/dao/basecontainerstorage.py +++ b/api/dao/basecontainerstorage.py @@ -188,8 +188,20 @@ def update_el(self, _id, payload, unset_payload=None, recursive=False, r_payload raise APIStorageException(e.message) if recursive and r_payload is not None: containerutil.propagate_changes(self.cont_name, _id, {}, {'$set': util.mongo_dict(r_payload)}) + + config.log.warning(update) return self.dbc.update_one({'_id': _id}, update) + def replace_el(self, _id, payload): + if self.use_object_id: + try: + _id = bson.ObjectId(_id) + except bson.errors.InvalidId as e: + raise APIStorageException(e.message) + payload['_id'] = _id + return self.dbc.replace_one({'_id': _id}, payload) + + def delete_el(self, _id): if self.use_object_id: try: diff --git a/api/dao/hierarchy.py b/api/dao/hierarchy.py index e3bce623a..e64832bc0 100644 --- a/api/dao/hierarchy.py +++ b/api/dao/hierarchy.py @@ -144,7 +144,15 @@ def check_req(cont, req_k, req_v): """ Return True if container satisfies specific requirement. """ - cont_v = cont.get(req_k) + + # If looking at classification, translate to list rather than dictionary + if req_k == 'classification': + cont_v = [] + for v in cont.get('classification', {}).itervalues(): + cont_v.extend(v) + else: + cont_v = cont.get(req_k) + if cont_v: if isinstance(req_v, dict): for k,v in req_v.iteritems(): diff --git a/api/dao/liststorage.py b/api/dao/liststorage.py index 12d38ca61..1d0d16e85 100644 --- a/api/dao/liststorage.py +++ b/api/dao/liststorage.py @@ -1,13 +1,14 @@ import bson.errors import bson.objectid +import copy import datetime -import pymongo from ..web.errors import APIStorageException, APIConflictException, APINotFoundException from . import consistencychecker from .. import config from .. import util from ..jobs import rules +from ..handlers.modalityhandler import check_and_format_classification from .containerstorage import SessionStorage, AcquisitionStorage log = config.log @@ -115,12 +116,23 @@ def _get_el(self, _id, query_params): if result and result.get(self.list_name): return result.get(self.list_name)[0] + def _update_session_compliance(self, _id): + if self.cont_name in ['sessions', 'acquisitions']: + if self.cont_name == 'sessions': + session_id = _id + else: + session_id = AcquisitionStorage().get_container(_id).get('session') + SessionStorage().recalc_session_compliance(session_id) + class FileStorage(ListStorage): def __init__(self, cont_name): super(FileStorage,self).__init__(cont_name, 'files', use_object_id=True) + def _create_jobs(self, container_before): + container_after = self.get_container(container_before['_id']) + return rules.create_jobs(config.db, container_before, container_after, self.cont_name) def _update_el(self, _id, query_params, payload, exclude_params): container_before = self.get_container(_id) @@ -145,11 +157,9 @@ def _update_el(self, _id, query_params, payload, exclude_params): '$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) + self.dbc.find_one_and_update(query, update) + self._update_session_compliance(_id) + jobs_spawned = self._create_jobs(container_before) return { 'modified': 1, @@ -162,12 +172,7 @@ def _delete_el(self, _id, query_params): if f['name'] == query_params['name']: f['deleted'] = datetime.datetime.utcnow() result = self.dbc.update_one({'_id': _id}, {'$set': {'files': files, 'modified': datetime.datetime.utcnow()}}) - if self.cont_name in ['sessions', 'acquisitions']: - if self.cont_name == 'sessions': - session_id = _id - else: - session_id = AcquisitionStorage().get_container(_id).get('session') - SessionStorage().recalc_session_compliance(session_id) + self._update_session_compliance(_id) return result def _get_el(self, _id, query_params): @@ -215,7 +220,61 @@ def modify_info(self, _id, query_params, payload): else: update['$set']['modified'] = datetime.datetime.utcnow() - return self.dbc.update_one(query, update) + result = self.dbc.update_one(query, update) + self._update_session_compliance(_id) + return result + + + def modify_classification(self, _id, query_params, payload): + container_before = self.get_container(_id) + update = {'$set': {'modified': datetime.datetime.utcnow()}} + + if self.use_object_id: + _id = bson.objectid.ObjectId(_id) + query = {'_id': _id } + query[self.list_name] = {'$elemMatch': query_params} + + modality = self.get_container(_id)['files'][0].get('modality') #TODO: make this more reliable if the file isn't there + add_payload = payload.get('add') + delete_payload = payload.get('delete') + replace_payload = payload.get('replace') + + if (add_payload or delete_payload) and replace_payload is not None: + raise APIStorageException('Cannot add or delete AND replace classification fields.') + + if replace_payload is not None: + replace_payload = check_and_format_classification(modality, replace_payload) + + r_update = copy.deepcopy(update) + r_update['$set'][self.list_name + '.$.classification'] = util.mongo_sanitize_fields(replace_payload) + + self.dbc.update_one(query, r_update) + + else: + if add_payload: + add_payload = check_and_format_classification(modality, add_payload) + + a_update = copy.deepcopy(update) + a_update['$addToSet'] = {} + for k,v in add_payload.iteritems(): + a_update['$addToSet'][self.list_name + '.$.classification.' + k] = {'$each': v} + + self.dbc.update_one(query, a_update) + + if delete_payload: + delete_payload = check_and_format_classification(modality, delete_payload) + + # TODO: Test to make sure $pull succeeds when key does not exist + d_update = copy.deepcopy(update) + d_update['$pullAll'] = {} + for k,v in delete_payload.iteritems(): + d_update['$pullAll'][self.list_name + '.$.classification.' + k] = v + + self.dbc.update_one(query, d_update) + + self._update_session_compliance(_id) + self._create_jobs(container_before) + class StringListStorage(ListStorage): diff --git a/api/handlers/collectionshandler.py b/api/handlers/collectionshandler.py index 703b277cc..13f2bbf47 100644 --- a/api/handlers/collectionshandler.py +++ b/api/handlers/collectionshandler.py @@ -168,8 +168,7 @@ def get_sessions(self, cid): sessions = list(containerstorage.SessionStorage().get_all_el(query, None, projection)) self._filter_all_permissions(sessions, self.uid) - if self.is_true('measurements'): - self._add_session_measurements(sessions) + for sess in sessions: sess = self.handle_origin(sess) return sessions diff --git a/api/handlers/containerhandler.py b/api/handlers/containerhandler.py index 2bc5c5609..fe5d7b08d 100644 --- a/api/handlers/containerhandler.py +++ b/api/handlers/containerhandler.py @@ -346,10 +346,6 @@ def get_all(self, cont_name, par_cont_name=None, par_id=None): # the "count" flag add a count for each container returned if self.is_true('counts'): self._add_results_counts(results, cont_name) - # the "measurements" flag applies only to query for sessions - # and add a list of the measurements in the child acquisitions - if cont_name == 'sessions' and self.is_true('measurements'): - self._add_session_measurements(results) modified_results = [] for result in results: @@ -620,19 +616,6 @@ def _get_validators(self): payload_validator = validators.from_schema_path(payload_schema_uri) return mongo_validator, payload_validator - def _add_session_measurements(self, results): - session_measurements = config.db.acquisitions.aggregate([ - {'$match': {'session': {'$in': [sess['_id'] for sess in results]}}}, - {'$project': { '_id': '$session', 'files':1 }}, - {'$unwind': '$files'}, - {'$project': { '_id': '$_id', 'files.measurements': 1}}, - {'$unwind': '$files.measurements'}, - {'$group': {'_id': '$_id', 'measurements': {'$addToSet': '$files.measurements'}}} - ]) - session_measurements = {sess['_id']: sess['measurements'] for sess in session_measurements} - for sess in results: - sess['measurements'] = session_measurements.get(sess['_id'], None) - def _get_parent_container(self, payload): if not self.config.get('parent_storage'): return None, None diff --git a/api/handlers/dataexplorerhandler.py b/api/handlers/dataexplorerhandler.py index 1e47702e6..4ad946829 100644 --- a/api/handlers/dataexplorerhandler.py +++ b/api/handlers/dataexplorerhandler.py @@ -206,9 +206,9 @@ "filter": {"term": {"container_type": "file"}}, "aggs": { - "file.measurements" : { + "file.classification" : { "terms" : { - "field" : "file.measurements.raw", + "field" : "file.classification.raw", "size" : 15, "missing": "null" } @@ -278,7 +278,7 @@ SOURCE_FILE = SOURCE_ANALYSIS + [ "file.created", - "file.measurements", + "file.classification", "file.name", "file.size", "file.type", diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index 5f731a7e9..6331b0139 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -195,7 +195,7 @@ def _initialize_request(self, cont_name, list_name, _id, query_params=None): else: permchecker = permchecker(self, container) else: - self.abort(404, 'Element {} not found in container {}'.format(_id, storage.cont_name)) + self.abort(404, 'Element {} not found in {} {}'.format(query_params.values()[0], containerutil.singularize(storage.cont_name), _id)) mongo_schema_uri = validators.schema_uri('mongo', conf.get('storage_schema_file')) mongo_validator = validators.decorator_from_schema_path(mongo_schema_uri) @@ -539,8 +539,18 @@ def modify_info(self, cont_name, list_name, **kwargs): # 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 modify_classification(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, 'classification-update.json', 'input', 'POST') + + permchecker(noop)('PUT', _id=_id, query_params=kwargs, payload=payload) + storage.modify_classification(_id, kwargs, payload) + def post(self, cont_name, list_name, **kwargs): _id = kwargs.pop('cid') diff --git a/api/handlers/modalityhandler.py b/api/handlers/modalityhandler.py new file mode 100644 index 000000000..b5baee3aa --- /dev/null +++ b/api/handlers/modalityhandler.py @@ -0,0 +1,170 @@ +from ..web import base +from .. import config +from ..auth import require_login, require_admin +from ..dao import containerstorage +from ..web.errors import APINotFoundException, APIValidationException +#from ..validators import validate_data + +log = config.log + + +class ModalityHandler(base.RequestHandler): + + def __init__(self, request=None, response=None): + super(ModalityHandler, self).__init__(request, response) + self.storage = containerstorage.ContainerStorage('modalities', use_object_id=False) + + @require_login + def get(self, modality_name): + return self.storage.get_container(modality_name) + + @require_login + def get_all(self): + return self.storage.get_all_el(None, None, None) + + @require_admin + def post(self): + payload = self.request.json_body + # Clean this up when validate_data method is fixed to use new schemas + # POST unnecessary, used to avoid run-time modification of schema + #validate_data(payload, 'modality.json', 'input', 'POST', optional=True) + + result = self.storage.create_el(payload) + return {'_id': result.inserted_id} + + @require_admin + def put(self, modality_name): + payload = self.request.json_body + # Clean this up when validate_data method is fixed to use new schemas + # POST unnecessary, used to avoid run-time modification of schema + #validate_data(payload, 'modality.json', 'input', 'POST', optional=True) + + result = self.storage.replace_el(modality_name, payload) + if result.matched_count != 1: + raise APINotFoundException('Modality with name {} not found, modality not updated'.format(modality_name)) + + @require_admin + def delete(self, modality_name): + result = self.storage.delete_el(modality_name) + if result.deleted_count != 1: + raise APINotFoundException('Modality with name {} not found, modality not deleted'.format(modality_name)) + + +class APIClassificationException(APIValidationException): + def __init__(self, modality, errors): + + if not modality: + error_msg = 'Unknown modality. Classification must be set under "custom" key' + else: + error_msg = 'Classification does not match format for modality {}. Unallowable key-value pairs: {}'.format(modality, errors) + + super(APIClassificationException, self).__init__(error_msg) + self.errors = {'unaccepted_keys': errors} + + +def case_insensitive_search(classifications, proposed_key, proposed_value): + """ + Looks for value in given classification map, returning: + + 1) found - a boolean that is true if the proposed_value was found, false if not + 2) key_name - the key name of the classification list where it was found + 3) value - the formatted string that should be saved to the file's classification + + NOTE: If the proposed_value was not found, key_name and value will be None + + This function is used mainly to preserve the existing stylization of the strings stored on + the modalities set classifications. + """ + + for k in classifications.iterkeys(): + if k.lower() == proposed_key.lower(): + for v in classifications[k]: + if v.lower() == proposed_value.lower(): + + # Both key and value were found + return True, k, v + + # Key was found but not value + return False, None, None + + # key was not found + return False, None, None + + +def check_and_format_classification(modality_name, classification_map): + """ + Given a modality name and a proposed classification map, + ensure: + - that a modality exists with that name and has a classification + map + - all keys in the classification_map exist in the + `classifications` map on the modality object + - all the values in the arrays in the classification_map + exist in the modality's classifications map + + And then return a map with the keys and values properly formatted + + For example: + Modality = { + "_id" = "Example_modality", + "classifications": { + "Example1": ["Blue", "Green"] + "Example2": ["one", "two"] + } + } + + Returns properly formatted classification map: + classification_map = { + "Example1": ["blue"], + "custom": ["anything"] + } + + Raises APIClassificationException: + classification_map = { + "Example1": ["Red"], # "Red" is not allowed + "Example2": ["one", "two"] + } + """ + try: + modality = containerstorage.ContainerStorage('modalities', use_object_id=False).get_container(modality_name) + except APINotFoundException: + keys = classification_map.keys() + if len(keys) == 1 and keys[0].lower() == 'custom': + # for unknown modalities allow only list of custom values + return classification_map + else: + raise APIClassificationException(None, []) + + classifications = modality.get('classification', {}) + + formatted_map = {} # the formatted map that will be returned + bad_kvs = [] # a list of errors to report, formatted like ['k:v', 'k:v', 'k:v'] + + for k,array in classification_map.iteritems(): + if k.lower() == 'custom': + # any unique value is allowed in custom list + formatted_map['Custom'] = array + + else: + for v in array: + + allowed, fk, fv = case_insensitive_search(classifications, k, v) + + if allowed: + if fk in formatted_map: + formatted_map[fk].append(fv) + else: + formatted_map[fk] = [fv] + + else: + bad_kvs.append(k+':'+v) + + if bad_kvs: + raise APIClassificationException(modality_name, bad_kvs) + + return formatted_map + + + + + diff --git a/api/jobs/queue.py b/api/jobs/queue.py index 89ea8cc10..1df08e47b 100644 --- a/api/jobs/queue.py +++ b/api/jobs/queue.py @@ -231,9 +231,20 @@ def enqueue_job(job_map, origin, perm_check_uid=None): cr = create_containerreference_from_filereference(inputs[x]) # Whitelist file fields passed to gear to those that are scientific-relevant - whitelisted_keys = ['info', 'tags', 'measurements', 'mimetype', 'type', 'modality', 'size'] + whitelisted_keys = ['info', 'tags', 'classification', 'mimetype', 'type', 'modality', 'size'] obj_projection = { key: obj.get(key) for key in whitelisted_keys } + ### + # recreate `measurements` list on object + # Can be removed when `classification` key has been adopted everywhere + + obj_projection['measurements'] = [] + if obj_projection.get('classification'): + for v in obj_projection['classification'].itervalues(): + obj_projection.extend(v) + # + ### + config_['inputs'][x] = { 'base': 'file', 'hierarchy': cr.__dict__, diff --git a/api/jobs/rules.py b/api/jobs/rules.py index b111baf59..0027c001d 100644 --- a/api/jobs/rules.py +++ b/api/jobs/rules.py @@ -1,5 +1,6 @@ import fnmatch import re +import itertools from .. import config from ..types import Origin @@ -33,7 +34,7 @@ # 'value': '*.dcm' # }, # { -# 'type': 'file.measurements', # Match any of the file's measurements +# 'type': 'file.classification', # Match any of the file's classification # 'value': 'diffusion' # }, # { @@ -41,7 +42,7 @@ # 'value': 'bvec' # }, # { -# 'type': 'container.has-measurement', # Match the container having any file (including this one) with this measurement +# 'type': 'container.has-classification', # Match the container having any file (including this one) with this classification # 'value': 'functional' # } # ] @@ -86,10 +87,11 @@ def match(value): elif match_type == 'file.name': return match(file_['name']) - # Match any of the file's measurements - elif match_type == 'file.measurements': + # Match any of the file's classification + elif match_type == 'file.classification': if match_param: - return any(match(value) for value in file_.get('measurements', [])) + classification_values = list(itertools.chain.from_iterable(file_.get('classification', {}).itervalues())) + return any(match(value) for value in classification_values) else: return False @@ -102,11 +104,12 @@ def match(value): return False - # Match the container having any file (including this one) with this measurement - elif match_type == 'container.has-measurement': + # Match the container having any file (including this one) with this classification + elif match_type == 'container.has-classification': if match_param: for c_file in container['files']: - if any(match(value) for value in c_file.get('measurements', [])): + classification_values = list(itertools.chain.from_iterable(c_file.get('classification', {}).itervalues())) + if any(match(value) for value in classification_values): return True return False diff --git a/api/placer.py b/api/placer.py index 77aa4eacf..6a3f24adb 100644 --- a/api/placer.py +++ b/api/placer.py @@ -263,27 +263,16 @@ def check(self): validators.validate_data(self.metadata, 'enginemetadata.json', 'input', 'POST', optional=True) ### - # Remove when switch to dmv2 is complete across all gears - c_metadata = self.metadata.get(self.container_type, {}) # pragma: no cover - if self.context.get('job_id') and c_metadata and not c_metadata.get('files', []): # pragma: no cover - job = Job.get(self.context.get('job_id')) - input_names = [{'name': v.name} for v in job.inputs.itervalues()] - - measurement = self.metadata.get(self.container_type, {}).pop('measurement', None) - info = self.metadata.get(self.container_type,{}).pop('metadata', None) - modality = self.metadata.get(self.container_type, {}).pop('instrument', None) - if measurement or info or modality: - files_ = self.metadata[self.container_type].get('files', []) - files_ += input_names - for f in files_: - if measurement: - f['measurements'] = [measurement] - if info: - f['info'] = info - if modality: - f['modality'] = modality - - self.metadata[self.container_type]['files'] = files_ + # Shuttle `measurements` key into `classification` on files + ### + + if self.metadata.get(self.container_type, {}): # pragma: no cover + + for f in self.metadata[self.container_type].get('files', []): + + if 'measurements' in f: + m = f.pop('measurements') + f['classification'] = {'Custom': m} ### def process_file_field(self, field, file_attrs): @@ -567,7 +556,7 @@ def finalize(self): # OPPORTUNITY: packfile endpoint could be extended someday to take additional metadata. 'modality': None, - 'measurements': [], + 'classification': {}, 'tags': [], 'info': {}, diff --git a/api/upload.py b/api/upload.py index 75de2e470..916e39c4a 100644 --- a/api/upload.py +++ b/api/upload.py @@ -127,7 +127,7 @@ def process_upload(request, strategy, container_type=None, id_=None, origin=None 'type': None, 'modality': None, - 'measurements': [], + 'classification': {}, 'tags': [], 'info': {} } diff --git a/bin/database.py b/bin/database.py index 375c4b76f..e5c34b441 100755 --- a/bin/database.py +++ b/bin/database.py @@ -22,7 +22,7 @@ from api.types import Origin from api.jobs import batch -CURRENT_DATABASE_VERSION = 42 # An int that is bumped when a new schema change is made +CURRENT_DATABASE_VERSION = 43 # An int that is bumped when a new schema change is made def get_db_version(): @@ -1360,6 +1360,115 @@ def upgrade_to_42(): process_cursor(cursor, upgrade_to_42_closure, context=cont_name) +def upgrade_files_to_43(cont, cont_name): + """ + if the file has a modality, we try to find a matching classification + key and value for each measurement in the modality's classification map + + if there is no modality or the modality cannot be found in the modalities + collection, all measurements are added to the custom key + """ + + files = cont['files'] + for f in cont['files']: + modality = f.get('modality') + measurements = f['measurements'] + modality_container = None + + if modality: + modality_container = config.db.modalities.find_one({'_id': modality}) + + if modality_container: + classification = {} + m_class = modality_container.get('classifications', {}) + + for m in measurements: + found = False + for k, v_array in m_class.iteritems(): + for v in v_array: + if v.lower() == m.lower(): + found = True + if classification.get(k): + classification[k].append(v) + else: + classification[k] = [v] + if not found: + if classification.get('Custom'): + classification['Custom'].append(m) + else: + classification['Custom'] = [m] + + else: + classification = {'Custom': measurements} + + f.pop('measurements') + f['classification'] = classification + + + config.db[cont_name].update_one({'_id': cont['_id']}, {'$set': {'files': files}}) + + return True + +def upgrade_rules_to_43(rule): + + def adjust_type(r): + if r['type'] == 'file.measurement': + r['type'] = 'file.classification' + elif r['type'] == 'container.has-measurement': + r['type'] = 'container.has-classification' + + for r in rule.get('any', []): + adjust_type(r) + + for r in rule.get('all', []): + adjust_type(r) + + config.db.project_rules.replace_one({'_id': rule['_id']}, rule) + + return True + +def upgrade_templates_to_43(project): + """ + Set any measurements keys to classification + """ + + template = project['template'] + + for a in template.get('acquisitions', []): + for f in a.get('files', []): + if 'measurements' in f: + cl = f.pop('measurements') + f['classification'] = cl + + config.db.projects.update_one({'_id': project['_id']}, {'$set': {'template': template}}) + + return True + +def upgrade_to_43(): + """ + Update classification for all files with existing measurements field + """ + + + for cont_name in ['groups', 'projects', 'collections', 'sessions', 'acquisitions', 'analyses']: + + cursor = config.db[cont_name].find({'files.measurements': {'$exists': True }}) + process_cursor(cursor, upgrade_files_to_43, context=cont_name) + + + cursor = config.db.project_rules.find({'$or': [ + {'all.type': {'$in': ['file.measurement', 'container.has-measurement']}}, + {'any.type': {'$in': ['file.measurement', 'container.has-measurement']}} + ]}) + process_cursor(cursor, upgrade_rules_to_43) + + cursor = config.db.projects.find({'template': {'$exists': True }}) + process_cursor(cursor, upgrade_templates_to_43) + + + + + ### ### BEGIN RESERVED UPGRADE SECTION ### diff --git a/swagger/examples/file_info_list.json b/swagger/examples/file_info_list.json index 70992e72e..34837d9af 100755 --- a/swagger/examples/file_info_list.json +++ b/swagger/examples/file_info_list.json @@ -8,7 +8,7 @@ "hash": "v0-sha384-12188e00a26650b2baa3f0195337dcf504f4362bb2136eef0cdbefb57159356b1355a0402fca0ab5ab081f21c305e5c2", "name": "cortical_surface_right_hemisphere.obj", "tags": [], - "measurements": [], + "classification": {}, "modified": "2016-10-18T15:26:35.701000+00:00", "modality": null, "size": 21804112, @@ -23,7 +23,7 @@ "hash": "v0-sha384-12188e00a26650b2baa3f0195337dcf504f4362bb2136eef0cdbefb57159356b1355a0402fca0ab5ab081f21c305e5c2", "name": "cortical_surface_right_hemisphere.obj", "tags": [], - "measurements": [], + "classification": {}, "modified": "2016-10-18T17:45:17.776000+00:00", "modality": null, "info": {}, diff --git a/swagger/examples/input/analysis.json b/swagger/examples/input/analysis.json index 57a68f4fe..4efde2a56 100644 --- a/swagger/examples/input/analysis.json +++ b/swagger/examples/input/analysis.json @@ -8,7 +8,7 @@ "hash": "v0-sha384-12188e00a26650b2baa3f0195337dcf504f4362bb2136eef0cdbefb57159356b1355a0402fca0ab5ab081f21c305e5c2", "name": "cortical_surface_right_hemisphere.obj", "tags": [], - "measurements": [], + "classification": {}, "modified": "2016-10-18T15:26:35.701000+00:00", "modality": null, "input": true, @@ -25,7 +25,7 @@ "hash": "v0-sha384-12188e00a26650b2baa3f0195337dcf504f4362bb2136eef0cdbefb57159356b1355a0402fca0ab5ab081f21c305e5c2", "name": "cortical_surface_right_hemisphere.obj", "tags": [], - "measurements": [], + "classification": {}, "modified": "2016-10-18T17:45:17.776000+00:00", "modality": null, "output": true, @@ -43,5 +43,5 @@ "description": "This is an analysis description", "job": "54759eb3c090d83494e2d804", "label": "Analysis label", - "user": "canakgun@flywheel.io" + "user": "canakgun@flywheel.io" } diff --git a/swagger/examples/input/classification-update.json b/swagger/examples/input/classification-update.json new file mode 100644 index 000000000..4c632e016 --- /dev/null +++ b/swagger/examples/input/classification-update.json @@ -0,0 +1,6 @@ +{ + "replace": { + "Contrast": ["B0", "Diffusion"], + "Intent": ["Structural"] + } +} diff --git a/swagger/examples/input/file.json b/swagger/examples/input/file.json index 798e3edc0..cefaf9fdf 100644 --- a/swagger/examples/input/file.json +++ b/swagger/examples/input/file.json @@ -2,7 +2,7 @@ "name": "4784_1_1_localizer_dicom.zip", "type": "dicom", "mimetype": "application/zip", - "measurements": [], + "classification": {}, "tags": [], "info": {} } diff --git a/swagger/examples/input/modality.json b/swagger/examples/input/modality.json new file mode 100644 index 000000000..19aebadf6 --- /dev/null +++ b/swagger/examples/input/modality.json @@ -0,0 +1,8 @@ +{ + "_id": "MR", + "classifications": { + "Contrast": ["B0", "B1", "T1", "T2", "T2*", "PD", "MT", "ASL", "Perfusion", "Diffusion", "Spectroscopy", "Susceptibility", "Velocity", "Fingerprinting"], + "Intent": ["Structural", "Functional", "Localizer", "Shim", "Calibration"], + "Features": ["Quantitative", "Multi-Shell", "Multi-Echo", "Multi-Flip", "Multi-Band", "Steady-State", "3D", "Compressed-Sensing", "Eddy-Current-Corrected", "Fieldmap-Corrected", "Gradient-Unwarped", "Motion-Corrected", "Physio-Corrected"] + } +} diff --git a/swagger/examples/input/project-template.json b/swagger/examples/input/project-template.json index a185b97dd..abd5353df 100644 --- a/swagger/examples/input/project-template.json +++ b/swagger/examples/input/project-template.json @@ -15,11 +15,11 @@ "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", "properties": { - "measurement": { + "classification": { "type": "string", "pattern": "^[aA]natomical$" } }, - "required": ["measurement"] + "required": ["classification"] }, "minimum": 2 }, @@ -28,11 +28,11 @@ "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", "properties": { - "measurement": { + "classification": { "type": "string", "pattern": "^functional$" } }, - "required": ["measurement"] + "required": ["classification"] }, "minimum": 1 }, @@ -41,13 +41,13 @@ "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", "properties": { - "measurement": { "enum": ["Localizer"] }, + "classification": { "enum": ["Localizer"] }, "label": { "type": "string", "pattern": "t1" } }, - "required": ["label", "measurement"] + "required": ["label", "classification"] }, "minimum": 1 } diff --git a/swagger/examples/input/rule-new.json b/swagger/examples/input/rule-new.json index f730cd4c8..66b147e54 100644 --- a/swagger/examples/input/rule-new.json +++ b/swagger/examples/input/rule-new.json @@ -6,7 +6,7 @@ "all": [ { "regex": true, - "type": "file.measurements", + "type": "file.classification", "value": "^(?!non-image).+$" }, { diff --git a/swagger/examples/output/acquisition-list.json b/swagger/examples/output/acquisition-list.json index 1b29d562c..e452cf6a1 100644 --- a/swagger/examples/output/acquisition-list.json +++ b/swagger/examples/output/acquisition-list.json @@ -9,7 +9,7 @@ "name": "Admin Import" }, "mimetype": "application/zip", - "measurements": [], + "classification": {}, "hash": "v0-sha384-dd3c97bfe0ad1fcba75ae6718c6e81038c59af4f447f5db194d52732efa4f955b28455db02eb64cad3e4e55f11e3679f", "name": "4784_1_1_localizer_dicom.zip", "tags": [], @@ -48,7 +48,7 @@ "name": "Admin Import" }, "mimetype": "application/zip", - "measurements": [], + "classification": {}, "hash": "v0-sha384-ca055fb36845db86e4278cf6e185f8674d11a96f4b29af27e401fc495cc82ef6b53a5729c3757713064649dc71c8c725", "name": "4784_3_1_t1_dicom.zip", "tags": [], @@ -87,7 +87,7 @@ "name": "Admin Import" }, "mimetype": "application/zip", - "measurements": [], + "classification": {}, "hash": "v0-sha384-537e42b1dd8f1feef9844fbfb4f60461361e71cafa7055556097e9d0b9f7fac68c8f234ed126af9412bd43a548948847", "name": "4784_5_1_fmri_dicom.zip", "tags": [], diff --git a/swagger/examples/output/acquisition.json b/swagger/examples/output/acquisition.json index fef6ba738..945dbb8ee 100644 --- a/swagger/examples/output/acquisition.json +++ b/swagger/examples/output/acquisition.json @@ -8,7 +8,7 @@ "name": "Admin Import" }, "mimetype": "application/zip", - "measurements": [], + "classification": {}, "hash": "v0-sha384-dd3c97bfe0ad1fcba75ae6718c6e81038c59af4f447f5db194d52732efa4f955b28455db02eb64cad3e4e55f11e3679f", "name": "4784_1_1_localizer_dicom.zip", "tags": [], diff --git a/swagger/examples/output/analysis.json b/swagger/examples/output/analysis.json index bb0c4caae..0c4d700f3 100644 --- a/swagger/examples/output/analysis.json +++ b/swagger/examples/output/analysis.json @@ -8,7 +8,7 @@ "hash": "v0-sha384-12188e00a26650b2baa3f0195337dcf504f4362bb2136eef0cdbefb57159356b1355a0402fca0ab5ab081f21c305e5c2", "name": "cortical_surface_right_hemisphere.obj", "tags": [], - "measurements": [], + "classification": {}, "modified": "2016-10-18T15:26:35.701000+00:00", "modality": null, "input": true, @@ -24,7 +24,7 @@ "hash": "v0-sha384-12188e00a26650b2baa3f0195337dcf504f4362bb2136eef0cdbefb57159356b1355a0402fca0ab5ab081f21c305e5c2", "name": "cortical_surface_right_hemisphere.obj", "tags": [], - "measurements": [], + "classification": {}, "modified": "2016-10-18T17:45:17.776000+00:00", "modality": null, "output": true, diff --git a/swagger/examples/output/rule-list.json b/swagger/examples/output/rule-list.json index eb6126a22..d7248d76b 100644 --- a/swagger/examples/output/rule-list.json +++ b/swagger/examples/output/rule-list.json @@ -4,7 +4,7 @@ "all": [ { "regex": true, - "type": "file.measurements", + "type": "file.classification", "value": "^(?!non-image).+$" }, { @@ -25,7 +25,7 @@ }, { "regex": true, - "type": "file.measurements", + "type": "file.classification", "value": "^(?!non-image).+$" } ], @@ -42,7 +42,7 @@ }, { "regex": true, - "type": "file.measurements", + "type": "file.classification", "value": "(functional|anatomy_t1w|anatomy_t2w)" } ], diff --git a/swagger/schemas/definitions/file.json b/swagger/schemas/definitions/file.json index f6cc81590..0ef2526c4 100644 --- a/swagger/schemas/definitions/file.json +++ b/swagger/schemas/definitions/file.json @@ -5,11 +5,7 @@ "file-type": { "type": "string" }, "mimetype": { "type": "string" }, "modality": { "type": "string" }, - "measurements": { - "items": { "type": "string"}, - "type": "array", - "uniqueItems": true - }, + "classification": { "type": "object" }, "tags": { "items": { "type": "string"}, "type": "array", @@ -36,7 +32,7 @@ "additionalProperties":false }, "hash":{ - "type":"string", + "type":"string", "minLength":106, "maxLength":106 }, @@ -53,40 +49,40 @@ {"type":"null"} ] }, - "measurements": {"$ref":"#/definitions/measurements"}, - "tags": {"$ref":"#/definitions/tags"}, - "info": {"$ref":"common.json#/definitions/info"}, - "origin": {"$ref":"#/definitions/file-origin"}, - "hash": {"$ref":"#/definitions/hash"}, - "created": {"$ref":"created-modified.json#/definitions/created"}, - "modified": {"$ref":"created-modified.json#/definitions/modified"}, - "size": {"$ref":"#/definitions/size"}, - "info_exists": {"type": "boolean"}, - "input": {"type":"boolean"}, - "output": {"type":"boolean"} + "classification": {"$ref":"#/definitions/classification"}, + "tags": {"$ref":"#/definitions/tags"}, + "info": {"$ref":"common.json#/definitions/info"}, + "origin": {"$ref":"#/definitions/file-origin"}, + "hash": {"$ref":"#/definitions/hash"}, + "created": {"$ref":"created-modified.json#/definitions/created"}, + "modified": {"$ref":"created-modified.json#/definitions/modified"}, + "size": {"$ref":"#/definitions/size"}, + "info_exists": {"type": "boolean"}, + "input": {"type":"boolean"}, + "output": {"type":"boolean"} }, "additionalProperties": false }, "file-input":{ "type": "object", "properties": { - "name": {"$ref":"#/definitions/name"}, - "type": {"$ref":"#/definitions/file-type"}, - "mimetype": {"$ref":"#/definitions/mimetype"}, - "modality": {"$ref":"#/definitions/modality"}, - "measurements": {"$ref":"#/definitions/measurements"}, - "tags": {"$ref":"#/definitions/tags"}, - "info": {"$ref":"common.json#/definitions/info"} + "name": {"$ref":"#/definitions/name"}, + "type": {"$ref":"#/definitions/file-type"}, + "mimetype": {"$ref":"#/definitions/mimetype"}, + "modality": {"$ref":"#/definitions/modality"}, + "classification": {"$ref":"#/definitions/classification"}, + "tags": {"$ref":"#/definitions/tags"}, + "info": {"$ref":"common.json#/definitions/info"} }, "additionalProperties": false }, "file-update":{ "type": "object", "properties": { - "type": {"$ref":"#/definitions/file-type"}, - "modality": {"$ref":"#/definitions/modality"}, - "measurements": {"$ref":"#/definitions/measurements"} - }, + "type": {"$ref":"#/definitions/file-type"}, + "modality": {"$ref":"#/definitions/modality"}, + "classification": {"$ref":"#/definitions/classification"} + }, "additionalProperties": false }, "file-output":{ diff --git a/swagger/schemas/definitions/modality.json b/swagger/schemas/definitions/modality.json new file mode 100644 index 000000000..bb84ab294 --- /dev/null +++ b/swagger/schemas/definitions/modality.json @@ -0,0 +1,30 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "definitions": { + "_id": { + "maxLength": 64, + "minLength": 2, + "pattern": "^[0-9a-zA-Z_-]*$" + }, + "classifications": { + "type": "object", + "patternProperties": { + "^[0-9a-zA-Z_-]*$":{ + "type": "array", + "items": { + "type": "string" + } + } + } + }, + "modality":{ + "type": "object", + "properties": { + "_id": {"$ref":"#/definitions/_id"}, + "classifications": {"$ref":"#/definitions/classifications"} + }, + "additionalProperties": false + } + } +} diff --git a/swagger/schemas/definitions/rule.json b/swagger/schemas/definitions/rule.json index 3e2585774..d2cb57bbb 100644 --- a/swagger/schemas/definitions/rule.json +++ b/swagger/schemas/definitions/rule.json @@ -11,9 +11,9 @@ "enum": [ "file.type", "file.name", - "file.measurements", + "file.classification", "container.has-type", - "container.has-measurement" + "container.has-classification" ] }, "value": { "type": "string" }, diff --git a/swagger/schemas/input/classification-update.json b/swagger/schemas/input/classification-update.json new file mode 100644 index 000000000..fc5d017c1 --- /dev/null +++ b/swagger/schemas/input/classification-update.json @@ -0,0 +1,18 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "Helper endpoint for editing a file's classification key", + "type": "object", + "oneOf": [ + { + "properties": { + "add": {"$ref":"../definitions/file.json#/definitions/classification"}, + "delete": {"$ref":"../definitions/file.json#/definitions/classification"} + }, "additionalProperties": false + }, + { + "properties": { + "replace": {"$ref":"../definitions/file.json#/definitions/classification"} + }, "additionalProperties": false + } + ] +} diff --git a/swagger/schemas/mongo/file.json b/swagger/schemas/mongo/file.json index 4e077d85a..d4ae60135 100644 --- a/swagger/schemas/mongo/file.json +++ b/swagger/schemas/mongo/file.json @@ -10,11 +10,7 @@ "size": { "type": "integer" }, "hash": { "type": "string" }, "modality": { "type": "string" }, - "measurements": { - "items": { "type": "string"}, - "type": "array", - "uniqueItems": true - }, + "classification": { "type": "object" }, "tags": { "items": { "type": "string"}, "type": "array", diff --git a/tests/integration_tests/python/test_classification.py b/tests/integration_tests/python/test_classification.py new file mode 100644 index 000000000..7bad23ae4 --- /dev/null +++ b/tests/integration_tests/python/test_classification.py @@ -0,0 +1,282 @@ +def test_modalities(data_builder, as_admin, as_user): + + payload = { + '_id': 'MR', + 'classification': { + 'Intent': ["Structural", "Functional", "Localizer"], + 'Contrast': ["B0", "B1", "T1", "T2"] + } + } + + # test adding new modality + r = as_admin.post('/modalities', json=payload) + assert r.ok + assert r.json()['_id'] == payload['_id'] + modality1 = payload['_id'] + + # get specific modality + r = as_user.get('/modalities/' + modality1) + assert r.ok + assert r.json() == payload + + # try replacing existing modality via POST + r = as_admin.post('/modalities', json=payload) + assert r.status_code == 409 + + # list modalities as non-admin + r = as_user.get('/modalities') + assert r.ok + modalities = r.json() + assert len(modalities) == 1 + assert modalities[0]['_id'] == modality1 + + # replace existing modality + update = { + 'classification': { + 'Intent': ["new", "stuff"] + } + } + r = as_admin.put('/modalities/' + modality1, json=update) + assert r.ok + r = as_admin.get('/modalities/' + modality1) + assert r.ok + assert r.json()['classification'] == update['classification'] + + # try to replace missing modality + r = as_admin.put('/modalities/' + 'madeup', json=update) + assert r.status_code == 404 + + # delete modality + r = as_admin.delete('/modalities/' + modality1) + assert r.ok + + # try to delete missing modality + r = as_admin.delete('/modalities/' + modality1) + assert r.status_code == 404 + + +def test_edit_file_classification(data_builder, as_admin, as_user, file_form): + + ## Setup + + # Add file + 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()['classification'] == {} + + + # add modality information + payload = { + '_id': 'MR', + 'classification': { + 'Intent': ["Structural", "Functional", "Localizer"], + 'Contrast': ["B0", "B1", "T1", "T2"] + } + } + + r = as_admin.post('/modalities', json=payload) + assert r.ok + assert r.json()['_id'] == payload['_id'] + modality1 = payload['_id'] + + # Add modality to file + r = as_admin.put('/projects/' + project + '/files/' + file_name, json={ + 'modality': 'MR' + }) + + + ## Classification editing + + # Send improper payload + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'delete': ['this', 'is'], + 'replace': {'not_going': 'to_happen'} + }) + assert r.status_code == 400 + + # Send improper payload + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'delete': ['should', 'be', 'a', 'map'] + }) + assert r.status_code == 400 + + # Send improper payload + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'set': 'cannot do this' + }) + assert r.status_code == 400 + + # Attempt full replace of classification + file_cls = { + 'Intent': ['Structural'], + 'Contrast': ['B1', 'T1'], + 'Custom': ['Custom Value'] + } + + + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'replace': file_cls + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + + # Use 'add' to add new key to list + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'add': {'Intent': ['Functional']} + }) + assert r.ok + + file_cls['Intent'].append('Functional') + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + + # Remove item from list + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'delete': {'Intent': ['Structural'], + 'Contrast': ['B1']} + }) + assert r.ok + + file_cls['Intent'] = ['Functional'] + file_cls['Contrast'] = ['T1'] + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # Add and delete from same list + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'add': {'Intent': ['Localizer']}, + 'delete': {'Intent': ['Functional']} + }) + assert r.ok + + file_cls['Intent'] = ['Localizer'] + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # Use 'delete' on keys that do not exist + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'delete': {'Intent': ['Structural', 'Functional']} + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # Use 'add' on keys that already exist + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'add': {'Intent': ['Localizer']} + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # Ensure lowercase gets formatted in correct format via modality's classification + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'add': {'contrast': ['t2', 'b0'], 'custom': ['lowercase']} + }) + assert r.ok + + file_cls['Contrast'].extend(['T2', 'B0']) + file_cls['Custom'].append('lowercase') + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # Ensure lowercase gets formatted in correct format via modality's classification + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'delete': {'contrast': ['t2'], 'custom': ['lowercase']} + }) + assert r.ok + + file_cls['Contrast'] = ['T1', 'B0'] + file_cls['Custom'] = ['Custom Value'] + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # Try to replace with bad key names and values + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'replace': { + 'made-up': ['fake'], + 'Intent': ['not real'] + } + }) + assert r.status_code == 422 + assert r.json()['unaccepted_keys'] == ['made-up:fake', 'Intent:not real'] + + + # Use 'replace' to set file classification to {} + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'replace': {} + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == {} + + # Add Custom field for unknown modality + r = as_admin.put('/projects/' + project + '/files/' + file_name, json={ + 'modality': 'new unknown' + }) + + file_cls = { + 'Custom': ['Custom Value'] + } + + # allows custom fields + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'replace': file_cls + }) + assert r.ok + + r = as_admin.get('/projects/' + project + '/files/' + file_name + '/info') + assert r.ok + assert r.json()['classification'] == file_cls + + # does not allow non-custom fields + file_cls = { + 'Intent': ['Structural'] + } + + r = as_admin.post('/projects/' + project + '/files/' + file_name + '/classification', json={ + 'replace': file_cls + }) + assert r.status_code == 422 + + + # Attempt to add to nonexistent file + r = as_admin.post('/projects/' + project + '/files/' + 'madeup.txt' + '/classification', json={ + 'add': {'Intent': ['Localizer']} + }) + assert r.status_code == 404 + + # Attempt to delete from nonexistent file + r = as_admin.post('/projects/' + project + '/files/' + 'madeup.txt' + '/classification', json={ + 'delete': {'Intent': ['Localizer']} + }) + assert r.status_code == 404 + + # Attempt to replae nonexistent file + r = as_admin.post('/projects/' + project + '/files/' + 'madeup.txt' + '/classification', json={ + 'replace': {'Intent': ['Localizer']} + }) + assert r.status_code == 404 + diff --git a/tests/integration_tests/python/test_containers.py b/tests/integration_tests/python/test_containers.py index 792481a7d..f41a9e6fb 100644 --- a/tests/integration_tests/python/test_containers.py +++ b/tests/integration_tests/python/test_containers.py @@ -63,6 +63,8 @@ def test_project_template(data_builder, file_form, as_admin): assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('non-compliant.txt')).ok assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('compliant1.csv')).ok assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('compliant2.csv')).ok + assert as_admin.post('/acquisitions/' + acquisition_2 + '/files/compliant1.csv/classification', json={'add': {'custom': ['diffusion']}}) + assert as_admin.post('/acquisitions/' + acquisition_2 + '/files/compliant2.csv/classification', json={'add': {'custom': ['diffusion']}}) # test the session before setting the template r = as_admin.get('/sessions/' + session) @@ -79,6 +81,7 @@ def test_project_template(data_builder, file_form, as_admin): 'files': [{ 'minimum': 2, 'mimetype': 'text/csv', + 'classification': 'diffusion' }] }] }) @@ -95,6 +98,7 @@ def test_project_template(data_builder, file_form, as_admin): 'files': [{ 'minimum': 2, 'mimetype': 'text/csv', + 'classification': 'diffusion' }] }] }) @@ -152,6 +156,8 @@ def satisfies_template(): assert as_admin.delete('/acquisitions/' + acquisition_2 + '/files/compliant2.csv').ok assert not satisfies_template() assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('compliant2.csv')).ok + assert not satisfies_template() + assert as_admin.post('/acquisitions/' + acquisition_2 + '/files/compliant2.csv/classification', json={'add': {'custom': ['diffusion']}}) # acquisitions.minimum assert satisfies_template() @@ -612,7 +618,7 @@ def test_edit_file_attributes(data_builder, as_admin, file_form): payload = { 'type': 'new type', 'modality': 'new modality', - 'measurements': ['measurement'] + 'classification': {'custom': ['measurement']} } assert as_admin.put('/projects/' + project + '/files/' + file_name, json=payload).ok @@ -622,7 +628,7 @@ def test_edit_file_attributes(data_builder, as_admin, file_form): file_object = r.json() assert file_object['type'] == payload['type'] - assert file_object['measurements'] == payload['measurements'] + assert file_object['classification'] == payload['classification'] assert file_object['modality'] == payload['modality'] diff --git a/tests/integration_tests/python/test_rules.py b/tests/integration_tests/python/test_rules.py index 0616e3311..439fc6102 100644 --- a/tests/integration_tests/python/test_rules.py +++ b/tests/integration_tests/python/test_rules.py @@ -42,7 +42,7 @@ def test_site_rules(randstr, data_builder, as_admin, as_user, as_public): 'name': 'invalid-regex-rule', 'any': [], 'all': [ - {'type': 'file.measurements', 'value': invalid_pattern, 'regex': True}, + {'type': 'file.classification', 'value': invalid_pattern, 'regex': True}, ] }) assert r.status_code == 422 @@ -101,7 +101,7 @@ def test_site_rules(randstr, data_builder, as_admin, as_user, as_public): # attempt to modify site rule with invalid regex r = as_admin.put('/site/rules/' + rule_id, json={ 'all': [ - {'type': 'file.measurements', 'value': invalid_pattern, 'regex': True}, + {'type': 'file.classification', 'value': invalid_pattern, 'regex': True}, ] }) assert r.status_code == 422 @@ -359,10 +359,10 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a # NOTE this is a legacy rule r = as_admin.post('/projects/' + project + '/rules', json={ 'alg': gear_name, - 'name': 'txt-job-trigger-rule-with-measurement', + 'name': 'txt-job-trigger-rule-with-classification', 'any': [ - {'type': 'container.has-measurement', 'value': 'functional'}, - {'type': 'container.has-measurement', 'value': 'anatomical'} + {'type': 'container.has-classification', 'value': 'functional'}, + {'type': 'container.has-classification', 'value': 'anatomical'} ], 'all': [ {'type': 'file.type', 'value': 'text'}, @@ -379,14 +379,14 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear_2})] assert len(gear_jobs) == 1 # still 1 from before - # update test2.csv's metadata to include a valid measurement to spawn job + # update test2.csv's metadata to include a valid classification to spawn job metadata = { 'project':{ 'label': 'rule project', 'files': [ { 'name': 'test2.csv', - 'measurements': ['functional'] + 'classification': {'intent': ['functional']} } ] } @@ -398,7 +398,7 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a ) assert r.ok - # Ensure file without type or measurements does not cause issues with rule evalution + # Ensure file without type or classification does not cause issues with rule evalution # upload file that matches only part of rule r = as_admin.post('/projects/' + project + '/files', files=file_form('test3.notreal')) assert r.ok @@ -417,7 +417,7 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a # NOTE this is a legacy rule r = as_admin.post('/projects/' + project + '/rules', json={ 'alg': gear_name, - 'name': 'file-measurement-regex', + 'name': 'file-classification-regex', 'any': [], 'all': [ {'type': 'file.name', 'value': 'test\d+\.(csv|txt)', 'regex': True}, @@ -438,8 +438,6 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a r = as_admin.delete('/projects/' + project + '/rules/' + rule3) assert r.ok - # TODO add and test 'new-style' rules - def test_disabled_rules(randstr, data_builder, api_db, as_admin, file_form): # Create gear, project and *disabled* rule triggering on any csv (once enabled) @@ -477,3 +475,4 @@ def test_disabled_rules(randstr, data_builder, api_db, as_admin, file_form): assert len(gear_jobs) == 1 assert len(gear_jobs[0]['inputs']) == 1 assert gear_jobs[0]['inputs'][0]['name'] == 'test2.csv' + diff --git a/tests/integration_tests/python/test_upgrades.py b/tests/integration_tests/python/test_upgrades.py index 4d0b51a05..055a98b4e 100644 --- a/tests/integration_tests/python/test_upgrades.py +++ b/tests/integration_tests/python/test_upgrades.py @@ -2,6 +2,7 @@ import sys import bson +import copy import pytest @@ -32,3 +33,118 @@ def test_42(data_builder, api_db, as_admin, database): # Verify archived was removed when false as well session_data = as_admin.get('/sessions/' + session2).json() assert 'archived' not in session_data + + +def test_43(data_builder, randstr, api_db, as_admin, database, file_form): + + # Set up files with measurements + + assert True + + containers = [ + ('collections', data_builder.create_collection()), + ('projects', data_builder.create_project()), + ('sessions', data_builder.create_session()), + ('acquisitions', data_builder.create_acquisition()) + ] + + for c in containers: + assert as_admin.post('/{}/{}/files'.format(c[0], c[1]), files=file_form('test.csv')).ok + assert as_admin.post('/{}/{}/files'.format(c[0], c[1]), files=file_form('test2.csv')).ok + api_db[c[0]].update_one({'_id': bson.ObjectId(c[1])}, + {'$set': { # Mangoes ... + 'files.0.measurements': ['diffusion', 'functional'], + 'files.1.measurements': ['diffusion', 'functional'] + }}) + + + # Set up rules referencing measurements + + rule = { + 'all' : [ + {'type' : 'file.measurement', 'value' : 'diffusion'}, + {'type' : 'container.has-measurement', 'value' : 'tests', 'regex': True} + ], + 'any' : [ + {'type' : 'file.measurement', 'value' : 'diffusion'}, + {'type' : 'container.has-measurement', 'value' : 'tests', 'regex': True} + ], + 'name' : 'Run dcm2niix on dicom', + 'alg' : 'dcm2niix', + 'project_id' : 'site' + } + + api_db.project_rules.insert(copy.deepcopy(rule)) + api_db.project_rules.insert(copy.deepcopy(rule)) + + + # Set up session templates referencing measurements + + t_project1 = data_builder.create_project() + t_project2 = data_builder.create_project() + + template = { + 'session': {'subject': {'code': '^compliant$'}}, + 'acquisitions': [{ + 'minimum': 1, + 'files': [{ + 'minimum': 2, + 'measurements': 'diffusion' + }] + }] + } + + assert as_admin.post('/projects/' + t_project1 + '/template', json=template).ok + assert as_admin.post('/projects/' + t_project2 + '/template', json=template).ok + + + ### RUN UPGRADE + + database.upgrade_to_43() + + #### + + + # Ensure files were updated + for c in containers: + files = as_admin.get('/{}/{}'.format(c[0], c[1])).json()['files'] + for f in files: + assert f['classification'] == {'Custom': ['diffusion', 'functional']} + + + # Ensure rules were updated + rule_after = { + 'all' : [ + {'type' : 'file.classification', 'value' : 'diffusion'}, + {'type' : 'container.has-classification', 'value' : 'tests', 'regex': True} + ], + 'any' : [ + {'type' : 'file.classification', 'value' : 'diffusion'}, + {'type' : 'container.has-classification', 'value' : 'tests', 'regex': True} + ], + 'name' : 'Run dcm2niix on dicom', + 'alg' : 'dcm2niix' + } + + rules = as_admin.get('/site/rules').json() + for r in rules: + r.pop('_id') + assert r == rule_after + + + # Ensure templates were updated + template_after = { + 'session': {'subject': {'code': '^compliant$'}}, + 'acquisitions': [{ + 'minimum': 1, + 'files': [{ + 'minimum': 2, + 'classification': 'diffusion' + }] + }] + } + for p in [t_project1, t_project2]: + assert as_admin.get('/projects/' + p).json()['template'] == template_after + + + diff --git a/tests/unit_tests/python/test_rules.py b/tests/unit_tests/python/test_rules.py index 8dca05532..0cedfd73e 100644 --- a/tests/unit_tests/python/test_rules.py +++ b/tests/unit_tests/python/test_rules.py @@ -90,8 +90,8 @@ def test_eval_match_file_name_match_regex(): result = rules.eval_match(*args) assert result == False -def test_eval_match_file_measurements(): - part = rulePart(match_type='file.measurements', file_={'measurements': ['a', 'diffusion', 'b'] }) +def test_eval_match_file_classification(): + part = rulePart(match_type='file.classification', file_={'classification': {'intent': ['a', 'diffusion', 'b'] }}) args = part.gen(match_param='diffusion') result = rules.eval_match(*args) @@ -101,13 +101,13 @@ def test_eval_match_file_measurements(): result = rules.eval_match(*args) assert result == False - # Check match returns false without raising when not given a file.measurement + # Check match returns false without raising when not given a file.classification args = part.gen(match_param='', file_={'a': 'b'}, container={'a': 'b'}) result = rules.eval_match(*args) assert result == False -def test_eval_match_file_measurements_regex(): - part = rulePart(match_type='file.measurements', file_={'measurements': ['diffusion']}, regex=True) +def test_eval_match_file_classification_regex(): + part = rulePart(match_type='file.classification', file_={'classification': {'intent': ['diffusion']}}, regex=True) args = part.gen(match_param='test|diffusion') result = rules.eval_match(*args)