From c6c34f442904a61ec22089f7d4cb4217df3b611d Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 5 Mar 2024 14:41:27 +0100 Subject: [PATCH 01/36] add linters for datatypes - tools should only use available datatypes in inputs, outputs and tests - discourage the use of custom datatypes_conf.xml --- lib/galaxy/tool_util/linters/datatypes.py | 83 +++++++++++++++++++++++ packages/tool_util/setup.cfg | 1 + test/unit/tool_util/test_tool_linters.py | 65 +++++++++++++++++- 3 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 lib/galaxy/tool_util/linters/datatypes.py diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py new file mode 100644 index 000000000000..c577e0dccd27 --- /dev/null +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -0,0 +1,83 @@ +import os.path +from typing import ( + Set, + TYPE_CHECKING, +) + +from galaxy import config +from galaxy.tool_util.lint import Linter +from galaxy.util import ( + listify, + parse_xml, +) + +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser import ToolSource + +DATATYPES_CONF = os.path.join(os.path.dirname(config.__file__), "sample", "datatypes_conf.xml.sample") + + +def _parse_datatypes(datatype_conf_path: str) -> Set[str]: + datatypes = set() + tree = parse_xml(datatype_conf_path) + root = tree.getroot() + for elem in root.findall(f"./registration/datatype"): + extension = elem.get("extension") + datatypes.add(extension) + auto_compressed_types = listify(elem.get("auto_compressed_types", "")) + for act in auto_compressed_types: + datatypes.add(f"{extension}.{act}") + return datatypes + + +class DatatypesCustomConf(Linter): + """ + Check if a custom datatypes_conf.xml is present + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + if not tool_source.source_path: + return + if not (tool_xml := getattr(tool_source, "xml_tree", None)): + return + tool_node = tool_xml.getroot() + tool_dir = os.path.dirname(tool_source.source_path) + datatypes_conf_path = os.path.join(tool_dir, "datatypes_conf.xml") + if os.path.exists(datatypes_conf_path): + lint_ctx.warn( + f"Tool uses a custom datatypes_conf.xml which is discouraged", + linter=cls.name(), + node=tool_node, + ) + + +class ValidDatatypes(Linter): + """ + Check that used datatypes are available + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + # get Galaxy built-in dataypes + datatypes = _parse_datatypes(DATATYPES_CONF) + # add custom tool data types + if tool_source.source_path: + tool_dir = os.path.dirname(tool_source.source_path) + datatypes_conf_path = os.path.join(tool_dir, "datatypes_conf.xml") + if os.path.exists(datatypes_conf_path): + datatypes |= _parse_datatypes(datatypes_conf_path) + for attrib in ["format", "ftype", "ext"]: + for elem in tool_xml.findall(f".//*[@{attrib}]"): + formats = elem.get(attrib, "").split(",") + for format in formats: + if format not in datatypes: + lint_ctx.error( + f"Unknown datatype [{format}] used in {elem.tag} element", + linter=cls.name(), + node=elem, + ) diff --git a/packages/tool_util/setup.cfg b/packages/tool_util/setup.cfg index 848a609a6b2c..b85c01eba4cd 100644 --- a/packages/tool_util/setup.cfg +++ b/packages/tool_util/setup.cfg @@ -33,6 +33,7 @@ version = 23.2.dev0 [options] include_package_data = True install_requires = + galaxy-config galaxy-util>=22.1 conda-package-streaming lxml!=4.2.2 diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 2db30308b950..f22a895727b0 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -16,6 +16,7 @@ from galaxy.tool_util.linters import ( citations, command, + datatypes, general, help, inputs, @@ -1947,6 +1948,68 @@ def test_xml_order(lint_ctx): assert lint_ctx.error_messages == ["Invalid XML: Element 'wrong_tag': This element is not expected."] +VALID_DATATYPES = """ + + + + + + + + + + + + + + + + + + + + + + + + +""" +DATATYPES_CONF = """ + + + + + +""" + + +def test_valid_datatypes(lint_ctx): + """ + test datatypes linters + """ + with tempfile.TemporaryDirectory() as tmp: + tool_path = os.path.join(tmp, "tool.xml") + datatypes_path = os.path.join(tmp, "datatypes_conf.xml") + with open(tool_path, "w") as tmpf: + tmpf.write(VALID_DATATYPES) + with open(datatypes_path, "w") as tmpf: + tmpf.write(DATATYPES_CONF) + tool_xml, _ = load_with_references(tool_path) + tool_source = XmlToolSource(tool_xml, source_path=tool_path) + run_lint_module(lint_ctx, datatypes, tool_source) + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert "Tool uses a custom datatypes_conf.xml which is discouraged" in lint_ctx.warn_messages + assert len(lint_ctx.warn_messages) == 1 + assert "Unknown datatype [invalid] used in param" in lint_ctx.error_messages + assert "Unknown datatype [another_invalid] used in data" in lint_ctx.error_messages + assert "Unknown datatype [just_another_invalid] used in when" in lint_ctx.error_messages + assert "Unknown datatype [collection_format] used in collection" in lint_ctx.error_messages + assert "Unknown datatype [invalid] used in param" in lint_ctx.error_messages + assert "Unknown datatype [invalid] used in discover_datasets" in lint_ctx.error_messages + assert len(lint_ctx.error_messages) == 6 + + DATA_MANAGER = """ @@ -2168,7 +2231,7 @@ def test_skip_by_module(lint_ctx): def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 133 + assert len(linter_names) == 135 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From eb61d68db60be0306ae62e46a4ef86a4a06f0259 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 5 Mar 2024 14:49:27 +0100 Subject: [PATCH 02/36] require python 3.8 for tool-util --- packages/tool_util/setup.cfg | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/tool_util/setup.cfg b/packages/tool_util/setup.cfg index b85c01eba4cd..9f532ed401cb 100644 --- a/packages/tool_util/setup.cfg +++ b/packages/tool_util/setup.cfg @@ -9,7 +9,6 @@ classifiers = Natural Language :: English Operating System :: POSIX Programming Language :: Python :: 3 - Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 @@ -45,7 +44,7 @@ install_requires = sortedcontainers typing-extensions packages = find: -python_requires = >=3.7 +python_requires = >=3.8 [options.entry_points] console_scripts = From 3065b36e3ec6d389956a9ffece6d4160e951b51c Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 5 Mar 2024 14:54:30 +0100 Subject: [PATCH 03/36] revert walrus operator usage to allow usage in python 3.7 --- lib/galaxy/tool_util/linters/datatypes.py | 3 ++- packages/tool_util/setup.cfg | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py index c577e0dccd27..aba7feca96d2 100644 --- a/lib/galaxy/tool_util/linters/datatypes.py +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -40,7 +40,8 @@ class DatatypesCustomConf(Linter): def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): if not tool_source.source_path: return - if not (tool_xml := getattr(tool_source, "xml_tree", None)): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: return tool_node = tool_xml.getroot() tool_dir = os.path.dirname(tool_source.source_path) diff --git a/packages/tool_util/setup.cfg b/packages/tool_util/setup.cfg index 9f532ed401cb..b85c01eba4cd 100644 --- a/packages/tool_util/setup.cfg +++ b/packages/tool_util/setup.cfg @@ -9,6 +9,7 @@ classifiers = Natural Language :: English Operating System :: POSIX Programming Language :: Python :: 3 + Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 @@ -44,7 +45,7 @@ install_requires = sortedcontainers typing-extensions packages = find: -python_requires = >=3.8 +python_requires = >=3.7 [options.entry_points] console_scripts = From 08f2124eb6a748e5b06165822d236157d4225b35 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 5 Mar 2024 15:02:14 +0100 Subject: [PATCH 04/36] fix linter errors --- lib/galaxy/tool_util/linters/datatypes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py index aba7feca96d2..98837d057ccc 100644 --- a/lib/galaxy/tool_util/linters/datatypes.py +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -22,7 +22,7 @@ def _parse_datatypes(datatype_conf_path: str) -> Set[str]: datatypes = set() tree = parse_xml(datatype_conf_path) root = tree.getroot() - for elem in root.findall(f"./registration/datatype"): + for elem in root.findall("./registration/datatype"): extension = elem.get("extension") datatypes.add(extension) auto_compressed_types = listify(elem.get("auto_compressed_types", "")) @@ -48,7 +48,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): datatypes_conf_path = os.path.join(tool_dir, "datatypes_conf.xml") if os.path.exists(datatypes_conf_path): lint_ctx.warn( - f"Tool uses a custom datatypes_conf.xml which is discouraged", + "Tool uses a custom datatypes_conf.xml which is discouraged", linter=cls.name(), node=tool_node, ) From 99e088a99c11b83eb0b41a1882f39067ad29367a Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 5 Mar 2024 15:54:01 +0100 Subject: [PATCH 05/36] fix return type --- lib/galaxy/tool_util/linters/datatypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py index 98837d057ccc..9fd3a08f4c9a 100644 --- a/lib/galaxy/tool_util/linters/datatypes.py +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -23,7 +23,7 @@ def _parse_datatypes(datatype_conf_path: str) -> Set[str]: tree = parse_xml(datatype_conf_path) root = tree.getroot() for elem in root.findall("./registration/datatype"): - extension = elem.get("extension") + extension = elem.get("extension", "") datatypes.add(extension) auto_compressed_types = listify(elem.get("auto_compressed_types", "")) for act in auto_compressed_types: From d4618c2c73ac6c06e9c25a0fe9015757a1ebf521 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Sat, 18 May 2024 12:08:40 +0200 Subject: [PATCH 06/36] try to add datatypes_conf sample to tool-util package --- packages/tool_util/MANIFEST.in | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/tool_util/MANIFEST.in b/packages/tool_util/MANIFEST.in index 3c6d371983de..496d970a6c3c 100644 --- a/packages/tool_util/MANIFEST.in +++ b/packages/tool_util/MANIFEST.in @@ -5,3 +5,4 @@ include galaxy/tool_util/toolbox/filters/*.sample include galaxy/tool_util/verify/test_config.sample.yml include galaxy/tool_util/xsd/* include galaxy/tool_util/ontologies/*tsv +include galaxy/config/sample/datatypes_conf.xml.sample \ No newline at end of file From c0263055cb9068a1919afde1c0ad8e9a4f8ae129 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Sat, 18 May 2024 12:13:11 +0200 Subject: [PATCH 07/36] revert addition of config as requirement --- packages/tool_util/setup.cfg | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/tool_util/setup.cfg b/packages/tool_util/setup.cfg index b85c01eba4cd..848a609a6b2c 100644 --- a/packages/tool_util/setup.cfg +++ b/packages/tool_util/setup.cfg @@ -33,7 +33,6 @@ version = 23.2.dev0 [options] include_package_data = True install_requires = - galaxy-config galaxy-util>=22.1 conda-package-streaming lxml!=4.2.2 From 6f044a17ef9ddbeab3a5db31249e687310851f76 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Sat, 18 May 2024 12:58:14 +0200 Subject: [PATCH 08/36] remove config import --- lib/galaxy/tool_util/linters/datatypes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py index 9fd3a08f4c9a..b51fd5dee50c 100644 --- a/lib/galaxy/tool_util/linters/datatypes.py +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -4,7 +4,7 @@ TYPE_CHECKING, ) -from galaxy import config +# from galaxy import config from galaxy.tool_util.lint import Linter from galaxy.util import ( listify, @@ -15,7 +15,7 @@ from galaxy.tool_util.lint import LintContext from galaxy.tool_util.parser import ToolSource -DATATYPES_CONF = os.path.join(os.path.dirname(config.__file__), "sample", "datatypes_conf.xml.sample") +# DATATYPES_CONF = os.path.join(os.path.dirname(config.__file__), "sample", "datatypes_conf.xml.sample") def _parse_datatypes(datatype_conf_path: str) -> Set[str]: From 2bfb7318e664a68dfc9de69fffdf5e328769d434 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Sat, 18 May 2024 13:40:29 +0200 Subject: [PATCH 09/36] try --- lib/galaxy/tool_util/linters/datatypes.py | 2 +- lib/galaxy/tool_util/linters/datatypes_conf.xml.sample | 1 + packages/tool_util/MANIFEST.in | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) create mode 120000 lib/galaxy/tool_util/linters/datatypes_conf.xml.sample diff --git a/lib/galaxy/tool_util/linters/datatypes.py b/lib/galaxy/tool_util/linters/datatypes.py index b51fd5dee50c..534868ed01ec 100644 --- a/lib/galaxy/tool_util/linters/datatypes.py +++ b/lib/galaxy/tool_util/linters/datatypes.py @@ -15,7 +15,7 @@ from galaxy.tool_util.lint import LintContext from galaxy.tool_util.parser import ToolSource -# DATATYPES_CONF = os.path.join(os.path.dirname(config.__file__), "sample", "datatypes_conf.xml.sample") +DATATYPES_CONF = os.path.join(os.path.dirname(__file__), "datatypes_conf.xml.sample") def _parse_datatypes(datatype_conf_path: str) -> Set[str]: diff --git a/lib/galaxy/tool_util/linters/datatypes_conf.xml.sample b/lib/galaxy/tool_util/linters/datatypes_conf.xml.sample new file mode 120000 index 000000000000..6a8d2e103481 --- /dev/null +++ b/lib/galaxy/tool_util/linters/datatypes_conf.xml.sample @@ -0,0 +1 @@ +../../config/sample/datatypes_conf.xml.sample \ No newline at end of file diff --git a/packages/tool_util/MANIFEST.in b/packages/tool_util/MANIFEST.in index 496d970a6c3c..a60b960699b6 100644 --- a/packages/tool_util/MANIFEST.in +++ b/packages/tool_util/MANIFEST.in @@ -5,4 +5,4 @@ include galaxy/tool_util/toolbox/filters/*.sample include galaxy/tool_util/verify/test_config.sample.yml include galaxy/tool_util/xsd/* include galaxy/tool_util/ontologies/*tsv -include galaxy/config/sample/datatypes_conf.xml.sample \ No newline at end of file +include galaxy/tool_util/linters/datatypes_conf.xml.sample \ No newline at end of file From f8478d7aa78450976cff9727106876a9e4db1045 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 21 Oct 2024 22:20:01 +0200 Subject: [PATCH 10/36] restrict ext, format, ftype in xsd --- lib/galaxy/tool_util/xsd/galaxy.xsd | 30 ++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 96405ed94b84..d1fd80bb08e0 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -1575,7 +1575,7 @@ used to load typed parameters. This string will be loaded as JSON and its type w attempt to be preserved through API requests to Galaxy. - + This attribute name should be included only with parameters of ``type`` ``data`` for the tool. If this @@ -1720,7 +1720,7 @@ generated as JSON. This can be useful for testing tool outputs that are not file ]]> - + - + The comma-separated list of accepted data formats for this input. The list of supported data formats is contained in the @@ -5639,7 +5639,7 @@ The default is ``galaxy.json``. - + The short name for the output datatype. The valid values for format can be found in @@ -6042,12 +6042,12 @@ More information can be found on Planemo's documentation for Indicates that the entire path of the discovered dataset relative to the specified directory should be available for matching patterns. - + Format (or datatype) of discovered datasets (an alias with ``ext``). - + Format (or datatype) of discovered datasets (an alias with ``format``). @@ -6115,12 +6115,12 @@ Galaxy, including: Indicates that the entire path of the discovered dataset relative to the specified directory should be available for matching patterns. - + Format (or datatype) of discovered datasets (an alias with ``ext``). - + Format (or datatype) of discovered datasets (an alias with ``format``). @@ -7418,7 +7418,7 @@ parameter (e.g. ``value="interval"`` above), or of the deprecated ``input_datase attribute. - + This value must be a supported data type (e.g. ``format="interval"``). See @@ -7844,6 +7844,18 @@ favour of a ``has_size`` assertion. + + + + + + + + + + + + Date: Tue, 22 Oct 2024 09:23:12 +0200 Subject: [PATCH 11/36] fix test: number of linters --- test/unit/tool_util/test_tool_linters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 6c3a900aea4e..ea84b6c2a79e 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -2208,7 +2208,7 @@ def test_skip_by_module(lint_ctx): def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 135 + assert len(linter_names) == 134 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From be3c54716147b3251a7c1690dfbad868fa112980 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 22 Oct 2024 09:28:31 +0200 Subject: [PATCH 12/36] document format of extension --- doc/source/dev/data_types.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/source/dev/data_types.md b/doc/source/dev/data_types.md index fb03c4fe5d04..16b970290be2 100644 --- a/doc/source/dev/data_types.md +++ b/doc/source/dev/data_types.md @@ -36,7 +36,8 @@ tag section of the `datatypes_conf.xml` file. Sample where - `extension` - the data type's Dataset file extension (e.g., `ab1`, `bed`, - `gff`, `qual`, etc.) + `gff`, `qual`, etc.). The extension must consist only of lowercase letters, + numbers, `_`, `-`, and `.`. - `type` - the path to the class for that data type. - `mimetype` - if present (it's optional), the data type's mime type - `display_in_upload` - if present (it's optional and defaults to False), the From 3b702569e1c2dcf3f1f94a622688e77093610b6c Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 22 Oct 2024 09:28:58 +0200 Subject: [PATCH 13/36] fix format --- lib/galaxy/tool_util/xsd/galaxy.xsd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 33edb06aa50c..d818f2438ab2 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -8030,7 +8030,7 @@ favour of a ``has_size`` assertion. - + From e37eba31cb9cce0fa05d8f381f870e32d74e948d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 16 Nov 2024 09:19:01 +0100 Subject: [PATCH 14/36] Fix production_aws_private_bucket.yml --- .../templates/examples/production_aws_private_bucket.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/files/templates/examples/production_aws_private_bucket.yml b/lib/galaxy/files/templates/examples/production_aws_private_bucket.yml index 81e62afa16ad..3086746dbacb 100644 --- a/lib/galaxy/files/templates/examples/production_aws_private_bucket.yml +++ b/lib/galaxy/files/templates/examples/production_aws_private_bucket.yml @@ -3,10 +3,10 @@ description: Setup access to a private AWS bucket using a secret access key. configuration: type: s3fs - bucket: "{{ bucket }}" - writable: "{{ writable }}" - secret: "{{ secret_key }}" - key: "{{ access_key }}" + bucket: "{{ variables.bucket }}" + writable: "{{ variables.writable }}" + secret: "{{ secrets.secret_key }}" + key: "{{ variables.access_key }}" variables: access_key: label: Access Key ID From 1bdd9e7af54c554ed12717be8a7ca6071166155c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 19 Nov 2024 11:00:23 +0100 Subject: [PATCH 15/36] Make runtime value in optional parameter open legacy workflow run form We can only accept disconnected optional data inputs, for all other parameters we need to replace the runtime value placeholder. --- client/src/components/Workflow/Run/model.js | 26 +++++++++------------ 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/client/src/components/Workflow/Run/model.js b/client/src/components/Workflow/Run/model.js index 29eeca92c369..1238146d7ba6 100644 --- a/client/src/components/Workflow/Run/model.js +++ b/client/src/components/Workflow/Run/model.js @@ -136,34 +136,30 @@ export class WorkflowRunModel { // or if an explicit reference is specified as data_ref and available _.each(this.steps, (step) => { if (step.step_type == "tool") { - var data_resolved = true; + let dataResolved = true; visitInputs(step.inputs, (input, name, context) => { - var is_runtime_value = input.value && input.value.__class__ == "RuntimeValue"; - var is_data_input = ["data", "data_collection"].indexOf(input.type) != -1; - var data_ref = context[input.data_ref]; + const isRuntimeValue = input.value && input.value.__class__ == "RuntimeValue"; + const isDataInput = ["data", "data_collection"].indexOf(input.type) != -1; + const dataRef = context[input.data_ref]; if (input.step_linked && !isDataStep(input.step_linked)) { - data_resolved = false; + dataResolved = false; } - if (input.options && ((input.options.length == 0 && !data_resolved) || input.wp_linked)) { + if (input.options && ((input.options.length == 0 && !dataResolved) || input.wp_linked)) { input.is_workflow = true; } - if (data_ref) { + if (dataRef) { input.is_workflow = - (data_ref.step_linked && !isDataStep(data_ref.step_linked)) || input.wp_linked; + (dataRef.step_linked && !isDataStep(dataRef.step_linked)) || input.wp_linked; } - if ( - !input.optional && - (is_data_input || - (input.value && input.value.__class__ == "RuntimeValue" && !input.step_linked)) - ) { + if ((isDataInput && !input.optional) || (!isDataInput && isRuntimeValue && !input.step_linked)) { step.expanded = true; hasOpenToolSteps = true; } - if (is_runtime_value) { + if (isRuntimeValue) { input.value = null; } input.flavor = "workflow"; - if (!is_runtime_value && !is_data_input && input.type !== "hidden" && !input.wp_linked) { + if (!isRuntimeValue && !isDataInput && input.type !== "hidden" && !input.wp_linked) { if (input.optional || (!isEmpty(input.value) && input.value !== "")) { input.collapsible_value = input.value; input.collapsible_preview = true; From 29da6fec23f16e1813b4a13cbff984a05bc32077 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 19 Nov 2024 13:26:17 +0100 Subject: [PATCH 16/36] Add selenium test for optional runtime parameter submission --- lib/galaxy_test/selenium/test_workflow_run.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/galaxy_test/selenium/test_workflow_run.py b/lib/galaxy_test/selenium/test_workflow_run.py index 047681aa0f45..5405545ce2e1 100644 --- a/lib/galaxy_test/selenium/test_workflow_run.py +++ b/lib/galaxy_test/selenium/test_workflow_run.py @@ -75,6 +75,29 @@ def test_runtime_parameters_simple(self): self._assert_has_3_lines_after_run(hid=2) + @selenium_test + @managed_history + def test_runtime_parameters_simple_optional(self): + self.workflow_run_open_workflow( + """ +class: GalaxyWorkflow +inputs: {} +steps: + int_step: + tool_id: expression_null_handling_integer + runtime_inputs: + - int_input +""" + ) + self.tool_parameter_div("int_input") + self._set_num_lines_to_3("int_input") + self.screenshot("workflow_run_optional_runtime_parameters_modified") + self.workflow_run_submit() + self.workflow_run_wait_for_ok(hid=1) + history_id = self.current_history_id() + content = self.dataset_populator.get_history_dataset_content(history_id, hid=1) + assert json.loads(content) == 3 + @selenium_test @managed_history def test_subworkflows_expanded(self): From 283c7fd139a76f205b98e8fb17f11327d0a994bd Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 19 Nov 2024 20:28:56 -0600 Subject: [PATCH 17/36] Move invocation view controls and loading indicator Used to be in the `tabs-end` slot. Here, we remove it from there and the BAlert and instead: - The icon on the top left is a loading one when workflow is running - The cancel button is next to the progress bars - We remove the "Generate PDF" button entirely Fixes https://github.com/galaxyproject/galaxy/issues/19158 --- .../Workflow/WorkflowNavigationTitle.vue | 6 +- .../WorkflowInvocationState.vue | 88 +++++++++---------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/client/src/components/Workflow/WorkflowNavigationTitle.vue b/client/src/components/Workflow/WorkflowNavigationTitle.vue index 4a5b9a413716..37c338c1ded2 100644 --- a/client/src/components/Workflow/WorkflowNavigationTitle.vue +++ b/client/src/components/Workflow/WorkflowNavigationTitle.vue @@ -1,5 +1,5 @@ From b3e57cb9a2cf7e4bb856c368270a935cb9c1e575 Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 31 Oct 2024 18:08:50 -0400 Subject: [PATCH 27/36] Cleanup sharing roles on user purge, refactor, test 1. Update sharing role name removing purged user email 2. If sharing role has only one user association, delete it. 3. Factor out cleanup, add test. Note: this will not work if user email has been udpated after the sharing role was created. --- lib/galaxy/managers/users.py | 29 ++++++++-- test/unit/data/model/db/test_user.py | 81 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index fc8644d1ba4e..53173519d088 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -40,6 +40,7 @@ from galaxy.managers.base import combine_lists from galaxy.model import ( Job, + Role, User, UserAddress, UserQuotaUsage, @@ -214,10 +215,7 @@ def purge(self, user, flush=True): # Delete UserGroupAssociations for uga in user.groups: self.session().delete(uga) - # Delete UserRoleAssociations EXCEPT FOR THE PRIVATE ROLE - for ura in user.roles: - if ura.role_id != private_role.id: - self.session().delete(ura) + _cleanup_nonprivate_user_roles(self.session(), user, private_role.id) # Delete UserAddresses for address in user.addresses: self.session().delete(address) @@ -891,3 +889,26 @@ def generate_next_available_username(session, username, model_class=User): while session.execute(select(model_class).where(model_class.username == f"{username}-{i}")).first(): i += 1 return f"{username}-{i}" + + +def _cleanup_nonprivate_user_roles(session, user, private_role_id): + """ + Delete UserRoleAssociations EXCEPT FOR THE PRIVATE ROLE; + Delete sharing roles that are associated with this user only; + Remove user email from sharing role names associated with multiple users. + + Note: this method updates the session without flushing or committing. + """ + user_roles = [ura for ura in user.roles if ura.role_id != private_role_id] + for user_role_assoc in user_roles: + role = user_role_assoc.role + if role.type == Role.types.SHARING: + if len(role.users) == 1: + # This role is associated with this user only, so we can delete it + session.delete(role) + elif user.email in role.name: + # Remove user email from sharing role's name + role.name = role.name.replace(user.email, "[USER PURGED]") + session.add(role) + # Delete user role association + session.delete(user_role_assoc) diff --git a/test/unit/data/model/db/test_user.py b/test/unit/data/model/db/test_user.py index 87d136a125a4..3a4d28286562 100644 --- a/test/unit/data/model/db/test_user.py +++ b/test/unit/data/model/db/test_user.py @@ -1,6 +1,12 @@ import pytest +from sqlalchemy import select from sqlalchemy.exc import IntegrityError +from galaxy.managers.users import _cleanup_nonprivate_user_roles +from galaxy.model import ( + Role, + UserRoleAssociation, +) from galaxy.model.db.user import ( get_user_by_email, get_user_by_username, @@ -80,3 +86,78 @@ def test_username_is_unique(make_user): make_user(username="a") with pytest.raises(IntegrityError): make_user(username="a") + + +def test_cleanup_nonprivate_user_roles(session, make_user_and_role, make_role, make_user_role_association): + # Create 3 users with private roles + user1, role_private1 = make_user_and_role(email="user1@foo.com") + user2, role_private2 = make_user_and_role(email="user2@foo.com") + user3, role_private3 = make_user_and_role(email="user3@foo.com") + + # Create role_sharing1 and associated it with user1 and user2 + role_sharing1 = make_role(type=Role.types.SHARING, name="sharing role for user1@foo.com, user2@foo.com") + make_user_role_association(user1, role_sharing1) + make_user_role_association(user2, role_sharing1) + + # Create role_sharing2 and associated it with user3 + role_sharing2 = make_role(type=Role.types.SHARING, name="sharing role for user3@foo.com") + make_user_role_association(user3, role_sharing2) + + # Create another role and associate it with all users + role6 = make_role() + make_user_role_association(user1, role6) + make_user_role_association(user2, role6) + make_user_role_association(user3, role6) + + # verify number of all user role associations + associations = session.scalars(select(UserRoleAssociation)).all() + assert len(associations) == 9 # 3 private, 2 + 1 sharing, 3 with role6 + + # verify user1 roles + assert len(user1.roles) == 3 + assert role_private1 in [ura.role for ura in user1.roles] + assert role_sharing1 in [ura.role for ura in user1.roles] + assert role6 in [ura.role for ura in user1.roles] + + # run cleanup on user user1 + _cleanup_nonprivate_user_roles(session, user1, role_private1.id) + session.commit() # must commit since method does not commit + + # private role not deleted, associations with role6 and with sharing role deleted, sharing role name updated + assert len(user1.roles) == 1 + assert user1.roles[0].id == role_private1.id + assert role_sharing1.name == "sharing role for [USER PURGED], user2@foo.com" + + # verify user2 roles + assert len(user2.roles) == 3 + assert role_private2 in [ura.role for ura in user2.roles] + assert role_sharing1 in [ura.role for ura in user2.roles] + assert role6 in [ura.role for ura in user2.roles] + + # run cleanup on user user2 + _cleanup_nonprivate_user_roles(session, user2, role_private2.id) + session.commit() + + # private role not deleted, association with sharing role deleted + assert len(user2.roles) == 1 + assert user2.roles[0].id == role_private2.id + # sharing role1 deleted since it has no more users associated with it + roles = session.scalars(select(Role)).all() + assert len(roles) == 5 # 3 private, role6, sharing2 + assert role_sharing1.id not in [role.id for role in roles] + + # verify user3 roles + assert len(user3.roles) == 3 + assert role_private3 in [ura.role for ura in user3.roles] + assert role_sharing2 in [ura.role for ura in user3.roles] + assert role6 in [ura.role for ura in user3.roles] + + # run cleanup on user user3 + _cleanup_nonprivate_user_roles(session, user3, role_private3.id) + session.commit() + + # remaining: 3 private roles + 3 associations, role6 + roles = session.scalars(select(Role)).all() + assert len(roles) == 4 + associations = session.scalars(select(UserRoleAssociation)).all() + assert len(associations) == 3 From 16f7700ea4d0bb4a1d6ff26e19b04cfaa00f69b2 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 1 Nov 2024 00:37:57 -0400 Subject: [PATCH 28/36] Move method out of managers into model To accommodate package unit testing --- lib/galaxy/managers/users.py | 25 +------------------------ lib/galaxy/model/db/user.py | 28 +++++++++++++++++++++++++++- test/unit/data/model/db/test_user.py | 2 +- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 53173519d088..d1f24e4cb0b6 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -40,13 +40,13 @@ from galaxy.managers.base import combine_lists from galaxy.model import ( Job, - Role, User, UserAddress, UserQuotaUsage, ) from galaxy.model.base import transaction from galaxy.model.db.user import ( + _cleanup_nonprivate_user_roles, get_user_by_email, get_user_by_username, ) @@ -889,26 +889,3 @@ def generate_next_available_username(session, username, model_class=User): while session.execute(select(model_class).where(model_class.username == f"{username}-{i}")).first(): i += 1 return f"{username}-{i}" - - -def _cleanup_nonprivate_user_roles(session, user, private_role_id): - """ - Delete UserRoleAssociations EXCEPT FOR THE PRIVATE ROLE; - Delete sharing roles that are associated with this user only; - Remove user email from sharing role names associated with multiple users. - - Note: this method updates the session without flushing or committing. - """ - user_roles = [ura for ura in user.roles if ura.role_id != private_role_id] - for user_role_assoc in user_roles: - role = user_role_assoc.role - if role.type == Role.types.SHARING: - if len(role.users) == 1: - # This role is associated with this user only, so we can delete it - session.delete(role) - elif user.email in role.name: - # Remove user email from sharing role's name - role.name = role.name.replace(user.email, "[USER PURGED]") - session.add(role) - # Delete user role association - session.delete(user_role_assoc) diff --git a/lib/galaxy/model/db/user.py b/lib/galaxy/model/db/user.py index 823247f725dd..00f824f5e1ee 100644 --- a/lib/galaxy/model/db/user.py +++ b/lib/galaxy/model/db/user.py @@ -8,7 +8,10 @@ true, ) -from galaxy.model import User +from galaxy.model import ( + Role, + User, +) from galaxy.model.scoped_session import galaxy_scoped_session @@ -64,3 +67,26 @@ def get_users_for_index( else: stmt = stmt.where(User.deleted == false()) return session.scalars(stmt).all() + + +def _cleanup_nonprivate_user_roles(session, user, private_role_id): + """ + Delete UserRoleAssociations EXCEPT FOR THE PRIVATE ROLE; + Delete sharing roles that are associated with this user only; + Remove user email from sharing role names associated with multiple users. + + Note: this method updates the session without flushing or committing. + """ + user_roles = [ura for ura in user.roles if ura.role_id != private_role_id] + for user_role_assoc in user_roles: + role = user_role_assoc.role + if role.type == Role.types.SHARING: + if len(role.users) == 1: + # This role is associated with this user only, so we can delete it + session.delete(role) + elif user.email in role.name: + # Remove user email from sharing role's name + role.name = role.name.replace(user.email, "[USER PURGED]") + session.add(role) + # Delete user role association + session.delete(user_role_assoc) diff --git a/test/unit/data/model/db/test_user.py b/test/unit/data/model/db/test_user.py index 3a4d28286562..042a541eb817 100644 --- a/test/unit/data/model/db/test_user.py +++ b/test/unit/data/model/db/test_user.py @@ -2,12 +2,12 @@ from sqlalchemy import select from sqlalchemy.exc import IntegrityError -from galaxy.managers.users import _cleanup_nonprivate_user_roles from galaxy.model import ( Role, UserRoleAssociation, ) from galaxy.model.db.user import ( + _cleanup_nonprivate_user_roles, get_user_by_email, get_user_by_username, get_users_by_ids, From 725c120428c0fa2e29be9d0c694b80f32963f294 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 1 Nov 2024 21:52:34 -0400 Subject: [PATCH 29/36] Cleanup wording, left over debug code --- lib/galaxy/webapps/galaxy/api/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index c9d4f8bd50ee..51b5dcea8ebd 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -734,7 +734,7 @@ def send_activation_email( @router.get( "/api/users/{user_id}/roles", name="get user roles", - description="Return a collection of roles associated with this user. Only admins can see user roles.", + description="Return a list of roles associated with this user. Only admins can see user roles.", require_admin=True, ) def get_user_roles( From df850ac7c86961b50f3c93890c6d5083bc593c78 Mon Sep 17 00:00:00 2001 From: John Davis Date: Sat, 2 Nov 2024 18:07:01 -0400 Subject: [PATCH 30/36] Rebuild schema --- client/src/api/schema/schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 7da851a70511..abc681fb8ca3 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -4804,7 +4804,7 @@ export interface paths { }; /** * Get User Roles - * @description Return a collection of roles associated with this user. Only admins can see user roles. + * @description Return a list of roles associated with this user. Only admins can see user roles. */ get: operations["get_user_roles_api_users__user_id__roles_get"]; put?: never; From 72b590a7f3ad74a61cdf1d05c73eed0540cd580e Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Wed, 20 Nov 2024 18:26:10 -0600 Subject: [PATCH 31/36] remove success message from `WorkflowRunSuccess` in case of 1 run and add it to `WorkflowNavigationTitle` where it fades out to reveal the title and actions --- .../Workflow/Run/WorkflowRunSuccess.vue | 4 +- .../Workflow/WorkflowNavigationTitle.vue | 53 ++++++++++++++----- .../WorkflowInvocationState.vue | 5 +- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/client/src/components/Workflow/Run/WorkflowRunSuccess.vue b/client/src/components/Workflow/Run/WorkflowRunSuccess.vue index c51868e53705..f5d71a820167 100644 --- a/client/src/components/Workflow/Run/WorkflowRunSuccess.vue +++ b/client/src/components/Workflow/Run/WorkflowRunSuccess.vue @@ -39,9 +39,9 @@ const wasNewHistoryTarget =