From 434301479208c96e519ba31ef62dd7eabe62ec1a Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 6 Jan 2021 16:11:16 +0100 Subject: [PATCH 1/6] add validation for collection parameters --- lib/galaxy/tools/parameters/basic.py | 62 +++++++++++++--- .../tools/validation_empty_dataset.xml | 73 +++++++++++++++---- 2 files changed, 107 insertions(+), 28 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index c756bba7a1f..4bf2ce3fc52 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1619,6 +1619,18 @@ class BaseDataToolParameter(ToolParameter): def __init__(self, tool, input_source, trans): super().__init__(tool, input_source) + self.min = input_source.get('min') + self.max = input_source.get('max') + if self.min: + try: + self.min = int(self.min) + except ValueError: + raise ParameterValueError("attribute 'min' must be an integer", self.name) + if self.max: + try: + self.max = int(self.max) + except ValueError: + raise ParameterValueError("attribute 'max' must be an integer", self.name) self.refresh_on_change = True # Find datatypes_registry if self.tool is None: @@ -1770,18 +1782,6 @@ class DataToolParameter(BaseDataToolParameter): self.validators.append(validation.MetadataValidator()) self._parse_formats(trans, input_source) self.multiple = input_source.get_bool('multiple', False) - self.min = input_source.get('min') - self.max = input_source.get('max') - if self.min: - try: - self.min = int(self.min) - except ValueError: - raise ParameterValueError("attribute 'min' must be an integer", self.name) - if self.max: - try: - self.max = int(self.max) - except ValueError: - raise ParameterValueError("attribute 'max' must be an integer", self.name) if not self.multiple and (self.min is not None): raise ParameterValueError("cannot specify 'min' property on single data parameter. Set multiple=\"true\" to enable this option", self.name) if not self.multiple and (self.max is not None): @@ -2176,6 +2176,44 @@ class DataCollectionToolParameter(BaseDataToolParameter): return display_text def validate(self, value, trans=None): + dataset_count = 0 + log.error("DataCollectionToolParameter validate %s %s" % (self.name, value)) + for validator in self.validators: + def do_validate(v): + if validator.requires_dataset_metadata and v and hasattr(v, 'dataset') and v.dataset.state != galaxy.model.Dataset.states.OK: + return + else: + validator.validate(v, trans) + + if not isinstance(value, list): + value = [value] + # TODO this code would be needed instead if multiple = true is possible + # if value and self.multiple: + # if not isinstance(value, list): + # value = [value] + # else: + # value = [value] + + for v in value: + if isinstance(v, galaxy.model.HistoryDatasetCollectionAssociation): + for dataset_instance in v.collection.dataset_instances: + dataset_count += 1 + do_validate(dataset_instance) + elif isinstance(v, galaxy.model.DatasetCollectionElement): + for dataset_instance in v.child_collection.dataset_instances: + dataset_count += 1 + do_validate(dataset_instance) + else: + if value: # this covers the case of optional="true" + dataset_count += 1 + do_validate(v) + + if self.min is not None: + if self.min > dataset_count: + raise ValueError("At least %d datasets are required for %s" % (self.min, self.name)) + if self.max is not None: + if self.max < dataset_count: + raise ValueError("At most %d datasets are required for %s" % (self.max, self.name)) return True # TODO def to_dict(self, trans, other_values=None): diff --git a/test/functional/tools/validation_empty_dataset.xml b/test/functional/tools/validation_empty_dataset.xml index 010f2bdd32d..ba609824f24 100644 --- a/test/functional/tools/validation_empty_dataset.xml +++ b/test/functional/tools/validation_empty_dataset.xml @@ -1,17 +1,58 @@ - - out1 - ]]> - - - - - - - - - - - - + + + echo "Hello World"; + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 08810b2174faf694db564e7e2a93d0f0d2e5fcf4 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 7 Jan 2021 10:07:21 +0100 Subject: [PATCH 2/6] add empty test file --- test-data/empty.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test-data/empty.txt diff --git a/test-data/empty.txt b/test-data/empty.txt new file mode 100644 index 00000000000..e69de29bb2d From b1a744c655be3e35852ac158905fc3926be521e1 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 7 Jan 2021 16:09:54 +0100 Subject: [PATCH 3/6] unify validate Data(Collection)ToolParameter as long as `multiple="True"` is not possible for DataCollectionParameter the code is exactly the same for both classes. --- lib/galaxy/tools/parameters/basic.py | 114 +++++++++------------------ 1 file changed, 37 insertions(+), 77 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 4bf2ce3fc52..4fe24d9c4b3 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1759,6 +1759,43 @@ class BaseDataToolParameter(ToolParameter): else: return app.model.context.query(app.model.HistoryDatasetAssociation).get(int(value)) + def validate(self, value, trans=None): + + def do_validate(v): + for validator in self.validators: + if validator.requires_dataset_metadata and v and hasattr(v, 'dataset') and v.dataset.state != galaxy.model.Dataset.states.OK: + return + else: + validator.validate(v, trans) + + dataset_count = 0 + if value: + if self.multiple: + if not isinstance(value, list): + value = [value] + else: + value = [value] + + for v in value: + if isinstance(v, galaxy.model.HistoryDatasetCollectionAssociation): + for dataset_instance in v.collection.dataset_instances: + dataset_count += 1 + do_validate(dataset_instance) + elif isinstance(v, galaxy.model.DatasetCollectionElement): + for dataset_instance in v.child_collection.dataset_instances: + dataset_count += 1 + do_validate(dataset_instance) + else: + dataset_count += 1 + do_validate(v) + + if self.min is not None: + if self.min > dataset_count: + raise ValueError("At least %d datasets are required for %s" % (self.min, self.name)) + if self.max is not None: + if self.max < dataset_count: + raise ValueError("At most %d datasets are required for %s" % (self.max, self.name)) + class DataToolParameter(BaseDataToolParameter): # TODO, Nate: Make sure the following unit tests appropriately test the dataset security @@ -1905,42 +1942,6 @@ class DataToolParameter(BaseDataToolParameter): pass return "No dataset." - def validate(self, value, trans=None): - dataset_count = 0 - for validator in self.validators: - def do_validate(v): - if validator.requires_dataset_metadata and v and hasattr(v, 'dataset') and v.dataset.state != galaxy.model.Dataset.states.OK: - return - else: - validator.validate(v, trans) - - if value and self.multiple: - if not isinstance(value, list): - value = [value] - for v in value: - if isinstance(v, galaxy.model.HistoryDatasetCollectionAssociation): - for dataset_instance in v.collection.dataset_instances: - dataset_count += 1 - do_validate(dataset_instance) - elif isinstance(v, galaxy.model.DatasetCollectionElement): - for dataset_instance in v.child_collection.dataset_instances: - dataset_count += 1 - do_validate(dataset_instance) - else: - dataset_count += 1 - do_validate(v) - else: - if value: - dataset_count += 1 - do_validate(value) - - if self.min is not None: - if self.min > dataset_count: - raise ValueError("At least %d datasets are required for %s" % (self.min, self.name)) - if self.max is not None: - if self.max < dataset_count: - raise ValueError("At most %d datasets are required for %s" % (self.max, self.name)) - def get_dependencies(self): """ Get the *names* of the other params this param depends on. @@ -2175,47 +2176,6 @@ class DataCollectionToolParameter(BaseDataToolParameter): display_text = "No dataset collection." return display_text - def validate(self, value, trans=None): - dataset_count = 0 - log.error("DataCollectionToolParameter validate %s %s" % (self.name, value)) - for validator in self.validators: - def do_validate(v): - if validator.requires_dataset_metadata and v and hasattr(v, 'dataset') and v.dataset.state != galaxy.model.Dataset.states.OK: - return - else: - validator.validate(v, trans) - - if not isinstance(value, list): - value = [value] - # TODO this code would be needed instead if multiple = true is possible - # if value and self.multiple: - # if not isinstance(value, list): - # value = [value] - # else: - # value = [value] - - for v in value: - if isinstance(v, galaxy.model.HistoryDatasetCollectionAssociation): - for dataset_instance in v.collection.dataset_instances: - dataset_count += 1 - do_validate(dataset_instance) - elif isinstance(v, galaxy.model.DatasetCollectionElement): - for dataset_instance in v.child_collection.dataset_instances: - dataset_count += 1 - do_validate(dataset_instance) - else: - if value: # this covers the case of optional="true" - dataset_count += 1 - do_validate(v) - - if self.min is not None: - if self.min > dataset_count: - raise ValueError("At least %d datasets are required for %s" % (self.min, self.name)) - if self.max is not None: - if self.max < dataset_count: - raise ValueError("At most %d datasets are required for %s" % (self.max, self.name)) - return True # TODO - def to_dict(self, trans, other_values=None): # create dictionary and fill default parameters other_values = other_values or {} From 7718b5c1afd2cf0f93fd491754ae26dc61b03ada Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 7 Jan 2021 17:11:42 +0100 Subject: [PATCH 4/6] document validator types --- lib/galaxy/tool_util/xsd/galaxy.xsd | 57 +++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 3a5dd199908..d9b991bab28 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -3504,10 +3504,61 @@ target file. `` tag set - it applies a +validator to the containing parameter. Tool submission will fail if a +single validator fails. See the [annotation_profiler](https://github.com/galaxyproject/tools-devteam/blob/master/tools/annotation_profiler/annotation_profiler.xml) -tool for an example of how to use this tag set. This tag set is contained within -the ```` tag set - it applies a validator to the containing parameter. +tool for an example of how to use this tag set. + +Note that validators for parameters with ``optional="true"`` are not +executed if no value is given. + +### Generic validators + +- ``expression``: Check if a one line python expression given expression +evaluates to True. The expression is given is the content of the validator tag. + +### Validators for ``data`` and ``data_collection`` parameters + +In case of ``data_collection`` parameters and +``data`` parameters with ``multiple="true"`` these validators are executed +separately for each of the contained data sets. Note that, for ``data`` +parameters a ``metadata`` validator is added automatically. + +- ``metadata``: Check for missing metadata. +- ``unspecified_build``: Check of a build is defined. +- ``dataset_ok_validator``: Check if the data set is in state OK. +- ``dataset_metadata_in_range``: Check if a numeric metadata value is within +a given range. +- ``dataset_metadata_in_file``: Check if a metadata value is contained in a +specific column of another data set. +- ``dataset_metadata_in_data_table`` (``dataset_metadata_not_in_data_table``): +Check if a metadata value is contained in a column of a data table. + +### Validators for textual inputs (``text``, ``select``, ...) + +``regex``: 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. + +For selects (in particular with dynamically defined options) the following +validator is useful: + +``no_options``: Check if options are available for a ``select`` parameter. +Useful for parameters with dynamically defined options. + +For ``text`` inputs the following validators are useful: + +``length``: Check if the length of the value is within a range. +``empty_field``: Check if the sting is not empty +``value_in_data_table`` (``value_not_in_data_table``): Check if the value is +contained in a column of a given data table. + +### Validators for numeric inputs (``integer``, ``float``) + +``in_range``: Check if the value is in a given range. ### Examples From 5a01f060d93c5f567433dce9d5c0e7514917a2d1 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 7 Jan 2021 17:21:03 +0100 Subject: [PATCH 5/6] add explicit formats to validation_empty_dataset.xml --- .../tools/validation_empty_dataset.xml | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/functional/tools/validation_empty_dataset.xml b/test/functional/tools/validation_empty_dataset.xml index ba609824f24..b30931fb40a 100644 --- a/test/functional/tools/validation_empty_dataset.xml +++ b/test/functional/tools/validation_empty_dataset.xml @@ -1,49 +1,49 @@ - echo "Hello World"; + echo "Hello World" > out1 - + - + - + - + - + - + - - + + - - + + - - + + From fd97fd4b5b13e51a1d06fae864c23719969d3100 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 8 Jan 2021 12:44:39 +0100 Subject: [PATCH 6/6] fix output assertion in test tool --- test/functional/tools/validation_empty_dataset.xml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/functional/tools/validation_empty_dataset.xml b/test/functional/tools/validation_empty_dataset.xml index b30931fb40a..e2c4f218f87 100644 --- a/test/functional/tools/validation_empty_dataset.xml +++ b/test/functional/tools/validation_empty_dataset.xml @@ -48,9 +48,11 @@ - - - + + + + +