Merge pull request #1572 from guerler/fix_unvalidated_008

Fix parameter validation

lib/galaxy/jobs/__init__.py

@@ -965,9 +965,6 @@ def fail( self, message, exception=False, stdout="", stderr="", exit_code=None )
                 # Get the exception and let the tool attempt to generate
                 # a better message
                 etype, evalue, tb = sys.exc_info()
-                m = self.tool.handle_job_failure_exception( evalue )
-                if m:
-                    message = m
             if self.app.config.outputs_to_working_directory:
                 for dataset_path in self.get_output_fnames():
                     try:
@@ -1301,9 +1298,7 @@ def path_rewriter( path ):
             if not data:
                 continue
             input_ext = data.ext
-        # why not re-use self.param_dict here? ##dunno...probably should, this causes
-        # tools.parameters.basic.UnvalidatedValue to be used in following methods
-        # instead of validated and transformed values during i.e. running workflows
+        # why not re-use self.param_dict here?
         param_dict = dict( [ ( p.name, p.value ) for p in job.parameters ] )
         param_dict = self.tool.params_from_strings( param_dict, self.app )
         # Create generated output children and primary datasets and add to param_dict

lib/galaxy/jobs/runners/__init__.py

@@ -169,9 +169,9 @@ def prepare_job(self, job_wrapper, include_metadata=False, include_work_dir_outp
                 include_metadata=include_metadata,
                 include_work_dir_outputs=include_work_dir_outputs,
             )
-        except:
+        except Exception as e:
             log.exception("(%s) Failure preparing job" % job_id)
-            job_wrapper.fail( "failure preparing job", exception=True )
+            job_wrapper.fail( e.message if hasattr( e, 'message' ) else "Job preparation failed", exception=True )
             return False
 
         if not job_wrapper.runner_command_line:

lib/galaxy/tools/actions/__init__.py

@@ -6,6 +6,7 @@
 from galaxy import model
 from galaxy.tools.parameters.basic import DataCollectionToolParameter, DataToolParameter
 from galaxy.tools.parameters.wrapped import WrappedParameters
+from galaxy.tools.parameters import update_param
 from galaxy.util import ExecutionTimer
 from galaxy.util.json import dumps
 from galaxy.util.none_like import NoneDataset
@@ -491,9 +492,10 @@ def handle_output( name, output, hidden=None ):
                                 if hda.state == hda.states.PAUSED:
                                     hda.state = hda.states.NEW
                                     hda.info = None
+                            input_values = dict( [ ( p.name, json.loads( p.value ) ) for p in job_to_remap.parameters ] )
+                            update_param( jtid.name, input_values, str( out_data[ jtod.name ].id ) )
                             for p in job_to_remap.parameters:
-                                if p.name == jtid.name and p.value == str(jtod.dataset.id):
-                                    p.value = str(out_data[jtod.name].id)
+                                p.value = json.dumps( input_values[ p.name ] )
                             jtid.dataset = out_data[jtod.name]
                             jtid.dataset.hid = jtod.dataset.hid
                             log.info('Job %s input HDA %s remapped to new HDA %s' % (job_to_remap.id, jtod.dataset.id, jtid.dataset.id))

lib/galaxy/tools/evaluation.py

@@ -21,11 +21,11 @@
     DataCollectionToolParameter,
     SelectToolParameter,
 )
-from galaxy.tools.parameters import wrapped_json
+from galaxy.tools.parameters import wrapped_json, visit_input_values
 from galaxy.tools.parameters.grouping import Conditional, Repeat, Section
 from galaxy.tools import global_tool_errors
 from galaxy.jobs.datasets import dataset_path_rewrites
-
+from galaxy.work.context import WorkRequestContext
 import logging
 log = logging.getLogger( __name__ )
 
@@ -52,8 +52,15 @@ def set_compute_environment( self, compute_environment, get_special=None ):
         job = self.job
         incoming = dict( [ ( p.name, p.value ) for p in job.parameters ] )
         incoming = self.tool.params_from_strings( incoming, self.app )
-        # Do any validation that could not be done at job creation
-        self.tool.handle_unvalidated_param_values( incoming, self.app )
+
+        # Full parameter validation
+        request_context = WorkRequestContext( app=self.app, user=job.history and job.history.user, history=job.history )
+
+        def validate_inputs( input, value, prefixed_name, prefixed_label, context ):
+            value = input.from_html( value, request_context, context )
+            input.validate( value, request_context )
+        visit_input_values( self.tool.inputs, incoming, validate_inputs, details=True )
+
         # Restore input / output data lists
         inp_data = dict( [ ( da.name, da.dataset ) for da in job.input_datasets ] )
         out_data = dict( [ ( da.name, da.dataset ) for da in job.output_datasets ] )

