Skip to content

Commit d7c0455

Browse files
sethfitzvcschapp
authored andcommitted
fix(validation): reject None in validate() and validate_json()
The non-discriminated union was built with reduce(or_, non_discriminated_models, None) whose None initializer made the resulting union None | Segment | ..., so validate(None) and validate_json("null") returned successfully instead of raising ValidationError. Guard the empty case instead, matching what the CLI's equivalent code in overture-schema-cli already does. Three annotation details follow from that: the generator is materialized into a tuple because a generator's truthiness cannot be tested for emptiness; non_discriminated_union widens to type[BaseModel] | UnionType | None because reduce over a single-element tuple returns the bare model type; and model_union gets an explicit type[BaseModel] | UnionType declaration, because mypy infers a branch variable from its first assignment and so read the narrower UnionType off the both-present branch. The non-discriminated bucket is non-empty in practice: Segment is an Annotated discriminated union rather than a class, so it fails _can_discriminate's isinstance(model_class, type) gate. Tests assert both directions -- the two null forms raise, and a genuine feature still round-trips -- so a regression that broke the adapter into rejecting everything could not pass the negative tests by accident. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
1 parent 75278d8 commit d7c0455

3 files changed

Lines changed: 39 additions & 4 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
`validate()` and `validate_json()` no longer accept `None`. The non-discriminated
2+
union was built with `reduce(or_, models, None)`, whose `None` initializer made the
3+
resulting union admit `None`, so `validate(None)` and `validate_json("null")`
4+
returned successfully instead of raising `ValidationError`.

packages/overture-schema-validation/src/overture/schema/validation/__init__.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
from collections.abc import Generator
21
from functools import reduce
32
from operator import or_
43
from types import UnionType
@@ -80,13 +79,14 @@ def _union_type_adapter() -> TypeAdapter:
8079
)
8180
discriminated_union: UnionType | None = _discriminated_union(discriminated_models)
8281

83-
non_discriminated_models: Generator[type[BaseModel], None, None] = (
82+
non_discriminated_models: tuple[type[BaseModel], ...] = tuple(
8483
m for m in models.values() if not _can_discriminate(m)
8584
)
86-
non_discriminated_union: UnionType | None = reduce(
87-
or_, non_discriminated_models, None
85+
non_discriminated_union: type[BaseModel] | UnionType | None = (
86+
reduce(or_, non_discriminated_models) if non_discriminated_models else None
8887
)
8988

89+
model_union: type[BaseModel] | UnionType
9090
if discriminated_union and non_discriminated_union:
9191
model_union = discriminated_union | non_discriminated_union
9292
elif discriminated_union:

packages/overture-schema-validation/tests/test_schema_validation.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,3 +247,34 @@ def test_counterexample_validation_flat(counterexample_file: str) -> None:
247247
assert not is_valid, (
248248
f"Counterexample should have failed validation (Python): {counterexample_file}"
249249
)
250+
251+
252+
def test_validate_none_raises() -> None:
253+
"""
254+
`validate(None)` must raise `ValidationError` rather than succeed. Regression test for the
255+
non-discriminated union's `reduce(or_, non_discriminated_models, None)` call, which seeded the
256+
reduction with `None` and so built a union that admitted `None` as valid.
257+
"""
258+
with pytest.raises(ValidationError):
259+
validate(None)
260+
261+
262+
def test_validate_json_null_raises() -> None:
263+
"""`validate_json("null")` must raise `ValidationError` rather than succeed."""
264+
with pytest.raises(ValidationError):
265+
validate_json("null")
266+
267+
268+
def test_validate_still_accepts_a_genuine_feature() -> None:
269+
"""
270+
Paired with `test_validate_none_raises` and `test_validate_json_null_raises`: confirms the
271+
union still validates a genuinely valid feature, so a regression that broke the adapter into
272+
rejecting everything could not pass those tests by accident.
273+
"""
274+
example_file = EXAMPLES_DIR / "buildings" / "empire-state-building.json"
275+
json_input = load_example_file(str(example_file))
276+
277+
model = validate_json(json.dumps(json_input))
278+
assert (
279+
model.model_dump(exclude_unset=True, by_alias=True, mode="json") == json_input
280+
)

0 commit comments

Comments
 (0)