From 0ca1401bd8387717edb31a69e11d3adc09258757 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 13:25:22 -0400 Subject: [PATCH 01/10] ToolBuildOptimize - Eliminate check_security on dataset matcher. It was only used for collections and we need to drop it as prohibitively expensive to calculate. No need to filter collections ahead of time that way anyhow - it is the tool action's job to block the execution of datasets without permission so hopefully we aren't deriving any security value from this filter. --- lib/galaxy/tools/parameters/basic.py | 4 ++-- .../tools/parameters/dataset_matcher.py | 22 +++++-------------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index e6b1b12c76f..869a003680e 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1487,7 +1487,7 @@ class BaseDataToolParameter(ToolParameter): dataset_matcher = DatasetMatcher(trans, self, None, other_values) if isinstance(self, DataToolParameter): for hda in reversed(history.active_datasets_and_roles): - match = dataset_matcher.hda_match(hda, check_security=False) + match = dataset_matcher.hda_match(hda) if match: return match.hda else: @@ -1812,7 +1812,7 @@ class DataToolParameter(BaseDataToolParameter): # add datasets hda_list = util.listify(other_values.get(self.name)) for hda in history.active_datasets_and_roles: - match = dataset_matcher.hda_match(hda, check_security=False) + match = dataset_matcher.hda_match(hda) if match: m = match.hda hda_list = [h for h in hda_list if h != m and h != hda] diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index 2cdb6277064..e6f7885e805 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -4,9 +4,6 @@ import galaxy.model log = getLogger(__name__) -ROLES_UNSET = object() -INVALID_STATES = [galaxy.model.Dataset.states.ERROR, galaxy.model.Dataset.states.DISCARDED] - class DatasetMatcher(object): """ Utility class to aid DataToolParameter and similar classes in reasoning @@ -22,7 +19,6 @@ class DatasetMatcher(object): self.param = param self.tool = param.tool self.value = value - self.current_user_roles = ROLES_UNSET filter_value = None if param.options and other_values: try: @@ -31,7 +27,7 @@ class DatasetMatcher(object): pass # no valid options self.filter_value = filter_value - def hda_accessible(self, hda, check_security=True): + def hda_accessible(self, hda): """ Does HDA correspond to dataset that is an a valid state and is accessible to user. """ @@ -42,9 +38,9 @@ class DatasetMatcher(object): else: valid_input_states = galaxy.model.Dataset.valid_input_states state_valid = dataset.state in valid_input_states - return state_valid and (not check_security or self.__can_access_dataset(dataset)) + return state_valid - def valid_hda_match(self, hda, check_implicit_conversions=True, check_security=False): + def valid_hda_match(self, hda, check_implicit_conversions=True): """ Return False of this parameter can not be matched to the supplied HDA, otherwise return a description of the match (either a HdaDirectMatch describing a direct match or a HdaImplicitMatch @@ -62,8 +58,6 @@ class DatasetMatcher(object): original_hda = hda if converted_dataset: hda = converted_dataset - if check_security and not self.__can_access_dataset(hda.dataset): - return False rval = HdaImplicitMatch(hda, target_ext, original_hda) else: return False @@ -71,12 +65,12 @@ class DatasetMatcher(object): return False return rval - def hda_match(self, hda, check_implicit_conversions=True, check_security=True, ensure_visible=True): + def hda_match(self, hda, check_implicit_conversions=True, ensure_visible=True): """ If HDA is accessible, return information about whether it could match this parameter and if so how. See valid_hda_match for more information. """ - accessible = self.hda_accessible(hda, check_security=check_security) + accessible = self.hda_accessible(hda) if accessible and (not ensure_visible or hda.visible or (self.selected(hda) and not hda.implicitly_converted_parent_datasets)): # If we are sending data to an external application, then we need to make sure there are no roles # associated with the dataset that restrict its access from "public". @@ -103,12 +97,6 @@ class DatasetMatcher(object): param = self.param return param.options and param.get_options_filter_attribute(hda) != self.filter_value - def __can_access_dataset(self, dataset): - # Lazily cache current_user_roles. - if self.current_user_roles is ROLES_UNSET: - self.current_user_roles = self.trans.get_current_user_roles() - return self.trans.app.security_agent.can_access_dataset(self.current_user_roles, dataset) - class HdaDirectMatch(object): """ Supplied HDA was a valid option directly (did not need to find implicit From c3303939c05ae8af9a2b11a12d5f433ee0d8072e Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 13:33:10 -0400 Subject: [PATCH 02/10] ToolBuildOptimize - Optimize state checking in dataset matcher. Pre-calculate valid states, skip now unneeded helper method. --- lib/galaxy/tools/parameters/dataset_matcher.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index e6f7885e805..26272333b86 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -26,19 +26,12 @@ class DatasetMatcher(object): except IndexError: pass # no valid options self.filter_value = filter_value - - def hda_accessible(self, hda): - """ Does HDA correspond to dataset that is an a valid state and is - accessible to user. - """ - dataset = hda.dataset has_tool = self.tool if has_tool: valid_input_states = self.tool.valid_input_states else: valid_input_states = galaxy.model.Dataset.valid_input_states - state_valid = dataset.state in valid_input_states - return state_valid + self.valid_input_states = valid_input_states def valid_hda_match(self, hda, check_implicit_conversions=True): """ Return False of this parameter can not be matched to the supplied @@ -70,12 +63,13 @@ class DatasetMatcher(object): match this parameter and if so how. See valid_hda_match for more information. """ - accessible = self.hda_accessible(hda) - if accessible and (not ensure_visible or hda.visible or (self.selected(hda) and not hda.implicitly_converted_parent_datasets)): + dataset = hda.dataset + valid_state = dataset.state in self.valid_input_states + if valid_state and (not ensure_visible or hda.visible or (self.selected(hda) and not hda.implicitly_converted_parent_datasets)): # If we are sending data to an external application, then we need to make sure there are no roles # associated with the dataset that restrict its access from "public". require_public = self.tool and self.tool.tool_type == 'data_destination' - if require_public and not self.trans.app.security_agent.dataset_is_public(hda.dataset): + if require_public and not self.trans.app.security_agent.dataset_is_public(dataset): return False if self.filter(hda): return False From 7ca2ef587cd26682e3b4b28c7cdb8907cfeae862 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 13:37:19 -0400 Subject: [PATCH 03/10] DatasetMatcherClean - Skip second, unneeded call to filter(). xref 8f813712f50ca21183d2efe59ee0e2a665520f95 to some degree. --- lib/galaxy/tools/parameters/dataset_matcher.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index 26272333b86..dd52cd57491 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -71,8 +71,6 @@ class DatasetMatcher(object): require_public = self.tool and self.tool.tool_type == 'data_destination' if require_public and not self.trans.app.security_agent.dataset_is_public(dataset): return False - if self.filter(hda): - return False return self.valid_hda_match(hda, check_implicit_conversions=check_implicit_conversions) def selected(self, hda): From 412f03d8446cb362bbfe27d5832bcf37aef2d4d6 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 13:44:40 -0400 Subject: [PATCH 04/10] DatasetMatcherClean - Eliminate DatasetMatcher.value - it is never set. Also eliminate any logic related to it having a value. This had a purpose originally, but is no longer set. Cleaning this up makes subsequent commits a bit cleaner also. --- lib/galaxy/tools/parameters/basic.py | 6 +++--- lib/galaxy/tools/parameters/dataset_matcher.py | 14 ++------------ 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 869a003680e..5ba9e9df630 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1484,7 +1484,7 @@ class BaseDataToolParameter(ToolParameter): return None history = trans.history if history is not None: - dataset_matcher = DatasetMatcher(trans, self, None, other_values) + dataset_matcher = DatasetMatcher(trans, self, other_values) if isinstance(self, DataToolParameter): for hda in reversed(history.active_datasets_and_roles): match = dataset_matcher.hda_match(hda) @@ -1797,7 +1797,7 @@ class DataToolParameter(BaseDataToolParameter): return d # prepare dataset/collection matching - dataset_matcher = DatasetMatcher(trans, self, None, other_values) + dataset_matcher = DatasetMatcher(trans, self, other_values) multiple = self.multiple # build and append a new select option @@ -1954,7 +1954,7 @@ class DataCollectionToolParameter(BaseDataToolParameter): return d # prepare dataset/collection matching - dataset_matcher = DatasetMatcher(trans, self, None, other_values) + dataset_matcher = DatasetMatcher(trans, self, other_values) # append directly matched collections for hdca in self.match_collections(trans, history, dataset_matcher): diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index dd52cd57491..93138c4739d 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -14,11 +14,10 @@ class DatasetMatcher(object): and permission handling. """ - def __init__(self, trans, param, value, other_values): + def __init__(self, trans, param, other_values): self.trans = trans self.param = param self.tool = param.tool - self.value = value filter_value = None if param.options and other_values: try: @@ -65,7 +64,7 @@ class DatasetMatcher(object): """ dataset = hda.dataset valid_state = dataset.state in self.valid_input_states - if valid_state and (not ensure_visible or hda.visible or (self.selected(hda) and not hda.implicitly_converted_parent_datasets)): + if valid_state and (not ensure_visible or hda.visible): # If we are sending data to an external application, then we need to make sure there are no roles # associated with the dataset that restrict its access from "public". require_public = self.tool and self.tool.tool_type == 'data_destination' @@ -73,15 +72,6 @@ class DatasetMatcher(object): return False return self.valid_hda_match(hda, check_implicit_conversions=check_implicit_conversions) - def selected(self, hda): - """ Given value for DataToolParameter, is this HDA "selected". - """ - value = self.value - if value and str(value[0]).isdigit(): - return hda.id in map(int, value) - else: - return value and hda in value - def filter(self, hda): """ Filter out this value based on other values for job (if applicable). From ca80232db3a93a02dcd42f2698b54e7a8d2aeef1 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 14:30:14 -0400 Subject: [PATCH 05/10] DatasetMatcherClean - Remove unused method. It calls things that I'm changing in subsequent commits so I thought I'd just axe it now to clear things up. --- lib/galaxy/tools/parameters/basic.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 5ba9e9df630..c28ac8014ed 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1603,13 +1603,6 @@ class DataToolParameter(BaseDataToolParameter): raise ValueError("Datatype class not found for extension '%s', which is used as 'type' attribute in conversion of data parameter '%s'" % (conv_type, self.name)) self.conversions.append((name, conv_extension, [conv_type])) - def match_collections(self, history, dataset_matcher, reduction=True): - dataset_collection_matcher = DatasetCollectionMatcher(dataset_matcher) - - for history_dataset_collection in history.active_dataset_collections: - if dataset_collection_matcher.hdca_match(history_dataset_collection, reduction=reduction): - yield history_dataset_collection - def from_json(self, value, trans, other_values={}): if trans.workflow_building_mode is workflow_building_modes.ENABLED: return None From efe5d8b973ffdd22963ec75db7820ff1731db916 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 15:52:58 -0400 Subject: [PATCH 06/10] DatasetMatcherClean - Implement DatasetMatcherFactory to reason for whole tool. In subsequent commit I'll use this central store of all the inputs for a tool to determine if summary data about collections can be used instead of processing individual datasets one at a time. Even this commit though uses the abstraction to optimize datatype checking and cache commons checks when possible - should lead to a lot fewer objects being created when processing a large history. --- lib/galaxy/tools/__init__.py | 6 +++ lib/galaxy/tools/parameters/basic.py | 28 ++++++------- .../tools/parameters/dataset_matcher.py | 40 +++++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 554daa01599..05ae0537648 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -56,6 +56,10 @@ from galaxy.tools.parameters.basic import ( ToolParameter, workflow_building_modes, ) +from galaxy.tools.parameters.dataset_matcher import ( + set_dataset_matcher_factory, + unset_dataset_matcher_factory, +) from galaxy.tools.parameters.grouping import Conditional, ConditionalWhen, Repeat, Section, UploadDataset from galaxy.tools.parameters.input_translation import ToolInputTranslator from galaxy.tools.parameters.meta import expand_meta_parameters @@ -1826,7 +1830,9 @@ class Tool(object, Dictifiable): # create tool model tool_model = self.to_dict(request_context) tool_model['inputs'] = [] + set_dataset_matcher_factory(request_context, self, state_inputs) self.populate_model(request_context, self.inputs, state_inputs, tool_model['inputs']) + unset_dataset_matcher_factory(request_context) # create tool help tool_help = '' diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index c28ac8014ed..e49856a64df 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -26,8 +26,7 @@ from galaxy.util.expressions import ExpressionContext from galaxy.web import url_for from . import validation from .dataset_matcher import ( - DatasetCollectionMatcher, - DatasetMatcher + get_dataset_matcher_factory, ) from .sanitize import ToolParameterSanitizer from ..parameters import ( @@ -1484,14 +1483,15 @@ class BaseDataToolParameter(ToolParameter): return None history = trans.history if history is not None: - dataset_matcher = DatasetMatcher(trans, self, other_values) + dataset_matcher_factory = get_dataset_matcher_factory(trans) + dataset_matcher = dataset_matcher_factory.dataset_matcher(self, other_values) if isinstance(self, DataToolParameter): for hda in reversed(history.active_datasets_and_roles): match = dataset_matcher.hda_match(hda) if match: return match.hda else: - dataset_collection_matcher = DatasetCollectionMatcher(dataset_matcher) + dataset_collection_matcher = dataset_matcher_factory.dataset_collection_matcher(dataset_matcher) for hdca in reversed(history.active_dataset_collections): if dataset_collection_matcher.hdca_match(hdca, reduction=self.multiple): return hdca @@ -1790,7 +1790,8 @@ class DataToolParameter(BaseDataToolParameter): return d # prepare dataset/collection matching - dataset_matcher = DatasetMatcher(trans, self, other_values) + dataset_matcher_factory = get_dataset_matcher_factory(trans) + dataset_matcher = dataset_matcher_factory.dataset_matcher(self, other_values) multiple = self.multiple # build and append a new select option @@ -1822,7 +1823,7 @@ class DataToolParameter(BaseDataToolParameter): append(d['options']['hda'], hda, '(%s) %s' % (hda_state, hda.name), 'hda', True) # add dataset collections - dataset_collection_matcher = DatasetCollectionMatcher(dataset_matcher) + dataset_collection_matcher = dataset_matcher_factory.dataset_collection_matcher(dataset_matcher) for hdca in history.active_dataset_collections: if dataset_collection_matcher.hdca_match(hdca, reduction=multiple): append(d['options']['hdca'], hdca, hdca.name, 'hdca') @@ -1859,18 +1860,15 @@ class DataCollectionToolParameter(BaseDataToolParameter): dataset_collection_type_descriptions = trans.app.dataset_collections_service.collection_type_descriptions return history_query.HistoryQuery.from_parameter(self, dataset_collection_type_descriptions) - def match_collections(self, trans, history, dataset_matcher): + def match_collections(self, trans, history, dataset_collection_matcher): dataset_collections = trans.app.dataset_collections_service.history_dataset_collections(history, self._history_query(trans)) - dataset_collection_matcher = DatasetCollectionMatcher(dataset_matcher) for dataset_collection_instance in dataset_collections: if not dataset_collection_matcher.hdca_match(dataset_collection_instance): continue yield dataset_collection_instance - def match_multirun_collections(self, trans, history, dataset_matcher): - dataset_collection_matcher = DatasetCollectionMatcher(dataset_matcher) - + def match_multirun_collections(self, trans, history, dataset_collection_matcher): for history_dataset_collection in history.active_dataset_collections: if not self._history_query(trans).can_map_over(history_dataset_collection): continue @@ -1947,10 +1945,12 @@ class DataCollectionToolParameter(BaseDataToolParameter): return d # prepare dataset/collection matching - dataset_matcher = DatasetMatcher(trans, self, other_values) + dataset_matcher_factory = get_dataset_matcher_factory(trans) + dataset_matcher = dataset_matcher_factory.dataset_matcher(self, other_values) + dataset_collection_matcher = dataset_matcher_factory.dataset_collection_matcher(dataset_matcher) # append directly matched collections - for hdca in self.match_collections(trans, history, dataset_matcher): + for hdca in self.match_collections(trans, history, dataset_collection_matcher): d['options']['hdca'].append({ 'id' : trans.security.encode_id(hdca.id), 'hid' : hdca.hid, @@ -1960,7 +1960,7 @@ class DataCollectionToolParameter(BaseDataToolParameter): }) # append matching subcollections - for hdca in self.match_multirun_collections(trans, history, dataset_matcher): + for hdca in self.match_multirun_collections(trans, history, dataset_collection_matcher): subcollection_type = self._history_query(trans).can_map_over(hdca).collection_type d['options']['hdca'].append({ 'id' : trans.security.encode_id(hdca.id), diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index 93138c4739d..96e760ce8d6 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -5,6 +5,46 @@ import galaxy.model log = getLogger(__name__) +def set_dataset_matcher_factory(trans, tool, param_values): + trans.dataset_matcher_factory = DatasetMatcherFactory(trans, tool, param_values) + + +def unset_dataset_matcher_factory(trans): + trans.dataset_matcher_factory = None + + +def get_dataset_matcher_factory(trans): + dataset_matcher_factory = getattr(trans, "dataset_matcher_factory", None) + return dataset_matcher_factory or DatasetMatcherFactory(trans) + + +class DatasetMatcherFactory(object): + """""" + + def __init__(self, trans, tool=None, param_values=None): + self._trans = trans + self._tool = tool + self._data_inputs = [] + if tool is not None and param_values is not None: + self._collect_data_inputs(tool, param_values) + + def _collect_data_inputs(self, tool, param_values): + def visitor(input, value, prefix, parent=None, **kwargs): + type_name = type(input).__name__ + if "DataToolParameter" in type_name: + self._data_inputs.append(input) + elif "DatasetCollectionToolParameter" in type_name: + self._data_inputs.append(input) + + tool.visit_inputs(param_values, visitor) + + def dataset_matcher(self, param, other_values): + return DatasetMatcher(self._trans, param, other_values) + + def dataset_collection_matcher(self, dataset_matcher): + return DatasetCollectionMatcher(dataset_matcher) + + class DatasetMatcher(object): """ Utility class to aid DataToolParameter and similar classes in reasoning about what HDAs could match or are selected for a parameter and value. From 2eea566c10eaf2b7dad21cdff82e0a14a1893fde Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 26 Apr 2018 16:42:56 -0400 Subject: [PATCH 07/10] ToolBuildOptimize - Add SummaryDatasetCollectionMatcher. Use summary data pulled from the database for collections when possible instead of loading potentially hundreds of thousands of individual datasets. --- lib/galaxy/model/__init__.py | 94 +++++++++++++++++-- .../tools/parameters/dataset_matcher.py | 81 +++++++++++++--- test/unit/tools/test_data_parameters.py | 22 ++--- test/unit/tools/test_dataset_matcher.py | 52 +++------- 4 files changed, 177 insertions(+), 72 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index b9385daec3e..f859951a5a2 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -18,6 +18,7 @@ from uuid import UUID, uuid4 from six import string_types from sqlalchemy import ( + alias, and_, func, inspect, @@ -1968,6 +1969,18 @@ class Dataset(StorableObject): return False +def datatype_for_extension(extension, datatypes_registry=None): + if datatypes_registry is None: + datatypes_registry = _get_datatypes_registry() + if not extension or extension == 'auto' or extension == '_sniff_': + extension = 'data' + ret = datatypes_registry.get_datatype_by_extension(extension) + if ret is None: + log.warning("Datatype class not found for extension '%s'" % extension) + return datatypes_registry.get_datatype_by_extension('data') + return ret + + class DatasetInstance(object): """A base class for all 'dataset instances', HDAs, LDAs, etc""" states = Dataset.states @@ -2045,14 +2058,7 @@ class DatasetInstance(object): @property def datatype(self): - extension = self.extension - if not extension or extension == 'auto' or extension == '_sniff_': - extension = 'data' - ret = _get_datatypes_registry().get_datatype_by_extension(extension) - if ret is None: - log.warning("Datatype class not found for extension '%s'" % extension) - return _get_datatypes_registry().get_datatype_by_extension('data') - return ret + return datatype_for_extension(self.extension) def get_metadata(self): # using weakref to store parent (to prevent circ ref), @@ -3140,6 +3146,78 @@ class DatasetCollection(object, Dictifiable, UsesAnnotations): if not populated: self.populated_state = DatasetCollection.populated_states.NEW + @property + def dataset_states_and_extensions_summary(self): + if not hasattr(self, '_dataset_states_and_extensions_summary'): + db_session = object_session(self) + + dc = alias(DatasetCollection.table) + de = alias(DatasetCollectionElement.table) + hda = alias(HistoryDatasetAssociation.table) + dataset = alias(Dataset.table) + + select_from = dc.outerjoin(de, de.c.dataset_collection_id == dc.c.id) + + depth_collection_type = self.collection_type + while ":" in depth_collection_type: + child_collection = alias(DatasetCollection.table) + child_collection_element = alias(DatasetCollectionElement.table) + select_from = select_from.outerjoin(child_collection, child_collection.c.id == de.c.child_collection_id) + select_from = select_from.outerjoin(child_collection_element, child_collection_element.c.dataset_collection_id == child_collection.c.id) + + de = child_collection_element + depth_collection_type = depth_collection_type.split(":", 1)[1] + + select_from = select_from.outerjoin(hda, hda.c.id == de.c.hda_id).outerjoin(dataset, hda.c.dataset_id == dataset.c.id) + select_stmt = select([hda.c.extension, dataset.c.state]).select_from(select_from).where(dc.c.id == self.id).distinct() + extensions = set() + states = set() + for extension, state in db_session.execute(select_stmt).fetchall(): + states.add(state) + extensions.add(extension) + + self._dataset_states_and_extensions_summary = (states, extensions) + + return self._dataset_states_and_extensions_summary + + @property + def populated_optimized(self): + if not hasattr(self, '_populated_optimized'): + _populated_optimized = True + if ":" not in self.collection_type: + _populated_optimized = self.populated_state == DatasetCollection.populated_states.OK + else: + db_session = object_session(self) + + dc = alias(DatasetCollection.table) + de = alias(DatasetCollectionElement.table) + + select_from = dc.outerjoin(de, de.c.dataset_collection_id == dc.c.id) + + collection_depth_aliases = [dc] + + depth_collection_type = self.collection_type + while ":" in depth_collection_type: + child_collection = alias(DatasetCollection.table) + child_collection_element = alias(DatasetCollectionElement.table) + select_from = select_from.outerjoin(child_collection, child_collection.c.id == de.c.child_collection_id) + select_from = select_from.outerjoin(child_collection_element, child_collection_element.c.dataset_collection_id == child_collection.c.id) + + collection_depth_aliases.append(child_collection) + + de = child_collection_element + depth_collection_type = depth_collection_type.split(":", 1)[1] + + select_stmt = select(list(map(lambda dc: dc.c.populated_state, collection_depth_aliases))).select_from(select_from).where(dc.c.id == self.id).distinct() + for populated_states in db_session.execute(select_stmt).fetchall(): + for populated_state in populated_states: + if populated_state != DatasetCollection.populated_states.OK: + _populated_optimized = False + + self._populated_optimized = _populated_optimized + + return self._populated_optimized + @property def populated(self): top_level_populated = self.populated_state == DatasetCollection.populated_states.OK diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index 96e760ce8d6..ecc3d98a07a 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -25,9 +25,43 @@ class DatasetMatcherFactory(object): self._trans = trans self._tool = tool self._data_inputs = [] + self._matches_format_cache = {} + if tool: + valid_input_states = tool.valid_input_states + else: + valid_input_states = galaxy.model.Dataset.valid_input_states + self.valid_input_states = valid_input_states + can_process_summary = False if tool is not None and param_values is not None: self._collect_data_inputs(tool, param_values) + require_public = self._tool and self._tool.tool_type == 'data_destination' + if not require_public and self._data_inputs: + can_process_summary = True + for data_input in self._data_inputs: + if data_input.options: + can_process_summary = False + break + self._can_process_summary = can_process_summary + + def matches_any_format(self, hda_extension, formats): + for format in formats: + if self.matches_format(hda_extension, format): + return True + return False + + def matches_format(self, hda_extension, format): + # cache datatype checking combinations for fast recall + if hda_extension not in self._matches_format_cache: + self._matches_format_cache[hda_extension] = {} + + formats = self._matches_format_cache[hda_extension] + if format not in formats: + datatype = galaxy.model.datatype_for_extension(hda_extension, datatypes_registry=self._trans.app.datatypes_registry) + formats[format] = datatype.matches_any([format]) + + return formats[format] + def _collect_data_inputs(self, tool, param_values): def visitor(input, value, prefix, parent=None, **kwargs): type_name = type(input).__name__ @@ -39,10 +73,13 @@ class DatasetMatcherFactory(object): tool.visit_inputs(param_values, visitor) def dataset_matcher(self, param, other_values): - return DatasetMatcher(self._trans, param, other_values) + return DatasetMatcher(self, self._trans, param, other_values) def dataset_collection_matcher(self, dataset_matcher): - return DatasetCollectionMatcher(dataset_matcher) + if self._can_process_summary: + return SummaryDatasetCollectionMatcher(self, dataset_matcher) + else: + return DatasetCollectionMatcher(dataset_matcher) class DatasetMatcher(object): @@ -54,7 +91,8 @@ class DatasetMatcher(object): and permission handling. """ - def __init__(self, trans, param, other_values): + def __init__(self, dataset_matcher_factory, trans, param, other_values): + self.dataset_matcher_factory = dataset_matcher_factory self.trans = trans self.param = param self.tool = param.tool @@ -65,12 +103,6 @@ class DatasetMatcher(object): except IndexError: pass # no valid options self.filter_value = filter_value - has_tool = self.tool - if has_tool: - valid_input_states = self.tool.valid_input_states - else: - valid_input_states = galaxy.model.Dataset.valid_input_states - self.valid_input_states = valid_input_states def valid_hda_match(self, hda, check_implicit_conversions=True): """ Return False of this parameter can not be matched to the supplied @@ -80,7 +112,7 @@ class DatasetMatcher(object): """ rval = False formats = self.param.formats - if hda.datatype.matches_any(formats): + if self.dataset_matcher_factory.matches_any_format(hda.extension, formats): rval = HdaDirectMatch(hda) else: if not check_implicit_conversions: @@ -103,7 +135,7 @@ class DatasetMatcher(object): information. """ dataset = hda.dataset - valid_state = dataset.state in self.valid_input_states + valid_state = dataset.state in self.dataset_matcher_factory.valid_input_states if valid_state and (not ensure_visible or hda.visible): # If we are sending data to an external application, then we need to make sure there are no roles # associated with the dataset that restrict its access from "public". @@ -148,6 +180,33 @@ class HdaImplicitMatch(object): return True +class SummaryDatasetCollectionMatcher(object): + + def __init__(self, dataset_matcher_factory, dataset_matcher): + self.dataset_matcher_factory = dataset_matcher_factory + self.dataset_matcher = dataset_matcher + + def hdca_match(self, history_dataset_collection_association, reduction=False): + dataset_collection = history_dataset_collection_association.collection + if reduction and dataset_collection.collection_type.find(":") > 0: + return False + + if not dataset_collection.populated_optimized: + return False + + (states, extensions) = dataset_collection.dataset_states_and_extensions_summary + for state in states: + if state not in self.dataset_matcher_factory.valid_input_states: + return False + + formats = self.dataset_matcher.param.formats + for extension in extensions: + if not self.dataset_matcher_factory.matches_any_format(extension, formats): + return False + + return True + + class DatasetCollectionMatcher(object): def __init__(self, dataset_matcher): diff --git a/test/unit/tools/test_data_parameters.py b/test/unit/tools/test_data_parameters.py index c7e0aa2e4e4..0b5d1587643 100644 --- a/test/unit/tools/test_data_parameters.py +++ b/test/unit/tools/test_data_parameters.py @@ -1,5 +1,4 @@ from galaxy import model -from galaxy.util import bunch from .test_parameter_parsing import BaseParameterTestCase from ..unittest_utils import galaxy_mock @@ -39,9 +38,9 @@ class DataToolParameterTestCase(BaseParameterTestCase): assert field['options']['hda'][0]['name'] == "hda2" assert field['options']['hda'][1]['name'] == "hda1" - hda2.datatype_matches = False + hda2.extension = 'data' field = self._simple_field() - assert len(field['options']['hda']) == 1 + assert len(field['options']['hda']) == 1, field assert field['options']['hda'][0]['name'] == "hda1" def test_field_display_hidden_hdas_only_if_selected(self): @@ -66,7 +65,7 @@ class DataToolParameterTestCase(BaseParameterTestCase): def test_field_implicit_conversion_new(self): hda1 = MockHistoryDatasetAssociation(name="hda1", id=1) - hda1.datatype_matches = False + hda1.extension = 'data' hda1.conversion_destination = ("tabular", None) self.stub_active_datasets(hda1) field = self._simple_field() @@ -76,7 +75,7 @@ class DataToolParameterTestCase(BaseParameterTestCase): def test_field_implicit_conversion_existing(self): hda1 = MockHistoryDatasetAssociation(name="hda1", id=1) - hda1.datatype_matches = False + hda1.extension = 'data' hda1.conversion_destination = ("tabular", MockHistoryDatasetAssociation(name="hda1converted", id=2)) self.stub_active_datasets(hda1) field = self._simple_field() @@ -124,7 +123,7 @@ class DataToolParameterTestCase(BaseParameterTestCase): def test_get_initial_with_previously_converted_data(self): hda1 = MockHistoryDatasetAssociation(name="hda1", id=1) - hda1.datatype_matches = False + hda1.extension = 'data' converted = MockHistoryDatasetAssociation(name="hda1converted", id=2) hda1.conversion_destination = ("tabular", converted) self.stub_active_datasets(hda1) @@ -132,10 +131,10 @@ class DataToolParameterTestCase(BaseParameterTestCase): def test_get_initial_with_to_be_converted_data(self): hda1 = MockHistoryDatasetAssociation(name="hda1", id=1) - hda1.datatype_matches = False + hda1.extension = 'data' hda1.conversion_destination = ("tabular", None) self.stub_active_datasets(hda1) - assert hda1 == self.param.get_initial_value(self.trans, {}) + assert hda1 == self.param.get_initial_value(self.trans, {}), hda1 def _new_hda(self): hda = model.HistoryDatasetAssociation() @@ -170,7 +169,7 @@ class DataToolParameterTestCase(BaseParameterTestCase): optional_text = "" if self.optional: optional_text = 'optional="True"' - template_xml = '''''' + template_xml = '''''' param_str = template_xml % (multi_text, optional_text) self._param = self._parameter_for(tool=self.mock_tool, xml=param_str) @@ -190,11 +189,8 @@ class MockHistoryDatasetAssociation(object): self.deleted = False self.dataset = test_dataset self.visible = True - self.datatype_matches = True self.conversion_destination = (None, None) - self.datatype = bunch.Bunch( - matches_any=lambda formats: self.datatype_matches, - ) + self.extension = "txt" self.dbkey = "hg19" self.implicitly_converted_parent_datasets = False self.name = name diff --git a/test/unit/tools/test_dataset_matcher.py b/test/unit/tools/test_dataset_matcher.py index 6962f1e4449..c057c717c2d 100644 --- a/test/unit/tools/test_dataset_matcher.py +++ b/test/unit/tools/test_dataset_matcher.py @@ -13,32 +13,6 @@ from ..tools_support import UsesApp class DatasetMatcherTestCase(TestCase, UsesApp): - def test_hda_accessible(self): - # Cannot access errored or discard datasets. - self.mock_hda.dataset.state = model.Dataset.states.ERROR - assert not self.test_context.hda_accessible(self.mock_hda) - - self.mock_hda.dataset.state = model.Dataset.states.DISCARDED - assert not self.test_context.hda_accessible(self.mock_hda) - - # Can access datasets in other states. - self.mock_hda.dataset.state = model.Dataset.states.OK - assert self.test_context.hda_accessible(self.mock_hda) - - self.mock_hda.dataset.state = model.Dataset.states.QUEUED - assert self.test_context.hda_accessible(self.mock_hda) - - # Cannot access dataset if security agent says no. - self.app.security_agent.can_access_dataset = lambda roles, dataset: False - assert not self.test_context.hda_accessible(self.mock_hda) - - def test_selected(self): - self.test_context.value = [] - assert not self.test_context.selected(self.mock_hda) - - self.test_context.value = [self.mock_hda] - assert self.test_context.selected(self.mock_hda) - def test_hda_mismatches(self): # Datasets not visible are not "valid" for param. self.mock_hda.visible = False @@ -46,13 +20,13 @@ class DatasetMatcherTestCase(TestCase, UsesApp): # Datasets that don't match datatype are not valid. self.mock_hda.visible = True - self.mock_hda.datatype_matches = False + self.mock_hda.extension = 'data' assert not self.test_context.hda_match(self.mock_hda) def test_valid_hda_direct_match(self): # Datasets that visible and matching are valid self.mock_hda.visible = True - self.mock_hda.datatype_matches = True + self.mock_hda.extension = 'txt' hda_match = self.test_context.hda_match(self.mock_hda, check_implicit_conversions=False) assert hda_match @@ -64,7 +38,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp): def test_valid_hda_implicit_convered(self): # Find conversion returns an HDA to an already implicitly converted # dataset. - self.mock_hda.datatype_matches = False + self.mock_hda.extension = 'data' converted_hda = model.HistoryDatasetAssociation() self.mock_hda.conversion_destination = ("tabular", converted_hda) hda_match = self.test_context.hda_match(self.mock_hda) @@ -77,7 +51,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp): def test_hda_match_implicit_can_convert(self): # Find conversion returns a target extension to convert to, but not # a previously implicitly converted dataset. - self.mock_hda.datatype_matches = False + self.mock_hda.extension = 'data' self.mock_hda.conversion_destination = ("tabular", None) hda_match = self.test_context.hda_match(self.mock_hda) @@ -87,7 +61,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp): assert hda_match.target_ext == "tabular" def test_hda_match_properly_skips_conversion(self): - self.mock_hda.datatype_matches = False + self.mock_hda.extension = 'data' self.mock_hda.conversion_destination = ("tabular", bunch.Bunch()) hda_match = self.test_context.hda_match(self.mock_hda, check_implicit_conversions=False) assert not hda_match @@ -148,20 +122,18 @@ class DatasetMatcherTestCase(TestCase, UsesApp): option_xml = "" if self.filtered_param: option_xml = '''''' - param_xml = XML('''%s''' % option_xml) + param_xml = XML('''%s''' % option_xml) self.param = basic.DataToolParameter( self.tool, param_xml, ) - - self._test_context = dataset_matcher.DatasetMatcher( - trans=bunch.Bunch( - app=self.app, - get_current_user_roles=lambda: self.current_user_roles, - workflow_building_mode=True, - ), + trans = bunch.Bunch( + app=self.app, + get_current_user_roles=lambda: self.current_user_roles, + workflow_building_mode=True, + ) + self._test_context = dataset_matcher.get_dataset_matcher_factory(trans).dataset_matcher( param=self.param, - value=[], other_values=self.other_values ) From 740c4c93445d5833038e74ccb39413752eeeeb8c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 25 Apr 2018 09:39:24 -0400 Subject: [PATCH 08/10] ToolBuildOptimize - do not fetch hidden datasets for inclusion. Only fetch visible datasets into big, cached list of history datasets under consideration. Hidden datasets don't seem to be used by the fetcher or initial value stuff so it seems fine to exclude them. The advantage should be clear for histories with a large number of datasets hidden below a signficantly smaller number collections. --- lib/galaxy/model/__init__.py | 16 ++++++++++++++++ lib/galaxy/tools/parameters/basic.py | 5 +++-- test/unit/tools/test_data_parameters.py | 1 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index f859951a5a2..eb470cb1166 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1557,6 +1557,22 @@ class History(HasTags, Dictifiable, UsesAnnotations, HasName): self._active_datasets_and_roles = query.all() return self._active_datasets_and_roles + @property + def active_visible_datasets_and_roles(self): + if not hasattr(self, '_active_visible_datasets_and_roles'): + db_session = object_session(self) + query = (db_session.query(HistoryDatasetAssociation) + .filter(HistoryDatasetAssociation.table.c.history_id == self.id) + .filter(not_(HistoryDatasetAssociation.deleted)) + .filter(HistoryDatasetAssociation.visible) + .order_by(HistoryDatasetAssociation.table.c.hid.asc()) + .options(joinedload("dataset"), + joinedload("dataset.actions"), + joinedload("dataset.actions.role"), + joinedload("tags"))) + self._active_visible_datasets_and_roles = query.all() + return self._active_visible_datasets_and_roles + @property def active_contents(self): """ Return all active contents ordered by hid. diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index e49856a64df..3f9343206ee 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1486,7 +1486,7 @@ class BaseDataToolParameter(ToolParameter): dataset_matcher_factory = get_dataset_matcher_factory(trans) dataset_matcher = dataset_matcher_factory.dataset_matcher(self, other_values) if isinstance(self, DataToolParameter): - for hda in reversed(history.active_datasets_and_roles): + for hda in reversed(history.active_visible_datasets_and_roles): match = dataset_matcher.hda_match(hda) if match: return match.hda @@ -1805,7 +1805,8 @@ class DataToolParameter(BaseDataToolParameter): # add datasets hda_list = util.listify(other_values.get(self.name)) - for hda in history.active_datasets_and_roles: + # Prefetch all at once, big list of visible, non-deleted datasets. + for hda in history.active_visible_datasets_and_roles: match = dataset_matcher.hda_match(hda) if match: m = match.hda diff --git a/test/unit/tools/test_data_parameters.py b/test/unit/tools/test_data_parameters.py index 0b5d1587643..abc40180a5c 100644 --- a/test/unit/tools/test_data_parameters.py +++ b/test/unit/tools/test_data_parameters.py @@ -156,6 +156,7 @@ class DataToolParameterTestCase(BaseParameterTestCase): def stub_active_datasets(self, *hdas): self.test_history._active_datasets_and_roles = [h for h in hdas if not h.deleted] + self.test_history._active_visible_datasets_and_roles = [h for h in hdas if not h.deleted and h.visible] def _simple_field(self, **kwds): return self.param.to_dict(trans=self.trans, **kwds) From afe938fe04f1b70d77a357afd03df8252a375770 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 25 Apr 2018 09:41:52 -0400 Subject: [PATCH 09/10] ToolBuildOptimize - fetch fewer collections, prefetch more of HDCA. Low hanging fruit is to exclude hidden collections, that probably won't buy much for typical uses. This eliminates one extra tag query per dataset collection that appears in the rendered result, this probably buys us a bit more than the hidden collection thing but is still probably a good choice (unless there collections with a large number of tags :(...). This also eliminates the extra fetch of the first, outer-est collection associated the history dataset collection - not its elements just the collection. This saves a number of queries roughly equal to the number of HDCAs in the history and unlike the tag thing there is no downside really here - there will always be one collection. Before and after profiling of a tool form build: https://gist.github.com/jmchilton/d68565662f7f4b7ee2640f09fbb92962 --- lib/galaxy/model/__init__.py | 14 ++++++++++++++ lib/galaxy/tools/parameters/basic.py | 6 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index eb470cb1166..c5831db3c0a 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1573,6 +1573,20 @@ class History(HasTags, Dictifiable, UsesAnnotations, HasName): self._active_visible_datasets_and_roles = query.all() return self._active_visible_datasets_and_roles + @property + def active_visible_dataset_collections(self): + if not hasattr(self, '_active_visible_dataset_collections'): + db_session = object_session(self) + query = (db_session.query(HistoryDatasetCollectionAssociation) + .filter(HistoryDatasetCollectionAssociation.table.c.history_id == self.id) + .filter(not_(HistoryDatasetCollectionAssociation.deleted)) + .filter(HistoryDatasetCollectionAssociation.visible) + .order_by(HistoryDatasetCollectionAssociation.table.c.hid.asc()) + .options(joinedload("collection"), + joinedload("tags"))) + self._active_visible_dataset_collections = query.all() + return self._active_visible_dataset_collections + @property def active_contents(self): """ Return all active contents ordered by hid. diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 3f9343206ee..ba905c3ec9d 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1492,7 +1492,7 @@ class BaseDataToolParameter(ToolParameter): return match.hda else: dataset_collection_matcher = dataset_matcher_factory.dataset_collection_matcher(dataset_matcher) - for hdca in reversed(history.active_dataset_collections): + for hdca in reversed(history.active_visible_dataset_collections): if dataset_collection_matcher.hdca_match(hdca, reduction=self.multiple): return hdca @@ -1825,7 +1825,7 @@ class DataToolParameter(BaseDataToolParameter): # add dataset collections dataset_collection_matcher = dataset_matcher_factory.dataset_collection_matcher(dataset_matcher) - for hdca in history.active_dataset_collections: + for hdca in history.active_visible_dataset_collections: if dataset_collection_matcher.hdca_match(hdca, reduction=multiple): append(d['options']['hdca'], hdca, hdca.name, 'hdca') @@ -1870,7 +1870,7 @@ class DataCollectionToolParameter(BaseDataToolParameter): yield dataset_collection_instance def match_multirun_collections(self, trans, history, dataset_collection_matcher): - for history_dataset_collection in history.active_dataset_collections: + for history_dataset_collection in history.active_visible_dataset_collections: if not self._history_query(trans).can_map_over(history_dataset_collection): continue From fbc7d49de1b71e7a1a0e544a9c18c5651dad9dd8 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 30 Apr 2018 08:18:34 -0400 Subject: [PATCH 10/10] Fix for data collection parameters in tool state optimization branch. --- lib/galaxy/tools/parameters/dataset_matcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/dataset_matcher.py b/lib/galaxy/tools/parameters/dataset_matcher.py index ecc3d98a07a..7fd5b8eea81 100644 --- a/lib/galaxy/tools/parameters/dataset_matcher.py +++ b/lib/galaxy/tools/parameters/dataset_matcher.py @@ -67,7 +67,7 @@ class DatasetMatcherFactory(object): type_name = type(input).__name__ if "DataToolParameter" in type_name: self._data_inputs.append(input) - elif "DatasetCollectionToolParameter" in type_name: + elif "DataCollectionToolParameter" in type_name: self._data_inputs.append(input) tool.visit_inputs(param_values, visitor)