lib/galaxy/tools/execute.py

@@ -39,7 +39,7 @@ def execute_single_job(params):
         # the state we about to execute one last time. Consider whether tool executions
         # should run this as well.
         if workflow_invocation_uuid:
-            messages = tool.check_and_update_param_values( params, trans, update_values=False, allow_workflow_parameters=False )
+            messages = tool.check_and_update_param_values( params, trans, update_values=False )
             if messages:
                 execution_tracker.record_error( messages )
                 return

lib/galaxy/tools/imp_exp/__init__.py

@@ -9,7 +9,6 @@
 
 from galaxy import model
 from galaxy.model.item_attrs import UsesAnnotations
-from galaxy.tools.parameters.basic import UnvalidatedValue
 from galaxy.util.json import dumps, loads
 from galaxy.web.framework.helpers import to_unicode
 
@@ -382,8 +381,6 @@ def default( self, obj ):
                     else:
                         rval['exported'] = True
                     return rval
-                if isinstance( obj, UnvalidatedValue ):
-                    return obj.__str__()
                 return json.JSONEncoder.default( self, obj )
 
         #

lib/galaxy/tools/parameters/__init__.py

@@ -1,15 +1,17 @@
 """
 Classes encapsulating Galaxy tool parameters.
 """
-
+import re
 from basic import DataCollectionToolParameter, DataToolParameter, SelectToolParameter
 from grouping import Conditional, Repeat, Section, UploadDataset
+from galaxy.util import string_as_bool
 from galaxy.util.json import dumps, json_fix, loads
+from galaxy.util.expressions import ExpressionContext
 
 REPLACE_ON_TRUTHY = object()
 
 
-def visit_input_values( inputs, input_values, callback, name_prefix="", label_prefix="", no_replacement_value=REPLACE_ON_TRUTHY ):
+def visit_input_values( inputs, input_values, callback, name_prefix="", label_prefix="", no_replacement_value=REPLACE_ON_TRUTHY, context=None, details=False ):
     """
     Given a tools parameter definition (`inputs`) and a specific set of
     parameter `values`, call `callback` for each non-grouping parameter,
@@ -22,30 +24,33 @@ def visit_input_values( inputs, input_values, callback, name_prefix="", label_pr
            Repeat and Group. This tracks labels and those do not. It would
            be nice to unify all the places that recursively visit inputs.
     """
+    context = ExpressionContext( input_values, context )
     for input in inputs.itervalues():
         if isinstance( input, Repeat ) or isinstance( input, UploadDataset ):
             for i, d in enumerate( input_values[ input.name ] ):
                 index = d['__index__']
                 new_name_prefix = name_prefix + "%s_%d|" % ( input.name, index )
                 new_label_prefix = label_prefix + "%s %d > " % ( input.title, i + 1 )
-                visit_input_values( input.inputs, d, callback, new_name_prefix, new_label_prefix, no_replacement_value=no_replacement_value )
+                visit_input_values( input.inputs, d, callback, new_name_prefix, new_label_prefix, no_replacement_value=no_replacement_value, context=context, details=details )
         elif isinstance( input, Conditional ):
             values = input_values[ input.name ]
             current = values["__current_case__"]
-            label_prefix = label_prefix
             new_name_prefix = name_prefix + input.name + "|"
-            visit_input_values( input.cases[current].inputs, values, callback, new_name_prefix, label_prefix, no_replacement_value=no_replacement_value )
+            visit_input_values( input.cases[current].inputs, values, callback, new_name_prefix, label_prefix, no_replacement_value=no_replacement_value, context=context, details=details )
         elif isinstance( input, Section ):
             values = input_values[ input.name ]
-            label_prefix = label_prefix
             new_name_prefix = name_prefix + input.name + "|"
-            visit_input_values( input.inputs, values, callback, new_name_prefix, label_prefix, no_replacement_value=no_replacement_value )
+            visit_input_values( input.inputs, values, callback, new_name_prefix, label_prefix, no_replacement_value=no_replacement_value, context=context, details=details )
         else:
-            new_value = callback( input,
-                                  input_values[input.name],
-                                  prefixed_name=name_prefix + input.name,
-                                  prefixed_label=label_prefix + input.label )
-
+            args = {
+                'input'             : input,
+                'value'             : input_values[ input.name ],
+                'prefixed_name'     : "%s%s" % ( name_prefix, input.name ),
+                'prefixed_label'    : "%s%s" % ( label_prefix, input.label )
+            }
+            if details:
+                args[ 'context' ] = context
+            new_value = callback( **args )
             if no_replacement_value is REPLACE_ON_TRUTHY:
                 replace = bool(new_value)
             else:
