From 5ff960e9aee07814b9808adbb2fd03f481d63a53 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 2 Sep 2022 16:51:15 +0100 Subject: [PATCH] Fix comparing a boolean ``InputValueWrapper`` to "True"/"False" strings Fix the following framework test failures: test/functional/test_toolbox_pytest.py::test_tool[output_action_change_format_test_1] E galaxy.tool_util.verify.interactor.JobOutputsError: Dataset metadata verification for [metadata_dbkey] failed, expected [hg19] but found [?]. Dataset API value was [{'id': '8ac6d96e308f61e5', 'name': 'output_action_change_format on data 1', 'history_id': '2cf235fc3a3387c3', 'hid': 2, 'history_content_type': 'dataset', 'deleted': False, 'visible': True, 'type_id': 'dataset-8ac6d96e308f61e5', 'type': 'file', 'create_time': '2022-09-01T18:07:56.177858', 'update_time': '2022-09-01T18:08:01.856249', 'url': '/api/histories/2cf235fc3a3387c3/contents/8ac6d96e308f61e5', 'tags': [], 'dataset_id': '8ac6d96e308f61e5', 'state': 'ok', 'extension': 'data', 'purged': False, 'misc_info': '', 'data_type': 'galaxy.datatypes.data.Data', 'creating_job': '7fbe67cfae825002', 'hda_ldda': 'hda', 'peek': None, 'created_from_basename': 'out1', 'display_types': [], 'download_url': '/api/histories/2cf235fc3a3387c3/contents/8ac6d96e308f61e5/display', 'display_apps': [], 'meta_files': [], 'file_ext': 'data', 'sources': [], 'copied_from_ldda_id': None, 'uuid': '6decb7d7-1e5a-4036-9895-6cea83a85737', 'file_name': '/tmp/tmpdsxagstf/tmp84ke9cq4/tmpv3mcqttw/database/objects/6/d/e/dataset_6decb7d7-1e5a-4036-9895-6cea83a85737.dat', 'api_type': 'file', 'misc_blurb': 'data', 'hashes': [], 'accessible': True, 'file_size': 4, 'model_class': 'HistoryDatasetAssociation', 'metadata_dbkey': '?', 'validated_state_message': None, 'resubmitted': False, 'permissions': {'manage': ['adb5f5c93f827949'], 'access': []}, 'annotation': None, 'validated_state': 'unknown', 'rerunnable': True, 'genome_build': '?'}]. test/functional/test_toolbox_pytest.py::test_tool[output_action_change_format_test_2] E galaxy.tool_util.verify.interactor.JobOutputsError: Dataset metadata verification for [metadata_dbkey] failed, expected [hg38] but found [?]. Dataset API value was [{'id': 'b316627aece188a7', 'name': 'output_action_change_format on data 1', 'history_id': 'a53aa9d4dc32b5ed', 'hid': 2, 'history_content_type': 'dataset', 'deleted': False, 'visible': True, 'type_id': 'dataset-b316627aece188a7', 'type': 'file', 'create_time': '2022-09-01T18:08:12.870025', 'update_time': '2022-09-01T18:08:18.490136', 'url': '/api/histories/a53aa9d4dc32b5ed/contents/b316627aece188a7', 'tags': [], 'dataset_id': 'b316627aece188a7', 'state': 'ok', 'extension': 'txt', 'purged': False, 'misc_info': '', 'data_type': 'galaxy.datatypes.data.Text', 'creating_job': '3607dcbef30930ec', 'hda_ldda': 'hda', 'peek': '
1\t2
', 'created_from_basename': 'out1', 'display_types': [], 'download_url': '/api/histories/a53aa9d4dc32b5ed/contents/b316627aece188a7/display', 'display_apps': [], 'meta_files': [], 'file_ext': 'txt', 'sources': [], 'copied_from_ldda_id': None, 'uuid': '4a58c935-6fea-4b55-b659-871e652021de', 'file_name': '/tmp/tmpdsxagstf/tmp84ke9cq4/tmpv3mcqttw/database/objects/4/a/5/dataset_4a58c935-6fea-4b55-b659-871e652021de.dat', 'api_type': 'file', 'misc_blurb': '1 line', 'hashes': [], 'accessible': True, 'file_size': 4, 'model_class': 'HistoryDatasetAssociation', 'metadata_dbkey': '?', 'validated_state_message': None, 'resubmitted': False, 'permissions': {'manage': ['adb5f5c93f827949'], 'access': []}, 'annotation': None, 'validated_state': 'unknown', 'metadata_data_lines': 1, 'rerunnable': True, 'genome_build': '?'}]. that started to appear after commit 94c75a083c25612b5f9e4aac6fc53d0ac2c77253 was merged forward. In particular, for the above tests the `ref` variable at https://github.com/galaxyproject/galaxy/commit/94c75a083c25612b5f9e4aac6fc53d0ac2c77253#diff-0251b89ba313ad7ec78f79bfc21e416597cefd52447314029a728af5d775dbc6R85 is now a boolean `InputValueWrapper` instance, which could not be meaningfully compared to `value` if it was a "True" or "False" string because `ref` would be converted to the `truevalue` or `falsevalue` of the parameter when casted to `str`. --- .../tools/parameters/dynamic_options.py | 11 ++++--- lib/galaxy/tools/wrappers.py | 29 ++++++++++++------- test/unit/app/tools/test_wrappers.py | 2 ++ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 14360f3c666..ef4cbabd063 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -14,10 +14,6 @@ from galaxy.model import ( MetadataFile, User, ) -from galaxy.tools.wrappers import ( - DatasetFilenameWrapper, - DatasetListWrapper, -) from galaxy.util import string_as_bool from . import validation @@ -467,6 +463,8 @@ class RemoveValueFilter(Filter): self.separator = elem.get("separator", ",") def filter_options(self, options, trans, other_values): + from galaxy.tools.wrappers import DatasetFilenameWrapper + if trans is not None and trans.workflow_building_mode: return options @@ -803,6 +801,11 @@ def _get_ref_data(other_values, ref_name): - a KeyError is raised if no such element exists - a ValueError is raised if the element is not of the type DatasetFilenameWrapper, HistoryDatasetAssociation, DatasetListWrapper, HistoryDatasetCollectionAssociation, list """ + from galaxy.tools.wrappers import ( + DatasetFilenameWrapper, + DatasetListWrapper, + ) + ref = other_values[ref_name] if not isinstance( ref, diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index ecc26378775..528bee38085 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -27,13 +27,18 @@ from galaxy.model import ( HasTags, HistoryDatasetCollectionAssociation, ) +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.wrapped_json import ( data_collection_input_to_staging_path_and_source_path, data_input_to_staging_path_and_source_path, ) -from galaxy.util import filesystem_safe_string +from galaxy.util import ( + filesystem_safe_string, + string_as_bool, +) if TYPE_CHECKING: from galaxy.datatypes.registry import Registry @@ -121,25 +126,29 @@ class InputValueWrapper(ToolParameterValueWrapper): self.value = value self._other_values: Dict[str, str] = other_values or {} - def _get_cast_value(self, other: Any) -> Union[str, int, float, bool, None]: - if self.input.type == "boolean" and isinstance(other, str): - return str(self) + def _get_cast_values(self, other: Any) -> Tuple[Union[str, int, float, bool, None], Any]: + if isinstance(self.input, BooleanToolParameter) and isinstance(other, str): + if other in (self.input.truevalue, self.input.falsevalue): + return str(self), other + else: + return bool(self), string_as_bool(other) # For backward compatibility, allow `$wrapper != ""` for optional non-text param if self.input.optional and self.value is None: if isinstance(other, str): - return str(self) + return str(self), other else: - return None + return None, other cast_table = { "text": str, "integer": int, "float": float, "boolean": bool, } - return cast(Union[str, int, float, bool], cast_table.get(self.input.type, str)(self)) + return cast(Union[str, int, float, bool], cast_table.get(self.input.type, str)(self)), other def __eq__(self, other: Any) -> bool: - return bool(self._get_cast_value(other) == other) + casted_self, casted_other = self._get_cast_values(other) + return casted_self == casted_other def __ne__(self, other: Any) -> bool: return not self == other @@ -162,7 +171,8 @@ class InputValueWrapper(ToolParameterValueWrapper): return getattr(self.value, key) def __gt__(self, other: Any) -> bool: - return bool(self._get_cast_value(other) > other) + casted_self, casted_other = self._get_cast_values(other) + return casted_self > casted_other def __int__(self) -> int: return int(float(self)) @@ -290,7 +300,6 @@ class DatasetFilenameWrapper(ToolParameterValueWrapper): if rval is None: rval = self.metadata.spec[name].no_value metadata_param = self.metadata.spec[name].param - from galaxy.model.metadata import FileParameter rval = metadata_param.to_safe_string(rval) if isinstance(metadata_param, FileParameter) and self.compute_environment: diff --git a/test/unit/app/tools/test_wrappers.py b/test/unit/app/tools/test_wrappers.py index d094aaec5a3..c9a20f12e39 100644 --- a/test/unit/app/tools/test_wrappers.py +++ b/test/unit/app/tools/test_wrappers.py @@ -160,10 +160,12 @@ def test_input_value_wrapper_comparison(tool): assert bool(wrapper) is True, wrapper assert str(wrapper) == "truevalue" assert wrapper == "truevalue" + assert wrapper == "true" wrapper = valuewrapper(tool, False, "boolean") assert bool(wrapper) is False, wrapper assert str(wrapper) == "falsevalue" assert wrapper == "falsevalue" + assert wrapper == "false" @with_mock_tool