mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Converge UDT collection output to flat Shape A
UDT collection outputs nested fields under structure:, but the YAML parser read Shape A at the top level — the parser returned collection_type=None and dropped discover_datasets, making any UDT collection output unusable in connection-validated workflows. Hoist the fields off ToolOutputCollectionStructure onto GenericToolOutputCollection. Lift pre-convergence rows still nested under structure: via lift_legacy_collection_structure, called from the pydantic model_validator(mode="before") and from the YAML parser (which bypasses pydantic via Toolbox.dynamic_tool_to_tool). Parser also accepts collection_type_source as a sibling of type_source.
This commit is contained in:
@@ -16039,6 +16039,19 @@ export interface components {
|
||||
};
|
||||
/** IncomingToolOutputCollection */
|
||||
IncomingToolOutputCollection: {
|
||||
/** Collection Type */
|
||||
collection_type?: string | null;
|
||||
/** Collection Type From Rules */
|
||||
collection_type_from_rules?: string | null;
|
||||
/** Collection Type Source */
|
||||
collection_type_source?: string | null;
|
||||
/** Discover Datasets */
|
||||
discover_datasets?:
|
||||
| (
|
||||
| components["schemas"]["FilePatternDatasetCollectionDescription"]
|
||||
| components["schemas"]["ToolProvidedMetadataDatasetCollection"]
|
||||
)[]
|
||||
| null;
|
||||
/**
|
||||
* Hidden
|
||||
* @description If true, the output will not be shown in the history.
|
||||
@@ -16054,7 +16067,8 @@ export interface components {
|
||||
* @description Parameter name. Used when referencing parameter in workflows.
|
||||
*/
|
||||
name?: string | null;
|
||||
structure: components["schemas"]["ToolOutputCollectionStructure"];
|
||||
/** Structured Like */
|
||||
structured_like?: string | null;
|
||||
/**
|
||||
* @description discriminator enum property added by openapi-typescript
|
||||
* @enum {string}
|
||||
@@ -24188,24 +24202,6 @@ export interface components {
|
||||
*/
|
||||
type: "boolean";
|
||||
};
|
||||
/** ToolOutputCollectionStructure */
|
||||
ToolOutputCollectionStructure: {
|
||||
/** Collection Type */
|
||||
collection_type?: string | null;
|
||||
/** Collection Type From Rules */
|
||||
collection_type_from_rules?: string | null;
|
||||
/** Collection Type Source */
|
||||
collection_type_source?: string | null;
|
||||
/** Discover Datasets */
|
||||
discover_datasets?:
|
||||
| (
|
||||
| components["schemas"]["FilePatternDatasetCollectionDescription"]
|
||||
| components["schemas"]["ToolProvidedMetadataDatasetCollection"]
|
||||
)[]
|
||||
| null;
|
||||
/** Structured Like */
|
||||
structured_like?: string | null;
|
||||
};
|
||||
/** ToolOutputFloat */
|
||||
ToolOutputFloat: {
|
||||
/**
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -15,7 +15,6 @@ from typing_extensions import TypedDict
|
||||
from galaxy.tool_util_models.tool_outputs import (
|
||||
ToolOutputBoolean as ToolOutputBooleanModel,
|
||||
ToolOutputCollection as ToolOutputCollectionModel,
|
||||
ToolOutputCollectionStructure as ToolOutputCollectionStructureModel,
|
||||
ToolOutputDataset as ToolOutputDataModel,
|
||||
ToolOutputFloat as ToolOutputFloatModel,
|
||||
ToolOutputInteger as ToolOutputIntegerModel,
|
||||
@@ -381,12 +380,19 @@ class ToolOutputCollection(ToolOutputBase):
|
||||
return as_dict
|
||||
|
||||
def to_model(self) -> ToolOutputCollectionModel:
|
||||
discover_datasets = []
|
||||
if self.structure.dataset_collector_descriptions:
|
||||
discover_datasets = [d.to_model() for d in self.structure.dataset_collector_descriptions]
|
||||
return ToolOutputCollectionModel(
|
||||
type="collection",
|
||||
name=self.name,
|
||||
label=self.label,
|
||||
hidden=self.hidden,
|
||||
structure=self.structure.to_model(),
|
||||
collection_type=self.structure.collection_type,
|
||||
collection_type_source=self.structure.collection_type_source,
|
||||
collection_type_from_rules=self.structure.collection_type_from_rules,
|
||||
structured_like=self.structure.structured_like,
|
||||
discover_datasets=discover_datasets,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
@@ -472,19 +478,16 @@ class ToolOutputCollectionStructure:
|
||||
return collection_prototype
|
||||
|
||||
def to_dict(self):
|
||||
return self.to_model().model_dump()
|
||||
|
||||
def to_model(self) -> ToolOutputCollectionStructureModel:
|
||||
discover_datasets = []
|
||||
discover_datasets: List[Dict[str, Any]] = []
|
||||
if self.dataset_collector_descriptions:
|
||||
discover_datasets = [d.to_model() for d in self.dataset_collector_descriptions]
|
||||
return ToolOutputCollectionStructureModel(
|
||||
collection_type=self.collection_type,
|
||||
collection_type_source=self.collection_type_source,
|
||||
collection_type_from_rules=self.collection_type_from_rules,
|
||||
structured_like=self.structured_like,
|
||||
discover_datasets=discover_datasets,
|
||||
)
|
||||
discover_datasets = [d.to_model().model_dump() for d in self.dataset_collector_descriptions]
|
||||
return {
|
||||
"collection_type": self.collection_type,
|
||||
"collection_type_source": self.collection_type_source,
|
||||
"collection_type_from_rules": self.collection_type_from_rules,
|
||||
"structured_like": self.structured_like,
|
||||
"discover_datasets": discover_datasets,
|
||||
}
|
||||
|
||||
@staticmethod
|
||||
def from_dict(as_dict) -> "ToolOutputCollectionStructure":
|
||||
|
||||
@@ -37,6 +37,7 @@ from galaxy.tool_util_models.testing_types import (
|
||||
AssertionList,
|
||||
DirectCredential,
|
||||
)
|
||||
from galaxy.tool_util_models.tool_outputs import lift_legacy_collection_structure
|
||||
from galaxy.tool_util_models.tool_source import (
|
||||
HelpContent,
|
||||
JsonTestCollectionDefDict,
|
||||
@@ -236,11 +237,18 @@ class YamlToolSource(ToolSource):
|
||||
return output
|
||||
|
||||
def _parse_output_collection(self, tool, name, output_dict):
|
||||
# YamlToolSource bypasses the pydantic UserToolSource model (see
|
||||
# Toolbox.dynamic_tool_to_tool), so the legacy ``structure:`` wrapper
|
||||
# has to be lifted here too — not just in the model_validator.
|
||||
output_dict = lift_legacy_collection_structure(output_dict)
|
||||
name = output_dict.get("name")
|
||||
label = output_dict.get("label")
|
||||
default_format = output_dict.get("format", "data")
|
||||
collection_type = output_dict.get("collection_type", None)
|
||||
collection_type_source = output_dict.get("type_source", None)
|
||||
# ``type_source`` is the XML attribute name; ``collection_type_source``
|
||||
# is the pydantic field name (Shape A authoring + stored UDT rows).
|
||||
# Accept both so the parser sees the same value regardless of source.
|
||||
collection_type_source = output_dict.get("collection_type_source") or output_dict.get("type_source", None)
|
||||
structured_like = output_dict.get("structured_like", None)
|
||||
inherit_format = False
|
||||
inherit_metadata = False
|
||||
|
||||
@@ -233,9 +233,9 @@ class _DynamicToolSourceBase(ToolSourceBaseModel):
|
||||
"(otherwise its bytes will never be claimed from the working directory)"
|
||||
)
|
||||
elif isinstance(output, IncomingToolOutputCollection):
|
||||
if not output.structure.discover_datasets:
|
||||
if not output.discover_datasets:
|
||||
errors.append(
|
||||
f"output collection '{output.name}' must set 'structure.discover_datasets' "
|
||||
f"output collection '{output.name}' must set 'discover_datasets' "
|
||||
"(otherwise no elements will be claimed from the working directory)"
|
||||
)
|
||||
if errors:
|
||||
|
||||
@@ -6,13 +6,18 @@ code where actual tool objects aren't created.
|
||||
"""
|
||||
|
||||
from typing import (
|
||||
Any,
|
||||
Dict,
|
||||
Generic,
|
||||
List,
|
||||
Optional,
|
||||
Union,
|
||||
)
|
||||
|
||||
from pydantic import Field
|
||||
from pydantic import (
|
||||
Field,
|
||||
model_validator,
|
||||
)
|
||||
from typing_extensions import (
|
||||
Annotated,
|
||||
Literal,
|
||||
@@ -114,12 +119,22 @@ class IncomingToolOutputDataset(
|
||||
format: Annotated[Optional[str], Field(description="The short name for the output datatype.")] = None
|
||||
|
||||
|
||||
class ToolOutputCollectionStructure(ToolSourceBaseModel):
|
||||
collection_type: Optional[str] = None
|
||||
collection_type_source: Optional[str] = None
|
||||
collection_type_from_rules: Optional[str] = None
|
||||
structured_like: Optional[str] = None
|
||||
discover_datasets: Optional[List[DatasetCollectionDescriptionT]] = None
|
||||
def lift_legacy_collection_structure(output_dict: Dict[str, Any]) -> Dict[str, Any]:
|
||||
# Pre-convergence DynamicTool.value rows nest collection fields under
|
||||
# ``structure:``. Inline them so downstream (parser + pydantic model) sees
|
||||
# the same flat Shape A regardless of which form was authored. Top-level
|
||||
# wins, but only when it carries a value — an explicit ``None`` at the
|
||||
# top level mustn't shadow a real legacy value, or a partial-merge writer
|
||||
# could silently drop fields. Returns the input untouched when there's no
|
||||
# ``structure:`` wrapper to lift.
|
||||
structure = output_dict.get("structure")
|
||||
if not isinstance(structure, dict):
|
||||
return output_dict
|
||||
lifted = {k: v for k, v in output_dict.items() if k != "structure"}
|
||||
for key, value in structure.items():
|
||||
if lifted.get(key) is None:
|
||||
lifted[key] = value
|
||||
return lifted
|
||||
|
||||
|
||||
class GenericToolOutputCollection(
|
||||
@@ -127,7 +142,18 @@ class GenericToolOutputCollection(
|
||||
Generic[IncomingNotRequiredBoolT, IncomingNotRequiredStringT],
|
||||
):
|
||||
type: Literal["collection"]
|
||||
structure: ToolOutputCollectionStructure
|
||||
collection_type: Optional[str] = None
|
||||
collection_type_source: Optional[str] = None
|
||||
collection_type_from_rules: Optional[str] = None
|
||||
structured_like: Optional[str] = None
|
||||
discover_datasets: Optional[List[DatasetCollectionDescriptionT]] = None
|
||||
|
||||
@model_validator(mode="before")
|
||||
@classmethod
|
||||
def _lift_legacy_structure(cls, values):
|
||||
if isinstance(values, dict):
|
||||
return lift_legacy_collection_structure(values)
|
||||
return values
|
||||
|
||||
|
||||
class ToolOutputCollection(GenericToolOutputCollection[bool, str]): ...
|
||||
|
||||
@@ -3644,6 +3644,19 @@ export interface components {
|
||||
}
|
||||
/** ToolOutputCollection */
|
||||
ToolOutputCollection: {
|
||||
/** Collection Type */
|
||||
collection_type?: string | null
|
||||
/** Collection Type From Rules */
|
||||
collection_type_from_rules?: string | null
|
||||
/** Collection Type Source */
|
||||
collection_type_source?: string | null
|
||||
/** Discover Datasets */
|
||||
discover_datasets?:
|
||||
| (
|
||||
| components["schemas"]["FilePatternDatasetCollectionDescription"]
|
||||
| components["schemas"]["ToolProvidedMetadataDatasetCollection"]
|
||||
)[]
|
||||
| null
|
||||
/**
|
||||
* Hidden
|
||||
* @description If true, the output will not be shown in the history.
|
||||
@@ -3659,31 +3672,14 @@ export interface components {
|
||||
* @description Parameter name. Used when referencing parameter in workflows.
|
||||
*/
|
||||
name: string
|
||||
structure: components["schemas"]["ToolOutputCollectionStructure"]
|
||||
/** Structured Like */
|
||||
structured_like?: string | null
|
||||
/**
|
||||
* @description discriminator enum property added by openapi-typescript
|
||||
* @enum {string}
|
||||
*/
|
||||
type: "collection"
|
||||
}
|
||||
/** ToolOutputCollectionStructure */
|
||||
ToolOutputCollectionStructure: {
|
||||
/** Collection Type */
|
||||
collection_type?: string | null
|
||||
/** Collection Type From Rules */
|
||||
collection_type_from_rules?: string | null
|
||||
/** Collection Type Source */
|
||||
collection_type_source?: string | null
|
||||
/** Discover Datasets */
|
||||
discover_datasets?:
|
||||
| (
|
||||
| components["schemas"]["FilePatternDatasetCollectionDescription"]
|
||||
| components["schemas"]["ToolProvidedMetadataDatasetCollection"]
|
||||
)[]
|
||||
| null
|
||||
/** Structured Like */
|
||||
structured_like?: string | null
|
||||
}
|
||||
/** ToolOutputDataset */
|
||||
ToolOutputDataset: {
|
||||
/** Discover Datasets */
|
||||
|
||||
@@ -741,8 +741,7 @@ class TestApplyRulesToolLoader(BaseLoaderTestCase):
|
||||
assert not output_model.hidden
|
||||
assert output_model.label == "${input.name} (re-organized)"
|
||||
output_collection_model = assert_output_model_of_type(output_model, ToolOutputCollection)
|
||||
structure = output_collection_model.structure
|
||||
assert structure.collection_type_from_rules == "rules"
|
||||
assert output_collection_model.collection_type_from_rules == "rules"
|
||||
|
||||
|
||||
class TestBuildListToolLoader(BaseLoaderTestCase):
|
||||
@@ -931,6 +930,77 @@ class TestCollectionOutputYaml(FunctionalTestToolTestCase):
|
||||
assert len(output_collections) == 1
|
||||
|
||||
|
||||
def test_yaml_parser_accepts_collection_type_source_alias():
|
||||
# ``collection_type_source`` is the pydantic field name (Shape A authoring +
|
||||
# stored UDT rows). The parser historically only read the XML-style
|
||||
# ``type_source`` attribute; accept both so a lifted legacy row carries
|
||||
# through.
|
||||
from galaxy.tool_util.parser.yaml import YamlToolSource
|
||||
|
||||
doc = {
|
||||
"class": "GalaxyTool",
|
||||
"id": "alias-tool",
|
||||
"name": "Alias tool",
|
||||
"version": "0.1",
|
||||
"shell_command": "touch outs/a.txt",
|
||||
"outputs": [
|
||||
{
|
||||
"name": "outs",
|
||||
"type": "collection",
|
||||
"collection_type_source": "input1",
|
||||
"discover_datasets": [
|
||||
{
|
||||
"discover_via": "pattern",
|
||||
"pattern": "__name_and_ext__",
|
||||
"directory": "outs",
|
||||
}
|
||||
],
|
||||
}
|
||||
],
|
||||
}
|
||||
tool_source = YamlToolSource(doc)
|
||||
_outputs, output_collections = tool_source.parse_outputs(None)
|
||||
assert output_collections["outs"].structure.collection_type_source == "input1"
|
||||
|
||||
|
||||
def test_yaml_parser_lifts_legacy_structure_wrapper():
|
||||
# Pre-convergence DynamicTool.value rows nest collection fields under
|
||||
# ``structure:``. The YAML parser path bypasses pydantic (see
|
||||
# ``Toolbox.dynamic_tool_to_tool``), so it normalizes the wrapper itself.
|
||||
from galaxy.tool_util.parser.yaml import YamlToolSource
|
||||
|
||||
legacy_doc = {
|
||||
"class": "GalaxyTool",
|
||||
"id": "legacy-collection-tool",
|
||||
"name": "Legacy collection tool",
|
||||
"version": "0.1",
|
||||
"shell_command": "mkdir -p outs && touch outs/a.txt",
|
||||
"outputs": [
|
||||
{
|
||||
"name": "outs",
|
||||
"type": "collection",
|
||||
"structure": {
|
||||
"collection_type": "list",
|
||||
"discover_datasets": [
|
||||
{
|
||||
"discover_via": "pattern",
|
||||
"pattern": "__name_and_ext__",
|
||||
"directory": "outs",
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
],
|
||||
}
|
||||
tool_source = YamlToolSource(legacy_doc)
|
||||
outputs, output_collections = tool_source.parse_outputs(None)
|
||||
assert "outs" in output_collections
|
||||
output = output_collections["outs"]
|
||||
assert output.structure.collection_type == "list"
|
||||
assert output.structure.dataset_collector_descriptions
|
||||
assert output.structure.dataset_collector_descriptions[0].discover_via == "pattern"
|
||||
|
||||
|
||||
class TestEnvironmentVariables(FunctionalTestToolTestCase):
|
||||
test_path = "environment_variables.xml"
|
||||
|
||||
|
||||
@@ -133,6 +133,22 @@ cases:
|
||||
msg_contains: from_work_dir
|
||||
|
||||
- name: collection_output_without_discover
|
||||
# Shape A — fields flat on the output. Surfaces the
|
||||
# ``output_unclaimed`` check when ``discover_datasets`` is missing.
|
||||
doc:
|
||||
outputs:
|
||||
- type: collection
|
||||
name: outs
|
||||
collection_type: list
|
||||
expected_errors:
|
||||
- code: dynamic_tool.output_unclaimed
|
||||
msg_contains: discover_datasets
|
||||
|
||||
- name: collection_output_legacy_structure_lifted_then_unclaimed
|
||||
# Shape B — fields nested under ``structure:``. Pre-convergence
|
||||
# ``DynamicTool.value`` rows look like this. The before-validator inlines
|
||||
# them onto the output, so the same ``output_unclaimed`` check then fires
|
||||
# against Shape A. Same error code, same loc — that's the lift working.
|
||||
doc:
|
||||
outputs:
|
||||
- type: collection
|
||||
|
||||
@@ -135,3 +135,101 @@ def test_lift_does_not_mutate_input():
|
||||
lift_user_tool_source(original)
|
||||
assert original["inputs"][0] == snapshot["inputs"][0]
|
||||
assert original["inputs"][1] == snapshot["inputs"][1]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Output collection convergence to Shape A (issue #22758).
|
||||
#
|
||||
# Stored rows authored before the schema converged nest collection fields
|
||||
# under ``structure:``. A pydantic ``model_validator(mode="before")`` on the
|
||||
# output inlines them so legacy rows still validate against the flat schema.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
LEGACY_COLLECTION_OUTPUT_SHAPE_B: Dict[str, Any] = {
|
||||
"type": "collection",
|
||||
"name": "outs",
|
||||
"label": None,
|
||||
"hidden": False,
|
||||
"structure": {
|
||||
"collection_type": "list",
|
||||
"collection_type_source": None,
|
||||
"collection_type_from_rules": None,
|
||||
"structured_like": None,
|
||||
"discover_datasets": [
|
||||
{
|
||||
"discover_via": "pattern",
|
||||
"pattern": "__name_and_ext__",
|
||||
"directory": "outs",
|
||||
"format": None,
|
||||
"visible": False,
|
||||
"assign_primary_output": False,
|
||||
"recurse": False,
|
||||
"match_relative_path": False,
|
||||
"sort_key": "filename",
|
||||
"sort_comp": "lexical",
|
||||
"sort_reverse": False,
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def _collection_tool_value(output):
|
||||
return {
|
||||
**BASE_TOOL,
|
||||
"shell_command": "touch outs/a.txt",
|
||||
"inputs": [],
|
||||
"outputs": [output],
|
||||
}
|
||||
|
||||
|
||||
def test_legacy_structure_wrapper_lifted_silently():
|
||||
value = _collection_tool_value(LEGACY_COLLECTION_OUTPUT_SHAPE_B)
|
||||
status, parsed, errors = lift_user_tool_source(value)
|
||||
assert status == "ok", errors
|
||||
assert isinstance(parsed, UserToolSource)
|
||||
output = parsed.outputs[0]
|
||||
# Fields surface at the top level after the lift.
|
||||
assert output.collection_type == "list"
|
||||
assert output.discover_datasets and output.discover_datasets[0].pattern == "__name_and_ext__"
|
||||
# Dumped representation is Shape A — no leftover wrapper.
|
||||
dumped = parsed.model_dump(by_alias=True)
|
||||
assert "structure" not in dumped["outputs"][0]
|
||||
assert dumped["outputs"][0]["collection_type"] == "list"
|
||||
|
||||
|
||||
def test_legacy_structure_not_shadowed_by_explicit_top_level_none():
|
||||
# Defensive: if a future writer ever produces a hybrid dict where the
|
||||
# top-level field is explicit ``None`` but the structure carries a real
|
||||
# value, the lift must not silently drop the structure value.
|
||||
hybrid_output = {
|
||||
"type": "collection",
|
||||
"name": "outs",
|
||||
"hidden": False,
|
||||
"collection_type": None,
|
||||
"structure": {
|
||||
"collection_type": "list",
|
||||
"discover_datasets": LEGACY_COLLECTION_OUTPUT_SHAPE_B["structure"]["discover_datasets"],
|
||||
},
|
||||
}
|
||||
value = _collection_tool_value(hybrid_output)
|
||||
status, parsed, errors = lift_user_tool_source(value)
|
||||
assert status == "ok", errors
|
||||
assert isinstance(parsed, UserToolSource)
|
||||
assert parsed.outputs[0].collection_type == "list"
|
||||
|
||||
|
||||
def test_shape_a_collection_output_validates_directly():
|
||||
shape_a_output = {
|
||||
"type": "collection",
|
||||
"name": "outs",
|
||||
"hidden": False,
|
||||
"collection_type": "list",
|
||||
"discover_datasets": LEGACY_COLLECTION_OUTPUT_SHAPE_B["structure"]["discover_datasets"],
|
||||
}
|
||||
value = _collection_tool_value(shape_a_output)
|
||||
status, parsed, errors = lift_user_tool_source(value)
|
||||
assert status == "ok", errors
|
||||
assert isinstance(parsed, UserToolSource)
|
||||
assert parsed.outputs[0].collection_type == "list"
|
||||
|
||||
Reference in New Issue
Block a user