Merge pull request galaxyproject#19219 from mvdbeek/required_paramete…

…rs_with_default

[24.2] Fix default value handling for parameters connected to required parameters

lib/galaxy/model/__init__.py

@@ -8174,10 +8174,6 @@ def input_type(self):
         assert self.is_input_type, "step.input_type can only be called on input step types"
         return self.STEP_TYPE_TO_INPUT_TYPE[self.type]
 
-    @property
-    def input_default_value(self):
-        self.get_input_default_value(None)
-
     def get_input_default_value(self, default_default):
         # parameter_input and the data parameters handle this slightly differently
         # unfortunately.

lib/galaxy/tool_util/models.py

@@ -172,6 +172,7 @@ class TestJob(StrictModel):
     doc: Optional[str]
     job: JobDict
     outputs: Dict[str, TestOutputAssertions]
+    expect_failure: Optional[bool] = False
 
 
 Tests = RootModel[List[TestJob]]
@@ -185,6 +186,7 @@ class TestJob(StrictModel):
 class TestJobDict(TypedDict):
     doc: NotRequired[str]
     job: NotRequired[JobDict]
+    expect_failure: NotRequired[bool]
     outputs: OutputsDict
 
 

lib/galaxy/tool_util/parser/parameter_validators.py

@@ -313,7 +313,7 @@ class EmptyFieldParameterValidatorModel(StaticValidatorModel):
 
     @staticmethod
     def empty_validate(value: Any, validator: "ValidatorDescription"):
-        raise_error_if_valiation_fails((value != ""), validator)
+        raise_error_if_valiation_fails((value not in ("", None)), validator)
 
     def statically_validate(self, value: Any) -> None:
         EmptyFieldParameterValidatorModel.empty_validate(value, self)

lib/galaxy/tools/parameters/basic.py

@@ -368,11 +368,7 @@ def __init__(self, tool, input_source):
 
     def to_json(self, value, app, use_security):
         """Convert a value to a string representation suitable for persisting"""
-        if value is None:
-            rval = "" if not self.optional else None
-        else:
-            rval = unicodify(value)
-        return rval
+        return unicodify(value)
 
     def get_initial_value(self, trans, other_values):
         return self.value

lib/galaxy/workflow/modules.py

@@ -1260,12 +1260,15 @@ def get_inputs(self):
 
             when_true = ConditionalWhen()
             when_true.value = "true"
-            when_true.inputs = {}
-            when_true.inputs["default"] = specify_default_cond
+            when_true.inputs = {"default": specify_default_cond}
 
             when_false = ConditionalWhen()
             when_false.value = "false"
-            when_false.inputs = {}
+            # This is only present for backwards compatibility,
+            # We don't need this conditional since you can set
+            # a default value for optional and required parameters.
+            # TODO: version the state and upgrade it to a simpler version
+            when_false.inputs = {"default": specify_default_cond}
 
             optional_cases = [when_true, when_false]
             optional_cond.cases = optional_cases
