From d7c04551eaf5b17ab16ad96abb978136b096527e Mon Sep 17 00:00:00 2001 From: Seth Fitzsimmons Date: Thu, 3 Sep 2026 10:48:54 -0700 Subject: [PATCH] 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 --- .../changelog.d/720.bugfix.md | 4 +++ .../overture/schema/validation/__init__.py | 8 ++--- .../tests/test_schema_validation.py | 31 +++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 packages/overture-schema-validation/changelog.d/720.bugfix.md diff --git a/packages/overture-schema-validation/changelog.d/720.bugfix.md b/packages/overture-schema-validation/changelog.d/720.bugfix.md new file mode 100644 index 000000000..00a006331 --- /dev/null +++ b/packages/overture-schema-validation/changelog.d/720.bugfix.md @@ -0,0 +1,4 @@ +`validate()` and `validate_json()` no longer accept `None`. The non-discriminated +union was built with `reduce(or_, models, None)`, whose `None` initializer made the +resulting union admit `None`, so `validate(None)` and `validate_json("null")` +returned successfully instead of raising `ValidationError`. diff --git a/packages/overture-schema-validation/src/overture/schema/validation/__init__.py b/packages/overture-schema-validation/src/overture/schema/validation/__init__.py index 0cea00b0c..1df332016 100644 --- a/packages/overture-schema-validation/src/overture/schema/validation/__init__.py +++ b/packages/overture-schema-validation/src/overture/schema/validation/__init__.py @@ -1,4 +1,3 @@ -from collections.abc import Generator from functools import reduce from operator import or_ from types import UnionType @@ -80,13 +79,14 @@ def _union_type_adapter() -> TypeAdapter: ) discriminated_union: UnionType | None = _discriminated_union(discriminated_models) - non_discriminated_models: Generator[type[BaseModel], None, None] = ( + non_discriminated_models: tuple[type[BaseModel], ...] = tuple( m for m in models.values() if not _can_discriminate(m) ) - non_discriminated_union: UnionType | None = reduce( - or_, non_discriminated_models, None + non_discriminated_union: type[BaseModel] | UnionType | None = ( + reduce(or_, non_discriminated_models) if non_discriminated_models else None ) + model_union: type[BaseModel] | UnionType if discriminated_union and non_discriminated_union: model_union = discriminated_union | non_discriminated_union elif discriminated_union: diff --git a/packages/overture-schema-validation/tests/test_schema_validation.py b/packages/overture-schema-validation/tests/test_schema_validation.py index 9b59083c0..1f3b3823b 100644 --- a/packages/overture-schema-validation/tests/test_schema_validation.py +++ b/packages/overture-schema-validation/tests/test_schema_validation.py @@ -247,3 +247,34 @@ def test_counterexample_validation_flat(counterexample_file: str) -> None: assert not is_valid, ( f"Counterexample should have failed validation (Python): {counterexample_file}" ) + + +def test_validate_none_raises() -> None: + """ + `validate(None)` must raise `ValidationError` rather than succeed. Regression test for the + non-discriminated union's `reduce(or_, non_discriminated_models, None)` call, which seeded the + reduction with `None` and so built a union that admitted `None` as valid. + """ + with pytest.raises(ValidationError): + validate(None) + + +def test_validate_json_null_raises() -> None: + """`validate_json("null")` must raise `ValidationError` rather than succeed.""" + with pytest.raises(ValidationError): + validate_json("null") + + +def test_validate_still_accepts_a_genuine_feature() -> None: + """ + Paired with `test_validate_none_raises` and `test_validate_json_null_raises`: confirms the + union still validates a genuinely valid feature, so a regression that broke the adapter into + rejecting everything could not pass those tests by accident. + """ + example_file = EXAMPLES_DIR / "buildings" / "empire-state-building.json" + json_input = load_example_file(str(example_file)) + + model = validate_json(json.dumps(json_input)) + assert ( + model.model_dump(exclude_unset=True, by_alias=True, mode="json") == json_input + )