From 5b46c431b416e619a7ac5467527ab30743eabd33 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 3 Apr 2017 05:53:29 -0400 Subject: [PATCH] Fix certain aspects of dataset reductions in conditionals/repeats. For instance, fixes #3859 restoring the correct ``element_identifier`` for reduces collections in conditionals. Add tests for combinations of repeats and conditionals. --- lib/galaxy/tools/actions/__init__.py | 34 +++++++---- test/api/test_tools.py | 60 +++++++++++++++++++ .../identifier_multiple_in_conditional.xml | 25 ++++++++ .../tools/identifier_multiple_in_repeat.xml | 27 +++++++++ test/functional/tools/samples_tool_conf.xml | 2 + 5 files changed, 138 insertions(+), 10 deletions(-) create mode 100644 test/functional/tools/identifier_multiple_in_conditional.xml create mode 100644 test/functional/tools/identifier_multiple_in_repeat.xml diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index f1143d8a1ee..69efc2fe900 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -148,14 +148,14 @@ class DefaultToolAction( object ): input_dataset_collections = dict() - def visitor( input, value, prefix, parent=None, **kwargs ): + def visitor( input, value, prefix, parent=None, prefixed_name=None, **kwargs ): if isinstance( input, DataToolParameter ): values = value if not isinstance( values, list ): values = [ value ] for i, value in enumerate(values): if isinstance( value, model.HistoryDatasetCollectionAssociation ): - append_to_key( input_dataset_collections, prefix + input.name, ( value, True ) ) + append_to_key( input_dataset_collections, prefixed_name, ( value, True ) ) target_dict = parent if not target_dict: target_dict = param_values @@ -541,19 +541,33 @@ class DefaultToolAction( object ): # FIXME: Don't need all of incoming here, just the defined parameters # from the tool. We need to deal with tools that pass all post # parameters to the command as a special case. + reductions = {} for name, dataset_collection_info_pairs in inp_dataset_collections.items(): - first_reduction = True for ( dataset_collection, reduced ) in dataset_collection_info_pairs: - # TODO: update incoming for list... - if reduced and first_reduction: - first_reduction = False - incoming[ name ] = [] if reduced: - incoming[ name ].append( { 'id': dataset_collection.id, 'src': 'hdca' } ) - # Should verify security? We check security of individual - # datasets below? + if name not in reductions: + reductions[name] = [] + reductions[name].append(dataset_collection) + # TODO: verify can have multiple with same name, don't want to loose tracability job.add_input_dataset_collection( name, dataset_collection ) + + # If this an input collection is a reduction, we expanded it for dataset security, type + # checking, and such, but the persisted input must be the original collection + # so we can recover things like element identifier during tool command evaluation. + def restore_reduction_visitor( input, value, prefix, parent=None, prefixed_name=None, **kwargs ): + if prefixed_name in reductions and isinstance( input, DataToolParameter ): + target_dict = parent + if not target_dict: + target_dict = incoming + + target_dict[ input.name ] = [] + for reduced_collection in reductions[prefixed_name]: + target_dict[ input.name ].append( { 'id': reduced_collection.id, 'src': 'hdca' } ) + + if reductions: + tool.visit_inputs( incoming, restore_reduction_visitor ) + for name, value in tool.params_to_strings( incoming, trans.app ).items(): job.add_parameter( name, value ) self._check_input_data_access( trans, job, inp_data, current_user_roles ) diff --git a/test/api/test_tools.py b/test/api/test_tools.py index a9b123ea568..6b330c48cc9 100644 --- a/test/api/test_tools.py +++ b/test/api/test_tools.py @@ -788,6 +788,66 @@ class ToolsTestCase( api.ApiTestCase ): output1_content = self.dataset_populator.get_history_dataset_content( history_id, dataset=output1 ) self.assertEquals( output1_content.strip(), "forward\nreverse" ) + @skip_without_tool( "identifier_multiple_in_conditional" ) + def test_identifier_multiple_reduce_in_conditional( self ): + history_id = self.dataset_populator.new_history() + hdca_id = self.__build_pair( history_id, [ "123", "456" ] ) + inputs = { + "outer_cond|inner_cond|input1": { 'src': 'hdca', 'id': hdca_id }, + } + create_response = self._run( "identifier_multiple_in_conditional", history_id, inputs ) + self._assert_status_code_is( create_response, 200 ) + create = create_response.json() + outputs = create[ 'outputs' ] + jobs = create[ 'jobs' ] + implicit_collections = create[ 'implicit_collections' ] + self.assertEquals( len( jobs ), 1 ) + self.assertEquals( len( outputs ), 1 ) + self.assertEquals( len( implicit_collections ), 0 ) + output1 = outputs[ 0 ] + output1_content = self.dataset_populator.get_history_dataset_content( history_id, dataset=output1 ) + self.assertEquals( output1_content.strip(), "forward\nreverse" ) + + @skip_without_tool( "identifier_multiple_in_repeat" ) + def test_identifier_multiple_reduce_in_repeat( self ): + history_id = self.dataset_populator.new_history() + hdca_id = self.__build_pair( history_id, [ "123", "456" ] ) + inputs = { + "the_repeat_0|the_data|input1": { 'src': 'hdca', 'id': hdca_id }, + } + create_response = self._run( "identifier_multiple_in_repeat", history_id, inputs ) + self._assert_status_code_is( create_response, 200 ) + create = create_response.json() + outputs = create[ 'outputs' ] + jobs = create[ 'jobs' ] + implicit_collections = create[ 'implicit_collections' ] + self.assertEquals( len( jobs ), 1 ) + self.assertEquals( len( outputs ), 1 ) + self.assertEquals( len( implicit_collections ), 0 ) + output1 = outputs[ 0 ] + output1_content = self.dataset_populator.get_history_dataset_content( history_id, dataset=output1 ) + self.assertEquals( output1_content.strip(), "forward\nreverse" ) + + @skip_without_tool( "identifier_multiple_in_conditional" ) + def test_identifier_multiple_in_conditional( self ): + history_id = self.dataset_populator.new_history() + new_dataset1 = self.dataset_populator.new_dataset( history_id, content='123', name="Normal HDA1" ) + inputs = { + "outer_cond|inner_cond|input1": { 'src': 'hda', 'id': new_dataset1["id"] }, + } + create_response = self._run( "identifier_multiple_in_conditional", history_id, inputs ) + self._assert_status_code_is( create_response, 200 ) + create = create_response.json() + outputs = create[ 'outputs' ] + jobs = create[ 'jobs' ] + implicit_collections = create[ 'implicit_collections' ] + self.assertEquals( len( jobs ), 1 ) + self.assertEquals( len( outputs ), 1 ) + self.assertEquals( len( implicit_collections ), 0 ) + output1 = outputs[ 0 ] + output1_content = self.dataset_populator.get_history_dataset_content( history_id, dataset=output1 ) + self.assertEquals( output1_content.strip(), "Normal HDA1" ) + @skip_without_tool( "identifier_multiple" ) def test_identifier_with_multiple_normal_datasets( self ): history_id = self.dataset_populator.new_history() diff --git a/test/functional/tools/identifier_multiple_in_conditional.xml b/test/functional/tools/identifier_multiple_in_conditional.xml new file mode 100644 index 00000000000..f6f397ae51a --- /dev/null +++ b/test/functional/tools/identifier_multiple_in_conditional.xml @@ -0,0 +1,25 @@ + + > 'output1'; + #end for# + ]]> + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/identifier_multiple_in_repeat.xml b/test/functional/tools/identifier_multiple_in_repeat.xml new file mode 100644 index 00000000000..90db71b9b53 --- /dev/null +++ b/test/functional/tools/identifier_multiple_in_repeat.xml @@ -0,0 +1,27 @@ + + > 'output1'; + #end for# + #end for# + ]]> + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index b9f013458c4..e27a46b33ed 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -77,6 +77,8 @@ + +