From 6ae000deb597a9b709b10b9acf1dcb08f4eb0b6a Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 8 Oct 2024 14:58:18 +0200 Subject: [PATCH 1/3] assert that data_column parameters have a valid data_ref - it needs to be specified in the XML - needs to refer to an existing parameter --- lib/galaxy/tool_shed/tools/tool_validator.py | 2 +- lib/galaxy/tools/__init__.py | 3 +-- lib/galaxy/tools/parameters/basic.py | 1 + .../missing_data_ref/missing_data_ref.xml | 19 +++++++++++++++++++ .../repos/wrong_data_ref/wrong_data_ref.xml | 19 +++++++++++++++++++ test/unit/tool_shed/test_tool_validation.py | 18 ++++++++++++++++++ 6 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 lib/tool_shed/test/test_data/repos/missing_data_ref/missing_data_ref.xml create mode 100644 lib/tool_shed/test/test_data/repos/wrong_data_ref/wrong_data_ref.xml diff --git a/lib/galaxy/tool_shed/tools/tool_validator.py b/lib/galaxy/tool_shed/tools/tool_validator.py index a73af2230c9e..428601c65113 100644 --- a/lib/galaxy/tool_shed/tools/tool_validator.py +++ b/lib/galaxy/tool_shed/tools/tool_validator.py @@ -97,7 +97,7 @@ def load_tool_from_config(self, repository_id, full_path): tool = None valid = False error_message = ( - f'This file requires an entry for "{str(e)}" in the tool_data_table_conf.xml file. Upload a file ' + f'This file requires an entry for "{str(e)}" in the tool_data_table_conf.xml file. Upload a file ' ) error_message += ( "named tool_data_table_conf.xml.sample to the repository that includes the required entry to correct " diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index af9aed7c604e..4439782829ae 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1665,8 +1665,7 @@ def parse_param_elem(self, input_source: InputSource, enctypes, context) -> Tool # form when it changes for name in param.get_dependencies(): # Let it throw exception, but give some hint what the problem might be - if name not in context: - log.error(f"Tool with id '{self.id}': Could not find dependency '{name}' of parameter '{param.name}'") + assert name in context, f"Tool with id '{self.id}': Could not find dependency '{name}' of parameter '{param.name}'" context[name].refresh_on_change = True return param diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index d3bb7f119c58..0fc140b750bc 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1403,6 +1403,7 @@ def __init__(self, tool, input_source): if self.accept_default: self.optional = True self.data_ref = input_source.get("data_ref", None) + assert self.data_ref is not None, f'data_column parameter {self.name} requires a valid data_ref attribute' self.ref_input = None # Legacy style default value specification... self.default_value = input_source.get("default_value", None) diff --git a/lib/tool_shed/test/test_data/repos/missing_data_ref/missing_data_ref.xml b/lib/tool_shed/test/test_data/repos/missing_data_ref/missing_data_ref.xml new file mode 100644 index 000000000000..090972695b10 --- /dev/null +++ b/lib/tool_shed/test/test_data/repos/missing_data_ref/missing_data_ref.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + diff --git a/lib/tool_shed/test/test_data/repos/wrong_data_ref/wrong_data_ref.xml b/lib/tool_shed/test/test_data/repos/wrong_data_ref/wrong_data_ref.xml new file mode 100644 index 000000000000..608dd7aad61c --- /dev/null +++ b/lib/tool_shed/test/test_data/repos/wrong_data_ref/wrong_data_ref.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + diff --git a/test/unit/tool_shed/test_tool_validation.py b/test/unit/tool_shed/test_tool_validation.py index a1f421120ecc..cf538e5599a1 100644 --- a/test/unit/tool_shed/test_tool_validation.py +++ b/test/unit/tool_shed/test_tool_validation.py @@ -11,6 +11,8 @@ BOWTIE2_INDICES = os.path.join( galaxy_directory(), "lib/tool_shed/test/test_data/bowtie2_loc_sample/bowtie2_indices.loc.sample" ) +MISSING_DATA_REF_DIR = os.path.join(galaxy_directory(), "lib/tool_shed/test/test_data/repos/missing_data_ref") +WRONG_DATA_REF_DIR = os.path.join(galaxy_directory(), "lib/tool_shed/test/test_data/repos/wrong_data_ref") def test_validate_valid_tool(): @@ -64,6 +66,22 @@ def test_validate_tool_without_index(): assert not tool.params_with_missing_index_file +def test_validate_missing_data_ref(): + repo_dir = MISSING_DATA_REF_DIR + with get_tool_validator() as tv: + full_path = os.path.join(repo_dir, "missing_data_ref.xml") + tool, valid, message = tv.load_tool_from_config(repository_id=None, full_path=full_path) + assert valid is False + + +def test_validate_wrong_data_ref(): + repo_dir = WRONG_DATA_REF_DIR + with get_tool_validator() as tv: + full_path = os.path.join(repo_dir, "wrong_data_ref.xml") + tool, valid, message = tv.load_tool_from_config(repository_id=None, full_path=full_path) + assert valid is False + + @contextmanager def get_tool_validator(): app = MockApp() From 661dd2a0de16a01ff91bac1e0a7fbbe0aea26cb8 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 21 Oct 2024 13:51:55 +0200 Subject: [PATCH 2/3] fix linting --- lib/galaxy/tools/__init__.py | 4 +++- lib/galaxy/tools/parameters/basic.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 4439782829ae..05617e0beb4c 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1665,7 +1665,9 @@ def parse_param_elem(self, input_source: InputSource, enctypes, context) -> Tool # form when it changes for name in param.get_dependencies(): # Let it throw exception, but give some hint what the problem might be - assert name in context, f"Tool with id '{self.id}': Could not find dependency '{name}' of parameter '{param.name}'" + assert ( + name in context + ), f"Tool with id '{self.id}': Could not find dependency '{name}' of parameter '{param.name}'" context[name].refresh_on_change = True return param diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 0fc140b750bc..cf445aaf67ce 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1403,7 +1403,7 @@ def __init__(self, tool, input_source): if self.accept_default: self.optional = True self.data_ref = input_source.get("data_ref", None) - assert self.data_ref is not None, f'data_column parameter {self.name} requires a valid data_ref attribute' + assert self.data_ref is not None, f"data_column parameter {self.name} requires a valid data_ref attribute" self.ref_input = None # Legacy style default value specification... self.default_value = input_source.get("default_value", None) From 78a8f91d7f2b5fc8a4664ba6c6e16aa1f136da78 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 13 Nov 2024 11:21:16 +0100 Subject: [PATCH 3/3] add profile version --- lib/galaxy/tool_util/xsd/galaxy.xsd | 4 ++++ lib/galaxy/tools/parameters/basic.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 8975b36408ec..0890052b56a0 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -68,6 +68,10 @@ List of behavior changes associated with profile versions: - Do not use Galaxy python environment for `data_source_async` tools. - Drop request parameters received by data source tools that are not declared in `` section. +### 24.2 + +- require a valid `data_ref` attribute for `data_column` parameters + ### Examples A normal tool: diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index cf445aaf67ce..b9964d96a890 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1403,7 +1403,10 @@ def __init__(self, tool, input_source): if self.accept_default: self.optional = True self.data_ref = input_source.get("data_ref", None) - assert self.data_ref is not None, f"data_column parameter {self.name} requires a valid data_ref attribute" + profile = tool.profile if tool else None + assert ( + profile is None or Version(str(profile)) < Version("24.2") or self.data_ref is not None + ), f"data_column parameter {self.name} requires a valid data_ref attribute" self.ref_input = None # Legacy style default value specification... self.default_value = input_source.get("default_value", None)