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 + )