@@ -1504,6 +1507,7 @@ def get_runtime_inputs(self, step, connections: Optional[Iterable[WorkflowStepCo
         parameter_def = self._parse_state_into_dict()
         parameter_type = parameter_def["parameter_type"]
         optional = parameter_def["optional"]
+        default_value_set = "default" in parameter_def
         default_value = parameter_def.get("default", self.default_default_value)
         if parameter_type not in ["text", "boolean", "integer", "float", "color", "directory_uri"]:
             raise ValueError("Invalid parameter type for workflow parameters encountered.")
@@ -1557,7 +1561,7 @@ def _parameter_def_list_to_options(parameter_value):
 
         parameter_class = parameter_types[client_parameter_type]
 
-        if optional:
+        if default_value_set:
             if client_parameter_type == "select":
                 parameter_kwds["selected"] = default_value
             else:
@@ -1633,7 +1637,6 @@ def step_state_to_tool_state(self, state):
         if "default" in state:
             default_set = True
             default_value = state["default"]
-            state["optional"] = True
         multiple = state.get("multiple")
         source_validators = state.get("validators")
         restrictions = state.get("restrictions")

lib/galaxy/workflow/run.py

@@ -574,8 +574,8 @@ def set_outputs_for_input(
         if self.inputs_by_step_id:
             step_id = step.id
             if step_id not in self.inputs_by_step_id and "output" not in outputs:
-                default_value = step.input_default_value
-                if default_value or step.input_optional:
+                default_value = step.get_input_default_value(modules.NO_REPLACEMENT)
+                if default_value is not modules.NO_REPLACEMENT:
                     outputs["output"] = default_value
                 else:
                     log.error(f"{step.log_str()} not found in inputs_step_id {self.inputs_by_step_id}")
@@ -588,7 +588,8 @@ def set_outputs_for_input(
                         )
                     )
             elif step_id in self.inputs_by_step_id:
-                outputs["output"] = self.inputs_by_step_id[step_id]
+                if self.inputs_by_step_id[step_id] is not None or "output" not in outputs:
+                    outputs["output"] = self.inputs_by_step_id[step_id]
 
         if step.label and step.type == "parameter_input" and "output" in outputs:
             self.runtime_replacements[step.label] = str(outputs["output"])

lib/galaxy_test/api/test_workflows.py

@@ -3212,7 +3212,7 @@ def test_export_invocation_ro_crate_adv(self):
 """,
                 test_data="""
 num_lines_param:
-  type: int
+  type: raw
   value: 2
 input collection 1:
   collection_type: list
@@ -7034,12 +7034,14 @@ def test_subworkflow_import_order_maintained(self, history_id):
   outer_input_1:
     type: int
     default: 1
+    optional: true
     position:
       left: 0
       top: 0
   outer_input_2:
     type: int
     default: 2
+    optional: true
     position:
       left: 100
       top: 0

lib/galaxy_test/base/populators.py

@@ -3362,8 +3362,6 @@ def read_test_data(test_dict):
             hda = dataset_populator.new_dataset(history_id, content=value)
             label_map[key] = dataset_populator.ds_entry(hda)
             inputs[key] = hda
-        else:
-            raise ValueError(f"Invalid test_data def {test_data}")
 
     return inputs, label_map, has_uploads
 

lib/galaxy_test/base/workflow_fixtures.py

@@ -617,6 +617,7 @@
   int_input:
     type: integer
     default: 3
+    optional: true
 steps:
   random:
     tool_id: random_lines1

lib/galaxy_test/workflow/default_values.gxwf-tests.yml

@@ -0,0 +1,34 @@
+- doc: |
+    Test that default value doesn't need to be supplied
+  job: {}
+  outputs:
+    out:
+      class: File
+      asserts:
+      - that: has_text
+        text: "1"
+- doc: |
+    Test that null is replaced with default value (follows https://www.commonwl.org/v1.2/Workflow.html#WorkflowInputParameter)
+  job:
+    required_int_with_default:
+      type: raw
+      value: null
+  outputs:
+    out:
+      class: File
+      asserts:
+      - that: has_text
+        text: "1"
+- doc: |
+    Test that empty string is not replaced and fails
+  expect_failure: true
+  job:
+    required_int_with_default:
+      type: raw
+      value: ""
+  outputs:
+    out:
+      class: File
+      asserts:
+      - that: has_text
+        text: "1"

lib/galaxy_test/workflow/default_values.gxwf.yml

@@ -0,0 +1,17 @@
+class: GalaxyWorkflow
+inputs:
+  required_int_with_default:
+    type: int
+    default: 1
+outputs:
+  out:
+    outputSource: integer_default/out_file1
+steps:
+  integer_default:
+    tool_id: integer_default
+    tool_state:
+      input1: 0
+      input2: 0
+    in:
+      input3:
+        source: required_int_with_default

lib/galaxy_test/workflow/default_values_optional.gxwf-tests.yml

@@ -0,0 +1,34 @@
+- doc: |
+    Test that default value doesn't need to be supplied
+  job: {}
+  outputs:
+    out:
+      class: File
+      asserts:
+      - that: has_text
+        text: "1"
+- doc: |
+    Test that null is replaced with default value (follows https://www.commonwl.org/v1.2/Workflow.html#WorkflowInputParameter)
+  job:
+    optional_int_with_default:
+      type: raw
+      value: null
+  outputs:
+    out:
+      class: File
+      asserts:
+      - that: has_text
+        text: "1"
+- doc: |
+    Test that empty string is not replaced and fails
+  expect_failure: true
+  job:
+    optional_int_with_default:
+      type: raw
+      value: ""
+  outputs:
+    out:
+      class: File
+      asserts:
+      - that: has_text
+        text: "1"

lib/galaxy_test/workflow/default_values_optional.gxwf.yml

@@ -0,0 +1,18 @@
+class: GalaxyWorkflow
+inputs:
+  optional_int_with_default:
+    type: int
+    default: 1
+    optional: true
+outputs:
+  out:
+    outputSource: integer_default/out_file1
+steps:
+  integer_default:
+    tool_id: integer_default
+    tool_state:
+      input1: 0
+      input2: 0
+    in:
+      input3:
+        source: optional_int_with_default

lib/galaxy_test/workflow/test_framework_workflows.py

@@ -64,14 +64,23 @@ def test_workflow(self, workflow_path: Path, test_job: TestJobDict):
         with workflow_path.open() as f:
             yaml_content = ordered_load(f)
         with self.dataset_populator.test_history() as history_id:
-            run_summary = self.workflow_populator.run_workflow(
-                yaml_content,
-                test_data=test_job["job"],
-                history_id=history_id,
-            )
-            if TEST_WORKFLOW_AFTER_RERUN:
-                run_summary = self.workflow_populator.rerun(run_summary)
-            self._verify(run_summary, test_job["outputs"])
+            exc = None
+            try:
+                run_summary = self.workflow_populator.run_workflow(
+                    yaml_content,
+                    test_data=test_job["job"],
+                    history_id=history_id,
+                )
+                if TEST_WORKFLOW_AFTER_RERUN:
+                    run_summary = self.workflow_populator.rerun(run_summary)
+                self._verify(run_summary, test_job["outputs"])
+            except Exception as e:
+                exc = e
+            if test_job.get("expect_failure"):
+                if not exc:
+                    raise Exception("Expected workflow test to fail but it passed")
+            elif exc:
+                raise exc
 
     def _verify(self, run_summary: RunJobsSummary, output_definitions: OutputsDict):
         for output_name, output_definition in output_definitions.items():