From 7200c381c7f2cbc737ae9e8db4e03cb639b54cbd Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 20 Dec 2020 12:38:26 +0100 Subject: [PATCH 1/2] Add test for text param input value restriction As part of that, allow sending history_id to /api/workflows/{encoded_workflow_id}/download instead of relying on trans.history for the right history. --- lib/galaxy/managers/workflows.py | 10 ++++---- lib/galaxy/tools/__init__.py | 20 +++++----------- lib/galaxy/webapps/galaxy/api/tools.py | 8 +++++-- lib/galaxy/webapps/galaxy/api/workflows.py | 6 ++++- lib/galaxy_test/api/test_workflows.py | 27 ++++++++++++++++++++-- lib/galaxy_test/base/populators.py | 4 +++- 6 files changed, 50 insertions(+), 25 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index f0977d9e2d5..07327cf944b 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -481,7 +481,7 @@ class WorkflowContentsManager(UsesAnnotations): return workflow, missing_tool_tups - def workflow_to_dict(self, trans, stored, style="export", version=None): + def workflow_to_dict(self, trans, stored, style="export", version=None, history=None): """ Export the workflow contents to a dictionary ready for JSON-ification and to be sent out via API for instance. There are three styles of export allowed 'export', 'instance', and 'editor'. The Galaxy team will do its best to preserve the backward compatibility of the @@ -508,7 +508,7 @@ class WorkflowContentsManager(UsesAnnotations): elif style == "instance": wf_dict = self._workflow_to_dict_instance(stored, workflow=workflow, legacy=False) elif style == "run": - wf_dict = self._workflow_to_dict_run(trans, stored, workflow=workflow) + wf_dict = self._workflow_to_dict_run(trans, stored, workflow=workflow, history=history or trans.history) elif style == "preview": wf_dict = self._workflow_to_dict_preview(trans, workflow=workflow) elif style == "format2": @@ -539,7 +539,7 @@ class WorkflowContentsManager(UsesAnnotations): wf_dict = from_galaxy_native(wf_dict, None, json_wrapper=True) f.write(wf_dict["yaml_content"]) - def _workflow_to_dict_run(self, trans, stored, workflow): + def _workflow_to_dict_run(self, trans, stored, workflow, history=None): """ Builds workflow dictionary used by run workflow form """ @@ -583,7 +583,7 @@ class WorkflowContentsManager(UsesAnnotations): incoming = {} tool = trans.app.toolbox.get_tool(step.tool_id, tool_version=step.tool_version, tool_uuid=step.tool_uuid) params_to_incoming(incoming, tool.inputs, step.state.inputs, trans.app) - step_model = tool.to_json(trans, incoming, workflow_building_mode=workflow_building_modes.USE_HISTORY) + step_model = tool.to_json(trans, incoming, workflow_building_mode=workflow_building_modes.USE_HISTORY, history=history) step_model['post_job_actions'] = [{ 'short_str' : ActionBox.get_short_str(pja), 'action_type' : pja.action_type, @@ -614,7 +614,7 @@ class WorkflowContentsManager(UsesAnnotations): step_models.append(step_model) return { 'id': trans.app.security.encode_id(stored.id), - 'history_id': trans.app.security.encode_id(trans.history.id) if trans.history else None, + 'history_id': trans.app.security.encode_id(history.id) if history else None, 'name': stored.name, 'steps': step_models, 'step_version_changes': step_version_changes, diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 747e64a9421..6283d15bfb4 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -2112,27 +2112,19 @@ class Tool(Dictifiable): return tool_dict - def to_json(self, trans, kwd=None, job=None, workflow_building_mode=False): + def to_json(self, trans, kwd=None, job=None, workflow_building_mode=False, history=None): """ Recursively creates a tool dictionary containing repeats, dynamic options and updated states. """ if kwd is None: kwd = {} - history_id = kwd.get('history_id', None) - history = None if workflow_building_mode is workflow_building_modes.USE_HISTORY or workflow_building_mode is workflow_building_modes.DISABLED: # We don't need a history when exporting a workflow for the workflow editor or when downloading a workflow - try: - if history_id is not None: - history = self.history_manager.get_owned(trans.security.decode_id(history_id), trans.user, current_history=trans.history) - else: - history = trans.get_history() - if history is None and job is not None: - history = self.history_manager.get_owned(job.history.id, trans.user, current_history=trans.history) - if history is None: - raise exceptions.MessageException('History unavailable. Please specify a valid history id') - except Exception as e: - raise exceptions.MessageException('[history_id={}] Failed to retrieve history. {}.'.format(history_id, unicodify(e))) + history = history or trans.get_history() + if history is None and job is not None: + history = self.history_manager.get_owned(job.history.id, trans.user, current_history=trans.history) + if history is None: + raise exceptions.MessageException('History unavailable. Please specify a valid history id') # build request context request_context = WorkRequestContext(app=trans.app, user=trans.user, history=history, workflow_building_mode=workflow_building_mode) diff --git a/lib/galaxy/webapps/galaxy/api/tools.py b/lib/galaxy/webapps/galaxy/api/tools.py index 4b6adb72495..af58193a62e 100644 --- a/lib/galaxy/webapps/galaxy/api/tools.py +++ b/lib/galaxy/webapps/galaxy/api/tools.py @@ -118,9 +118,13 @@ class ToolsController(BaseAPIController, UsesVisualizationMixin): """ if 'payload' in kwd: kwd = kwd.get('payload') - tool_version = kwd.get('tool_version', None) + tool_version = kwd.get('tool_version') + history_id = kwd.pop('history_id', None) + history = None + if history_id: + history = self.history_manager.get_owned(self.decode_id(history_id), trans.user, current_history=trans.history) tool = self._get_tool(id, tool_version=tool_version, user=trans.user) - return tool.to_json(trans, kwd.get('inputs', kwd)) + return tool.to_json(trans, kwd.get('inputs', kwd), history=history) @web.require_admin @expose_api diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index ad9d50f7659..fc9dc2d24a4 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -473,7 +473,11 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn style = kwd.get("style", "export") download_format = kwd.get('format') version = kwd.get('version') - ret_dict = self.workflow_contents_manager.workflow_to_dict(trans, stored_workflow, style=style, version=version) + history_id = kwd.get('history_id') + history = None + if history_id: + history = self.history_manager.get_accessible(self.decode_id(history_id), trans.user, current_history=trans.history) + ret_dict = self.workflow_contents_manager.workflow_to_dict(trans, stored_workflow, style=style, version=version, history=history) if download_format == 'json-download': sname = stored_workflow.name sname = ''.join(c in util.FILENAME_VALID_CHARS and c or '_' for c in sname)[0:150] diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 28bbb140fb1..bcc26d45128 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -195,8 +195,8 @@ class BaseWorkflowsApiTestCase(ApiTestCase): jobs = self._history_jobs(history_id) self.assertEqual(len(jobs), n) - def _download_workflow(self, workflow_id, style=None): - return self.workflow_populator.download_workflow(workflow_id, style=style) + def _download_workflow(self, workflow_id, style=None, history_id=None): + return self.workflow_populator.download_workflow(workflow_id, style=style, history_id=history_id) def wait_for_invocation_and_jobs(self, history_id, workflow_id, invocation_id, assert_ok=True): state = self.workflow_populator.wait_for_invocation(workflow_id, invocation_id) @@ -3563,6 +3563,29 @@ steps: hda2 = self.dataset_populator.get_history_dataset_details(history_id, hid=2) assert hda2["validated_state"] == "invalid" + def test_value_restriction_with_select_and_text_param(self): + workflow_id = self.workflow_populator.upload_yaml_workflow(""" +class: GalaxyWorkflow +inputs: + select_text: + type: text + restrictOnConnections: true +steps: + select: + tool_id: multi_select + in: + select_ex: select_text + tool_with_text_input: + tool_id: param_text_option + in: + text_param: select_text +""") + with self.dataset_populator.test_history() as history_id: + run_workflow = self._download_workflow(workflow_id, style='run', history_id=history_id) + options = run_workflow['steps'][0]['inputs'][0]['options'] + assert len(options) == 5 + assert options[0] == ['Ex1', '--ex1', False] + @skip_without_tool("random_lines1") def test_run_replace_params_by_tool(self): workflow_request, history_id = self._setup_random_x2_workflow("test_for_replace_tool_params") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 4c29b96d352..3ff8aa949a0 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -882,10 +882,12 @@ class BaseWorkflowPopulator: api_asserts.assert_status_code_is(response, 200) return response.json() - def download_workflow(self, workflow_id, style=None): + def download_workflow(self, workflow_id, style=None, history_id=None): params = {} if style is not None: params["style"] = style + if history_id is not None: + params['history_id'] = history_id response = self._get("workflows/%s/download" % workflow_id, data=params) api_asserts.assert_status_code_is(response, 200) if style != "format2": From b7703ebafe8c8e130fd6a7f14ff53544cad91047 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 20 Dec 2020 12:40:08 +0100 Subject: [PATCH 2/2] Limit select value restriction to select parameters Only `SelectToolParameter` have a get_options attribute. All other parameters would fall through to the exception handling without the change when that is not necessary. Fixes https://github.com/galaxyproject/galaxy/issues/10974. --- lib/galaxy/workflow/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index ed74a32b81c..6c64c9a01c1 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -980,7 +980,7 @@ class InputParameterModule(WorkflowModule): tool_inputs = module.tool.inputs # may not be set, but we're catching the Exception below. def callback(input, prefixed_name, context, **kwargs): - if prefixed_name == connection.input_name: + if prefixed_name == connection.input_name and hasattr(input, 'get_options'): static_options.append(input.get_options(self.trans, {})) visit_input_values(tool_inputs, module.state.inputs, callback)