Merge pull request #18949 from bernt-matthias/data_col_ref

Assert that `data_column` parameters have a valid `data_ref`
galaxyproject · Nov 15, 2024 · edac0ff · edac0ff
2 parents 215b459 + 78a8f91
commit edac0ff
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 3 deletions.
diff --git 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/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 `<request_param_translation>` section.
 
+### 24.2
+
+- require a valid `data_ref` attribute for `data_column` parameters
+
 ### Examples
 
 A normal tool:

diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
@@ -1665,8 +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
-            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
@@ -1404,6 +1404,10 @@ def __init__(self, tool, input_source):
         if self.accept_default:
             self.optional = True
         self.data_ref = input_source.get("data_ref", None)
+        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)

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
@@ -0,0 +1,19 @@
+<tool id="missing_data_ref" name="missing_data_ref" version="1.0" profile="23.0">
+    <requirements>
+    </requirements>
+    <command detect_errors="aggressive">
+        <![CDATA[
+        echo $column
+        ]]>
+    </command>
+    <inputs>
+        <param name="input" type="data" format="tabular"/>
+        <param name="column" type="data_column" />
+    </inputs>
+    <outputs>
+        <data name="output" format="text"/>
+    </outputs>
+    <tests>
+    </tests>
+    <help/>
+</tool>
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
@@ -0,0 +1,19 @@
+<tool id="wrong_data_ref" name="wrong_data_ref" version="1.0" profile="23.0">
+    <requirements>
+    </requirements>
+    <command detect_errors="aggressive">
+        <![CDATA[
+        echo $column
+        ]]>
+    </command>
+    <inputs>
+        <param name="input" type="data" format="tabular"/>
+        <param name="column" type="data_column" data_ref="nonsense" />
+    </inputs>
+    <outputs>
+        <data name="output" format="text"/>
+    </outputs>
+    <tests>
+    </tests>
+    <help/>
+</tool>
diff --git 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()