@@ -54,7 +59,7 @@ def visit_input_values( inputs, input_values, callback, name_prefix="", label_pr
                 input_values[input.name] = new_value
 
 
-def check_param( trans, param, incoming_value, param_values, source='html', history=None, workflow_building_mode=False ):
+def check_param( trans, param, incoming_value, param_values, source='html', boolean_fix=False ):
     """
     Check the value of a single parameter `param`. The value in
     `incoming_value` is converted from its HTML encoding and validated.
@@ -65,21 +70,20 @@ def check_param( trans, param, incoming_value, param_values, source='html', hist
     value = incoming_value
     error = None
     try:
-        if history is None:
-            history = trans.history
+        # resolves the inconsistent definition of boolean parameters (see base.py) without modifying shared code
+        if boolean_fix and param.type == 'boolean' and isinstance( value, basestring ):
+            return [ string_as_bool( value ), None ]
         if value is not None or isinstance( param, DataToolParameter ) or isinstance( param, DataCollectionToolParameter ):
             # Convert value from HTML representation
             if source == 'html':
                 value = param.from_html( value, trans, param_values )
             else:
                 value = param.from_json( value, trans, param_values )
-            # Allow the value to be converted if necessary
-            filtered_value = param.filter_value( value, trans, param_values )
             # Then do any further validation on the value
-            param.validate( filtered_value, history, workflow_building_mode=workflow_building_mode )
+            param.validate( value, trans )
         elif value is None and isinstance( param, SelectToolParameter ):
             # An empty select list or column list
-            param.validate( value, history, workflow_building_mode=workflow_building_mode )
+            param.validate( value, trans )
     except ValueError, e:
         error = str( e )
     return value, error
@@ -117,7 +121,7 @@ def params_from_strings( params, param_values, app, ignore_errors=False ):
     return rval
 
 
-def params_to_incoming( incoming, inputs, input_values, app, name_prefix="", to_html=True ):
+def params_to_incoming( incoming, inputs, input_values, app, name_prefix="" ):
     """
     Given a tool's parameter definition (`inputs`) and a specific set of
     parameter `input_values` objects, populate `incoming` with the html values.
@@ -129,19 +133,36 @@ def params_to_incoming( incoming, inputs, input_values, app, name_prefix="", to_
             for d in input_values[ input.name ]:
                 index = d['__index__']
                 new_name_prefix = name_prefix + "%s_%d|" % ( input.name, index )
-                params_to_incoming( incoming, input.inputs, d, app, new_name_prefix, to_html=to_html)
+                params_to_incoming( incoming, input.inputs, d, app, new_name_prefix )
         elif isinstance( input, Conditional ):
             values = input_values[ input.name ]
             current = values["__current_case__"]
             new_name_prefix = name_prefix + input.name + "|"
             incoming[ new_name_prefix + input.test_param.name ] = values[ input.test_param.name ]
-            params_to_incoming( incoming, input.cases[current].inputs, values, app, new_name_prefix, to_html=to_html )
+            params_to_incoming( incoming, input.cases[current].inputs, values, app, new_name_prefix )
         elif isinstance( input, Section ):
             values = input_values[ input.name ]
             new_name_prefix = name_prefix + input.name + "|"
-            params_to_incoming( incoming, input.inputs, values, app, new_name_prefix, to_html=to_html )
+            params_to_incoming( incoming, input.inputs, values, app, new_name_prefix )
         else:
             value = input_values.get( input.name )
-            if to_html:
-                value = input.to_html_value( value, app )
             incoming[ name_prefix + input.name ] = value
+
+
+def update_param( prefixed_name, input_values, new_value ):
+    """
+    Given a prefixed parameter name, e.g. 'parameter_0|parameter_1', update
+    the corresponding input value in a nested input values dictionary.
+    """
+    for key in input_values:
+        match = re.match( '^' + key + '_(\d+)\|(.+)', prefixed_name )
+        if match:
+            index = int( match.group( 1 ) )
+            if isinstance( input_values[ key ], list ) and len( input_values[ key ] ) > index:
+                update_param( match.group( 2 ), input_values[ key ][ index ], new_value )
+        else:
+            match = re.match( '^' + key + '\|(.+)', prefixed_name )
+            if isinstance( input_values[ key ], dict ) and match:
+                update_param( match.group( 1 ), input_values[ key ], new_value )
+            elif prefixed_name == key:
+                input_values[ key ] = new_value