Skip to content

Commit 5b414ae

Browse files
committed
fix(python): origin-missing-attr emits ERR_INVALID_ORIGIN; ops_for_subtype empty for unknown subtypes; fix own-vs-effective comment (TS parity)
Three TS-parity fixes in validation_passes.py: 1. _validate_origin_paths: emit ERR_INVALID_ORIGIN when a required origin attr is absent (@from on passthrough, @of/@via on aggregate), matching the TS reference which emits this in addition to ERR_MISSING_REQUIRED_ATTR. 2. ops_for_subtype: return frozenset() for unknown/extension field subtypes instead of the full operator union — TS and C# both use a closed allowlist. 3. Comment on the required-attr check: correct the misleading "own or inherited" claim. node.attr() is own-only today; effective/inherited resolution is deferred (no conformance fixture exercises that path yet). New tests: 7 origin-path tests (missing @from/@of/@via emit ERR_INVALID_ORIGIN); 4 ops_for_subtype tests (unknown returns empty, string/boolean/numerics unchanged).
1 parent 163c431 commit 5b414ae

3 files changed

Lines changed: 162 additions & 9 deletions

File tree

server/python/src/metaobjects/loader/validation_passes.py

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,15 @@ def _validate_attr_schema(
119119

120120
schema_by_name: dict[str, AttrSchema] = {s.name: s for s in schemas}
121121

122-
# --- Check 1: required attrs must be present (effective = own + inherited) ---
122+
# --- Check 1: required attrs must be present ---
123123
for schema in schemas:
124124
if not schema.required:
125125
continue
126-
# node.attr() returns None when the attr is absent (own or inherited).
126+
# node.attr() checks OWN attrs only — there is no effective/inherited
127+
# attr accessor today. Effective required-attr resolution (satisfying
128+
# a requirement via an inherited attr from the super chain) is deferred:
129+
# no conformance fixture currently inherits a required attr, so this
130+
# own-only check is sufficient until that fixture is added.
127131
if node.attr(schema.name) is None:
128132
errors.append(
129133
MetaError(
@@ -234,8 +238,8 @@ def ops_for_subtype(field_subtype: str) -> frozenset[str]:
234238
return _OPS_BOOLEAN
235239
if field_subtype in _NUMERIC_TEMPORAL_SUBTYPES:
236240
return _OPS_NUMERIC_TEMPORAL
237-
# Unknown/extension subtypes: allow the full union (open policy).
238-
return _OPS_STRING | _OPS_NUMERIC_TEMPORAL
241+
# Unknown/extension subtypes: closed allowlist — no operators permitted.
242+
return frozenset()
239243

240244

241245
# ---------------------------------------------------------------------------
@@ -480,22 +484,43 @@ def _validate_origin_paths(
480484

481485
if origin.sub_type == ORIGIN_SUBTYPE_PASSTHROUGH:
482486
from_ref = origin.attr(ORIGIN_ATTR_FROM)
483-
if isinstance(from_ref, str):
487+
if not isinstance(from_ref, str) or not from_ref:
488+
errors.append(
489+
MetaError(
490+
f"{ctx} is missing required attribute '@{ORIGIN_ATTR_FROM}'",
491+
ErrorCode.ERR_INVALID_ORIGIN,
492+
)
493+
)
494+
else:
484495
_validate_entity_field_ref(
485496
from_ref, ORIGIN_ATTR_FROM, ctx, object_index, errors
486497
)
487498
via = origin.attr(ORIGIN_ATTR_VIA)
488-
if isinstance(via, str):
499+
if isinstance(via, str) and via:
489500
_validate_via_path(via, ctx, object_index, errors)
490501

491502
elif origin.sub_type == ORIGIN_SUBTYPE_AGGREGATE:
492503
of_ref = origin.attr(ORIGIN_ATTR_OF)
493-
if isinstance(of_ref, str):
504+
if not isinstance(of_ref, str) or not of_ref:
505+
errors.append(
506+
MetaError(
507+
f"{ctx} is missing required attribute '@{ORIGIN_ATTR_OF}'",
508+
ErrorCode.ERR_INVALID_ORIGIN,
509+
)
510+
)
511+
else:
494512
_validate_entity_field_ref(
495513
of_ref, ORIGIN_ATTR_OF, ctx, object_index, errors
496514
)
497515
via = origin.attr(ORIGIN_ATTR_VIA)
498-
if isinstance(via, str):
516+
if not isinstance(via, str) or not via:
517+
errors.append(
518+
MetaError(
519+
f"{ctx} is missing required attribute '@{ORIGIN_ATTR_VIA}'",
520+
ErrorCode.ERR_INVALID_ORIGIN,
521+
)
522+
)
523+
else:
499524
_validate_via_path(via, ctx, object_index, errors)
500525

501526

server/python/tests/unit/test_validation_filter_values.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
from metaobjects.core_types import core_provider
55
from metaobjects.errors import ErrorCode, MetaError
6-
from metaobjects.loader.validation_passes import run_validations
6+
from metaobjects.loader.validation_passes import ops_for_subtype, run_validations
77
from metaobjects.meta.core.attr.attr_constants import (
88
ATTR_SUBTYPE_BOOLEAN,
99
ATTR_SUBTYPE_FILTER,
@@ -155,3 +155,41 @@ def test_valid_filter_no_error() -> None:
155155

156156
bad = [e for e in errors if e.code == ErrorCode.ERR_BAD_ATTR_FILTER]
157157
assert not bad, f"Unexpected ERR_BAD_ATTR_FILTER: {bad}"
158+
159+
160+
# ---------------------------------------------------------------------------
161+
# Tests: ops_for_subtype closed allowlist (TS/C# parity)
162+
# ---------------------------------------------------------------------------
163+
164+
165+
def test_ops_for_subtype_unknown_returns_empty() -> None:
166+
"""An unknown field subtype must return an empty frozenset (closed allowlist).
167+
168+
TS and C# both return [] for unrecognised subtypes; Python must match.
169+
"""
170+
assert ops_for_subtype("uuid") == frozenset()
171+
assert ops_for_subtype("blob") == frozenset()
172+
assert ops_for_subtype("") == frozenset()
173+
assert ops_for_subtype("custom.extension") == frozenset()
174+
175+
176+
def test_ops_for_subtype_string() -> None:
177+
"""string subtype must return the string operator set."""
178+
ops = ops_for_subtype("string")
179+
assert ops == frozenset({"eq", "ne", "in", "like", "isNull"})
180+
181+
182+
def test_ops_for_subtype_boolean() -> None:
183+
"""boolean subtype must return only eq and isNull."""
184+
ops = ops_for_subtype("boolean")
185+
assert ops == frozenset({"eq", "isNull"})
186+
187+
188+
def test_ops_for_subtype_numeric_subtypes() -> None:
189+
"""Numeric and temporal subtypes must return the full numeric/temporal operator set."""
190+
numeric_temporal = {"int", "short", "byte", "long", "double", "float", "decimal", "date", "time", "timestamp"}
191+
expected = frozenset({"eq", "ne", "gt", "gte", "lt", "lte", "in", "isNull"})
192+
for subtype in numeric_temporal:
193+
assert ops_for_subtype(subtype) == expected, (
194+
f"ops_for_subtype('{subtype}') should be {expected}"
195+
)

server/python/tests/unit/test_validation_origin_paths.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,93 @@ def test_invalid_passthrough_from_unknown_entity() -> None:
204204
assert ErrorCode.ERR_INVALID_ORIGIN in codes, (
205205
f"Expected ERR_INVALID_ORIGIN in {codes}"
206206
)
207+
208+
209+
# ---------------------------------------------------------------------------
210+
# Tests: missing required origin attrs emit ERR_INVALID_ORIGIN (TS parity)
211+
# ---------------------------------------------------------------------------
212+
# The TS reference emits ERR_INVALID_ORIGIN for a *missing* required attr
213+
# IN ADDITION to the attr-schema pass's ERR_MISSING_REQUIRED_ATTR.
214+
215+
216+
def _build_aggregate_tree_missing_attrs() -> MetaData:
217+
"""Build a minimal tree with an origin.aggregate node that has NO @of or @via."""
218+
from metaobjects.meta.core.field.meta_field import MetaField
219+
from metaobjects.meta.core.object.meta_object import MetaObject
220+
221+
root = MetaRoot(TYPE_METADATA, SUBTYPE_ROOT, "")
222+
223+
program = MetaObject(TYPE_OBJECT, "entity", "Program")
224+
prog_id = MetaField(TYPE_FIELD, "long", "id")
225+
program.add_child(prog_id)
226+
root.add_child(program)
227+
228+
summary = MetaObject(TYPE_OBJECT, "entity", "ProgramSummary")
229+
field = MetaField(TYPE_FIELD, "int", "weekCount")
230+
# origin.aggregate with neither @of nor @via set
231+
origin = MetaData(TYPE_ORIGIN, "aggregate", "")
232+
field.add_child(origin)
233+
summary.add_child(field)
234+
root.add_child(summary)
235+
236+
return root
237+
238+
239+
def _build_passthrough_tree_missing_from() -> MetaData:
240+
"""Build a minimal tree with an origin.passthrough node that has NO @from."""
241+
from metaobjects.meta.core.field.meta_field import MetaField
242+
from metaobjects.meta.core.object.meta_object import MetaObject
243+
244+
root = MetaRoot(TYPE_METADATA, SUBTYPE_ROOT, "")
245+
246+
program = MetaObject(TYPE_OBJECT, "entity", "Program")
247+
prog_id = MetaField(TYPE_FIELD, "long", "id")
248+
program.add_child(prog_id)
249+
root.add_child(program)
250+
251+
summary = MetaObject(TYPE_OBJECT, "entity", "ProgramSummary")
252+
field = MetaField(TYPE_FIELD, "string", "displayTitle")
253+
# origin.passthrough with no @from
254+
origin = MetaData(TYPE_ORIGIN, "passthrough", "")
255+
field.add_child(origin)
256+
summary.add_child(field)
257+
root.add_child(summary)
258+
259+
return root
260+
261+
262+
def test_aggregate_missing_of_emits_err_invalid_origin() -> None:
263+
"""An origin.aggregate with no @of must emit ERR_INVALID_ORIGIN.
264+
265+
ERR_MISSING_REQUIRED_ATTR may also be present (from the attr-schema pass);
266+
this test only asserts that ERR_INVALID_ORIGIN is included too (TS parity).
267+
"""
268+
root = _build_aggregate_tree_missing_attrs()
269+
errors, _ = _errors_and_warnings(root)
270+
codes = [e.code for e in errors]
271+
assert ErrorCode.ERR_INVALID_ORIGIN in codes, (
272+
f"Expected ERR_INVALID_ORIGIN for missing @of; got codes: {codes}"
273+
)
274+
275+
276+
def test_aggregate_missing_via_emits_err_invalid_origin() -> None:
277+
"""An origin.aggregate with no @via must emit ERR_INVALID_ORIGIN."""
278+
root = _build_aggregate_tree_missing_attrs()
279+
errors, _ = _errors_and_warnings(root)
280+
codes = [e.code for e in errors]
281+
# Both @of and @via are missing — ERR_INVALID_ORIGIN should appear at least once
282+
invalid_origin_errors = [e for e in errors if e.code == ErrorCode.ERR_INVALID_ORIGIN]
283+
assert len(invalid_origin_errors) >= 2, (
284+
f"Expected at least 2 ERR_INVALID_ORIGIN errors (one for @of, one for @via); "
285+
f"got: {invalid_origin_errors}"
286+
)
287+
288+
289+
def test_passthrough_missing_from_emits_err_invalid_origin() -> None:
290+
"""An origin.passthrough with no @from must emit ERR_INVALID_ORIGIN."""
291+
root = _build_passthrough_tree_missing_from()
292+
errors, _ = _errors_and_warnings(root)
293+
codes = [e.code for e in errors]
294+
assert ErrorCode.ERR_INVALID_ORIGIN in codes, (
295+
f"Expected ERR_INVALID_ORIGIN for missing @from; got codes: {codes}"
296+
)

0 commit comments

Comments
 (0)