mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-01 15:37:32 +08:00
Tighten RulesModel: typed discriminated-union rules, constrained mappings, regex validation
Replace `rules: list[dict[str, Any]]` with 18 typed Pydantic rule models keyed on a `type` discriminator (1:1 with the runtime rules_dsl rule set). Constrain RulesMapping.type to the known mapping Literals. Validate regex `expression` fields (AfterValidator). Fix add_filter_compare value int->float. RulesMapping/ RulesModel become BaseModel (extra ignored) so model_dump emits DSL-only fields. Give RulesParameterModel a workflow-aware pydantic_template mirroring the other inline params (optional in unlinked workflow_step, allow_connected_value when linked) so gx_rules validates across state representations. Adds gx_rules.xml framework tool + gx_rules parameter_specification.yml cases. Pairs with editor UI-metadata leak fix #22823 and IWC cleanup galaxyproject/iwc#1278. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
John Chilton
@@ -1,6 +1,7 @@
|
||||
# attempt to model requires_value...
|
||||
# conditional can descend...
|
||||
import builtins
|
||||
import re
|
||||
from abc import abstractmethod
|
||||
from collections.abc import (
|
||||
Callable,
|
||||
@@ -1684,13 +1685,159 @@ class DirectoryUriParameterModel(BaseGalaxyToolParameterModelDefinition):
|
||||
return True
|
||||
|
||||
|
||||
class RulesMapping(StrictModel):
|
||||
type: str
|
||||
def _validate_regex_expression(v: str) -> str:
|
||||
try:
|
||||
re.compile(v)
|
||||
except re.error as e:
|
||||
raise ValueError(f"Invalid regular expression: {e}") from e
|
||||
return v
|
||||
|
||||
|
||||
ValidRegex = Annotated[str, AfterValidator(_validate_regex_expression)]
|
||||
|
||||
|
||||
class AddColumnMetadataRule(BaseModel):
|
||||
type: Literal["add_column_metadata"]
|
||||
value: str
|
||||
|
||||
|
||||
class AddColumnGroupTagValueRule(BaseModel):
|
||||
type: Literal["add_column_group_tag_value"]
|
||||
value: str
|
||||
default_value: str | None = None
|
||||
|
||||
|
||||
class AddColumnConcatenateRule(BaseModel):
|
||||
type: Literal["add_column_concatenate"]
|
||||
target_column_0: StrictInt
|
||||
target_column_1: StrictInt
|
||||
|
||||
|
||||
class AddColumnBasenameRule(BaseModel):
|
||||
type: Literal["add_column_basename"]
|
||||
target_column: StrictInt
|
||||
|
||||
|
||||
class AddColumnRegexRule(BaseModel):
|
||||
type: Literal["add_column_regex"]
|
||||
target_column: StrictInt
|
||||
expression: ValidRegex
|
||||
replacement: str | None = None
|
||||
group_count: StrictInt | None = None
|
||||
allow_unmatched: StrictBool | None = None
|
||||
|
||||
|
||||
class AddColumnRownumRule(BaseModel):
|
||||
type: Literal["add_column_rownum"]
|
||||
start: StrictInt
|
||||
|
||||
|
||||
class AddColumnValueRule(BaseModel):
|
||||
type: Literal["add_column_value"]
|
||||
value: str
|
||||
|
||||
|
||||
class AddColumnSubstrRule(BaseModel):
|
||||
type: Literal["add_column_substr"]
|
||||
target_column: StrictInt
|
||||
length: StrictInt
|
||||
substr_type: Literal["keep_prefix", "drop_prefix", "keep_suffix", "drop_suffix"]
|
||||
|
||||
|
||||
class AddColumnFromSampleSheetIndexRule(BaseModel):
|
||||
type: Literal["add_column_from_sample_sheet_index"]
|
||||
value: StrictInt
|
||||
|
||||
|
||||
class RemoveColumnsRule(BaseModel):
|
||||
type: Literal["remove_columns"]
|
||||
target_columns: list[StrictInt]
|
||||
|
||||
|
||||
class AddFilterRegexRule(BaseModel):
|
||||
type: Literal["add_filter_regex"]
|
||||
target_column: StrictInt
|
||||
invert: StrictBool
|
||||
expression: ValidRegex
|
||||
|
||||
|
||||
class AddFilterCountRule(BaseModel):
|
||||
type: Literal["add_filter_count"]
|
||||
count: StrictInt
|
||||
invert: StrictBool
|
||||
which: Literal["first", "last"]
|
||||
|
||||
|
||||
class AddFilterEmptyRule(BaseModel):
|
||||
type: Literal["add_filter_empty"]
|
||||
target_column: StrictInt
|
||||
invert: StrictBool
|
||||
|
||||
|
||||
class AddFilterMatchesRule(BaseModel):
|
||||
type: Literal["add_filter_matches"]
|
||||
target_column: StrictInt
|
||||
invert: StrictBool
|
||||
value: str
|
||||
|
||||
|
||||
class AddFilterCompareRule(BaseModel):
|
||||
type: Literal["add_filter_compare"]
|
||||
target_column: StrictInt
|
||||
value: float
|
||||
compare_type: Literal["less_than", "less_than_equal", "greater_than", "greater_than_equal"]
|
||||
|
||||
|
||||
class SortRule(BaseModel):
|
||||
type: Literal["sort"]
|
||||
target_column: StrictInt
|
||||
numeric: StrictBool
|
||||
|
||||
|
||||
class SwapColumnsRule(BaseModel):
|
||||
type: Literal["swap_columns"]
|
||||
target_column_0: StrictInt
|
||||
target_column_1: StrictInt
|
||||
|
||||
|
||||
class SplitColumnsRule(BaseModel):
|
||||
type: Literal["split_columns"]
|
||||
target_columns_0: list[StrictInt]
|
||||
target_columns_1: list[StrictInt]
|
||||
|
||||
|
||||
RuleDefinition = Annotated[
|
||||
AddColumnMetadataRule
|
||||
| AddColumnGroupTagValueRule
|
||||
| AddColumnConcatenateRule
|
||||
| AddColumnBasenameRule
|
||||
| AddColumnRegexRule
|
||||
| AddColumnRownumRule
|
||||
| AddColumnValueRule
|
||||
| AddColumnSubstrRule
|
||||
| AddColumnFromSampleSheetIndexRule
|
||||
| RemoveColumnsRule
|
||||
| AddFilterRegexRule
|
||||
| AddFilterCountRule
|
||||
| AddFilterEmptyRule
|
||||
| AddFilterMatchesRule
|
||||
| AddFilterCompareRule
|
||||
| SortRule
|
||||
| SwapColumnsRule
|
||||
| SplitColumnsRule,
|
||||
Discriminator("type"),
|
||||
]
|
||||
|
||||
MAPPING_TYPES = Literal["list_identifiers", "paired_identifier", "paired_or_unpaired_identifier"]
|
||||
|
||||
|
||||
class RulesMapping(BaseModel):
|
||||
type: MAPPING_TYPES
|
||||
columns: list[StrictInt]
|
||||
|
||||
|
||||
class RulesModel(StrictModel):
|
||||
rules: list[dict[str, Any]]
|
||||
class RulesModel(BaseModel):
|
||||
rules: list[RuleDefinition]
|
||||
mapping: list[RulesMapping]
|
||||
|
||||
|
||||
@@ -1703,7 +1850,17 @@ class RulesParameterModel(BaseGalaxyToolParameterModelDefinition):
|
||||
return RulesModel
|
||||
|
||||
def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation:
|
||||
return dynamic_model_information_from_py_type(self, self.py_type)
|
||||
py_type = self.py_type
|
||||
requires_value = self.request_requires_value
|
||||
if state_representation == "workflow_step_linked":
|
||||
py_type = allow_connected_value(py_type)
|
||||
elif state_representation == "workflow_step":
|
||||
# allow it to be linked in so force allow optional...
|
||||
py_type = optional(py_type)
|
||||
requires_value = False
|
||||
if state_representation in ("job_internal", "job_runtime"):
|
||||
requires_value = True
|
||||
return dynamic_model_information_from_py_type(self, py_type, requires_value=requires_value)
|
||||
|
||||
@property
|
||||
def request_requires_value(self) -> bool:
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
<tool id="gx_rules" name="gx_rules" version="1.0.0" profile="23.0">
|
||||
<command><![CDATA[
|
||||
echo '$parameter' >> '$output'
|
||||
]]></command>
|
||||
<inputs>
|
||||
<param name="parameter" type="rules" />
|
||||
</inputs>
|
||||
<outputs>
|
||||
<data name="output" format="txt" />
|
||||
</outputs>
|
||||
</tool>
|
||||
@@ -4320,3 +4320,223 @@ gx_conditional_underscore_name:
|
||||
- { __cond_opts__: { __selector__: "a", param_a: "test" } }
|
||||
- { __cond_opts__: { __selector__: "a", param_a: {__class__: ConnectedValue} } }
|
||||
- {}
|
||||
|
||||
gx_rules:
|
||||
request_valid: &gx_rules_request_valid
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_regex
|
||||
target_column: 0
|
||||
expression: '(o)+'
|
||||
mapping:
|
||||
- type: list_identifiers
|
||||
columns: [1]
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_metadata
|
||||
value: identifier0
|
||||
- type: add_filter_regex
|
||||
target_column: 0
|
||||
invert: false
|
||||
expression: '^sample'
|
||||
mapping:
|
||||
- type: list_identifiers
|
||||
columns: [1]
|
||||
- type: paired_identifier
|
||||
columns: [2]
|
||||
- parameter:
|
||||
rules:
|
||||
- type: sort
|
||||
target_column: 0
|
||||
numeric: false
|
||||
mapping: []
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_filter_compare
|
||||
target_column: 0
|
||||
value: 13.5
|
||||
compare_type: less_than
|
||||
mapping: []
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_substr
|
||||
target_column: 0
|
||||
length: 3
|
||||
substr_type: keep_prefix
|
||||
mapping:
|
||||
- type: paired_or_unpaired_identifier
|
||||
columns: [0]
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_value
|
||||
value: "moo"
|
||||
- type: remove_columns
|
||||
target_columns: [0, 1]
|
||||
- type: swap_columns
|
||||
target_column_0: 0
|
||||
target_column_1: 1
|
||||
- type: split_columns
|
||||
target_columns_0: [0]
|
||||
target_columns_1: [1]
|
||||
- type: add_column_rownum
|
||||
start: 1
|
||||
- type: add_column_basename
|
||||
target_column: 0
|
||||
- type: add_column_concatenate
|
||||
target_column_0: 0
|
||||
target_column_1: 1
|
||||
- type: add_column_group_tag_value
|
||||
value: where
|
||||
default_value: barn
|
||||
- type: add_filter_count
|
||||
count: 1
|
||||
invert: false
|
||||
which: first
|
||||
- type: add_filter_empty
|
||||
target_column: 0
|
||||
invert: false
|
||||
- type: add_filter_matches
|
||||
target_column: 0
|
||||
invert: false
|
||||
value: "cow"
|
||||
- type: add_column_from_sample_sheet_index
|
||||
value: 0
|
||||
mapping: []
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_regex
|
||||
target_column: 0
|
||||
expression: '(f)(o)'
|
||||
group_count: 2
|
||||
replacement: null
|
||||
allow_unmatched: true
|
||||
mapping: []
|
||||
request_invalid: &gx_rules_request_invalid
|
||||
# missing entirely
|
||||
- {}
|
||||
# null not allowed
|
||||
- parameter: null
|
||||
# not a dict
|
||||
- parameter: "rules"
|
||||
- parameter: 5
|
||||
# missing required fields
|
||||
- parameter: {rules: []}
|
||||
- parameter: {mapping: []}
|
||||
# invalid rule type
|
||||
- parameter:
|
||||
rules:
|
||||
- type: bogus_rule_type
|
||||
mapping: []
|
||||
# rule missing required keys
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_regex
|
||||
target_column: 0
|
||||
mapping: []
|
||||
# invalid regex expression
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_regex
|
||||
target_column: 0
|
||||
expression: '(unclosed'
|
||||
mapping: []
|
||||
# invalid mapping type
|
||||
- parameter:
|
||||
rules:
|
||||
- type: sort
|
||||
target_column: 0
|
||||
numeric: false
|
||||
mapping:
|
||||
- type: bogus_mapping
|
||||
columns: [0]
|
||||
# invalid substr_type enum
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_substr
|
||||
target_column: 0
|
||||
length: 3
|
||||
substr_type: invalid_type
|
||||
mapping: []
|
||||
# invalid compare_type enum
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_filter_compare
|
||||
target_column: 0
|
||||
value: 13
|
||||
compare_type: invalid_compare
|
||||
mapping: []
|
||||
# invalid which enum
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_filter_count
|
||||
count: 1
|
||||
invert: false
|
||||
which: middle
|
||||
mapping: []
|
||||
# string where int expected
|
||||
- parameter:
|
||||
rules:
|
||||
- type: sort
|
||||
target_column: "zero"
|
||||
numeric: false
|
||||
mapping: []
|
||||
# ConnectedValue not allowed in request
|
||||
- parameter: {__class__: 'ConnectedValue'}
|
||||
request_internal_valid:
|
||||
*gx_rules_request_valid
|
||||
request_internal_invalid:
|
||||
*gx_rules_request_invalid
|
||||
request_internal_dereferenced_valid:
|
||||
*gx_rules_request_valid
|
||||
request_internal_dereferenced_invalid:
|
||||
*gx_rules_request_invalid
|
||||
job_internal_valid:
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_metadata
|
||||
value: identifier0
|
||||
mapping:
|
||||
- type: list_identifiers
|
||||
columns: [1]
|
||||
job_internal_invalid:
|
||||
- {}
|
||||
- parameter: null
|
||||
workflow_step_valid:
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_metadata
|
||||
value: identifier0
|
||||
mapping:
|
||||
- type: list_identifiers
|
||||
columns: [1]
|
||||
# rules not required in workflow_step (connected params absent)
|
||||
- {}
|
||||
workflow_step_invalid:
|
||||
- parameter: "not_a_rules_dict"
|
||||
- parameter: 5
|
||||
- parameterx:
|
||||
rules: []
|
||||
mapping: []
|
||||
workflow_step_linked_valid:
|
||||
- parameter:
|
||||
rules:
|
||||
- type: add_column_metadata
|
||||
value: identifier0
|
||||
mapping:
|
||||
- type: list_identifiers
|
||||
columns: [1]
|
||||
- parameter: {__class__: 'ConnectedValue'}
|
||||
workflow_step_linked_invalid:
|
||||
- parameter: null
|
||||
- parameter: "not_a_rules_dict"
|
||||
- parameter: {__class__: 'ConnectedValue2'}
|
||||
_json_schema_skip:
|
||||
# index 8 is the unclosed-regex case: add_column_regex expression validity is an
|
||||
# AfterValidator (regex compilation) not representable in JSON Schema. Every other
|
||||
# invalid entry in these combos is rejected by the generated schema, so skip only [8].
|
||||
request_invalid:
|
||||
8: "add_column_regex expression validator uses AfterValidator"
|
||||
request_internal_invalid:
|
||||
8: "add_column_regex expression validator uses AfterValidator"
|
||||
request_internal_dereferenced_invalid:
|
||||
8: "add_column_regex expression validator uses AfterValidator"
|
||||
|
||||
@@ -8,7 +8,9 @@ Many Galaxy validators now emit native JSON Schema keywords (pattern, minimum/ma
|
||||
minLength/maxLength, exclusiveMinimum/exclusiveMaximum, and negated length via not:{}).
|
||||
Remaining AfterValidator-only constraints (expression, empty_field) that cannot be represented
|
||||
in JSON Schema are annotated with _json_schema_skip in parameter_specification.yml so the test
|
||||
knows to tolerate those *_invalid entries passing validation.
|
||||
knows to tolerate those *_invalid entries passing validation. A skip value is either a string
|
||||
(tolerate every entry in that combo) or a mapping of index -> reason (tolerate only those
|
||||
entries, keeping JSON-Schema-catchable cases in the same combo under test).
|
||||
"""
|
||||
|
||||
import sys
|
||||
@@ -99,10 +101,19 @@ def _test_file_json_schema(
|
||||
parameter_bundle = parameter_bundle_for_file(file)
|
||||
assert parameter_bundle
|
||||
|
||||
json_schema_skip: dict[str, str] = combos.get("_json_schema_skip", {}) or {}
|
||||
json_schema_valid_skip: dict[str, str] = combos.get("_json_schema_valid_skip", {}) or {}
|
||||
skipped_invalid_keys: set[str] = set(json_schema_skip.keys())
|
||||
skipped_valid_keys: set[str] = set(json_schema_valid_skip.keys())
|
||||
# A skip value may be a string (skip every entry in the combo, for combos where all cases
|
||||
# exercise the same AfterValidator-only constraint) or a mapping of index -> reason (skip only
|
||||
# those entries, so JSON-Schema-catchable cases in the same combo remain covered).
|
||||
json_schema_skip: dict[str, Any] = combos.get("_json_schema_skip", {}) or {}
|
||||
json_schema_valid_skip: dict[str, Any] = combos.get("_json_schema_valid_skip", {}) or {}
|
||||
|
||||
def _is_skipped(skip_map: dict[str, Any], key: str, index: int) -> bool:
|
||||
entry = skip_map.get(key)
|
||||
if entry is None:
|
||||
return False
|
||||
if isinstance(entry, dict):
|
||||
return index in entry
|
||||
return True
|
||||
|
||||
failures: list[str] = []
|
||||
|
||||
@@ -130,9 +141,9 @@ def _test_file_json_schema(
|
||||
for i, test_case in enumerate(test_cases):
|
||||
passes = _json_schema_validates(schema, test_case)
|
||||
|
||||
if is_valid and not passes and combo_key not in skipped_valid_keys:
|
||||
if is_valid and not passes and not _is_skipped(json_schema_valid_skip, combo_key, i):
|
||||
failures.append(f"{file}/{combo_key}[{i}]: valid entry REJECTED by JSON Schema: {test_case}")
|
||||
elif not is_valid and passes and combo_key not in skipped_invalid_keys:
|
||||
elif not is_valid and passes and not _is_skipped(json_schema_skip, combo_key, i):
|
||||
failures.append(
|
||||
f"{file}/{combo_key}[{i}]: invalid entry ACCEPTED by JSON Schema (not skipped): {test_case}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user