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

@@ -148,14 +148,14 @@ def append_to_key( the_dict, key, value ):
 
         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 @@ def _record_inputs( self, trans, tool, job, incoming, inp_data, inp_dataset_coll
         # 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 )

test/api/test_tools.py

@@ -788,6 +788,66 @@ def test_identifier_in_multiple_reduce( self ):
         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()

test/functional/tools/identifier_multiple_in_conditional.xml

@@ -0,0 +1,25 @@
+<tool id="identifier_multiple_in_conditional" name="identifier_multiple_in_conditional">
+  <command><![CDATA[
+    #for $input in $outer_cond.inner_cond.input1#
+    echo '$input.element_identifier' >> 'output1';
+    #end for#
+  ]]></command>
+  <inputs>
+    <conditional name="outer_cond">
+      <param name="cond_param_outer" type="boolean" checked="true" />
+      <when value="true">
+        <conditional name="inner_cond">
+          <param name="cond_param_inner" type="boolean" checked="true" />
+          <when value="true">
+            <param type="data" name="input1" label="True Input" multiple="true" />
+          </when>
+        </conditional>
+      </when>
+    </conditional>
+  </inputs>
+  <outputs>
+    <data name="output1" format="tabular" from_work_dir="output1" />
+  </outputs>
+  <tests>
+  </tests>
+</tool>

test/functional/tools/identifier_multiple_in_repeat.xml

@@ -0,0 +1,27 @@
+<tool id="identifier_multiple_in_repeat" name="identifier_multiple_in_repeat">
+  <command><![CDATA[
+    #for $repeat_instance in $the_repeat#
+      #for $input in $repeat_instance.the_data.input1#
+        echo '$input.element_identifier' >> 'output1';
+      #end for#
+    #end for#
+  ]]></command>
+  <inputs>
+    <repeat name="the_repeat" title="Repeat Inputs">
+      <conditional name="the_data">
+        <param name="cond_param" type="boolean" />
+        <when value="true">
+          <param type="data" name="input1" label="True Input" multiple="true" />
+        </when>
+        <when value="false">
+          <param type="data" name="input1" label="False Input" multiple="true" />
+        </when>
+      </conditional>
+    </repeat>
+  </inputs>
+  <outputs>
+    <data name="output1" format="tabular" from_work_dir="output1" />
+  </outputs>
+  <tests>
+  </tests>
+</tool>

test/functional/tools/samples_tool_conf.xml

@@ -77,6 +77,8 @@
   <tool file="explicit_conversion.xml" />
   <tool file="identifier_single.xml" />
   <tool file="identifier_multiple.xml" />
+  <tool file="identifier_multiple_in_conditional.xml" />
+  <tool file="identifier_multiple_in_repeat.xml" />
   <tool file="identifier_collection.xml" />
   <tool file="tool_directory.xml" />
   <tool file="output_action_change_format.xml" />