-
Notifications
You must be signed in to change notification settings - Fork 18
Upgrade Resolver for gears, analyses and lookup #1098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0060ca3
89363b4
1bc08eb
5478bf6
aeada8d
8b2feef
081bbdb
18c4b8d
275e0f5
89de560
ea0d39b
58bc7f3
c11464f
1631af8
1b250ce
866949f
b20a26a
89cc372
06e4c99
1135641
7496d0f
452f617
e3eb466
5bda420
11c709c
786a2aa
510cc77
9be94d1
0a47869
3677395
68c8c68
fee194e
e0bddb9
cb7de2e
e467742
08bdbd9
6086aa0
4d35b4d
087a3b1
49f97eb
8c70ad3
9304596
0073041
3049c32
22712cb
277deed
31f0a5b
94d2fd6
234274a
4791196
21e34fc
7c73798
230a39e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,6 +108,9 @@ def get_container(self, _id, projection=None, get_children=False): | |
cont[CHILD_MAP[self.cont_name]] = children | ||
return cont | ||
|
||
def get_child_container_name(self): | ||
return CHILD_MAP.get(self.cont_name) | ||
|
||
def get_children(self, _id, projection=None, uid=None): | ||
try: | ||
child_name = CHILD_MAP[self.cont_name] | ||
|
@@ -210,11 +213,21 @@ def get_el(self, _id, projection=None, fill_defaults=False): | |
self._from_mongo(cont) | ||
if fill_defaults: | ||
self._fill_default_values(cont) | ||
if cont is not None and cont.get('files', []): | ||
cont['files'] = [f for f in cont['files'] if 'deleted' not in f] | ||
self.filter_deleted_files(cont) | ||
return cont | ||
|
||
def get_all_el(self, query, user, projection, fill_defaults=False): | ||
def get_all_el(self, query, user, projection, fill_defaults=False, **kwargs): | ||
""" | ||
Get all elements matching query for this container. | ||
|
||
Args: | ||
query (dict): The query object, or None for all elements | ||
user (dict): The user object, if filtering on permissions is desired, otherwise None | ||
projection (dict): The optional projection to use for returned elements | ||
fill_defaults (bool): Whether or not to populate the default values for returned elements. Default is False. | ||
**kwargs: Additional arguments to pass to the underlying find function | ||
|
||
""" | ||
if query is None: | ||
query = {} | ||
if user: | ||
|
@@ -238,10 +251,9 @@ def get_all_el(self, query, user, projection, fill_defaults=False): | |
else: | ||
replace_info_with_bool = False | ||
|
||
results = list(self.dbc.find(query, projection)) | ||
results = list(self.dbc.find(query, projection, **kwargs)) | ||
for cont in results: | ||
if cont.get('files', []): | ||
cont['files'] = [f for f in cont['files'] if 'deleted' not in f] | ||
self.filter_deleted_files(cont) | ||
self._from_mongo(cont) | ||
if fill_defaults: | ||
self._fill_default_values(cont) | ||
|
@@ -304,3 +316,18 @@ def modify_info(self, _id, payload, modify_subject=False): | |
update['$set']['modified'] = datetime.datetime.utcnow() | ||
|
||
return self.dbc.update_one(query, update) | ||
|
||
def filter_deleted_files(self, cont): | ||
""" | ||
Update container object, removing any files that are marked deleted. | ||
""" | ||
if cont is not None and 'files' in cont: | ||
cont['files'] = [f for f in cont['files'] if 'deleted' not in f] | ||
|
||
|
||
def get_list_projection(self): | ||
""" | ||
Return a copy of the list projection to use with this container, or None. | ||
It is safe to modify the returned copy. | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm so-so on this function, but I can see why you did it. |
||
return None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,6 @@ def create_el(self, payload): | |
|
||
|
||
class ProjectStorage(ContainerStorage): | ||
|
||
def __init__(self): | ||
super(ProjectStorage,self).__init__('projects', use_object_id=True, use_delete_tag=True) | ||
|
||
|
@@ -96,6 +95,9 @@ def recalc_sessions_compliance(self, project_id=None): | |
changed_sessions.append(s['_id']) | ||
return changed_sessions | ||
|
||
def get_list_projection(self): | ||
return {'info': 0, 'files.info': 0} | ||
|
||
|
||
class SessionStorage(ContainerStorage): | ||
|
||
|
@@ -220,6 +222,14 @@ def get_all_for_targets(self, target_type, target_ids, user=None, projection=Non | |
|
||
return self.get_all_el(query, user, projection) | ||
|
||
def get_list_projection(self): | ||
# Remove subject first/last from list view to better log access to this information | ||
return {'info': 0, 'analyses': 0, 'subject.firstname': 0, | ||
'subject.lastname': 0, 'subject.sex': 0, 'subject.age': 0, | ||
'subject.race': 0, 'subject.ethnicity': 0, 'subject.info': 0, | ||
'files.info': 0, 'tags': 0} | ||
|
||
|
||
|
||
|
||
class AcquisitionStorage(ContainerStorage): | ||
|
@@ -287,12 +297,18 @@ def get_all_for_targets(self, target_type, target_ids, user=None, projection=Non | |
query['collections'] = collection_id | ||
return self.get_all_el(query, user, projection) | ||
|
||
def get_list_projection(self): | ||
return {'info': 0, 'collections': 0, 'files.info': 0, 'tags': 0} | ||
|
||
|
||
class CollectionStorage(ContainerStorage): | ||
|
||
def __init__(self): | ||
super(CollectionStorage, self).__init__('collections', use_object_id=True, use_delete_tag=True) | ||
|
||
def get_list_projection(self): | ||
return {'info': 0} | ||
|
||
|
||
class AnalysisStorage(ContainerStorage): | ||
|
||
|
@@ -305,10 +321,13 @@ def get_parent(self, parent_type, parent_id): | |
return parent_storage.get_container(parent_id) | ||
|
||
|
||
def get_analyses(self, parent_type, parent_id, inflate_job_info=False): | ||
parent_type = containerutil.singularize(parent_type) | ||
parent_id = bson.ObjectId(parent_id) | ||
analyses = self.get_all_el({'parent.type': parent_type, 'parent.id': parent_id}, None, None) | ||
def get_analyses(self, query, parent_type, parent_id, inflate_job_info=False, projection=None, **kwargs): | ||
if query is None: | ||
query = {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeing as query is a required param, I would advocate for not tolerating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
query['parent.type'] = containerutil.singularize(parent_type) | ||
query['parent.id'] = bson.ObjectId(parent_id) | ||
|
||
analyses = self.get_all_el(query, None, projection, **kwargs) | ||
if inflate_job_info: | ||
for analysis in analyses: | ||
self.inflate_job_info(analysis) | ||
|
@@ -410,3 +429,6 @@ def inflate_job_info(self, analysis): | |
|
||
analysis['job'] = job | ||
return analysis | ||
|
||
def get_list_projection(self): | ||
return {'info': 0, 'files.info': 0, 'tags': 0} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,6 @@ class ContainerHandler(base.RequestHandler): | |
'sessions': True, | ||
'acquisitions': True | ||
} | ||
default_list_projection = ['files', 'notes', 'timestamp', 'timezone', 'public'] | ||
|
||
# This configurations are used by the ContainerHandler class to load the storage, | ||
# the permissions checker and the json schema validators used to handle a request. | ||
|
@@ -57,7 +56,6 @@ class ContainerHandler(base.RequestHandler): | |
'parent_storage': containerstorage.GroupStorage(), | ||
'storage_schema_file': 'project.json', | ||
'payload_schema_file': 'project.json', | ||
'list_projection': {'info': 0, 'files.info': 0}, | ||
'propagated_properties': ['public'], | ||
'children_cont': 'sessions' | ||
}, | ||
|
@@ -67,20 +65,14 @@ class ContainerHandler(base.RequestHandler): | |
'parent_storage': containerstorage.ProjectStorage(), | ||
'storage_schema_file': 'session.json', | ||
'payload_schema_file': 'session.json', | ||
# Remove subject first/last from list view to better log access to this information | ||
'list_projection': {'info': 0, 'analyses': 0, 'subject.firstname': 0, | ||
'subject.lastname': 0, 'subject.sex': 0, 'subject.age': 0, | ||
'subject.race': 0, 'subject.ethnicity': 0, 'subject.info': 0, | ||
'files.info': 0, 'tags': 0}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👏 to this moving out of this file |
||
'children_cont': 'acquisitions' | ||
}, | ||
'acquisitions': { | ||
'storage': containerstorage.AcquisitionStorage(), | ||
'permchecker': containerauth.default_container, | ||
'parent_storage': containerstorage.SessionStorage(), | ||
'storage_schema_file': 'acquisition.json', | ||
'payload_schema_file': 'acquisition.json', | ||
'list_projection': {'info': 0, 'collections': 0, 'files.info': 0, 'tags': 0} | ||
'payload_schema_file': 'acquisition.json' | ||
} | ||
} | ||
|
||
|
@@ -114,7 +106,10 @@ def get(self, cont_name, **kwargs): | |
fileinfo['path'] = util.path_from_hash(fileinfo['hash']) | ||
|
||
inflate_job_info = cont_name == 'sessions' | ||
result['analyses'] = AnalysisStorage().get_analyses(cont_name, _id, inflate_job_info) | ||
result['analyses'] = AnalysisStorage().get_analyses(None, cont_name, _id, inflate_job_info) | ||
|
||
util.add_container_type(self.request, result) | ||
|
||
return self.handle_origin(result) | ||
|
||
def handle_origin(self, result): | ||
|
@@ -240,7 +235,7 @@ def get_jobs(self, cid): | |
|
||
permchecker(noop)('GET', cid) | ||
|
||
analyses = AnalysisStorage().get_analyses('session', cont['_id']) | ||
analyses = AnalysisStorage().get_analyses(None, 'session', cont['_id']) | ||
acquisitions = cont.get('acquisitions', []) | ||
|
||
results = [] | ||
|
@@ -311,7 +306,7 @@ def get_all(self, cont_name, par_cont_name=None, par_id=None): | |
self.config = self.container_handler_configurations[cont_name] | ||
self.storage = self.config['storage'] | ||
|
||
projection = self.config['list_projection'].copy() | ||
projection = self.storage.get_list_projection() | ||
|
||
if self.is_true('permissions'): | ||
if not projection: | ||
|
@@ -384,7 +379,7 @@ def _add_results_counts(self, results, cont_name): | |
def get_all_for_user(self, cont_name, uid): | ||
self.config = self.container_handler_configurations[cont_name] | ||
self.storage = self.config['storage'] | ||
projection = self.config['list_projection'] | ||
projection = self.storage.get_list_projection() | ||
# select which permission filter will be applied to the list of results. | ||
if self.superuser_request or self.user_is_admin: | ||
permchecker = always_ok | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ def get(self, _id): | |
self._filter_permissions([result], self.uid) | ||
if self.is_true('join_avatars'): | ||
ContainerHandler.join_user_info([result]) | ||
util.add_container_type(self.request, result) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This & its brethren are adding that extra key onto each single-container GET, correct? If so, good, just making sure I understand correctly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will only show up if you set the |
||
return result | ||
|
||
def delete(self, _id): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