From 4ae079a29dcf02cb331044f54701294c2cb8899c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 13 Oct 2024 10:46:59 -0400 Subject: [PATCH 01/12] tool input format test fixutre --- lib/galaxy_test/api/conftest.py | 6 ++++ lib/galaxy_test/api/test_tool_execute.py | 23 +++++---------- lib/galaxy_test/base/populators.py | 37 ++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/lib/galaxy_test/api/conftest.py b/lib/galaxy_test/api/conftest.py index 5c0b36753995..5c36c5d03c77 100644 --- a/lib/galaxy_test/api/conftest.py +++ b/lib/galaxy_test/api/conftest.py @@ -25,6 +25,7 @@ check_missing_tool, DatasetCollectionPopulator, DatasetPopulator, + DescribeToolInputs, get_tool_ids, RequiredTool, TargetHistory, @@ -136,6 +137,11 @@ def required_tool(dataset_populator: DatasetPopulator, history_id: str, required return tool +@pytest.fixture(params=["legacy", "21.01"]) +def tool_input_format(request) -> Iterator[DescribeToolInputs]: + yield DescribeToolInputs(request.param) + + @pytest.fixture(autouse=True) def check_required_tools(anonymous_galaxy_interactor, request): for marker in request.node.iter_markers(): diff --git a/lib/galaxy_test/api/test_tool_execute.py b/lib/galaxy_test/api/test_tool_execute.py index b77ebcbb0cd1..1494d58990a4 100644 --- a/lib/galaxy_test/api/test_tool_execute.py +++ b/lib/galaxy_test/api/test_tool_execute.py @@ -9,6 +9,7 @@ from galaxy_test.base.decorators import requires_tool_id from galaxy_test.base.populators import ( + DescribeToolInputs, RequiredTool, TargetHistory, ) @@ -80,24 +81,13 @@ def test_identifier_in_multiple_reduce(target_history: TargetHistory, required_t @requires_tool_id("identifier_in_conditional") -def test_identifier_map_over_multiple_input_in_conditional_legacy_format( - target_history: TargetHistory, required_tool: RequiredTool -): - hdca = target_history.with_pair() - execute = required_tool.execute.with_inputs( - { - "outer_cond|input1": hdca.src_dict, - } - ) - execute.assert_has_single_job.assert_has_single_output.with_contents_stripped("forward\nreverse") - - -@requires_tool_id("identifier_in_conditional") -def test_identifier_map_over_multiple_input_in_conditional_21_01_format( - target_history: TargetHistory, required_tool: RequiredTool +def test_identifier_map_over_multiple_input_in_conditional( + target_history: TargetHistory, required_tool: RequiredTool, tool_input_format: DescribeToolInputs ): hdca = target_history.with_pair() - execute = required_tool.execute.with_nested_inputs( + inputs = tool_input_format.when.flat({ + "outer_cond|input1": hdca.src_dict, + }).when.nested( { "outer_cond": { "multi_input": True, @@ -105,6 +95,7 @@ def test_identifier_map_over_multiple_input_in_conditional_21_01_format( }, } ) + execute = required_tool.execute.with_inputs(inputs) execute.assert_has_single_job.assert_has_single_output.with_contents_stripped("forward\nreverse") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index e2c9e1544971..6c904f8b7dd0 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -3603,6 +3603,34 @@ def execute(self) -> "DescribeToolExecution": return execution +class DescribeToolInputs: + _input_format: str = "legacy" + _inputs: Optional[Dict[str, Any]] + + def __init__(self, input_format: str): + self._input_format = input_format + self._inputs = None + + def any(self, inputs: Dict[str, Any]) -> Self: + self._inputs = inputs + return self + + def flat(self, inputs: Dict[str, Any]) -> Self: + if self._input_format == "legacy": + self._inputs = inputs + return self + + def nested(self, inputs: Dict[str, Any]) -> Self: + if self._input_format == "21.01": + self._inputs = inputs + return self + + # aliases for self to create silly little English sentense... inputs.when.flat().when.legacy() + @property + def when(self) -> Self: + return self + + class DescribeToolExecution: _history_id: Optional[str] = None _execute_response: Optional[Response] = None @@ -3621,8 +3649,13 @@ def in_history(self, has_history_id: Union[str, "TargetHistory"]) -> Self: self._history_id = has_history_id._history_id return self - def with_inputs(self, inputs: Dict[str, Any]) -> Self: - self._inputs = inputs + def with_inputs(self, inputs: Union[DescribeToolInputs, Dict[str, Any]]) -> Self: + if isinstance(inputs, DescribeToolInputs): + self._inputs = inputs._inputs or {} + self._input_format = inputs._input_format + else: + self._inputs = inputs + self._input_format = "legacy" return self def with_nested_inputs(self, inputs: Dict[str, Any]) -> Self: From 64c9832ba1ed96f817467af4394ac19bf11dd62e Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 4 Oct 2024 10:12:00 -0400 Subject: [PATCH 02/12] More tool tests. --- lib/galaxy_test/api/conftest.py | 11 ++ lib/galaxy_test/api/test_tool_execute.py | 107 +++++++++++++++++- lib/galaxy_test/api/test_tools.py | 51 --------- lib/galaxy_test/base/populators.py | 32 +++++- .../tools/parameters/gx_drill_down_code.py | 24 ++++ .../tools/parameters/gx_drill_down_code.xml | 38 +++++++ .../tools/parameters/gx_select_dynamic.xml | 36 ++++++ .../parameters/gx_select_dynamic_empty.py | 8 ++ .../parameters/gx_select_dynamic_empty.xml | 15 +++ .../gx_select_dynamic_empty_validated.xml | 16 +++ .../parameters/gx_select_dynamic_options.py | 10 ++ .../gx_select_no_options_validation.xml | 20 ++++ ..._select_optional_no_options_validation.xml | 20 ++++ test/functional/tools/parameters/gx_text.xml | 7 +- .../parameters/gx_text_empty_validation.xml | 20 ++++ .../tools/parameters/gx_text_optional.xml | 7 +- .../parameters/gx_text_optional_false.xml | 18 +++ 17 files changed, 381 insertions(+), 59 deletions(-) create mode 100644 test/functional/tools/parameters/gx_drill_down_code.py create mode 100644 test/functional/tools/parameters/gx_drill_down_code.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic_empty.py create mode 100644 test/functional/tools/parameters/gx_select_dynamic_empty.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic_options.py create mode 100644 test/functional/tools/parameters/gx_select_no_options_validation.xml create mode 100644 test/functional/tools/parameters/gx_select_optional_no_options_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_empty_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_optional_false.xml diff --git a/lib/galaxy_test/api/conftest.py b/lib/galaxy_test/api/conftest.py index 5c36c5d03c77..a011532c7af5 100644 --- a/lib/galaxy_test/api/conftest.py +++ b/lib/galaxy_test/api/conftest.py @@ -128,6 +128,17 @@ def target_history( return TargetHistory(dataset_populator, dataset_collection_populator, history_id) +@pytest.fixture +def required_tools( + dataset_populator: DatasetPopulator, history_id: str, required_tool_ids: List[str] +) -> List[RequiredTool]: + tools = [] + for tool_id in required_tool_ids: + tool = RequiredTool(dataset_populator, tool_id, history_id) + tools.append(tool) + return tools + + @pytest.fixture def required_tool(dataset_populator: DatasetPopulator, history_id: str, required_tool_ids: List[str]) -> RequiredTool: if len(required_tool_ids) != 1: diff --git a/lib/galaxy_test/api/test_tool_execute.py b/lib/galaxy_test/api/test_tool_execute.py index 1494d58990a4..3d9c60f0ef79 100644 --- a/lib/galaxy_test/api/test_tool_execute.py +++ b/lib/galaxy_test/api/test_tool_execute.py @@ -7,6 +7,8 @@ files, etc..). """ +from typing import List + from galaxy_test.base.decorators import requires_tool_id from galaxy_test.base.populators import ( DescribeToolInputs, @@ -85,9 +87,11 @@ def test_identifier_map_over_multiple_input_in_conditional( target_history: TargetHistory, required_tool: RequiredTool, tool_input_format: DescribeToolInputs ): hdca = target_history.with_pair() - inputs = tool_input_format.when.flat({ - "outer_cond|input1": hdca.src_dict, - }).when.nested( + inputs = tool_input_format.when.flat( + { + "outer_cond|input1": hdca.src_dict, + } + ).when.nested( { "outer_cond": { "multi_input": True, @@ -209,3 +213,100 @@ def test_optional_repeats_with_mins_filled_id(target_history: TargetHistory, req # tool test framework filling in a default. Creating a raw request here # verifies that currently select parameters don't require a selection. required_tool.execute.assert_has_single_job.with_single_output.containing("false").containing("length: 2") + + +@requires_tool_id("gx_select") +@requires_tool_id("gx_select_no_options_validation") +def test_select_first_by_default(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + empty = tool_input_format.when.any({}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_has_single_job.with_output("output").with_contents_stripped( + "--ex1" + ) + + +@requires_tool_id("gx_select") +@requires_tool_id("gx_select_no_options_validation") +@requires_tool_id("gx_select_dynamic_empty") +@requires_tool_id("gx_select_dynamic_empty_validated") +def test_select_on_null_errors(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + # test_select_first_by_default verifies the first option will just be selected, despite that if an explicit null + # is passed, an error (rightfully) occurs. This test verifies that. + null_parameter = tool_input_format.when.any({"parameter": None}) + for required_tool in required_tools: + required_tool.execute.with_inputs(null_parameter).assert_fails.with_error_containing("an invalid option") + + +@requires_tool_id("gx_select_dynamic_empty") +@requires_tool_id("gx_select_dynamic_empty_validated") +def test_select_empty_causes_error_regardless( + required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs +): + # despite selects otherwise selecting defaults - nothing can be done if the select option list is empty + empty = tool_input_format.when.any({}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_fails.with_error_containing("an invalid option") + + +@requires_tool_id("gx_select_optional") +@requires_tool_id("gx_select_optional_no_options_validation") +def test_select_optional_null_by_default(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + # test_select_first_by_default shows that required select values will pick an option by default, + # this test verify that doesn't occur for optional selects. + empty = tool_input_format.when.any({}) + null_parameter = tool_input_format.when.any({"parameter": None}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_has_single_job.with_output("output").with_contents_stripped( + "None" + ) + required_tool.execute.with_inputs(null_parameter).assert_has_single_job.with_output( + "output" + ).with_contents_stripped("None") + + +@requires_tool_id("gx_select_multiple") +@requires_tool_id("gx_select_multiple_optional") +def test_select_multiple_does_not_select_first_by_default( + required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs +): + # unlike single selects - no selection is forced and these serve as optional by default + empty = tool_input_format.when.any({}) + null_parameter = tool_input_format.when.any({"parameter": None}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_has_single_job.with_output("output").with_contents_stripped( + "None" + ) + required_tool.execute.with_inputs(null_parameter).assert_has_single_job.with_output( + "output" + ).with_contents_stripped("None") + + +@requires_tool_id("gx_text") +@requires_tool_id("gx_text_optional_false") +def test_null_to_text_tools(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + for required_tool in required_tools: + execute = required_tool.execute.with_inputs(tool_input_format.when.any({})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": ""}) + + execute = required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": None})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": ""}) + + +@requires_tool_id("gx_text_optional") +def test_null_to_optinal_text_tool(required_tool: RequiredTool, tool_input_format: DescribeToolInputs): + execute = required_tool.execute.with_inputs(tool_input_format.when.any({})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": None}) + + execute = required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": None})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": None}) + + +@requires_tool_id("gx_text_empty_validation") +def test_null_to_text_tool_with_validation(required_tool: RequiredTool, tool_input_format: DescribeToolInputs): + required_tool.execute.with_inputs(tool_input_format.when.any({})).assert_fails() + required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": None})).assert_fails() + required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": ""})).assert_fails() diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 46c6cdad3473..12c11f3a634e 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -879,57 +879,6 @@ def test_dataset_hidden_after_job_finish(self): output_details = self.dataset_populator.get_history_dataset_details(history_id, dataset=output, wait=True) assert not output_details["visible"] - @skip_without_tool("gx_select") - def test_select_first_by_default(self): - # we have a tool test for this but I wanted to verify it wasn't just the - # tool test framework filling in a default. Creating a raw request here - # verifies that currently select parameters don't require a selection. - with self.dataset_populator.test_history(require_new=False) as history_id: - inputs: Dict[str, Any] = {} - response = self._run("gx_select", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "--ex1" - - inputs = { - "parameter": None, - } - response = self._run("gx_select", history_id, inputs, assert_ok=False) - self._assert_status_code_is(response, 400) - assert "an invalid option" in response.text - - @skip_without_tool("gx_select_multiple") - @skip_without_tool("gx_select_multiple_optional") - def test_select_multiple_null_handling(self): - with self.dataset_populator.test_history(require_new=False) as history_id: - inputs: Dict[str, Any] = {} - response = self._run("gx_select_multiple", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - - inputs = {} - response = self._run("gx_select_multiple_optional", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - - inputs = { - "parameter": None, - } - response = self._run("gx_select_multiple", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - - inputs = { - "parameter": None, - } - response = self._run("gx_select_multiple_optional", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - @skip_without_tool("gx_drill_down_exact") @skip_without_tool("gx_drill_down_exact_multiple") @skip_without_tool("gx_drill_down_recurse") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 6c904f8b7dd0..698ceeff7966 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -3455,6 +3455,17 @@ def with_file_ext(self, expected_ext: str) -> Self: raise AssertionError(f"Output dataset had file extension {ext}, not the expected extension {expected_ext}") return self + @property + def json(self) -> Any: + contents = self.contents + return json.loads(contents) + + def with_json(self, expected_json: Any) -> Self: + json = self.json + if json != expected_json: + raise AssertionError(f"Output dataset contianed JSON {json}, not {expected_json} as expected") + return self + # aliases that might help make tests more like English in particular cases. Declaring them explicitly # instead quick little aliases because of https://github.com/python/mypy/issues/6700 def assert_contains(self, expected_contents: str) -> Self: @@ -3579,6 +3590,9 @@ class DescribeFailure: def __init__(self, response: Response): self._response = response + def __call__(self) -> Self: + return self + def with_status_code(self, code: int) -> Self: api_asserts.assert_status_code_is(self._response, code) return self @@ -3722,12 +3736,24 @@ def assert_has_job(self, job_index: int = 0) -> DescribeJob: return DescribeJob(self._dataset_populator, history_id, job["id"]) @property - def assert_fails(self) -> DescribeFailure: + def that_fails(self) -> DescribeFailure: self._ensure_executed() execute_response = self._execute_response assert execute_response is not None - api_asserts.assert_status_code_is_not_ok(execute_response) - return DescribeFailure(execute_response) + if execute_response.status_code != 200: + return DescribeFailure(execute_response) + else: + response = self._assert_executed_ok() + jobs = response["jobs"] + for job in jobs: + final_state = self._dataset_populator.wait_for_job(job["id"]) + assert final_state == "error" + return DescribeFailure(execute_response) + + # alternative assert_ syntax for cases where it reads better. + @property + def assert_fails(self) -> DescribeFailure: + return self.that_fails class GiHttpMixin: diff --git a/test/functional/tools/parameters/gx_drill_down_code.py b/test/functional/tools/parameters/gx_drill_down_code.py new file mode 100644 index 000000000000..518cb67c6a7e --- /dev/null +++ b/test/functional/tools/parameters/gx_drill_down_code.py @@ -0,0 +1,24 @@ +def collate_table(path: str) -> list: + with open(path, "r") as f: + contents = f.read() + first_options = [] + second_options = [] + values = [ + {"name": "First Column", "value": "first", "selected": False, "options": first_options}, + {"name": "Second Column", "value": "second", "selected": False, "options": second_options}, + ] + seen_in_column_1 = set() + seen_in_column_2 = set() + for line in contents.splitlines(): + parts = line.split("\t") + if len(parts) >= 1: + val = parts[0] + if val not in seen_in_column_1: + first_options.append({"name": val.upper(), "value": val, "selected": False, "options": []}) + seen_in_column_1.add(val) + if len(parts) >= 2: + val = parts[1] + if val not in seen_in_column_2: + second_options.append({"name": val.upper(), "value": val, "selected": False, "options": []}) + seen_in_column_2.add(val) + return values diff --git a/test/functional/tools/parameters/gx_drill_down_code.xml b/test/functional/tools/parameters/gx_drill_down_code.xml new file mode 100644 index 000000000000..42ae442574e4 --- /dev/null +++ b/test/functional/tools/parameters/gx_drill_down_code.xml @@ -0,0 +1,38 @@ + + + macros.xml + + + '$output' + ]]> + + + +g + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic.xml b/test/functional/tools/parameters/gx_select_dynamic.xml new file mode 100644 index 000000000000..8959f3d0e3d4 --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic.xml @@ -0,0 +1,36 @@ + + > '$output' + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic_empty.py b/test/functional/tools/parameters/gx_select_dynamic_empty.py new file mode 100644 index 000000000000..6a79a047f3cb --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_empty.py @@ -0,0 +1,8 @@ +from typing import ( + List, + Tuple, +) + + +def empty_list() -> List[Tuple[str, str, bool]]: + return [] diff --git a/test/functional/tools/parameters/gx_select_dynamic_empty.xml b/test/functional/tools/parameters/gx_select_dynamic_empty.xml new file mode 100644 index 000000000000..7554604033fd --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_empty.xml @@ -0,0 +1,15 @@ + + > '$output' + ]]> + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml b/test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml new file mode 100644 index 000000000000..39ffa263174e --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml @@ -0,0 +1,16 @@ + + > '$output' + ]]> + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic_options.py b/test/functional/tools/parameters/gx_select_dynamic_options.py new file mode 100644 index 000000000000..734174b6da1d --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_options.py @@ -0,0 +1,10 @@ +from typing import ( + List, + Tuple, +) + + +def every_other_word(path: str) -> List[Tuple[str, str, bool]]: + with open(path, "r") as f: + contents = f.read() + return [(r.strip(), r.strip(), False) for (i, r) in enumerate(contents.split()) if i % 2 == 0] diff --git a/test/functional/tools/parameters/gx_select_no_options_validation.xml b/test/functional/tools/parameters/gx_select_no_options_validation.xml new file mode 100644 index 000000000000..217b95b53639 --- /dev/null +++ b/test/functional/tools/parameters/gx_select_no_options_validation.xml @@ -0,0 +1,20 @@ + + > '$output' + ]]> + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_optional_no_options_validation.xml b/test/functional/tools/parameters/gx_select_optional_no_options_validation.xml new file mode 100644 index 000000000000..6e3e78de0400 --- /dev/null +++ b/test/functional/tools/parameters/gx_select_optional_no_options_validation.xml @@ -0,0 +1,20 @@ + + > '$output' + ]]> + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text.xml b/test/functional/tools/parameters/gx_text.xml index 16707f63e878..3b9e2e00dfbc 100644 --- a/test/functional/tools/parameters/gx_text.xml +++ b/test/functional/tools/parameters/gx_text.xml @@ -1,12 +1,17 @@ > '$output' +echo '$parameter' >> '$output'; +cat '$inputs' >> $inputs_json; ]]> + + + + diff --git a/test/functional/tools/parameters/gx_text_empty_validation.xml b/test/functional/tools/parameters/gx_text_empty_validation.xml new file mode 100644 index 000000000000..76374be72ded --- /dev/null +++ b/test/functional/tools/parameters/gx_text_empty_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_optional.xml b/test/functional/tools/parameters/gx_text_optional.xml index 41fe11cea418..ae10d3278376 100644 --- a/test/functional/tools/parameters/gx_text_optional.xml +++ b/test/functional/tools/parameters/gx_text_optional.xml @@ -1,12 +1,17 @@ > '$output' +echo '$parameter' >> '$output'; +cat '$inputs' >> $inputs_json; ]]> + + + + diff --git a/test/functional/tools/parameters/gx_text_optional_false.xml b/test/functional/tools/parameters/gx_text_optional_false.xml new file mode 100644 index 000000000000..9e29d53fc4ca --- /dev/null +++ b/test/functional/tools/parameters/gx_text_optional_false.xml @@ -0,0 +1,18 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + From b9d52287050b5edb614b46499f4eb394e8b69ef0 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 1 Oct 2024 08:45:13 -0400 Subject: [PATCH 03/12] Refactor Tool._populate for reuse of hook checks. --- lib/galaxy/tools/__init__.py | 22 ++++---- lib/galaxy/tools/parameters/__init__.py | 47 ---------------- test/unit/app/tools/test_populate_state.py | 63 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 56 deletions(-) create mode 100644 test/unit/app/tools/test_populate_state.py diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index a7b6c96bc1f3..171edd8ad661 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1900,17 +1900,21 @@ def _populate( simple_errors=False, input_format=input_format, ) - # If the tool provides a `validate_input` hook, call it. - validate_input = self.get_hook("validate_input") - if validate_input: - # hooks are so terrible ... this is specifically for https://github.com/galaxyproject/tools-devteam/blob/main/tool_collections/gops/basecoverage/operation_filter.py - legacy_non_dce_params = { - k: v.hda if isinstance(v, model.DatasetCollectionElement) and v.hda else v - for k, v in params.items() - } - validate_input(request_context, errors, legacy_non_dce_params, self.inputs) + self._handle_validate_input_hook(request_context, params, errors) return params, errors + def _handle_validate_input_hook( + self, request_context, params: ToolStateJobInstancePopulatedT, errors: ParameterValidationErrorsT + ): + # If the tool provides a `validate_input` hook, call it. + validate_input = self.get_hook("validate_input") + if validate_input: + # hooks are so terrible ... this is specifically for https://github.com/galaxyproject/tools-devteam/blob/main/tool_collections/gops/basecoverage/operation_filter.py + legacy_non_dce_params = { + k: v.hda if isinstance(v, model.DatasetCollectionElement) and v.hda else v for k, v in params.items() + } + validate_input(request_context, errors, legacy_non_dce_params, self.inputs) + def completed_jobs( self, trans, use_cached_job: bool, all_params: List[ToolStateJobInstancePopulatedT] ) -> Dict[int, Optional[model.Job]]: diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 25d6775446f9..9de78f2fbff5 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -389,53 +389,6 @@ def populate_state( ): """ Populates nested state dict from incoming parameter values. - >>> from galaxy.util import XML - >>> from galaxy.util.bunch import Bunch - >>> from galaxy.tools.parameters.basic import TextToolParameter, BooleanToolParameter - >>> from galaxy.tools.parameters.grouping import Repeat - >>> trans = Bunch(workflow_building_mode=False) - >>> a = TextToolParameter(None, XML('')) - >>> b = Repeat('b') - >>> b.min = 0 - >>> b.max = 1 - >>> c = TextToolParameter(None, XML('')) - >>> d = Repeat('d') - >>> d.min = 0 - >>> d.max = 1 - >>> e = TextToolParameter(None, XML('')) - >>> f = Conditional('f') - >>> g = BooleanToolParameter(None, XML('')) - >>> h = TextToolParameter(None, XML('')) - >>> i = TextToolParameter(None, XML('')) - >>> b.inputs = dict([('c', c), ('d', d)]) - >>> d.inputs = dict([('e', e), ('f', f)]) - >>> f.test_param = g - >>> f.cases = [Bunch(value='true', inputs= { 'h': h }), Bunch(value='false', inputs= { 'i': i })] - >>> inputs = dict([('a',a),('b',b)]) - >>> flat = dict([('a', 1), ('b_0|c', 2), ('b_0|d_0|e', 3), ('b_0|d_0|f|h', 4), ('b_0|d_0|f|g', True)]) - >>> state = {} - >>> populate_state(trans, inputs, flat, state, check=False) - >>> print(state['a']) - 1 - >>> print(state['b'][0]['c']) - 2 - >>> print(state['b'][0]['d'][0]['e']) - 3 - >>> print(state['b'][0]['d'][0]['f']['h']) - 4 - >>> # now test with input_format='21.01' - >>> nested = {'a': 1, 'b': [{'c': 2, 'd': [{'e': 3, 'f': {'h': 4, 'g': True}}]}]} - >>> state_new = {} - >>> populate_state(trans, inputs, nested, state_new, check=False, input_format='21.01') - >>> print(state_new['a']) - 1 - >>> print(state_new['b'][0]['c']) - 2 - >>> print(state_new['b'][0]['d'][0]['e']) - 3 - >>> print(state_new['b'][0]['d'][0]['f']['h']) - 4 - """ if errors is None: errors = {} diff --git a/test/unit/app/tools/test_populate_state.py b/test/unit/app/tools/test_populate_state.py new file mode 100644 index 000000000000..d641ff079397 --- /dev/null +++ b/test/unit/app/tools/test_populate_state.py @@ -0,0 +1,63 @@ +from typing import ( + Any, + cast, + Dict, +) + +from galaxy.tools.parameters import ( + populate_state, + ToolInputsT, +) +from galaxy.tools.parameters.basic import ( + BooleanToolParameter, + TextToolParameter, +) +from galaxy.tools.parameters.grouping import ( + Conditional, + ConditionalWhen, + Repeat, +) +from galaxy.util import XML +from galaxy.util.bunch import Bunch + +trans = Bunch(workflow_building_mode=False) + + +def mock_when(**kwd): + return cast(ConditionalWhen, Bunch(**kwd)) + + +def test_populate_state(): + a = TextToolParameter(None, XML('')) + b = Repeat("b") + b.min = 0 + b.max = 1 + c = TextToolParameter(None, XML('')) + d = Repeat("d") + d.min = 0 + d.max = 1 + e = TextToolParameter(None, XML('')) + f = Conditional("f") + g = BooleanToolParameter(None, XML('')) + h = TextToolParameter(None, XML('')) + i = TextToolParameter(None, XML('')) + b.inputs = {"c": c, "d": d} + d.inputs = {"e": e, "f": f} + f.test_param = g + f.cases = [mock_when(value="true", inputs={"h": h}), mock_when(value="false", inputs={"i": i})] + inputs = {"a": a, "b": b} + flat = {"a": 1, "b_0|c": 2, "b_0|d_0|e": 3, "b_0|d_0|f|h": 4, "b_0|d_0|f|g": True} + state: Dict[str, Any] = {} + populate_state(trans, cast(ToolInputsT, inputs), flat, state, check=False) + assert state["a"] == 1 + assert state["b"][0]["c"] == 2 + assert state["b"][0]["d"][0]["e"] == 3 + assert state["b"][0]["d"][0]["f"]["h"] == 4 + # now test with input_format='21.01' + nested = {"a": 1, "b": [{"c": 2, "d": [{"e": 3, "f": {"h": 4, "g": True}}]}]} + state_new: Dict[str, Any] = {} + populate_state(trans, cast(ToolInputsT, inputs), nested, state_new, check=False, input_format="21.01") + assert state_new["a"] == 1 + assert state_new["b"][0]["c"] == 2 + assert state_new["b"][0]["d"][0]["e"] == 3 + assert state_new["b"][0]["d"][0]["f"]["h"] == 4 From 8a3dde76a850e94198f0d8b926c26451271a2dd1 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 27 Sep 2024 10:50:03 -0400 Subject: [PATCH 04/12] Spelling error. --- test/unit/tool_util/parameter_specification.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml index 761a1c3fb680..2eb07ada9f38 100644 --- a/test/unit/tool_util/parameter_specification.yml +++ b/test/unit/tool_util/parameter_specification.yml @@ -445,7 +445,7 @@ gx_hidden: - parameter: moocow - {} workflow_step_invalid: - - parmaeter: 5 + - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } workflow_step_linked_valid: @@ -470,7 +470,7 @@ gx_hidden_optional: - {} - parameter: null workflow_step_invalid: - - parmaeter: 5 + - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } workflow_step_linked_valid: From a8727e294013f50fa492fcf56add8baaeb9f254d Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 27 Sep 2024 10:52:34 -0400 Subject: [PATCH 05/12] Refactor for reuse in galaxy.tool_util.parameters. --- lib/galaxy/tool_util/parameters/__init__.py | 4 + lib/galaxy/tool_util/parameters/models.py | 80 ++++----- .../tool_util/test_parameter_specification.py | 170 ++++-------------- 3 files changed, 72 insertions(+), 182 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/__init__.py b/lib/galaxy/tool_util/parameters/__init__.py index 4a54ba0a8373..8df9db0c1f16 100644 --- a/lib/galaxy/tool_util/parameters/__init__.py +++ b/lib/galaxy/tool_util/parameters/__init__.py @@ -39,6 +39,7 @@ HiddenParameterModel, IntegerParameterModel, LabelValue, + RawStateDict, RepeatParameterModel, RulesParameterModel, SelectParameterModel, @@ -55,6 +56,7 @@ validate_test_case, validate_workflow_step, validate_workflow_step_linked, + ValidationFunctionT, ) from .state import ( JobInternalToolState, @@ -113,6 +115,8 @@ "ConditionalParameterModel", "ConditionalWhen", "RepeatParameterModel", + "RawStateDict", + "ValidationFunctionT", "validate_against_model", "validate_internal_job", "validate_internal_request", diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py index 11957c37099a..329423793d57 100644 --- a/lib/galaxy/tool_util/parameters/models.py +++ b/lib/galaxy/tool_util/parameters/models.py @@ -70,6 +70,9 @@ "workflow_step_linked", ] +DEFAULT_MODEL_NAME = "DynamicModelForTool" +RawStateDict = Dict[str, Any] + # could be made more specific - validators need to be classmethod ValidatorDictT = Dict[str, Callable] @@ -1368,34 +1371,21 @@ def create_model_strict(*args, **kwd) -> Type[BaseModel]: return create_model(*args, __config__=model_config, **kwd) -def create_request_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "request") - - -def create_request_internal_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "request_internal") - - -def create_request_internal_dereferenced_model( - tool: ToolParameterBundle, name: str = "DynamicModelForTool" -) -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "request_internal_dereferenced") - - -def create_job_internal_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "job_internal") +def create_model_factory(state_representation: StateRepresentationT): + def create_method(tool: ToolParameterBundle, name: str = DEFAULT_MODEL_NAME) -> Type[BaseModel]: + return create_field_model(tool.parameters, name, state_representation) -def create_test_case_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "test_case_xml") + return create_method -def create_workflow_step_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "workflow_step") - - -def create_workflow_step_linked_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "workflow_step_linked") +create_request_model = create_model_factory("request") +create_request_internal_model = create_model_factory("request_internal") +create_request_internal_dereferenced_model = create_model_factory("request_internal_dereferenced") +create_job_internal_model = create_model_factory("job_internal") +create_test_case_model = create_model_factory("test_case_xml") +create_workflow_step_model = create_model_factory("workflow_step") +create_workflow_step_linked_model = create_model_factory("workflow_step_linked") def create_field_model( @@ -1432,36 +1422,26 @@ def validate_against_model(pydantic_model: Type[BaseModel], parameter_state: Dic raise RequestParameterInvalidException(str(e)) -def validate_request(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_request_model(tool) - validate_against_model(pydantic_model, request) - - -def validate_internal_request(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_request_internal_model(tool) - validate_against_model(pydantic_model, request) +class ValidationFunctionT(Protocol): + def __call__(self, tool: ToolParameterBundle, request: RawStateDict, name: str = DEFAULT_MODEL_NAME) -> None: ... -def validate_internal_request_dereferenced(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_request_internal_dereferenced_model(tool) - validate_against_model(pydantic_model, request) +def validate_model_type_factory(state_representation: StateRepresentationT) -> ValidationFunctionT: -def validate_internal_job(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_job_internal_model(tool) - validate_against_model(pydantic_model, request) - - -def validate_test_case(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_test_case_model(tool) - validate_against_model(pydantic_model, request) - + def validate_request(tool: ToolParameterBundle, request: Dict[str, Any], name: str = DEFAULT_MODEL_NAME) -> None: + pydantic_model = create_field_model( + tool.parameters, name=DEFAULT_MODEL_NAME, state_representation=state_representation + ) + validate_against_model(pydantic_model, request) -def validate_workflow_step(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_workflow_step_model(tool) - validate_against_model(pydantic_model, request) + return validate_request -def validate_workflow_step_linked(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_workflow_step_linked_model(tool) - validate_against_model(pydantic_model, request) +validate_request = validate_model_type_factory("request") +validate_internal_request = validate_model_type_factory("request_internal") +validate_internal_request_dereferenced = validate_model_type_factory("request_internal_dereferenced") +validate_internal_job = validate_model_type_factory("job_internal") +validate_test_case = validate_model_type_factory("test_case_xml") +validate_workflow_step = validate_model_type_factory("workflow_step") +validate_workflow_step_linked = validate_model_type_factory("workflow_step_linked") diff --git a/test/unit/tool_util/test_parameter_specification.py b/test/unit/tool_util/test_parameter_specification.py index dd54ac7276be..012213a02499 100644 --- a/test/unit/tool_util/test_parameter_specification.py +++ b/test/unit/tool_util/test_parameter_specification.py @@ -1,9 +1,7 @@ import sys from functools import partial from typing import ( - Any, Callable, - Dict, List, Optional, ) @@ -15,6 +13,7 @@ from galaxy.tool_util.parameters import ( decode, encode, + RawStateDict, RequestInternalToolState, RequestToolState, ToolParameterBundleModel, @@ -25,6 +24,7 @@ validate_test_case, validate_workflow_step, validate_workflow_step_linked, + ValidationFunctionT, ) from galaxy.tool_util.parameters.json import to_json_schema_string from galaxy.tool_util.unittest_utils.parameters import ( @@ -33,9 +33,6 @@ ) from galaxy.util.resources import resource_string -RawStateDict = Dict[str, Any] - - if sys.version_info < (3, 8): # noqa: UP036 pytest.skip(reason="Pydantic tool parameter models require python3.8 or higher", allow_module_level=True) @@ -126,139 +123,48 @@ def _for_each(test: Callable, parameters: ToolParameterBundleModel, requests: Li test(parameters, request) -def _assert_request_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - try: - validate_request(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate request {request}. {e}") - - -def _assert_request_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_request(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on request {request} as expected." - - -def _assert_internal_request_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - try: - validate_internal_request(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate internal request {request}. {e}") - - -def _assert_internal_request_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_internal_request(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on internal request {request} as expected." - - -def _assert_internal_request_dereferenced_validates( - parameters: ToolParameterBundleModel, request: RawStateDict -) -> None: - try: - validate_internal_request_dereferenced(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate dereferenced internal request {request}. {e}") +def model_assertion_function_factory(validate_function: ValidationFunctionT, what: str): + def _assert_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: + try: + validate_function(parameters, request) + except RequestParameterInvalidException as e: + raise AssertionError(f"Parameters {parameters} failed to validate {what} {request}. {e}") -def _assert_internal_request_dereferenced_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_internal_request_dereferenced(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on dereferenced internal request {request} as expected." + def _assert_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: + exc = None + try: + validate_function(parameters, request) + except RequestParameterInvalidException as e: + exc = e + if exc is None: + raise AssertionError( + f"Parameters {parameters} didn't result in validation error on {what} {request} as expected." + ) -def _assert_internal_job_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - try: - validate_internal_job(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate internal job description {request}. {e}") + return _assert_validates, _assert_invalid -def _assert_internal_job_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_internal_job(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on internal job description {request} as expected." - - -def _assert_test_case_validates(parameters: ToolParameterBundleModel, test_case: RawStateDict) -> None: - try: - validate_test_case(parameters, test_case) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate test_case {test_case}. {e}") - - -def _assert_test_case_invalid(parameters: ToolParameterBundleModel, test_case: RawStateDict) -> None: - exc = None - try: - validate_test_case(parameters, test_case) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on test_case {test_case} as expected." - - -def _assert_workflow_step_validates(parameters: ToolParameterBundleModel, workflow_step: RawStateDict) -> None: - try: - validate_workflow_step(parameters, workflow_step) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate workflow step {workflow_step}. {e}") - - -def _assert_workflow_step_invalid(parameters: ToolParameterBundleModel, workflow_step: RawStateDict) -> None: - exc = None - try: - validate_workflow_step(parameters, workflow_step) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on workflow step {workflow_step} as expected." - - -def _assert_workflow_step_linked_validates( - parameters: ToolParameterBundleModel, workflow_step_linked: RawStateDict -) -> None: - try: - validate_workflow_step_linked(parameters, workflow_step_linked) - except RequestParameterInvalidException as e: - raise AssertionError( - f"Parameters {parameters} failed to validate linked workflow step {workflow_step_linked}. {e}" - ) - - -def _assert_workflow_step_linked_invalid( - parameters: ToolParameterBundleModel, workflow_step_linked: RawStateDict -) -> None: - exc = None - try: - validate_workflow_step_linked(parameters, workflow_step_linked) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on linked workflow step {workflow_step_linked} as expected." - +_assert_request_validates, _assert_request_invalid = model_assertion_function_factory(validate_request, "request") +_assert_internal_request_validates, _assert_internal_request_invalid = model_assertion_function_factory( + validate_internal_request, "internal request" +) +_assert_internal_request_dereferenced_validates, _assert_internal_request_dereferenced_invalid = ( + model_assertion_function_factory(validate_internal_request_dereferenced, "dereferenced internal request") +) +_assert_internal_job_validates, _assert_internal_job_invalid = model_assertion_function_factory( + validate_internal_job, "internal job description" +) +_assert_test_case_validates, _assert_test_case_invalid = model_assertion_function_factory( + validate_test_case, "XML derived test case" +) +_assert_workflow_step_validates, _assert_workflow_step_invalid = model_assertion_function_factory( + validate_workflow_step, "workflow step tool state (unlinked)" +) +_assert_workflow_step_linked_validates, _assert_workflow_step_linked_invalid = model_assertion_function_factory( + validate_workflow_step_linked, "linked workflow step tool state" +) _assert_requests_validate = partial(_for_each, _assert_request_validates) _assert_requests_invalid = partial(_for_each, _assert_request_invalid) From 0ebabc66a2b58324f82ce364846166519066df15 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 30 Sep 2024 14:49:25 -0400 Subject: [PATCH 06/12] Refactor encode/decode for reuse. --- lib/galaxy/tool_util/parameters/convert.py | 134 +++++++++++---------- 1 file changed, 73 insertions(+), 61 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index 9484cc6c7e71..bb1ebc911089 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -46,6 +46,7 @@ TestCaseToolState, ) from .visitor import ( + Callback, validate_explicit_conditional_test_value, visit_input_values, VISITOR_NO_REPLACEMENT, @@ -54,40 +55,22 @@ log = logging.getLogger(__name__) +DecodeFunctionT = Callable[[str], int] +EncodeFunctionT = Callable[[int], str] +DereferenceCallable = Callable[[DataRequestUri], DataRequestInternalHda] +# interfaces for adapting test data dictionaries to tool request dictionaries +# e.g. {class: File, path: foo.bed} => {src: hda, id: ab1235cdfea3} +AdaptDatasets = Callable[[JsonTestDatasetDefDict], DataRequestHda] +AdaptCollections = Callable[[JsonTestCollectionDefDict], DataCollectionRequest] + + def decode( external_state: RequestToolState, input_models: ToolParameterBundle, decode_id: Callable[[str], int] ) -> RequestInternalToolState: """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" external_state.validate(input_models) - - def decode_src_dict(src_dict: dict): - if "id" in src_dict: - decoded_dict = src_dict.copy() - decoded_dict["id"] = decode_id(src_dict["id"]) - return decoded_dict - else: - return src_dict - - def decode_callback(parameter: ToolParameterT, value: Any): - if parameter.parameter_type == "gx_data": - if value is None: - return VISITOR_NO_REPLACEMENT - data_parameter = cast(DataParameterModel, parameter) - if data_parameter.multiple: - assert isinstance(value, list), str(value) - return list(map(decode_src_dict, value)) - else: - assert isinstance(value, dict), str(value) - return decode_src_dict(value) - elif parameter.parameter_type == "gx_data_collection": - if value is None: - return VISITOR_NO_REPLACEMENT - assert isinstance(value, dict), str(value) - return decode_src_dict(value) - else: - return VISITOR_NO_REPLACEMENT - + decode_callback = _decode_callback_for(decode_id) internal_state_dict = visit_input_values( input_models, external_state, @@ -100,33 +83,11 @@ def decode_callback(parameter: ToolParameterT, value: Any): def encode( - external_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: Callable[[int], str] + external_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT ) -> RequestToolState: """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" - def encode_src_dict(src_dict: dict): - if "id" in src_dict: - encoded_dict = src_dict.copy() - encoded_dict["id"] = encode_id(src_dict["id"]) - return encoded_dict - else: - return src_dict - - def encode_callback(parameter: ToolParameterT, value: Any): - if parameter.parameter_type == "gx_data": - data_parameter = cast(DataParameterModel, parameter) - if data_parameter.multiple: - assert isinstance(value, list), str(value) - return list(map(encode_src_dict, value)) - else: - assert isinstance(value, dict), str(value) - return encode_src_dict(value) - elif parameter.parameter_type == "gx_data_collection": - assert isinstance(value, dict), str(value) - return encode_src_dict(value) - else: - return VISITOR_NO_REPLACEMENT - + encode_callback = _encode_callback_for(encode_id) request_state_dict = visit_input_values( input_models, external_state, @@ -137,9 +98,6 @@ def encode_callback(parameter: ToolParameterT, value: Any): return request_state -DereferenceCallable = Callable[[DataRequestUri], DataRequestInternalHda] - - def dereference( internal_state: RequestInternalToolState, input_models: ToolParameterBundle, dereference: DereferenceCallable ) -> RequestInternalDereferencedToolState: @@ -177,12 +135,6 @@ def dereference_callback(parameter: ToolParameterT, value: Any): return request_state -# interfaces for adapting test data dictionaries to tool request dictionaries -# e.g. {class: File, path: foo.bed} => {src: hda, id: ab1235cdfea3} -AdaptDatasets = Callable[[JsonTestDatasetDefDict], DataRequestHda] -AdaptCollections = Callable[[JsonTestCollectionDefDict], DataCollectionRequest] - - def encode_test( test_case_state: TestCaseToolState, input_models: ToolParameterBundle, @@ -358,3 +310,63 @@ def _select_which_when( raise Exception( f"Invalid conditional test value ({test_value}) for parameter ({conditional.test_parameter.name})" ) + + +def _encode_callback_for(encode_id: EncodeFunctionT) -> Callback: + + def encode_src_dict(src_dict: dict): + if "id" in src_dict: + encoded_dict = src_dict.copy() + encoded_dict["id"] = encode_id(src_dict["id"]) + return encoded_dict + else: + return src_dict + + def encode_callback(parameter: ToolParameterT, value: Any): + if parameter.parameter_type == "gx_data": + data_parameter = cast(DataParameterModel, parameter) + if data_parameter.multiple: + assert isinstance(value, list), str(value) + return list(map(encode_src_dict, value)) + else: + assert isinstance(value, dict), str(value) + return encode_src_dict(value) + elif parameter.parameter_type == "gx_data_collection": + assert isinstance(value, dict), str(value) + return encode_src_dict(value) + else: + return VISITOR_NO_REPLACEMENT + + return encode_callback + + +def _decode_callback_for(decode_id: DecodeFunctionT) -> Callback: + + def decode_src_dict(src_dict: dict): + if "id" in src_dict: + decoded_dict = src_dict.copy() + decoded_dict["id"] = decode_id(src_dict["id"]) + return decoded_dict + else: + return src_dict + + def decode_callback(parameter: ToolParameterT, value: Any): + if parameter.parameter_type == "gx_data": + if value is None: + return VISITOR_NO_REPLACEMENT + data_parameter = cast(DataParameterModel, parameter) + if data_parameter.multiple: + assert isinstance(value, list), str(value) + return list(map(decode_src_dict, value)) + else: + assert isinstance(value, dict), str(value) + return decode_src_dict(value) + elif parameter.parameter_type == "gx_data_collection": + if value is None: + return VISITOR_NO_REPLACEMENT + assert isinstance(value, dict), str(value) + return decode_src_dict(value) + else: + return VISITOR_NO_REPLACEMENT + + return decode_callback From 3e3664f9d56d1a125c378854f6a633358bbf21ca Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 1 Oct 2024 19:05:30 -0400 Subject: [PATCH 07/12] Use get_color_value in basic.py. --- lib/galaxy/tools/parameters/basic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 08ae06864732..d656665be21b 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -41,6 +41,7 @@ ) from galaxy.model.dataset_collections import builder from galaxy.schema.fetch_data import FilesPayload +from galaxy.tool_util.parameters.factory import get_color_value from galaxy.tool_util.parser import get_input_source as ensure_input_source from galaxy.tool_util.parser.util import ( boolean_is_checked, @@ -848,7 +849,7 @@ class ColorToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) - self.value = input_source.get("value", "#000000") + self.value = get_color_value(input_source) self.rgb = input_source.get_bool("rgb", False) def get_initial_value(self, trans, other_values): From d96ea1c8882c2efd064ff078f488b8a23869f267 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 27 Sep 2024 10:22:34 -0400 Subject: [PATCH 08/12] models for tool landing request state... --- lib/galaxy/tool_util/parameters/__init__.py | 12 + lib/galaxy/tool_util/parameters/convert.py | 44 ++- lib/galaxy/tool_util/parameters/models.py | 46 ++- lib/galaxy/tool_util/parameters/state.py | 18 + lib/galaxy/tools/parameters/basic.py | 13 +- lib/galaxy/tools/wrappers.py | 16 +- .../tool_util/parameter_specification.yml | 330 ++++++++++++++++++ test/unit/tool_util/test_parameter_convert.py | 17 + .../tool_util/test_parameter_specification.py | 16 + 9 files changed, 494 insertions(+), 18 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/__init__.py b/lib/galaxy/tool_util/parameters/__init__.py index 8df9db0c1f16..45cd770a5e3f 100644 --- a/lib/galaxy/tool_util/parameters/__init__.py +++ b/lib/galaxy/tool_util/parameters/__init__.py @@ -5,6 +5,8 @@ encode, encode_test, fill_static_defaults, + landing_decode, + landing_encode, ) from .factory import ( from_input_source, @@ -50,8 +52,10 @@ ToolParameterT, validate_against_model, validate_internal_job, + validate_internal_landing_request, validate_internal_request, validate_internal_request_dereferenced, + validate_landing_request, validate_request, validate_test_case, validate_workflow_step, @@ -60,6 +64,8 @@ ) from .state import ( JobInternalToolState, + LandingRequestInternalToolState, + LandingRequestToolState, RequestInternalDereferencedToolState, RequestInternalToolState, RequestToolState, @@ -119,8 +125,10 @@ "ValidationFunctionT", "validate_against_model", "validate_internal_job", + "validate_internal_landing_request", "validate_internal_request", "validate_internal_request_dereferenced", + "validate_landing_request", "validate_request", "validate_test_case", "validate_workflow_step", @@ -134,6 +142,8 @@ "RequestToolState", "RequestInternalToolState", "RequestInternalDereferencedToolState", + "LandingRequestToolState", + "LandingRequestInternalToolState", "flat_state_path", "keys_starting_with", "visit_input_values", @@ -143,6 +153,8 @@ "encode", "encode_test", "fill_static_defaults", + "landing_decode", + "landing_encode", "dereference", "WorkflowStepToolState", "WorkflowStepLinkedToolState", diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index bb1ebc911089..94e4d0c5e569 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -40,6 +40,8 @@ ) from .state import ( JobInternalToolState, + LandingRequestInternalToolState, + LandingRequestToolState, RequestInternalDereferencedToolState, RequestInternalToolState, RequestToolState, @@ -67,7 +69,7 @@ def decode( external_state: RequestToolState, input_models: ToolParameterBundle, decode_id: Callable[[str], int] ) -> RequestInternalToolState: - """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + """Prepare an internal representation of tool state (request_internal) for storing in the database.""" external_state.validate(input_models) decode_callback = _decode_callback_for(decode_id) @@ -83,14 +85,14 @@ def decode( def encode( - external_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT + internal_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT ) -> RequestToolState: - """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + """Prepare an external representation of tool state (request) from persisted state in the database (request_internal).""" encode_callback = _encode_callback_for(encode_id) request_state_dict = visit_input_values( input_models, - external_state, + internal_state, encode_callback, ) request_state = RequestToolState(request_state_dict) @@ -98,6 +100,40 @@ def encode( return request_state +def landing_decode( + external_state: LandingRequestToolState, input_models: ToolParameterBundle, decode_id: Callable[[str], int] +) -> LandingRequestInternalToolState: + """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + + external_state.validate(input_models) + decode_callback = _decode_callback_for(decode_id) + internal_state_dict = visit_input_values( + input_models, + external_state, + decode_callback, + ) + + internal_request_state = LandingRequestInternalToolState(internal_state_dict) + internal_request_state.validate(input_models) + return internal_request_state + + +def landing_encode( + internal_state: LandingRequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT +) -> LandingRequestToolState: + """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + + encode_callback = _encode_callback_for(encode_id) + request_state_dict = visit_input_values( + input_models, + internal_state, + encode_callback, + ) + request_state = LandingRequestToolState(request_state_dict) + request_state.validate(input_models) + return request_state + + def dereference( internal_state: RequestInternalToolState, input_models: ToolParameterBundle, dereference: DereferenceCallable ) -> RequestInternalDereferencedToolState: diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py index 329423793d57..d731560cc895 100644 --- a/lib/galaxy/tool_util/parameters/models.py +++ b/lib/galaxy/tool_util/parameters/models.py @@ -64,6 +64,8 @@ "request", "request_internal", "request_internal_dereferenced", + "landing_request", + "landing_request_internal", "job_internal", "test_case_xml", "workflow_step", @@ -219,6 +221,8 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam requires_value = self.request_requires_value if state_representation == "job_internal": requires_value = True + elif _is_landing_request(state_representation): + requires_value = False return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property @@ -240,7 +244,12 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam py_type = self.py_type if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) - return dynamic_model_information_from_py_type(self, py_type) + requires_value = self.request_requires_value + if state_representation == "job_internal": + requires_value = True + elif _is_landing_request(state_representation): + requires_value = False + return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property def request_requires_value(self) -> bool: @@ -405,10 +414,19 @@ def py_type_test_case(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: if state_representation == "request": return allow_batching(dynamic_model_information_from_py_type(self, self.py_type), BatchDataInstance) + if state_representation == "landing_request": + return allow_batching( + dynamic_model_information_from_py_type(self, self.py_type, requires_value=False), BatchDataInstance + ) elif state_representation == "request_internal": return allow_batching( dynamic_model_information_from_py_type(self, self.py_type_internal), BatchDataInstanceInternal ) + elif state_representation == "landing_request_internal": + return allow_batching( + dynamic_model_information_from_py_type(self, self.py_type_internal, requires_value=False), + BatchDataInstanceInternal, + ) elif state_representation == "request_internal_dereferenced": return allow_batching( dynamic_model_information_from_py_type(self, self.py_type_internal_dereferenced), @@ -455,6 +473,12 @@ def py_type_internal(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: if state_representation == "request": return allow_batching(dynamic_model_information_from_py_type(self, self.py_type)) + elif state_representation == "landing_request": + return allow_batching(dynamic_model_information_from_py_type(self, self.py_type, requires_value=False)) + elif state_representation == "landing_request_internal": + return allow_batching( + dynamic_model_information_from_py_type(self, self.py_type_internal, requires_value=False) + ) elif state_representation in ["request_internal", "request_internal_dereferenced"]: return allow_batching(dynamic_model_information_from_py_type(self, self.py_type_internal)) elif state_representation == "job_internal": @@ -600,7 +624,10 @@ def py_type(self) -> Type: return AnyUrl def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: - return dynamic_model_information_from_py_type(self, self.py_type) + requires_value = self.request_requires_value + if _is_landing_request(state_representation): + requires_value = False + return dynamic_model_information_from_py_type(self, self.py_type, requires_value=requires_value) @property def request_requires_value(self) -> bool: @@ -1080,9 +1107,14 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam instance_class: Type[BaseModel] = create_field_model( self.parameters, f"Repeat_{self.name}", state_representation ) + min_length = self.min + max_length = self.max requires_value = self.request_requires_value if state_representation == "job_internal": requires_value = True + elif _is_landing_request(state_representation): + requires_value = False + min_length = 0 # in a landing request - parameters can be partially filled initialize_repeat: Any if requires_value: @@ -1091,7 +1123,7 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam initialize_repeat = None class RepeatType(RootModel): - root: List[instance_class] = Field(initialize_repeat, min_length=self.min, max_length=self.max) # type: ignore[valid-type] + root: List[instance_class] = Field(initialize_repeat, min_length=min_length, max_length=max_length) # type: ignore[valid-type] return DynamicModelInformation( self.name, @@ -1382,6 +1414,8 @@ def create_method(tool: ToolParameterBundle, name: str = DEFAULT_MODEL_NAME) -> create_request_model = create_model_factory("request") create_request_internal_model = create_model_factory("request_internal") create_request_internal_dereferenced_model = create_model_factory("request_internal_dereferenced") +create_landing_request_model = create_model_factory("landing_request") +create_landing_request_internal_model = create_model_factory("landing_request_internal") create_job_internal_model = create_model_factory("job_internal") create_test_case_model = create_model_factory("test_case_xml") create_workflow_step_model = create_model_factory("workflow_step") @@ -1413,6 +1447,10 @@ def create_field_model( return pydantic_model +def _is_landing_request(state_representation: StateRepresentationT): + return state_representation in ["landing_request", "landing_request_internal"] + + def validate_against_model(pydantic_model: Type[BaseModel], parameter_state: Dict[str, Any]) -> None: try: pydantic_model(**parameter_state) @@ -1441,6 +1479,8 @@ def validate_request(tool: ToolParameterBundle, request: Dict[str, Any], name: s validate_request = validate_model_type_factory("request") validate_internal_request = validate_model_type_factory("request_internal") validate_internal_request_dereferenced = validate_model_type_factory("request_internal_dereferenced") +validate_landing_request = validate_model_type_factory("landing_request") +validate_internal_landing_request = validate_model_type_factory("landing_request_internal") validate_internal_job = validate_model_type_factory("job_internal") validate_test_case = validate_model_type_factory("test_case_xml") validate_workflow_step = validate_model_type_factory("workflow_step") diff --git a/lib/galaxy/tool_util/parameters/state.py b/lib/galaxy/tool_util/parameters/state.py index af15cf23ac7b..3edc96f69c6e 100644 --- a/lib/galaxy/tool_util/parameters/state.py +++ b/lib/galaxy/tool_util/parameters/state.py @@ -15,6 +15,8 @@ from .models import ( create_job_internal_model, + create_landing_request_internal_model, + create_landing_request_model, create_request_internal_dereferenced_model, create_request_internal_model, create_request_model, @@ -84,6 +86,22 @@ def _parameter_model_for(cls, parameters: ToolParameterBundle) -> Type[BaseModel return create_request_internal_model(parameters) +class LandingRequestToolState(ToolState): + state_representation: Literal["landing_request"] = "landing_request" + + @classmethod + def _parameter_model_for(cls, parameters: ToolParameterBundle) -> Type[BaseModel]: + return create_landing_request_model(parameters) + + +class LandingRequestInternalToolState(ToolState): + state_representation: Literal["landing_request_internal"] = "landing_request_internal" + + @classmethod + def _parameter_model_for(cls, parameters: ToolParameterBundle) -> Type[BaseModel]: + return create_landing_request_internal_model(parameters) + + class RequestInternalDereferencedToolState(ToolState): state_representation: Literal["request_internal_dereferenced"] = "request_internal_dereferenced" diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index d656665be21b..5aa07efdd204 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -187,7 +187,6 @@ def __init__(self, tool, input_source, context=None): self.hidden = input_source.get_bool("hidden", False) self.refresh_on_change = input_source.get_bool("refresh_on_change", False) self.optional = input_source.parse_optional() - self.optionality_inferred = False self.is_dynamic = False self.label = input_source.parse_label() self.help = input_source.parse_help() @@ -352,6 +351,7 @@ def parse_name(input_source): class SimpleTextToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) + self.optionality_inferred = False super().__init__(tool, input_source) optional = input_source.get("optional", None) if optional is not None: @@ -405,6 +405,7 @@ class TextToolParameter(SimpleTextToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) + self.profile = tool.profile self.datalist = [] for title, value, _ in input_source.parse_static_options(): self.datalist.append({"label": title, "value": value}) @@ -420,6 +421,16 @@ def validate(self, value, trans=None): ): return super().validate(value, trans) + @property + def wrapper_default() -> Optional[str]: + """Handle change in default handling pre and post 23.0 profiles.""" + profile = self.profile + legacy_behavior = (profile is None or Version(str(profile)) < Version("23.0")) + default_value = None + if self.optional and self.optionality_inferred and legacy_behavior: + default_value = "" + return default_value + def to_dict(self, trans, other_values=None): d = super().to_dict(trans) other_values = other_values or {} diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 01e8e8491bbc..34e5b84737e2 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -19,7 +19,6 @@ Union, ) -from packaging.version import Version from typing_extensions import TypeAlias from galaxy.model import ( @@ -33,7 +32,10 @@ from galaxy.model.metadata import FileParameter from galaxy.model.none_like import NoneDataset from galaxy.security.object_wrapper import wrap_with_safe_string -from galaxy.tools.parameters.basic import BooleanToolParameter +from galaxy.tools.parameters.basic import ( + BooleanToolParameter, + TextToolParameter, +) from galaxy.tools.parameters.wrapped_json import ( data_collection_input_to_staging_path_and_source_path, data_input_to_staging_path_and_source_path, @@ -126,15 +128,9 @@ def __init__( profile: Optional[float] = None, ) -> None: self.input = input - if ( - value is None - and input.type == "text" - and input.optional - and input.optionality_inferred - and (profile is None or Version(str(profile)) < Version("23.0")) - ): + if value is None and input.type == "text": # Tools with old profile versions may treat an optional text parameter as `""` - value = "" + value = cast(TextToolParameter, input).wrapper_default() self.value = value self._other_values: Dict[str, str] = other_values or {} diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml index 2eb07ada9f38..29911941e714 100644 --- a/test/unit/tool_util/parameter_specification.yml +++ b/test/unit/tool_util/parameter_specification.yml @@ -32,6 +32,18 @@ gx_int: job_internal_invalid: - parameter: null - {} + landing_request_valid: + - {} + - parameter: 5 + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: 5 + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" test_case_xml_valid: - parameter: 5 - {} @@ -71,6 +83,21 @@ gx_boolean: - parameter: false job_internal_invalid: - {} + landing_request_valid: + - {} + - parameter: true + - parameter: false + landing_request_invalid: + - parameter: "true" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: true + - parameter: false + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "true" + - parameter: "5" workflow_step_valid: - parameter: True - {} @@ -102,6 +129,20 @@ gx_int_optional: - parameter: null job_internal_invalid: - {} + landing_request_valid: + - {} + - parameter: 5 + - parameter: null + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: 5 + - parameter: null + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" workflow_step_valid: - parameter: 5 - parameter: null @@ -132,6 +173,20 @@ gx_int_required: &gx_int_required - parameter: 5 job_internal_invalid: - {} + # and actual request will require a value but the landing only needs to specify parameters + # of interest - forcing users to make the rest of the selections. + landing_request_valid: + - {} + - parameter: 5 + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: 5 + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" gx_int_required_via_empty_string: <<: *gx_int_required @@ -156,6 +211,14 @@ gx_text: *gx_text_request_valid request_internal_dereferenced_invalid: *gx_text_request_invalid + landing_request_valid: + *gx_text_request_valid + landing_request_invalid: + *gx_text_request_invalid + landing_request_internal_valid: + *gx_text_request_valid + landing_request_internal_invalid: + *gx_text_request_invalid job_internal_valid: - parameter: moocow - parameter: 'some spaces' @@ -198,6 +261,18 @@ gx_text_optional: - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } + landing_request_valid: &gx_text_optional_landing_request_valid + - parameter: "moo cow" + - {} + - parameter: null + landing_request_invalid: &gx_text_optional_landing_request_invalid + - parameter: 5 + - parameter: 6 + landing_request_internal_valid: + *gx_text_optional_landing_request_valid + landing_request_internal_invalid: + *gx_text_optional_landing_request_invalid + workflow_step_valid: - parameter: moocow - parameter: 'some spaces' @@ -243,6 +318,18 @@ gx_select: *gx_select_request_valid request_internal_dereferenced_invalid: *gx_select_request_invalid + landing_request_valid: &gx_select_landing_request_valid + - parameter: "--ex1" + - parameter: "ex2" + - {} + landing_request_invalid: &gx_select_landing_request_invalid + - parameter: 5 + - parameter: null + - parameter: "Ex1" + landing_request_internal_valid: + *gx_select_landing_request_valid + landing_request_internal_invalid: + *gx_select_landing_request_invalid job_internal_valid: - parameter: '--ex1' - parameter: 'ex2' @@ -283,6 +370,19 @@ gx_select_optional: - parameter: ["ex2"] - parameter: {} - parameter: 5 + landing_request_valid: &gx_select_optional_landing_request_valid + - parameter: "--ex1" + - parameter: "ex2" + - parameter: null + - {} + landing_request_invalid: &gx_select_optional_landing_request_invalid + - parameter: 5 + - parameter: "Ex1" + - parameter: {} + landing_request_internal_valid: + *gx_select_optional_landing_request_valid + landing_request_internal_invalid: + *gx_select_optional_landing_request_invalid job_internal_valid: - parameter: '--ex1' - parameter: 'ex2' @@ -326,6 +426,20 @@ gx_select_multiple: - parameter: ["Ex1"] - parameter: {} - parameter: 5 + landing_request_valid: &gx_select_multiple_landing_request_valid + - parameter: ["--ex1"] + - parameter: ["ex2"] + - parameter: null + - {} + landing_request_invalid: &gx_select_multiple_landing_request_invalid + - parameter: 5 + - parameter: ["Ex1"] + - parameter: {} + - parameter: false + landing_request_internal_valid: + *gx_select_multiple_landing_request_valid + landing_request_internal_invalid: + *gx_select_multiple_landing_request_invalid workflow_step_valid: - parameter: ["--ex1"] - parameter: ["ex2"] @@ -379,6 +493,17 @@ gx_genomebuild: request_invalid: - parameter: null - parameter: 9 + landing_request_valid: &gx_genomebuild_landing_request_valid + - parameter: hg19 + - parameter: hg18 + - {} + landing_request_invalid: &gx_genomebuild_landing_request_invalid + - parameter: null + - parameter: 9 + landing_request_internal_valid: + *gx_genomebuild_landing_request_valid + landing_request_internal_invalid: + *gx_genomebuild_landing_request_invalid job_internal_valid: - parameter: hg19 job_internal_invalid: @@ -394,6 +519,17 @@ gx_genomebuild_optional: request_invalid: - parameter: 8 - parameter: true + landing_request_valid: &gx_genomebuild_optional_landing_request_valid + - parameter: hg19 + - parameter: hg18 + - {} + - parameter: null + landing_request_invalid: &gx_genomebuild_optional_landing_request_invalid + - parameter: 9 + landing_request_internal_valid: + *gx_genomebuild_optional_landing_request_valid + landing_request_internal_invalid: + *gx_genomebuild_optional_landing_request_invalid job_internal_valid: - parameter: null job_internal_invalid: @@ -415,6 +551,18 @@ gx_directory_uri: - parameter: "justsomestring" - parameter: true - parameter: null + landing_request_valid: &gx_directory_uri_landing_request_valid + - parameter: "gxfiles://foobar/" + - parameter: "gxfiles://foobar" + - {} + landing_request_invalid: &gx_directory_uri_landing_request_invalid + - parameter: true + - parameter: null + - parameter: 8 + landing_request_internal_valid: + *gx_directory_uri_landing_request_valid + landing_request_internal_invalid: + *gx_directory_uri_landing_request_invalid job_internal_valid: - parameter: "gxfiles://foobar/" - parameter: "gxfiles://foobar" @@ -435,6 +583,20 @@ gx_hidden: - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } + landing_request_valid: &gx_hidden_landing_request_valid + - parameter: moocow + - parameter: 'some spaces' + - parameter: '' + - {} + landing_request_invalid: &gx_hidden_landing_request_invalid + - parameter: null + - parameter: 5 + - parameter: {} + - parameter: { "moo": "cow" } + landing_request_internal_valid: + *gx_hidden_landing_request_valid + landing_request_internal_invalid: + *gx_hidden_landing_request_invalid job_internal_valid: - parameter: moocow - parameter: 'some spaces' @@ -465,6 +627,20 @@ gx_hidden_optional: - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } + landing_request_valid: &gx_hidden_optional_landing_request_valid + - parameter: moocow + - parameter: 'some spaces' + - parameter: '' + - {} + - parameter: null + landing_request_invalid: &gx_hidden_optional_landing_request_invalid + - parameter: 5 + - parameter: {} + - parameter: { "moo": "cow" } + landing_request_internal_valid: + *gx_hidden_optional_landing_request_valid + landing_request_internal_invalid: + *gx_hidden_optional_landing_request_invalid workflow_step_valid: - parameter: moocow - {} @@ -492,6 +668,30 @@ gx_float: - parameter: "5" - parameter: "5.0" - parameter: { "moo": "cow" } + job_internal_valid: + - parameter: 5 + - parameter: 5.0 + job_internal_invalid: + - {} + - parameter: "5" + - parameter: "5.0" + - parameter: null + landing_request_valid: + - {} + - parameter: 5 + - parameter: 5.0 + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" + landing_request_internal_valid: + - {} + - parameter: 5 + - parameter: 5.0 + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" test_case_xml_valid: - parameter: 5 - parameter: 5.0 @@ -529,6 +729,32 @@ gx_float_optional: - parameter: "5.0" - parameter: {} - parameter: { "moo": "cow" } + job_internal_valid: + - parameter: 5 + - parameter: 5.0 + - parameter: null + job_internal_invalid: + - {} + - parameter: "5" + - parameter: "5.0" + landing_request_valid: + - {} + - parameter: 5 + - parameter: 5.0 + - parameter: null + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" + landing_request_internal_valid: + - {} + - parameter: 5 + - parameter: 5.0 + - parameter: null + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" test_case_xml_valid: - parameter: 5 - parameter: 5.0 @@ -616,6 +842,24 @@ gx_data: # the difference between request internal and request internal dereferenced is that these have been converted # to datasets. - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", ext: "txt"} + landing_request_valid: + - parameter: {src: hda, id: abcdabcd} + - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", "ext": "txt"} + - parameter: {__class__: "Batch", values: [{src: hdca, id: abcdabcd}]} + - {} + - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", "ext": "txt"} + landing_request_invalid: + - parameter: {__class__: "Batch", values: [{src: hdca, id: 5}]} + - parameter: {src: hda, id: 5} + - parameter: {src: hda, id: 0} + landing_request_internal_valid: + - parameter: {src: hda, id: 5} + - parameter: {__class__: "Batch", values: [{src: hdca, id: 5}]} + - {} + - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", "ext": "txt"} + landing_request_internal_invalid: + - parameter: {src: hda, id: abcdabcd} + - parameter: {__class__: "Batch", values: [{src: hdca, id: abcdabcd}]} job_internal_valid: - parameter: {src: hda, id: 7} job_internal_invalid: @@ -744,6 +988,16 @@ gx_data_multiple: # the difference between request internal and request internal dereferenced is that these have been converted # to datasets. - parameter: [{src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", ext: "txt"}] + landing_request_valid: + - parameter: [{src: hda, id: abcdabcd}] + - {} + landing_request_invalid: + - parameter: [{src: hda, id: 6}] + landing_request_internal_valid: + - parameter: [{src: hda, id: 6}] + - {} + landing_request_internal_invalid: + - parameter: [{src: hda, id: abcdabcd}] job_internal_valid: - parameter: {src: hda, id: 5} - parameter: [{src: hda, id: 5}, {src: hda, id: 6}] @@ -824,6 +1078,18 @@ gx_data_collection: - parameter: {src: hdca, id: 5} request_internal_dereferenced_invalid: - parameter: {src: hdca, id: abcdabcd} + landing_request_valid: + - parameter: {src: hdca, id: abcdabcd} + - {} + landing_request_invalid: + - parameter: {src: hdca, id: 5} + - parameter: [{src: hdca, id: abcdabcd}] + landing_request_internal_valid: + - parameter: {src: hdca, id: 5} + - {} + landing_request_internal_invalid: + - parameter: {src: hdca, id: abcdabcd} + - parameter: [{src: hdca, id: abcdabcd}] workflow_step_valid: - {} workflow_step_invalid: @@ -909,6 +1175,20 @@ gx_data_collection_optional: - parameter: 5 - parameter: "5" - parameter: {} + landing_request_valid: + - parameter: {src: hdca, id: abcdabcd} + - parameter: null + - {} + landing_request_invalid: + - parameter: {src: hdca, id: 5} + - parameter: [{src: hdca, id: abcdabcd}] + landing_request_internal_valid: + - parameter: {src: hdca, id: 5} + - parameter: null + - {} + landing_request_internal_invalid: + - parameter: {src: hdca, id: abcdabcd} + - parameter: [{src: hdca, id: abcdabcd}] job_internal_valid: - parameter: {src: hdca, id: 5} - parameter: null @@ -957,6 +1237,16 @@ gx_conditional_boolean: # in that case having an integer_parameter is not acceptable. - conditional_parameter: integer_parameter: 5 + landing_request_valid: &gx_conditional_boolean_landing_request_valid + - {} + landing_request_invalid: &gx_conditional_boolean_landing_request_invalid + - conditional_parameter: + test_parameter: false + integer_parameter: 1 + landing_request_internal_valid: + *gx_conditional_boolean_landing_request_valid + landing_request_internal_invalid: + *gx_conditional_boolean_landing_request_invalid job_internal_valid: - conditional_parameter: test_parameter: true @@ -1106,6 +1396,20 @@ gx_repeat_boolean: - { boolean_parameter: false } - { boolean_parameter: 4 } - parameter: 5 + landing_request_valid: &gx_repeat_boolean_landing_request_valid + - {} + - parameter: + - { boolean_parameter: true } + - { boolean_parameter: false } + - parameter: [{}] + - parameter: [{}, {}] + landing_request_invalid: &gx_repeat_boolean_landing_request_invalid + - parameter: + - { boolean_parameter: 4 } + landing_request_internal_valid: + *gx_repeat_boolean_landing_request_valid + landing_request_internal_invalid: + *gx_repeat_boolean_landing_request_invalid job_internal_valid: - parameter: - { boolean_parameter: true } @@ -1174,6 +1478,16 @@ gx_repeat_data: request_internal_invalid: - parameter: - { data_parameter: { src: hda, id: abcdabcd } } + landing_request_valid: &gx_repeat_data_landing_request_valid + - {} + # unlike above - in landing mode all parameters are optional so it is find to have an unset data parameters + - parameter: [{}] + landing_request_invalid: &gx_repeat_data_landing_request_invalid + - parameter: 5 + landing_request_internal_valid: + *gx_repeat_data_landing_request_valid + landing_request_internal_invalid: + *gx_repeat_data_landing_request_invalid job_internal_valid: - parameter: - { data_parameter: { src: hda, id: 5 } } @@ -1197,6 +1511,22 @@ gx_repeat_data_min: - parameter: [{}, {}] - parameter: [{}] - parameter: 5 + landing_request_valid: + - {} + # ignore the min here - can just specify the first set of parameters and I think that should work + - parameter: + - { data_parameter: {src: hda, id: abcdabcd} } + landing_request_invalid: + - parameter: + - { data_parameter: {src: hda, id: 5} } + landing_request_internal_valid: + - {} + # ignore the min here - can just specify the first set of parameters and I think that should work + - parameter: + - { data_parameter: {src: hda, id: 5} } + landing_request_internal_invalid: + - parameter: + - { data_parameter: {src: hda, id: abcdabcd} } request_internal_valid: - parameter: - { data_parameter: { src: hda, id: 5 } } diff --git a/test/unit/tool_util/test_parameter_convert.py b/test/unit/tool_util/test_parameter_convert.py index f86750026766..a14eab6d5657 100644 --- a/test/unit/tool_util/test_parameter_convert.py +++ b/test/unit/tool_util/test_parameter_convert.py @@ -12,6 +12,9 @@ encode, fill_static_defaults, input_models_for_tool_source, + landing_decode, + landing_encode, + LandingRequestToolState, RequestInternalDereferencedToolState, RequestInternalToolState, RequestToolState, @@ -102,6 +105,20 @@ def test_multi_data(): assert encoded_state.input_state["parameter"][1]["id"] == EXAMPLE_ID_2_ENCODED +def test_landing_encode_data(): + tool_source = tool_source_for("parameters/gx_data") + bundle = input_models_for_tool_source(tool_source) + request_state = LandingRequestToolState({"parameter": {"src": "hda", "id": EXAMPLE_ID_1_ENCODED}}) + request_state.validate(bundle) + decoded_state = landing_decode(request_state, bundle, _fake_decode) + assert decoded_state.input_state["parameter"]["src"] == "hda" + assert decoded_state.input_state["parameter"]["id"] == EXAMPLE_ID_1 + + encoded_state = landing_encode(decoded_state, bundle, _fake_encode) + assert encoded_state.input_state["parameter"]["src"] == "hda" + assert encoded_state.input_state["parameter"]["id"] == EXAMPLE_ID_1_ENCODED + + def test_dereference(): tool_source = tool_source_for("parameters/gx_data") bundle = input_models_for_tool_source(tool_source) diff --git a/test/unit/tool_util/test_parameter_specification.py b/test/unit/tool_util/test_parameter_specification.py index 012213a02499..ed4b437449b5 100644 --- a/test/unit/tool_util/test_parameter_specification.py +++ b/test/unit/tool_util/test_parameter_specification.py @@ -18,8 +18,10 @@ RequestToolState, ToolParameterBundleModel, validate_internal_job, + validate_internal_landing_request, validate_internal_request, validate_internal_request_dereferenced, + validate_landing_request, validate_request, validate_test_case, validate_workflow_step, @@ -97,6 +99,10 @@ def _test_file(file: str, specification=None, parameter_bundle: Optional[ToolPar "request_internal_invalid": _assert_internal_requests_invalid, "request_internal_dereferenced_valid": _assert_internal_requests_dereferenced_validate, "request_internal_dereferenced_invalid": _assert_internal_requests_dereferenced_invalid, + "landing_request_valid": _assert_landing_requests_validate, + "landing_request_invalid": _assert_landing_requests_invalid, + "landing_request_internal_valid": _assert_internal_landing_requests_validate, + "landing_request_internal_invalid": _assert_internal_landing_requests_invalid, "job_internal_valid": _assert_internal_jobs_validate, "job_internal_invalid": _assert_internal_jobs_invalid, "test_case_xml_valid": _assert_test_cases_validate, @@ -165,6 +171,12 @@ def _assert_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) _assert_workflow_step_linked_validates, _assert_workflow_step_linked_invalid = model_assertion_function_factory( validate_workflow_step_linked, "linked workflow step tool state" ) +_assert_landing_request_validates, _assert_landing_request_invalid = model_assertion_function_factory( + validate_landing_request, "landing request" +) +_assert_internal_landing_request_validates, _assert_internal_landing_request_invalid = model_assertion_function_factory( + validate_internal_landing_request, " internallanding request" +) _assert_requests_validate = partial(_for_each, _assert_request_validates) _assert_requests_invalid = partial(_for_each, _assert_request_invalid) @@ -180,6 +192,10 @@ def _assert_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) _assert_workflow_steps_invalid = partial(_for_each, _assert_workflow_step_invalid) _assert_workflow_steps_linked_validate = partial(_for_each, _assert_workflow_step_linked_validates) _assert_workflow_steps_linked_invalid = partial(_for_each, _assert_workflow_step_linked_invalid) +_assert_landing_requests_validate = partial(_for_each, _assert_landing_request_validates) +_assert_landing_requests_invalid = partial(_for_each, _assert_landing_request_invalid) +_assert_internal_landing_requests_validate = partial(_for_each, _assert_internal_landing_request_validates) +_assert_internal_landing_requests_invalid = partial(_for_each, _assert_internal_landing_request_invalid) def decode_val(val: str) -> int: From b11b7d42a4c2c218284fb5a67f70db6410a1569c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 7 Oct 2024 11:07:49 -0400 Subject: [PATCH 09/12] Pydantic models for parameter validators. --- lib/galaxy/tool_util/parameters/_types.py | 11 + lib/galaxy/tool_util/parameters/factory.py | 76 +- lib/galaxy/tool_util/parameters/models.py | 116 ++- lib/galaxy/tool_util/parser/interface.py | 3 +- .../tool_util/parser/parameter_validators.py | 725 ++++++++++++++++++ lib/galaxy/tool_util/parser/util.py | 24 + lib/galaxy/tool_util/parser/xml.py | 8 +- .../tool_util/unittest_utils/sample_data.py | 53 ++ lib/galaxy/tools/parameters/basic.py | 35 +- .../tools/parameters/dynamic_options.py | 5 +- lib/galaxy/tools/parameters/validation.py | 398 ++++------ lib/galaxy/tools/wrappers.py | 2 +- .../gx_directory_uri_validation.xml | 16 + .../tools/parameters/gx_float_min_max.xml | 14 + .../parameters/gx_float_validation_range.xml | 15 + .../tools/parameters/gx_hidden_validation.xml | 17 + .../tools/parameters/gx_int_min_max.xml | 14 + .../parameters/gx_int_validation_range.xml | 15 + .../gx_text_expression_validation.xml | 20 + .../parameters/gx_text_length_validation.xml | 20 + .../gx_text_length_validation_negate.xml | 20 + .../parameters/gx_text_regex_validation.xml | 20 + test/unit/app/tools/test_dynamic_options.py | 6 +- .../app/tools/test_parameter_validation.py | 4 +- .../unit/app/tools/test_validation_parsing.py | 41 + .../tool_util/parameter_specification.yml | 147 +++- .../test_parameter_validator_models.py | 28 + 27 files changed, 1549 insertions(+), 304 deletions(-) create mode 100644 lib/galaxy/tool_util/parser/parameter_validators.py create mode 100644 test/functional/tools/parameters/gx_directory_uri_validation.xml create mode 100644 test/functional/tools/parameters/gx_float_min_max.xml create mode 100644 test/functional/tools/parameters/gx_float_validation_range.xml create mode 100644 test/functional/tools/parameters/gx_hidden_validation.xml create mode 100644 test/functional/tools/parameters/gx_int_min_max.xml create mode 100644 test/functional/tools/parameters/gx_int_validation_range.xml create mode 100644 test/functional/tools/parameters/gx_text_expression_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_length_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_length_validation_negate.xml create mode 100644 test/functional/tools/parameters/gx_text_regex_validation.xml create mode 100644 test/unit/app/tools/test_validation_parsing.py create mode 100644 test/unit/tool_util/test_parameter_validator_models.py diff --git a/lib/galaxy/tool_util/parameters/_types.py b/lib/galaxy/tool_util/parameters/_types.py index 2b97ee16c200..64fa9a3274fc 100644 --- a/lib/galaxy/tool_util/parameters/_types.py +++ b/lib/galaxy/tool_util/parameters/_types.py @@ -6,6 +6,7 @@ """ from typing import ( + Any, cast, List, Optional, @@ -15,6 +16,7 @@ # https://stackoverflow.com/questions/56832881/check-if-a-field-is-typing-optional from typing_extensions import ( + Annotated, get_args, get_origin, ) @@ -46,3 +48,12 @@ def cast_as_type(arg) -> Type: def is_optional(field) -> bool: return get_origin(field) is Union and type(None) in get_args(field) + + +def expand_annotation(field: Type, new_annotations: List[Any]) -> Type: + is_annotation = get_origin(field) is Annotated + if is_annotation: + args = get_args(field) # noqa: F841 + return Annotated[tuple([args[0], *args[1:], *new_annotations])] # type: ignore[return-value] + else: + return Annotated[tuple([field, *new_annotations])] # type: ignore[return-value] diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py index e200b67aeb0e..13d52d5f9c77 100644 --- a/lib/galaxy/tool_util/parameters/factory.py +++ b/lib/galaxy/tool_util/parameters/factory.py @@ -14,7 +14,19 @@ PagesSource, ToolSource, ) -from galaxy.tool_util.parser.util import parse_profile_version +from galaxy.tool_util.parser.parameter_validators import ( + EmptyFieldParameterValidatorModel, + ExpressionParameterValidatorModel, + InRangeParameterValidatorModel, + LengthParameterValidatorModel, + NoOptionsParameterValidatorModel, + RegexParameterValidatorModel, + static_validators, +) +from galaxy.tool_util.parser.util import ( + parse_profile_version, + text_input_is_optional, +) from galaxy.util import string_as_bool from .models import ( BaseUrlParameterModel, @@ -42,10 +54,13 @@ HiddenParameterModel, IntegerParameterModel, LabelValue, + NumberCompatiableValidators, RepeatParameterModel, RulesParameterModel, SectionParameterModel, + SelectCompatiableValidators, SelectParameterModel, + TextCompatiableValidators, TextParameterModel, ToolParameterBundle, ToolParameterBundleModel, @@ -82,7 +97,23 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool int_value = None else: raise ParameterDefinitionError() - return IntegerParameterModel(name=input_source.parse_name(), optional=optional, value=int_value) + static_validator_models = static_validators(input_source.parse_validators()) + int_validators: List[NumberCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "in_range": + int_validators.append(cast(InRangeParameterValidatorModel, static_validator)) + min_raw = input_source.get("min", None) + max_raw = input_source.get("max", None) + min_int = int(min_raw) if min_raw is not None else None + max_int = int(max_raw) if max_raw is not None else None + return IntegerParameterModel( + name=input_source.parse_name(), + optional=optional, + value=int_value, + min=min_int, + max=max_int, + validators=int_validators, + ) elif param_type == "boolean": nullable = input_source.parse_optional() value = input_source.get_bool_or_none("checked", None if nullable else False) @@ -92,10 +123,12 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool value=value, ) elif param_type == "text": - optional = input_source.parse_optional() + optional, optionality_inferred = text_input_is_optional(input_source) + text_validators: List[TextCompatiableValidators] = _text_validators(input_source) return TextParameterModel( name=input_source.parse_name(), optional=optional, + validators=text_validators, ) elif param_type == "float": optional = input_source.parse_optional() @@ -107,18 +140,32 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool float_value = None else: raise ParameterDefinitionError() + static_validator_models = static_validators(input_source.parse_validators()) + float_validators: List[NumberCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "in_range": + float_validators.append(cast(InRangeParameterValidatorModel, static_validator)) + min_raw = input_source.get("min", None) + max_raw = input_source.get("max", None) + min_float = float(min_raw) if min_raw is not None else None + max_float = float(max_raw) if max_raw is not None else None return FloatParameterModel( name=input_source.parse_name(), optional=optional, value=float_value, + min=min_float, + max=max_float, + validators=float_validators, ) elif param_type == "hidden": optional = input_source.parse_optional() value = input_source.get("value") + hidden_validators: List[TextCompatiableValidators] = _text_validators(input_source) return HiddenParameterModel( name=input_source.parse_name(), optional=optional, value=value, + validators=hidden_validators, ) elif param_type == "color": optional = input_source.parse_optional() @@ -158,11 +205,17 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool options = [] for option_label, option_value, selected in input_source.parse_static_options(): options.append(LabelValue(label=option_label, value=option_value, selected=selected)) + static_validator_models = static_validators(input_source.parse_validators()) + select_validators: List[SelectCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "no_options": + select_validators.append(cast(NoOptionsParameterValidatorModel, static_validator)) return SelectParameterModel( name=input_source.parse_name(), optional=optional, options=options, multiple=multiple, + validators=select_validators, ) elif param_type == "drill_down": multiple = input_source.get_bool("multiple", False) @@ -206,8 +259,10 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool multiple=multiple, ) elif param_type == "directory_uri": + directory_uri_validators: List[TextCompatiableValidators] = _text_validators(input_source) return DirectoryUriParameterModel( name=input_source.parse_name(), + validators=directory_uri_validators, ) else: raise UnknownParameterTypeError(f"Unknown Galaxy parameter type {param_type}") @@ -304,6 +359,21 @@ def _simple_cwl_type_to_model(simple_type: str, input_source: CwlInputSource): ) +def _text_validators(input_source: InputSource) -> List[TextCompatiableValidators]: + static_validator_models = static_validators(input_source.parse_validators()) + text_validators: List[TextCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "length": + text_validators.append(cast(LengthParameterValidatorModel, static_validator)) + elif static_validator.type == "regex": + text_validators.append(cast(RegexParameterValidatorModel, static_validator)) + elif static_validator.type == "expression": + text_validators.append(cast(ExpressionParameterValidatorModel, static_validator)) + elif static_validator.type == "empty_field": + text_validators.append(cast(EmptyFieldParameterValidatorModel, static_validator)) + return text_validators + + def _from_input_source_cwl(input_source: CwlInputSource) -> ToolParameterT: schema_salad_field = input_source.field if schema_salad_field is None: diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py index d731560cc895..5019a92f5f70 100644 --- a/lib/galaxy/tool_util/parameters/models.py +++ b/lib/galaxy/tool_util/parameters/models.py @@ -11,11 +11,14 @@ Mapping, NamedTuple, Optional, + Sequence, Type, + TypeVar, Union, ) from pydantic import ( + AfterValidator, AnyUrl, BaseModel, ConfigDict, @@ -44,8 +47,18 @@ JsonTestCollectionDefDict, JsonTestDatasetDefDict, ) +from galaxy.tool_util.parser.parameter_validators import ( + EmptyFieldParameterValidatorModel, + ExpressionParameterValidatorModel, + InRangeParameterValidatorModel, + LengthParameterValidatorModel, + NoOptionsParameterValidatorModel, + RegexParameterValidatorModel, + StaticValidatorModel, +) from ._types import ( cast_as_type, + expand_annotation, is_optional, list_type, optional, @@ -54,7 +67,6 @@ ) # TODO: -# - implement job vs request... # - implement data_ref on rules and implement some cross model validation # + request: Return info needed to build request pydantic model at runtime. @@ -179,18 +191,64 @@ class LabelValue(BaseModel): selected: bool +TextCompatiableValidators = Union[ + LengthParameterValidatorModel, + RegexParameterValidatorModel, + ExpressionParameterValidatorModel, + EmptyFieldParameterValidatorModel, +] + + +def pydantic_to_galaxy_type(value: Any) -> Any: + """We use advanced Pydantic types like URL but the Galaxy validators only expect strings for these.""" + if isinstance(value, AnyUrl): + return str(value) + + return value + + +VT = TypeVar("VT", bound=StaticValidatorModel) + + +def decorate_type_with_validators_if_needed(py_type: Type, static_validator_models: Sequence[VT]) -> Type: + pydantic_validator = pydantic_validator_for(static_validator_models) + if pydantic_validator: + return expand_annotation(py_type, [pydantic_validator]) + else: + return py_type + + +# Looks like Annotated only work with one PlainValidator so condensing all static validators +# into a single PlainValidator for pydantic. +def pydantic_validator_for(static_validator_models: Sequence[VT]) -> Optional[AfterValidator]: + + if static_validator_models: + + def validator(v: Any) -> Any: + gx_val = pydantic_to_galaxy_type(v) + + for static_validator_model in static_validator_models: + static_validator_model.statically_validate(gx_val) + return v + + return AfterValidator(validator) + else: + return None + + class TextParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_text"] = "gx_text" area: bool = False default_value: Optional[str] = Field(default=None, alias="value") default_options: List[LabelValue] = [] + validators: List[TextCompatiableValidators] = [] @property def py_type(self) -> Type: return optional_if_needed(StrictStr, self.optional) def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: - py_type = self.py_type + py_type = decorate_type_with_validators_if_needed(self.py_type, self.validators) if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) requires_value = self.request_requires_value @@ -203,12 +261,16 @@ def request_requires_value(self) -> bool: return False +NumberCompatiableValidators = Union[InRangeParameterValidatorModel,] + + class IntegerParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_integer"] = "gx_integer" optional: bool value: Optional[int] = None min: Optional[int] = None max: Optional[int] = None + validators: List[NumberCompatiableValidators] = [] @property def py_type(self) -> Type: @@ -216,6 +278,10 @@ def py_type(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: py_type = self.py_type + validators = self.validators[:] + if self.min is not None or self.max is not None: + validators.append(InRangeParameterValidatorModel(min=self.min, max=self.max, implicit=True)) + py_type = decorate_type_with_validators_if_needed(py_type, validators) if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) requires_value = self.request_requires_value @@ -235,6 +301,7 @@ class FloatParameterModel(BaseGalaxyToolParameterModelDefinition): value: Optional[float] = None min: Optional[float] = None max: Optional[float] = None + validators: List[NumberCompatiableValidators] = [] @property def py_type(self) -> Type: @@ -249,6 +316,10 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam requires_value = True elif _is_landing_request(state_representation): requires_value = False + validators = self.validators[:] + if self.min is not None or self.max is not None: + validators.append(InRangeParameterValidatorModel(min=self.min, max=self.max, implicit=True)) + py_type = decorate_type_with_validators_if_needed(py_type, validators) return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property @@ -502,6 +573,7 @@ def request_requires_value(self) -> bool: class HiddenParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_hidden"] = "gx_hidden" value: Optional[str] + validators: List[TextCompatiableValidators] = [] @property def py_type(self) -> Type: @@ -510,6 +582,7 @@ def py_type(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: py_type = self.py_type requires_value = not self.optional and self.value is None + py_type = decorate_type_with_validators_if_needed(py_type, self.validators) if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) elif state_representation == "workflow_step" and not self.optional: @@ -618,16 +691,21 @@ def request_requires_value(self) -> bool: class DirectoryUriParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_directory_uri"] = "gx_directory_uri" + validators: List[TextCompatiableValidators] = [] @property def py_type(self) -> Type: return AnyUrl def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: + py_type = self.py_type + py_type = decorate_type_with_validators_if_needed(py_type, self.validators) + if state_representation == "workflow_step_linked": + py_type = allow_connected_value(py_type) requires_value = self.request_requires_value if _is_landing_request(state_representation): requires_value = False - return dynamic_model_information_from_py_type(self, self.py_type, requires_value=requires_value) + return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property def request_requires_value(self) -> bool: @@ -659,10 +737,14 @@ def request_requires_value(self) -> bool: return True +SelectCompatiableValidators = Union[NoOptionsParameterValidatorModel,] + + class SelectParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_select"] = "gx_select" options: Optional[List[LabelValue]] = None multiple: bool + validators: List[SelectCompatiableValidators] @staticmethod def split_str(cls, data: Any) -> Any: @@ -699,28 +781,30 @@ def py_type_workflow_step(self) -> Type: return optional(self.py_type_if_required()) def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: + validators = {} + requires_value = self.request_requires_value + py_type = None if state_representation == "workflow_step": - return dynamic_model_information_from_py_type(self, self.py_type_workflow_step, requires_value=False) + py_type = self.py_type_workflow_step elif state_representation == "workflow_step_linked": py_type = self.py_type_if_required(allow_connections=True) - return dynamic_model_information_from_py_type( - self, optional_if_needed(py_type, self.optional or self.multiple) - ) + py_type = optional_if_needed(py_type, self.optional or self.multiple) elif state_representation == "test_case_xml": # in a YAML test case representation this can be string, in XML we are still expecting a comma separated string py_type = self.py_type_if_required(allow_connections=False) if self.multiple: validators = {"from_string": field_validator(self.name, mode="before")(SelectParameterModel.split_str)} - else: - validators = {} - return dynamic_model_information_from_py_type( - self, optional_if_needed(py_type, self.optional), validators=validators - ) + py_type = optional_if_needed(py_type, self.optional) + elif state_representation == "job_internal": + requires_value = True + py_type = self.py_type else: - requires_value = self.request_requires_value - if state_representation == "job_internal": - requires_value = True - return dynamic_model_information_from_py_type(self, self.py_type, requires_value=requires_value) + py_type = self.py_type + + py_type = decorate_type_with_validators_if_needed(py_type, self.validators) + return dynamic_model_information_from_py_type( + self, py_type, validators=validators, requires_value=requires_value + ) @property def has_selected_static_option(self): diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index 441985902805..b9218ad54270 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -28,6 +28,7 @@ from galaxy.util import Element from galaxy.util.path import safe_walk +from .parameter_validators import AnyValidatorModel from .util import _parse_name if TYPE_CHECKING: @@ -502,7 +503,7 @@ def parse_sanitizer_elem(self): """ return None - def parse_validator_elems(self): + def parse_validators(self) -> List[AnyValidatorModel]: """Return an XML description of sanitizers. This is a stop gap until we can rework galaxy.tools.parameters.validation to not explicitly depend on XML. diff --git a/lib/galaxy/tool_util/parser/parameter_validators.py b/lib/galaxy/tool_util/parser/parameter_validators.py new file mode 100644 index 000000000000..6525b7d90fb8 --- /dev/null +++ b/lib/galaxy/tool_util/parser/parameter_validators.py @@ -0,0 +1,725 @@ +import json +from typing import ( + Any, + cast, + List, + Optional, + Sequence, + Union, +) + +from pydantic import ( + BaseModel, + ConfigDict, + Field, + model_validator, + PrivateAttr, +) +from typing_extensions import ( + Annotated, + get_args, + Literal, + Protocol, + Self, +) + +from galaxy.util import ( + asbool, + Element, +) + +try: + import regex +except ImportError: + import re as regex + + +class ValidationArgument: + doc: Optional[str] + xml_body: bool + xml_allow_json_load: bool + + def __init__( + self, + doc: Optional[str], + xml_body: bool = False, + xml_allow_json_load: bool = False, + ): + self.doc = doc + self.xml_body = xml_body + self.xml_allow_json_load = xml_allow_json_load + + +Negate = Annotated[ + bool, + ValidationArgument("Negates the result of the validator."), +] +NEGATE_DEFAULT = False +SPLIT_DEFAULT = "\t" +DEFAULT_VALIDATOR_MESSAGE = "Parameter validation error." + +ValidatorType = Literal[ + "expression", + "regex", + "in_range", + "length", + "metadata", + "dataset_metadata_equal", + "unspecified_build", + "no_options", + "empty_field", + "empty_dataset", + "empty_extra_files_path", + "dataset_metadata_in_data_table", + "dataset_metadata_not_in_data_table", + "dataset_metadata_in_range", + "value_in_data_table", + "value_not_in_data_table", + "dataset_ok_validator", + "dataset_metadata_in_file", +] + + +class StrictModel(BaseModel): + model_config = ConfigDict(extra="forbid") + + +class ParameterValidatorModel(StrictModel): + type: ValidatorType + message: Annotated[ + Optional[str], + ValidationArgument( + """The error message displayed on the tool form if validation fails. A placeholder string ``%s`` will be repaced by the ``value``""" + ), + ] = None + # track validators setup by other input parameters and not validation explicitly + implicit: bool = False + _static: bool = PrivateAttr(False) + _deprecated: bool = PrivateAttr(False) + + @model_validator(mode="after") + def set_default_message(self) -> Self: + if self.message is None: + self.message = self.default_message + return self + + @property + def default_message(self) -> str: + return DEFAULT_VALIDATOR_MESSAGE + + +class StaticValidatorModel(ParameterValidatorModel): + _static: bool = PrivateAttr(True) + + def statically_validate(self, v: Any) -> None: ... + + +class ExpressionParameterValidatorModel(StaticValidatorModel): + """Check if a one line python expression given expression evaluates to True. + + The expression is given is the content of the validator tag.""" + + type: Literal["expression"] = "expression" + negate: Negate = NEGATE_DEFAULT + expression: Annotated[str, ValidationArgument("Python expression to validate.", xml_body=True)] + + def statically_validate(self, value: Any) -> None: + ExpressionParameterValidatorModel.expression_validation(self.expression, value, self) + + @staticmethod + def ensure_compiled(expression: Union[str, Any]) -> Any: + if isinstance(expression, str): + return compile(expression, "", "eval") + else: + return expression + + @staticmethod + def expression_validation( + expression: str, value: Any, validator: "ValidatorDescription", compiled_expression: Optional[Any] = None + ): + if compiled_expression is None: + compiled_expression = ExpressionParameterValidatorModel.ensure_compiled(expression) + message = None + try: + evalresult = eval(compiled_expression, dict(value=value)) + except Exception: + message = f"Validator '{expression}' could not be evaluated on '{value}'" + evalresult = False + + raise_error_if_valiation_fails(bool(evalresult), validator, message=message, value_to_show=value) + + @property + def default_message(self) -> str: + return f"Value '%s' does not evaluate to {'True' if not self.negate else 'False'} for '{self.expression}'" + + +class RegexParameterValidatorModel(StaticValidatorModel): + """Check if a regular expression **matches** the value, i.e. appears + at the beginning of the value. To enforce a match of the complete value use + ``$`` at the end of the expression. The expression is given is the content + of the validator tag. Note that for ``selects`` each option is checked + separately.""" + + type: Literal["regex"] = "regex" + negate: Negate = NEGATE_DEFAULT + expression: Annotated[str, ValidationArgument("Regular expression to validate against.", xml_body=True)] + + @property + def default_message(self) -> str: + return f"Value '%s' does {'not ' if not self.negate else ''}match regular expression '{self.expression.replace('%', '%%')}'" + + def statically_validate(self, value: Any) -> None: + if value and not isinstance(value, str): + raise ValueError(f"Wrong type found value {value}") + RegexParameterValidatorModel.regex_validation(self.expression, value, self) + + @staticmethod + def regex_validation(expression: str, value: Any, validator: "ValidatorDescription"): + if not isinstance(value, list): + value = [value] + for val in value: + match = regex.match(expression, val or "") + raise_error_if_valiation_fails(match is not None, validator, value_to_show=val) + + +class InRangeParameterValidatorModel(StaticValidatorModel): + type: Literal["in_range"] = "in_range" + min: Optional[Union[float, int]] = None + max: Optional[Union[float, int]] = None + exclude_min: bool = False + exclude_max: bool = False + negate: Negate = NEGATE_DEFAULT + + def statically_validate(self, value: Any): + if isinstance(value, (int, float)): + validates = True + if self.min is not None and value == self.min and self.exclude_min: + validates = False + elif self.min is not None and value < self.min: + validates = False + elif self.max is not None and value == self.max and self.exclude_max: + validates = False + if self.max is not None and value > self.max: + validates = False + raise_error_if_valiation_fails(validates, self) + + @property + def default_message(self) -> str: + op1 = "<=" + op2 = "<=" + if self.exclude_min: + op1 = "<" + if self.exclude_max: + op2 = "<" + range_description_str = f"({self.min} {op1} value {op2} {self.max})" + return f"Value ('%s') must {'not ' if self.negate else ''}fulfill {range_description_str}" + + +class LengthParameterValidatorModel(StaticValidatorModel): + type: Literal["length"] = "length" + min: Optional[int] = None + max: Optional[int] = None + negate: Negate = NEGATE_DEFAULT + + def statically_validate(self, value: Any): + if isinstance(value, str): + length = len(value) + validates = True + if self.min is not None and length < self.min: + validates = False + if self.max is not None and length > self.max: + validates = False + raise_error_if_valiation_fails(validates, self) + + @property + def default_message(self) -> str: + return f"Must {'not ' if self.negate else ''}have length of at least {self.min} and at most {self.max}" + + +class MetadataParameterValidatorModel(ParameterValidatorModel): + type: Literal["metadata"] = "metadata" + check: Optional[List[str]] = None + skip: Optional[List[str]] = None + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + check = self.check + skip = self.skip + message = DEFAULT_VALIDATOR_MESSAGE + if not self.negate: + message = "Metadata '%s' missing, click the pencil icon in the history item to edit / save the metadata attributes" + else: + if check: + message = f"""At least one of the checked metadata '{",".join(check)}' is set, click the pencil icon in the history item to edit / save the metadata attributes""" + elif skip: + message = f"""At least one of the non skipped metadata '{",".join(skip)}' is set, click the pencil icon in the history item to edit / save the metadata attributes""" + return message + + +class DatasetMetadataEqualParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_equal"] = "dataset_metadata_equal" + metadata_name: str + value: Annotated[Any, ValidationArgument("Value to test against", xml_allow_json_load=True)] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + if not self.negate: + message = f"Metadata value for '{self.metadata_name}' must be '{self.value}', but it is '%s'." + else: + message = f"Metadata value for '{self.metadata_name}' must not be '{self.value}' but it is." + return message + + +class UnspecifiedBuildParameterValidatorModel(ParameterValidatorModel): + type: Literal["unspecified_build"] = "unspecified_build" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"{'Unspecified' if not self.negate else 'Specified'} genome build, click the pencil icon in the history item to {'set' if not self.negate else 'remove'} the genome build" + + +class NoOptionsParameterValidatorModel(StaticValidatorModel): + type: Literal["no_options"] = "no_options" + negate: Negate = NEGATE_DEFAULT + + @staticmethod + def no_options_validate(value: Any, validator: "ValidatorDescription"): + raise_error_if_valiation_fails(value is not None, validator) + + def statically_validate(self, value: Any) -> None: + NoOptionsParameterValidatorModel.no_options_validate(value, self) + + @property + def default_message(self) -> str: + return f"{'No options' if not self.negate else 'Options'} available for selection" + + +class EmptyFieldParameterValidatorModel(StaticValidatorModel): + type: Literal["empty_field"] = "empty_field" + negate: Negate = NEGATE_DEFAULT + + @staticmethod + def empty_validate(value: Any, validator: "ValidatorDescription"): + raise_error_if_valiation_fails((value != ""), validator) + + def statically_validate(self, value: Any) -> None: + EmptyFieldParameterValidatorModel.empty_validate(value, self) + + @property + def default_message(self) -> str: + if not self.negate: + message = "Field requires a value" + else: + message = "Field must not set a value" + return message + + +class EmptyDatasetParameterValidatorModel(ParameterValidatorModel): + type: Literal["empty_dataset"] = "empty_dataset" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"The selected dataset is {'non-' if self.negate else ''}empty, this tool expects {'non-' if not self.negate else ''}empty files." + + +class EmptyExtraFilesPathParameterValidatorModel(ParameterValidatorModel): + type: Literal["empty_extra_files_path"] = "empty_extra_files_path" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + negate = self.negate + return f"The selected dataset's extra_files_path directory is {'non-' if negate else ''}empty or does {'not ' if not negate else ''}exist, this tool expects {'non-' if not negate else ''}empty extra_files_path directories associated with the selected input." + + +class DatasetMetadataInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_in_data_table"] = "dataset_metadata_in_data_table" + table_name: str + metadata_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"Value for metadata {self.metadata_name} was not found in {self.table_name}." + + +class DatasetMetadataNotInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_not_in_data_table"] = "dataset_metadata_not_in_data_table" + table_name: str + metadata_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"Value for metadata {self.metadata_name} was not found in {self.table_name}." + + +class DatasetMetadataInRangeParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_in_range"] = "dataset_metadata_in_range" + metadata_name: str + min: Optional[Union[float, int]] = None + max: Optional[Union[float, int]] = None + exclude_min: bool = False + exclude_max: bool = False + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + op1 = "<=" + op2 = "<=" + if self.exclude_min: + op1 = "<" + if self.exclude_max: + op2 = "<" + range_description_str = f"({self.min} {op1} value {op2} {self.max})" + return f"Value ('%s') must {'not ' if self.negate else ''}fulfill {range_description_str}" + + +class ValueInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["value_in_data_table"] = "value_in_data_table" + table_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return "Value for metadata not found." + + +class ValueNotInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["value_not_in_data_table"] = "value_not_in_data_table" + table_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"Value was not found in {self.table_name}." + + +class DatasetOkValidatorParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_ok_validator"] = "dataset_ok_validator" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + if not self.negate: + message = ( + "The selected dataset is still being generated, select another dataset or wait until it is completed" + ) + else: + message = "The selected dataset must not be in state OK" + return message + + +class DatasetMetadataInFileParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_in_file"] = "dataset_metadata_in_file" + filename: str + metadata_name: str + metadata_column: Union[int, str] + line_startswith: Optional[str] = None + split: str = SPLIT_DEFAULT + negate: Negate = NEGATE_DEFAULT + _deprecated: bool = PrivateAttr(True) + + @property + def default_message(self) -> str: + return f"Value for metadata {self.metadata_name} was not found in {self.filename}." + + +AnyValidatorModel = Annotated[ + Union[ + ExpressionParameterValidatorModel, + RegexParameterValidatorModel, + InRangeParameterValidatorModel, + LengthParameterValidatorModel, + MetadataParameterValidatorModel, + DatasetMetadataEqualParameterValidatorModel, + UnspecifiedBuildParameterValidatorModel, + NoOptionsParameterValidatorModel, + EmptyFieldParameterValidatorModel, + EmptyDatasetParameterValidatorModel, + EmptyExtraFilesPathParameterValidatorModel, + DatasetMetadataInDataTableParameterValidatorModel, + DatasetMetadataNotInDataTableParameterValidatorModel, + DatasetMetadataInRangeParameterValidatorModel, + ValueInDataTableParameterValidatorModel, + ValueNotInDataTableParameterValidatorModel, + DatasetOkValidatorParameterValidatorModel, + DatasetMetadataInFileParameterValidatorModel, + ], + Field(discriminator="type"), +] + + +def parse_xml_validators(input_elem: Element) -> List[AnyValidatorModel]: + validator_els: List[Element] = input_elem.findall("validator") or [] + models = [] + for validator_el in validator_els: + models.append(parse_xml_validator(validator_el)) + return models + + +def static_validators(validator_models: List[AnyValidatorModel]) -> List[AnyValidatorModel]: + static_validators = [] + for validator_model in validator_models: + print(validator_model._static) + if validator_model._static: + static_validators.append(validator_model) + return static_validators + + +def parse_xml_validator(validator_el: Element) -> AnyValidatorModel: + _type = validator_el.get("type") + if _type is None: + raise ValueError("Required 'type' attribute missing from validator") + valid_types = get_args(ValidatorType) + if _type not in valid_types: + raise ValueError(f"Unknown 'type' attribute in validator {_type}") + validator_type: ValidatorType = cast(ValidatorType, _type) + if validator_type == "expression": + return ExpressionParameterValidatorModel( + type="expression", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + expression=validator_el.text, + ) + elif validator_type == "regex": + return RegexParameterValidatorModel( + type="regex", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + expression=validator_el.text, + ) + elif validator_type == "in_range": + return InRangeParameterValidatorModel( + type="in_range", + message=_parse_message(validator_el), + min=_parse_number(validator_el, "min"), + max=_parse_number(validator_el, "max"), + exclude_min=_parse_bool(validator_el, "exclude_min", False), + exclude_max=_parse_bool(validator_el, "exclude_max", False), + negate=_parse_negate(validator_el), + ) + elif validator_type == "length": + return LengthParameterValidatorModel( + type="length", + min=_parse_int(validator_el, "min"), + max=_parse_int(validator_el, "max"), + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "metadata": + return MetadataParameterValidatorModel( + type="metadata", + message=_parse_message(validator_el), + check=_parse_str_list(validator_el, "check"), + skip=_parse_str_list(validator_el, "skip"), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_equal": + return DatasetMetadataEqualParameterValidatorModel( + type="dataset_metadata_equal", + metadata_name=validator_el.get("metadata_name"), + value=_parse_json_value(validator_el), + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "unspecified_build": + return UnspecifiedBuildParameterValidatorModel( + type="unspecified_build", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "no_options": + return NoOptionsParameterValidatorModel( + type="no_options", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "empty_field": + return EmptyFieldParameterValidatorModel( + type="empty_field", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "empty_dataset": + return EmptyDatasetParameterValidatorModel( + type="empty_dataset", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "empty_extra_files_path": + return EmptyExtraFilesPathParameterValidatorModel( + type="empty_extra_files_path", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_in_data_table": + return DatasetMetadataInDataTableParameterValidatorModel( + type="dataset_metadata_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_name=validator_el.get("metadata_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_not_in_data_table": + return DatasetMetadataNotInDataTableParameterValidatorModel( + type="dataset_metadata_not_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_name=validator_el.get("metadata_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_in_range": + return DatasetMetadataInRangeParameterValidatorModel( + type="dataset_metadata_in_range", + message=_parse_message(validator_el), + metadata_name=validator_el.get("metadata_name"), + min=_parse_number(validator_el, "min"), + max=_parse_number(validator_el, "max"), + exclude_min=_parse_bool(validator_el, "exclude_min", False), + exclude_max=_parse_bool(validator_el, "exclude_max", False), + negate=_parse_negate(validator_el), + ) + elif validator_type == "value_in_data_table": + return ValueInDataTableParameterValidatorModel( + type="value_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "value_not_in_data_table": + return ValueNotInDataTableParameterValidatorModel( + type="value_not_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_ok_validator": + return DatasetOkValidatorParameterValidatorModel( + type="dataset_ok_validator", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_in_file": + filename = validator_el.get("filename") + return DatasetMetadataInFileParameterValidatorModel( + type="dataset_metadata_in_file", + message=_parse_message(validator_el), + filename=filename, + metadata_name=validator_el.get("metadata_name"), + metadata_column=_parse_metadata_column(validator_el), + line_startswith=validator_el.get("line_startswith"), + split=validator_el.get("split", SPLIT_DEFAULT), + negate=_parse_negate(validator_el), + ) + else: + raise ValueError(f"Unhandled 'type' attribute in validator {validator_type}") + + +class ValidatorDescription(Protocol): + + @property + def negate(self) -> bool: ... + + @property + def message(self) -> Optional[str]: ... + + +def raise_error_if_valiation_fails( + value: bool, validator: ValidatorDescription, message: Optional[str] = None, value_to_show: Optional[str] = None +): + if not isinstance(value, bool): + raise AssertionError("Validator logic problem - computed validation value must be boolean") + if message is None: + message = validator.message + if message is None: + message = DEFAULT_VALIDATOR_MESSAGE + assert message + if value_to_show and "%s" in message: + message = message % value_to_show + negate = validator.negate + if (not negate and value) or (negate and not value): + return + else: + raise ValueError(message) + + +def _parse_message(xml_el: Element) -> Optional[str]: + message = cast(Optional[str], xml_el.get("message")) + return message + + +def _parse_int(xml_el: Element, attribute: str) -> Optional[int]: + raw_value = xml_el.get(attribute) + if raw_value: + return int(raw_value) + else: + return None + + +def _parse_number(xml_el: Element, attribute: str) -> Optional[Union[float, int]]: + raw_value = xml_el.get(attribute) + if raw_value and ("." in raw_value or "e" in raw_value): + return float(raw_value) + elif raw_value: + return int(raw_value) + else: + return None + + +def _parse_negate(xml_el: Element) -> bool: + return _parse_bool(xml_el, "negate", False) + + +def _parse_bool(xml_el: Element, attribute: str, default_value: bool) -> bool: + return asbool(xml_el.get(attribute, default_value)) + + +def _parse_str_list(xml_el: Element, attribute: str) -> List[str]: + raw_value = xml_el.get(attribute) + if not raw_value: + return [] + else: + return [v.strip() for v in raw_value.split(",")] + + +def _parse_json_value(xml_el: Element) -> Any: + value = xml_el.get("value", None) or json.loads(xml_el.get("value_json", "null")) + return value + + +def _parse_metadata_column(xml_el: Element) -> Union[int, str]: + column = xml_el.get("metadata_column", 0) + try: + return int(column) + except ValueError: + return column + + +def static_tool_validators(validators: Sequence[ParameterValidatorModel]) -> List[StaticValidatorModel]: + static_validators: List[StaticValidatorModel] = [] + for validator in validators: + if isinstance(validator, StaticValidatorModel): + static_validators.append(validator) + return static_validators + + +def statically_validates(validators: Sequence[ParameterValidatorModel], value: Any) -> bool: + for validator in static_tool_validators(validators): + try: + validator.statically_validate(value) + except ValueError: + return False + return True diff --git a/lib/galaxy/tool_util/parser/util.py b/lib/galaxy/tool_util/parser/util.py index 53009057128e..34f3e0943eea 100644 --- a/lib/galaxy/tool_util/parser/util.py +++ b/lib/galaxy/tool_util/parser/util.py @@ -8,6 +8,9 @@ from packaging.version import Version +from galaxy.util import string_as_bool +from .parameter_validators import statically_validates + if TYPE_CHECKING: from .interface import ( InputSource, @@ -83,6 +86,27 @@ def boolean_true_and_false_values(input_source, profile: Optional[Union[float, s return (truevalue, falsevalue) +def text_input_is_optional(input_source: "InputSource") -> Tuple[bool, bool]: + # Optionality not explicitly defined, default to False + optional: Optional[bool] = False + optionality_inferred: bool = False + + optional = input_source.get("optional", None) + if optional is not None: + optional = string_as_bool(optional) + else: + # A text parameter that doesn't raise a validation error on empty string + # is considered to be optional + if statically_validates(input_source.parse_validators(), ""): + optional = True + optionality_inferred = True + else: + optional = False + + assert isinstance(optional, bool) + return optional, optionality_inferred + + class ParameterParseException(Exception): message: str diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index b94cf7a84457..06a25c78f29b 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -71,6 +71,10 @@ ToolOutputCollection, ToolOutputCollectionStructure, ) +from .parameter_validators import ( + AnyValidatorModel, + parse_xml_validators, +) from .stdio import ( aggressive_error_checks, error_on_exit_code, @@ -1339,8 +1343,8 @@ def parse_help(self): def parse_sanitizer_elem(self): return self.input_elem.find("sanitizer") - def parse_validator_elems(self): - return self.input_elem.findall("validator") + def parse_validators(self) -> List[AnyValidatorModel]: + return parse_xml_validators(self.input_elem) def parse_dynamic_options(self) -> Optional[XmlDynamicOptions]: """Return a XmlDynamicOptions to describe dynamic options if options elem is available.""" diff --git a/lib/galaxy/tool_util/unittest_utils/sample_data.py b/lib/galaxy/tool_util/unittest_utils/sample_data.py index d4b6ddb6f027..e9b19283401a 100644 --- a/lib/galaxy/tool_util/unittest_utils/sample_data.py +++ b/lib/galaxy/tool_util/unittest_utils/sample_data.py @@ -17,3 +17,56 @@ """ ) + +VALID_XML_VALIDATORS = [ + """""", + """""", + """""", + """value == 7""", + """mycoolexpression""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", +] + +INVALID_XML_VALIDATORS = [ + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""" + """""" + """""", + """""", + """""", + """""", +] diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 5aa07efdd204..4860f97b5d51 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -47,6 +47,7 @@ boolean_is_checked, boolean_true_and_false_values, ParameterParseException, + text_input_is_optional, ) from galaxy.tools.parameters.workflow_utils import workflow_building_modes from galaxy.util import ( @@ -194,9 +195,7 @@ def __init__(self, tool, input_source, context=None): self.sanitizer = ToolParameterSanitizer.from_element(sanitizer_elem) else: self.sanitizer = None - self.validators = [] - for elem in input_source.parse_validator_elems(): - self.validators.append(validation.Validator.from_element(self, elem)) + self.validators = validation.to_validators(tool.app if tool else None, input_source.parse_validators()) @property def visible(self) -> bool: @@ -351,7 +350,6 @@ def parse_name(input_source): class SimpleTextToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) - self.optionality_inferred = False super().__init__(tool, input_source) optional = input_source.get("optional", None) if optional is not None: @@ -359,18 +357,7 @@ def __init__(self, tool, input_source): else: # Optionality not explicitly defined, default to False optional = False - if self.type == "text": - # A text parameter that doesn't raise a validation error on empty string - # is considered to be optional - try: - for validator in self.validators: - validator.validate("") - optional = True - self.optionality_inferred = True - except ValueError: - pass self.optional = optional - if self.optional: self.value = None else: @@ -405,10 +392,16 @@ class TextToolParameter(SimpleTextToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) - self.profile = tool.profile + self.profile = tool.profile if tool else None self.datalist = [] for title, value, _ in input_source.parse_static_options(): self.datalist.append({"label": title, "value": value}) + + # why does Integer and Float subclass this :_( + if self.type == "text": + self.optional, self.optionality_inferred = text_input_is_optional(input_source) + else: + self.optionality_inferred = False self.value = input_source.get("value") self.area = input_source.get_bool("area", False) @@ -422,10 +415,10 @@ def validate(self, value, trans=None): return super().validate(value, trans) @property - def wrapper_default() -> Optional[str]: + def wrapper_default(self) -> Optional[str]: """Handle change in default handling pre and post 23.0 profiles.""" profile = self.profile - legacy_behavior = (profile is None or Version(str(profile)) < Version("23.0")) + legacy_behavior = profile is None or Version(str(profile)) < Version("23.0") default_value = None if self.optional and self.optionality_inferred and legacy_behavior: default_value = "" @@ -478,7 +471,7 @@ def __init__(self, tool, input_source): except ValueError: raise ParameterValueError("attribute 'max' must be an integer", self.name, self.max) if self.min is not None or self.max is not None: - self.validators.append(validation.InRangeValidator(None, self.min, self.max)) + self.validators.append(validation.InRangeValidator.simple_range_validator(self.min, self.max)) def from_json(self, value, trans, other_values=None): other_values = other_values or {} @@ -551,7 +544,7 @@ def __init__(self, tool, input_source): except ValueError: raise ParameterValueError("attribute 'max' must be a real number", self.name, self.max) if self.min is not None or self.max is not None: - self.validators.append(validation.InRangeValidator(None, self.min, self.max)) + self.validators.append(validation.InRangeValidator.simple_range_validator(self.min, self.max)) def from_json(self, value, trans, other_values=None): other_values = other_values or {} @@ -2056,7 +2049,7 @@ def __init__(self, tool, input_source, trans=None): self.load_contents = int(input_source.get("load_contents", 0)) # Add metadata validator if not input_source.get_bool("no_validation", False): - self.validators.append(validation.MetadataValidator()) + self.validators.append(validation.MetadataValidator.default_metadata_validator()) self._parse_formats(trans, input_source) tag = input_source.get("tag") self.multiple = input_source.get_bool("multiple", False) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index e354c93eabb5..8d9488128a03 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -621,8 +621,9 @@ def load_from_parameter(from_parameter, transform_lines=None): self.filters.append(Filter.from_element(self, filter_elem)) # Load Validators - for validator in elem.findall("validator"): - self.validators.append(validation.Validator.from_element(self.tool_param, validator)) + validators = validation.parse_xml_validators(self.tool_param.tool.app, elem) + if validators: + self.validators = validators if self.dataset_ref_name: tool_param.data_ref = self.dataset_ref_name diff --git a/lib/galaxy/tools/parameters/validation.py b/lib/galaxy/tools/parameters/validation.py index 6334fd95f8b8..895cd1dfadef 100644 --- a/lib/galaxy/tools/parameters/validation.py +++ b/lib/galaxy/tools/parameters/validation.py @@ -3,16 +3,30 @@ """ import abc -import json import logging -import os.path - -import regex +import os +from typing import ( + Any, + cast, + List, + Optional, + Union, +) from galaxy import ( model, util, ) +from galaxy.tool_util.parser.parameter_validators import ( + AnyValidatorModel, + EmptyFieldParameterValidatorModel, + ExpressionParameterValidatorModel, + InRangeParameterValidatorModel, + MetadataParameterValidatorModel, + parse_xml_validators as parse_xml_validators_models, + raise_error_if_valiation_fails, + RegexParameterValidatorModel, +) log = logging.getLogger(__name__) @@ -24,27 +38,7 @@ class Validator(abc.ABC): requires_dataset_metadata = False - @classmethod - def from_element(cls, param, elem): - """ - Initialize the appropriate Validator class - - example call `validation.Validator.from_element(ToolParameter_object, Validator_object)` - - needs to be implemented in the subclasses and should return the - corresponding Validator object by a call to `cls( ... )` which calls the - `__init__` method of the corresponding validator - - param cls the Validator class - param param the element to be evaluated (which contains the validator) - param elem the validator element - return an object of a Validator subclass that corresponds to the type attribute of the validator element - """ - _type = elem.get("type") - assert _type is not None, "Required 'type' attribute missing from validator" - return validator_types[_type].from_element(param, elem) - - def __init__(self, message, negate=False): + def __init__(self, message: str, negate: bool = False): self.message = message self.negate = util.asbool(negate) super().__init__() @@ -68,15 +62,7 @@ def validate(self, value, trans=None, message=None, value_to_show=None): return None if positive validation, otherwise a ValueError is raised """ - assert isinstance(value, bool), "value must be boolean" - if message is None: - message = self.message - if value_to_show and "%s" in message: - message = message % value_to_show - if (not self.negate and value) or (self.negate and not value): - return - else: - raise ValueError(message) + raise_error_if_valiation_fails(value, self, message=message, value_to_show=value_to_show) class RegexValidator(Validator): @@ -84,24 +70,14 @@ class RegexValidator(Validator): Validator that evaluates a regular expression """ - @classmethod - def from_element(cls, param, elem): - return cls(elem.get("message"), elem.text, elem.get("negate", "false")) - - def __init__(self, message, expression, negate): - if message is None: - message = f"Value '%s' does {'not ' if negate == 'false' else ''}match regular expression '{expression.replace('%', '%%')}'" + def __init__(self, message: str, expression: str, negate: bool): super().__init__(message, negate) # Compile later. RE objects used to not be thread safe. Not sure about # the sre module. self.expression = expression def validate(self, value, trans=None): - if not isinstance(value, list): - value = [value] - for val in value: - match = regex.match(self.expression, val or "") - super().validate(match is not None, value_to_show=val) + RegexParameterValidatorModel.regex_validation(self.expression, value, self) class ExpressionValidator(Validator): @@ -109,24 +85,16 @@ class ExpressionValidator(Validator): Validator that evaluates a python expression using the value """ - @classmethod - def from_element(cls, param, elem): - return cls(elem.get("message"), elem.text, elem.get("negate", "false")) - - def __init__(self, message, expression, negate): - if message is None: - message = f"Value '%s' does not evaluate to {'True' if negate == 'false' else 'False'} for '{expression}'" + def __init__(self, message: str, expression: str, negate: bool): super().__init__(message, negate) self.expression = expression # Save compiled expression, code objects are thread safe (right?) - self.compiled_expression = compile(expression, "", "eval") + self.compiled_expression = ExpressionParameterValidatorModel.ensure_compiled(expression) def validate(self, value, trans=None): - try: - evalresult = eval(self.compiled_expression, dict(value=value)) - except Exception: - super().validate(False, message=f"Validator '{self.expression}' could not be evaluated on '{value}'") - super().validate(bool(evalresult), value_to_show=value) + ExpressionParameterValidatorModel.expression_validation( + self.expression, value, self, compiled_expression=self.compiled_expression + ) class InRangeValidator(ExpressionValidator): @@ -134,18 +102,15 @@ class InRangeValidator(ExpressionValidator): Validator that ensures a number is in a specified range """ - @classmethod - def from_element(cls, param, elem): - return cls( - elem.get("message"), - elem.get("min"), - elem.get("max"), - elem.get("exclude_min", "false"), - elem.get("exclude_max", "false"), - elem.get("negate", "false"), - ) - - def __init__(self, message, range_min, range_max, exclude_min=False, exclude_max=False, negate=False): + def __init__( + self, + message: str, + min: Optional[float] = None, + max: Optional[float] = None, + exclude_min: bool = False, + exclude_max: bool = False, + negate: bool = False, + ): """ When the optional exclude_min and exclude_max attributes are set to true, the range excludes the end points (i.e., min < value < max), @@ -153,10 +118,10 @@ def __init__(self, message, range_min, range_max, exclude_min=False, exclude_max (1.e., min <= value <= max). Combinations of exclude_min and exclude_max values are allowed. """ - self.min = range_min if range_min is not None else "-inf" - self.exclude_min = util.asbool(exclude_min) - self.max = range_max if range_max is not None else "inf" - self.exclude_max = util.asbool(exclude_max) + self.min = str(min) if min is not None else "-inf" + self.exclude_min = exclude_min + self.max = str(max) if max is not None else "inf" + self.exclude_max = exclude_max assert float(self.min) <= float(self.max), "min must be less than or equal to max" # Remove unneeded 0s and decimal from floats to make message pretty. op1 = "<=" @@ -166,24 +131,23 @@ def __init__(self, message, range_min, range_max, exclude_min=False, exclude_max if self.exclude_max: op2 = "<" expression = f"float('{self.min}') {op1} float(value) {op2} float('{self.max}')" - if message is None: - message = f"Value ('%s') must {'not ' if negate == 'true' else ''}fulfill {expression}" super().__init__(message, expression, negate) + @staticmethod + def simple_range_validator(min: Optional[float], max: Optional[float]): + return cast( + InRangeParameterValidatorModel, + _to_validator(None, InRangeParameterValidatorModel(min=min, max=max, implicit=True)), + ) + class LengthValidator(InRangeValidator): """ Validator that ensures the length of the provided string (value) is in a specific range """ - @classmethod - def from_element(cls, param, elem): - return cls(elem.get("message"), elem.get("min"), elem.get("max"), elem.get("negate", "false")) - - def __init__(self, message, length_min, length_max, negate): - if message is None: - message = f"Must {'not ' if negate == 'true' else ''}have length of at least {length_min} and at most {length_max}" - super().__init__(message, range_min=length_min, range_max=length_max, negate=negate) + def __init__(self, message: str, min: float, max: float, negate: bool): + super().__init__(message, min=min, max=max, negate=negate) def validate(self, value, trans=None): if value is None: @@ -196,16 +160,8 @@ class DatasetOkValidator(Validator): Validator that checks if a dataset is in an 'ok' state """ - @classmethod - def from_element(cls, param, elem): - negate = elem.get("negate", "false") - message = elem.get("message") - if message is None: - if negate == "false": - message = "The selected dataset is still being generated, select another dataset or wait until it is completed" - else: - message = "The selected dataset must not be in state OK" - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): if value: @@ -217,13 +173,8 @@ class DatasetEmptyValidator(Validator): Validator that checks if a dataset has a positive file size. """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"The selected dataset is {'non-' if negate == 'true' else ''}empty, this tool expects {'non-' if negate == 'false' else ''}empty files." - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): if value: @@ -235,13 +186,8 @@ class DatasetExtraFilesPathEmptyValidator(Validator): Validator that checks if a dataset's extra_files_path exists and is not empty. """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"The selected dataset's extra_files_path directory is {'non-' if negate == 'true' else ''}empty or does {'not ' if negate == 'false' else ''}exist, this tool expects {'non-' if negate == 'false' else ''}empty extra_files_path directories associated with the selected input." - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): if value: @@ -255,25 +201,20 @@ class MetadataValidator(Validator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - return cls( - message=message, check=elem.get("check", ""), skip=elem.get("skip", ""), negate=elem.get("negate", "false") - ) - - def __init__(self, message=None, check="", skip="", negate="false"): - if not message: - if not util.asbool(negate): - message = "Metadata '%s' missing, click the pencil icon in the history item to edit / save the metadata attributes" - else: - if check != "": - message = f"At least one of the checked metadata '{check}' is set, click the pencil icon in the history item to edit / save the metadata attributes" - elif skip != "": - message = f"At least one of the non skipped metadata '{skip}' is set, click the pencil icon in the history item to edit / save the metadata attributes" + def __init__( + self, + message: str, + check: Optional[List[str]] = None, + skip: Optional[List[str]] = None, + negate: bool = False, + ): super().__init__(message, negate) - self.check = check.split(",") if check else None - self.skip = skip.split(",") if skip else None + self.check = check + self.skip = skip + + @staticmethod + def default_metadata_validator() -> "MetadataValidator": + return cast(MetadataValidator, _to_validator(None, MetadataParameterValidatorModel(implicit=True))) def validate(self, value, trans=None): if value: @@ -293,25 +234,10 @@ class MetadataEqualValidator(Validator): requires_dataset_metadata = True def __init__(self, metadata_name=None, value=None, message=None, negate="false"): - if not message: - if not util.asbool(negate): - message = f"Metadata value for '{metadata_name}' must be '{value}', but it is '%s'." - else: - message = f"Metadata value for '{metadata_name}' must not be '{value}' but it is." super().__init__(message, negate) self.metadata_name = metadata_name self.value = value - @classmethod - def from_element(cls, param, elem): - value = elem.get("value", None) or json.loads(elem.get("value_json", "null")) - return cls( - metadata_name=elem.get("metadata_name", None), - value=value, - message=elem.get("message", None), - negate=elem.get("negate", "false"), - ) - def validate(self, value, trans=None): if value: metadata_value = getattr(value.metadata, self.metadata_name) @@ -325,13 +251,8 @@ class UnspecifiedBuildValidator(Validator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"{'Unspecified' if negate == 'false' else 'Specified'} genome build, click the pencil icon in the history item to {'set' if negate == 'false' else 'remove'} the genome build" - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): # if value is None, we cannot validate @@ -348,13 +269,8 @@ class NoOptionsValidator(Validator): Validator that checks for empty select list """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"{'No options' if negate == 'false' else 'Options'} available for selection" - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): super().validate(value is not None) @@ -365,19 +281,11 @@ class EmptyTextfieldValidator(Validator): Validator that checks for empty text field """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - if negate == "false": - message = elem.get("message", "Field requires a value") - else: - message = elem.get("message", "Field must not set a value") - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): - super().validate(value != "") + EmptyFieldParameterValidatorModel.empty_validate(value, self) class MetadataInFileColumnValidator(Validator): @@ -391,35 +299,19 @@ class MetadataInFileColumnValidator(Validator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - filename = elem.get("filename") - assert filename, f"Required 'filename' attribute missing from {elem.get('type')} validator." - filename = f"{param.tool.app.config.tool_data_path}/{filename.strip()}" - assert os.path.exists(filename), f"File {filename} specified by the 'filename' attribute not found" - metadata_name = elem.get("metadata_name") - assert metadata_name, f"Required 'metadata_name' attribute missing from {elem.get('type')} validator." - metadata_name = metadata_name.strip() - metadata_column = int(elem.get("metadata_column", 0)) - split = elem.get("split", "\t") - message = elem.get("message", f"Value for metadata {metadata_name} was not found in {filename}.") - line_startswith = elem.get("line_startswith") - if line_startswith: - line_startswith = line_startswith.strip() - negate = elem.get("negate", "false") - return cls(filename, metadata_name, metadata_column, message, line_startswith, split, negate) - def __init__( self, - filename, - metadata_name, - metadata_column, - message="Value for metadata not found.", - line_startswith=None, - split="\t", - negate="false", + filename: str, + metadata_name: str, + metadata_column: int, + message: str, + line_startswith: Optional[str] = None, + split: str = "\t", + negate: bool = False, ): super().__init__(message, negate) + assert filename + assert os.path.exists(filename), f"File {filename} specified by the 'filename' attribute not found" self.metadata_name = metadata_name self.valid_values = set() with open(filename) as fh: @@ -445,28 +337,20 @@ class ValueInDataTableColumnValidator(Validator): note: this is covered in a framework test (validation_value_in_datatable) """ - @classmethod - def from_element(cls, param, elem): - table_name = elem.get("table_name") - assert table_name, f"Required 'table_name' attribute missing from {elem.get('type')} validator." - tool_data_table = param.tool.app.tool_data_tables[table_name] - column = elem.get("metadata_column", 0) - try: - column = int(column) - except ValueError: - pass - message = elem.get("message", f"Value was not found in {table_name}.") - negate = elem.get("negate", "false") - return cls(tool_data_table, column, message, negate) - - def __init__(self, tool_data_table, column, message="Value not found.", negate="false"): + def __init__( + self, + tool_data_table, + metadata_column: Union[str, int], + message: str, + negate: bool = False, + ): super().__init__(message, negate) - self.valid_values = [] + self.valid_values: List[Any] = [] self._data_table_content_version = None self._tool_data_table = tool_data_table - if isinstance(column, str): - column = tool_data_table.columns[column] - self._column = column + if isinstance(metadata_column, str): + metadata_column = tool_data_table.columns[metadata_column] + self._column = metadata_column self._load_values() def _load_values(self): @@ -496,7 +380,9 @@ class ValueNotInDataTableColumnValidator(ValueInDataTableColumnValidator): note: this is covered in a framework test (validation_value_in_datatable) """ - def __init__(self, tool_data_table, metadata_column, message="Value already present.", negate="false"): + def __init__( + self, tool_data_table, metadata_column: Union[str, int], message="Value already present.", negate="false" + ): super().__init__(tool_data_table, metadata_column, message, negate) def validate(self, value, trans=None): @@ -517,26 +403,13 @@ class MetadataInDataTableColumnValidator(ValueInDataTableColumnValidator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - table_name = elem.get("table_name") - assert table_name, f"Required 'table_name' attribute missing from {elem.get('type')} validator." - tool_data_table = param.tool.app.tool_data_tables[table_name] - metadata_name = elem.get("metadata_name") - assert metadata_name, f"Required 'metadata_name' attribute missing from {elem.get('type')} validator." - metadata_name = metadata_name.strip() - # TODO rename to column? - metadata_column = elem.get("metadata_column", 0) - try: - metadata_column = int(metadata_column) - except ValueError: - pass - message = elem.get("message", f"Value for metadata {metadata_name} was not found in {table_name}.") - negate = elem.get("negate", "false") - return cls(tool_data_table, metadata_name, metadata_column, message, negate) - def __init__( - self, tool_data_table, metadata_name, metadata_column, message="Value for metadata not found.", negate="false" + self, + tool_data_table, + metadata_name: str, + metadata_column: Union[str, int], + message: str, + negate: bool = False, ): super().__init__(tool_data_table, metadata_column, message, negate) self.metadata_name = metadata_name @@ -558,7 +431,12 @@ class MetadataNotInDataTableColumnValidator(MetadataInDataTableColumnValidator): requires_dataset_metadata = True def __init__( - self, tool_data_table, metadata_name, metadata_column, message="Value for metadata not found.", negate="false" + self, + tool_data_table, + metadata_name: str, + metadata_column: Union[str, int], + message: str, + negate: bool = False, ): super().__init__(tool_data_table, metadata_name, metadata_column, message, negate) @@ -580,26 +458,18 @@ class MetadataInRangeValidator(InRangeValidator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - metadata_name = elem.get("metadata_name") - assert metadata_name, f"Required 'metadata_name' attribute missing from {elem.get('type')} validator." - metadata_name = metadata_name.strip() - ret = cls( - metadata_name, - elem.get("message"), - elem.get("min"), - elem.get("max"), - elem.get("exclude_min", "false"), - elem.get("exclude_max", "false"), - elem.get("negate", "false"), - ) - ret.message = "Metadata: " + ret.message - return ret - - def __init__(self, metadata_name, message, range_min, range_max, exclude_min, exclude_max, negate): + def __init__( + self, + metadata_name: str, + message: str, + min: Optional[float] = None, + max: Optional[float] = None, + exclude_min: bool = False, + exclude_max: bool = False, + negate: bool = False, + ): self.metadata_name = metadata_name - super().__init__(message, range_min, range_max, exclude_min, exclude_max, negate) + super().__init__(message, min, max, exclude_min, exclude_max, negate) def validate(self, value, trans=None): if value: @@ -638,3 +508,29 @@ def validate(self, value, trans=None): deprecated_validator_types = dict(dataset_metadata_in_file=MetadataInFileColumnValidator) validator_types.update(deprecated_validator_types) + + +def parse_xml_validators(app, xml_el: util.Element) -> List[Validator]: + return to_validators(app, parse_xml_validators_models(xml_el)) + + +def to_validators(app, validator_models: List[AnyValidatorModel]) -> List[Validator]: + validators = [] + for validator_model in validator_models: + validators.append(_to_validator(app, validator_model)) + return validators + + +def _to_validator(app, validator_model: AnyValidatorModel) -> Validator: + as_dict = validator_model.model_dump() + validator_type = as_dict.pop("type") + del as_dict["implicit"] + if "table_name" in as_dict and app is not None: + table_name = as_dict.pop("table_name") + tool_data_table = app.tool_data_tables[table_name] + as_dict["tool_data_table"] = tool_data_table + if "filename" in as_dict and app is not None: + filename = as_dict.pop("filename") + as_dict["filename"] = f"{app.config.tool_data_path}/{filename}" + + return validator_types[validator_type](**as_dict) diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 34e5b84737e2..6dd47e7f9bb4 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -130,7 +130,7 @@ def __init__( self.input = input if value is None and input.type == "text": # Tools with old profile versions may treat an optional text parameter as `""` - value = cast(TextToolParameter, input).wrapper_default() + value = cast(TextToolParameter, input).wrapper_default self.value = value self._other_values: Dict[str, str] = other_values or {} diff --git a/test/functional/tools/parameters/gx_directory_uri_validation.xml b/test/functional/tools/parameters/gx_directory_uri_validation.xml new file mode 100644 index 000000000000..7375c298b711 --- /dev/null +++ b/test/functional/tools/parameters/gx_directory_uri_validation.xml @@ -0,0 +1,16 @@ + + + macros.xml + + $output + ]]> + + + + 'api' in value + ^.*json$ + + + + diff --git a/test/functional/tools/parameters/gx_float_min_max.xml b/test/functional/tools/parameters/gx_float_min_max.xml new file mode 100644 index 000000000000..55a4d6fb0881 --- /dev/null +++ b/test/functional/tools/parameters/gx_float_min_max.xml @@ -0,0 +1,14 @@ + + > '$output' + ]]> + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_float_validation_range.xml b/test/functional/tools/parameters/gx_float_validation_range.xml new file mode 100644 index 000000000000..8988e92506df --- /dev/null +++ b/test/functional/tools/parameters/gx_float_validation_range.xml @@ -0,0 +1,15 @@ + + > '$output' + ]]> + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_hidden_validation.xml b/test/functional/tools/parameters/gx_hidden_validation.xml new file mode 100644 index 000000000000..e40a67aae964 --- /dev/null +++ b/test/functional/tools/parameters/gx_hidden_validation.xml @@ -0,0 +1,17 @@ + + > '$output' + ]]> + + + + 'api' in value + ^.*json$ + + + + + + + + diff --git a/test/functional/tools/parameters/gx_int_min_max.xml b/test/functional/tools/parameters/gx_int_min_max.xml new file mode 100644 index 000000000000..c826948d746c --- /dev/null +++ b/test/functional/tools/parameters/gx_int_min_max.xml @@ -0,0 +1,14 @@ + + > '$output' + ]]> + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_int_validation_range.xml b/test/functional/tools/parameters/gx_int_validation_range.xml new file mode 100644 index 000000000000..5f7862497abf --- /dev/null +++ b/test/functional/tools/parameters/gx_int_validation_range.xml @@ -0,0 +1,15 @@ + + > '$output' + ]]> + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_expression_validation.xml b/test/functional/tools/parameters/gx_text_expression_validation.xml new file mode 100644 index 000000000000..fe5113ec8fef --- /dev/null +++ b/test/functional/tools/parameters/gx_text_expression_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + 'foobar' in value + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_length_validation.xml b/test/functional/tools/parameters/gx_text_length_validation.xml new file mode 100644 index 000000000000..4d5434aa6c58 --- /dev/null +++ b/test/functional/tools/parameters/gx_text_length_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_length_validation_negate.xml b/test/functional/tools/parameters/gx_text_length_validation_negate.xml new file mode 100644 index 000000000000..4b0b835fe562 --- /dev/null +++ b/test/functional/tools/parameters/gx_text_length_validation_negate.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_regex_validation.xml b/test/functional/tools/parameters/gx_text_regex_validation.xml new file mode 100644 index 000000000000..2e1bd7325d8a --- /dev/null +++ b/test/functional/tools/parameters/gx_text_regex_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + ^[actg]*$ + + + + + + + + + diff --git a/test/unit/app/tools/test_dynamic_options.py b/test/unit/app/tools/test_dynamic_options.py index 38626d65b991..77d9eb263ecf 100644 --- a/test/unit/app/tools/test_dynamic_options.py +++ b/test/unit/app/tools/test_dynamic_options.py @@ -6,6 +6,10 @@ def get_from_url_option(): + tool_param = Bunch() + tool_param.tool = Bunch() + tool_param.tool.app = Bunch() + return DynamicOptions( XML( """ @@ -26,7 +30,7 @@ def get_from_url_option(): """ ), - Bunch(), + tool_param, ) diff --git a/test/unit/app/tools/test_parameter_validation.py b/test/unit/app/tools/test_parameter_validation.py index a7798927eb65..4b69034fac4b 100644 --- a/test/unit/app/tools/test_parameter_validation.py +++ b/test/unit/app/tools/test_parameter_validation.py @@ -220,12 +220,12 @@ def test_InRangeValidator(self): p.validate(10) with self.assertRaisesRegex( ValueError, - r"Parameter blah: Value \('15'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)", + r"Parameter blah: Value \('15'\) must not fulfill \(10 < value <= 20\)", ): p.validate(15) with self.assertRaisesRegex( ValueError, - r"Parameter blah: Value \('20'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)", + r"Parameter blah: Value \('20'\) must not fulfill \(10 < value <= 20\)", ): p.validate(20) p.validate(21) diff --git a/test/unit/app/tools/test_validation_parsing.py b/test/unit/app/tools/test_validation_parsing.py new file mode 100644 index 000000000000..c13b0973e6a0 --- /dev/null +++ b/test/unit/app/tools/test_validation_parsing.py @@ -0,0 +1,41 @@ +from typing import Optional + +from galaxy.tool_util.unittest_utils.sample_data import ( + INVALID_XML_VALIDATORS, + VALID_XML_VALIDATORS, +) +from galaxy.tools.parameters.validation import parse_xml_validators +from galaxy.util import XML + + +class MockApp: + + @property + def tool_data_tables(self): + return {"mycooltable": MockTable()} + + +class MockTable: + + def get_version_fields(self): + return (1, []) + + +def test_xml_validation_valid(): + for xml_validator in VALID_XML_VALIDATORS: + _validate_xml_str(xml_validator) + + +def test_xml_validation_invalid(): + for xml_validator in INVALID_XML_VALIDATORS: + exc: Optional[Exception] = None + try: + _validate_xml_str(xml_validator) + except ValueError as e: + exc = e + assert exc is not None, f"{xml_validator} - validated when it wasn't expected to" + + +def _validate_xml_str(xml_str: str): + xml_el = XML(f"{xml_str}") + parse_xml_validators(MockApp(), xml_el) diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml index 29911941e714..f444e7189869 100644 --- a/test/unit/tool_util/parameter_specification.yml +++ b/test/unit/tool_util/parameter_specification.yml @@ -191,15 +191,81 @@ gx_int_required: &gx_int_required gx_int_required_via_empty_string: <<: *gx_int_required +gx_int_validation_range: + request_valid: + - parameter: 1 + - parameter: 9 + request_invalid: + - parameter: {} + - parameter: {__class__: 'ConnectedValue'} + - parameter: -1 + - parameter: 11 + - parameter: 10 + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + +gx_int_min_max: + request_valid: + - parameter: 1 + - parameter: 9 + - parameter: 10 + request_invalid: + - parameter: -1 + - parameter: 11 + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + workflow_step_linked_invalid: + - parameter: -1 + +gx_text_regex_validation: + request_valid: + - parameter: acgt + - parameter: a + - parameter: aaaggttac + request_invalid: + - parameter: acgu + - parameter: nucleic + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + +gx_text_expression_validation: + request_valid: + - parameter: the code was foobar + - parameter: foobar + request_invalid: + - parameter: the code was not foo bar + - parameter: '' + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + workflow_step_linked_invalid: + - parameter: '' + +gx_text_empty_validation: + request_valid: + - parameter: foobar + - {} + request_invalid: + - parameter: '' + job_internal_valid: + - parameter: foobar + job_internal_invalid: + - {} + - parameter: '' + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + workflow_step_linked_invalid: + - parameter: '' + gx_text: request_valid: &gx_text_request_valid - parameter: moocow - parameter: 'some spaces' - parameter: '' - {} + # need to explicitly mark these as non-optional + - parameter: null request_invalid: &gx_text_request_invalid - parameter: 5 - - parameter: null - parameter: {} - parameter: { "moo": "cow" } - parameter: {__class__: 'ConnectedValue'} @@ -223,18 +289,18 @@ gx_text: - parameter: moocow - parameter: 'some spaces' - parameter: '' + - parameter: null job_internal_invalid: - {} - - parameter: null - parameter: { "moo": "cow" } workflow_step_valid: - parameter: moocow - parameter: 'some spaces' - parameter: '' - {} + - parameter: null workflow_step_invalid: - parameter: 5 - - parameter: null - parameter: {} - parameter: { "moo": "cow" } - parameter: {__class__: 'ConnectedValue'} @@ -244,9 +310,9 @@ gx_text: - parameter: '' - {} - parameter: {__class__: 'ConnectedValue'} + - parameter: null workflow_step_linked_invalid: - parameter: 5 - - parameter: null - parameter: {} - parameter: { "moo": "cow" } - parameter: {"class": 'ConnectedValue'} @@ -294,6 +360,21 @@ gx_text_optional: - parameter: {} - parameter: { "moo": "cow" } +gx_text_length_validation: + request_valid: + - parameter: "mytext" + - parameter: "mytext123" + request_invalid: + - parameter: "s" # too short + - parameter: "mytext1231231231sd" # too long + +gx_text_length_validation_negate: + request_valid: + - parameter: "m" + - parameter: "mytext123mocowdowees" + request_invalid: + - parameter: "goldilocks" + gx_select: request_valid: - parameter: "--ex1" @@ -484,6 +565,12 @@ gx_select_multiple_optional: - parameter: {} - parameter: 5 +gx_select_no_options_validation: + job_internal_valid: + - parameter: "--ex1" + job_internal_invalid: + - {} + gx_genomebuild: request_valid: - parameter: hg19 @@ -571,6 +658,22 @@ gx_directory_uri: - parameter: true - parameter: null +gx_directory_uri_validation: + request_valid: + - parameter: "gxfiles://mycoolsource/api/index.json" + request_invalid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "gxfiles://tooshort/index.json" + - parameter: "gxfiles://mycoolsource/api/wrongex.js" + - parameter: "gxfiles://mycoolsource/badexp.json" + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "gxfiles://mycoolsource/api/index.json" + workflow_step_linked_invalid: + - parameter: "gxfiles://tooshort/index.json" + - parameter: "gxfiles://mycoolsource/api/wrongex.js" + - parameter: "gxfiles://mycoolsource/badexp.json" + gx_hidden: request_valid: - parameter: moocow @@ -656,6 +759,22 @@ gx_hidden_optional: workflow_step_linked_invalid: - parameter: 5 +gx_hidden_validation: + request_valid: + - parameter: "http://mycoolservice.com/api/index.json" + request_invalid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "http://mycoolservice.com/index.json" + - parameter: "http://mycoolservice.com/api/wrongex.js" + - parameter: "http://mycoolservice.com/badexp.json" + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "http://mycoolservice.com/api/index.json" + workflow_step_linked_invalid: + - parameter: "http://mycoolservice.com/index.json" + - parameter: "http://mycoolservice.com/api/wrongex.js" + - parameter: "http://mycoolservice.com/badexp.json" + gx_float: request_valid: - parameter: 5 @@ -779,6 +898,26 @@ gx_float_optional: - parameter: 'foobar' - parameter: {__class__: 'ConnectedValue2'} +gx_float_validation_range: + request_valid: + - parameter: 0.1 + - parameter: 9.7 + - parameter: 5 + request_invalid: + - parameter: 10 + - parameter: 0 + - parameter: 9.8 # endpoint not included because specified in validator + +gx_float_min_max: + request_valid: + - parameter: 0.1 + - parameter: 9.7 + - parameter: 5 + - parameter: 9.8 # endpoint implicitly included when max attribute set on param + request_invalid: + - parameter: 10 + - parameter: 0 + gx_color: request_valid: - parameter: '#aabbcc' diff --git a/test/unit/tool_util/test_parameter_validator_models.py b/test/unit/tool_util/test_parameter_validator_models.py new file mode 100644 index 000000000000..17af98f2ce6c --- /dev/null +++ b/test/unit/tool_util/test_parameter_validator_models.py @@ -0,0 +1,28 @@ +from typing import Optional + +from galaxy.tool_util.parser.parameter_validators import parse_xml_validators +from galaxy.tool_util.unittest_utils.sample_data import ( + INVALID_XML_VALIDATORS, + VALID_XML_VALIDATORS, +) +from galaxy.util import XML + + +def test_xml_validation_valid(): + for xml_validator in VALID_XML_VALIDATORS: + _validate_xml_str(xml_validator) + + +def test_xml_validation_invalid(): + for xml_validator in INVALID_XML_VALIDATORS: + exc: Optional[Exception] = None + try: + _validate_xml_str(xml_validator) + except ValueError as e: + exc = e + assert exc is not None, f"{xml_validator} - validated when it wasn't expected to" + + +def _validate_xml_str(xml_str: str): + xml_el = XML(f"{xml_str}") + parse_xml_validators(xml_el) From 2ddc437e1533afa3c348d417d1b81f323a174055 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 8 Oct 2024 12:10:36 -0400 Subject: [PATCH 10/12] Fix param models for dynamic options and truevalue/falsevalue --- lib/galaxy/tool_util/parameters/convert.py | 1 - lib/galaxy/tool_util/parameters/factory.py | 7 ++++++- lib/galaxy/tool_util/parser/interface.py | 4 ++++ lib/galaxy/tool_util/parser/xml.py | 18 ++++++++++++++---- lib/galaxy/tools/parameters/basic.py | 2 ++ test/unit/app/tools/test_dynamic_options.py | 8 +++++--- test/unit/tool_util/test_parameter_convert.py | 13 +++++++++++++ 7 files changed, 44 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index 94e4d0c5e569..63dbb9ab58b9 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -312,7 +312,6 @@ def _fill_default_for(tool_state: Dict[str, Any], parameter: ToolParameterT) -> ) test_value = validate_explicit_conditional_test_value(test_parameter_name, explicit_test_value) when = _select_which_when(conditional, test_value, conditional_state) - test_parameter = conditional.test_parameter _fill_default_for(conditional_state, test_parameter) _fill_defaults(conditional_state, when) elif parameter_type in ["gx_repeat"]: diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py index 13d52d5f9c77..932195effb9d 100644 --- a/lib/galaxy/tool_util/parameters/factory.py +++ b/lib/galaxy/tool_util/parameters/factory.py @@ -276,7 +276,12 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool default_test_value = cond_test_parameter_default_value(test_parameter) for value, case_inputs_sources in input_source.parse_when_input_sources(): if isinstance(test_parameter, BooleanParameterModel): - # TODO: investigate truevalue/falsevalue when... + true_value = test_param_input_source.get("truevalue") + false_value = test_param_input_source.get("falsevalue") + if isinstance(value, str) and value == true_value: + value = True + elif isinstance(value, str) and value == false_value: + value = False typed_value = string_as_bool(value) else: typed_value = value diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index b9218ad54270..af72bf4a4825 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -435,6 +435,10 @@ def elem(self) -> Element: # used with other tool sources. raise NotImplementedError(NOT_IMPLEMENTED_MESSAGE) + @abstractmethod + def get_dynamic_options_code(self) -> Optional[str]: + """If dynamic options are a piece of code to eval, return it.""" + @abstractmethod def get_data_table_name(self) -> Optional[str]: """If dynamic options are loaded from a data table, return the name.""" diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 06a25c78f29b..aba5a6874248 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -1300,18 +1300,23 @@ def parse_input_sources(self): class XmlDynamicOptions(DynamicOptions): - def __init__(self, options_elem: Element): + def __init__(self, options_elem: Element, dynamic_option_code: Optional[str]): self._options_elem = options_elem + self._dynamic_options_code = dynamic_option_code def elem(self) -> Element: return self._options_elem + def get_dynamic_options_code(self) -> Optional[str]: + """If dynamic options are a piece of code to eval, return it.""" + return self._dynamic_options_code + def get_data_table_name(self) -> Optional[str]: """If dynamic options are loaded from a data table, return the name.""" - return self._options_elem.get("from_data_table") + return self._options_elem.get("from_data_table") if self._options_elem is not None else None def get_index_file_name(self) -> Optional[str]: - return self._options_elem.get("from_file") + return self._options_elem.get("from_file") if self._options_elem is not None else None class XmlInputSource(InputSource): @@ -1349,7 +1354,12 @@ def parse_validators(self) -> List[AnyValidatorModel]: def parse_dynamic_options(self) -> Optional[XmlDynamicOptions]: """Return a XmlDynamicOptions to describe dynamic options if options elem is available.""" options_elem = self.input_elem.find("options") - return XmlDynamicOptions(options_elem) if options_elem is not None else None + dynamic_option_code = self.input_elem.get("dynamic_options") + is_dynamic = options_elem is not None or dynamic_option_code is not None + if is_dynamic: + return XmlDynamicOptions(options_elem, dynamic_option_code) + else: + return None def parse_static_options(self) -> List[Tuple[str, str, bool]]: """ diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 4860f97b5d51..7ed9c657691a 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -123,6 +123,8 @@ def parse_dynamic_options(param, input_source): if not dynamic_options_config: return None options_elem = dynamic_options_config.elem() + if options_elem is None: + return None return dynamic_options.DynamicOptions(options_elem, param) diff --git a/test/unit/app/tools/test_dynamic_options.py b/test/unit/app/tools/test_dynamic_options.py index 77d9eb263ecf..05ec365a379e 100644 --- a/test/unit/app/tools/test_dynamic_options.py +++ b/test/unit/app/tools/test_dynamic_options.py @@ -6,9 +6,11 @@ def get_from_url_option(): - tool_param = Bunch() - tool_param.tool = Bunch() - tool_param.tool.app = Bunch() + tool_param = Bunch( + tool=Bunch( + app=Bunch(), + ), + ) return DynamicOptions( XML( diff --git a/test/unit/tool_util/test_parameter_convert.py b/test/unit/tool_util/test_parameter_convert.py index a14eab6d5657..7f5deff41e85 100644 --- a/test/unit/tool_util/test_parameter_convert.py +++ b/test/unit/tool_util/test_parameter_convert.py @@ -217,6 +217,19 @@ def test_fill_defaults(): assert with_defaults["cond"]["cond"] == "single" assert with_defaults["cond"]["inner_cond"]["inner_cond"] == "single" + # dynamic parameters should just stay empty - null would cause runtime to skip over population + with_defaults = fill_state_for({}, "parameters/gx_select_dynamic", partial=True) + assert "parameter" not in with_defaults + + # dynamic parameters should just stay empty - null would cause runtime to skip over population + with_defaults = fill_state_for( + {"conditional_parameter": {"test_parameter": False}}, + "parameters/gx_conditional_boolean_discriminate_on_string_value", + ) + assert "conditional_parameter" in with_defaults + assert "boolean_parameter" in with_defaults["conditional_parameter"] + assert with_defaults["conditional_parameter"]["boolean_parameter"] is False + def _fake_dereference(input: DataRequestUri) -> DataRequestInternalHda: return DataRequestInternalHda(id=EXAMPLE_ID_1) From 9b3cd3ec4d997319d772ed38871e6a17ce012183 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 8 Oct 2024 12:33:10 -0400 Subject: [PATCH 11/12] Update tool shed schema for parameter model changes. --- .../webapp/frontend/src/schema/schema.ts | 208 ++++++++++++++++++ 1 file changed, 208 insertions(+) diff --git a/lib/tool_shed/webapp/frontend/src/schema/schema.ts b/lib/tool_shed/webapp/frontend/src/schema/schema.ts index 5667b073d0e6..14aab45a3c36 100644 --- a/lib/tool_shed/webapp/frontend/src/schema/schema.ts +++ b/lib/tool_shed/webapp/frontend/src/schema/schema.ts @@ -1386,6 +1386,16 @@ export interface components { * @enum {string} */ parameter_type: "gx_directory_uri" + /** + * Validators + * @default [] + */ + validators: ( + | components["schemas"]["LengthParameterValidatorModel"] + | components["schemas"]["RegexParameterValidatorModel"] + | components["schemas"]["ExpressionParameterValidatorModel"] + | components["schemas"]["EmptyFieldParameterValidatorModel"] + )[] } /** DrillDownOptionsDict */ DrillDownOptionsDict: { @@ -1440,6 +1450,57 @@ export interface components { */ parameter_type: "gx_drill_down" } + /** EmptyFieldParameterValidatorModel */ + EmptyFieldParameterValidatorModel: { + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default empty_field + * @constant + * @enum {string} + */ + type: "empty_field" + } + /** + * ExpressionParameterValidatorModel + * @description Check if a one line python expression given expression evaluates to True. + * + * The expression is given is the content of the validator tag. + */ + ExpressionParameterValidatorModel: { + /** Expression */ + expression: string + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default expression + * @constant + * @enum {string} + */ + type: "expression" + } /** FailedRepositoryUpdateMessage */ FailedRepositoryUpdateMessage: { /** Err Msg */ @@ -1514,6 +1575,11 @@ export interface components { * @enum {string} */ parameter_type: "gx_float" + /** + * Validators + * @default [] + */ + validators: components["schemas"]["InRangeParameterValidatorModel"][] /** Value */ value?: number | null } @@ -1629,6 +1695,16 @@ export interface components { * @enum {string} */ parameter_type: "gx_hidden" + /** + * Validators + * @default [] + */ + validators: ( + | components["schemas"]["LengthParameterValidatorModel"] + | components["schemas"]["RegexParameterValidatorModel"] + | components["schemas"]["ExpressionParameterValidatorModel"] + | components["schemas"]["EmptyFieldParameterValidatorModel"] + )[] /** Value */ value: string | null } @@ -1666,6 +1742,42 @@ export interface components { * @enum {string} */ ImageType: "Docker" | "Singularity" | "Conda" + /** InRangeParameterValidatorModel */ + InRangeParameterValidatorModel: { + /** + * Exclude Max + * @default false + */ + exclude_max: boolean + /** + * Exclude Min + * @default false + */ + exclude_min: boolean + /** + * Implicit + * @default false + */ + implicit: boolean + /** Max */ + max?: number | null + /** Message */ + message?: string | null + /** Min */ + min?: number | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default in_range + * @constant + * @enum {string} + */ + type: "in_range" + } /** InstallInfo */ InstallInfo: { metadata_info?: components["schemas"]["RepositoryMetadataInstallInfo"] | null @@ -1704,6 +1816,11 @@ export interface components { * @enum {string} */ parameter_type: "gx_integer" + /** + * Validators + * @default [] + */ + validators: components["schemas"]["InRangeParameterValidatorModel"][] /** Value */ value?: number | null } @@ -1716,6 +1833,32 @@ export interface components { /** Value */ value: string } + /** LengthParameterValidatorModel */ + LengthParameterValidatorModel: { + /** + * Implicit + * @default false + */ + implicit: boolean + /** Max */ + max?: number | null + /** Message */ + message?: string | null + /** Min */ + min?: number | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default length + * @constant + * @enum {string} + */ + type: "length" + } /** MessageExceptionModel */ MessageExceptionModel: { /** Err Code */ @@ -1723,6 +1866,28 @@ export interface components { /** Err Msg */ err_msg: string } + /** NoOptionsParameterValidatorModel */ + NoOptionsParameterValidatorModel: { + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default no_options + * @constant + * @enum {string} + */ + type: "no_options" + } /** Organization */ Organization: { /** @@ -1800,6 +1965,37 @@ export interface components { /** Xrefs */ xrefs: components["schemas"]["XrefDict"][] } + /** + * RegexParameterValidatorModel + * @description Check if a regular expression **matches** the value, i.e. appears + * at the beginning of the value. To enforce a match of the complete value use + * ``$`` at the end of the expression. The expression is given is the content + * of the validator tag. Note that for ``selects`` each option is checked + * separately. + */ + RegexParameterValidatorModel: { + /** Expression */ + expression: string + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default regex + * @constant + * @enum {string} + */ + type: "regex" + } /** RepeatParameterModel */ RepeatParameterModel: { /** Argument */ @@ -2254,6 +2450,8 @@ export interface components { * @enum {string} */ parameter_type: "gx_select" + /** Validators */ + validators: components["schemas"]["NoOptionsParameterValidatorModel"][] } /** Service */ Service: { @@ -2366,6 +2564,16 @@ export interface components { * @enum {string} */ parameter_type: "gx_text" + /** + * Validators + * @default [] + */ + validators: ( + | components["schemas"]["LengthParameterValidatorModel"] + | components["schemas"]["RegexParameterValidatorModel"] + | components["schemas"]["ExpressionParameterValidatorModel"] + | components["schemas"]["EmptyFieldParameterValidatorModel"] + )[] /** Value */ value?: string | null } From 0a01326cacc723b31b012aa0a3b3a638bedfd3b5 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 17 Oct 2024 15:59:38 -0400 Subject: [PATCH 12/12] Cleanup stock tool sources parsed in galaxy.tools.stock. Was causing a randomly failing test. --- lib/galaxy/tools/stock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/stock.py b/lib/galaxy/tools/stock.py index 1a86354f1370..da890fe13a8f 100644 --- a/lib/galaxy/tools/stock.py +++ b/lib/galaxy/tools/stock.py @@ -26,7 +26,7 @@ def stock_tool_sources(): def _walk_directory_for_tools(path): - if path.is_file() and path.name.endswith(".xml"): + if path.is_file() and path.name.endswith(".xml") and "macros" not in path.name and "_conf.xml" not in path.name: yield path elif path.is_dir(): for directory in path.iterdir():