Merge pull request #10694 from jmchilton/null_handling

Harden, formalize 'null' handling around boolean tool parameters.

lib/galaxy/tool_util/parser/xml.py

@@ -1,3 +1,4 @@
+import json
 import logging
 import re
 import uuid
@@ -620,6 +621,10 @@ def __parse_test_attributes(output_elem, attrib, parse_elements=False, parse_dis
 
     # File no longer required if an list of assertions was present.
     attributes = {}
+
+    if 'value_json' in attrib:
+        attributes['object'] = json.loads(attrib.pop('value_json'))
+
     # Method of comparison
     attributes['compare'] = attrib.pop('compare', 'diff').lower()
     # Number of lines to allow to vary in logs (for dates, etc)
@@ -654,7 +659,8 @@ def __parse_test_attributes(output_elem, attrib, parse_elements=False, parse_dis
 
     has_checksum = md5sum or checksum
     has_nested_tests = extra_files or element_tests or primary_datasets
-    if not (assert_list or file or metadata or has_checksum or has_nested_tests):
+    has_object = 'object' in attributes
+    if not (assert_list or file or metadata or has_checksum or has_nested_tests or has_object):
         raise Exception("Test output defines nothing to check (e.g. must have a 'file' check against, assertions to check, metadata or checksum tests, etc...)")
     attributes['assert_list'] = assert_list
     attributes['extra_files'] = extra_files
@@ -774,8 +780,11 @@ def __parse_param_elem(param_elem, i=0):
         value = attrib['values'].split(',')
     elif 'value' in attrib:
         value = attrib['value']
+    elif 'value_json' in attrib:
+        value = json.loads(attrib['value_json'])
     else:
         value = None
+
     children_elem = param_elem
     if children_elem is not None:
         # At this time, we can assume having children only

lib/galaxy/tool_util/verify/__init__.py

@@ -3,6 +3,7 @@
 import difflib
 import filecmp
 import hashlib
+import json
 import logging
 import os
 import os.path
@@ -88,7 +89,25 @@ def verify(
     if attributes is None:
         attributes = {}
 
-    if filename is not None:
+    # expected object might be None, so don't pull unless available
+    has_expected_object = 'object' in attributes
+    if has_expected_object:
+        assert filename is None
+        expected_object = attributes.get('object')
+        actual_object = json.loads(output_content)
+
+        expected_object_type = type(expected_object)
+        actual_object_type = type(actual_object)
+
+        if expected_object_type != actual_object_type:
+            message = f"Type mismatch between expected object ({expected_object_type}) and actual object ({actual_object_type})"
+            raise AssertionError(message)
+
+        if expected_object != actual_object:
+            message = f"Expected object ({expected_object}) does not match actual object ({actual_object})"
+            raise AssertionError(message)
+
+    elif filename is not None:
         temp_name = make_temp_fname(fname=filename)
         with open(temp_name, 'wb') as f:
             f.write(output_content)

lib/galaxy/tool_util/xsd/galaxy.xsd

@@ -1218,6 +1218,13 @@ associated input parameter (``param``).</xs:documentation>
 values that can be assigned to an input parameter.</xs:documentation>
       </xs:annotation>
     </xs:attribute>
+    <xs:attribute name="value_json" type="xs:string">
+      <xs:annotation>
+        <xs:documentation xml:lang="en">This variant of the value parameters can be
+used to load typed parameters. This string will be loaded as JSON and its type will
+attempt to be preserved through API requests to Galaxy.</xs:documentation>
+      </xs:annotation>
+    </xs:attribute>
     <xs:attribute name="ftype" type="xs:string">
       <xs:annotation>
         <xs:documentation xml:lang="en">This attribute name should be included
@@ -1307,6 +1314,16 @@ If specified, this value is the name of the output file stored in the target
 ``test-data`` directory which will be used to compare the results of executing
 the tool via the functional test framework.
 
+]]></xs:documentation>
+      </xs:annotation>
+    </xs:attribute>
+    <xs:attribute name="value_json" type="xs:string">
+      <xs:annotation>
+        <xs:documentation xml:lang="en"><![CDATA[
+
+If specified, this value will be loaded as JSON and compared against the output
+generated as JSON. This can be useful for testing tool outputs that are not files.
+
 ]]></xs:documentation>
       </xs:annotation>
     </xs:attribute>

lib/galaxy/tools/expressions/evaluation.py

@@ -35,4 +35,5 @@ def evaluate(config, input):
         message = "Expression engine returned non-zero exit code on evaluation of\n%s%s%s" % args
         raise Exception(message)
 
-    return json.loads(stdoutdata.decode("utf-8"))
+    rval_raw = stdoutdata.decode("utf-8")
+    return json.loads(rval_raw)

lib/galaxy/tools/parameters/basic.py

@@ -17,6 +17,7 @@ from galaxy.tool_util.parser import get_input_source as ensure_input_source
 from galaxy.util import (
     sanitize_param,
     string_as_bool,
+    string_as_bool_or_none,
     unicodify,
     XML,
 )
@@ -515,19 +516,23 @@ class BooleanToolParameter(ToolParameter):
         super().__init__(tool, input_source)
         self.truevalue = input_source.get('truevalue', 'true')
         self.falsevalue = input_source.get('falsevalue', 'false')
-        self.checked = input_source.get_bool('checked', False)
+        nullable = input_source.get_bool('optional', False)
+        self.optional = nullable
+        self.checked = input_source.get_bool('checked', None if nullable else False)
 
     def from_json(self, value, trans=None, other_values={}):
         return self.to_python(value)
 
     def to_python(self, value, app=None):
-        return (value in [True, 'True', 'true'])
+        if not self.optional:
+            ret_val = string_as_bool(value)
+        else:
+            ret_val = string_as_bool_or_none(value)
+        return ret_val
 
     def to_json(self, value, app, use_security):
-        if self.to_python(value, app):
-            return 'true'
-        else:
-            return 'false'
+        rval = json.dumps(self.to_python(value, app))
+        return rval
 
     def get_initial_value(self, trans, other_values):
         return self.checked
@@ -542,6 +547,7 @@ class BooleanToolParameter(ToolParameter):
         d = super().to_dict(trans)
         d['truevalue'] = self.truevalue
         d['falsevalue'] = self.falsevalue
+        d['optional'] = self.optional
         return d
 
     @property

lib/galaxy/tools/parameters/wrapped_json.py

@@ -92,7 +92,10 @@ def _json_wrap_input(input, value_wrapper, profile, handle_files="skip"):
     elif input_type == "integer":
         json_value = _cast_if_not_none(value_wrapper, int, empty_to_none=True)
     elif input_type == "boolean":
-        json_value = _cast_if_not_none(value_wrapper, bool)
+        if input.optional and value_wrapper is not None and value_wrapper.value is None:
+            json_value = None
+        else:
+            json_value = _cast_if_not_none(value_wrapper, bool, empty_to_none=input.optional)
     elif input_type == "select":
         if input.multiple and packaging.version.parse(str(profile)) >= packaging.version.parse('20.05'):
             json_value = [_ for _ in _cast_if_not_none(value_wrapper.value, list)]

lib/galaxy/tools/test.py

@@ -8,6 +8,7 @@ import galaxy.tools.parameters.grouping
 from galaxy.tool_util.verify.interactor import ToolTestDescription
 from galaxy.util import (
     string_as_bool,
+    string_as_bool_or_none,
     unicodify,
 )
 
@@ -271,7 +272,10 @@ def _process_bool_param_value(param, param_value):
     elif param.falsevalue == param_value:
         processed_value = False
     else:
-        processed_value = string_as_bool(param_value)
+        if param.optional:
+            processed_value = string_as_bool_or_none(param_value)
+        else:
+            processed_value = string_as_bool(param_value)
     return [processed_value] if was_list else processed_value
 
 

lib/galaxy/util/__init__.py

@@ -958,7 +958,7 @@ def string_as_bool_or_none(string):
     string = str(string).lower()
     if string in ('true', 'yes', 'on'):
         return True
-    elif string == 'none':
+    elif string in ['none', 'null']:
         return None
     else:
         return False

lib/galaxy_ext/expressions/handle_job.py

@@ -38,10 +38,16 @@ def run(environment_path=None):
     del inputs["outputs"]
 
     result = evaluate(None, inputs)
-
     for output in outputs:
         path = output["path"]
-        from_expression = "$(" + output["from_expression"] + ")"
-        output_value = expression.interpolate(from_expression, result)
+        from_expression = output["from_expression"]
+        # if it is just a simple value, short-cut all the interpolation
+        # interpolate seems to fail with None values so this worksaround
+        # that for now.
+        if from_expression in result:
+            output_value = result[from_expression]
+        else:
+            from_expression = f"$({from_expression})"
+            output_value = expression.interpolate(from_expression, result)
         with open(path, "w") as f:
             json.dump(output_value, f)

lib/galaxy_test/api/test_tools.py

@@ -336,6 +336,19 @@ class ToolsTestCase(ApiTestCase, TestsTools):
         self._assert_has_keys(test_case, "inputs", "outputs", "output_collections", "required_files")
         assert len(test_case["inputs"]) == 1, test_case
 
+    @skip_without_tool("expression_null_handling_1")
+    def test_test_data_null_boolean_inputs(self):
+        test_data_response = self._get("tools/%s/test_data" % "expression_null_handling_1")
+        assert test_data_response.status_code == 200
+        test_data = test_data_response.json()
+        assert len(test_data) == 3
+        test_case = test_data[2]
+        self._assert_has_keys(test_case, "inputs", "outputs", "output_collections", "required_files")
+        inputs = test_case["inputs"]
+        assert len(inputs) == 1, test_case
+        assert "bool_input" in inputs, inputs
+        assert inputs["bool_input"] is None, inputs
+
     @skip_without_tool("simple_constructs_y")
     def test_test_data_yaml_tools(self):
         test_data_response = self._get("tools/%s/test_data" % "simple_constructs_y")

test/functional/tools/expression_null_handling_boolean.xml

@@ -0,0 +1,26 @@
+<tool name="expression_null_handling_boolean" id="expression_null_handling_boolean"
+	  version="0.1.0" tool_type="expression">
+    <expression type="ecma5.1">
+        {return {'bool_out': $job.bool_input};}
+    </expression>
+    <inputs>
+        <param type="boolean" label="Booelan input." name="bool_input" optional="true" />
+    </inputs>
+    <outputs>
+        <output type="bool" name="bool_out" from="bool_out" />
+    </outputs>
+    <tests>
+        <test>
+            <param name="bool_input" value_json="true" />
+            <output name="bool_out" value_json="true" />
+        </test>
+        <test>
+            <param name="bool_input" value_json="false" />
+            <output name="bool_out" value_json="false" />
+        </test>
+        <test>
+            <param name="bool_input" value_json="null" />
+            <output name="bool_out" value_json="null" />
+        </test>
+    </tests>
+</tool>

test/functional/tools/samples_tool_conf.xml

@@ -71,7 +71,7 @@
   <tool file="sim_size_delta.xml" />
   <tool file="composite_shapefile.xml" />
   <tool file="is_valid_xml.xml" />
-  <tool file="param_value_from_file.xml"/>
+  <tool file="parse_values_from_file.xml"/>
   <!--
   TODO: Figure out why this transiently fails on Jenkins.
   <tool file="maxseconds.xml" />
@@ -173,6 +173,7 @@
   <tool file="expression_forty_two.xml" />
   <tool file="expression_parse_int.xml" />
   <tool file="expression_log_line_count.xml" />
+  <tool file="expression_null_handling_boolean.xml" />
   <tool file="cheetah_casting.xml" />
   <tool file="cheetah_problem_unbound_var.xml" />
   <tool file="cheetah_problem_unbound_var_input.xml" />

test/unit/tool_util/test_parsing.py

@@ -589,6 +589,35 @@ class BuildListToolLoaderTestCase(BaseLoaderTestCase):
         assert tool_module[1] == "BuildListCollectionTool"
 
 
+class ExpressionTestToolLoaderTestCase(BaseLoaderTestCase):
+    source_file_name = os.path.join(galaxy_directory(), "test/functional/tools/expression_null_handling_boolean.xml")
+    source_contents = None
+
+    def test_test(self):
+        test_dicts = self._tool_source.parse_tests_to_dict()['tests']
+        assert len(test_dicts) == 3
+        test_dict_0 = test_dicts[0]
+        assert 'outputs' in test_dict_0, test_dict_0
+        outputs = test_dict_0['outputs']
+        output0 = outputs[0]
+        assert 'object' in output0['attributes']
+        assert output0['attributes']['object'] is True
+
+        test_dict_1 = test_dicts[1]
+        assert 'outputs' in test_dict_1, test_dict_1
+        outputs = test_dict_1['outputs']
+        output0 = outputs[0]
+        assert 'object' in output0['attributes']
+        assert output0['attributes']['object'] is False
+
+        test_dict_2 = test_dicts[2]
+        assert 'outputs' in test_dict_2, test_dict_2
+        outputs = test_dict_2['outputs']
+        output0 = outputs[0]
+        assert 'object' in output0['attributes']
+        assert output0['attributes']['object'] is None
+
+
 class SpecialToolLoaderTestCase(BaseLoaderTestCase):
     source_file_name = os.path.join(galaxy_directory(), "lib/galaxy/tools/imp_exp/exp_history_to_archive.xml")
     source_contents = None