From 9797fb35d9855ccfb023b5286c682cf3b40ec5b5 Mon Sep 17 00:00:00 2001 From: Thomas <34217413+thommann@users.noreply.github.com> Date: Mon, 7 Jul 2025 15:49:58 +0200 Subject: [PATCH 1/6] feat(jambo): Add oneOf parser (#5) * Add support for `oneOf` type parsing with validation and example cases * Improve `oneOf` type parsing: refine validators, add discriminator support, and expand test coverage * Add hashable and non-hashable value support to `ConstTypeParser` with expanded test cases * Refine `field_props` check in `_type_parser` for cleaner default handling * Update `StringTypeParser` to refine `format` handling and enrich `json_schema_extra` * Remove outdated `oneOf` examples from docs, expand test cases and provide refined examples with discriminator support --- docs/source/usage.oneof.rst | 108 ++++++ docs/source/usage.rst | 1 + jambo/parser/__init__.py | 4 +- jambo/parser/_type_parser.py | 23 ++ jambo/parser/const_type_parser.py | 24 +- jambo/parser/oneof_type_parser.py | 69 ++++ jambo/parser/string_type_parser.py | 5 +- tests/parser/test_const_type_parser.py | 61 ++- tests/parser/test_oneof_type_parser.py | 493 +++++++++++++++++++++++++ 9 files changed, 774 insertions(+), 14 deletions(-) create mode 100644 docs/source/usage.oneof.rst create mode 100644 jambo/parser/oneof_type_parser.py create mode 100644 tests/parser/test_oneof_type_parser.py diff --git a/docs/source/usage.oneof.rst b/docs/source/usage.oneof.rst new file mode 100644 index 0000000..a836df4 --- /dev/null +++ b/docs/source/usage.oneof.rst @@ -0,0 +1,108 @@ +OneOf Type +================= + +The OneOf type is used to specify that an object must conform to exactly one of the specified schemas. Unlike AnyOf which allows matching multiple schemas, OneOf enforces that the data matches one and only one of the provided schemas. + + +Examples +----------------- + +1. **Overlapping String Example** - A field that accepts strings with overlapping constraints: + +.. code-block:: python + + from jambo import SchemaConverter + + schema = { + "title": "SimpleExample", + "type": "object", + "properties": { + "value": { + "oneOf": [ + {"type": "string", "maxLength": 6}, + {"type": "string", "minLength": 4} + ] + } + }, + "required": ["value"] + } + + Model = SchemaConverter.build(schema) + + # Valid: Short string (matches first schema only) + obj1 = Model(value="hi") + print(obj1.value) # Output: hi + + # Valid: Long string (matches second schema only) + obj2 = Model(value="very long string") + print(obj2.value) # Output: very long string + + # Invalid: Medium string (matches BOTH schemas - violates oneOf) + try: + obj3 = Model(value="hello") # 5 chars: matches maxLength=6 AND minLength=4 + except ValueError as e: + print("Validation fails as expected:", e) + + +2. **Discriminator Example** - Different shapes with a type field: + +.. code-block:: python + + from jambo import SchemaConverter + + schema = { + "title": "Shape", + "type": "object", + "properties": { + "shape": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "circle"}, + "radius": {"type": "number", "minimum": 0} + }, + "required": ["type", "radius"] + }, + { + "type": "object", + "properties": { + "type": {"const": "rectangle"}, + "width": {"type": "number", "minimum": 0}, + "height": {"type": "number", "minimum": 0} + }, + "required": ["type", "width", "height"] + } + ], + "discriminator": { + "propertyName": "type" + } + } + }, + "required": ["shape"] + } + + Model = SchemaConverter.build(schema) + + # Valid: Circle + circle = Model(shape={"type": "circle", "radius": 5.0}) + print(circle.shape.type) # Output: circle + + # Valid: Rectangle + rectangle = Model(shape={"type": "rectangle", "width": 10, "height": 20}) + print(rectangle.shape.type) # Output: rectangle + + # Invalid: Wrong properties for the type + try: + invalid = Model(shape={"type": "circle", "width": 10}) + except ValueError as e: + print("Validation fails as expected:", e) + + +.. note:: + + OneOf ensures exactly one schema matches. The discriminator helps Pydantic efficiently determine which schema to use based on a specific property value. + +.. warning:: + + If your data could match multiple schemas in a oneOf, validation will fail. Ensure schemas are mutually exclusive. diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 8896842..3bdb2d9 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -45,5 +45,6 @@ For more complex schemas and types see our documentation on usage.reference usage.allof usage.anyof + usage.oneof usage.enum usage.const \ No newline at end of file diff --git a/jambo/parser/__init__.py b/jambo/parser/__init__.py index f3b8b25..44b4424 100644 --- a/jambo/parser/__init__.py +++ b/jambo/parser/__init__.py @@ -9,6 +9,7 @@ from .float_type_parser import FloatTypeParser from .int_type_parser import IntTypeParser from .null_type_parser import NullTypeParser from .object_type_parser import ObjectTypeParser +from .oneof_type_parser import OneOfTypeParser from .ref_type_parser import RefTypeParser from .string_type_parser import StringTypeParser @@ -25,6 +26,7 @@ __all__ = [ "IntTypeParser", "NullTypeParser", "ObjectTypeParser", + "OneOfTypeParser", "StringTypeParser", "RefTypeParser", -] +] \ No newline at end of file diff --git a/jambo/parser/_type_parser.py b/jambo/parser/_type_parser.py index 080965c..6c5cdc9 100644 --- a/jambo/parser/_type_parser.py +++ b/jambo/parser/_type_parser.py @@ -124,3 +124,26 @@ class GenericTypeParser(ABC, Generic[T]): return False return True + + @staticmethod + def _has_meaningful_constraints(field_props): + """ + Check if field properties contain meaningful constraints that require Field wrapping. + + Returns False if: + - field_props is None or empty + - field_props only contains {'default': None} + + Returns True if: + - field_props contains a non-None default value + - field_props contains other constraint properties (min_length, max_length, pattern, etc.) + """ + if not field_props: + return False + + # If only default is set and it's None, no meaningful constraints + if field_props == {"default": None}: + return False + + # If there are multiple properties or non-None default, that's meaningful + return True diff --git a/jambo/parser/const_type_parser.py b/jambo/parser/const_type_parser.py index b5c846f..1e4ce84 100644 --- a/jambo/parser/const_type_parser.py +++ b/jambo/parser/const_type_parser.py @@ -3,7 +3,7 @@ from jambo.types.json_schema_type import JSONSchemaNativeTypes from jambo.types.type_parser_options import TypeParserOptions from pydantic import AfterValidator -from typing_extensions import Annotated, Any, Unpack +from typing_extensions import Annotated, Any, Literal, Unpack class ConstTypeParser(GenericTypeParser): @@ -33,11 +33,19 @@ class ConstTypeParser(GenericTypeParser): return const_type, parsed_properties def _build_const_type(self, const_value): - def _validate_const_value(value: Any) -> Any: - if value != const_value: - raise ValueError( - f"Value must be equal to the constant value: {const_value}" - ) - return value + # Try to use Literal for hashable types (required for discriminated unions) + # Fall back to validator approach for non-hashable types + try: + # Test if the value is hashable (can be used in Literal) + hash(const_value) + return Literal[const_value] + except TypeError: + # Non-hashable type (like list, dict), use validator approach + def _validate_const_value(value: Any) -> Any: + if value != const_value: + raise ValueError( + f"Value must be equal to the constant value: {const_value}" + ) + return value - return Annotated[type(const_value), AfterValidator(_validate_const_value)] + return Annotated[type(const_value), AfterValidator(_validate_const_value)] \ No newline at end of file diff --git a/jambo/parser/oneof_type_parser.py b/jambo/parser/oneof_type_parser.py new file mode 100644 index 0000000..79146b9 --- /dev/null +++ b/jambo/parser/oneof_type_parser.py @@ -0,0 +1,69 @@ +from jambo.parser._type_parser import GenericTypeParser +from jambo.types.type_parser_options import TypeParserOptions + +from pydantic import Field, BeforeValidator, TypeAdapter, ValidationError +from typing_extensions import Annotated, Union, Unpack, Any + + +class OneOfTypeParser(GenericTypeParser): + mapped_type = Union + + json_schema_type = "oneOf" + + def from_properties_impl( + self, name, properties, **kwargs: Unpack[TypeParserOptions] + ): + if "oneOf" not in properties: + raise ValueError(f"Invalid JSON Schema: {properties}") + + if not isinstance(properties["oneOf"], list): + raise ValueError(f"Invalid JSON Schema: {properties['oneOf']}") + + mapped_properties = self.mappings_properties_builder(properties, **kwargs) + + sub_properties = properties["oneOf"] + + sub_types = [ + GenericTypeParser.type_from_properties(name, subProperty, **kwargs) + for subProperty in sub_properties + ] + + if not kwargs.get("required", False): + mapped_properties["default"] = mapped_properties.get("default") + + field_types = [ + Annotated[t, Field(**v)] if self._has_meaningful_constraints(v) else t + for t, v in sub_types + ] + + union_type = Union[(*field_types,)] + + discriminator = properties.get("discriminator") + if discriminator and isinstance(discriminator, dict): + property_name = discriminator.get("propertyName") + if property_name: + validated_type = Annotated[union_type, Field(discriminator=property_name)] + return validated_type, mapped_properties + + def validate_one_of(value: Any) -> Any: + matched_count = 0 + validation_errors = [] + + for field_type in field_types: + try: + adapter = TypeAdapter(field_type) + adapter.validate_python(value) + matched_count += 1 + except ValidationError as e: + validation_errors.append(str(e)) + continue + + if matched_count == 0: + raise ValueError(f"Value does not match any of the oneOf schemas") + elif matched_count > 1: + raise ValueError(f"Value matches multiple oneOf schemas, exactly one expected") + + return value + + validated_type = Annotated[union_type, BeforeValidator(validate_one_of)] + return validated_type, mapped_properties diff --git a/jambo/parser/string_type_parser.py b/jambo/parser/string_type_parser.py index 389f8f4..9f43034 100644 --- a/jambo/parser/string_type_parser.py +++ b/jambo/parser/string_type_parser.py @@ -16,7 +16,6 @@ class StringTypeParser(GenericTypeParser): "maxLength": "max_length", "minLength": "min_length", "pattern": "pattern", - "format": "format", } format_type_mapping = { @@ -53,4 +52,8 @@ class StringTypeParser(GenericTypeParser): if format_type in self.format_pattern_mapping: mapped_properties["pattern"] = self.format_pattern_mapping[format_type] + if "json_schema_extra" not in mapped_properties: + mapped_properties["json_schema_extra"] = {} + mapped_properties["json_schema_extra"]["format"] = format_type + return mapped_type, mapped_properties diff --git a/tests/parser/test_const_type_parser.py b/tests/parser/test_const_type_parser.py index ca92bb0..5a8c9c1 100644 --- a/tests/parser/test_const_type_parser.py +++ b/tests/parser/test_const_type_parser.py @@ -1,12 +1,13 @@ from jambo.parser import ConstTypeParser -from typing_extensions import Annotated, get_args, get_origin +from typing_extensions import Annotated, Literal, get_args, get_origin from unittest import TestCase class TestConstTypeParser(TestCase): - def test_const_type_parser(self): + def test_const_type_parser_hashable_value(self): + """Test const parser with hashable values (uses Literal)""" parser = ConstTypeParser() expected_const_value = "United States of America" @@ -16,8 +17,60 @@ class TestConstTypeParser(TestCase): "country", properties ) + # Check that we get a Literal type for hashable values + self.assertEqual(get_origin(parsed_type), Literal) + self.assertEqual(get_args(parsed_type), (expected_const_value,)) + + self.assertEqual(parsed_properties["default"], expected_const_value) + + def test_const_type_parser_non_hashable_value(self): + """Test const parser with non-hashable values (uses Annotated with validator)""" + parser = ConstTypeParser() + + expected_const_value = [1, 2, 3] # Lists are not hashable + properties = {"const": expected_const_value} + + parsed_type, parsed_properties = parser.from_properties_impl( + "list_const", properties + ) + + # Check that we get an Annotated type for non-hashable values self.assertEqual(get_origin(parsed_type), Annotated) - self.assertIn(str, get_args(parsed_type)) + self.assertIn(list, get_args(parsed_type)) + + self.assertEqual(parsed_properties["default"], expected_const_value) + + def test_const_type_parser_integer_value(self): + """Test const parser with integer values (uses Literal)""" + parser = ConstTypeParser() + + expected_const_value = 42 + properties = {"const": expected_const_value} + + parsed_type, parsed_properties = parser.from_properties_impl( + "int_const", properties + ) + + # Check that we get a Literal type for hashable values + self.assertEqual(get_origin(parsed_type), Literal) + self.assertEqual(get_args(parsed_type), (expected_const_value,)) + + self.assertEqual(parsed_properties["default"], expected_const_value) + + def test_const_type_parser_boolean_value(self): + """Test const parser with boolean values (uses Literal)""" + parser = ConstTypeParser() + + expected_const_value = True + properties = {"const": expected_const_value} + + parsed_type, parsed_properties = parser.from_properties_impl( + "bool_const", properties + ) + + # Check that we get a Literal type for hashable values + self.assertEqual(get_origin(parsed_type), Literal) + self.assertEqual(get_args(parsed_type), (expected_const_value,)) self.assertEqual(parsed_properties["default"], expected_const_value) @@ -46,4 +99,4 @@ class TestConstTypeParser(TestCase): self.assertIn( "Const type invalid_country must have 'const' value of allowed types", str(context.exception), - ) + ) \ No newline at end of file diff --git a/tests/parser/test_oneof_type_parser.py b/tests/parser/test_oneof_type_parser.py new file mode 100644 index 0000000..8c75f04 --- /dev/null +++ b/tests/parser/test_oneof_type_parser.py @@ -0,0 +1,493 @@ +from jambo import SchemaConverter + +from unittest import TestCase + + +class TestOneOfTypeParser(TestCase): + def test_oneof_basic_integer_and_string(self): + schema = { + "title": "Person", + "description": "A person with an ID that can be either an integer or a formatted string", + "type": "object", + "properties": { + "id": { + "oneOf": [ + {"type": "integer", "minimum": 1}, + {"type": "string", "pattern": "^[A-Z]{2}[0-9]{4}$"}, + ] + }, + }, + "required": ["id"], + } + + Model = SchemaConverter.build(schema) + + obj1 = Model(id=123) + self.assertEqual(obj1.id, 123) + + obj2 = Model(id="AB1234") + self.assertEqual(obj2.id, "AB1234") + + def test_oneof_validation_failures(self): + schema = { + "title": "Person", + "type": "object", + "properties": { + "id": { + "oneOf": [ + {"type": "integer", "minimum": 1}, + {"type": "string", "pattern": "^[A-Z]{2}[0-9]{4}$"}, + ] + }, + }, + "required": ["id"], + } + + Model = SchemaConverter.build(schema) + + with self.assertRaises(ValueError): + Model(id=-5) + + with self.assertRaises(ValueError): + Model(id="invalid") + + with self.assertRaises(ValueError): + Model(id=123.45) + + def test_oneof_with_conflicting_schemas(self): + schema = { + "title": "Value", + "type": "object", + "properties": { + "data": { + "oneOf": [ + {"type": "number", "multipleOf": 2}, + {"type": "number", "multipleOf": 3}, + ] + }, + }, + "required": ["data"], + } + + Model = SchemaConverter.build(schema) + + obj1 = Model(data=4) + self.assertEqual(obj1.data, 4) + + obj2 = Model(data=9) + self.assertEqual(obj2.data, 9) + + with self.assertRaises(ValueError) as cm: + Model(data=6) + self.assertIn("matches multiple oneOf schemas", str(cm.exception)) + + with self.assertRaises(ValueError): + Model(data=5) + + def test_oneof_with_objects(self): + schema = { + "title": "Contact", + "type": "object", + "properties": { + "contact_info": { + "oneOf": [ + { + "type": "object", + "properties": { + "email": {"type": "string", "format": "email"} + }, + "required": ["email"], + "additionalProperties": False + }, + { + "type": "object", + "properties": { + "phone": {"type": "string", "pattern": "^[0-9-]+$"} + }, + "required": ["phone"], + "additionalProperties": False + } + ] + }, + }, + "required": ["contact_info"], + } + + Model = SchemaConverter.build(schema) + + obj1 = Model(contact_info={"email": "user@example.com"}) + self.assertEqual(obj1.contact_info.email, "user@example.com") + + obj2 = Model(contact_info={"phone": "123-456-7890"}) + self.assertEqual(obj2.contact_info.phone, "123-456-7890") + + with self.assertRaises(ValueError): + Model(contact_info={"email": "user@example.com", "phone": "123-456-7890"}) + + def test_oneof_with_discriminator_basic(self): + schema = { + "title": "Pet", + "type": "object", + "properties": { + "pet": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "cat"}, + "meows": {"type": "boolean"} + }, + "required": ["type", "meows"] + }, + { + "type": "object", + "properties": { + "type": {"const": "dog"}, + "barks": {"type": "boolean"} + }, + "required": ["type", "barks"] + } + ], + "discriminator": { + "propertyName": "type" + } + } + }, + "required": ["pet"] + } + + Model = SchemaConverter.build(schema) + + cat = Model(pet={"type": "cat", "meows": True}) + self.assertEqual(cat.pet.type, "cat") + self.assertEqual(cat.pet.meows, True) + + dog = Model(pet={"type": "dog", "barks": False}) + self.assertEqual(dog.pet.type, "dog") + self.assertEqual(dog.pet.barks, False) + + with self.assertRaises(ValueError): + Model(pet={"type": "cat", "barks": True}) + + with self.assertRaises(ValueError): + Model(pet={"type": "bird", "flies": True}) + + def test_oneof_with_discriminator_mapping(self): + schema = { + "title": "Vehicle", + "type": "object", + "properties": { + "vehicle": { + "oneOf": [ + { + "type": "object", + "properties": { + "vehicle_type": {"const": "car"}, + "doors": {"type": "integer", "minimum": 2, "maximum": 4} + }, + "required": ["vehicle_type", "doors"] + }, + { + "type": "object", + "properties": { + "vehicle_type": {"const": "motorcycle"}, + "engine_size": {"type": "number", "minimum": 125} + }, + "required": ["vehicle_type", "engine_size"] + } + ], + "discriminator": { + "propertyName": "vehicle_type", + "mapping": { + "car": "#/properties/vehicle/oneOf/0", + "motorcycle": "#/properties/vehicle/oneOf/1" + } + } + } + }, + "required": ["vehicle"] + } + + Model = SchemaConverter.build(schema) + + car = Model(vehicle={"vehicle_type": "car", "doors": 4}) + self.assertEqual(car.vehicle.vehicle_type, "car") + self.assertEqual(car.vehicle.doors, 4) + + motorcycle = Model(vehicle={"vehicle_type": "motorcycle", "engine_size": 600.0}) + self.assertEqual(motorcycle.vehicle.vehicle_type, "motorcycle") + self.assertEqual(motorcycle.vehicle.engine_size, 600.0) + + def test_oneof_with_discriminator_invalid_values(self): + schema = { + "title": "Shape", + "type": "object", + "properties": { + "shape": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "circle"}, + "radius": {"type": "number", "minimum": 0} + }, + "required": ["type", "radius"] + }, + { + "type": "object", + "properties": { + "type": {"const": "square"}, + "side": {"type": "number", "minimum": 0} + }, + "required": ["type", "side"] + } + ], + "discriminator": { + "propertyName": "type" + } + } + }, + "required": ["shape"] + } + + Model = SchemaConverter.build(schema) + + with self.assertRaises(ValueError): + Model(shape={"type": "triangle", "base": 5, "height": 3}) + + with self.assertRaises(ValueError): + Model(shape={"type": "circle", "side": 5}) + + with self.assertRaises(ValueError): + Model(shape={"radius": 5}) + + def test_oneof_missing_properties(self): + schema = { + "title": "Test", + "type": "object", + "properties": { + "value": { + "notOneOf": [ + {"type": "string"}, + {"type": "integer"}, + ] + }, + }, + } + + with self.assertRaises(ValueError): + SchemaConverter.build(schema) + + def test_oneof_invalid_properties(self): + schema = { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": None + }, + }, + } + + with self.assertRaises(ValueError): + SchemaConverter.build(schema) + + def test_oneof_with_default_value(self): + schema = { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [ + {"type": "string"}, + {"type": "integer"}, + ], + "default": "test" + }, + }, + } + + Model = SchemaConverter.build(schema) + obj = Model() + self.assertEqual(obj.value, "test") + + def test_oneof_with_invalid_default_value(self): + schema = { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [ + {"type": "string", "minLength": 5}, + {"type": "integer", "minimum": 10}, + ], + "default": "hi" + }, + }, + } + + with self.assertRaises(ValueError): + SchemaConverter.build(schema) + + def test_oneof_discriminator_without_property_name(self): + schema = { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "a"}, + "value": {"type": "string"} + } + }, + { + "type": "object", + "properties": { + "type": {"const": "b"}, + "value": {"type": "integer"} + } + } + ], + "discriminator": {} # discriminator without propertyName + } + } + } + + Model = SchemaConverter.build(schema) + + # Should succeed because input matches exactly one schema (the first one) + # The first schema matches: type="a" matches const("a"), value="test" is a string + # The second schema doesn't match: type="a" does not match const("b") + obj = Model(value={"type": "a", "value": "test", "extra": "invalid"}) + self.assertEqual(obj.value.type, "a") + self.assertEqual(obj.value.value, "test") + + # Test with input that matches the second schema + obj2 = Model(value={"type": "b", "value": 42}) + self.assertEqual(obj2.value.type, "b") + self.assertEqual(obj2.value.value, 42) + + # Test with input that matches neither schema (should fail) + with self.assertRaises(ValueError) as cm: + Model(value={"type": "c", "value": "test"}) + self.assertIn("does not match any of the oneOf schemas", str(cm.exception)) + + def test_oneof_multiple_matches_without_discriminator(self): + """Test case where input genuinely matches multiple oneOf schemas""" + schema = { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [ + { + "type": "object", + "properties": { + "data": {"type": "string"} + } + }, + { + "type": "object", + "properties": { + "data": {"type": "string"}, + "optional": {"type": "string"} + } + } + ], + "discriminator": {} # discriminator without propertyName + } + } + } + + Model = SchemaConverter.build(schema) + + # This input matches both schemas since both accept data as string + # and neither requires specific additional properties + with self.assertRaises(ValueError) as cm: + Model(value={"data": "test"}) + self.assertIn("matches multiple oneOf schemas", str(cm.exception)) + + def test_oneof_overlapping_strings_from_docs(self): + """Test the overlapping strings example from documentation""" + schema = { + "title": "SimpleExample", + "type": "object", + "properties": { + "value": { + "oneOf": [ + {"type": "string", "maxLength": 6}, + {"type": "string", "minLength": 4} + ] + } + }, + "required": ["value"] + } + + Model = SchemaConverter.build(schema) + + # Valid: Short string (matches first schema only) + obj1 = Model(value="hi") + self.assertEqual(obj1.value, "hi") + + # Valid: Long string (matches second schema only) + obj2 = Model(value="very long string") + self.assertEqual(obj2.value, "very long string") + + # Invalid: Medium string (matches BOTH schemas - violates oneOf) + with self.assertRaises(ValueError) as cm: + Model(value="hello") # 5 chars: matches maxLength=6 AND minLength=4 + self.assertIn("matches multiple oneOf schemas", str(cm.exception)) + + def test_oneof_shapes_discriminator_from_docs(self): + """Test the shapes discriminator example from documentation""" + schema = { + "title": "Shape", + "type": "object", + "properties": { + "shape": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "circle"}, + "radius": {"type": "number", "minimum": 0} + }, + "required": ["type", "radius"] + }, + { + "type": "object", + "properties": { + "type": {"const": "rectangle"}, + "width": {"type": "number", "minimum": 0}, + "height": {"type": "number", "minimum": 0} + }, + "required": ["type", "width", "height"] + } + ], + "discriminator": { + "propertyName": "type" + } + } + }, + "required": ["shape"] + } + + Model = SchemaConverter.build(schema) + + # Valid: Circle + circle = Model(shape={"type": "circle", "radius": 5.0}) + self.assertEqual(circle.shape.type, "circle") + self.assertEqual(circle.shape.radius, 5.0) + + # Valid: Rectangle + rectangle = Model(shape={"type": "rectangle", "width": 10, "height": 20}) + self.assertEqual(rectangle.shape.type, "rectangle") + self.assertEqual(rectangle.shape.width, 10) + self.assertEqual(rectangle.shape.height, 20) + + # Invalid: Wrong properties for the type + with self.assertRaises(ValueError): + Model(shape={"type": "circle", "width": 10}) -- 2.49.1 From cc6f2d42d572c4b249c3491822ba3b68256a9802 Mon Sep 17 00:00:00 2001 From: Vitor Hideyoshi Date: Tue, 19 Aug 2025 18:39:37 -0300 Subject: [PATCH 2/6] Separates PR for Better Testing and Readability --- jambo/parser/_type_parser.py | 23 ----- jambo/parser/const_type_parser.py | 24 ++--- jambo/parser/oneof_type_parser.py | 37 ++++++-- jambo/parser/string_type_parser.py | 9 +- tests/parser/test_const_type_parser.py | 61 +------------ tests/parser/test_oneof_type_parser.py | 119 ++++++++++++------------- 6 files changed, 100 insertions(+), 173 deletions(-) diff --git a/jambo/parser/_type_parser.py b/jambo/parser/_type_parser.py index 6c5cdc9..080965c 100644 --- a/jambo/parser/_type_parser.py +++ b/jambo/parser/_type_parser.py @@ -124,26 +124,3 @@ class GenericTypeParser(ABC, Generic[T]): return False return True - - @staticmethod - def _has_meaningful_constraints(field_props): - """ - Check if field properties contain meaningful constraints that require Field wrapping. - - Returns False if: - - field_props is None or empty - - field_props only contains {'default': None} - - Returns True if: - - field_props contains a non-None default value - - field_props contains other constraint properties (min_length, max_length, pattern, etc.) - """ - if not field_props: - return False - - # If only default is set and it's None, no meaningful constraints - if field_props == {"default": None}: - return False - - # If there are multiple properties or non-None default, that's meaningful - return True diff --git a/jambo/parser/const_type_parser.py b/jambo/parser/const_type_parser.py index 1e4ce84..b5c846f 100644 --- a/jambo/parser/const_type_parser.py +++ b/jambo/parser/const_type_parser.py @@ -3,7 +3,7 @@ from jambo.types.json_schema_type import JSONSchemaNativeTypes from jambo.types.type_parser_options import TypeParserOptions from pydantic import AfterValidator -from typing_extensions import Annotated, Any, Literal, Unpack +from typing_extensions import Annotated, Any, Unpack class ConstTypeParser(GenericTypeParser): @@ -33,19 +33,11 @@ class ConstTypeParser(GenericTypeParser): return const_type, parsed_properties def _build_const_type(self, const_value): - # Try to use Literal for hashable types (required for discriminated unions) - # Fall back to validator approach for non-hashable types - try: - # Test if the value is hashable (can be used in Literal) - hash(const_value) - return Literal[const_value] - except TypeError: - # Non-hashable type (like list, dict), use validator approach - def _validate_const_value(value: Any) -> Any: - if value != const_value: - raise ValueError( - f"Value must be equal to the constant value: {const_value}" - ) - return value + def _validate_const_value(value: Any) -> Any: + if value != const_value: + raise ValueError( + f"Value must be equal to the constant value: {const_value}" + ) + return value - return Annotated[type(const_value), AfterValidator(_validate_const_value)] \ No newline at end of file + return Annotated[type(const_value), AfterValidator(_validate_const_value)] diff --git a/jambo/parser/oneof_type_parser.py b/jambo/parser/oneof_type_parser.py index 79146b9..c0eeecc 100644 --- a/jambo/parser/oneof_type_parser.py +++ b/jambo/parser/oneof_type_parser.py @@ -1,8 +1,8 @@ from jambo.parser._type_parser import GenericTypeParser from jambo.types.type_parser_options import TypeParserOptions -from pydantic import Field, BeforeValidator, TypeAdapter, ValidationError -from typing_extensions import Annotated, Union, Unpack, Any +from pydantic import BeforeValidator, Field, TypeAdapter, ValidationError +from typing_extensions import Annotated, Any, Union, Unpack class OneOfTypeParser(GenericTypeParser): @@ -11,7 +11,7 @@ class OneOfTypeParser(GenericTypeParser): json_schema_type = "oneOf" def from_properties_impl( - self, name, properties, **kwargs: Unpack[TypeParserOptions] + self, name, properties, **kwargs: Unpack[TypeParserOptions] ): if "oneOf" not in properties: raise ValueError(f"Invalid JSON Schema: {properties}") @@ -42,7 +42,9 @@ class OneOfTypeParser(GenericTypeParser): if discriminator and isinstance(discriminator, dict): property_name = discriminator.get("propertyName") if property_name: - validated_type = Annotated[union_type, Field(discriminator=property_name)] + validated_type = Annotated[ + union_type, Field(discriminator=property_name) + ] return validated_type, mapped_properties def validate_one_of(value: Any) -> Any: @@ -59,11 +61,34 @@ class OneOfTypeParser(GenericTypeParser): continue if matched_count == 0: - raise ValueError(f"Value does not match any of the oneOf schemas") + raise ValueError("Value does not match any of the oneOf schemas") elif matched_count > 1: - raise ValueError(f"Value matches multiple oneOf schemas, exactly one expected") + raise ValueError( + "Value matches multiple oneOf schemas, exactly one expected" + ) return value validated_type = Annotated[union_type, BeforeValidator(validate_one_of)] return validated_type, mapped_properties + + @staticmethod + def _has_meaningful_constraints(field_props): + """ + Check if field properties contain meaningful constraints that require Field wrapping. + Returns False if: + - field_props is None or empty + - field_props only contains {'default': None} + Returns True if: + - field_props contains a non-None default value + - field_props contains other constraint properties (min_length, max_length, pattern, etc.) + """ + if not field_props: + return False + + # If only default is set and it's None, no meaningful constraints + if field_props == {"default": None}: + return False + + # If there are multiple properties or non-None default, that's meaningful + return True diff --git a/jambo/parser/string_type_parser.py b/jambo/parser/string_type_parser.py index 9f43034..7e94b0d 100644 --- a/jambo/parser/string_type_parser.py +++ b/jambo/parser/string_type_parser.py @@ -16,6 +16,7 @@ class StringTypeParser(GenericTypeParser): "maxLength": "max_length", "minLength": "min_length", "pattern": "pattern", + "format": "format", } format_type_mapping = { @@ -37,9 +38,7 @@ class StringTypeParser(GenericTypeParser): def from_properties_impl( self, name, properties, **kwargs: Unpack[TypeParserOptions] ): - mapped_properties = self.mappings_properties_builder( - properties, **kwargs - ) + mapped_properties = self.mappings_properties_builder(properties, **kwargs) format_type = properties.get("format") if not format_type: @@ -52,8 +51,4 @@ class StringTypeParser(GenericTypeParser): if format_type in self.format_pattern_mapping: mapped_properties["pattern"] = self.format_pattern_mapping[format_type] - if "json_schema_extra" not in mapped_properties: - mapped_properties["json_schema_extra"] = {} - mapped_properties["json_schema_extra"]["format"] = format_type - return mapped_type, mapped_properties diff --git a/tests/parser/test_const_type_parser.py b/tests/parser/test_const_type_parser.py index 5a8c9c1..ca92bb0 100644 --- a/tests/parser/test_const_type_parser.py +++ b/tests/parser/test_const_type_parser.py @@ -1,13 +1,12 @@ from jambo.parser import ConstTypeParser -from typing_extensions import Annotated, Literal, get_args, get_origin +from typing_extensions import Annotated, get_args, get_origin from unittest import TestCase class TestConstTypeParser(TestCase): - def test_const_type_parser_hashable_value(self): - """Test const parser with hashable values (uses Literal)""" + def test_const_type_parser(self): parser = ConstTypeParser() expected_const_value = "United States of America" @@ -17,60 +16,8 @@ class TestConstTypeParser(TestCase): "country", properties ) - # Check that we get a Literal type for hashable values - self.assertEqual(get_origin(parsed_type), Literal) - self.assertEqual(get_args(parsed_type), (expected_const_value,)) - - self.assertEqual(parsed_properties["default"], expected_const_value) - - def test_const_type_parser_non_hashable_value(self): - """Test const parser with non-hashable values (uses Annotated with validator)""" - parser = ConstTypeParser() - - expected_const_value = [1, 2, 3] # Lists are not hashable - properties = {"const": expected_const_value} - - parsed_type, parsed_properties = parser.from_properties_impl( - "list_const", properties - ) - - # Check that we get an Annotated type for non-hashable values self.assertEqual(get_origin(parsed_type), Annotated) - self.assertIn(list, get_args(parsed_type)) - - self.assertEqual(parsed_properties["default"], expected_const_value) - - def test_const_type_parser_integer_value(self): - """Test const parser with integer values (uses Literal)""" - parser = ConstTypeParser() - - expected_const_value = 42 - properties = {"const": expected_const_value} - - parsed_type, parsed_properties = parser.from_properties_impl( - "int_const", properties - ) - - # Check that we get a Literal type for hashable values - self.assertEqual(get_origin(parsed_type), Literal) - self.assertEqual(get_args(parsed_type), (expected_const_value,)) - - self.assertEqual(parsed_properties["default"], expected_const_value) - - def test_const_type_parser_boolean_value(self): - """Test const parser with boolean values (uses Literal)""" - parser = ConstTypeParser() - - expected_const_value = True - properties = {"const": expected_const_value} - - parsed_type, parsed_properties = parser.from_properties_impl( - "bool_const", properties - ) - - # Check that we get a Literal type for hashable values - self.assertEqual(get_origin(parsed_type), Literal) - self.assertEqual(get_args(parsed_type), (expected_const_value,)) + self.assertIn(str, get_args(parsed_type)) self.assertEqual(parsed_properties["default"], expected_const_value) @@ -99,4 +46,4 @@ class TestConstTypeParser(TestCase): self.assertIn( "Const type invalid_country must have 'const' value of allowed types", str(context.exception), - ) \ No newline at end of file + ) diff --git a/tests/parser/test_oneof_type_parser.py b/tests/parser/test_oneof_type_parser.py index 8c75f04..fbe362c 100644 --- a/tests/parser/test_oneof_type_parser.py +++ b/tests/parser/test_oneof_type_parser.py @@ -97,7 +97,7 @@ class TestOneOfTypeParser(TestCase): "email": {"type": "string", "format": "email"} }, "required": ["email"], - "additionalProperties": False + "additionalProperties": False, }, { "type": "object", @@ -105,8 +105,8 @@ class TestOneOfTypeParser(TestCase): "phone": {"type": "string", "pattern": "^[0-9-]+$"} }, "required": ["phone"], - "additionalProperties": False - } + "additionalProperties": False, + }, ] }, }, @@ -135,25 +135,23 @@ class TestOneOfTypeParser(TestCase): "type": "object", "properties": { "type": {"const": "cat"}, - "meows": {"type": "boolean"} + "meows": {"type": "boolean"}, }, - "required": ["type", "meows"] + "required": ["type", "meows"], }, { "type": "object", "properties": { "type": {"const": "dog"}, - "barks": {"type": "boolean"} + "barks": {"type": "boolean"}, }, - "required": ["type", "barks"] - } + "required": ["type", "barks"], + }, ], - "discriminator": { - "propertyName": "type" - } + "discriminator": {"propertyName": "type"}, } }, - "required": ["pet"] + "required": ["pet"], } Model = SchemaConverter.build(schema) @@ -183,29 +181,33 @@ class TestOneOfTypeParser(TestCase): "type": "object", "properties": { "vehicle_type": {"const": "car"}, - "doors": {"type": "integer", "minimum": 2, "maximum": 4} + "doors": { + "type": "integer", + "minimum": 2, + "maximum": 4, + }, }, - "required": ["vehicle_type", "doors"] + "required": ["vehicle_type", "doors"], }, { "type": "object", "properties": { "vehicle_type": {"const": "motorcycle"}, - "engine_size": {"type": "number", "minimum": 125} + "engine_size": {"type": "number", "minimum": 125}, }, - "required": ["vehicle_type", "engine_size"] - } + "required": ["vehicle_type", "engine_size"], + }, ], "discriminator": { "propertyName": "vehicle_type", "mapping": { "car": "#/properties/vehicle/oneOf/0", - "motorcycle": "#/properties/vehicle/oneOf/1" - } - } + "motorcycle": "#/properties/vehicle/oneOf/1", + }, + }, } }, - "required": ["vehicle"] + "required": ["vehicle"], } Model = SchemaConverter.build(schema) @@ -229,25 +231,23 @@ class TestOneOfTypeParser(TestCase): "type": "object", "properties": { "type": {"const": "circle"}, - "radius": {"type": "number", "minimum": 0} + "radius": {"type": "number", "minimum": 0}, }, - "required": ["type", "radius"] + "required": ["type", "radius"], }, { "type": "object", "properties": { "type": {"const": "square"}, - "side": {"type": "number", "minimum": 0} + "side": {"type": "number", "minimum": 0}, }, - "required": ["type", "side"] - } + "required": ["type", "side"], + }, ], - "discriminator": { - "propertyName": "type" - } + "discriminator": {"propertyName": "type"}, } }, - "required": ["shape"] + "required": ["shape"], } Model = SchemaConverter.build(schema) @@ -283,9 +283,7 @@ class TestOneOfTypeParser(TestCase): "title": "Test", "type": "object", "properties": { - "value": { - "oneOf": None - }, + "value": {"oneOf": None}, }, } @@ -302,7 +300,7 @@ class TestOneOfTypeParser(TestCase): {"type": "string"}, {"type": "integer"}, ], - "default": "test" + "default": "test", }, }, } @@ -321,7 +319,7 @@ class TestOneOfTypeParser(TestCase): {"type": "string", "minLength": 5}, {"type": "integer", "minimum": 10}, ], - "default": "hi" + "default": "hi", }, }, } @@ -340,20 +338,20 @@ class TestOneOfTypeParser(TestCase): "type": "object", "properties": { "type": {"const": "a"}, - "value": {"type": "string"} - } + "value": {"type": "string"}, + }, }, { "type": "object", "properties": { "type": {"const": "b"}, - "value": {"type": "integer"} - } - } + "value": {"type": "integer"}, + }, + }, ], - "discriminator": {} # discriminator without propertyName + "discriminator": {}, # discriminator without propertyName } - } + }, } Model = SchemaConverter.build(schema) @@ -383,23 +381,18 @@ class TestOneOfTypeParser(TestCase): "properties": { "value": { "oneOf": [ - { - "type": "object", - "properties": { - "data": {"type": "string"} - } - }, + {"type": "object", "properties": {"data": {"type": "string"}}}, { "type": "object", "properties": { "data": {"type": "string"}, - "optional": {"type": "string"} - } - } + "optional": {"type": "string"}, + }, + }, ], - "discriminator": {} # discriminator without propertyName + "discriminator": {}, # discriminator without propertyName } - } + }, } Model = SchemaConverter.build(schema) @@ -419,11 +412,11 @@ class TestOneOfTypeParser(TestCase): "value": { "oneOf": [ {"type": "string", "maxLength": 6}, - {"type": "string", "minLength": 4} + {"type": "string", "minLength": 4}, ] } }, - "required": ["value"] + "required": ["value"], } Model = SchemaConverter.build(schema) @@ -453,26 +446,24 @@ class TestOneOfTypeParser(TestCase): "type": "object", "properties": { "type": {"const": "circle"}, - "radius": {"type": "number", "minimum": 0} + "radius": {"type": "number", "minimum": 0}, }, - "required": ["type", "radius"] + "required": ["type", "radius"], }, { "type": "object", "properties": { "type": {"const": "rectangle"}, "width": {"type": "number", "minimum": 0}, - "height": {"type": "number", "minimum": 0} + "height": {"type": "number", "minimum": 0}, }, - "required": ["type", "width", "height"] - } + "required": ["type", "width", "height"], + }, ], - "discriminator": { - "propertyName": "type" - } + "discriminator": {"propertyName": "type"}, } }, - "required": ["shape"] + "required": ["shape"], } Model = SchemaConverter.build(schema) -- 2.49.1 From 86894fa918f8f603c37b5e897584d849d74d80ae Mon Sep 17 00:00:00 2001 From: Vitor Hideyoshi Date: Tue, 19 Aug 2025 20:08:34 -0300 Subject: [PATCH 3/6] (feature): Fix OneOf behavior on invalid discriminator According to the spec, propertyName is required when using a discriminator. If it is missing, the schema is invalid and should throw. --- jambo/parser/oneof_type_parser.py | 58 ++++++--- tests/parser/test_oneof_type_parser.py | 169 +++++++++---------------- 2 files changed, 99 insertions(+), 128 deletions(-) diff --git a/jambo/parser/oneof_type_parser.py b/jambo/parser/oneof_type_parser.py index c0eeecc..f0a1c88 100644 --- a/jambo/parser/oneof_type_parser.py +++ b/jambo/parser/oneof_type_parser.py @@ -31,33 +31,50 @@ class OneOfTypeParser(GenericTypeParser): if not kwargs.get("required", False): mapped_properties["default"] = mapped_properties.get("default") - field_types = [ - Annotated[t, Field(**v)] if self._has_meaningful_constraints(v) else t - for t, v in sub_types - ] - - union_type = Union[(*field_types,)] + subfield_types = [Annotated[t, Field(**v)] for t, v in sub_types] + # Added with the understanding of discriminator are not in the JsonSchema Spec, + # they were added by OpenAI and not all implementations may support them, + # and they do not always generate a model one-to-one to the Pydantic model + # TL;DR: Discriminators were added by OpenAI and not a Official JSON Schema feature discriminator = properties.get("discriminator") - if discriminator and isinstance(discriminator, dict): - property_name = discriminator.get("propertyName") - if property_name: - validated_type = Annotated[ - union_type, Field(discriminator=property_name) - ] - return validated_type, mapped_properties + if discriminator is not None: + validated_type = self._build_type_one_of_with_discriminator( + subfield_types, discriminator + ) + else: + validated_type = self._build_type_one_of_with_func(subfield_types) + + return validated_type, mapped_properties + + @staticmethod + def _build_type_one_of_with_discriminator( + subfield_types: list[Annotated], discriminator_prop: dict + ) -> Annotated: + if not isinstance(discriminator_prop, dict): + raise ValueError("Discriminator must be a dictionary") + + property_name = discriminator_prop.get("propertyName") + if property_name is None or not isinstance(property_name, str): + raise ValueError("Discriminator must have a 'propertyName' key") + + return Annotated[Union[(*subfield_types,)], Field(discriminator=property_name)] + + @staticmethod + def _build_type_one_of_with_func(subfield_types: list[Annotated]) -> Annotated: + """ + Build a validation function for the oneOf constraint. + This function will validate that the value matches exactly one of the schemas. + """ def validate_one_of(value: Any) -> Any: matched_count = 0 - validation_errors = [] - for field_type in field_types: + for field_type in subfield_types: try: - adapter = TypeAdapter(field_type) - adapter.validate_python(value) + TypeAdapter(field_type).validate_python(value) matched_count += 1 - except ValidationError as e: - validation_errors.append(str(e)) + except ValidationError: continue if matched_count == 0: @@ -69,8 +86,7 @@ class OneOfTypeParser(GenericTypeParser): return value - validated_type = Annotated[union_type, BeforeValidator(validate_one_of)] - return validated_type, mapped_properties + return Annotated[Union[(*subfield_types,)], BeforeValidator(validate_one_of)] @staticmethod def _has_meaningful_constraints(field_props): diff --git a/tests/parser/test_oneof_type_parser.py b/tests/parser/test_oneof_type_parser.py index fbe362c..80c2096 100644 --- a/tests/parser/test_oneof_type_parser.py +++ b/tests/parser/test_oneof_type_parser.py @@ -354,131 +354,86 @@ class TestOneOfTypeParser(TestCase): }, } - Model = SchemaConverter.build(schema) + # Should throw because the spec determines propertyName is required for discriminator + with self.assertRaises(ValueError): + SchemaConverter.build(schema) - # Should succeed because input matches exactly one schema (the first one) - # The first schema matches: type="a" matches const("a"), value="test" is a string - # The second schema doesn't match: type="a" does not match const("b") - obj = Model(value={"type": "a", "value": "test", "extra": "invalid"}) - self.assertEqual(obj.value.type, "a") - self.assertEqual(obj.value.value, "test") - - # Test with input that matches the second schema - obj2 = Model(value={"type": "b", "value": 42}) - self.assertEqual(obj2.value.type, "b") - self.assertEqual(obj2.value.value, 42) - - # Test with input that matches neither schema (should fail) - with self.assertRaises(ValueError) as cm: - Model(value={"type": "c", "value": "test"}) - self.assertIn("does not match any of the oneOf schemas", str(cm.exception)) - - def test_oneof_multiple_matches_without_discriminator(self): - """Test case where input genuinely matches multiple oneOf schemas""" + def test_oneof_overlapping_strings_from_docs(self): + """Test the overlapping strings example from documentation""" schema = { - "title": "Test", + "title": "SimpleExample", "type": "object", "properties": { "value": { "oneOf": [ - {"type": "object", "properties": {"data": {"type": "string"}}}, - { - "type": "object", - "properties": { - "data": {"type": "string"}, - "optional": {"type": "string"}, - }, - }, - ], - "discriminator": {}, # discriminator without propertyName + {"type": "string", "maxLength": 6}, + {"type": "string", "minLength": 4}, + ] } }, + "required": ["value"], } Model = SchemaConverter.build(schema) - # This input matches both schemas since both accept data as string - # and neither requires specific additional properties + # Valid: Short string (matches first schema only) + obj1 = Model(value="hi") + self.assertEqual(obj1.value, "hi") + + # Valid: Long string (matches second schema only) + obj2 = Model(value="very long string") + self.assertEqual(obj2.value, "very long string") + + # Invalid: Medium string (matches BOTH schemas - violates oneOf) with self.assertRaises(ValueError) as cm: - Model(value={"data": "test"}) + Model(value="hello") # 5 chars: matches maxLength=6 AND minLength=4 self.assertIn("matches multiple oneOf schemas", str(cm.exception)) - def test_oneof_overlapping_strings_from_docs(self): - """Test the overlapping strings example from documentation""" - schema = { - "title": "SimpleExample", - "type": "object", - "properties": { - "value": { - "oneOf": [ - {"type": "string", "maxLength": 6}, - {"type": "string", "minLength": 4}, - ] - } - }, - "required": ["value"], - } - - Model = SchemaConverter.build(schema) - - # Valid: Short string (matches first schema only) - obj1 = Model(value="hi") - self.assertEqual(obj1.value, "hi") - - # Valid: Long string (matches second schema only) - obj2 = Model(value="very long string") - self.assertEqual(obj2.value, "very long string") - - # Invalid: Medium string (matches BOTH schemas - violates oneOf) - with self.assertRaises(ValueError) as cm: - Model(value="hello") # 5 chars: matches maxLength=6 AND minLength=4 - self.assertIn("matches multiple oneOf schemas", str(cm.exception)) - - def test_oneof_shapes_discriminator_from_docs(self): - """Test the shapes discriminator example from documentation""" - schema = { - "title": "Shape", - "type": "object", - "properties": { - "shape": { - "oneOf": [ - { - "type": "object", - "properties": { - "type": {"const": "circle"}, - "radius": {"type": "number", "minimum": 0}, - }, - "required": ["type", "radius"], + def test_oneof_shapes_discriminator_from_docs(self): + """Test the shapes discriminator example from documentation""" + schema = { + "title": "Shape", + "type": "object", + "properties": { + "shape": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "circle"}, + "radius": {"type": "number", "minimum": 0}, }, - { - "type": "object", - "properties": { - "type": {"const": "rectangle"}, - "width": {"type": "number", "minimum": 0}, - "height": {"type": "number", "minimum": 0}, - }, - "required": ["type", "width", "height"], + "required": ["type", "radius"], + }, + { + "type": "object", + "properties": { + "type": {"const": "rectangle"}, + "width": {"type": "number", "minimum": 0}, + "height": {"type": "number", "minimum": 0}, }, - ], - "discriminator": {"propertyName": "type"}, - } - }, - "required": ["shape"], - } + "required": ["type", "width", "height"], + }, + ], + "discriminator": {"propertyName": "type"}, + } + }, + "required": ["shape"], + } - Model = SchemaConverter.build(schema) + Model = SchemaConverter.build(schema) - # Valid: Circle - circle = Model(shape={"type": "circle", "radius": 5.0}) - self.assertEqual(circle.shape.type, "circle") - self.assertEqual(circle.shape.radius, 5.0) + # Valid: Circle + circle = Model(shape={"type": "circle", "radius": 5.0}) + self.assertEqual(circle.shape.type, "circle") + self.assertEqual(circle.shape.radius, 5.0) - # Valid: Rectangle - rectangle = Model(shape={"type": "rectangle", "width": 10, "height": 20}) - self.assertEqual(rectangle.shape.type, "rectangle") - self.assertEqual(rectangle.shape.width, 10) - self.assertEqual(rectangle.shape.height, 20) + # Valid: Rectangle + rectangle = Model(shape={"type": "rectangle", "width": 10, "height": 20}) + self.assertEqual(rectangle.shape.type, "rectangle") + self.assertEqual(rectangle.shape.width, 10) + self.assertEqual(rectangle.shape.height, 20) - # Invalid: Wrong properties for the type - with self.assertRaises(ValueError): - Model(shape={"type": "circle", "width": 10}) + # Invalid: Wrong properties for the type + with self.assertRaises(ValueError): + Model(shape={"type": "circle", "width": 10}) -- 2.49.1 From 79932bb5952df4e9404e45d4d2a2c62621fbfa4c Mon Sep 17 00:00:00 2001 From: Vitor Hideyoshi Date: Tue, 19 Aug 2025 20:29:25 -0300 Subject: [PATCH 4/6] (feature): Removes _has_meaningful_constraints Removes _has_meaningful_constraints since nowhere in the spec says that a subproperty should have a meaningful value other that its type --- jambo/parser/oneof_type_parser.py | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/jambo/parser/oneof_type_parser.py b/jambo/parser/oneof_type_parser.py index f0a1c88..12b62fe 100644 --- a/jambo/parser/oneof_type_parser.py +++ b/jambo/parser/oneof_type_parser.py @@ -51,6 +51,9 @@ class OneOfTypeParser(GenericTypeParser): def _build_type_one_of_with_discriminator( subfield_types: list[Annotated], discriminator_prop: dict ) -> Annotated: + """ + Build a type with a discriminator. + """ if not isinstance(discriminator_prop, dict): raise ValueError("Discriminator must be a dictionary") @@ -63,8 +66,7 @@ class OneOfTypeParser(GenericTypeParser): @staticmethod def _build_type_one_of_with_func(subfield_types: list[Annotated]) -> Annotated: """ - Build a validation function for the oneOf constraint. - This function will validate that the value matches exactly one of the schemas. + Build a type with a validation function for the oneOf constraint. """ def validate_one_of(value: Any) -> Any: @@ -87,24 +89,3 @@ class OneOfTypeParser(GenericTypeParser): return value return Annotated[Union[(*subfield_types,)], BeforeValidator(validate_one_of)] - - @staticmethod - def _has_meaningful_constraints(field_props): - """ - Check if field properties contain meaningful constraints that require Field wrapping. - Returns False if: - - field_props is None or empty - - field_props only contains {'default': None} - Returns True if: - - field_props contains a non-None default value - - field_props contains other constraint properties (min_length, max_length, pattern, etc.) - """ - if not field_props: - return False - - # If only default is set and it's None, no meaningful constraints - if field_props == {"default": None}: - return False - - # If there are multiple properties or non-None default, that's meaningful - return True -- 2.49.1 From 15944549a0c212efefb455b7847c89310281397e Mon Sep 17 00:00:00 2001 From: Vitor Hideyoshi Date: Tue, 19 Aug 2025 20:40:49 -0300 Subject: [PATCH 5/6] (feat): Adds Aditional Tests --- tests/parser/test_oneof_type_parser.py | 111 +++++++++++++++++++------ 1 file changed, 84 insertions(+), 27 deletions(-) diff --git a/tests/parser/test_oneof_type_parser.py b/tests/parser/test_oneof_type_parser.py index 80c2096..b559a0a 100644 --- a/tests/parser/test_oneof_type_parser.py +++ b/tests/parser/test_oneof_type_parser.py @@ -1,9 +1,35 @@ from jambo import SchemaConverter +from jambo.parser.oneof_type_parser import OneOfTypeParser from unittest import TestCase class TestOneOfTypeParser(TestCase): + def test_oneof_raises_on_invalid_property(self): + with self.assertRaises(ValueError): + OneOfTypeParser().from_properties_impl( + "test_field", + { + # Invalid schema, should have property "oneOf" + }, + required=True, + context={}, + ref_cache={}, + ) + + with self.assertRaises(ValueError): + SchemaConverter.build( + { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [], # should throw because oneOf requires at least one schema + } + }, + } + ) + def test_oneof_basic_integer_and_string(self): schema = { "title": "Person", @@ -328,35 +354,66 @@ class TestOneOfTypeParser(TestCase): SchemaConverter.build(schema) def test_oneof_discriminator_without_property_name(self): - schema = { - "title": "Test", - "type": "object", - "properties": { - "value": { - "oneOf": [ - { - "type": "object", - "properties": { - "type": {"const": "a"}, - "value": {"type": "string"}, - }, - }, - { - "type": "object", - "properties": { - "type": {"const": "b"}, - "value": {"type": "integer"}, - }, - }, - ], - "discriminator": {}, # discriminator without propertyName - } - }, - } - # Should throw because the spec determines propertyName is required for discriminator with self.assertRaises(ValueError): - SchemaConverter.build(schema) + SchemaConverter.build( + { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "a"}, + "value": {"type": "string"}, + }, + }, + { + "type": "object", + "properties": { + "type": {"const": "b"}, + "value": {"type": "integer"}, + }, + }, + ], + "discriminator": {}, # discriminator without propertyName + } + }, + } + ) + + def test_oneof_discriminator_with_invalid_discriminator(self): + # Should throw because a valid discriminator is required + with self.assertRaises(ValueError): + SchemaConverter.build( + { + "title": "Test", + "type": "object", + "properties": { + "value": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "a"}, + "value": {"type": "string"}, + }, + }, + { + "type": "object", + "properties": { + "type": {"const": "b"}, + "value": {"type": "integer"}, + }, + }, + ], + "discriminator": "invalid", # discriminator without propertyName + } + }, + } + ) def test_oneof_overlapping_strings_from_docs(self): """Test the overlapping strings example from documentation""" -- 2.49.1 From c5e70402dbb77d231d2c9302f516aebf602c20f5 Mon Sep 17 00:00:00 2001 From: Vitor Hideyoshi Date: Tue, 19 Aug 2025 20:44:16 -0300 Subject: [PATCH 6/6] (feat): Adds Warning to Docs About Discriminator Keyword --- docs/source/usage.oneof.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/source/usage.oneof.rst b/docs/source/usage.oneof.rst index a836df4..6e875a4 100644 --- a/docs/source/usage.oneof.rst +++ b/docs/source/usage.oneof.rst @@ -106,3 +106,7 @@ Examples .. warning:: If your data could match multiple schemas in a oneOf, validation will fail. Ensure schemas are mutually exclusive. + +.. warning:: + + The discriminator feature is not officially in the JSON Schema specification, it was introduced by OpenAI. Use it with caution and ensure it fits your use case. -- 2.49.1