From 7f7710d3925564855fb520e35aed224a656ba0a3 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 13 Oct 2024 21:38:24 -0400 Subject: [PATCH 01/42] Implement recovering normalized workflow request from invocation. --- lib/galaxy/model/__init__.py | 65 ++++++++- lib/galaxy/schema/invocation.py | 64 ++++++++- lib/galaxy/schema/workflows.py | 126 +++++++++++------- lib/galaxy/webapps/galaxy/api/workflows.py | 12 ++ .../webapps/galaxy/services/invocations.py | 85 +++++++++++- lib/galaxy/workflow/extract.py | 4 +- lib/galaxy/workflow/run_request.py | 6 +- lib/galaxy_test/api/test_workflows.py | 56 +++++++- lib/galaxy_test/base/populators.py | 52 +++++++- .../workflow/test_framework_workflows.py | 4 + ...ection_with_user_preferred_object_store.py | 20 ++- test/unit/data/model/test_model_store.py | 4 +- 12 files changed, 427 insertions(+), 71 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 64e3c143e736..4bbc83f787fb 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -8015,7 +8015,7 @@ class WorkflowStep(Base, RepresentById, UsesCreateAndUpdateTime): type: Mapped[Optional[str]] = mapped_column(String(64)) tool_id: Mapped[Optional[str]] = mapped_column(TEXT) tool_version: Mapped[Optional[str]] = mapped_column(TEXT) - tool_inputs: Mapped[Optional[bytes]] = mapped_column(JSONType) + tool_inputs: Mapped[Optional[Dict[str, Any]]] = mapped_column(JSONType) tool_errors: Mapped[Optional[bytes]] = mapped_column(JSONType) position: Mapped[Optional[bytes]] = mapped_column(MutableJSONType) config: Mapped[Optional[bytes]] = mapped_column(JSONType) @@ -8085,11 +8085,13 @@ def init_on_load(self): def tool_uuid(self): return self.dynamic_tool and self.dynamic_tool.uuid + @property + def is_input_type(self) -> bool: + return bool(self.type and self.type in self.STEP_TYPE_TO_INPUT_TYPE) + @property def input_type(self): - assert ( - self.type and self.type in self.STEP_TYPE_TO_INPUT_TYPE - ), "step.input_type can only be called on input step types" + assert self.is_input_type, "step.input_type can only be called on input step types" return self.STEP_TYPE_TO_INPUT_TYPE[self.type] @property @@ -8310,6 +8312,17 @@ def log_str(self): f"WorkflowStep[index={self.order_index},type={self.type},label={self.label},uuid={self.uuid},id={self.id}]" ) + @property + def effective_label(self) -> Optional[str]: + label = self.label + if label is not None: + return label + elif self.is_input_type: + tool_inputs = self.tool_inputs + if tool_inputs is not None: + return cast(Optional[str], tool_inputs.get("name")) + return None + def clear_module_extras(self): # the module code adds random dynamic state to the step, this # attempts to clear that. @@ -9112,6 +9125,50 @@ def attach_step(request_to_content): attach_step(request_to_content) self.input_step_parameters.append(request_to_content) + def recover_inputs(self) -> Tuple[Dict[str, Any], str]: + inputs = {} + inputs_by = "name" + + have_referenced_steps_by_order_index = False + + def best_step_reference(workflow_step: "WorkflowStep") -> str: + label = workflow_step.effective_label + if label is not None: + return label + nonlocal have_referenced_steps_by_order_index + have_referenced_steps_by_order_index = True + return str(workflow_step.order_index) + + def ensure_step(step: Optional["WorkflowStep"]) -> "WorkflowStep": + if step is None: + raise galaxy.exceptions.InconsistentDatabase( + "workflow input found without step definition, this should not happen" + ) + assert step + return step + + for input_dataset_assoc in self.input_datasets: + workflow_step = ensure_step(input_dataset_assoc.workflow_step) + input_dataset = input_dataset_assoc.dataset + input_index = best_step_reference(workflow_step) + inputs[input_index] = input_dataset + + for input_dataset_collection_assoc in self.input_dataset_collections: + workflow_step = ensure_step(input_dataset_collection_assoc.workflow_step) + input_dataset_collection = input_dataset_collection_assoc.dataset_collection + input_index = best_step_reference(workflow_step) + inputs[input_index] = input_dataset_collection + + for input_step_parameter_assoc in self.input_step_parameters: + workflow_step = ensure_step(input_step_parameter_assoc.workflow_step) + value = input_step_parameter_assoc.parameter_value + input_index = best_step_reference(workflow_step) + inputs[input_index] = value + + if have_referenced_steps_by_order_index: + inputs_by = "name|step_index" + return inputs, inputs_by + def add_message(self, message: "InvocationMessageUnion"): self.messages.append( WorkflowInvocationMessage( # type:ignore[abstract] diff --git a/lib/galaxy/schema/invocation.py b/lib/galaxy/schema/invocation.py index 1040aac4d1b4..8872889ac078 100644 --- a/lib/galaxy/schema/invocation.py +++ b/lib/galaxy/schema/invocation.py @@ -47,6 +47,19 @@ UpdateTimeField, WithModelClass, ) +from .workflows import ( + INPUTS_BY_DESCRIPTION, + PreferredIntermediateObjectStoreIdField, + PreferredObjectStoreIdField, + PreferredOutputsObjectStoreIdField, + ReplacementParametersField, + ResourceParametersField, + STEP_PARAMETERS_DESCRIPTION, + STEP_PARAMETERS_NORMALIZED_DESCRIPTION, + STEP_PARAMETERS_NORMALIZED_TITLE, + STEP_PARAMETERS_TITLE, + UseCachedJobField, +) INVOCATION_STEP_OUTPUT_SRC = Literal["hda"] INVOCATION_STEP_COLLECTION_OUTPUT_SRC = Literal["hdca"] @@ -531,13 +544,16 @@ class InvocationOutputCollection(InvocationIOBase): ) +InvocationWorkflowIdField = Field( + title="Workflow ID", description="The encoded Workflow ID associated with the invocation." +) + + class WorkflowInvocationCollectionView(Model, WithModelClass): id: EncodedDatabaseIdField = InvocationIdField create_time: datetime = CreateTimeField update_time: datetime = UpdateTimeField - workflow_id: EncodedDatabaseIdField = Field( - title="Workflow ID", description="The encoded Workflow ID associated with the invocation." - ) + workflow_id: EncodedDatabaseIdField = InvocationWorkflowIdField history_id: EncodedDatabaseIdField = Field( default=..., title="History ID", @@ -583,6 +599,48 @@ class WorkflowInvocationResponse(RootModel): ] +class WorkflowInvocationRequestModel(Model): + """Model a workflow invocation request (InvokeWorkflowPayload) for an existing invocation.""" + + history_id: str = Field( + ..., + title="History ID", + description="The encoded history id the workflow was run in.", + ) + workflow_id: str = Field(title="Workflow ID", description="The encoded Workflow ID associated with the invocation.") + inputs: Dict[str, Any] = Field( + ..., + title="Inputs", + description="Values for inputs", + ) + inputs_by: str = Field( + ..., + title="Inputs by", + description=INPUTS_BY_DESCRIPTION, + ) + replacement_params: Optional[Dict[str, Any]] = ReplacementParametersField + resource_params: Optional[Dict[str, Any]] = ResourceParametersField + use_cached_job: bool = UseCachedJobField + preferred_object_store_id: Optional[str] = PreferredObjectStoreIdField + preferred_intermediate_object_store_id: Optional[str] = PreferredIntermediateObjectStoreIdField + preferred_outputs_object_store_id: Optional[str] = PreferredOutputsObjectStoreIdField + parameters_normalized: Literal[True] = Field( + True, + title=STEP_PARAMETERS_NORMALIZED_TITLE, + description=STEP_PARAMETERS_NORMALIZED_DESCRIPTION, + ) + parameters: Optional[Dict[str, Any]] = Field( + None, + title=STEP_PARAMETERS_TITLE, + description=f"{STEP_PARAMETERS_DESCRIPTION} If these are set, the workflow was not executed in a best-practice fashion and we the resulting invocation request may not fully reflect the executed workflow state.", + ) + instance: Literal[True] = Field( + True, + title="Is instance", + description="This API yields a particular workflow instance, newer workflows belonging to the same storedworkflow may have different state.", + ) + + class InvocationJobsSummaryBaseModel(Model): id: EncodedDatabaseIdField = InvocationIdField states: Dict[JobState, int] = Field( diff --git a/lib/galaxy/schema/workflows.py b/lib/galaxy/schema/workflows.py index 4e80dcb7c55e..960e2dc3e978 100644 --- a/lib/galaxy/schema/workflows.py +++ b/lib/galaxy/schema/workflows.py @@ -1,6 +1,7 @@ import json from typing import ( Any, + cast, Dict, List, Optional, @@ -8,6 +9,7 @@ ) from pydantic import ( + AfterValidator, Field, field_validator, ) @@ -22,13 +24,73 @@ Organization, PauseStep, Person, - PreferredObjectStoreIdField, StoredWorkflowSummary, SubworkflowStep, ToolStep, WorkflowInput, ) +TargetHistoryIdField = Field( + None, + title="History ID", + # description="The history to import the workflow into.", + description="The encoded history id into which to import.", +) +INPUTS_BY_DESCRIPTION = ( + "How the 'inputs' field maps its inputs (datasets/collections/step parameters) to workflows steps." +) +STEP_PARAMETERS_NORMALIZED_TITLE = "Legacy Step Parameters Normalized" +STEP_PARAMETERS_NORMALIZED_DESCRIPTION = "Indicates if legacy parameters are already normalized to be indexed by the order_index and are specified as a dictionary per step. Legacy-style parameters could previously be specified as one parameter per step or by tool ID." +STEP_PARAMETERS_TITLE = "Legacy Step Parameters" +STEP_PARAMETERS_DESCRIPTION = "Parameters specified per-step for the workflow invocation, this is legacy and you should generally use inputs and only specify the formal parameters of a workflow instead." +ReplacementParametersField = Field( + {}, + title="Replacement Parameters", + description="Class of parameters mostly used for string replacement in PJAs. In best practice workflows, these should be replaced with input parameters", +) +UseCachedJobField = Field( + False, + title="Use cached job", + description="Indicated whether to use a cached job for workflow invocation.", +) +PreferredObjectStoreIdField = Field( + default=None, + title="Preferred Object Store ID", + description="The ID of the object store that should be used to store all datasets (can instead specify object store IDs for intermediate and outputs datasts separately) - - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences", +) +PreferredIntermediateObjectStoreIdField = Field( + None, + title="Preferred Intermediate Object Store ID", + description="The ID of the object store that should be used to store the intermediate datasets of this workflow - - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences", +) +PreferredOutputsObjectStoreIdField = Field( + None, + title="Preferred Outputs Object Store ID", + description="The ID of the object store that should be used to store the marked output datasets of this workflow - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences.", +) +ResourceParametersField = Field( + {}, + title="Resource Parameters", + description="If a workflow_resource_params_file file is defined and the target workflow is configured to consumer resource parameters, they can be specified with this parameter. See https://github.com/galaxyproject/galaxy/pull/4830 for more information.", +) + +VALID_INPUTS_BY_ITEMS = ["step_id", "step_index", "step_uuid", "name"] + + +def validateInputsBy(inputsBy: Optional[str]) -> Optional[str]: + if inputsBy is not None: + if not isinstance(inputsBy, str): + raise ValueError(f"Invalid type for inputsBy {inputsBy}") + inputsByStr = cast(str, inputsBy) + inputsByArray: List[str] = inputsByStr.split("|") + for inputsByItem in inputsByArray: + if inputsByItem not in VALID_INPUTS_BY_ITEMS: + raise ValueError(f"Invalid inputsBy delineation {inputsByItem}") + return inputsBy + + +InputsByValidator = AfterValidator(validateInputsBy) + class GetTargetHistoryPayload(Model): # TODO - Are the descriptions correct? @@ -38,12 +100,7 @@ class GetTargetHistoryPayload(Model): # description="The encoded history id - passed exactly like this 'hist_id=...' - to import the workflow into. Or the name of the new history to import the workflow into.", description="The encoded history id - passed exactly like this 'hist_id=...' - into which to import. Or the name of the new history into which to import.", ) - history_id: Optional[str] = Field( - None, - title="History ID", - # description="The history to import the workflow into.", - description="The encoded history id into which to import.", - ) + history_id: Optional[str] = TargetHistoryIdField new_history_name: Optional[str] = Field( None, title="New History Name", @@ -85,15 +142,11 @@ class InvokeWorkflowPayload(GetTargetHistoryPayload): title="Allow tool state corrections", description="Indicates if tool state corrections are allowed for workflow invocation.", ) - use_cached_job: Optional[bool] = Field( - False, - title="Use cached job", - description="Indicated whether to use a cached job for workflow invocation.", - ) + use_cached_job: Optional[bool] = UseCachedJobField parameters_normalized: Optional[bool] = Field( False, - title="Parameters Normalized", - description="Indicates if parameters are already normalized for workflow invocation.", + title=STEP_PARAMETERS_NORMALIZED_TITLE, + description=STEP_PARAMETERS_NORMALIZED_DESCRIPTION, ) @field_validator( @@ -102,7 +155,6 @@ class InvokeWorkflowPayload(GetTargetHistoryPayload): "ds_map", "resource_params", "replacement_params", - "step_parameters", mode="before", check_fields=False, ) @@ -114,34 +166,22 @@ def inputs_string_to_json(cls, v): parameters: Optional[Dict[str, Any]] = Field( {}, - title="Parameters", - description="The raw parameters for the workflow invocation.", + title=STEP_PARAMETERS_TITLE, + description=STEP_PARAMETERS_DESCRIPTION, ) inputs: Optional[Dict[str, Any]] = Field( None, title="Inputs", - description="TODO", + description="Specify values for formal inputs to the workflow", ) ds_map: Optional[Dict[str, Dict[str, Any]]] = Field( {}, - title="Dataset Map", - description="TODO", - ) - resource_params: Optional[Dict[str, Any]] = Field( - {}, - title="Resource Parameters", - description="TODO", - ) - replacement_params: Optional[Dict[str, Any]] = Field( - {}, - title="Replacement Parameters", - description="TODO", - ) - step_parameters: Optional[Dict[str, Any]] = Field( - None, - title="Step Parameters", - description="TODO", + title="Legacy Dataset Map", + description="An older alternative to specifying inputs using database IDs, do not use this and use inputs instead", + deprecated=True, ) + resource_params: Optional[Dict[str, Any]] = ResourceParametersField + replacement_params: Optional[Dict[str, Any]] = ReplacementParametersField no_add_to_history: Optional[bool] = Field( False, title="No Add to History", @@ -152,11 +192,11 @@ def inputs_string_to_json(cls, v): title="Legacy", description="Indicating if to use legacy workflow invocation.", ) - inputs_by: Optional[str] = Field( + inputs_by: Annotated[Optional[str], InputsByValidator] = Field( None, title="Inputs By", # lib/galaxy/workflow/run_request.py - see line 60 - description="How inputs maps to inputs (datasets/collections) to workflows steps.", + description=INPUTS_BY_DESCRIPTION, ) effective_outputs: Optional[Any] = Field( None, @@ -164,17 +204,9 @@ def inputs_string_to_json(cls, v): # lib/galaxy/workflow/run_request.py - see line 455 description="TODO", ) - preferred_intermediate_object_store_id: Optional[str] = Field( - None, - title="Preferred Intermediate Object Store ID", - description="The ID of the ? object store that should be used to store ? datasets in this history.", - ) - preferred_outputs_object_store_id: Optional[str] = Field( - None, - title="Preferred Outputs Object Store ID", - description="The ID of the object store that should be used to store ? datasets in this history.", - ) preferred_object_store_id: Optional[str] = PreferredObjectStoreIdField + preferred_intermediate_object_store_id: Optional[str] = PreferredIntermediateObjectStoreIdField + preferred_outputs_object_store_id: Optional[str] = PreferredOutputsObjectStoreIdField class StoredWorkflowDetailed(StoredWorkflowSummary): diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 719029e818d6..703d652f0f74 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -64,6 +64,7 @@ InvocationStepJobsResponseJobModel, InvocationStepJobsResponseStepModel, InvocationUpdatePayload, + WorkflowInvocationRequestModel, WorkflowInvocationResponse, ) from galaxy.schema.schema import ( @@ -1469,6 +1470,17 @@ def show_invocation( ) return self.invocations_service.show(trans, invocation_id, serialization_params, eager=True) + @router.get( + "/api/invocations/{invocation_id}/request", + summary="Get a description modeling an API request to invoke this workflow - this is recreated and will be more specific in some ways than the initial creation request.", + ) + def invocation_as_request( + self, + invocation_id: InvocationIDPathParam, + trans: ProvidesUserContext = DependsOnTrans, + ) -> WorkflowInvocationRequestModel: + return self.invocations_service.as_request(trans, invocation_id) + @router.get( "/api/workflows/{workflow_id}/invocations/{invocation_id}", summary="Get detailed description of a workflow invocation.", diff --git a/lib/galaxy/webapps/galaxy/services/invocations.py b/lib/galaxy/webapps/galaxy/services/invocations.py index 2195c7202dd7..ce9610ac0422 100644 --- a/lib/galaxy/webapps/galaxy/services/invocations.py +++ b/lib/galaxy/webapps/galaxy/services/invocations.py @@ -1,3 +1,4 @@ +import json import logging from typing import ( Any, @@ -14,9 +15,13 @@ ) from galaxy.exceptions import ( AdminRequiredException, + InconsistentDatabase, ObjectNotFound, ) -from galaxy.managers.context import ProvidesHistoryContext +from galaxy.managers.context import ( + ProvidesHistoryContext, + ProvidesUserContext, +) from galaxy.managers.histories import HistoryManager from galaxy.managers.jobs import ( fetch_job_states, @@ -24,8 +29,11 @@ ) from galaxy.managers.workflows import WorkflowsManager from galaxy.model import ( + HistoryDatasetAssociation, + HistoryDatasetCollectionAssociation, WorkflowInvocation, WorkflowInvocationStep, + WorkflowRequestInputParameter, ) from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.invocation import ( @@ -33,6 +41,7 @@ InvocationSerializationParams, InvocationSerializationView, InvocationStep, + WorkflowInvocationRequestModel, WorkflowInvocationResponse, ) from galaxy.schema.schema import ( @@ -133,6 +142,12 @@ def show(self, trans, invocation_id, serialization_params, eager=False): ) return self.serialize_workflow_invocation(wfi, serialization_params) + def as_request(self, trans: ProvidesUserContext, invocation_id) -> WorkflowInvocationRequestModel: + wfi = self._workflows_manager.get_invocation( + trans, invocation_id, True, check_ownership=True, check_accessible=True + ) + return self.serialize_workflow_invocation_to_request(trans, wfi) + def cancel(self, trans, invocation_id, serialization_params): wfi = self._workflows_manager.request_invocation_cancellation(trans, invocation_id) return self.serialize_workflow_invocation(wfi, serialization_params) @@ -252,3 +267,71 @@ def create_from_store( history=history, ) return self.serialize_workflow_invocations(object_tracker.invocations_by_key.values(), serialization_params) + + def serialize_workflow_invocation_to_request( + self, trans: ProvidesUserContext, invocation: WorkflowInvocation + ) -> WorkflowInvocationRequestModel: + history_id = trans.security.encode_id(invocation.history.id) + workflow_id = trans.security.encode_id(invocation.workflow.id) + inputs, inputs_by = invocation.recover_inputs() + export_inputs = {} + for key, value in inputs.items(): + if isinstance(value, HistoryDatasetAssociation): + export_inputs[key] = {"src": "hda", "id": trans.security.encode_id(value.id)} + elif isinstance(value, HistoryDatasetCollectionAssociation): + export_inputs[key] = {"src": "hdca", "id": trans.security.encode_id(value.id)} + else: + export_inputs[key] = value + + param_types = WorkflowRequestInputParameter.types + steps_by_id = invocation.workflow.steps_by_id + + replacement_dict = {} + resource_params = {} + use_cached_job = False + preferred_object_store_id = None + preferred_intermediate_object_store_id = None + preferred_outputs_object_store_id = None + step_param_map: Dict[str, Dict] = {} + for parameter in invocation.input_parameters: + parameter_type = parameter.type + + if parameter_type == param_types.REPLACEMENT_PARAMETERS: + replacement_dict[parameter.name] = parameter.value + elif parameter_type == param_types.META_PARAMETERS: + # copy_inputs_to_history is being skipped here sort of intentionally, + # we wouldn't want to include this on re-run. + if parameter.name == "use_cached_job": + use_cached_job = parameter.value == "true" + if parameter.name == "preferred_object_store_id": + preferred_object_store_id = parameter.value + if parameter.name == "preferred_intermediate_object_store_id": + preferred_intermediate_object_store_id = parameter.value + if parameter.name == "preferred_outputs_object_store_id": + preferred_outputs_object_store_id = parameter.value + elif parameter_type == param_types.RESOURCE_PARAMETERS: + resource_params[parameter.name] = parameter.value + elif parameter_type == param_types.STEP_PARAMETERS: + # TODO: test subworkflows and ensure this works + step_id: int + try: + step_id = int(parameter.name) + except TypeError: + raise InconsistentDatabase("saved workflow step parameters not in the format expected") + step_param_map[str(steps_by_id[step_id].order_index)] = json.loads(parameter.value) + + return WorkflowInvocationRequestModel( + history_id=history_id, + workflow_id=workflow_id, + instance=True, # this is a workflow ID and not a stored workflow ID + inputs=export_inputs, + inputs_by=inputs_by, + replacement_params=None if not replacement_dict else replacement_dict, + resource_params=None if not resource_params else resource_params, + use_cached_job=use_cached_job, + preferred_object_store_id=preferred_object_store_id, + preferred_intermediate_object_store_id=preferred_intermediate_object_store_id, + preferred_outputs_object_store_id=preferred_outputs_object_store_id, + parameters_normalized=True, + parameters=None if not step_param_map else step_param_map, + ) diff --git a/lib/galaxy/workflow/extract.py b/lib/galaxy/workflow/extract.py index 6072f322d9cb..d96be13b35cc 100644 --- a/lib/galaxy/workflow/extract.py +++ b/lib/galaxy/workflow/extract.py @@ -116,7 +116,7 @@ def extract_steps( if name not in step_labels: step.label = name step_labels.add(name) - step.tool_inputs = dict(name=name) # type:ignore[assignment] + step.tool_inputs = dict(name=name) hid_to_output_pair[hid] = (step, "output") steps.append(step) for i, hid in enumerate(dataset_collection_ids): @@ -132,7 +132,7 @@ def extract_steps( if name not in step_labels: step.label = name step_labels.add(name) - step.tool_inputs = dict(name=name, collection_type=collection_type) # type:ignore[assignment] + step.tool_inputs = dict(name=name, collection_type=collection_type) hid_to_output_pair[hid] = (step, "output") steps.append(step) # Tool steps diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index 24b81fff849d..b8f6b753a924 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -121,7 +121,9 @@ def _normalize_inputs( elif inputs_by_el == "step_uuid": possible_input_keys.append(str(step.uuid)) elif inputs_by_el == "name": - possible_input_keys.append(step.label or step.tool_inputs.get("name")) # type:ignore[union-attr] + label = step.effective_label + if label: + possible_input_keys.append(label) else: raise exceptions.MessageException( "Workflow cannot be run because unexpected inputs_by value specified." @@ -317,7 +319,7 @@ def build_workflow_run_configs( add_to_history = "no_add_to_history" not in payload legacy = payload.get("legacy", False) already_normalized = payload.get("parameters_normalized", False) - raw_parameters = payload.get("parameters", {}) + raw_parameters = payload.get("parameters") or {} requires_materialization: bool = False run_configs = [] unexpanded_param_map = _normalize_step_parameters( diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 8408837df347..14b8b99b17dd 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -3460,6 +3460,31 @@ def test_workflow_request(self): invocation_id = run_workflow_response.json()["id"] self.workflow_populator.wait_for_invocation_and_jobs(history_id, workflow_id, invocation_id) + def test_workflow_request_recover(self): + workflow = self.workflow_populator.load_workflow(name="test_for_queue") + workflow_request, history_id, workflow_id = self._setup_workflow_run(workflow) + run_workflow_response = self.workflow_populator.invoke_workflow_raw( + workflow_id, workflow_request, assert_ok=True + ) + invocation_id = run_workflow_response.json()["id"] + self.workflow_populator.wait_for_invocation_and_jobs(history_id, workflow_id, invocation_id) + request = self.workflow_populator.invocation_to_request(invocation_id) + assert request["history_id"] == history_id + assert request["replacement_params"] is None + assert request["use_cached_job"] is False + assert request["preferred_object_store_id"] is None + assert request["preferred_intermediate_object_store_id"] is None + assert request["preferred_outputs_object_store_id"] is None + assert request["parameters_normalized"] is True + assert request["parameters"] is None + + assert request["inputs"]["WorkflowInput1"]["src"] == "hda" + encoded_id = request["inputs"]["WorkflowInput1"]["id"] + assert self.dataset_populator.get_history_dataset_content(history_id, dataset_id=encoded_id).strip() == "1 2 3" + assert request["inputs"]["WorkflowInput2"]["src"] == "hda" + encoded_id = request["inputs"]["WorkflowInput2"]["id"] + assert self.dataset_populator.get_history_dataset_content(history_id, dataset_id=encoded_id).strip() == "4 5 6" + def test_workflow_new_autocreated_history(self): workflow = self.workflow_populator.load_workflow(name="test_for_new_autocreated_history") workflow_request, history_id, workflow_id = self._setup_workflow_run(workflow) @@ -4898,7 +4923,7 @@ def test_run_with_non_optional_data_unspecified_fails_invocation(self): def test_run_with_optional_collection_specified(self): with self.dataset_populator.test_history() as history_id: - self._run_jobs( + result = self._run_workflow( WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, test_data=""" input1: @@ -4919,6 +4944,12 @@ def test_run_with_optional_collection_specified(self): content = self.dataset_populator.get_history_dataset_content(history_id) assert "GAATTGATCAGGACATAGGACAACTGTAGGCACCAT" in content + invocation_id = result.invocation_id + request = self.workflow_populator.invocation_to_request(invocation_id) + assert request["history_id"] == history_id + assert request["inputs"]["input1"]["src"] == "hdca" + assert request["inputs"]["input1"]["id"] + def test_run_with_optional_collection_unspecified(self): with self.dataset_populator.test_history() as history_id: self._run_jobs( @@ -5008,9 +5039,14 @@ def test_run_with_int_parameter(self): # self.dataset_populator.wait_for_history(history_id, assert_ok=True) content = self.dataset_populator.get_history_dataset_content(history_id) assert len(content.splitlines()) == 1, content - invocation = self.workflow_populator.get_invocation(run_response.invocation_id) + invocation_id = run_response.invocation_id + invocation = self.workflow_populator.get_invocation(invocation_id) assert invocation["input_step_parameters"]["int_input"]["parameter_value"] == 1 + request = self.workflow_populator.invocation_to_request(invocation_id) + assert request["history_id"] == history_id + assert request["inputs"]["int_input"] == 1 + run_response = self._run_workflow( WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL, test_data=""" @@ -5359,6 +5395,10 @@ def test_workflow_rerun_with_use_cached_job(self): workflow_id, new_workflow_request, assert_ok=True ).json() invocation_id = new_workflow_response["id"] + + request = self.workflow_populator.invocation_to_request(invocation_id) + assert request["use_cached_job"] is True + self.workflow_populator.wait_for_invocation_and_jobs(history_id_two, workflow_id, invocation_id) # get_history_dataset_details defaults to last item in history, so since we've done @@ -5558,6 +5598,9 @@ def test_run_with_pja(self): content = self.dataset_populator.get_history_dataset_details(history_id, wait=True, assert_ok=True) assert content["name"] == "foo was replaced" + request = self.workflow_populator.invocation_to_request(invocation_id) + assert request["replacement_params"]["replaceme"] == "was replaced" + @skip_without_tool("hidden_param") def test_hidden_param_in_workflow(self): with self.dataset_populator.test_history() as history_id: @@ -7060,7 +7103,7 @@ def test_run_replace_params_nested_normalized(self): @skip_without_tool("random_lines1") def test_run_replace_params_over_default(self): with self.dataset_populator.test_history() as history_id: - self._run_jobs( + wf_run = self._run_workflow( WORKFLOW_ONE_STEP_DEFAULT, test_data=""" step_parameters: @@ -7078,6 +7121,13 @@ def test_run_replace_params_over_default(self): result = self.dataset_populator.get_history_dataset_content(history_id) assert result.count("\n") == 4 + request = self.workflow_populator.invocation_to_request(wf_run.invocation_id) + assert request["parameters"]["1"]["num_lines"] == 4 + + self.workflow_populator.rerun(wf_run) + result = self.dataset_populator.get_history_dataset_content(history_id) + assert result.count("\n") == 4 + @skip_without_tool("random_lines1") def test_defaults_editor(self): workflow_id = self._upload_yaml_workflow(WORKFLOW_ONE_STEP_DEFAULT, publish=True) diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index a31c5fe73956..2ec7dbe8b533 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -1953,7 +1953,7 @@ def invoke_workflow_raw(self, workflow_id: str, request: dict, assert_ok: bool = url = f"workflows/{workflow_id}/invocations" invocation_response = self._post(url, data=request, json=True) if assert_ok: - invocation_response.raise_for_status() + api_asserts.assert_status_code_is_ok(invocation_response) return invocation_response def invoke_workflow( @@ -2042,6 +2042,11 @@ def download_workflow( else: return ordered_load(response.text) + def invocation_to_request(self, invocation_id: str): + request_response = self._get(f"invocations/{invocation_id}/request") + api_asserts.assert_status_code_is_ok(request_response) + return request_response.json() + def set_tags(self, workflow_id: str, tags: List[str]) -> None: update_payload = {"tags": tags} response = self.update_workflow(workflow_id, update_payload) @@ -2140,7 +2145,51 @@ def run_workflow( workflow_request.update(extra_invocation_kwds) if has_uploads: self.dataset_populator.wait_for_history(history_id, assert_ok=True) + + return self._request_to_summary( + history_id, + workflow_id, + workflow_request, + inputs=inputs, + wait=wait, + assert_ok=assert_ok, + invocations=invocations, + expected_response=expected_response, + ) + + def rerun(self, run_jobs_summary: "RunJobsSummary", wait: bool = True, assert_ok: bool = True) -> "RunJobsSummary": + history_id = run_jobs_summary.history_id + invocation_id = run_jobs_summary.invocation_id + inputs = run_jobs_summary.inputs + workflow_request = self.invocation_to_request(invocation_id) + workflow_id = workflow_request["workflow_id"] + assert workflow_request["history_id"] == history_id + assert workflow_request["instance"] is True + return self._request_to_summary( + history_id, + workflow_id, + workflow_request, + inputs=inputs, + wait=wait, + assert_ok=assert_ok, + invocations=1, + expected_response=200, + ) + + def _request_to_summary( + self, + history_id: str, + workflow_id: str, + workflow_request: Dict[str, Any], + inputs, + wait: bool, + assert_ok: bool, + invocations: int, + expected_response: int, + ): + workflow_populator = self assert invocations > 0 + jobs = [] for _ in range(invocations): invocation_response = workflow_populator.invoke_workflow_raw(workflow_id, workflow_request) @@ -2156,6 +2205,7 @@ def run_workflow( if wait: workflow_populator.wait_for_workflow(workflow_id, invocation_id, history_id, assert_ok=assert_ok) jobs.extend(self.dataset_populator.invocation_jobs(invocation_id)) + return RunJobsSummary( history_id=history_id, workflow_id=workflow_id, diff --git a/lib/galaxy_test/workflow/test_framework_workflows.py b/lib/galaxy_test/workflow/test_framework_workflows.py index a358433025ca..264f039d2751 100644 --- a/lib/galaxy_test/workflow/test_framework_workflows.py +++ b/lib/galaxy_test/workflow/test_framework_workflows.py @@ -21,6 +21,7 @@ get_metadata_to_test, verify_collection, ) +from galaxy.util import asbool from galaxy_test.api._framework import ApiTestCase from galaxy_test.base.populators import ( DatasetCollectionPopulator, @@ -30,6 +31,7 @@ ) SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) +TEST_WORKFLOW_AFTER_RERUN = asbool(os.environ.get("GALAXY_TEST_WORKFLOW_AFTER_RERUN", "0")) def find_workflows(): @@ -67,6 +69,8 @@ def test_workflow(self, workflow_path: Path, test_job: TestJobDict): test_data=test_job["job"], history_id=history_id, ) + if TEST_WORKFLOW_AFTER_RERUN: + run_summary = self.workflow_populator.rerun(run_summary) self._verify(run_summary, test_job["outputs"]) def _verify(self, run_summary: RunJobsSummary, output_definitions: OutputsDict): diff --git a/test/integration/objectstore/test_selection_with_user_preferred_object_store.py b/test/integration/objectstore/test_selection_with_user_preferred_object_store.py index 5bf562bc8b5b..fee15742b8a2 100644 --- a/test/integration/objectstore/test_selection_with_user_preferred_object_store.py +++ b/test/integration/objectstore/test_selection_with_user_preferred_object_store.py @@ -244,17 +244,22 @@ def _run_tool(tool_id, inputs, preferred_object_store_id=None): def test_workflow_objectstore_selection(self): with self.dataset_populator.test_history() as history_id: - output_dict, intermediate_dict = self._run_workflow_get_output_storage_info_dicts(history_id) + output_dict, intermediate_dict, _ = self._run_workflow_get_output_storage_info_dicts(history_id) assert_storage_name_is(output_dict, "Default Store") assert_storage_name_is(intermediate_dict, "Default Store") - output_dict, intermediate_dict = self._run_workflow_get_output_storage_info_dicts( + output_dict, intermediate_dict, wf_run = self._run_workflow_get_output_storage_info_dicts( history_id, {"preferred_object_store_id": "static"} ) assert_storage_name_is(output_dict, "Static Storage") assert_storage_name_is(intermediate_dict, "Static Storage") - output_dict, intermediate_dict = self._run_workflow_get_output_storage_info_dicts( + request = self.workflow_populator.invocation_to_request(wf_run.invocation_id) + assert request["preferred_object_store_id"] == "static" + assert request["preferred_outputs_object_store_id"] is None + assert request["preferred_intermediate_object_store_id"] is None + + output_dict, intermediate_dict, wf_run = self._run_workflow_get_output_storage_info_dicts( history_id, { "preferred_outputs_object_store_id": "static", @@ -264,6 +269,11 @@ def test_workflow_objectstore_selection(self): assert_storage_name_is(output_dict, "Static Storage") assert_storage_name_is(intermediate_dict, "Dynamic EBS") + request = self.workflow_populator.invocation_to_request(wf_run.invocation_id) + assert request["preferred_object_store_id"] is None + assert request["preferred_outputs_object_store_id"] == "static" + assert request["preferred_intermediate_object_store_id"] == "dynamic_ebs" + def test_simple_subworkflow_objectstore_selection(self): with self.dataset_populator.test_history() as history_id: output_dict, intermediate_dict = self._run_simple_nested_workflow_get_output_storage_info_dicts( @@ -428,7 +438,6 @@ def _run_nested_workflow_with_effective_output_get_output_storage_info_dicts( extra_invocation_kwds=extra_invocation_kwds, ) jobs = wf_run.jobs_for_tool("cat1") - print(jobs) assert len(jobs) == 2 intermediate_info = self._storage_info_for_job_id(jobs[1]["id"]) @@ -448,11 +457,10 @@ def _run_workflow_get_output_storage_info_dicts( extra_invocation_kwds=extra_invocation_kwds, ) jobs = wf_run.jobs_for_tool("cat") - print(jobs) assert len(jobs) == 2 output_info = self._storage_info_for_job_id(jobs[0]["id"]) intermediate_info = self._storage_info_for_job_id(jobs[1]["id"]) - return output_info, intermediate_info + return output_info, intermediate_info, wf_run def _storage_info_for_job_id(self, job_id: str) -> Dict[str, Any]: job_dict = self.dataset_populator.get_job_details(job_id, full=True).json() diff --git a/test/unit/data/model/test_model_store.py b/test/unit/data/model/test_model_store.py index 99dab6ddcb9e..cbf8afa5a7e2 100644 --- a/test/unit/data/model/test_model_store.py +++ b/test/unit/data/model/test_model_store.py @@ -1066,7 +1066,7 @@ def _setup_collection_invocation(app): workflow_step_1 = model.WorkflowStep() workflow_step_1.order_index = 0 workflow_step_1.type = "data_collection_input" - workflow_step_1.tool_inputs = {} # type:ignore[assignment] + workflow_step_1.tool_inputs = {} sa_session.add(workflow_step_1) workflow_1 = _workflow_from_steps(u, [workflow_step_1]) workflow_1.license = "MIT" @@ -1092,7 +1092,7 @@ def _setup_simple_invocation(app): workflow_step_1 = model.WorkflowStep() workflow_step_1.order_index = 0 workflow_step_1.type = "data_input" - workflow_step_1.tool_inputs = {} # type:ignore[assignment] + workflow_step_1.tool_inputs = {} sa_session.add(workflow_step_1) workflow = _workflow_from_steps(u, [workflow_step_1]) workflow.license = "MIT" From 019ab2e5a2b170a91d14ad3379bd0ac14fb569a2 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 13 Oct 2024 23:02:36 -0400 Subject: [PATCH 02/42] Rebuild schema for workflow request model changes. --- client/src/api/schema/schema.ts | 173 ++++++++++++++++++++++++++++---- 1 file changed, 155 insertions(+), 18 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index a8ddacee9b22..72a354a7690c 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -2645,6 +2645,23 @@ export interface paths { patch?: never; trace?: never; }; + "/api/invocations/{invocation_id}/request": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Get a description modeling an API request to invoke this workflow - this is recreated and will be more specific in some ways than the initial creation request. */ + get: operations["invocation_as_request_api_invocations__invocation_id__request_get"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/invocations/{invocation_id}/step_jobs_summary": { parameters: { query?: never; @@ -12385,8 +12402,9 @@ export interface components { */ batch: boolean | null; /** - * Dataset Map - * @description TODO + * Legacy Dataset Map + * @deprecated + * @description An older alternative to specifying inputs using database IDs, do not use this and use inputs instead * @default {} */ ds_map: { @@ -12409,12 +12427,12 @@ export interface components { history_id?: string | null; /** * Inputs - * @description TODO + * @description Specify values for formal inputs to the workflow */ inputs?: Record | null; /** * Inputs By - * @description How inputs maps to inputs (datasets/collections) to workflows steps. + * @description How the 'inputs' field maps its inputs (datasets/collections/step parameters) to workflows steps. */ inputs_by?: string | null; /** @@ -12441,35 +12459,35 @@ export interface components { */ no_add_to_history: boolean | null; /** - * Parameters - * @description The raw parameters for the workflow invocation. + * Legacy Step Parameters + * @description Parameters specified per-step for the workflow invocation, this is legacy and you should generally use inputs and only specify the formal parameters of a workflow instead. * @default {} */ parameters: Record | null; /** - * Parameters Normalized - * @description Indicates if parameters are already normalized for workflow invocation. + * Legacy Step Parameters Normalized + * @description Indicates if legacy parameters are already normalized to be indexed by the order_index and are specified as a dictionary per step. Legacy-style parameters could previously be specified as one parameter per step or by tool ID. * @default false */ parameters_normalized: boolean | null; /** * Preferred Intermediate Object Store ID - * @description The ID of the ? object store that should be used to store ? datasets in this history. + * @description The ID of the object store that should be used to store the intermediate datasets of this workflow - - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences */ preferred_intermediate_object_store_id?: string | null; /** * Preferred Object Store ID - * @description The ID of the object store that should be used to store new datasets in this history. + * @description The ID of the object store that should be used to store all datasets (can instead specify object store IDs for intermediate and outputs datasts separately) - - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences */ preferred_object_store_id?: string | null; /** * Preferred Outputs Object Store ID - * @description The ID of the object store that should be used to store ? datasets in this history. + * @description The ID of the object store that should be used to store the marked output datasets of this workflow - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences. */ preferred_outputs_object_store_id?: string | null; /** * Replacement Parameters - * @description TODO + * @description Class of parameters mostly used for string replacement in PJAs. In best practice workflows, these should be replaced with input parameters * @default {} */ replacement_params: Record | null; @@ -12481,7 +12499,7 @@ export interface components { require_exact_tool_versions: boolean | null; /** * Resource Parameters - * @description TODO + * @description If a workflow_resource_params_file file is defined and the target workflow is configured to consumer resource parameters, they can be specified with this parameter. See https://github.com/galaxyproject/galaxy/pull/4830 for more information. * @default {} */ resource_params: Record | null; @@ -12490,11 +12508,6 @@ export interface components { * @description Scheduler to use for workflow invocation. */ scheduler?: string | null; - /** - * Step Parameters - * @description TODO - */ - step_parameters?: Record | null; /** * Use cached job * @description Indicated whether to use a cached job for workflow invocation. @@ -18188,6 +18201,86 @@ export interface components { */ workflow_id: string; }; + /** + * WorkflowInvocationRequestModel + * @description Model a workflow invocation request (InvokeWorkflowPayload) for an existing invocation. + */ + WorkflowInvocationRequestModel: { + /** + * History ID + * @description The encoded history id the workflow was run in. + */ + history_id: string; + /** + * Inputs + * @description Values for inputs + */ + inputs: Record; + /** + * Inputs by + * @description How the 'inputs' field maps its inputs (datasets/collections/step parameters) to workflows steps. + */ + inputs_by: string; + /** + * Is instance + * @description This API yields a particular workflow instance, newer workflows belonging to the same storedworkflow may have different state. + * @default true + * @constant + * @enum {boolean} + */ + instance: true; + /** + * Legacy Step Parameters + * @description Parameters specified per-step for the workflow invocation, this is legacy and you should generally use inputs and only specify the formal parameters of a workflow instead. If these are set, the workflow was not executed in a best-practice fashion and we the resulting invocation request may not fully reflect the executed workflow state. + */ + parameters?: Record | null; + /** + * Legacy Step Parameters Normalized + * @description Indicates if legacy parameters are already normalized to be indexed by the order_index and are specified as a dictionary per step. Legacy-style parameters could previously be specified as one parameter per step or by tool ID. + * @default true + * @constant + * @enum {boolean} + */ + parameters_normalized: true; + /** + * Preferred Intermediate Object Store ID + * @description The ID of the object store that should be used to store the intermediate datasets of this workflow - - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences + */ + preferred_intermediate_object_store_id?: string | null; + /** + * Preferred Object Store ID + * @description The ID of the object store that should be used to store all datasets (can instead specify object store IDs for intermediate and outputs datasts separately) - - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences + */ + preferred_object_store_id?: string | null; + /** + * Preferred Outputs Object Store ID + * @description The ID of the object store that should be used to store the marked output datasets of this workflow - Galaxy's job configuration may override this in some cases but this workflow preference will override tool and user preferences. + */ + preferred_outputs_object_store_id?: string | null; + /** + * Replacement Parameters + * @description Class of parameters mostly used for string replacement in PJAs. In best practice workflows, these should be replaced with input parameters + * @default {} + */ + replacement_params: Record | null; + /** + * Resource Parameters + * @description If a workflow_resource_params_file file is defined and the target workflow is configured to consumer resource parameters, they can be specified with this parameter. See https://github.com/galaxyproject/galaxy/pull/4830 for more information. + * @default {} + */ + resource_params: Record | null; + /** + * Use cached job + * @description Indicated whether to use a cached job for workflow invocation. + * @default false + */ + use_cached_job: boolean; + /** + * Workflow ID + * @description The encoded Workflow ID associated with the invocation. + */ + workflow_id: string; + }; /** WorkflowInvocationResponse */ WorkflowInvocationResponse: | components["schemas"]["WorkflowInvocationElementView"] @@ -27056,6 +27149,50 @@ export interface operations { }; }; }; + invocation_as_request_api_invocations__invocation_id__request_get: { + parameters: { + query?: never; + header?: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + "run-as"?: string | null; + }; + path: { + /** @description The encoded database identifier of the Invocation. */ + invocation_id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["WorkflowInvocationRequestModel"]; + }; + }; + /** @description Request Error */ + "4XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + /** @description Server Error */ + "5XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["MessageExceptionModel"]; + }; + }; + }; + }; invocation_step_jobs_summary_api_invocations__invocation_id__step_jobs_summary_get: { parameters: { query?: never; From 12fed2add3286c26c4b6d2490b8898bf8f7080db Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 13 Oct 2024 22:56:32 -0400 Subject: [PATCH 03/42] Swap workflow framework tests to do a rerun. --- .github/workflows/framework_workflows.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/framework_workflows.yaml b/.github/workflows/framework_workflows.yaml index 93ab6c0bbf1a..018463833e22 100644 --- a/.github/workflows/framework_workflows.yaml +++ b/.github/workflows/framework_workflows.yaml @@ -16,6 +16,7 @@ on: env: GALAXY_TEST_DBURI: 'postgresql://postgres:postgres@localhost:5432/galaxy?client_encoding=utf8' GALAXY_TEST_RAISE_EXCEPTION_ON_HISTORYLESS_HDA: '1' + GALAXY_TEST_WORKFLOW_AFTER_RERUN: '1' concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true From cc8f1b65b8da880d1b22135eea39220c3198b9c0 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 14 Oct 2024 11:18:36 -0400 Subject: [PATCH 04/42] If loading a workflow without a version by instance id - use that instance. --- lib/galaxy/model/__init__.py | 10 ++++++++++ lib/galaxy/webapps/galaxy/services/workflows.py | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 4bbc83f787fb..2cd12d85057f 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -7749,6 +7749,16 @@ def get_internal_version(self, version): raise galaxy.exceptions.RequestParameterInvalidException("Version does not exist") return list(reversed(self.workflows))[version] + def get_internal_version_by_id(self, workflow_instance_id: int): + sa_session = object_session(self) + assert sa_session + workflow = sa_session.get(Workflow, workflow_instance_id) + if not workflow: + raise galaxy.exceptions.ObjectNotFound() + elif workflow.stored_workflow != self: + raise galaxy.exceptions.RequestParameterInvalidException() + return workflow + def version_of(self, workflow): for version, workflow_instance in enumerate(reversed(self.workflows)): if workflow_instance.id == workflow.id: diff --git a/lib/galaxy/webapps/galaxy/services/workflows.py b/lib/galaxy/webapps/galaxy/services/workflows.py index 72414b169abf..0f7f13b3aa05 100644 --- a/lib/galaxy/webapps/galaxy/services/workflows.py +++ b/lib/galaxy/webapps/galaxy/services/workflows.py @@ -131,7 +131,10 @@ def invoke_workflow( by_stored_id = not payload.instance stored_workflow = self._workflows_manager.get_stored_accessible_workflow(trans, workflow_id, by_stored_id) version = payload.version - workflow = stored_workflow.get_internal_version(version) + if version is None and payload.instance: + workflow = stored_workflow.get_internal_version_by_id(workflow_id) + else: + workflow = stored_workflow.get_internal_version(version) run_configs = build_workflow_run_configs(trans, workflow, payload.model_dump(exclude_unset=True)) is_batch = payload.batch if not is_batch and len(run_configs) != 1: From d3e0e28ce358dc3c220986050bd559f844da4bd0 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 14 Oct 2024 09:53:49 -0400 Subject: [PATCH 05/42] Do not skip CI tests on missing tools. --- .github/workflows/api.yaml | 1 + lib/galaxy_test/api/test_tools.py | 13 ------------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/.github/workflows/api.yaml b/.github/workflows/api.yaml index 8eb5a2dd564a..0ca4399f9314 100644 --- a/.github/workflows/api.yaml +++ b/.github/workflows/api.yaml @@ -18,6 +18,7 @@ env: GALAXY_TEST_DBURI: 'postgresql://postgres:postgres@localhost:5432/galaxy?client_encoding=utf8' GALAXY_TEST_RAISE_EXCEPTION_ON_HISTORYLESS_HDA: '1' GALAXY_CONFIG_SQLALCHEMY_WARN_20: '1' + GALAXY_TEST_REQUIRE_ALL_NEEDED_TOOLS: '1' concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 46c6cdad3473..b99a059a65a7 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -29,7 +29,6 @@ BaseDatasetCollectionPopulator, DatasetCollectionPopulator, DatasetPopulator, - LibraryPopulator, skip_without_tool, stage_rules_example, ) @@ -1027,18 +1026,6 @@ def test_data_column_defaults(self): content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output[0]) assert "parameter: 1,2" in content - @skip_without_tool("library_data") - def test_library_data_param(self): - with self.dataset_populator.test_history(require_new=False) as history_id: - ld = LibraryPopulator(self.galaxy_interactor).new_library_dataset("lda_test_library") - inputs = {"library_dataset": ld["ldda_id"], "library_dataset_multiple": [ld["ldda_id"], ld["ldda_id"]]} - response = self._run("library_data", history_id, inputs, assert_ok=True) - output = response["outputs"] - output_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output[0]) - assert output_content == "TestData\n", output_content - output_multiple_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output[1]) - assert output_multiple_content == "TestData\nTestData\n", output_multiple_content - @skip_without_tool("cat1") def test_run_cat1(self): with self.dataset_populator.test_history(require_new=False) as history_id: From ccddeaa88dc27d64bb48c710aaec720ae9e09fef Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 8 Oct 2024 23:36:44 -0400 Subject: [PATCH 06/42] Improved error messages when there are problems generating tool tests. --- lib/galaxy/tool_util/verify/parse.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/verify/parse.py b/lib/galaxy/tool_util/verify/parse.py index a3aee97eef0c..8dd51f525944 100644 --- a/lib/galaxy/tool_util/verify/parse.py +++ b/lib/galaxy/tool_util/verify/parse.py @@ -1,5 +1,6 @@ import logging import os +import traceback from typing import ( Any, Iterable, @@ -141,7 +142,7 @@ def _description_from_tool_source( "error": False, } ) - except Exception as e: + except Exception: processed_test_dict = InvalidToolTestDict( { "tool_id": tool_id, @@ -149,7 +150,7 @@ def _description_from_tool_source( "test_index": test_index, "inputs": {}, "error": True, - "exception": unicodify(e), + "exception": unicodify(traceback.format_exc()), "maxseconds": maxseconds, } ) From b601802e0220130c01daa96973feb38e2b41b718 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 8 Oct 2024 23:36:13 -0400 Subject: [PATCH 07/42] Cleaner startup when there are tool problems. --- lib/galaxy/tool_util/toolbox/__init__.py | 2 ++ lib/galaxy/tool_util/toolbox/base.py | 8 ++++++++ lib/galaxy/tools/__init__.py | 11 +++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/toolbox/__init__.py b/lib/galaxy/tool_util/toolbox/__init__.py index 34c2070b09f1..b0d611cbff4f 100644 --- a/lib/galaxy/tool_util/toolbox/__init__.py +++ b/lib/galaxy/tool_util/toolbox/__init__.py @@ -3,6 +3,7 @@ from .base import ( AbstractToolBox, AbstractToolTagManager, + ToolLoadError, ) from .panel import ( panel_item_types, @@ -14,6 +15,7 @@ "AbstractToolBox", "AbstractToolTagManager", "panel_item_types", + "ToolLoadError", "ToolSection", "ToolSectionLabel", ) diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index f6ab7bfa98f1..a04845f65ab8 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -133,6 +133,10 @@ def handle_tags(self, tool_id, tool_definition_source) -> None: return None +class ToolLoadError(Exception): + pass + + class AbstractToolBox(ManagesIntegratedToolPanelMixin): """ Abstract container for managing a ToolPanel - containing tools and @@ -1073,6 +1077,10 @@ def quick_load(tool_file, async_load=True): self._load_tool_panel_views() self._save_integrated_tool_panel() return tool.id + except ToolLoadError as e: + # no need for full stack trace - ToolLoadError corresponds to a known load + # error with defined cause that is included in the message + log.error(f"Failed to load potential tool {tool_file} - {e}") except Exception: log.exception("Failed to load potential tool %s.", tool_file) return None diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index a7b6c96bc1f3..16fcfce0ba8e 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -95,6 +95,7 @@ from galaxy.tool_util.toolbox import ( AbstractToolBox, AbstractToolTagManager, + ToolLoadError, ToolSection, ) from galaxy.tool_util.toolbox.views.sources import StaticToolBoxViewSources @@ -380,7 +381,10 @@ def create_tool_from_source(app, tool_source: ToolSource, config_file: Optional[ elif tool_type := tool_source.parse_tool_type(): ToolClass = tool_types.get(tool_type) if not ToolClass: - raise ValueError(f"Unrecognized tool type: {tool_type}") + if tool_type == "cwl": + raise ToolLoadError("Runtime support for CWL tools is not implemented currently") + else: + raise ToolLoadError(f"Parsed unrecognized tool type ({tool_type}) from tool") else: # Normal tool root = getattr(tool_source, "root", None) @@ -3227,9 +3231,8 @@ class InteractiveTool(Tool): produces_entry_points = True def __init__(self, config_file, tool_source, app, **kwd): - assert app.config.interactivetools_enable, ValueError( - "Trying to load an InteractiveTool, but InteractiveTools are not enabled." - ) + if not app.config.interactivetools_enable: + raise ToolLoadError("Trying to load an InteractiveTool, but InteractiveTools are not enabled.") super().__init__(config_file, tool_source, app, **kwd) def __remove_interactivetool_by_job(self, job): From d07081d86931f6a79623bd1a7acd9c910f8f761c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 13 Oct 2024 10:46:59 -0400 Subject: [PATCH 08/42] tool input format test fixutre --- lib/galaxy_test/api/conftest.py | 6 ++++ lib/galaxy_test/api/test_tool_execute.py | 23 +++++---------- lib/galaxy_test/base/populators.py | 37 ++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/lib/galaxy_test/api/conftest.py b/lib/galaxy_test/api/conftest.py index 5c0b36753995..5c36c5d03c77 100644 --- a/lib/galaxy_test/api/conftest.py +++ b/lib/galaxy_test/api/conftest.py @@ -25,6 +25,7 @@ check_missing_tool, DatasetCollectionPopulator, DatasetPopulator, + DescribeToolInputs, get_tool_ids, RequiredTool, TargetHistory, @@ -136,6 +137,11 @@ def required_tool(dataset_populator: DatasetPopulator, history_id: str, required return tool +@pytest.fixture(params=["legacy", "21.01"]) +def tool_input_format(request) -> Iterator[DescribeToolInputs]: + yield DescribeToolInputs(request.param) + + @pytest.fixture(autouse=True) def check_required_tools(anonymous_galaxy_interactor, request): for marker in request.node.iter_markers(): diff --git a/lib/galaxy_test/api/test_tool_execute.py b/lib/galaxy_test/api/test_tool_execute.py index b77ebcbb0cd1..1494d58990a4 100644 --- a/lib/galaxy_test/api/test_tool_execute.py +++ b/lib/galaxy_test/api/test_tool_execute.py @@ -9,6 +9,7 @@ from galaxy_test.base.decorators import requires_tool_id from galaxy_test.base.populators import ( + DescribeToolInputs, RequiredTool, TargetHistory, ) @@ -80,24 +81,13 @@ def test_identifier_in_multiple_reduce(target_history: TargetHistory, required_t @requires_tool_id("identifier_in_conditional") -def test_identifier_map_over_multiple_input_in_conditional_legacy_format( - target_history: TargetHistory, required_tool: RequiredTool -): - hdca = target_history.with_pair() - execute = required_tool.execute.with_inputs( - { - "outer_cond|input1": hdca.src_dict, - } - ) - execute.assert_has_single_job.assert_has_single_output.with_contents_stripped("forward\nreverse") - - -@requires_tool_id("identifier_in_conditional") -def test_identifier_map_over_multiple_input_in_conditional_21_01_format( - target_history: TargetHistory, required_tool: RequiredTool +def test_identifier_map_over_multiple_input_in_conditional( + target_history: TargetHistory, required_tool: RequiredTool, tool_input_format: DescribeToolInputs ): hdca = target_history.with_pair() - execute = required_tool.execute.with_nested_inputs( + inputs = tool_input_format.when.flat({ + "outer_cond|input1": hdca.src_dict, + }).when.nested( { "outer_cond": { "multi_input": True, @@ -105,6 +95,7 @@ def test_identifier_map_over_multiple_input_in_conditional_21_01_format( }, } ) + execute = required_tool.execute.with_inputs(inputs) execute.assert_has_single_job.assert_has_single_output.with_contents_stripped("forward\nreverse") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index f68321634984..1e2f754c2104 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -3609,6 +3609,34 @@ def execute(self) -> "DescribeToolExecution": return execution +class DescribeToolInputs: + _input_format: str = "legacy" + _inputs: Optional[Dict[str, Any]] + + def __init__(self, input_format: str): + self._input_format = input_format + self._inputs = None + + def any(self, inputs: Dict[str, Any]) -> Self: + self._inputs = inputs + return self + + def flat(self, inputs: Dict[str, Any]) -> Self: + if self._input_format == "legacy": + self._inputs = inputs + return self + + def nested(self, inputs: Dict[str, Any]) -> Self: + if self._input_format == "21.01": + self._inputs = inputs + return self + + # aliases for self to create silly little English sentense... inputs.when.flat().when.legacy() + @property + def when(self) -> Self: + return self + + class DescribeToolExecution: _history_id: Optional[str] = None _execute_response: Optional[Response] = None @@ -3627,8 +3655,13 @@ def in_history(self, has_history_id: Union[str, "TargetHistory"]) -> Self: self._history_id = has_history_id._history_id return self - def with_inputs(self, inputs: Dict[str, Any]) -> Self: - self._inputs = inputs + def with_inputs(self, inputs: Union[DescribeToolInputs, Dict[str, Any]]) -> Self: + if isinstance(inputs, DescribeToolInputs): + self._inputs = inputs._inputs or {} + self._input_format = inputs._input_format + else: + self._inputs = inputs + self._input_format = "legacy" return self def with_nested_inputs(self, inputs: Dict[str, Any]) -> Self: From 951ab47f47ed27f10ce29c48129d09cd519577cb Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 4 Oct 2024 10:12:00 -0400 Subject: [PATCH 09/42] More tool tests. --- lib/galaxy_test/api/conftest.py | 11 ++ lib/galaxy_test/api/test_tool_execute.py | 107 +++++++++++++++++- lib/galaxy_test/api/test_tools.py | 51 --------- lib/galaxy_test/base/populators.py | 32 +++++- .../tools/parameters/gx_drill_down_code.py | 24 ++++ .../tools/parameters/gx_drill_down_code.xml | 38 +++++++ .../tools/parameters/gx_select_dynamic.xml | 36 ++++++ .../parameters/gx_select_dynamic_empty.py | 8 ++ .../parameters/gx_select_dynamic_empty.xml | 15 +++ .../gx_select_dynamic_empty_validated.xml | 16 +++ .../parameters/gx_select_dynamic_options.py | 10 ++ .../gx_select_no_options_validation.xml | 20 ++++ ..._select_optional_no_options_validation.xml | 20 ++++ test/functional/tools/parameters/gx_text.xml | 7 +- .../parameters/gx_text_empty_validation.xml | 20 ++++ .../tools/parameters/gx_text_optional.xml | 7 +- .../parameters/gx_text_optional_false.xml | 18 +++ 17 files changed, 381 insertions(+), 59 deletions(-) create mode 100644 test/functional/tools/parameters/gx_drill_down_code.py create mode 100644 test/functional/tools/parameters/gx_drill_down_code.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic_empty.py create mode 100644 test/functional/tools/parameters/gx_select_dynamic_empty.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml create mode 100644 test/functional/tools/parameters/gx_select_dynamic_options.py create mode 100644 test/functional/tools/parameters/gx_select_no_options_validation.xml create mode 100644 test/functional/tools/parameters/gx_select_optional_no_options_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_empty_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_optional_false.xml diff --git a/lib/galaxy_test/api/conftest.py b/lib/galaxy_test/api/conftest.py index 5c36c5d03c77..a011532c7af5 100644 --- a/lib/galaxy_test/api/conftest.py +++ b/lib/galaxy_test/api/conftest.py @@ -128,6 +128,17 @@ def target_history( return TargetHistory(dataset_populator, dataset_collection_populator, history_id) +@pytest.fixture +def required_tools( + dataset_populator: DatasetPopulator, history_id: str, required_tool_ids: List[str] +) -> List[RequiredTool]: + tools = [] + for tool_id in required_tool_ids: + tool = RequiredTool(dataset_populator, tool_id, history_id) + tools.append(tool) + return tools + + @pytest.fixture def required_tool(dataset_populator: DatasetPopulator, history_id: str, required_tool_ids: List[str]) -> RequiredTool: if len(required_tool_ids) != 1: diff --git a/lib/galaxy_test/api/test_tool_execute.py b/lib/galaxy_test/api/test_tool_execute.py index 1494d58990a4..3d9c60f0ef79 100644 --- a/lib/galaxy_test/api/test_tool_execute.py +++ b/lib/galaxy_test/api/test_tool_execute.py @@ -7,6 +7,8 @@ files, etc..). """ +from typing import List + from galaxy_test.base.decorators import requires_tool_id from galaxy_test.base.populators import ( DescribeToolInputs, @@ -85,9 +87,11 @@ def test_identifier_map_over_multiple_input_in_conditional( target_history: TargetHistory, required_tool: RequiredTool, tool_input_format: DescribeToolInputs ): hdca = target_history.with_pair() - inputs = tool_input_format.when.flat({ - "outer_cond|input1": hdca.src_dict, - }).when.nested( + inputs = tool_input_format.when.flat( + { + "outer_cond|input1": hdca.src_dict, + } + ).when.nested( { "outer_cond": { "multi_input": True, @@ -209,3 +213,100 @@ def test_optional_repeats_with_mins_filled_id(target_history: TargetHistory, req # tool test framework filling in a default. Creating a raw request here # verifies that currently select parameters don't require a selection. required_tool.execute.assert_has_single_job.with_single_output.containing("false").containing("length: 2") + + +@requires_tool_id("gx_select") +@requires_tool_id("gx_select_no_options_validation") +def test_select_first_by_default(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + empty = tool_input_format.when.any({}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_has_single_job.with_output("output").with_contents_stripped( + "--ex1" + ) + + +@requires_tool_id("gx_select") +@requires_tool_id("gx_select_no_options_validation") +@requires_tool_id("gx_select_dynamic_empty") +@requires_tool_id("gx_select_dynamic_empty_validated") +def test_select_on_null_errors(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + # test_select_first_by_default verifies the first option will just be selected, despite that if an explicit null + # is passed, an error (rightfully) occurs. This test verifies that. + null_parameter = tool_input_format.when.any({"parameter": None}) + for required_tool in required_tools: + required_tool.execute.with_inputs(null_parameter).assert_fails.with_error_containing("an invalid option") + + +@requires_tool_id("gx_select_dynamic_empty") +@requires_tool_id("gx_select_dynamic_empty_validated") +def test_select_empty_causes_error_regardless( + required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs +): + # despite selects otherwise selecting defaults - nothing can be done if the select option list is empty + empty = tool_input_format.when.any({}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_fails.with_error_containing("an invalid option") + + +@requires_tool_id("gx_select_optional") +@requires_tool_id("gx_select_optional_no_options_validation") +def test_select_optional_null_by_default(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + # test_select_first_by_default shows that required select values will pick an option by default, + # this test verify that doesn't occur for optional selects. + empty = tool_input_format.when.any({}) + null_parameter = tool_input_format.when.any({"parameter": None}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_has_single_job.with_output("output").with_contents_stripped( + "None" + ) + required_tool.execute.with_inputs(null_parameter).assert_has_single_job.with_output( + "output" + ).with_contents_stripped("None") + + +@requires_tool_id("gx_select_multiple") +@requires_tool_id("gx_select_multiple_optional") +def test_select_multiple_does_not_select_first_by_default( + required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs +): + # unlike single selects - no selection is forced and these serve as optional by default + empty = tool_input_format.when.any({}) + null_parameter = tool_input_format.when.any({"parameter": None}) + for required_tool in required_tools: + required_tool.execute.with_inputs(empty).assert_has_single_job.with_output("output").with_contents_stripped( + "None" + ) + required_tool.execute.with_inputs(null_parameter).assert_has_single_job.with_output( + "output" + ).with_contents_stripped("None") + + +@requires_tool_id("gx_text") +@requires_tool_id("gx_text_optional_false") +def test_null_to_text_tools(required_tools: List[RequiredTool], tool_input_format: DescribeToolInputs): + for required_tool in required_tools: + execute = required_tool.execute.with_inputs(tool_input_format.when.any({})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": ""}) + + execute = required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": None})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": ""}) + + +@requires_tool_id("gx_text_optional") +def test_null_to_optinal_text_tool(required_tool: RequiredTool, tool_input_format: DescribeToolInputs): + execute = required_tool.execute.with_inputs(tool_input_format.when.any({})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": None}) + + execute = required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": None})) + execute.assert_has_single_job.with_output("output").with_contents_stripped("") + execute.assert_has_single_job.with_output("inputs_json").with_json({"parameter": None}) + + +@requires_tool_id("gx_text_empty_validation") +def test_null_to_text_tool_with_validation(required_tool: RequiredTool, tool_input_format: DescribeToolInputs): + required_tool.execute.with_inputs(tool_input_format.when.any({})).assert_fails() + required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": None})).assert_fails() + required_tool.execute.with_inputs(tool_input_format.when.any({"parameter": ""})).assert_fails() diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 46c6cdad3473..12c11f3a634e 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -879,57 +879,6 @@ def test_dataset_hidden_after_job_finish(self): output_details = self.dataset_populator.get_history_dataset_details(history_id, dataset=output, wait=True) assert not output_details["visible"] - @skip_without_tool("gx_select") - def test_select_first_by_default(self): - # we have a tool test for this but I wanted to verify it wasn't just the - # tool test framework filling in a default. Creating a raw request here - # verifies that currently select parameters don't require a selection. - with self.dataset_populator.test_history(require_new=False) as history_id: - inputs: Dict[str, Any] = {} - response = self._run("gx_select", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "--ex1" - - inputs = { - "parameter": None, - } - response = self._run("gx_select", history_id, inputs, assert_ok=False) - self._assert_status_code_is(response, 400) - assert "an invalid option" in response.text - - @skip_without_tool("gx_select_multiple") - @skip_without_tool("gx_select_multiple_optional") - def test_select_multiple_null_handling(self): - with self.dataset_populator.test_history(require_new=False) as history_id: - inputs: Dict[str, Any] = {} - response = self._run("gx_select_multiple", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - - inputs = {} - response = self._run("gx_select_multiple_optional", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - - inputs = { - "parameter": None, - } - response = self._run("gx_select_multiple", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - - inputs = { - "parameter": None, - } - response = self._run("gx_select_multiple_optional", history_id, inputs, assert_ok=True) - output = response["outputs"][0] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output) - assert output1_content.strip() == "None" - @skip_without_tool("gx_drill_down_exact") @skip_without_tool("gx_drill_down_exact_multiple") @skip_without_tool("gx_drill_down_recurse") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 1e2f754c2104..05ac97b73fbb 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -3461,6 +3461,17 @@ def with_file_ext(self, expected_ext: str) -> Self: raise AssertionError(f"Output dataset had file extension {ext}, not the expected extension {expected_ext}") return self + @property + def json(self) -> Any: + contents = self.contents + return json.loads(contents) + + def with_json(self, expected_json: Any) -> Self: + json = self.json + if json != expected_json: + raise AssertionError(f"Output dataset contianed JSON {json}, not {expected_json} as expected") + return self + # aliases that might help make tests more like English in particular cases. Declaring them explicitly # instead quick little aliases because of https://github.com/python/mypy/issues/6700 def assert_contains(self, expected_contents: str) -> Self: @@ -3585,6 +3596,9 @@ class DescribeFailure: def __init__(self, response: Response): self._response = response + def __call__(self) -> Self: + return self + def with_status_code(self, code: int) -> Self: api_asserts.assert_status_code_is(self._response, code) return self @@ -3728,12 +3742,24 @@ def assert_has_job(self, job_index: int = 0) -> DescribeJob: return DescribeJob(self._dataset_populator, history_id, job["id"]) @property - def assert_fails(self) -> DescribeFailure: + def that_fails(self) -> DescribeFailure: self._ensure_executed() execute_response = self._execute_response assert execute_response is not None - api_asserts.assert_status_code_is_not_ok(execute_response) - return DescribeFailure(execute_response) + if execute_response.status_code != 200: + return DescribeFailure(execute_response) + else: + response = self._assert_executed_ok() + jobs = response["jobs"] + for job in jobs: + final_state = self._dataset_populator.wait_for_job(job["id"]) + assert final_state == "error" + return DescribeFailure(execute_response) + + # alternative assert_ syntax for cases where it reads better. + @property + def assert_fails(self) -> DescribeFailure: + return self.that_fails class GiHttpMixin: diff --git a/test/functional/tools/parameters/gx_drill_down_code.py b/test/functional/tools/parameters/gx_drill_down_code.py new file mode 100644 index 000000000000..518cb67c6a7e --- /dev/null +++ b/test/functional/tools/parameters/gx_drill_down_code.py @@ -0,0 +1,24 @@ +def collate_table(path: str) -> list: + with open(path, "r") as f: + contents = f.read() + first_options = [] + second_options = [] + values = [ + {"name": "First Column", "value": "first", "selected": False, "options": first_options}, + {"name": "Second Column", "value": "second", "selected": False, "options": second_options}, + ] + seen_in_column_1 = set() + seen_in_column_2 = set() + for line in contents.splitlines(): + parts = line.split("\t") + if len(parts) >= 1: + val = parts[0] + if val not in seen_in_column_1: + first_options.append({"name": val.upper(), "value": val, "selected": False, "options": []}) + seen_in_column_1.add(val) + if len(parts) >= 2: + val = parts[1] + if val not in seen_in_column_2: + second_options.append({"name": val.upper(), "value": val, "selected": False, "options": []}) + seen_in_column_2.add(val) + return values diff --git a/test/functional/tools/parameters/gx_drill_down_code.xml b/test/functional/tools/parameters/gx_drill_down_code.xml new file mode 100644 index 000000000000..42ae442574e4 --- /dev/null +++ b/test/functional/tools/parameters/gx_drill_down_code.xml @@ -0,0 +1,38 @@ + + + macros.xml + + + '$output' + ]]> + + + +g + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic.xml b/test/functional/tools/parameters/gx_select_dynamic.xml new file mode 100644 index 000000000000..8959f3d0e3d4 --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic.xml @@ -0,0 +1,36 @@ + + > '$output' + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic_empty.py b/test/functional/tools/parameters/gx_select_dynamic_empty.py new file mode 100644 index 000000000000..6a79a047f3cb --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_empty.py @@ -0,0 +1,8 @@ +from typing import ( + List, + Tuple, +) + + +def empty_list() -> List[Tuple[str, str, bool]]: + return [] diff --git a/test/functional/tools/parameters/gx_select_dynamic_empty.xml b/test/functional/tools/parameters/gx_select_dynamic_empty.xml new file mode 100644 index 000000000000..7554604033fd --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_empty.xml @@ -0,0 +1,15 @@ + + > '$output' + ]]> + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml b/test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml new file mode 100644 index 000000000000..39ffa263174e --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_empty_validated.xml @@ -0,0 +1,16 @@ + + > '$output' + ]]> + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_dynamic_options.py b/test/functional/tools/parameters/gx_select_dynamic_options.py new file mode 100644 index 000000000000..734174b6da1d --- /dev/null +++ b/test/functional/tools/parameters/gx_select_dynamic_options.py @@ -0,0 +1,10 @@ +from typing import ( + List, + Tuple, +) + + +def every_other_word(path: str) -> List[Tuple[str, str, bool]]: + with open(path, "r") as f: + contents = f.read() + return [(r.strip(), r.strip(), False) for (i, r) in enumerate(contents.split()) if i % 2 == 0] diff --git a/test/functional/tools/parameters/gx_select_no_options_validation.xml b/test/functional/tools/parameters/gx_select_no_options_validation.xml new file mode 100644 index 000000000000..217b95b53639 --- /dev/null +++ b/test/functional/tools/parameters/gx_select_no_options_validation.xml @@ -0,0 +1,20 @@ + + > '$output' + ]]> + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_select_optional_no_options_validation.xml b/test/functional/tools/parameters/gx_select_optional_no_options_validation.xml new file mode 100644 index 000000000000..6e3e78de0400 --- /dev/null +++ b/test/functional/tools/parameters/gx_select_optional_no_options_validation.xml @@ -0,0 +1,20 @@ + + > '$output' + ]]> + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text.xml b/test/functional/tools/parameters/gx_text.xml index 16707f63e878..3b9e2e00dfbc 100644 --- a/test/functional/tools/parameters/gx_text.xml +++ b/test/functional/tools/parameters/gx_text.xml @@ -1,12 +1,17 @@ > '$output' +echo '$parameter' >> '$output'; +cat '$inputs' >> $inputs_json; ]]> + + + + diff --git a/test/functional/tools/parameters/gx_text_empty_validation.xml b/test/functional/tools/parameters/gx_text_empty_validation.xml new file mode 100644 index 000000000000..76374be72ded --- /dev/null +++ b/test/functional/tools/parameters/gx_text_empty_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_optional.xml b/test/functional/tools/parameters/gx_text_optional.xml index 41fe11cea418..ae10d3278376 100644 --- a/test/functional/tools/parameters/gx_text_optional.xml +++ b/test/functional/tools/parameters/gx_text_optional.xml @@ -1,12 +1,17 @@ > '$output' +echo '$parameter' >> '$output'; +cat '$inputs' >> $inputs_json; ]]> + + + + diff --git a/test/functional/tools/parameters/gx_text_optional_false.xml b/test/functional/tools/parameters/gx_text_optional_false.xml new file mode 100644 index 000000000000..9e29d53fc4ca --- /dev/null +++ b/test/functional/tools/parameters/gx_text_optional_false.xml @@ -0,0 +1,18 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + From 8c10f36dd2d85ec7a0089d2f702c65d0637f5361 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 1 Oct 2024 08:45:13 -0400 Subject: [PATCH 10/42] Refactor Tool._populate for reuse of hook checks. --- lib/galaxy/tools/__init__.py | 22 ++++---- lib/galaxy/tools/parameters/__init__.py | 47 ---------------- test/unit/app/tools/test_populate_state.py | 63 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 56 deletions(-) create mode 100644 test/unit/app/tools/test_populate_state.py diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index a65a45b4b088..49afd5ad216e 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1900,17 +1900,21 @@ def _populate( simple_errors=False, input_format=input_format, ) - # If the tool provides a `validate_input` hook, call it. - validate_input = self.get_hook("validate_input") - if validate_input: - # hooks are so terrible ... this is specifically for https://github.com/galaxyproject/tools-devteam/blob/main/tool_collections/gops/basecoverage/operation_filter.py - legacy_non_dce_params = { - k: v.hda if isinstance(v, model.DatasetCollectionElement) and v.hda else v - for k, v in params.items() - } - validate_input(request_context, errors, legacy_non_dce_params, self.inputs) + self._handle_validate_input_hook(request_context, params, errors) return params, errors + def _handle_validate_input_hook( + self, request_context, params: ToolStateJobInstancePopulatedT, errors: ParameterValidationErrorsT + ): + # If the tool provides a `validate_input` hook, call it. + validate_input = self.get_hook("validate_input") + if validate_input: + # hooks are so terrible ... this is specifically for https://github.com/galaxyproject/tools-devteam/blob/main/tool_collections/gops/basecoverage/operation_filter.py + legacy_non_dce_params = { + k: v.hda if isinstance(v, model.DatasetCollectionElement) and v.hda else v for k, v in params.items() + } + validate_input(request_context, errors, legacy_non_dce_params, self.inputs) + def completed_jobs( self, trans, use_cached_job: bool, all_params: List[ToolStateJobInstancePopulatedT] ) -> Dict[int, Optional[model.Job]]: diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 25d6775446f9..9de78f2fbff5 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -389,53 +389,6 @@ def populate_state( ): """ Populates nested state dict from incoming parameter values. - >>> from galaxy.util import XML - >>> from galaxy.util.bunch import Bunch - >>> from galaxy.tools.parameters.basic import TextToolParameter, BooleanToolParameter - >>> from galaxy.tools.parameters.grouping import Repeat - >>> trans = Bunch(workflow_building_mode=False) - >>> a = TextToolParameter(None, XML('')) - >>> b = Repeat('b') - >>> b.min = 0 - >>> b.max = 1 - >>> c = TextToolParameter(None, XML('')) - >>> d = Repeat('d') - >>> d.min = 0 - >>> d.max = 1 - >>> e = TextToolParameter(None, XML('')) - >>> f = Conditional('f') - >>> g = BooleanToolParameter(None, XML('')) - >>> h = TextToolParameter(None, XML('')) - >>> i = TextToolParameter(None, XML('')) - >>> b.inputs = dict([('c', c), ('d', d)]) - >>> d.inputs = dict([('e', e), ('f', f)]) - >>> f.test_param = g - >>> f.cases = [Bunch(value='true', inputs= { 'h': h }), Bunch(value='false', inputs= { 'i': i })] - >>> inputs = dict([('a',a),('b',b)]) - >>> flat = dict([('a', 1), ('b_0|c', 2), ('b_0|d_0|e', 3), ('b_0|d_0|f|h', 4), ('b_0|d_0|f|g', True)]) - >>> state = {} - >>> populate_state(trans, inputs, flat, state, check=False) - >>> print(state['a']) - 1 - >>> print(state['b'][0]['c']) - 2 - >>> print(state['b'][0]['d'][0]['e']) - 3 - >>> print(state['b'][0]['d'][0]['f']['h']) - 4 - >>> # now test with input_format='21.01' - >>> nested = {'a': 1, 'b': [{'c': 2, 'd': [{'e': 3, 'f': {'h': 4, 'g': True}}]}]} - >>> state_new = {} - >>> populate_state(trans, inputs, nested, state_new, check=False, input_format='21.01') - >>> print(state_new['a']) - 1 - >>> print(state_new['b'][0]['c']) - 2 - >>> print(state_new['b'][0]['d'][0]['e']) - 3 - >>> print(state_new['b'][0]['d'][0]['f']['h']) - 4 - """ if errors is None: errors = {} diff --git a/test/unit/app/tools/test_populate_state.py b/test/unit/app/tools/test_populate_state.py new file mode 100644 index 000000000000..d641ff079397 --- /dev/null +++ b/test/unit/app/tools/test_populate_state.py @@ -0,0 +1,63 @@ +from typing import ( + Any, + cast, + Dict, +) + +from galaxy.tools.parameters import ( + populate_state, + ToolInputsT, +) +from galaxy.tools.parameters.basic import ( + BooleanToolParameter, + TextToolParameter, +) +from galaxy.tools.parameters.grouping import ( + Conditional, + ConditionalWhen, + Repeat, +) +from galaxy.util import XML +from galaxy.util.bunch import Bunch + +trans = Bunch(workflow_building_mode=False) + + +def mock_when(**kwd): + return cast(ConditionalWhen, Bunch(**kwd)) + + +def test_populate_state(): + a = TextToolParameter(None, XML('')) + b = Repeat("b") + b.min = 0 + b.max = 1 + c = TextToolParameter(None, XML('')) + d = Repeat("d") + d.min = 0 + d.max = 1 + e = TextToolParameter(None, XML('')) + f = Conditional("f") + g = BooleanToolParameter(None, XML('')) + h = TextToolParameter(None, XML('')) + i = TextToolParameter(None, XML('')) + b.inputs = {"c": c, "d": d} + d.inputs = {"e": e, "f": f} + f.test_param = g + f.cases = [mock_when(value="true", inputs={"h": h}), mock_when(value="false", inputs={"i": i})] + inputs = {"a": a, "b": b} + flat = {"a": 1, "b_0|c": 2, "b_0|d_0|e": 3, "b_0|d_0|f|h": 4, "b_0|d_0|f|g": True} + state: Dict[str, Any] = {} + populate_state(trans, cast(ToolInputsT, inputs), flat, state, check=False) + assert state["a"] == 1 + assert state["b"][0]["c"] == 2 + assert state["b"][0]["d"][0]["e"] == 3 + assert state["b"][0]["d"][0]["f"]["h"] == 4 + # now test with input_format='21.01' + nested = {"a": 1, "b": [{"c": 2, "d": [{"e": 3, "f": {"h": 4, "g": True}}]}]} + state_new: Dict[str, Any] = {} + populate_state(trans, cast(ToolInputsT, inputs), nested, state_new, check=False, input_format="21.01") + assert state_new["a"] == 1 + assert state_new["b"][0]["c"] == 2 + assert state_new["b"][0]["d"][0]["e"] == 3 + assert state_new["b"][0]["d"][0]["f"]["h"] == 4 From f27b2025005fdaf3c4521d0286f90f41a70bb13e Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 27 Sep 2024 10:50:03 -0400 Subject: [PATCH 11/42] Spelling error. --- test/unit/tool_util/parameter_specification.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml index 761a1c3fb680..2eb07ada9f38 100644 --- a/test/unit/tool_util/parameter_specification.yml +++ b/test/unit/tool_util/parameter_specification.yml @@ -445,7 +445,7 @@ gx_hidden: - parameter: moocow - {} workflow_step_invalid: - - parmaeter: 5 + - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } workflow_step_linked_valid: @@ -470,7 +470,7 @@ gx_hidden_optional: - {} - parameter: null workflow_step_invalid: - - parmaeter: 5 + - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } workflow_step_linked_valid: From 7ef5cc22e4a88f707759887647347bf942f9c372 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 27 Sep 2024 10:52:34 -0400 Subject: [PATCH 12/42] Refactor for reuse in galaxy.tool_util.parameters. --- lib/galaxy/tool_util/parameters/__init__.py | 4 + lib/galaxy/tool_util/parameters/models.py | 80 ++++----- .../tool_util/test_parameter_specification.py | 170 ++++-------------- 3 files changed, 72 insertions(+), 182 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/__init__.py b/lib/galaxy/tool_util/parameters/__init__.py index 4a54ba0a8373..8df9db0c1f16 100644 --- a/lib/galaxy/tool_util/parameters/__init__.py +++ b/lib/galaxy/tool_util/parameters/__init__.py @@ -39,6 +39,7 @@ HiddenParameterModel, IntegerParameterModel, LabelValue, + RawStateDict, RepeatParameterModel, RulesParameterModel, SelectParameterModel, @@ -55,6 +56,7 @@ validate_test_case, validate_workflow_step, validate_workflow_step_linked, + ValidationFunctionT, ) from .state import ( JobInternalToolState, @@ -113,6 +115,8 @@ "ConditionalParameterModel", "ConditionalWhen", "RepeatParameterModel", + "RawStateDict", + "ValidationFunctionT", "validate_against_model", "validate_internal_job", "validate_internal_request", diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py index 11957c37099a..329423793d57 100644 --- a/lib/galaxy/tool_util/parameters/models.py +++ b/lib/galaxy/tool_util/parameters/models.py @@ -70,6 +70,9 @@ "workflow_step_linked", ] +DEFAULT_MODEL_NAME = "DynamicModelForTool" +RawStateDict = Dict[str, Any] + # could be made more specific - validators need to be classmethod ValidatorDictT = Dict[str, Callable] @@ -1368,34 +1371,21 @@ def create_model_strict(*args, **kwd) -> Type[BaseModel]: return create_model(*args, __config__=model_config, **kwd) -def create_request_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "request") - - -def create_request_internal_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "request_internal") - - -def create_request_internal_dereferenced_model( - tool: ToolParameterBundle, name: str = "DynamicModelForTool" -) -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "request_internal_dereferenced") - - -def create_job_internal_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "job_internal") +def create_model_factory(state_representation: StateRepresentationT): + def create_method(tool: ToolParameterBundle, name: str = DEFAULT_MODEL_NAME) -> Type[BaseModel]: + return create_field_model(tool.parameters, name, state_representation) -def create_test_case_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "test_case_xml") + return create_method -def create_workflow_step_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "workflow_step") - - -def create_workflow_step_linked_model(tool: ToolParameterBundle, name: str = "DynamicModelForTool") -> Type[BaseModel]: - return create_field_model(tool.parameters, name, "workflow_step_linked") +create_request_model = create_model_factory("request") +create_request_internal_model = create_model_factory("request_internal") +create_request_internal_dereferenced_model = create_model_factory("request_internal_dereferenced") +create_job_internal_model = create_model_factory("job_internal") +create_test_case_model = create_model_factory("test_case_xml") +create_workflow_step_model = create_model_factory("workflow_step") +create_workflow_step_linked_model = create_model_factory("workflow_step_linked") def create_field_model( @@ -1432,36 +1422,26 @@ def validate_against_model(pydantic_model: Type[BaseModel], parameter_state: Dic raise RequestParameterInvalidException(str(e)) -def validate_request(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_request_model(tool) - validate_against_model(pydantic_model, request) - - -def validate_internal_request(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_request_internal_model(tool) - validate_against_model(pydantic_model, request) +class ValidationFunctionT(Protocol): + def __call__(self, tool: ToolParameterBundle, request: RawStateDict, name: str = DEFAULT_MODEL_NAME) -> None: ... -def validate_internal_request_dereferenced(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_request_internal_dereferenced_model(tool) - validate_against_model(pydantic_model, request) +def validate_model_type_factory(state_representation: StateRepresentationT) -> ValidationFunctionT: -def validate_internal_job(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_job_internal_model(tool) - validate_against_model(pydantic_model, request) - - -def validate_test_case(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_test_case_model(tool) - validate_against_model(pydantic_model, request) - + def validate_request(tool: ToolParameterBundle, request: Dict[str, Any], name: str = DEFAULT_MODEL_NAME) -> None: + pydantic_model = create_field_model( + tool.parameters, name=DEFAULT_MODEL_NAME, state_representation=state_representation + ) + validate_against_model(pydantic_model, request) -def validate_workflow_step(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_workflow_step_model(tool) - validate_against_model(pydantic_model, request) + return validate_request -def validate_workflow_step_linked(tool: ToolParameterBundle, request: Dict[str, Any]) -> None: - pydantic_model = create_workflow_step_linked_model(tool) - validate_against_model(pydantic_model, request) +validate_request = validate_model_type_factory("request") +validate_internal_request = validate_model_type_factory("request_internal") +validate_internal_request_dereferenced = validate_model_type_factory("request_internal_dereferenced") +validate_internal_job = validate_model_type_factory("job_internal") +validate_test_case = validate_model_type_factory("test_case_xml") +validate_workflow_step = validate_model_type_factory("workflow_step") +validate_workflow_step_linked = validate_model_type_factory("workflow_step_linked") diff --git a/test/unit/tool_util/test_parameter_specification.py b/test/unit/tool_util/test_parameter_specification.py index dd54ac7276be..012213a02499 100644 --- a/test/unit/tool_util/test_parameter_specification.py +++ b/test/unit/tool_util/test_parameter_specification.py @@ -1,9 +1,7 @@ import sys from functools import partial from typing import ( - Any, Callable, - Dict, List, Optional, ) @@ -15,6 +13,7 @@ from galaxy.tool_util.parameters import ( decode, encode, + RawStateDict, RequestInternalToolState, RequestToolState, ToolParameterBundleModel, @@ -25,6 +24,7 @@ validate_test_case, validate_workflow_step, validate_workflow_step_linked, + ValidationFunctionT, ) from galaxy.tool_util.parameters.json import to_json_schema_string from galaxy.tool_util.unittest_utils.parameters import ( @@ -33,9 +33,6 @@ ) from galaxy.util.resources import resource_string -RawStateDict = Dict[str, Any] - - if sys.version_info < (3, 8): # noqa: UP036 pytest.skip(reason="Pydantic tool parameter models require python3.8 or higher", allow_module_level=True) @@ -126,139 +123,48 @@ def _for_each(test: Callable, parameters: ToolParameterBundleModel, requests: Li test(parameters, request) -def _assert_request_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - try: - validate_request(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate request {request}. {e}") - - -def _assert_request_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_request(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on request {request} as expected." - - -def _assert_internal_request_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - try: - validate_internal_request(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate internal request {request}. {e}") - - -def _assert_internal_request_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_internal_request(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on internal request {request} as expected." - - -def _assert_internal_request_dereferenced_validates( - parameters: ToolParameterBundleModel, request: RawStateDict -) -> None: - try: - validate_internal_request_dereferenced(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate dereferenced internal request {request}. {e}") +def model_assertion_function_factory(validate_function: ValidationFunctionT, what: str): + def _assert_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: + try: + validate_function(parameters, request) + except RequestParameterInvalidException as e: + raise AssertionError(f"Parameters {parameters} failed to validate {what} {request}. {e}") -def _assert_internal_request_dereferenced_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_internal_request_dereferenced(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on dereferenced internal request {request} as expected." + def _assert_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: + exc = None + try: + validate_function(parameters, request) + except RequestParameterInvalidException as e: + exc = e + if exc is None: + raise AssertionError( + f"Parameters {parameters} didn't result in validation error on {what} {request} as expected." + ) -def _assert_internal_job_validates(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - try: - validate_internal_job(parameters, request) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate internal job description {request}. {e}") + return _assert_validates, _assert_invalid -def _assert_internal_job_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) -> None: - exc = None - try: - validate_internal_job(parameters, request) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on internal job description {request} as expected." - - -def _assert_test_case_validates(parameters: ToolParameterBundleModel, test_case: RawStateDict) -> None: - try: - validate_test_case(parameters, test_case) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate test_case {test_case}. {e}") - - -def _assert_test_case_invalid(parameters: ToolParameterBundleModel, test_case: RawStateDict) -> None: - exc = None - try: - validate_test_case(parameters, test_case) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on test_case {test_case} as expected." - - -def _assert_workflow_step_validates(parameters: ToolParameterBundleModel, workflow_step: RawStateDict) -> None: - try: - validate_workflow_step(parameters, workflow_step) - except RequestParameterInvalidException as e: - raise AssertionError(f"Parameters {parameters} failed to validate workflow step {workflow_step}. {e}") - - -def _assert_workflow_step_invalid(parameters: ToolParameterBundleModel, workflow_step: RawStateDict) -> None: - exc = None - try: - validate_workflow_step(parameters, workflow_step) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on workflow step {workflow_step} as expected." - - -def _assert_workflow_step_linked_validates( - parameters: ToolParameterBundleModel, workflow_step_linked: RawStateDict -) -> None: - try: - validate_workflow_step_linked(parameters, workflow_step_linked) - except RequestParameterInvalidException as e: - raise AssertionError( - f"Parameters {parameters} failed to validate linked workflow step {workflow_step_linked}. {e}" - ) - - -def _assert_workflow_step_linked_invalid( - parameters: ToolParameterBundleModel, workflow_step_linked: RawStateDict -) -> None: - exc = None - try: - validate_workflow_step_linked(parameters, workflow_step_linked) - except RequestParameterInvalidException as e: - exc = e - assert ( - exc is not None - ), f"Parameters {parameters} didn't result in validation error on linked workflow step {workflow_step_linked} as expected." - +_assert_request_validates, _assert_request_invalid = model_assertion_function_factory(validate_request, "request") +_assert_internal_request_validates, _assert_internal_request_invalid = model_assertion_function_factory( + validate_internal_request, "internal request" +) +_assert_internal_request_dereferenced_validates, _assert_internal_request_dereferenced_invalid = ( + model_assertion_function_factory(validate_internal_request_dereferenced, "dereferenced internal request") +) +_assert_internal_job_validates, _assert_internal_job_invalid = model_assertion_function_factory( + validate_internal_job, "internal job description" +) +_assert_test_case_validates, _assert_test_case_invalid = model_assertion_function_factory( + validate_test_case, "XML derived test case" +) +_assert_workflow_step_validates, _assert_workflow_step_invalid = model_assertion_function_factory( + validate_workflow_step, "workflow step tool state (unlinked)" +) +_assert_workflow_step_linked_validates, _assert_workflow_step_linked_invalid = model_assertion_function_factory( + validate_workflow_step_linked, "linked workflow step tool state" +) _assert_requests_validate = partial(_for_each, _assert_request_validates) _assert_requests_invalid = partial(_for_each, _assert_request_invalid) From b704a01020de4ff6bb2cf439de1657d65f6ec02b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 30 Sep 2024 14:49:25 -0400 Subject: [PATCH 13/42] Refactor encode/decode for reuse. --- lib/galaxy/tool_util/parameters/convert.py | 134 +++++++++++---------- 1 file changed, 73 insertions(+), 61 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index 9484cc6c7e71..bb1ebc911089 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -46,6 +46,7 @@ TestCaseToolState, ) from .visitor import ( + Callback, validate_explicit_conditional_test_value, visit_input_values, VISITOR_NO_REPLACEMENT, @@ -54,40 +55,22 @@ log = logging.getLogger(__name__) +DecodeFunctionT = Callable[[str], int] +EncodeFunctionT = Callable[[int], str] +DereferenceCallable = Callable[[DataRequestUri], DataRequestInternalHda] +# interfaces for adapting test data dictionaries to tool request dictionaries +# e.g. {class: File, path: foo.bed} => {src: hda, id: ab1235cdfea3} +AdaptDatasets = Callable[[JsonTestDatasetDefDict], DataRequestHda] +AdaptCollections = Callable[[JsonTestCollectionDefDict], DataCollectionRequest] + + def decode( external_state: RequestToolState, input_models: ToolParameterBundle, decode_id: Callable[[str], int] ) -> RequestInternalToolState: """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" external_state.validate(input_models) - - def decode_src_dict(src_dict: dict): - if "id" in src_dict: - decoded_dict = src_dict.copy() - decoded_dict["id"] = decode_id(src_dict["id"]) - return decoded_dict - else: - return src_dict - - def decode_callback(parameter: ToolParameterT, value: Any): - if parameter.parameter_type == "gx_data": - if value is None: - return VISITOR_NO_REPLACEMENT - data_parameter = cast(DataParameterModel, parameter) - if data_parameter.multiple: - assert isinstance(value, list), str(value) - return list(map(decode_src_dict, value)) - else: - assert isinstance(value, dict), str(value) - return decode_src_dict(value) - elif parameter.parameter_type == "gx_data_collection": - if value is None: - return VISITOR_NO_REPLACEMENT - assert isinstance(value, dict), str(value) - return decode_src_dict(value) - else: - return VISITOR_NO_REPLACEMENT - + decode_callback = _decode_callback_for(decode_id) internal_state_dict = visit_input_values( input_models, external_state, @@ -100,33 +83,11 @@ def decode_callback(parameter: ToolParameterT, value: Any): def encode( - external_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: Callable[[int], str] + external_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT ) -> RequestToolState: """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" - def encode_src_dict(src_dict: dict): - if "id" in src_dict: - encoded_dict = src_dict.copy() - encoded_dict["id"] = encode_id(src_dict["id"]) - return encoded_dict - else: - return src_dict - - def encode_callback(parameter: ToolParameterT, value: Any): - if parameter.parameter_type == "gx_data": - data_parameter = cast(DataParameterModel, parameter) - if data_parameter.multiple: - assert isinstance(value, list), str(value) - return list(map(encode_src_dict, value)) - else: - assert isinstance(value, dict), str(value) - return encode_src_dict(value) - elif parameter.parameter_type == "gx_data_collection": - assert isinstance(value, dict), str(value) - return encode_src_dict(value) - else: - return VISITOR_NO_REPLACEMENT - + encode_callback = _encode_callback_for(encode_id) request_state_dict = visit_input_values( input_models, external_state, @@ -137,9 +98,6 @@ def encode_callback(parameter: ToolParameterT, value: Any): return request_state -DereferenceCallable = Callable[[DataRequestUri], DataRequestInternalHda] - - def dereference( internal_state: RequestInternalToolState, input_models: ToolParameterBundle, dereference: DereferenceCallable ) -> RequestInternalDereferencedToolState: @@ -177,12 +135,6 @@ def dereference_callback(parameter: ToolParameterT, value: Any): return request_state -# interfaces for adapting test data dictionaries to tool request dictionaries -# e.g. {class: File, path: foo.bed} => {src: hda, id: ab1235cdfea3} -AdaptDatasets = Callable[[JsonTestDatasetDefDict], DataRequestHda] -AdaptCollections = Callable[[JsonTestCollectionDefDict], DataCollectionRequest] - - def encode_test( test_case_state: TestCaseToolState, input_models: ToolParameterBundle, @@ -358,3 +310,63 @@ def _select_which_when( raise Exception( f"Invalid conditional test value ({test_value}) for parameter ({conditional.test_parameter.name})" ) + + +def _encode_callback_for(encode_id: EncodeFunctionT) -> Callback: + + def encode_src_dict(src_dict: dict): + if "id" in src_dict: + encoded_dict = src_dict.copy() + encoded_dict["id"] = encode_id(src_dict["id"]) + return encoded_dict + else: + return src_dict + + def encode_callback(parameter: ToolParameterT, value: Any): + if parameter.parameter_type == "gx_data": + data_parameter = cast(DataParameterModel, parameter) + if data_parameter.multiple: + assert isinstance(value, list), str(value) + return list(map(encode_src_dict, value)) + else: + assert isinstance(value, dict), str(value) + return encode_src_dict(value) + elif parameter.parameter_type == "gx_data_collection": + assert isinstance(value, dict), str(value) + return encode_src_dict(value) + else: + return VISITOR_NO_REPLACEMENT + + return encode_callback + + +def _decode_callback_for(decode_id: DecodeFunctionT) -> Callback: + + def decode_src_dict(src_dict: dict): + if "id" in src_dict: + decoded_dict = src_dict.copy() + decoded_dict["id"] = decode_id(src_dict["id"]) + return decoded_dict + else: + return src_dict + + def decode_callback(parameter: ToolParameterT, value: Any): + if parameter.parameter_type == "gx_data": + if value is None: + return VISITOR_NO_REPLACEMENT + data_parameter = cast(DataParameterModel, parameter) + if data_parameter.multiple: + assert isinstance(value, list), str(value) + return list(map(decode_src_dict, value)) + else: + assert isinstance(value, dict), str(value) + return decode_src_dict(value) + elif parameter.parameter_type == "gx_data_collection": + if value is None: + return VISITOR_NO_REPLACEMENT + assert isinstance(value, dict), str(value) + return decode_src_dict(value) + else: + return VISITOR_NO_REPLACEMENT + + return decode_callback From e00b1957a8f1e8c97f04e647d3ac88a961b04008 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 1 Oct 2024 19:05:30 -0400 Subject: [PATCH 14/42] Use get_color_value in basic.py. --- lib/galaxy/tools/parameters/basic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 08ae06864732..d656665be21b 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -41,6 +41,7 @@ ) from galaxy.model.dataset_collections import builder from galaxy.schema.fetch_data import FilesPayload +from galaxy.tool_util.parameters.factory import get_color_value from galaxy.tool_util.parser import get_input_source as ensure_input_source from galaxy.tool_util.parser.util import ( boolean_is_checked, @@ -848,7 +849,7 @@ class ColorToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) - self.value = input_source.get("value", "#000000") + self.value = get_color_value(input_source) self.rgb = input_source.get_bool("rgb", False) def get_initial_value(self, trans, other_values): From c198c7cb9750e1eb1729e68ff50b44d458beb081 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 27 Sep 2024 10:22:34 -0400 Subject: [PATCH 15/42] models for tool landing request state... --- lib/galaxy/tool_util/parameters/__init__.py | 12 + lib/galaxy/tool_util/parameters/convert.py | 44 ++- lib/galaxy/tool_util/parameters/models.py | 46 ++- lib/galaxy/tool_util/parameters/state.py | 18 + lib/galaxy/tools/parameters/basic.py | 13 +- lib/galaxy/tools/wrappers.py | 16 +- .../tool_util/parameter_specification.yml | 330 ++++++++++++++++++ test/unit/tool_util/test_parameter_convert.py | 17 + .../tool_util/test_parameter_specification.py | 16 + 9 files changed, 494 insertions(+), 18 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/__init__.py b/lib/galaxy/tool_util/parameters/__init__.py index 8df9db0c1f16..45cd770a5e3f 100644 --- a/lib/galaxy/tool_util/parameters/__init__.py +++ b/lib/galaxy/tool_util/parameters/__init__.py @@ -5,6 +5,8 @@ encode, encode_test, fill_static_defaults, + landing_decode, + landing_encode, ) from .factory import ( from_input_source, @@ -50,8 +52,10 @@ ToolParameterT, validate_against_model, validate_internal_job, + validate_internal_landing_request, validate_internal_request, validate_internal_request_dereferenced, + validate_landing_request, validate_request, validate_test_case, validate_workflow_step, @@ -60,6 +64,8 @@ ) from .state import ( JobInternalToolState, + LandingRequestInternalToolState, + LandingRequestToolState, RequestInternalDereferencedToolState, RequestInternalToolState, RequestToolState, @@ -119,8 +125,10 @@ "ValidationFunctionT", "validate_against_model", "validate_internal_job", + "validate_internal_landing_request", "validate_internal_request", "validate_internal_request_dereferenced", + "validate_landing_request", "validate_request", "validate_test_case", "validate_workflow_step", @@ -134,6 +142,8 @@ "RequestToolState", "RequestInternalToolState", "RequestInternalDereferencedToolState", + "LandingRequestToolState", + "LandingRequestInternalToolState", "flat_state_path", "keys_starting_with", "visit_input_values", @@ -143,6 +153,8 @@ "encode", "encode_test", "fill_static_defaults", + "landing_decode", + "landing_encode", "dereference", "WorkflowStepToolState", "WorkflowStepLinkedToolState", diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index bb1ebc911089..94e4d0c5e569 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -40,6 +40,8 @@ ) from .state import ( JobInternalToolState, + LandingRequestInternalToolState, + LandingRequestToolState, RequestInternalDereferencedToolState, RequestInternalToolState, RequestToolState, @@ -67,7 +69,7 @@ def decode( external_state: RequestToolState, input_models: ToolParameterBundle, decode_id: Callable[[str], int] ) -> RequestInternalToolState: - """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + """Prepare an internal representation of tool state (request_internal) for storing in the database.""" external_state.validate(input_models) decode_callback = _decode_callback_for(decode_id) @@ -83,14 +85,14 @@ def decode( def encode( - external_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT + internal_state: RequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT ) -> RequestToolState: - """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + """Prepare an external representation of tool state (request) from persisted state in the database (request_internal).""" encode_callback = _encode_callback_for(encode_id) request_state_dict = visit_input_values( input_models, - external_state, + internal_state, encode_callback, ) request_state = RequestToolState(request_state_dict) @@ -98,6 +100,40 @@ def encode( return request_state +def landing_decode( + external_state: LandingRequestToolState, input_models: ToolParameterBundle, decode_id: Callable[[str], int] +) -> LandingRequestInternalToolState: + """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + + external_state.validate(input_models) + decode_callback = _decode_callback_for(decode_id) + internal_state_dict = visit_input_values( + input_models, + external_state, + decode_callback, + ) + + internal_request_state = LandingRequestInternalToolState(internal_state_dict) + internal_request_state.validate(input_models) + return internal_request_state + + +def landing_encode( + internal_state: LandingRequestInternalToolState, input_models: ToolParameterBundle, encode_id: EncodeFunctionT +) -> LandingRequestToolState: + """Prepare an external representation of tool state (request) for storing in the database (request_internal).""" + + encode_callback = _encode_callback_for(encode_id) + request_state_dict = visit_input_values( + input_models, + internal_state, + encode_callback, + ) + request_state = LandingRequestToolState(request_state_dict) + request_state.validate(input_models) + return request_state + + def dereference( internal_state: RequestInternalToolState, input_models: ToolParameterBundle, dereference: DereferenceCallable ) -> RequestInternalDereferencedToolState: diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py index 329423793d57..d731560cc895 100644 --- a/lib/galaxy/tool_util/parameters/models.py +++ b/lib/galaxy/tool_util/parameters/models.py @@ -64,6 +64,8 @@ "request", "request_internal", "request_internal_dereferenced", + "landing_request", + "landing_request_internal", "job_internal", "test_case_xml", "workflow_step", @@ -219,6 +221,8 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam requires_value = self.request_requires_value if state_representation == "job_internal": requires_value = True + elif _is_landing_request(state_representation): + requires_value = False return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property @@ -240,7 +244,12 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam py_type = self.py_type if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) - return dynamic_model_information_from_py_type(self, py_type) + requires_value = self.request_requires_value + if state_representation == "job_internal": + requires_value = True + elif _is_landing_request(state_representation): + requires_value = False + return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property def request_requires_value(self) -> bool: @@ -405,10 +414,19 @@ def py_type_test_case(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: if state_representation == "request": return allow_batching(dynamic_model_information_from_py_type(self, self.py_type), BatchDataInstance) + if state_representation == "landing_request": + return allow_batching( + dynamic_model_information_from_py_type(self, self.py_type, requires_value=False), BatchDataInstance + ) elif state_representation == "request_internal": return allow_batching( dynamic_model_information_from_py_type(self, self.py_type_internal), BatchDataInstanceInternal ) + elif state_representation == "landing_request_internal": + return allow_batching( + dynamic_model_information_from_py_type(self, self.py_type_internal, requires_value=False), + BatchDataInstanceInternal, + ) elif state_representation == "request_internal_dereferenced": return allow_batching( dynamic_model_information_from_py_type(self, self.py_type_internal_dereferenced), @@ -455,6 +473,12 @@ def py_type_internal(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: if state_representation == "request": return allow_batching(dynamic_model_information_from_py_type(self, self.py_type)) + elif state_representation == "landing_request": + return allow_batching(dynamic_model_information_from_py_type(self, self.py_type, requires_value=False)) + elif state_representation == "landing_request_internal": + return allow_batching( + dynamic_model_information_from_py_type(self, self.py_type_internal, requires_value=False) + ) elif state_representation in ["request_internal", "request_internal_dereferenced"]: return allow_batching(dynamic_model_information_from_py_type(self, self.py_type_internal)) elif state_representation == "job_internal": @@ -600,7 +624,10 @@ def py_type(self) -> Type: return AnyUrl def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: - return dynamic_model_information_from_py_type(self, self.py_type) + requires_value = self.request_requires_value + if _is_landing_request(state_representation): + requires_value = False + return dynamic_model_information_from_py_type(self, self.py_type, requires_value=requires_value) @property def request_requires_value(self) -> bool: @@ -1080,9 +1107,14 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam instance_class: Type[BaseModel] = create_field_model( self.parameters, f"Repeat_{self.name}", state_representation ) + min_length = self.min + max_length = self.max requires_value = self.request_requires_value if state_representation == "job_internal": requires_value = True + elif _is_landing_request(state_representation): + requires_value = False + min_length = 0 # in a landing request - parameters can be partially filled initialize_repeat: Any if requires_value: @@ -1091,7 +1123,7 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam initialize_repeat = None class RepeatType(RootModel): - root: List[instance_class] = Field(initialize_repeat, min_length=self.min, max_length=self.max) # type: ignore[valid-type] + root: List[instance_class] = Field(initialize_repeat, min_length=min_length, max_length=max_length) # type: ignore[valid-type] return DynamicModelInformation( self.name, @@ -1382,6 +1414,8 @@ def create_method(tool: ToolParameterBundle, name: str = DEFAULT_MODEL_NAME) -> create_request_model = create_model_factory("request") create_request_internal_model = create_model_factory("request_internal") create_request_internal_dereferenced_model = create_model_factory("request_internal_dereferenced") +create_landing_request_model = create_model_factory("landing_request") +create_landing_request_internal_model = create_model_factory("landing_request_internal") create_job_internal_model = create_model_factory("job_internal") create_test_case_model = create_model_factory("test_case_xml") create_workflow_step_model = create_model_factory("workflow_step") @@ -1413,6 +1447,10 @@ def create_field_model( return pydantic_model +def _is_landing_request(state_representation: StateRepresentationT): + return state_representation in ["landing_request", "landing_request_internal"] + + def validate_against_model(pydantic_model: Type[BaseModel], parameter_state: Dict[str, Any]) -> None: try: pydantic_model(**parameter_state) @@ -1441,6 +1479,8 @@ def validate_request(tool: ToolParameterBundle, request: Dict[str, Any], name: s validate_request = validate_model_type_factory("request") validate_internal_request = validate_model_type_factory("request_internal") validate_internal_request_dereferenced = validate_model_type_factory("request_internal_dereferenced") +validate_landing_request = validate_model_type_factory("landing_request") +validate_internal_landing_request = validate_model_type_factory("landing_request_internal") validate_internal_job = validate_model_type_factory("job_internal") validate_test_case = validate_model_type_factory("test_case_xml") validate_workflow_step = validate_model_type_factory("workflow_step") diff --git a/lib/galaxy/tool_util/parameters/state.py b/lib/galaxy/tool_util/parameters/state.py index af15cf23ac7b..3edc96f69c6e 100644 --- a/lib/galaxy/tool_util/parameters/state.py +++ b/lib/galaxy/tool_util/parameters/state.py @@ -15,6 +15,8 @@ from .models import ( create_job_internal_model, + create_landing_request_internal_model, + create_landing_request_model, create_request_internal_dereferenced_model, create_request_internal_model, create_request_model, @@ -84,6 +86,22 @@ def _parameter_model_for(cls, parameters: ToolParameterBundle) -> Type[BaseModel return create_request_internal_model(parameters) +class LandingRequestToolState(ToolState): + state_representation: Literal["landing_request"] = "landing_request" + + @classmethod + def _parameter_model_for(cls, parameters: ToolParameterBundle) -> Type[BaseModel]: + return create_landing_request_model(parameters) + + +class LandingRequestInternalToolState(ToolState): + state_representation: Literal["landing_request_internal"] = "landing_request_internal" + + @classmethod + def _parameter_model_for(cls, parameters: ToolParameterBundle) -> Type[BaseModel]: + return create_landing_request_internal_model(parameters) + + class RequestInternalDereferencedToolState(ToolState): state_representation: Literal["request_internal_dereferenced"] = "request_internal_dereferenced" diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index d656665be21b..5aa07efdd204 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -187,7 +187,6 @@ def __init__(self, tool, input_source, context=None): self.hidden = input_source.get_bool("hidden", False) self.refresh_on_change = input_source.get_bool("refresh_on_change", False) self.optional = input_source.parse_optional() - self.optionality_inferred = False self.is_dynamic = False self.label = input_source.parse_label() self.help = input_source.parse_help() @@ -352,6 +351,7 @@ def parse_name(input_source): class SimpleTextToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) + self.optionality_inferred = False super().__init__(tool, input_source) optional = input_source.get("optional", None) if optional is not None: @@ -405,6 +405,7 @@ class TextToolParameter(SimpleTextToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) + self.profile = tool.profile self.datalist = [] for title, value, _ in input_source.parse_static_options(): self.datalist.append({"label": title, "value": value}) @@ -420,6 +421,16 @@ def validate(self, value, trans=None): ): return super().validate(value, trans) + @property + def wrapper_default() -> Optional[str]: + """Handle change in default handling pre and post 23.0 profiles.""" + profile = self.profile + legacy_behavior = (profile is None or Version(str(profile)) < Version("23.0")) + default_value = None + if self.optional and self.optionality_inferred and legacy_behavior: + default_value = "" + return default_value + def to_dict(self, trans, other_values=None): d = super().to_dict(trans) other_values = other_values or {} diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 01e8e8491bbc..34e5b84737e2 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -19,7 +19,6 @@ Union, ) -from packaging.version import Version from typing_extensions import TypeAlias from galaxy.model import ( @@ -33,7 +32,10 @@ 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.basic import ( + BooleanToolParameter, + TextToolParameter, +) from galaxy.tools.parameters.wrapped_json import ( data_collection_input_to_staging_path_and_source_path, data_input_to_staging_path_and_source_path, @@ -126,15 +128,9 @@ def __init__( profile: Optional[float] = None, ) -> None: self.input = input - if ( - value is None - and input.type == "text" - and input.optional - and input.optionality_inferred - and (profile is None or Version(str(profile)) < Version("23.0")) - ): + if value is None and input.type == "text": # Tools with old profile versions may treat an optional text parameter as `""` - value = "" + value = cast(TextToolParameter, input).wrapper_default() self.value = value self._other_values: Dict[str, str] = other_values or {} diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml index 2eb07ada9f38..29911941e714 100644 --- a/test/unit/tool_util/parameter_specification.yml +++ b/test/unit/tool_util/parameter_specification.yml @@ -32,6 +32,18 @@ gx_int: job_internal_invalid: - parameter: null - {} + landing_request_valid: + - {} + - parameter: 5 + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: 5 + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" test_case_xml_valid: - parameter: 5 - {} @@ -71,6 +83,21 @@ gx_boolean: - parameter: false job_internal_invalid: - {} + landing_request_valid: + - {} + - parameter: true + - parameter: false + landing_request_invalid: + - parameter: "true" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: true + - parameter: false + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "true" + - parameter: "5" workflow_step_valid: - parameter: True - {} @@ -102,6 +129,20 @@ gx_int_optional: - parameter: null job_internal_invalid: - {} + landing_request_valid: + - {} + - parameter: 5 + - parameter: null + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: 5 + - parameter: null + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" workflow_step_valid: - parameter: 5 - parameter: null @@ -132,6 +173,20 @@ gx_int_required: &gx_int_required - parameter: 5 job_internal_invalid: - {} + # and actual request will require a value but the landing only needs to specify parameters + # of interest - forcing users to make the rest of the selections. + landing_request_valid: + - {} + - parameter: 5 + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + landing_request_internal_valid: + - {} + - parameter: 5 + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" gx_int_required_via_empty_string: <<: *gx_int_required @@ -156,6 +211,14 @@ gx_text: *gx_text_request_valid request_internal_dereferenced_invalid: *gx_text_request_invalid + landing_request_valid: + *gx_text_request_valid + landing_request_invalid: + *gx_text_request_invalid + landing_request_internal_valid: + *gx_text_request_valid + landing_request_internal_invalid: + *gx_text_request_invalid job_internal_valid: - parameter: moocow - parameter: 'some spaces' @@ -198,6 +261,18 @@ gx_text_optional: - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } + landing_request_valid: &gx_text_optional_landing_request_valid + - parameter: "moo cow" + - {} + - parameter: null + landing_request_invalid: &gx_text_optional_landing_request_invalid + - parameter: 5 + - parameter: 6 + landing_request_internal_valid: + *gx_text_optional_landing_request_valid + landing_request_internal_invalid: + *gx_text_optional_landing_request_invalid + workflow_step_valid: - parameter: moocow - parameter: 'some spaces' @@ -243,6 +318,18 @@ gx_select: *gx_select_request_valid request_internal_dereferenced_invalid: *gx_select_request_invalid + landing_request_valid: &gx_select_landing_request_valid + - parameter: "--ex1" + - parameter: "ex2" + - {} + landing_request_invalid: &gx_select_landing_request_invalid + - parameter: 5 + - parameter: null + - parameter: "Ex1" + landing_request_internal_valid: + *gx_select_landing_request_valid + landing_request_internal_invalid: + *gx_select_landing_request_invalid job_internal_valid: - parameter: '--ex1' - parameter: 'ex2' @@ -283,6 +370,19 @@ gx_select_optional: - parameter: ["ex2"] - parameter: {} - parameter: 5 + landing_request_valid: &gx_select_optional_landing_request_valid + - parameter: "--ex1" + - parameter: "ex2" + - parameter: null + - {} + landing_request_invalid: &gx_select_optional_landing_request_invalid + - parameter: 5 + - parameter: "Ex1" + - parameter: {} + landing_request_internal_valid: + *gx_select_optional_landing_request_valid + landing_request_internal_invalid: + *gx_select_optional_landing_request_invalid job_internal_valid: - parameter: '--ex1' - parameter: 'ex2' @@ -326,6 +426,20 @@ gx_select_multiple: - parameter: ["Ex1"] - parameter: {} - parameter: 5 + landing_request_valid: &gx_select_multiple_landing_request_valid + - parameter: ["--ex1"] + - parameter: ["ex2"] + - parameter: null + - {} + landing_request_invalid: &gx_select_multiple_landing_request_invalid + - parameter: 5 + - parameter: ["Ex1"] + - parameter: {} + - parameter: false + landing_request_internal_valid: + *gx_select_multiple_landing_request_valid + landing_request_internal_invalid: + *gx_select_multiple_landing_request_invalid workflow_step_valid: - parameter: ["--ex1"] - parameter: ["ex2"] @@ -379,6 +493,17 @@ gx_genomebuild: request_invalid: - parameter: null - parameter: 9 + landing_request_valid: &gx_genomebuild_landing_request_valid + - parameter: hg19 + - parameter: hg18 + - {} + landing_request_invalid: &gx_genomebuild_landing_request_invalid + - parameter: null + - parameter: 9 + landing_request_internal_valid: + *gx_genomebuild_landing_request_valid + landing_request_internal_invalid: + *gx_genomebuild_landing_request_invalid job_internal_valid: - parameter: hg19 job_internal_invalid: @@ -394,6 +519,17 @@ gx_genomebuild_optional: request_invalid: - parameter: 8 - parameter: true + landing_request_valid: &gx_genomebuild_optional_landing_request_valid + - parameter: hg19 + - parameter: hg18 + - {} + - parameter: null + landing_request_invalid: &gx_genomebuild_optional_landing_request_invalid + - parameter: 9 + landing_request_internal_valid: + *gx_genomebuild_optional_landing_request_valid + landing_request_internal_invalid: + *gx_genomebuild_optional_landing_request_invalid job_internal_valid: - parameter: null job_internal_invalid: @@ -415,6 +551,18 @@ gx_directory_uri: - parameter: "justsomestring" - parameter: true - parameter: null + landing_request_valid: &gx_directory_uri_landing_request_valid + - parameter: "gxfiles://foobar/" + - parameter: "gxfiles://foobar" + - {} + landing_request_invalid: &gx_directory_uri_landing_request_invalid + - parameter: true + - parameter: null + - parameter: 8 + landing_request_internal_valid: + *gx_directory_uri_landing_request_valid + landing_request_internal_invalid: + *gx_directory_uri_landing_request_invalid job_internal_valid: - parameter: "gxfiles://foobar/" - parameter: "gxfiles://foobar" @@ -435,6 +583,20 @@ gx_hidden: - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } + landing_request_valid: &gx_hidden_landing_request_valid + - parameter: moocow + - parameter: 'some spaces' + - parameter: '' + - {} + landing_request_invalid: &gx_hidden_landing_request_invalid + - parameter: null + - parameter: 5 + - parameter: {} + - parameter: { "moo": "cow" } + landing_request_internal_valid: + *gx_hidden_landing_request_valid + landing_request_internal_invalid: + *gx_hidden_landing_request_invalid job_internal_valid: - parameter: moocow - parameter: 'some spaces' @@ -465,6 +627,20 @@ gx_hidden_optional: - parameter: 5 - parameter: {} - parameter: { "moo": "cow" } + landing_request_valid: &gx_hidden_optional_landing_request_valid + - parameter: moocow + - parameter: 'some spaces' + - parameter: '' + - {} + - parameter: null + landing_request_invalid: &gx_hidden_optional_landing_request_invalid + - parameter: 5 + - parameter: {} + - parameter: { "moo": "cow" } + landing_request_internal_valid: + *gx_hidden_optional_landing_request_valid + landing_request_internal_invalid: + *gx_hidden_optional_landing_request_invalid workflow_step_valid: - parameter: moocow - {} @@ -492,6 +668,30 @@ gx_float: - parameter: "5" - parameter: "5.0" - parameter: { "moo": "cow" } + job_internal_valid: + - parameter: 5 + - parameter: 5.0 + job_internal_invalid: + - {} + - parameter: "5" + - parameter: "5.0" + - parameter: null + landing_request_valid: + - {} + - parameter: 5 + - parameter: 5.0 + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" + landing_request_internal_valid: + - {} + - parameter: 5 + - parameter: 5.0 + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" test_case_xml_valid: - parameter: 5 - parameter: 5.0 @@ -529,6 +729,32 @@ gx_float_optional: - parameter: "5.0" - parameter: {} - parameter: { "moo": "cow" } + job_internal_valid: + - parameter: 5 + - parameter: 5.0 + - parameter: null + job_internal_invalid: + - {} + - parameter: "5" + - parameter: "5.0" + landing_request_valid: + - {} + - parameter: 5 + - parameter: 5.0 + - parameter: null + landing_request_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" + landing_request_internal_valid: + - {} + - parameter: 5 + - parameter: 5.0 + - parameter: null + landing_request_internal_invalid: + - parameter: "moo" + - parameter: "5" + - parameter: "5.0" test_case_xml_valid: - parameter: 5 - parameter: 5.0 @@ -616,6 +842,24 @@ gx_data: # the difference between request internal and request internal dereferenced is that these have been converted # to datasets. - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", ext: "txt"} + landing_request_valid: + - parameter: {src: hda, id: abcdabcd} + - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", "ext": "txt"} + - parameter: {__class__: "Batch", values: [{src: hdca, id: abcdabcd}]} + - {} + - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", "ext": "txt"} + landing_request_invalid: + - parameter: {__class__: "Batch", values: [{src: hdca, id: 5}]} + - parameter: {src: hda, id: 5} + - parameter: {src: hda, id: 0} + landing_request_internal_valid: + - parameter: {src: hda, id: 5} + - parameter: {__class__: "Batch", values: [{src: hdca, id: 5}]} + - {} + - parameter: {src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", "ext": "txt"} + landing_request_internal_invalid: + - parameter: {src: hda, id: abcdabcd} + - parameter: {__class__: "Batch", values: [{src: hdca, id: abcdabcd}]} job_internal_valid: - parameter: {src: hda, id: 7} job_internal_invalid: @@ -744,6 +988,16 @@ gx_data_multiple: # the difference between request internal and request internal dereferenced is that these have been converted # to datasets. - parameter: [{src: url, url: "https://raw.githubusercontent.com/galaxyproject/planemo/7be1bf5b3971a43eaa73f483125bfb8cabf1c440/tests/data/hello.txt", ext: "txt"}] + landing_request_valid: + - parameter: [{src: hda, id: abcdabcd}] + - {} + landing_request_invalid: + - parameter: [{src: hda, id: 6}] + landing_request_internal_valid: + - parameter: [{src: hda, id: 6}] + - {} + landing_request_internal_invalid: + - parameter: [{src: hda, id: abcdabcd}] job_internal_valid: - parameter: {src: hda, id: 5} - parameter: [{src: hda, id: 5}, {src: hda, id: 6}] @@ -824,6 +1078,18 @@ gx_data_collection: - parameter: {src: hdca, id: 5} request_internal_dereferenced_invalid: - parameter: {src: hdca, id: abcdabcd} + landing_request_valid: + - parameter: {src: hdca, id: abcdabcd} + - {} + landing_request_invalid: + - parameter: {src: hdca, id: 5} + - parameter: [{src: hdca, id: abcdabcd}] + landing_request_internal_valid: + - parameter: {src: hdca, id: 5} + - {} + landing_request_internal_invalid: + - parameter: {src: hdca, id: abcdabcd} + - parameter: [{src: hdca, id: abcdabcd}] workflow_step_valid: - {} workflow_step_invalid: @@ -909,6 +1175,20 @@ gx_data_collection_optional: - parameter: 5 - parameter: "5" - parameter: {} + landing_request_valid: + - parameter: {src: hdca, id: abcdabcd} + - parameter: null + - {} + landing_request_invalid: + - parameter: {src: hdca, id: 5} + - parameter: [{src: hdca, id: abcdabcd}] + landing_request_internal_valid: + - parameter: {src: hdca, id: 5} + - parameter: null + - {} + landing_request_internal_invalid: + - parameter: {src: hdca, id: abcdabcd} + - parameter: [{src: hdca, id: abcdabcd}] job_internal_valid: - parameter: {src: hdca, id: 5} - parameter: null @@ -957,6 +1237,16 @@ gx_conditional_boolean: # in that case having an integer_parameter is not acceptable. - conditional_parameter: integer_parameter: 5 + landing_request_valid: &gx_conditional_boolean_landing_request_valid + - {} + landing_request_invalid: &gx_conditional_boolean_landing_request_invalid + - conditional_parameter: + test_parameter: false + integer_parameter: 1 + landing_request_internal_valid: + *gx_conditional_boolean_landing_request_valid + landing_request_internal_invalid: + *gx_conditional_boolean_landing_request_invalid job_internal_valid: - conditional_parameter: test_parameter: true @@ -1106,6 +1396,20 @@ gx_repeat_boolean: - { boolean_parameter: false } - { boolean_parameter: 4 } - parameter: 5 + landing_request_valid: &gx_repeat_boolean_landing_request_valid + - {} + - parameter: + - { boolean_parameter: true } + - { boolean_parameter: false } + - parameter: [{}] + - parameter: [{}, {}] + landing_request_invalid: &gx_repeat_boolean_landing_request_invalid + - parameter: + - { boolean_parameter: 4 } + landing_request_internal_valid: + *gx_repeat_boolean_landing_request_valid + landing_request_internal_invalid: + *gx_repeat_boolean_landing_request_invalid job_internal_valid: - parameter: - { boolean_parameter: true } @@ -1174,6 +1478,16 @@ gx_repeat_data: request_internal_invalid: - parameter: - { data_parameter: { src: hda, id: abcdabcd } } + landing_request_valid: &gx_repeat_data_landing_request_valid + - {} + # unlike above - in landing mode all parameters are optional so it is find to have an unset data parameters + - parameter: [{}] + landing_request_invalid: &gx_repeat_data_landing_request_invalid + - parameter: 5 + landing_request_internal_valid: + *gx_repeat_data_landing_request_valid + landing_request_internal_invalid: + *gx_repeat_data_landing_request_invalid job_internal_valid: - parameter: - { data_parameter: { src: hda, id: 5 } } @@ -1197,6 +1511,22 @@ gx_repeat_data_min: - parameter: [{}, {}] - parameter: [{}] - parameter: 5 + landing_request_valid: + - {} + # ignore the min here - can just specify the first set of parameters and I think that should work + - parameter: + - { data_parameter: {src: hda, id: abcdabcd} } + landing_request_invalid: + - parameter: + - { data_parameter: {src: hda, id: 5} } + landing_request_internal_valid: + - {} + # ignore the min here - can just specify the first set of parameters and I think that should work + - parameter: + - { data_parameter: {src: hda, id: 5} } + landing_request_internal_invalid: + - parameter: + - { data_parameter: {src: hda, id: abcdabcd} } request_internal_valid: - parameter: - { data_parameter: { src: hda, id: 5 } } diff --git a/test/unit/tool_util/test_parameter_convert.py b/test/unit/tool_util/test_parameter_convert.py index f86750026766..a14eab6d5657 100644 --- a/test/unit/tool_util/test_parameter_convert.py +++ b/test/unit/tool_util/test_parameter_convert.py @@ -12,6 +12,9 @@ encode, fill_static_defaults, input_models_for_tool_source, + landing_decode, + landing_encode, + LandingRequestToolState, RequestInternalDereferencedToolState, RequestInternalToolState, RequestToolState, @@ -102,6 +105,20 @@ def test_multi_data(): assert encoded_state.input_state["parameter"][1]["id"] == EXAMPLE_ID_2_ENCODED +def test_landing_encode_data(): + tool_source = tool_source_for("parameters/gx_data") + bundle = input_models_for_tool_source(tool_source) + request_state = LandingRequestToolState({"parameter": {"src": "hda", "id": EXAMPLE_ID_1_ENCODED}}) + request_state.validate(bundle) + decoded_state = landing_decode(request_state, bundle, _fake_decode) + assert decoded_state.input_state["parameter"]["src"] == "hda" + assert decoded_state.input_state["parameter"]["id"] == EXAMPLE_ID_1 + + encoded_state = landing_encode(decoded_state, bundle, _fake_encode) + assert encoded_state.input_state["parameter"]["src"] == "hda" + assert encoded_state.input_state["parameter"]["id"] == EXAMPLE_ID_1_ENCODED + + def test_dereference(): tool_source = tool_source_for("parameters/gx_data") bundle = input_models_for_tool_source(tool_source) diff --git a/test/unit/tool_util/test_parameter_specification.py b/test/unit/tool_util/test_parameter_specification.py index 012213a02499..ed4b437449b5 100644 --- a/test/unit/tool_util/test_parameter_specification.py +++ b/test/unit/tool_util/test_parameter_specification.py @@ -18,8 +18,10 @@ RequestToolState, ToolParameterBundleModel, validate_internal_job, + validate_internal_landing_request, validate_internal_request, validate_internal_request_dereferenced, + validate_landing_request, validate_request, validate_test_case, validate_workflow_step, @@ -97,6 +99,10 @@ def _test_file(file: str, specification=None, parameter_bundle: Optional[ToolPar "request_internal_invalid": _assert_internal_requests_invalid, "request_internal_dereferenced_valid": _assert_internal_requests_dereferenced_validate, "request_internal_dereferenced_invalid": _assert_internal_requests_dereferenced_invalid, + "landing_request_valid": _assert_landing_requests_validate, + "landing_request_invalid": _assert_landing_requests_invalid, + "landing_request_internal_valid": _assert_internal_landing_requests_validate, + "landing_request_internal_invalid": _assert_internal_landing_requests_invalid, "job_internal_valid": _assert_internal_jobs_validate, "job_internal_invalid": _assert_internal_jobs_invalid, "test_case_xml_valid": _assert_test_cases_validate, @@ -165,6 +171,12 @@ def _assert_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) _assert_workflow_step_linked_validates, _assert_workflow_step_linked_invalid = model_assertion_function_factory( validate_workflow_step_linked, "linked workflow step tool state" ) +_assert_landing_request_validates, _assert_landing_request_invalid = model_assertion_function_factory( + validate_landing_request, "landing request" +) +_assert_internal_landing_request_validates, _assert_internal_landing_request_invalid = model_assertion_function_factory( + validate_internal_landing_request, " internallanding request" +) _assert_requests_validate = partial(_for_each, _assert_request_validates) _assert_requests_invalid = partial(_for_each, _assert_request_invalid) @@ -180,6 +192,10 @@ def _assert_invalid(parameters: ToolParameterBundleModel, request: RawStateDict) _assert_workflow_steps_invalid = partial(_for_each, _assert_workflow_step_invalid) _assert_workflow_steps_linked_validate = partial(_for_each, _assert_workflow_step_linked_validates) _assert_workflow_steps_linked_invalid = partial(_for_each, _assert_workflow_step_linked_invalid) +_assert_landing_requests_validate = partial(_for_each, _assert_landing_request_validates) +_assert_landing_requests_invalid = partial(_for_each, _assert_landing_request_invalid) +_assert_internal_landing_requests_validate = partial(_for_each, _assert_internal_landing_request_validates) +_assert_internal_landing_requests_invalid = partial(_for_each, _assert_internal_landing_request_invalid) def decode_val(val: str) -> int: From 730ea2ee03a284107dbad648c7ece1692225213c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 7 Oct 2024 11:07:49 -0400 Subject: [PATCH 16/42] Pydantic models for parameter validators. --- lib/galaxy/tool_util/parameters/_types.py | 11 + lib/galaxy/tool_util/parameters/factory.py | 76 +- lib/galaxy/tool_util/parameters/models.py | 116 ++- lib/galaxy/tool_util/parser/interface.py | 3 +- .../tool_util/parser/parameter_validators.py | 725 ++++++++++++++++++ lib/galaxy/tool_util/parser/util.py | 24 + lib/galaxy/tool_util/parser/xml.py | 8 +- .../tool_util/unittest_utils/sample_data.py | 53 ++ lib/galaxy/tools/parameters/basic.py | 35 +- .../tools/parameters/dynamic_options.py | 5 +- lib/galaxy/tools/parameters/validation.py | 398 ++++------ lib/galaxy/tools/wrappers.py | 2 +- .../gx_directory_uri_validation.xml | 16 + .../tools/parameters/gx_float_min_max.xml | 14 + .../parameters/gx_float_validation_range.xml | 15 + .../tools/parameters/gx_hidden_validation.xml | 17 + .../tools/parameters/gx_int_min_max.xml | 14 + .../parameters/gx_int_validation_range.xml | 15 + .../gx_text_expression_validation.xml | 20 + .../parameters/gx_text_length_validation.xml | 20 + .../gx_text_length_validation_negate.xml | 20 + .../parameters/gx_text_regex_validation.xml | 20 + test/unit/app/tools/test_dynamic_options.py | 6 +- .../app/tools/test_parameter_validation.py | 4 +- .../unit/app/tools/test_validation_parsing.py | 41 + .../tool_util/parameter_specification.yml | 147 +++- .../test_parameter_validator_models.py | 28 + 27 files changed, 1549 insertions(+), 304 deletions(-) create mode 100644 lib/galaxy/tool_util/parser/parameter_validators.py create mode 100644 test/functional/tools/parameters/gx_directory_uri_validation.xml create mode 100644 test/functional/tools/parameters/gx_float_min_max.xml create mode 100644 test/functional/tools/parameters/gx_float_validation_range.xml create mode 100644 test/functional/tools/parameters/gx_hidden_validation.xml create mode 100644 test/functional/tools/parameters/gx_int_min_max.xml create mode 100644 test/functional/tools/parameters/gx_int_validation_range.xml create mode 100644 test/functional/tools/parameters/gx_text_expression_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_length_validation.xml create mode 100644 test/functional/tools/parameters/gx_text_length_validation_negate.xml create mode 100644 test/functional/tools/parameters/gx_text_regex_validation.xml create mode 100644 test/unit/app/tools/test_validation_parsing.py create mode 100644 test/unit/tool_util/test_parameter_validator_models.py diff --git a/lib/galaxy/tool_util/parameters/_types.py b/lib/galaxy/tool_util/parameters/_types.py index 2b97ee16c200..64fa9a3274fc 100644 --- a/lib/galaxy/tool_util/parameters/_types.py +++ b/lib/galaxy/tool_util/parameters/_types.py @@ -6,6 +6,7 @@ """ from typing import ( + Any, cast, List, Optional, @@ -15,6 +16,7 @@ # https://stackoverflow.com/questions/56832881/check-if-a-field-is-typing-optional from typing_extensions import ( + Annotated, get_args, get_origin, ) @@ -46,3 +48,12 @@ def cast_as_type(arg) -> Type: def is_optional(field) -> bool: return get_origin(field) is Union and type(None) in get_args(field) + + +def expand_annotation(field: Type, new_annotations: List[Any]) -> Type: + is_annotation = get_origin(field) is Annotated + if is_annotation: + args = get_args(field) # noqa: F841 + return Annotated[tuple([args[0], *args[1:], *new_annotations])] # type: ignore[return-value] + else: + return Annotated[tuple([field, *new_annotations])] # type: ignore[return-value] diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py index e200b67aeb0e..13d52d5f9c77 100644 --- a/lib/galaxy/tool_util/parameters/factory.py +++ b/lib/galaxy/tool_util/parameters/factory.py @@ -14,7 +14,19 @@ PagesSource, ToolSource, ) -from galaxy.tool_util.parser.util import parse_profile_version +from galaxy.tool_util.parser.parameter_validators import ( + EmptyFieldParameterValidatorModel, + ExpressionParameterValidatorModel, + InRangeParameterValidatorModel, + LengthParameterValidatorModel, + NoOptionsParameterValidatorModel, + RegexParameterValidatorModel, + static_validators, +) +from galaxy.tool_util.parser.util import ( + parse_profile_version, + text_input_is_optional, +) from galaxy.util import string_as_bool from .models import ( BaseUrlParameterModel, @@ -42,10 +54,13 @@ HiddenParameterModel, IntegerParameterModel, LabelValue, + NumberCompatiableValidators, RepeatParameterModel, RulesParameterModel, SectionParameterModel, + SelectCompatiableValidators, SelectParameterModel, + TextCompatiableValidators, TextParameterModel, ToolParameterBundle, ToolParameterBundleModel, @@ -82,7 +97,23 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool int_value = None else: raise ParameterDefinitionError() - return IntegerParameterModel(name=input_source.parse_name(), optional=optional, value=int_value) + static_validator_models = static_validators(input_source.parse_validators()) + int_validators: List[NumberCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "in_range": + int_validators.append(cast(InRangeParameterValidatorModel, static_validator)) + min_raw = input_source.get("min", None) + max_raw = input_source.get("max", None) + min_int = int(min_raw) if min_raw is not None else None + max_int = int(max_raw) if max_raw is not None else None + return IntegerParameterModel( + name=input_source.parse_name(), + optional=optional, + value=int_value, + min=min_int, + max=max_int, + validators=int_validators, + ) elif param_type == "boolean": nullable = input_source.parse_optional() value = input_source.get_bool_or_none("checked", None if nullable else False) @@ -92,10 +123,12 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool value=value, ) elif param_type == "text": - optional = input_source.parse_optional() + optional, optionality_inferred = text_input_is_optional(input_source) + text_validators: List[TextCompatiableValidators] = _text_validators(input_source) return TextParameterModel( name=input_source.parse_name(), optional=optional, + validators=text_validators, ) elif param_type == "float": optional = input_source.parse_optional() @@ -107,18 +140,32 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool float_value = None else: raise ParameterDefinitionError() + static_validator_models = static_validators(input_source.parse_validators()) + float_validators: List[NumberCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "in_range": + float_validators.append(cast(InRangeParameterValidatorModel, static_validator)) + min_raw = input_source.get("min", None) + max_raw = input_source.get("max", None) + min_float = float(min_raw) if min_raw is not None else None + max_float = float(max_raw) if max_raw is not None else None return FloatParameterModel( name=input_source.parse_name(), optional=optional, value=float_value, + min=min_float, + max=max_float, + validators=float_validators, ) elif param_type == "hidden": optional = input_source.parse_optional() value = input_source.get("value") + hidden_validators: List[TextCompatiableValidators] = _text_validators(input_source) return HiddenParameterModel( name=input_source.parse_name(), optional=optional, value=value, + validators=hidden_validators, ) elif param_type == "color": optional = input_source.parse_optional() @@ -158,11 +205,17 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool options = [] for option_label, option_value, selected in input_source.parse_static_options(): options.append(LabelValue(label=option_label, value=option_value, selected=selected)) + static_validator_models = static_validators(input_source.parse_validators()) + select_validators: List[SelectCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "no_options": + select_validators.append(cast(NoOptionsParameterValidatorModel, static_validator)) return SelectParameterModel( name=input_source.parse_name(), optional=optional, options=options, multiple=multiple, + validators=select_validators, ) elif param_type == "drill_down": multiple = input_source.get_bool("multiple", False) @@ -206,8 +259,10 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool multiple=multiple, ) elif param_type == "directory_uri": + directory_uri_validators: List[TextCompatiableValidators] = _text_validators(input_source) return DirectoryUriParameterModel( name=input_source.parse_name(), + validators=directory_uri_validators, ) else: raise UnknownParameterTypeError(f"Unknown Galaxy parameter type {param_type}") @@ -304,6 +359,21 @@ def _simple_cwl_type_to_model(simple_type: str, input_source: CwlInputSource): ) +def _text_validators(input_source: InputSource) -> List[TextCompatiableValidators]: + static_validator_models = static_validators(input_source.parse_validators()) + text_validators: List[TextCompatiableValidators] = [] + for static_validator in static_validator_models: + if static_validator.type == "length": + text_validators.append(cast(LengthParameterValidatorModel, static_validator)) + elif static_validator.type == "regex": + text_validators.append(cast(RegexParameterValidatorModel, static_validator)) + elif static_validator.type == "expression": + text_validators.append(cast(ExpressionParameterValidatorModel, static_validator)) + elif static_validator.type == "empty_field": + text_validators.append(cast(EmptyFieldParameterValidatorModel, static_validator)) + return text_validators + + def _from_input_source_cwl(input_source: CwlInputSource) -> ToolParameterT: schema_salad_field = input_source.field if schema_salad_field is None: diff --git a/lib/galaxy/tool_util/parameters/models.py b/lib/galaxy/tool_util/parameters/models.py index d731560cc895..5019a92f5f70 100644 --- a/lib/galaxy/tool_util/parameters/models.py +++ b/lib/galaxy/tool_util/parameters/models.py @@ -11,11 +11,14 @@ Mapping, NamedTuple, Optional, + Sequence, Type, + TypeVar, Union, ) from pydantic import ( + AfterValidator, AnyUrl, BaseModel, ConfigDict, @@ -44,8 +47,18 @@ JsonTestCollectionDefDict, JsonTestDatasetDefDict, ) +from galaxy.tool_util.parser.parameter_validators import ( + EmptyFieldParameterValidatorModel, + ExpressionParameterValidatorModel, + InRangeParameterValidatorModel, + LengthParameterValidatorModel, + NoOptionsParameterValidatorModel, + RegexParameterValidatorModel, + StaticValidatorModel, +) from ._types import ( cast_as_type, + expand_annotation, is_optional, list_type, optional, @@ -54,7 +67,6 @@ ) # TODO: -# - implement job vs request... # - implement data_ref on rules and implement some cross model validation # + request: Return info needed to build request pydantic model at runtime. @@ -179,18 +191,64 @@ class LabelValue(BaseModel): selected: bool +TextCompatiableValidators = Union[ + LengthParameterValidatorModel, + RegexParameterValidatorModel, + ExpressionParameterValidatorModel, + EmptyFieldParameterValidatorModel, +] + + +def pydantic_to_galaxy_type(value: Any) -> Any: + """We use advanced Pydantic types like URL but the Galaxy validators only expect strings for these.""" + if isinstance(value, AnyUrl): + return str(value) + + return value + + +VT = TypeVar("VT", bound=StaticValidatorModel) + + +def decorate_type_with_validators_if_needed(py_type: Type, static_validator_models: Sequence[VT]) -> Type: + pydantic_validator = pydantic_validator_for(static_validator_models) + if pydantic_validator: + return expand_annotation(py_type, [pydantic_validator]) + else: + return py_type + + +# Looks like Annotated only work with one PlainValidator so condensing all static validators +# into a single PlainValidator for pydantic. +def pydantic_validator_for(static_validator_models: Sequence[VT]) -> Optional[AfterValidator]: + + if static_validator_models: + + def validator(v: Any) -> Any: + gx_val = pydantic_to_galaxy_type(v) + + for static_validator_model in static_validator_models: + static_validator_model.statically_validate(gx_val) + return v + + return AfterValidator(validator) + else: + return None + + class TextParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_text"] = "gx_text" area: bool = False default_value: Optional[str] = Field(default=None, alias="value") default_options: List[LabelValue] = [] + validators: List[TextCompatiableValidators] = [] @property def py_type(self) -> Type: return optional_if_needed(StrictStr, self.optional) def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: - py_type = self.py_type + py_type = decorate_type_with_validators_if_needed(self.py_type, self.validators) if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) requires_value = self.request_requires_value @@ -203,12 +261,16 @@ def request_requires_value(self) -> bool: return False +NumberCompatiableValidators = Union[InRangeParameterValidatorModel,] + + class IntegerParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_integer"] = "gx_integer" optional: bool value: Optional[int] = None min: Optional[int] = None max: Optional[int] = None + validators: List[NumberCompatiableValidators] = [] @property def py_type(self) -> Type: @@ -216,6 +278,10 @@ def py_type(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: py_type = self.py_type + validators = self.validators[:] + if self.min is not None or self.max is not None: + validators.append(InRangeParameterValidatorModel(min=self.min, max=self.max, implicit=True)) + py_type = decorate_type_with_validators_if_needed(py_type, validators) if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) requires_value = self.request_requires_value @@ -235,6 +301,7 @@ class FloatParameterModel(BaseGalaxyToolParameterModelDefinition): value: Optional[float] = None min: Optional[float] = None max: Optional[float] = None + validators: List[NumberCompatiableValidators] = [] @property def py_type(self) -> Type: @@ -249,6 +316,10 @@ def pydantic_template(self, state_representation: StateRepresentationT) -> Dynam requires_value = True elif _is_landing_request(state_representation): requires_value = False + validators = self.validators[:] + if self.min is not None or self.max is not None: + validators.append(InRangeParameterValidatorModel(min=self.min, max=self.max, implicit=True)) + py_type = decorate_type_with_validators_if_needed(py_type, validators) return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property @@ -502,6 +573,7 @@ def request_requires_value(self) -> bool: class HiddenParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_hidden"] = "gx_hidden" value: Optional[str] + validators: List[TextCompatiableValidators] = [] @property def py_type(self) -> Type: @@ -510,6 +582,7 @@ def py_type(self) -> Type: def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: py_type = self.py_type requires_value = not self.optional and self.value is None + py_type = decorate_type_with_validators_if_needed(py_type, self.validators) if state_representation == "workflow_step_linked": py_type = allow_connected_value(py_type) elif state_representation == "workflow_step" and not self.optional: @@ -618,16 +691,21 @@ def request_requires_value(self) -> bool: class DirectoryUriParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_directory_uri"] = "gx_directory_uri" + validators: List[TextCompatiableValidators] = [] @property def py_type(self) -> Type: return AnyUrl def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: + py_type = self.py_type + py_type = decorate_type_with_validators_if_needed(py_type, self.validators) + if state_representation == "workflow_step_linked": + py_type = allow_connected_value(py_type) requires_value = self.request_requires_value if _is_landing_request(state_representation): requires_value = False - return dynamic_model_information_from_py_type(self, self.py_type, requires_value=requires_value) + return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value) @property def request_requires_value(self) -> bool: @@ -659,10 +737,14 @@ def request_requires_value(self) -> bool: return True +SelectCompatiableValidators = Union[NoOptionsParameterValidatorModel,] + + class SelectParameterModel(BaseGalaxyToolParameterModelDefinition): parameter_type: Literal["gx_select"] = "gx_select" options: Optional[List[LabelValue]] = None multiple: bool + validators: List[SelectCompatiableValidators] @staticmethod def split_str(cls, data: Any) -> Any: @@ -699,28 +781,30 @@ def py_type_workflow_step(self) -> Type: return optional(self.py_type_if_required()) def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation: + validators = {} + requires_value = self.request_requires_value + py_type = None if state_representation == "workflow_step": - return dynamic_model_information_from_py_type(self, self.py_type_workflow_step, requires_value=False) + py_type = self.py_type_workflow_step elif state_representation == "workflow_step_linked": py_type = self.py_type_if_required(allow_connections=True) - return dynamic_model_information_from_py_type( - self, optional_if_needed(py_type, self.optional or self.multiple) - ) + py_type = optional_if_needed(py_type, self.optional or self.multiple) elif state_representation == "test_case_xml": # in a YAML test case representation this can be string, in XML we are still expecting a comma separated string py_type = self.py_type_if_required(allow_connections=False) if self.multiple: validators = {"from_string": field_validator(self.name, mode="before")(SelectParameterModel.split_str)} - else: - validators = {} - return dynamic_model_information_from_py_type( - self, optional_if_needed(py_type, self.optional), validators=validators - ) + py_type = optional_if_needed(py_type, self.optional) + elif state_representation == "job_internal": + requires_value = True + py_type = self.py_type else: - requires_value = self.request_requires_value - if state_representation == "job_internal": - requires_value = True - return dynamic_model_information_from_py_type(self, self.py_type, requires_value=requires_value) + py_type = self.py_type + + py_type = decorate_type_with_validators_if_needed(py_type, self.validators) + return dynamic_model_information_from_py_type( + self, py_type, validators=validators, requires_value=requires_value + ) @property def has_selected_static_option(self): diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index 441985902805..b9218ad54270 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -28,6 +28,7 @@ from galaxy.util import Element from galaxy.util.path import safe_walk +from .parameter_validators import AnyValidatorModel from .util import _parse_name if TYPE_CHECKING: @@ -502,7 +503,7 @@ def parse_sanitizer_elem(self): """ return None - def parse_validator_elems(self): + def parse_validators(self) -> List[AnyValidatorModel]: """Return an XML description of sanitizers. This is a stop gap until we can rework galaxy.tools.parameters.validation to not explicitly depend on XML. diff --git a/lib/galaxy/tool_util/parser/parameter_validators.py b/lib/galaxy/tool_util/parser/parameter_validators.py new file mode 100644 index 000000000000..6525b7d90fb8 --- /dev/null +++ b/lib/galaxy/tool_util/parser/parameter_validators.py @@ -0,0 +1,725 @@ +import json +from typing import ( + Any, + cast, + List, + Optional, + Sequence, + Union, +) + +from pydantic import ( + BaseModel, + ConfigDict, + Field, + model_validator, + PrivateAttr, +) +from typing_extensions import ( + Annotated, + get_args, + Literal, + Protocol, + Self, +) + +from galaxy.util import ( + asbool, + Element, +) + +try: + import regex +except ImportError: + import re as regex + + +class ValidationArgument: + doc: Optional[str] + xml_body: bool + xml_allow_json_load: bool + + def __init__( + self, + doc: Optional[str], + xml_body: bool = False, + xml_allow_json_load: bool = False, + ): + self.doc = doc + self.xml_body = xml_body + self.xml_allow_json_load = xml_allow_json_load + + +Negate = Annotated[ + bool, + ValidationArgument("Negates the result of the validator."), +] +NEGATE_DEFAULT = False +SPLIT_DEFAULT = "\t" +DEFAULT_VALIDATOR_MESSAGE = "Parameter validation error." + +ValidatorType = Literal[ + "expression", + "regex", + "in_range", + "length", + "metadata", + "dataset_metadata_equal", + "unspecified_build", + "no_options", + "empty_field", + "empty_dataset", + "empty_extra_files_path", + "dataset_metadata_in_data_table", + "dataset_metadata_not_in_data_table", + "dataset_metadata_in_range", + "value_in_data_table", + "value_not_in_data_table", + "dataset_ok_validator", + "dataset_metadata_in_file", +] + + +class StrictModel(BaseModel): + model_config = ConfigDict(extra="forbid") + + +class ParameterValidatorModel(StrictModel): + type: ValidatorType + message: Annotated[ + Optional[str], + ValidationArgument( + """The error message displayed on the tool form if validation fails. A placeholder string ``%s`` will be repaced by the ``value``""" + ), + ] = None + # track validators setup by other input parameters and not validation explicitly + implicit: bool = False + _static: bool = PrivateAttr(False) + _deprecated: bool = PrivateAttr(False) + + @model_validator(mode="after") + def set_default_message(self) -> Self: + if self.message is None: + self.message = self.default_message + return self + + @property + def default_message(self) -> str: + return DEFAULT_VALIDATOR_MESSAGE + + +class StaticValidatorModel(ParameterValidatorModel): + _static: bool = PrivateAttr(True) + + def statically_validate(self, v: Any) -> None: ... + + +class ExpressionParameterValidatorModel(StaticValidatorModel): + """Check if a one line python expression given expression evaluates to True. + + The expression is given is the content of the validator tag.""" + + type: Literal["expression"] = "expression" + negate: Negate = NEGATE_DEFAULT + expression: Annotated[str, ValidationArgument("Python expression to validate.", xml_body=True)] + + def statically_validate(self, value: Any) -> None: + ExpressionParameterValidatorModel.expression_validation(self.expression, value, self) + + @staticmethod + def ensure_compiled(expression: Union[str, Any]) -> Any: + if isinstance(expression, str): + return compile(expression, "", "eval") + else: + return expression + + @staticmethod + def expression_validation( + expression: str, value: Any, validator: "ValidatorDescription", compiled_expression: Optional[Any] = None + ): + if compiled_expression is None: + compiled_expression = ExpressionParameterValidatorModel.ensure_compiled(expression) + message = None + try: + evalresult = eval(compiled_expression, dict(value=value)) + except Exception: + message = f"Validator '{expression}' could not be evaluated on '{value}'" + evalresult = False + + raise_error_if_valiation_fails(bool(evalresult), validator, message=message, value_to_show=value) + + @property + def default_message(self) -> str: + return f"Value '%s' does not evaluate to {'True' if not self.negate else 'False'} for '{self.expression}'" + + +class RegexParameterValidatorModel(StaticValidatorModel): + """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.""" + + type: Literal["regex"] = "regex" + negate: Negate = NEGATE_DEFAULT + expression: Annotated[str, ValidationArgument("Regular expression to validate against.", xml_body=True)] + + @property + def default_message(self) -> str: + return f"Value '%s' does {'not ' if not self.negate else ''}match regular expression '{self.expression.replace('%', '%%')}'" + + def statically_validate(self, value: Any) -> None: + if value and not isinstance(value, str): + raise ValueError(f"Wrong type found value {value}") + RegexParameterValidatorModel.regex_validation(self.expression, value, self) + + @staticmethod + def regex_validation(expression: str, value: Any, validator: "ValidatorDescription"): + if not isinstance(value, list): + value = [value] + for val in value: + match = regex.match(expression, val or "") + raise_error_if_valiation_fails(match is not None, validator, value_to_show=val) + + +class InRangeParameterValidatorModel(StaticValidatorModel): + type: Literal["in_range"] = "in_range" + min: Optional[Union[float, int]] = None + max: Optional[Union[float, int]] = None + exclude_min: bool = False + exclude_max: bool = False + negate: Negate = NEGATE_DEFAULT + + def statically_validate(self, value: Any): + if isinstance(value, (int, float)): + validates = True + if self.min is not None and value == self.min and self.exclude_min: + validates = False + elif self.min is not None and value < self.min: + validates = False + elif self.max is not None and value == self.max and self.exclude_max: + validates = False + if self.max is not None and value > self.max: + validates = False + raise_error_if_valiation_fails(validates, self) + + @property + def default_message(self) -> str: + op1 = "<=" + op2 = "<=" + if self.exclude_min: + op1 = "<" + if self.exclude_max: + op2 = "<" + range_description_str = f"({self.min} {op1} value {op2} {self.max})" + return f"Value ('%s') must {'not ' if self.negate else ''}fulfill {range_description_str}" + + +class LengthParameterValidatorModel(StaticValidatorModel): + type: Literal["length"] = "length" + min: Optional[int] = None + max: Optional[int] = None + negate: Negate = NEGATE_DEFAULT + + def statically_validate(self, value: Any): + if isinstance(value, str): + length = len(value) + validates = True + if self.min is not None and length < self.min: + validates = False + if self.max is not None and length > self.max: + validates = False + raise_error_if_valiation_fails(validates, self) + + @property + def default_message(self) -> str: + return f"Must {'not ' if self.negate else ''}have length of at least {self.min} and at most {self.max}" + + +class MetadataParameterValidatorModel(ParameterValidatorModel): + type: Literal["metadata"] = "metadata" + check: Optional[List[str]] = None + skip: Optional[List[str]] = None + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + check = self.check + skip = self.skip + message = DEFAULT_VALIDATOR_MESSAGE + if not self.negate: + message = "Metadata '%s' missing, click the pencil icon in the history item to edit / save the metadata attributes" + else: + if check: + message = f"""At least one of the checked metadata '{",".join(check)}' is set, click the pencil icon in the history item to edit / save the metadata attributes""" + elif skip: + message = f"""At least one of the non skipped metadata '{",".join(skip)}' is set, click the pencil icon in the history item to edit / save the metadata attributes""" + return message + + +class DatasetMetadataEqualParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_equal"] = "dataset_metadata_equal" + metadata_name: str + value: Annotated[Any, ValidationArgument("Value to test against", xml_allow_json_load=True)] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + if not self.negate: + message = f"Metadata value for '{self.metadata_name}' must be '{self.value}', but it is '%s'." + else: + message = f"Metadata value for '{self.metadata_name}' must not be '{self.value}' but it is." + return message + + +class UnspecifiedBuildParameterValidatorModel(ParameterValidatorModel): + type: Literal["unspecified_build"] = "unspecified_build" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"{'Unspecified' if not self.negate else 'Specified'} genome build, click the pencil icon in the history item to {'set' if not self.negate else 'remove'} the genome build" + + +class NoOptionsParameterValidatorModel(StaticValidatorModel): + type: Literal["no_options"] = "no_options" + negate: Negate = NEGATE_DEFAULT + + @staticmethod + def no_options_validate(value: Any, validator: "ValidatorDescription"): + raise_error_if_valiation_fails(value is not None, validator) + + def statically_validate(self, value: Any) -> None: + NoOptionsParameterValidatorModel.no_options_validate(value, self) + + @property + def default_message(self) -> str: + return f"{'No options' if not self.negate else 'Options'} available for selection" + + +class EmptyFieldParameterValidatorModel(StaticValidatorModel): + type: Literal["empty_field"] = "empty_field" + negate: Negate = NEGATE_DEFAULT + + @staticmethod + def empty_validate(value: Any, validator: "ValidatorDescription"): + raise_error_if_valiation_fails((value != ""), validator) + + def statically_validate(self, value: Any) -> None: + EmptyFieldParameterValidatorModel.empty_validate(value, self) + + @property + def default_message(self) -> str: + if not self.negate: + message = "Field requires a value" + else: + message = "Field must not set a value" + return message + + +class EmptyDatasetParameterValidatorModel(ParameterValidatorModel): + type: Literal["empty_dataset"] = "empty_dataset" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"The selected dataset is {'non-' if self.negate else ''}empty, this tool expects {'non-' if not self.negate else ''}empty files." + + +class EmptyExtraFilesPathParameterValidatorModel(ParameterValidatorModel): + type: Literal["empty_extra_files_path"] = "empty_extra_files_path" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + negate = self.negate + return f"The selected dataset's extra_files_path directory is {'non-' if negate else ''}empty or does {'not ' if not negate else ''}exist, this tool expects {'non-' if not negate else ''}empty extra_files_path directories associated with the selected input." + + +class DatasetMetadataInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_in_data_table"] = "dataset_metadata_in_data_table" + table_name: str + metadata_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"Value for metadata {self.metadata_name} was not found in {self.table_name}." + + +class DatasetMetadataNotInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_not_in_data_table"] = "dataset_metadata_not_in_data_table" + table_name: str + metadata_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"Value for metadata {self.metadata_name} was not found in {self.table_name}." + + +class DatasetMetadataInRangeParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_in_range"] = "dataset_metadata_in_range" + metadata_name: str + min: Optional[Union[float, int]] = None + max: Optional[Union[float, int]] = None + exclude_min: bool = False + exclude_max: bool = False + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + op1 = "<=" + op2 = "<=" + if self.exclude_min: + op1 = "<" + if self.exclude_max: + op2 = "<" + range_description_str = f"({self.min} {op1} value {op2} {self.max})" + return f"Value ('%s') must {'not ' if self.negate else ''}fulfill {range_description_str}" + + +class ValueInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["value_in_data_table"] = "value_in_data_table" + table_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return "Value for metadata not found." + + +class ValueNotInDataTableParameterValidatorModel(ParameterValidatorModel): + type: Literal["value_not_in_data_table"] = "value_not_in_data_table" + table_name: str + metadata_column: Union[int, str] + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + return f"Value was not found in {self.table_name}." + + +class DatasetOkValidatorParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_ok_validator"] = "dataset_ok_validator" + negate: Negate = NEGATE_DEFAULT + + @property + def default_message(self) -> str: + if not self.negate: + message = ( + "The selected dataset is still being generated, select another dataset or wait until it is completed" + ) + else: + message = "The selected dataset must not be in state OK" + return message + + +class DatasetMetadataInFileParameterValidatorModel(ParameterValidatorModel): + type: Literal["dataset_metadata_in_file"] = "dataset_metadata_in_file" + filename: str + metadata_name: str + metadata_column: Union[int, str] + line_startswith: Optional[str] = None + split: str = SPLIT_DEFAULT + negate: Negate = NEGATE_DEFAULT + _deprecated: bool = PrivateAttr(True) + + @property + def default_message(self) -> str: + return f"Value for metadata {self.metadata_name} was not found in {self.filename}." + + +AnyValidatorModel = Annotated[ + Union[ + ExpressionParameterValidatorModel, + RegexParameterValidatorModel, + InRangeParameterValidatorModel, + LengthParameterValidatorModel, + MetadataParameterValidatorModel, + DatasetMetadataEqualParameterValidatorModel, + UnspecifiedBuildParameterValidatorModel, + NoOptionsParameterValidatorModel, + EmptyFieldParameterValidatorModel, + EmptyDatasetParameterValidatorModel, + EmptyExtraFilesPathParameterValidatorModel, + DatasetMetadataInDataTableParameterValidatorModel, + DatasetMetadataNotInDataTableParameterValidatorModel, + DatasetMetadataInRangeParameterValidatorModel, + ValueInDataTableParameterValidatorModel, + ValueNotInDataTableParameterValidatorModel, + DatasetOkValidatorParameterValidatorModel, + DatasetMetadataInFileParameterValidatorModel, + ], + Field(discriminator="type"), +] + + +def parse_xml_validators(input_elem: Element) -> List[AnyValidatorModel]: + validator_els: List[Element] = input_elem.findall("validator") or [] + models = [] + for validator_el in validator_els: + models.append(parse_xml_validator(validator_el)) + return models + + +def static_validators(validator_models: List[AnyValidatorModel]) -> List[AnyValidatorModel]: + static_validators = [] + for validator_model in validator_models: + print(validator_model._static) + if validator_model._static: + static_validators.append(validator_model) + return static_validators + + +def parse_xml_validator(validator_el: Element) -> AnyValidatorModel: + _type = validator_el.get("type") + if _type is None: + raise ValueError("Required 'type' attribute missing from validator") + valid_types = get_args(ValidatorType) + if _type not in valid_types: + raise ValueError(f"Unknown 'type' attribute in validator {_type}") + validator_type: ValidatorType = cast(ValidatorType, _type) + if validator_type == "expression": + return ExpressionParameterValidatorModel( + type="expression", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + expression=validator_el.text, + ) + elif validator_type == "regex": + return RegexParameterValidatorModel( + type="regex", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + expression=validator_el.text, + ) + elif validator_type == "in_range": + return InRangeParameterValidatorModel( + type="in_range", + message=_parse_message(validator_el), + min=_parse_number(validator_el, "min"), + max=_parse_number(validator_el, "max"), + exclude_min=_parse_bool(validator_el, "exclude_min", False), + exclude_max=_parse_bool(validator_el, "exclude_max", False), + negate=_parse_negate(validator_el), + ) + elif validator_type == "length": + return LengthParameterValidatorModel( + type="length", + min=_parse_int(validator_el, "min"), + max=_parse_int(validator_el, "max"), + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "metadata": + return MetadataParameterValidatorModel( + type="metadata", + message=_parse_message(validator_el), + check=_parse_str_list(validator_el, "check"), + skip=_parse_str_list(validator_el, "skip"), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_equal": + return DatasetMetadataEqualParameterValidatorModel( + type="dataset_metadata_equal", + metadata_name=validator_el.get("metadata_name"), + value=_parse_json_value(validator_el), + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "unspecified_build": + return UnspecifiedBuildParameterValidatorModel( + type="unspecified_build", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "no_options": + return NoOptionsParameterValidatorModel( + type="no_options", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "empty_field": + return EmptyFieldParameterValidatorModel( + type="empty_field", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "empty_dataset": + return EmptyDatasetParameterValidatorModel( + type="empty_dataset", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "empty_extra_files_path": + return EmptyExtraFilesPathParameterValidatorModel( + type="empty_extra_files_path", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_in_data_table": + return DatasetMetadataInDataTableParameterValidatorModel( + type="dataset_metadata_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_name=validator_el.get("metadata_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_not_in_data_table": + return DatasetMetadataNotInDataTableParameterValidatorModel( + type="dataset_metadata_not_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_name=validator_el.get("metadata_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_in_range": + return DatasetMetadataInRangeParameterValidatorModel( + type="dataset_metadata_in_range", + message=_parse_message(validator_el), + metadata_name=validator_el.get("metadata_name"), + min=_parse_number(validator_el, "min"), + max=_parse_number(validator_el, "max"), + exclude_min=_parse_bool(validator_el, "exclude_min", False), + exclude_max=_parse_bool(validator_el, "exclude_max", False), + negate=_parse_negate(validator_el), + ) + elif validator_type == "value_in_data_table": + return ValueInDataTableParameterValidatorModel( + type="value_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "value_not_in_data_table": + return ValueNotInDataTableParameterValidatorModel( + type="value_not_in_data_table", + message=_parse_message(validator_el), + table_name=validator_el.get("table_name"), + metadata_column=_parse_metadata_column(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_ok_validator": + return DatasetOkValidatorParameterValidatorModel( + type="dataset_ok_validator", + message=_parse_message(validator_el), + negate=_parse_negate(validator_el), + ) + elif validator_type == "dataset_metadata_in_file": + filename = validator_el.get("filename") + return DatasetMetadataInFileParameterValidatorModel( + type="dataset_metadata_in_file", + message=_parse_message(validator_el), + filename=filename, + metadata_name=validator_el.get("metadata_name"), + metadata_column=_parse_metadata_column(validator_el), + line_startswith=validator_el.get("line_startswith"), + split=validator_el.get("split", SPLIT_DEFAULT), + negate=_parse_negate(validator_el), + ) + else: + raise ValueError(f"Unhandled 'type' attribute in validator {validator_type}") + + +class ValidatorDescription(Protocol): + + @property + def negate(self) -> bool: ... + + @property + def message(self) -> Optional[str]: ... + + +def raise_error_if_valiation_fails( + value: bool, validator: ValidatorDescription, message: Optional[str] = None, value_to_show: Optional[str] = None +): + if not isinstance(value, bool): + raise AssertionError("Validator logic problem - computed validation value must be boolean") + if message is None: + message = validator.message + if message is None: + message = DEFAULT_VALIDATOR_MESSAGE + assert message + if value_to_show and "%s" in message: + message = message % value_to_show + negate = validator.negate + if (not negate and value) or (negate and not value): + return + else: + raise ValueError(message) + + +def _parse_message(xml_el: Element) -> Optional[str]: + message = cast(Optional[str], xml_el.get("message")) + return message + + +def _parse_int(xml_el: Element, attribute: str) -> Optional[int]: + raw_value = xml_el.get(attribute) + if raw_value: + return int(raw_value) + else: + return None + + +def _parse_number(xml_el: Element, attribute: str) -> Optional[Union[float, int]]: + raw_value = xml_el.get(attribute) + if raw_value and ("." in raw_value or "e" in raw_value): + return float(raw_value) + elif raw_value: + return int(raw_value) + else: + return None + + +def _parse_negate(xml_el: Element) -> bool: + return _parse_bool(xml_el, "negate", False) + + +def _parse_bool(xml_el: Element, attribute: str, default_value: bool) -> bool: + return asbool(xml_el.get(attribute, default_value)) + + +def _parse_str_list(xml_el: Element, attribute: str) -> List[str]: + raw_value = xml_el.get(attribute) + if not raw_value: + return [] + else: + return [v.strip() for v in raw_value.split(",")] + + +def _parse_json_value(xml_el: Element) -> Any: + value = xml_el.get("value", None) or json.loads(xml_el.get("value_json", "null")) + return value + + +def _parse_metadata_column(xml_el: Element) -> Union[int, str]: + column = xml_el.get("metadata_column", 0) + try: + return int(column) + except ValueError: + return column + + +def static_tool_validators(validators: Sequence[ParameterValidatorModel]) -> List[StaticValidatorModel]: + static_validators: List[StaticValidatorModel] = [] + for validator in validators: + if isinstance(validator, StaticValidatorModel): + static_validators.append(validator) + return static_validators + + +def statically_validates(validators: Sequence[ParameterValidatorModel], value: Any) -> bool: + for validator in static_tool_validators(validators): + try: + validator.statically_validate(value) + except ValueError: + return False + return True diff --git a/lib/galaxy/tool_util/parser/util.py b/lib/galaxy/tool_util/parser/util.py index 53009057128e..34f3e0943eea 100644 --- a/lib/galaxy/tool_util/parser/util.py +++ b/lib/galaxy/tool_util/parser/util.py @@ -8,6 +8,9 @@ from packaging.version import Version +from galaxy.util import string_as_bool +from .parameter_validators import statically_validates + if TYPE_CHECKING: from .interface import ( InputSource, @@ -83,6 +86,27 @@ def boolean_true_and_false_values(input_source, profile: Optional[Union[float, s return (truevalue, falsevalue) +def text_input_is_optional(input_source: "InputSource") -> Tuple[bool, bool]: + # Optionality not explicitly defined, default to False + optional: Optional[bool] = False + optionality_inferred: bool = False + + optional = input_source.get("optional", None) + if optional is not None: + optional = string_as_bool(optional) + else: + # A text parameter that doesn't raise a validation error on empty string + # is considered to be optional + if statically_validates(input_source.parse_validators(), ""): + optional = True + optionality_inferred = True + else: + optional = False + + assert isinstance(optional, bool) + return optional, optionality_inferred + + class ParameterParseException(Exception): message: str diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index b94cf7a84457..06a25c78f29b 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -71,6 +71,10 @@ ToolOutputCollection, ToolOutputCollectionStructure, ) +from .parameter_validators import ( + AnyValidatorModel, + parse_xml_validators, +) from .stdio import ( aggressive_error_checks, error_on_exit_code, @@ -1339,8 +1343,8 @@ def parse_help(self): def parse_sanitizer_elem(self): return self.input_elem.find("sanitizer") - def parse_validator_elems(self): - return self.input_elem.findall("validator") + def parse_validators(self) -> List[AnyValidatorModel]: + return parse_xml_validators(self.input_elem) def parse_dynamic_options(self) -> Optional[XmlDynamicOptions]: """Return a XmlDynamicOptions to describe dynamic options if options elem is available.""" diff --git a/lib/galaxy/tool_util/unittest_utils/sample_data.py b/lib/galaxy/tool_util/unittest_utils/sample_data.py index d4b6ddb6f027..e9b19283401a 100644 --- a/lib/galaxy/tool_util/unittest_utils/sample_data.py +++ b/lib/galaxy/tool_util/unittest_utils/sample_data.py @@ -17,3 +17,56 @@ """ ) + +VALID_XML_VALIDATORS = [ + """""", + """""", + """""", + """value == 7""", + """mycoolexpression""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", +] + +INVALID_XML_VALIDATORS = [ + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""", + """""" + """""" + """""", + """""", + """""", + """""", +] diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 5aa07efdd204..4860f97b5d51 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -47,6 +47,7 @@ boolean_is_checked, boolean_true_and_false_values, ParameterParseException, + text_input_is_optional, ) from galaxy.tools.parameters.workflow_utils import workflow_building_modes from galaxy.util import ( @@ -194,9 +195,7 @@ def __init__(self, tool, input_source, context=None): self.sanitizer = ToolParameterSanitizer.from_element(sanitizer_elem) else: self.sanitizer = None - self.validators = [] - for elem in input_source.parse_validator_elems(): - self.validators.append(validation.Validator.from_element(self, elem)) + self.validators = validation.to_validators(tool.app if tool else None, input_source.parse_validators()) @property def visible(self) -> bool: @@ -351,7 +350,6 @@ def parse_name(input_source): class SimpleTextToolParameter(ToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) - self.optionality_inferred = False super().__init__(tool, input_source) optional = input_source.get("optional", None) if optional is not None: @@ -359,18 +357,7 @@ def __init__(self, tool, input_source): else: # Optionality not explicitly defined, default to False optional = False - if self.type == "text": - # A text parameter that doesn't raise a validation error on empty string - # is considered to be optional - try: - for validator in self.validators: - validator.validate("") - optional = True - self.optionality_inferred = True - except ValueError: - pass self.optional = optional - if self.optional: self.value = None else: @@ -405,10 +392,16 @@ class TextToolParameter(SimpleTextToolParameter): def __init__(self, tool, input_source): input_source = ensure_input_source(input_source) super().__init__(tool, input_source) - self.profile = tool.profile + self.profile = tool.profile if tool else None self.datalist = [] for title, value, _ in input_source.parse_static_options(): self.datalist.append({"label": title, "value": value}) + + # why does Integer and Float subclass this :_( + if self.type == "text": + self.optional, self.optionality_inferred = text_input_is_optional(input_source) + else: + self.optionality_inferred = False self.value = input_source.get("value") self.area = input_source.get_bool("area", False) @@ -422,10 +415,10 @@ def validate(self, value, trans=None): return super().validate(value, trans) @property - def wrapper_default() -> Optional[str]: + def wrapper_default(self) -> Optional[str]: """Handle change in default handling pre and post 23.0 profiles.""" profile = self.profile - legacy_behavior = (profile is None or Version(str(profile)) < Version("23.0")) + legacy_behavior = profile is None or Version(str(profile)) < Version("23.0") default_value = None if self.optional and self.optionality_inferred and legacy_behavior: default_value = "" @@ -478,7 +471,7 @@ def __init__(self, tool, input_source): except ValueError: raise ParameterValueError("attribute 'max' must be an integer", self.name, self.max) if self.min is not None or self.max is not None: - self.validators.append(validation.InRangeValidator(None, self.min, self.max)) + self.validators.append(validation.InRangeValidator.simple_range_validator(self.min, self.max)) def from_json(self, value, trans, other_values=None): other_values = other_values or {} @@ -551,7 +544,7 @@ def __init__(self, tool, input_source): except ValueError: raise ParameterValueError("attribute 'max' must be a real number", self.name, self.max) if self.min is not None or self.max is not None: - self.validators.append(validation.InRangeValidator(None, self.min, self.max)) + self.validators.append(validation.InRangeValidator.simple_range_validator(self.min, self.max)) def from_json(self, value, trans, other_values=None): other_values = other_values or {} @@ -2056,7 +2049,7 @@ def __init__(self, tool, input_source, trans=None): self.load_contents = int(input_source.get("load_contents", 0)) # Add metadata validator if not input_source.get_bool("no_validation", False): - self.validators.append(validation.MetadataValidator()) + self.validators.append(validation.MetadataValidator.default_metadata_validator()) self._parse_formats(trans, input_source) tag = input_source.get("tag") self.multiple = input_source.get_bool("multiple", False) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index e354c93eabb5..8d9488128a03 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -621,8 +621,9 @@ def load_from_parameter(from_parameter, transform_lines=None): self.filters.append(Filter.from_element(self, filter_elem)) # Load Validators - for validator in elem.findall("validator"): - self.validators.append(validation.Validator.from_element(self.tool_param, validator)) + validators = validation.parse_xml_validators(self.tool_param.tool.app, elem) + if validators: + self.validators = validators if self.dataset_ref_name: tool_param.data_ref = self.dataset_ref_name diff --git a/lib/galaxy/tools/parameters/validation.py b/lib/galaxy/tools/parameters/validation.py index 6334fd95f8b8..895cd1dfadef 100644 --- a/lib/galaxy/tools/parameters/validation.py +++ b/lib/galaxy/tools/parameters/validation.py @@ -3,16 +3,30 @@ """ import abc -import json import logging -import os.path - -import regex +import os +from typing import ( + Any, + cast, + List, + Optional, + Union, +) from galaxy import ( model, util, ) +from galaxy.tool_util.parser.parameter_validators import ( + AnyValidatorModel, + EmptyFieldParameterValidatorModel, + ExpressionParameterValidatorModel, + InRangeParameterValidatorModel, + MetadataParameterValidatorModel, + parse_xml_validators as parse_xml_validators_models, + raise_error_if_valiation_fails, + RegexParameterValidatorModel, +) log = logging.getLogger(__name__) @@ -24,27 +38,7 @@ class Validator(abc.ABC): requires_dataset_metadata = False - @classmethod - def from_element(cls, param, elem): - """ - Initialize the appropriate Validator class - - example call `validation.Validator.from_element(ToolParameter_object, Validator_object)` - - needs to be implemented in the subclasses and should return the - corresponding Validator object by a call to `cls( ... )` which calls the - `__init__` method of the corresponding validator - - param cls the Validator class - param param the element to be evaluated (which contains the validator) - param elem the validator element - return an object of a Validator subclass that corresponds to the type attribute of the validator element - """ - _type = elem.get("type") - assert _type is not None, "Required 'type' attribute missing from validator" - return validator_types[_type].from_element(param, elem) - - def __init__(self, message, negate=False): + def __init__(self, message: str, negate: bool = False): self.message = message self.negate = util.asbool(negate) super().__init__() @@ -68,15 +62,7 @@ def validate(self, value, trans=None, message=None, value_to_show=None): return None if positive validation, otherwise a ValueError is raised """ - assert isinstance(value, bool), "value must be boolean" - if message is None: - message = self.message - if value_to_show and "%s" in message: - message = message % value_to_show - if (not self.negate and value) or (self.negate and not value): - return - else: - raise ValueError(message) + raise_error_if_valiation_fails(value, self, message=message, value_to_show=value_to_show) class RegexValidator(Validator): @@ -84,24 +70,14 @@ class RegexValidator(Validator): Validator that evaluates a regular expression """ - @classmethod - def from_element(cls, param, elem): - return cls(elem.get("message"), elem.text, elem.get("negate", "false")) - - def __init__(self, message, expression, negate): - if message is None: - message = f"Value '%s' does {'not ' if negate == 'false' else ''}match regular expression '{expression.replace('%', '%%')}'" + def __init__(self, message: str, expression: str, negate: bool): super().__init__(message, negate) # Compile later. RE objects used to not be thread safe. Not sure about # the sre module. self.expression = expression def validate(self, value, trans=None): - if not isinstance(value, list): - value = [value] - for val in value: - match = regex.match(self.expression, val or "") - super().validate(match is not None, value_to_show=val) + RegexParameterValidatorModel.regex_validation(self.expression, value, self) class ExpressionValidator(Validator): @@ -109,24 +85,16 @@ class ExpressionValidator(Validator): Validator that evaluates a python expression using the value """ - @classmethod - def from_element(cls, param, elem): - return cls(elem.get("message"), elem.text, elem.get("negate", "false")) - - def __init__(self, message, expression, negate): - if message is None: - message = f"Value '%s' does not evaluate to {'True' if negate == 'false' else 'False'} for '{expression}'" + def __init__(self, message: str, expression: str, negate: bool): super().__init__(message, negate) self.expression = expression # Save compiled expression, code objects are thread safe (right?) - self.compiled_expression = compile(expression, "", "eval") + self.compiled_expression = ExpressionParameterValidatorModel.ensure_compiled(expression) def validate(self, value, trans=None): - try: - evalresult = eval(self.compiled_expression, dict(value=value)) - except Exception: - super().validate(False, message=f"Validator '{self.expression}' could not be evaluated on '{value}'") - super().validate(bool(evalresult), value_to_show=value) + ExpressionParameterValidatorModel.expression_validation( + self.expression, value, self, compiled_expression=self.compiled_expression + ) class InRangeValidator(ExpressionValidator): @@ -134,18 +102,15 @@ class InRangeValidator(ExpressionValidator): Validator that ensures a number is in a specified range """ - @classmethod - def from_element(cls, param, elem): - return cls( - elem.get("message"), - elem.get("min"), - elem.get("max"), - elem.get("exclude_min", "false"), - elem.get("exclude_max", "false"), - elem.get("negate", "false"), - ) - - def __init__(self, message, range_min, range_max, exclude_min=False, exclude_max=False, negate=False): + def __init__( + self, + message: str, + min: Optional[float] = None, + max: Optional[float] = None, + exclude_min: bool = False, + exclude_max: bool = False, + negate: bool = False, + ): """ When the optional exclude_min and exclude_max attributes are set to true, the range excludes the end points (i.e., min < value < max), @@ -153,10 +118,10 @@ def __init__(self, message, range_min, range_max, exclude_min=False, exclude_max (1.e., min <= value <= max). Combinations of exclude_min and exclude_max values are allowed. """ - self.min = range_min if range_min is not None else "-inf" - self.exclude_min = util.asbool(exclude_min) - self.max = range_max if range_max is not None else "inf" - self.exclude_max = util.asbool(exclude_max) + self.min = str(min) if min is not None else "-inf" + self.exclude_min = exclude_min + self.max = str(max) if max is not None else "inf" + self.exclude_max = exclude_max assert float(self.min) <= float(self.max), "min must be less than or equal to max" # Remove unneeded 0s and decimal from floats to make message pretty. op1 = "<=" @@ -166,24 +131,23 @@ def __init__(self, message, range_min, range_max, exclude_min=False, exclude_max if self.exclude_max: op2 = "<" expression = f"float('{self.min}') {op1} float(value) {op2} float('{self.max}')" - if message is None: - message = f"Value ('%s') must {'not ' if negate == 'true' else ''}fulfill {expression}" super().__init__(message, expression, negate) + @staticmethod + def simple_range_validator(min: Optional[float], max: Optional[float]): + return cast( + InRangeParameterValidatorModel, + _to_validator(None, InRangeParameterValidatorModel(min=min, max=max, implicit=True)), + ) + class LengthValidator(InRangeValidator): """ Validator that ensures the length of the provided string (value) is in a specific range """ - @classmethod - def from_element(cls, param, elem): - return cls(elem.get("message"), elem.get("min"), elem.get("max"), elem.get("negate", "false")) - - def __init__(self, message, length_min, length_max, negate): - if message is None: - message = f"Must {'not ' if negate == 'true' else ''}have length of at least {length_min} and at most {length_max}" - super().__init__(message, range_min=length_min, range_max=length_max, negate=negate) + def __init__(self, message: str, min: float, max: float, negate: bool): + super().__init__(message, min=min, max=max, negate=negate) def validate(self, value, trans=None): if value is None: @@ -196,16 +160,8 @@ class DatasetOkValidator(Validator): Validator that checks if a dataset is in an 'ok' state """ - @classmethod - def from_element(cls, param, elem): - negate = elem.get("negate", "false") - message = elem.get("message") - if message is None: - if negate == "false": - message = "The selected dataset is still being generated, select another dataset or wait until it is completed" - else: - message = "The selected dataset must not be in state OK" - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): if value: @@ -217,13 +173,8 @@ class DatasetEmptyValidator(Validator): Validator that checks if a dataset has a positive file size. """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"The selected dataset is {'non-' if negate == 'true' else ''}empty, this tool expects {'non-' if negate == 'false' else ''}empty files." - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): if value: @@ -235,13 +186,8 @@ class DatasetExtraFilesPathEmptyValidator(Validator): Validator that checks if a dataset's extra_files_path exists and is not empty. """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"The selected dataset's extra_files_path directory is {'non-' if negate == 'true' else ''}empty or does {'not ' if negate == 'false' else ''}exist, this tool expects {'non-' if negate == 'false' else ''}empty extra_files_path directories associated with the selected input." - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): if value: @@ -255,25 +201,20 @@ class MetadataValidator(Validator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - return cls( - message=message, check=elem.get("check", ""), skip=elem.get("skip", ""), negate=elem.get("negate", "false") - ) - - def __init__(self, message=None, check="", skip="", negate="false"): - if not message: - if not util.asbool(negate): - message = "Metadata '%s' missing, click the pencil icon in the history item to edit / save the metadata attributes" - else: - if check != "": - message = f"At least one of the checked metadata '{check}' is set, click the pencil icon in the history item to edit / save the metadata attributes" - elif skip != "": - message = f"At least one of the non skipped metadata '{skip}' is set, click the pencil icon in the history item to edit / save the metadata attributes" + def __init__( + self, + message: str, + check: Optional[List[str]] = None, + skip: Optional[List[str]] = None, + negate: bool = False, + ): super().__init__(message, negate) - self.check = check.split(",") if check else None - self.skip = skip.split(",") if skip else None + self.check = check + self.skip = skip + + @staticmethod + def default_metadata_validator() -> "MetadataValidator": + return cast(MetadataValidator, _to_validator(None, MetadataParameterValidatorModel(implicit=True))) def validate(self, value, trans=None): if value: @@ -293,25 +234,10 @@ class MetadataEqualValidator(Validator): requires_dataset_metadata = True def __init__(self, metadata_name=None, value=None, message=None, negate="false"): - if not message: - if not util.asbool(negate): - message = f"Metadata value for '{metadata_name}' must be '{value}', but it is '%s'." - else: - message = f"Metadata value for '{metadata_name}' must not be '{value}' but it is." super().__init__(message, negate) self.metadata_name = metadata_name self.value = value - @classmethod - def from_element(cls, param, elem): - value = elem.get("value", None) or json.loads(elem.get("value_json", "null")) - return cls( - metadata_name=elem.get("metadata_name", None), - value=value, - message=elem.get("message", None), - negate=elem.get("negate", "false"), - ) - def validate(self, value, trans=None): if value: metadata_value = getattr(value.metadata, self.metadata_name) @@ -325,13 +251,8 @@ class UnspecifiedBuildValidator(Validator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"{'Unspecified' if negate == 'false' else 'Specified'} genome build, click the pencil icon in the history item to {'set' if negate == 'false' else 'remove'} the genome build" - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): # if value is None, we cannot validate @@ -348,13 +269,8 @@ class NoOptionsValidator(Validator): Validator that checks for empty select list """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - message = f"{'No options' if negate == 'false' else 'Options'} available for selection" - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): super().validate(value is not None) @@ -365,19 +281,11 @@ class EmptyTextfieldValidator(Validator): Validator that checks for empty text field """ - @classmethod - def from_element(cls, param, elem): - message = elem.get("message") - negate = elem.get("negate", "false") - if not message: - if negate == "false": - message = elem.get("message", "Field requires a value") - else: - message = elem.get("message", "Field must not set a value") - return cls(message, negate) + def __init__(self, message: str, negate: bool = False): + super().__init__(message, negate=negate) def validate(self, value, trans=None): - super().validate(value != "") + EmptyFieldParameterValidatorModel.empty_validate(value, self) class MetadataInFileColumnValidator(Validator): @@ -391,35 +299,19 @@ class MetadataInFileColumnValidator(Validator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - filename = elem.get("filename") - assert filename, f"Required 'filename' attribute missing from {elem.get('type')} validator." - filename = f"{param.tool.app.config.tool_data_path}/{filename.strip()}" - assert os.path.exists(filename), f"File {filename} specified by the 'filename' attribute not found" - metadata_name = elem.get("metadata_name") - assert metadata_name, f"Required 'metadata_name' attribute missing from {elem.get('type')} validator." - metadata_name = metadata_name.strip() - metadata_column = int(elem.get("metadata_column", 0)) - split = elem.get("split", "\t") - message = elem.get("message", f"Value for metadata {metadata_name} was not found in {filename}.") - line_startswith = elem.get("line_startswith") - if line_startswith: - line_startswith = line_startswith.strip() - negate = elem.get("negate", "false") - return cls(filename, metadata_name, metadata_column, message, line_startswith, split, negate) - def __init__( self, - filename, - metadata_name, - metadata_column, - message="Value for metadata not found.", - line_startswith=None, - split="\t", - negate="false", + filename: str, + metadata_name: str, + metadata_column: int, + message: str, + line_startswith: Optional[str] = None, + split: str = "\t", + negate: bool = False, ): super().__init__(message, negate) + assert filename + assert os.path.exists(filename), f"File {filename} specified by the 'filename' attribute not found" self.metadata_name = metadata_name self.valid_values = set() with open(filename) as fh: @@ -445,28 +337,20 @@ class ValueInDataTableColumnValidator(Validator): note: this is covered in a framework test (validation_value_in_datatable) """ - @classmethod - def from_element(cls, param, elem): - table_name = elem.get("table_name") - assert table_name, f"Required 'table_name' attribute missing from {elem.get('type')} validator." - tool_data_table = param.tool.app.tool_data_tables[table_name] - column = elem.get("metadata_column", 0) - try: - column = int(column) - except ValueError: - pass - message = elem.get("message", f"Value was not found in {table_name}.") - negate = elem.get("negate", "false") - return cls(tool_data_table, column, message, negate) - - def __init__(self, tool_data_table, column, message="Value not found.", negate="false"): + def __init__( + self, + tool_data_table, + metadata_column: Union[str, int], + message: str, + negate: bool = False, + ): super().__init__(message, negate) - self.valid_values = [] + self.valid_values: List[Any] = [] self._data_table_content_version = None self._tool_data_table = tool_data_table - if isinstance(column, str): - column = tool_data_table.columns[column] - self._column = column + if isinstance(metadata_column, str): + metadata_column = tool_data_table.columns[metadata_column] + self._column = metadata_column self._load_values() def _load_values(self): @@ -496,7 +380,9 @@ class ValueNotInDataTableColumnValidator(ValueInDataTableColumnValidator): note: this is covered in a framework test (validation_value_in_datatable) """ - def __init__(self, tool_data_table, metadata_column, message="Value already present.", negate="false"): + def __init__( + self, tool_data_table, metadata_column: Union[str, int], message="Value already present.", negate="false" + ): super().__init__(tool_data_table, metadata_column, message, negate) def validate(self, value, trans=None): @@ -517,26 +403,13 @@ class MetadataInDataTableColumnValidator(ValueInDataTableColumnValidator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - table_name = elem.get("table_name") - assert table_name, f"Required 'table_name' attribute missing from {elem.get('type')} validator." - tool_data_table = param.tool.app.tool_data_tables[table_name] - metadata_name = elem.get("metadata_name") - assert metadata_name, f"Required 'metadata_name' attribute missing from {elem.get('type')} validator." - metadata_name = metadata_name.strip() - # TODO rename to column? - metadata_column = elem.get("metadata_column", 0) - try: - metadata_column = int(metadata_column) - except ValueError: - pass - message = elem.get("message", f"Value for metadata {metadata_name} was not found in {table_name}.") - negate = elem.get("negate", "false") - return cls(tool_data_table, metadata_name, metadata_column, message, negate) - def __init__( - self, tool_data_table, metadata_name, metadata_column, message="Value for metadata not found.", negate="false" + self, + tool_data_table, + metadata_name: str, + metadata_column: Union[str, int], + message: str, + negate: bool = False, ): super().__init__(tool_data_table, metadata_column, message, negate) self.metadata_name = metadata_name @@ -558,7 +431,12 @@ class MetadataNotInDataTableColumnValidator(MetadataInDataTableColumnValidator): requires_dataset_metadata = True def __init__( - self, tool_data_table, metadata_name, metadata_column, message="Value for metadata not found.", negate="false" + self, + tool_data_table, + metadata_name: str, + metadata_column: Union[str, int], + message: str, + negate: bool = False, ): super().__init__(tool_data_table, metadata_name, metadata_column, message, negate) @@ -580,26 +458,18 @@ class MetadataInRangeValidator(InRangeValidator): requires_dataset_metadata = True - @classmethod - def from_element(cls, param, elem): - metadata_name = elem.get("metadata_name") - assert metadata_name, f"Required 'metadata_name' attribute missing from {elem.get('type')} validator." - metadata_name = metadata_name.strip() - ret = cls( - metadata_name, - elem.get("message"), - elem.get("min"), - elem.get("max"), - elem.get("exclude_min", "false"), - elem.get("exclude_max", "false"), - elem.get("negate", "false"), - ) - ret.message = "Metadata: " + ret.message - return ret - - def __init__(self, metadata_name, message, range_min, range_max, exclude_min, exclude_max, negate): + def __init__( + self, + metadata_name: str, + message: str, + min: Optional[float] = None, + max: Optional[float] = None, + exclude_min: bool = False, + exclude_max: bool = False, + negate: bool = False, + ): self.metadata_name = metadata_name - super().__init__(message, range_min, range_max, exclude_min, exclude_max, negate) + super().__init__(message, min, max, exclude_min, exclude_max, negate) def validate(self, value, trans=None): if value: @@ -638,3 +508,29 @@ def validate(self, value, trans=None): deprecated_validator_types = dict(dataset_metadata_in_file=MetadataInFileColumnValidator) validator_types.update(deprecated_validator_types) + + +def parse_xml_validators(app, xml_el: util.Element) -> List[Validator]: + return to_validators(app, parse_xml_validators_models(xml_el)) + + +def to_validators(app, validator_models: List[AnyValidatorModel]) -> List[Validator]: + validators = [] + for validator_model in validator_models: + validators.append(_to_validator(app, validator_model)) + return validators + + +def _to_validator(app, validator_model: AnyValidatorModel) -> Validator: + as_dict = validator_model.model_dump() + validator_type = as_dict.pop("type") + del as_dict["implicit"] + if "table_name" in as_dict and app is not None: + table_name = as_dict.pop("table_name") + tool_data_table = app.tool_data_tables[table_name] + as_dict["tool_data_table"] = tool_data_table + if "filename" in as_dict and app is not None: + filename = as_dict.pop("filename") + as_dict["filename"] = f"{app.config.tool_data_path}/{filename}" + + return validator_types[validator_type](**as_dict) diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 34e5b84737e2..6dd47e7f9bb4 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -130,7 +130,7 @@ def __init__( self.input = input if value is None and input.type == "text": # Tools with old profile versions may treat an optional text parameter as `""` - value = cast(TextToolParameter, input).wrapper_default() + value = cast(TextToolParameter, input).wrapper_default self.value = value self._other_values: Dict[str, str] = other_values or {} diff --git a/test/functional/tools/parameters/gx_directory_uri_validation.xml b/test/functional/tools/parameters/gx_directory_uri_validation.xml new file mode 100644 index 000000000000..7375c298b711 --- /dev/null +++ b/test/functional/tools/parameters/gx_directory_uri_validation.xml @@ -0,0 +1,16 @@ + + + macros.xml + + $output + ]]> + + + + 'api' in value + ^.*json$ + + + + diff --git a/test/functional/tools/parameters/gx_float_min_max.xml b/test/functional/tools/parameters/gx_float_min_max.xml new file mode 100644 index 000000000000..55a4d6fb0881 --- /dev/null +++ b/test/functional/tools/parameters/gx_float_min_max.xml @@ -0,0 +1,14 @@ + + > '$output' + ]]> + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_float_validation_range.xml b/test/functional/tools/parameters/gx_float_validation_range.xml new file mode 100644 index 000000000000..8988e92506df --- /dev/null +++ b/test/functional/tools/parameters/gx_float_validation_range.xml @@ -0,0 +1,15 @@ + + > '$output' + ]]> + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_hidden_validation.xml b/test/functional/tools/parameters/gx_hidden_validation.xml new file mode 100644 index 000000000000..e40a67aae964 --- /dev/null +++ b/test/functional/tools/parameters/gx_hidden_validation.xml @@ -0,0 +1,17 @@ + + > '$output' + ]]> + + + + 'api' in value + ^.*json$ + + + + + + + + diff --git a/test/functional/tools/parameters/gx_int_min_max.xml b/test/functional/tools/parameters/gx_int_min_max.xml new file mode 100644 index 000000000000..c826948d746c --- /dev/null +++ b/test/functional/tools/parameters/gx_int_min_max.xml @@ -0,0 +1,14 @@ + + > '$output' + ]]> + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_int_validation_range.xml b/test/functional/tools/parameters/gx_int_validation_range.xml new file mode 100644 index 000000000000..5f7862497abf --- /dev/null +++ b/test/functional/tools/parameters/gx_int_validation_range.xml @@ -0,0 +1,15 @@ + + > '$output' + ]]> + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_expression_validation.xml b/test/functional/tools/parameters/gx_text_expression_validation.xml new file mode 100644 index 000000000000..fe5113ec8fef --- /dev/null +++ b/test/functional/tools/parameters/gx_text_expression_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + 'foobar' in value + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_length_validation.xml b/test/functional/tools/parameters/gx_text_length_validation.xml new file mode 100644 index 000000000000..4d5434aa6c58 --- /dev/null +++ b/test/functional/tools/parameters/gx_text_length_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_length_validation_negate.xml b/test/functional/tools/parameters/gx_text_length_validation_negate.xml new file mode 100644 index 000000000000..4b0b835fe562 --- /dev/null +++ b/test/functional/tools/parameters/gx_text_length_validation_negate.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + + + + + + + + + + diff --git a/test/functional/tools/parameters/gx_text_regex_validation.xml b/test/functional/tools/parameters/gx_text_regex_validation.xml new file mode 100644 index 000000000000..2e1bd7325d8a --- /dev/null +++ b/test/functional/tools/parameters/gx_text_regex_validation.xml @@ -0,0 +1,20 @@ + + > '$output'; +cat '$inputs' >> $inputs_json; + ]]> + + + + + + ^[actg]*$ + + + + + + + + + diff --git a/test/unit/app/tools/test_dynamic_options.py b/test/unit/app/tools/test_dynamic_options.py index 38626d65b991..77d9eb263ecf 100644 --- a/test/unit/app/tools/test_dynamic_options.py +++ b/test/unit/app/tools/test_dynamic_options.py @@ -6,6 +6,10 @@ def get_from_url_option(): + tool_param = Bunch() + tool_param.tool = Bunch() + tool_param.tool.app = Bunch() + return DynamicOptions( XML( """ @@ -26,7 +30,7 @@ def get_from_url_option(): """ ), - Bunch(), + tool_param, ) diff --git a/test/unit/app/tools/test_parameter_validation.py b/test/unit/app/tools/test_parameter_validation.py index a7798927eb65..4b69034fac4b 100644 --- a/test/unit/app/tools/test_parameter_validation.py +++ b/test/unit/app/tools/test_parameter_validation.py @@ -220,12 +220,12 @@ def test_InRangeValidator(self): p.validate(10) with self.assertRaisesRegex( ValueError, - r"Parameter blah: Value \('15'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)", + r"Parameter blah: Value \('15'\) must not fulfill \(10 < value <= 20\)", ): p.validate(15) with self.assertRaisesRegex( ValueError, - r"Parameter blah: Value \('20'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)", + r"Parameter blah: Value \('20'\) must not fulfill \(10 < value <= 20\)", ): p.validate(20) p.validate(21) diff --git a/test/unit/app/tools/test_validation_parsing.py b/test/unit/app/tools/test_validation_parsing.py new file mode 100644 index 000000000000..c13b0973e6a0 --- /dev/null +++ b/test/unit/app/tools/test_validation_parsing.py @@ -0,0 +1,41 @@ +from typing import Optional + +from galaxy.tool_util.unittest_utils.sample_data import ( + INVALID_XML_VALIDATORS, + VALID_XML_VALIDATORS, +) +from galaxy.tools.parameters.validation import parse_xml_validators +from galaxy.util import XML + + +class MockApp: + + @property + def tool_data_tables(self): + return {"mycooltable": MockTable()} + + +class MockTable: + + def get_version_fields(self): + return (1, []) + + +def test_xml_validation_valid(): + for xml_validator in VALID_XML_VALIDATORS: + _validate_xml_str(xml_validator) + + +def test_xml_validation_invalid(): + for xml_validator in INVALID_XML_VALIDATORS: + exc: Optional[Exception] = None + try: + _validate_xml_str(xml_validator) + except ValueError as e: + exc = e + assert exc is not None, f"{xml_validator} - validated when it wasn't expected to" + + +def _validate_xml_str(xml_str: str): + xml_el = XML(f"{xml_str}") + parse_xml_validators(MockApp(), xml_el) diff --git a/test/unit/tool_util/parameter_specification.yml b/test/unit/tool_util/parameter_specification.yml index 29911941e714..f444e7189869 100644 --- a/test/unit/tool_util/parameter_specification.yml +++ b/test/unit/tool_util/parameter_specification.yml @@ -191,15 +191,81 @@ gx_int_required: &gx_int_required gx_int_required_via_empty_string: <<: *gx_int_required +gx_int_validation_range: + request_valid: + - parameter: 1 + - parameter: 9 + request_invalid: + - parameter: {} + - parameter: {__class__: 'ConnectedValue'} + - parameter: -1 + - parameter: 11 + - parameter: 10 + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + +gx_int_min_max: + request_valid: + - parameter: 1 + - parameter: 9 + - parameter: 10 + request_invalid: + - parameter: -1 + - parameter: 11 + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + workflow_step_linked_invalid: + - parameter: -1 + +gx_text_regex_validation: + request_valid: + - parameter: acgt + - parameter: a + - parameter: aaaggttac + request_invalid: + - parameter: acgu + - parameter: nucleic + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + +gx_text_expression_validation: + request_valid: + - parameter: the code was foobar + - parameter: foobar + request_invalid: + - parameter: the code was not foo bar + - parameter: '' + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + workflow_step_linked_invalid: + - parameter: '' + +gx_text_empty_validation: + request_valid: + - parameter: foobar + - {} + request_invalid: + - parameter: '' + job_internal_valid: + - parameter: foobar + job_internal_invalid: + - {} + - parameter: '' + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + workflow_step_linked_invalid: + - parameter: '' + gx_text: request_valid: &gx_text_request_valid - parameter: moocow - parameter: 'some spaces' - parameter: '' - {} + # need to explicitly mark these as non-optional + - parameter: null request_invalid: &gx_text_request_invalid - parameter: 5 - - parameter: null - parameter: {} - parameter: { "moo": "cow" } - parameter: {__class__: 'ConnectedValue'} @@ -223,18 +289,18 @@ gx_text: - parameter: moocow - parameter: 'some spaces' - parameter: '' + - parameter: null job_internal_invalid: - {} - - parameter: null - parameter: { "moo": "cow" } workflow_step_valid: - parameter: moocow - parameter: 'some spaces' - parameter: '' - {} + - parameter: null workflow_step_invalid: - parameter: 5 - - parameter: null - parameter: {} - parameter: { "moo": "cow" } - parameter: {__class__: 'ConnectedValue'} @@ -244,9 +310,9 @@ gx_text: - parameter: '' - {} - parameter: {__class__: 'ConnectedValue'} + - parameter: null workflow_step_linked_invalid: - parameter: 5 - - parameter: null - parameter: {} - parameter: { "moo": "cow" } - parameter: {"class": 'ConnectedValue'} @@ -294,6 +360,21 @@ gx_text_optional: - parameter: {} - parameter: { "moo": "cow" } +gx_text_length_validation: + request_valid: + - parameter: "mytext" + - parameter: "mytext123" + request_invalid: + - parameter: "s" # too short + - parameter: "mytext1231231231sd" # too long + +gx_text_length_validation_negate: + request_valid: + - parameter: "m" + - parameter: "mytext123mocowdowees" + request_invalid: + - parameter: "goldilocks" + gx_select: request_valid: - parameter: "--ex1" @@ -484,6 +565,12 @@ gx_select_multiple_optional: - parameter: {} - parameter: 5 +gx_select_no_options_validation: + job_internal_valid: + - parameter: "--ex1" + job_internal_invalid: + - {} + gx_genomebuild: request_valid: - parameter: hg19 @@ -571,6 +658,22 @@ gx_directory_uri: - parameter: true - parameter: null +gx_directory_uri_validation: + request_valid: + - parameter: "gxfiles://mycoolsource/api/index.json" + request_invalid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "gxfiles://tooshort/index.json" + - parameter: "gxfiles://mycoolsource/api/wrongex.js" + - parameter: "gxfiles://mycoolsource/badexp.json" + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "gxfiles://mycoolsource/api/index.json" + workflow_step_linked_invalid: + - parameter: "gxfiles://tooshort/index.json" + - parameter: "gxfiles://mycoolsource/api/wrongex.js" + - parameter: "gxfiles://mycoolsource/badexp.json" + gx_hidden: request_valid: - parameter: moocow @@ -656,6 +759,22 @@ gx_hidden_optional: workflow_step_linked_invalid: - parameter: 5 +gx_hidden_validation: + request_valid: + - parameter: "http://mycoolservice.com/api/index.json" + request_invalid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "http://mycoolservice.com/index.json" + - parameter: "http://mycoolservice.com/api/wrongex.js" + - parameter: "http://mycoolservice.com/badexp.json" + workflow_step_linked_valid: + - parameter: {__class__: 'ConnectedValue'} + - parameter: "http://mycoolservice.com/api/index.json" + workflow_step_linked_invalid: + - parameter: "http://mycoolservice.com/index.json" + - parameter: "http://mycoolservice.com/api/wrongex.js" + - parameter: "http://mycoolservice.com/badexp.json" + gx_float: request_valid: - parameter: 5 @@ -779,6 +898,26 @@ gx_float_optional: - parameter: 'foobar' - parameter: {__class__: 'ConnectedValue2'} +gx_float_validation_range: + request_valid: + - parameter: 0.1 + - parameter: 9.7 + - parameter: 5 + request_invalid: + - parameter: 10 + - parameter: 0 + - parameter: 9.8 # endpoint not included because specified in validator + +gx_float_min_max: + request_valid: + - parameter: 0.1 + - parameter: 9.7 + - parameter: 5 + - parameter: 9.8 # endpoint implicitly included when max attribute set on param + request_invalid: + - parameter: 10 + - parameter: 0 + gx_color: request_valid: - parameter: '#aabbcc' diff --git a/test/unit/tool_util/test_parameter_validator_models.py b/test/unit/tool_util/test_parameter_validator_models.py new file mode 100644 index 000000000000..17af98f2ce6c --- /dev/null +++ b/test/unit/tool_util/test_parameter_validator_models.py @@ -0,0 +1,28 @@ +from typing import Optional + +from galaxy.tool_util.parser.parameter_validators import parse_xml_validators +from galaxy.tool_util.unittest_utils.sample_data import ( + INVALID_XML_VALIDATORS, + VALID_XML_VALIDATORS, +) +from galaxy.util import XML + + +def test_xml_validation_valid(): + for xml_validator in VALID_XML_VALIDATORS: + _validate_xml_str(xml_validator) + + +def test_xml_validation_invalid(): + for xml_validator in INVALID_XML_VALIDATORS: + exc: Optional[Exception] = None + try: + _validate_xml_str(xml_validator) + except ValueError as e: + exc = e + assert exc is not None, f"{xml_validator} - validated when it wasn't expected to" + + +def _validate_xml_str(xml_str: str): + xml_el = XML(f"{xml_str}") + parse_xml_validators(xml_el) From 078ad6114b0aec3a782b46ce83f934f4b92cff8b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 8 Oct 2024 12:10:36 -0400 Subject: [PATCH 17/42] Fix param models for dynamic options and truevalue/falsevalue --- lib/galaxy/tool_util/parameters/convert.py | 1 - lib/galaxy/tool_util/parameters/factory.py | 7 ++++++- lib/galaxy/tool_util/parser/interface.py | 4 ++++ lib/galaxy/tool_util/parser/xml.py | 18 ++++++++++++++---- lib/galaxy/tools/parameters/basic.py | 2 ++ test/unit/app/tools/test_dynamic_options.py | 8 +++++--- test/unit/tool_util/test_parameter_convert.py | 13 +++++++++++++ 7 files changed, 44 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index 94e4d0c5e569..63dbb9ab58b9 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -312,7 +312,6 @@ def _fill_default_for(tool_state: Dict[str, Any], parameter: ToolParameterT) -> ) test_value = validate_explicit_conditional_test_value(test_parameter_name, explicit_test_value) when = _select_which_when(conditional, test_value, conditional_state) - test_parameter = conditional.test_parameter _fill_default_for(conditional_state, test_parameter) _fill_defaults(conditional_state, when) elif parameter_type in ["gx_repeat"]: diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py index 13d52d5f9c77..932195effb9d 100644 --- a/lib/galaxy/tool_util/parameters/factory.py +++ b/lib/galaxy/tool_util/parameters/factory.py @@ -276,7 +276,12 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool default_test_value = cond_test_parameter_default_value(test_parameter) for value, case_inputs_sources in input_source.parse_when_input_sources(): if isinstance(test_parameter, BooleanParameterModel): - # TODO: investigate truevalue/falsevalue when... + true_value = test_param_input_source.get("truevalue") + false_value = test_param_input_source.get("falsevalue") + if isinstance(value, str) and value == true_value: + value = True + elif isinstance(value, str) and value == false_value: + value = False typed_value = string_as_bool(value) else: typed_value = value diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index b9218ad54270..af72bf4a4825 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -435,6 +435,10 @@ def elem(self) -> Element: # used with other tool sources. raise NotImplementedError(NOT_IMPLEMENTED_MESSAGE) + @abstractmethod + def get_dynamic_options_code(self) -> Optional[str]: + """If dynamic options are a piece of code to eval, return it.""" + @abstractmethod def get_data_table_name(self) -> Optional[str]: """If dynamic options are loaded from a data table, return the name.""" diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 06a25c78f29b..aba5a6874248 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -1300,18 +1300,23 @@ def parse_input_sources(self): class XmlDynamicOptions(DynamicOptions): - def __init__(self, options_elem: Element): + def __init__(self, options_elem: Element, dynamic_option_code: Optional[str]): self._options_elem = options_elem + self._dynamic_options_code = dynamic_option_code def elem(self) -> Element: return self._options_elem + def get_dynamic_options_code(self) -> Optional[str]: + """If dynamic options are a piece of code to eval, return it.""" + return self._dynamic_options_code + def get_data_table_name(self) -> Optional[str]: """If dynamic options are loaded from a data table, return the name.""" - return self._options_elem.get("from_data_table") + return self._options_elem.get("from_data_table") if self._options_elem is not None else None def get_index_file_name(self) -> Optional[str]: - return self._options_elem.get("from_file") + return self._options_elem.get("from_file") if self._options_elem is not None else None class XmlInputSource(InputSource): @@ -1349,7 +1354,12 @@ def parse_validators(self) -> List[AnyValidatorModel]: def parse_dynamic_options(self) -> Optional[XmlDynamicOptions]: """Return a XmlDynamicOptions to describe dynamic options if options elem is available.""" options_elem = self.input_elem.find("options") - return XmlDynamicOptions(options_elem) if options_elem is not None else None + dynamic_option_code = self.input_elem.get("dynamic_options") + is_dynamic = options_elem is not None or dynamic_option_code is not None + if is_dynamic: + return XmlDynamicOptions(options_elem, dynamic_option_code) + else: + return None def parse_static_options(self) -> List[Tuple[str, str, bool]]: """ diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 4860f97b5d51..7ed9c657691a 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -123,6 +123,8 @@ def parse_dynamic_options(param, input_source): if not dynamic_options_config: return None options_elem = dynamic_options_config.elem() + if options_elem is None: + return None return dynamic_options.DynamicOptions(options_elem, param) diff --git a/test/unit/app/tools/test_dynamic_options.py b/test/unit/app/tools/test_dynamic_options.py index 77d9eb263ecf..05ec365a379e 100644 --- a/test/unit/app/tools/test_dynamic_options.py +++ b/test/unit/app/tools/test_dynamic_options.py @@ -6,9 +6,11 @@ def get_from_url_option(): - tool_param = Bunch() - tool_param.tool = Bunch() - tool_param.tool.app = Bunch() + tool_param = Bunch( + tool=Bunch( + app=Bunch(), + ), + ) return DynamicOptions( XML( diff --git a/test/unit/tool_util/test_parameter_convert.py b/test/unit/tool_util/test_parameter_convert.py index a14eab6d5657..7f5deff41e85 100644 --- a/test/unit/tool_util/test_parameter_convert.py +++ b/test/unit/tool_util/test_parameter_convert.py @@ -217,6 +217,19 @@ def test_fill_defaults(): assert with_defaults["cond"]["cond"] == "single" assert with_defaults["cond"]["inner_cond"]["inner_cond"] == "single" + # dynamic parameters should just stay empty - null would cause runtime to skip over population + with_defaults = fill_state_for({}, "parameters/gx_select_dynamic", partial=True) + assert "parameter" not in with_defaults + + # dynamic parameters should just stay empty - null would cause runtime to skip over population + with_defaults = fill_state_for( + {"conditional_parameter": {"test_parameter": False}}, + "parameters/gx_conditional_boolean_discriminate_on_string_value", + ) + assert "conditional_parameter" in with_defaults + assert "boolean_parameter" in with_defaults["conditional_parameter"] + assert with_defaults["conditional_parameter"]["boolean_parameter"] is False + def _fake_dereference(input: DataRequestUri) -> DataRequestInternalHda: return DataRequestInternalHda(id=EXAMPLE_ID_1) From e053eee52af02aac6253d5a4f36fa2748d32cfa2 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 8 Oct 2024 12:33:10 -0400 Subject: [PATCH 18/42] Update tool shed schema for parameter model changes. --- .../webapp/frontend/src/schema/schema.ts | 208 ++++++++++++++++++ 1 file changed, 208 insertions(+) diff --git a/lib/tool_shed/webapp/frontend/src/schema/schema.ts b/lib/tool_shed/webapp/frontend/src/schema/schema.ts index 5667b073d0e6..14aab45a3c36 100644 --- a/lib/tool_shed/webapp/frontend/src/schema/schema.ts +++ b/lib/tool_shed/webapp/frontend/src/schema/schema.ts @@ -1386,6 +1386,16 @@ export interface components { * @enum {string} */ parameter_type: "gx_directory_uri" + /** + * Validators + * @default [] + */ + validators: ( + | components["schemas"]["LengthParameterValidatorModel"] + | components["schemas"]["RegexParameterValidatorModel"] + | components["schemas"]["ExpressionParameterValidatorModel"] + | components["schemas"]["EmptyFieldParameterValidatorModel"] + )[] } /** DrillDownOptionsDict */ DrillDownOptionsDict: { @@ -1440,6 +1450,57 @@ export interface components { */ parameter_type: "gx_drill_down" } + /** EmptyFieldParameterValidatorModel */ + EmptyFieldParameterValidatorModel: { + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default empty_field + * @constant + * @enum {string} + */ + type: "empty_field" + } + /** + * ExpressionParameterValidatorModel + * @description Check if a one line python expression given expression evaluates to True. + * + * The expression is given is the content of the validator tag. + */ + ExpressionParameterValidatorModel: { + /** Expression */ + expression: string + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default expression + * @constant + * @enum {string} + */ + type: "expression" + } /** FailedRepositoryUpdateMessage */ FailedRepositoryUpdateMessage: { /** Err Msg */ @@ -1514,6 +1575,11 @@ export interface components { * @enum {string} */ parameter_type: "gx_float" + /** + * Validators + * @default [] + */ + validators: components["schemas"]["InRangeParameterValidatorModel"][] /** Value */ value?: number | null } @@ -1629,6 +1695,16 @@ export interface components { * @enum {string} */ parameter_type: "gx_hidden" + /** + * Validators + * @default [] + */ + validators: ( + | components["schemas"]["LengthParameterValidatorModel"] + | components["schemas"]["RegexParameterValidatorModel"] + | components["schemas"]["ExpressionParameterValidatorModel"] + | components["schemas"]["EmptyFieldParameterValidatorModel"] + )[] /** Value */ value: string | null } @@ -1666,6 +1742,42 @@ export interface components { * @enum {string} */ ImageType: "Docker" | "Singularity" | "Conda" + /** InRangeParameterValidatorModel */ + InRangeParameterValidatorModel: { + /** + * Exclude Max + * @default false + */ + exclude_max: boolean + /** + * Exclude Min + * @default false + */ + exclude_min: boolean + /** + * Implicit + * @default false + */ + implicit: boolean + /** Max */ + max?: number | null + /** Message */ + message?: string | null + /** Min */ + min?: number | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default in_range + * @constant + * @enum {string} + */ + type: "in_range" + } /** InstallInfo */ InstallInfo: { metadata_info?: components["schemas"]["RepositoryMetadataInstallInfo"] | null @@ -1704,6 +1816,11 @@ export interface components { * @enum {string} */ parameter_type: "gx_integer" + /** + * Validators + * @default [] + */ + validators: components["schemas"]["InRangeParameterValidatorModel"][] /** Value */ value?: number | null } @@ -1716,6 +1833,32 @@ export interface components { /** Value */ value: string } + /** LengthParameterValidatorModel */ + LengthParameterValidatorModel: { + /** + * Implicit + * @default false + */ + implicit: boolean + /** Max */ + max?: number | null + /** Message */ + message?: string | null + /** Min */ + min?: number | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default length + * @constant + * @enum {string} + */ + type: "length" + } /** MessageExceptionModel */ MessageExceptionModel: { /** Err Code */ @@ -1723,6 +1866,28 @@ export interface components { /** Err Msg */ err_msg: string } + /** NoOptionsParameterValidatorModel */ + NoOptionsParameterValidatorModel: { + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default no_options + * @constant + * @enum {string} + */ + type: "no_options" + } /** Organization */ Organization: { /** @@ -1800,6 +1965,37 @@ export interface components { /** Xrefs */ xrefs: components["schemas"]["XrefDict"][] } + /** + * RegexParameterValidatorModel + * @description 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. + */ + RegexParameterValidatorModel: { + /** Expression */ + expression: string + /** + * Implicit + * @default false + */ + implicit: boolean + /** Message */ + message?: string | null + /** + * Negate + * @default false + */ + negate: boolean + /** + * Type + * @default regex + * @constant + * @enum {string} + */ + type: "regex" + } /** RepeatParameterModel */ RepeatParameterModel: { /** Argument */ @@ -2254,6 +2450,8 @@ export interface components { * @enum {string} */ parameter_type: "gx_select" + /** Validators */ + validators: components["schemas"]["NoOptionsParameterValidatorModel"][] } /** Service */ Service: { @@ -2366,6 +2564,16 @@ export interface components { * @enum {string} */ parameter_type: "gx_text" + /** + * Validators + * @default [] + */ + validators: ( + | components["schemas"]["LengthParameterValidatorModel"] + | components["schemas"]["RegexParameterValidatorModel"] + | components["schemas"]["ExpressionParameterValidatorModel"] + | components["schemas"]["EmptyFieldParameterValidatorModel"] + )[] /** Value */ value?: string | null } From 93df8de0e824115ba7c4f66de657db6349062072 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 23 Oct 2024 09:41:32 -0400 Subject: [PATCH 19/42] Tool shed sort repositories by name instead of time. --- .../pages/RepositoriesByCategory.vue | 2 +- lib/tool_shed/webapp/frontend/src/gql/gql.ts | 6 +- .../webapp/frontend/src/gql/graphql.ts | 77 ++++++++++++++++++- 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/lib/tool_shed/webapp/frontend/src/components/pages/RepositoriesByCategory.vue b/lib/tool_shed/webapp/frontend/src/components/pages/RepositoriesByCategory.vue index 868415b47241..899341171af9 100644 --- a/lib/tool_shed/webapp/frontend/src/components/pages/RepositoriesByCategory.vue +++ b/lib/tool_shed/webapp/frontend/src/components/pages/RepositoriesByCategory.vue @@ -30,7 +30,7 @@ const categoryName = computed(() => { const query = graphql(/* GraphQL */ ` query repositoriesByCategory($categoryId: String, $cursor: String) { - relayRepositoriesForCategory(encodedId: $categoryId, sort: UPDATE_TIME_DESC, first: 10, after: $cursor) { + relayRepositoriesForCategory(encodedId: $categoryId, sort: NAME_ASC, first: 10, after: $cursor) { edges { cursor node { diff --git a/lib/tool_shed/webapp/frontend/src/gql/gql.ts b/lib/tool_shed/webapp/frontend/src/gql/gql.ts index 0be807e10175..8faa4df3e4c2 100644 --- a/lib/tool_shed/webapp/frontend/src/gql/gql.ts +++ b/lib/tool_shed/webapp/frontend/src/gql/gql.ts @@ -19,7 +19,7 @@ const documents = { types.RecentRepositoryUpdatesDocument, "\n query repositoriesByOwner($username: String, $cursor: String) {\n relayRepositoriesForOwner(username: $username, sort: UPDATE_TIME_DESC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n": types.RepositoriesByOwnerDocument, - "\n query repositoriesByCategory($categoryId: String, $cursor: String) {\n relayRepositoriesForCategory(encodedId: $categoryId, sort: UPDATE_TIME_DESC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n": + "\n query repositoriesByCategory($categoryId: String, $cursor: String) {\n relayRepositoriesForCategory(encodedId: $categoryId, sort: NAME_ASC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n": types.RepositoriesByCategoryDocument, "\n fragment RepositoryListItemFragment on RelayRepository {\n encodedId\n name\n user {\n username\n }\n description\n type\n updateTime\n homepageUrl\n remoteRepositoryUrl\n }\n": types.RepositoryListItemFragmentFragmentDoc, @@ -65,8 +65,8 @@ export function graphql( * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. */ export function graphql( - source: "\n query repositoriesByCategory($categoryId: String, $cursor: String) {\n relayRepositoriesForCategory(encodedId: $categoryId, sort: UPDATE_TIME_DESC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n" -): (typeof documents)["\n query repositoriesByCategory($categoryId: String, $cursor: String) {\n relayRepositoriesForCategory(encodedId: $categoryId, sort: UPDATE_TIME_DESC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n"] + source: "\n query repositoriesByCategory($categoryId: String, $cursor: String) {\n relayRepositoriesForCategory(encodedId: $categoryId, sort: NAME_ASC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n" +): (typeof documents)["\n query repositoriesByCategory($categoryId: String, $cursor: String) {\n relayRepositoriesForCategory(encodedId: $categoryId, sort: NAME_ASC, first: 10, after: $cursor) {\n edges {\n cursor\n node {\n ...RepositoryListItemFragment\n }\n }\n pageInfo {\n endCursor\n hasNextPage\n }\n }\n }\n"] /** * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. */ diff --git a/lib/tool_shed/webapp/frontend/src/gql/graphql.ts b/lib/tool_shed/webapp/frontend/src/gql/graphql.ts index 0c63d3f99ba0..e7a02dc13d62 100644 --- a/lib/tool_shed/webapp/frontend/src/gql/graphql.ts +++ b/lib/tool_shed/webapp/frontend/src/gql/graphql.ts @@ -20,6 +20,27 @@ export type Scalars = { DateTime: any } +export type BooleanFilter = { + eq?: InputMaybe + in?: InputMaybe>> + nEq?: InputMaybe + notIn?: InputMaybe>> +} + +export type DefaultDateTimeScalarFilter = { + eq?: InputMaybe + in?: InputMaybe>> + nEq?: InputMaybe + notIn?: InputMaybe>> +} + +export type IdFilter = { + eq?: InputMaybe + in?: InputMaybe>> + nEq?: InputMaybe + notIn?: InputMaybe>> +} + /** An object with an ID */ export type Node = { /** The ID of the object */ @@ -61,6 +82,7 @@ export type QueryNodeArgs = { export type QueryRelayCategoriesArgs = { after?: InputMaybe before?: InputMaybe + filter?: InputMaybe first?: InputMaybe last?: InputMaybe sort?: InputMaybe>> @@ -69,6 +91,7 @@ export type QueryRelayCategoriesArgs = { export type QueryRelayRepositoriesArgs = { after?: InputMaybe before?: InputMaybe + filter?: InputMaybe first?: InputMaybe last?: InputMaybe sort?: InputMaybe>> @@ -78,6 +101,7 @@ export type QueryRelayRepositoriesForCategoryArgs = { after?: InputMaybe before?: InputMaybe encodedId?: InputMaybe + filter?: InputMaybe first?: InputMaybe id?: InputMaybe last?: InputMaybe @@ -87,6 +111,7 @@ export type QueryRelayRepositoriesForCategoryArgs = { export type QueryRelayRepositoriesForOwnerArgs = { after?: InputMaybe before?: InputMaybe + filter?: InputMaybe first?: InputMaybe last?: InputMaybe sort?: InputMaybe>> @@ -96,6 +121,7 @@ export type QueryRelayRepositoriesForOwnerArgs = { export type QueryRelayRevisionsArgs = { after?: InputMaybe before?: InputMaybe + filter?: InputMaybe first?: InputMaybe last?: InputMaybe sort?: InputMaybe>> @@ -104,6 +130,7 @@ export type QueryRelayRevisionsArgs = { export type QueryRelayUsersArgs = { after?: InputMaybe before?: InputMaybe + filter?: InputMaybe first?: InputMaybe last?: InputMaybe sort?: InputMaybe>> @@ -138,6 +165,17 @@ export type RelayCategoryEdge = { node?: Maybe } +export type RelayCategoryFilter = { + and?: InputMaybe>> + createTime?: InputMaybe + deleted?: InputMaybe + description?: InputMaybe + id?: InputMaybe + name?: InputMaybe + or?: InputMaybe>> + updateTime?: InputMaybe +} + /** An enumeration. */ export enum RelayCategorySortEnum { CreateTimeAsc = "CREATE_TIME_ASC", @@ -187,6 +225,20 @@ export type RelayRepositoryEdge = { node?: Maybe } +export type RelayRepositoryFilter = { + and?: InputMaybe>> + createTime?: InputMaybe + description?: InputMaybe + homepageUrl?: InputMaybe + id?: InputMaybe + longDescription?: InputMaybe + name?: InputMaybe + or?: InputMaybe>> + remoteRepositoryUrl?: InputMaybe + type?: InputMaybe + updateTime?: InputMaybe +} + export type RelayRepositoryMetadata = Node & { __typename?: "RelayRepositoryMetadata" changesetRevision: Scalars["String"] @@ -217,6 +269,12 @@ export type RelayRepositoryMetadataEdge = { node?: Maybe } +export type RelayRepositoryMetadataFilter = { + and?: InputMaybe>> + id?: InputMaybe + or?: InputMaybe>> +} + /** An enumeration. */ export enum RelayRepositoryMetadataSortEnum { IdAsc = "ID_ASC", @@ -269,6 +327,13 @@ export type RelayUserEdge = { node?: Maybe } +export type RelayUserFilter = { + and?: InputMaybe>> + id?: InputMaybe + or?: InputMaybe>> + username?: InputMaybe +} + /** An enumeration. */ export enum RelayUserSortEnum { IdAsc = "ID_ASC", @@ -327,6 +392,16 @@ export type SimpleUser = { username: Scalars["String"] } +export type StringFilter = { + eq?: InputMaybe + ilike?: InputMaybe + in?: InputMaybe>> + like?: InputMaybe + nEq?: InputMaybe + notIn?: InputMaybe>> + notlike?: InputMaybe +} + export type RecentlyCreatedRepositoriesQueryVariables = Exact<{ [key: string]: never }> export type RecentlyCreatedRepositoriesQuery = { @@ -760,7 +835,7 @@ export const RepositoriesByCategoryDocument = { { kind: "Argument", name: { kind: "Name", value: "sort" }, - value: { kind: "EnumValue", value: "UPDATE_TIME_DESC" }, + value: { kind: "EnumValue", value: "NAME_ASC" }, }, { kind: "Argument", From 7e4e62eac9af358018090b3cebcb4e453a52334c Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Wed, 23 Oct 2024 17:48:14 -0400 Subject: [PATCH 20/42] Unpin social-auth-core in galaxy-data package --- packages/data/setup.cfg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/data/setup.cfg b/packages/data/setup.cfg index 809981ea6394..e10db9e0733c 100644 --- a/packages/data/setup.cfg +++ b/packages/data/setup.cfg @@ -58,7 +58,7 @@ install_requires = python-magic pysam>=0.21 rocrate - social-auth-core[openidconnect]==4.0.3 + social-auth-core[openidconnect]>=4.0.3 SQLAlchemy>=2.0,<2.1 tifffile typing-extensions @@ -75,4 +75,4 @@ console_scripts = [options.packages.find] exclude = - tests* \ No newline at end of file + tests* From fb4e1d0e617c8fe4ae9392ad3fe00d22a62cd4f2 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Thu, 24 Oct 2024 08:36:46 -0400 Subject: [PATCH 21/42] Update packages/data/setup.cfg Co-authored-by: Nicola Soranzo --- packages/data/setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data/setup.cfg b/packages/data/setup.cfg index e10db9e0733c..1087e15bca3d 100644 --- a/packages/data/setup.cfg +++ b/packages/data/setup.cfg @@ -58,7 +58,7 @@ install_requires = python-magic pysam>=0.21 rocrate - social-auth-core[openidconnect]>=4.0.3 + social-auth-core>=4.5.0 SQLAlchemy>=2.0,<2.1 tifffile typing-extensions From 84e66579502ceccd194129906fa48f7472726652 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Wed, 23 Oct 2024 14:56:47 -0400 Subject: [PATCH 22/42] Fix galaxy-manage-db for the installed context --- lib/galaxy/model/orm/scripts.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/galaxy/model/orm/scripts.py b/lib/galaxy/model/orm/scripts.py index e48d639e92f6..77dc660d69f2 100644 --- a/lib/galaxy/model/orm/scripts.py +++ b/lib/galaxy/model/orm/scripts.py @@ -28,6 +28,8 @@ DEFAULT_CONFIG_NAMES = ["galaxy", "universe_wsgi"] DEFAULT_CONFIG_PREFIX = "" DEFAULT_DATABASE = "galaxy" +ALEMBIC_CONFIG = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, "migrations", "alembic.ini")) + DATABASE = { "galaxy": { @@ -163,6 +165,9 @@ def _insert_x_argument(key, value): _insert_x_argument("tsi_url", tsi_config.url) _insert_x_argument("gxy_url", gxy_config.url) + sys.argv.insert(1, "--config") + sys.argv.insert(2, ALEMBIC_CONFIG) + if "heads" in sys.argv and "upgrade" in sys.argv: i = sys.argv.index("heads") sys.argv[i] = f"{GXY}@head" From 8f87afb807aae73d0dec97655f851a546560556c Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Wed, 23 Oct 2024 14:57:15 -0400 Subject: [PATCH 23/42] Don't require galaxy.yml to start with defaults --- lib/galaxy/util/properties.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/util/properties.py b/lib/galaxy/util/properties.py index 7cf3b8710a75..72a53dabd911 100644 --- a/lib/galaxy/util/properties.py +++ b/lib/galaxy/util/properties.py @@ -103,7 +103,7 @@ def load_app_properties( config_section = config_section or ini_section # read from file or init w/no file - if config_file: + if config_file and os.path.exists(config_file): properties = read_properties_from_file(config_file, config_section) else: properties = {"__file__": None} @@ -208,6 +208,8 @@ def get_data_dir(properties): if data_dir is None: if running_from_source: data_dir = "./database" + elif properties["__file__"] is None: + data_dir = "./data" else: config_dir = properties.get("config_dir", os.path.dirname(properties["__file__"])) data_dir = os.path.join(config_dir, "data") From 3021508b978f9b23baeeae1b1bde0041a8f27bd9 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Wed, 23 Oct 2024 14:57:32 -0400 Subject: [PATCH 24/42] Add mercurial as a dep to app package for tool installs --- packages/app/setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/app/setup.cfg b/packages/app/setup.cfg index 285e9b8c7bf0..8864f0c693d7 100644 --- a/packages/app/setup.cfg +++ b/packages/app/setup.cfg @@ -57,6 +57,7 @@ install_requires = Mako Markdown MarkupSafe + mercurial packaging paramiko!=2.9.0,!=2.9.1 pebble From 0a6d7b8f91beadbabb75f4fe1c5f9db92ec4d6de Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Wed, 23 Oct 2024 15:01:32 -0400 Subject: [PATCH 25/42] Add a script for pip installing the packages in dev mode --- packages/package-dev-install.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100755 packages/package-dev-install.sh diff --git a/packages/package-dev-install.sh b/packages/package-dev-install.sh new file mode 100755 index 000000000000..19801a74d1b9 --- /dev/null +++ b/packages/package-dev-install.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# +# Install packages in development mode, for running installed Galaxy from the source +# +set -euo pipefail + +up_to="${1:-}" + +if [ -n "$up_to" -a ! -d "$up_to" ]; then + echo "ERROR: package does not exist: $up_to" + exit 1 +fi + +while read package; do + [ -n "$package" ] || continue + pushd $package + pip install -e . + popd + [ "$package" != "$up_to" ] || exit +done < packages_by_dep_dag.txt From 3b2d81d33b3ecbc856959f38ed0c6d7092f89489 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Wed, 23 Oct 2024 15:50:38 -0400 Subject: [PATCH 26/42] Add simple command line interface to launch uvicorn --- lib/galaxy/webapps/galaxy/script.py | 30 +++++++++++++++++++++++++++++ packages/web_apps/setup.cfg | 4 ++++ 2 files changed, 34 insertions(+) create mode 100644 lib/galaxy/webapps/galaxy/script.py diff --git a/lib/galaxy/webapps/galaxy/script.py b/lib/galaxy/webapps/galaxy/script.py new file mode 100644 index 000000000000..59708d945924 --- /dev/null +++ b/lib/galaxy/webapps/galaxy/script.py @@ -0,0 +1,30 @@ +import os +from argparse import ArgumentParser + +import uvicorn + + +def main() -> None: + parser = ArgumentParser( + prog="galaxy-web", + description="Run Galaxy Web Server", + ) + parser.add_argument("--config", "-c", help="Galaxy config file") + parser.add_argument("--single-user", "-s", help="Single user mode as user SINGLE_USER") + parser.add_argument("--bind", "-b", default="localhost:8080", help="Bind to :") + parser.add_argument( + "--client-path", "-n", default="node_modules/@galaxyproject/galaxy-client", help="Path to Galaxy client" + ) + args = parser.parse_args() + env = os.environ.copy() + if args.config: + env["GALAXY_CONFIG_FILE"] = args.config + if args.single_user: + env["GALAXY_CONFIG_SINGLE_USER"] = args.single_user + env["GALAXY_CONFIG_ADMIN_USERS"] = args.single_user + uvicorn.run( + "galaxy.webapps.galaxy.fast_factory:factory", + factory=True, + host=args.bind.split(":")[0], + port=int(args.bind.split(":")[1]), + ) diff --git a/packages/web_apps/setup.cfg b/packages/web_apps/setup.cfg index 914d43a895d2..bcf88a282c43 100644 --- a/packages/web_apps/setup.cfg +++ b/packages/web_apps/setup.cfg @@ -68,6 +68,10 @@ install_requires = packages = find: python_requires = >=3.8 +[options.entry_points] +console_scripts = + galaxy-web = galaxy.webapps.galaxy.script:main + [options.packages.find] exclude = tests* From 90caa402a5cf42768c8c959ad87cf43d485e0219 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Thu, 24 Oct 2024 10:05:41 -0400 Subject: [PATCH 27/42] Disable conda_auto_init default = true when not running from source --- doc/source/admin/galaxy_options.rst | 3 ++- lib/galaxy/config/__init__.py | 3 +++ lib/galaxy/config/sample/galaxy.yml.sample | 3 ++- lib/galaxy/config/schemas/config_schema.yml | 3 ++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index 45386d72d861..393b54cb92a6 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -509,7 +509,8 @@ :Description: Set to true to instruct Galaxy to install Conda from the web automatically if it cannot find a local copy and conda_exec is not - configured. + configured. The default is true if running Galaxy from source, and + false if running from installed packages. :Default: ``true`` :Type: bool diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index 80d57ab04bf6..812ca96fe6b9 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -982,6 +982,9 @@ def _process_config(self, kwargs: Dict[str, Any]) -> None: # self, populate config_dict self.config_dict["conda_mapping_files"] = [self.local_conda_mapping_file, _default_mapping] + if not running_from_source and kwargs.get("conda_auto_init") is None: + self.config_dict["conda_auto_init"] = False + if self.container_resolvers_config_file: self.container_resolvers_config_file = self._in_config_dir(self.container_resolvers_config_file) diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index d6718cf176e7..dca66e799347 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -589,7 +589,8 @@ galaxy: # Set to true to instruct Galaxy to install Conda from the web # automatically if it cannot find a local copy and conda_exec is not - # configured. + # configured. The default is true if running Galaxy from source, and + # false if running from installed packages. #conda_auto_init: true # You must set this to true if conda_prefix and job_working_directory diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 3ed0c7455e74..06963d364186 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -386,7 +386,8 @@ mapping: required: false desc: | Set to true to instruct Galaxy to install Conda from the web automatically - if it cannot find a local copy and conda_exec is not configured. + if it cannot find a local copy and conda_exec is not configured. The default is + true if running Galaxy from source, and false if running from installed packages. conda_copy_dependencies: type: bool From 3d6579b948a8348a6002bf71762b8b3bafacfa55 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Thu, 24 Oct 2024 10:08:13 -0400 Subject: [PATCH 28/42] [WIP] Add `galaxy-upload` link to `UploadModal` --- client/src/components/Upload/UploadModal.vue | 22 +++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/client/src/components/Upload/UploadModal.vue b/client/src/components/Upload/UploadModal.vue index a78b0ce0adea..258e7f78d868 100644 --- a/client/src/components/Upload/UploadModal.vue +++ b/client/src/components/Upload/UploadModal.vue @@ -1,4 +1,5 @@