From c1fd345e0ae237b68f426045ad025b2066e00d64 Mon Sep 17 00:00:00 2001 From: Adrian Kreuziger Date: Mon, 29 Jan 2024 13:36:33 -0800 Subject: [PATCH 1/2] Handle "type" being an array of strings --- recap/converters/json_schema.py | 18 ++++++- recap/types.py | 8 +++ tests/unit/converters/test_json_schema.py | 66 +++++++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/recap/converters/json_schema.py b/recap/converters/json_schema.py index d413e15..99379ff 100644 --- a/recap/converters/json_schema.py +++ b/recap/converters/json_schema.py @@ -50,10 +50,14 @@ def to_recap( def _parse( self, - json_schema: dict, + json_schema: dict | str, alias_strategy: AliasStrategy, ) -> RecapType: extra_attrs = {} + # Check if json_schema is just a string representing a basic type, and convert + # to a dict with a "type" property if so + if isinstance(json_schema, str): + json_schema = {"type": json_schema} if "description" in json_schema: extra_attrs["doc"] = json_schema["description"] if "default" in json_schema: @@ -65,12 +69,22 @@ def _parse( self.json_registry = resource @ self.json_registry extra_attrs["alias"] = alias_strategy(resource_id) + # Special handling for "type" defined as a list of strings like + # {"type": ["string", "boolean"]} + if "type" in json_schema and isinstance(json_schema["type"], list): + types = [self._parse(s, alias_strategy) for s in json_schema["type"]] + return UnionType(types, **extra_attrs) + match json_schema: case {"type": "object", "properties": properties}: fields = [] for name, prop in properties.items(): field = self._parse(prop, alias_strategy) - if name not in json_schema.get("required", []): + # Make type nullable if it's not required, but avoid double nullable types + if ( + name not in json_schema.get("required", []) + and not field.is_nullable() + ): field = field.make_nullable() field.extra_attrs["name"] = name fields.append(field) diff --git a/recap/types.py b/recap/types.py index fdbe679..ac0875f 100644 --- a/recap/types.py +++ b/recap/types.py @@ -55,6 +55,14 @@ def make_nullable(self) -> UnionType: type_copy = RecapTypeClass(**attrs, **extra_attrs) return UnionType([NullType(), type_copy], **union_attrs) + def is_nullable(self) -> bool: + """ + Returns True if the type is nullable. + :return: True if the type is nullable. + """ + + return isinstance(self, UnionType) and NullType() in self.types + def validate(self) -> None: # Default to valid type pass diff --git a/tests/unit/converters/test_json_schema.py b/tests/unit/converters/test_json_schema.py index b61401d..29a28cc 100644 --- a/tests/unit/converters/test_json_schema.py +++ b/tests/unit/converters/test_json_schema.py @@ -65,6 +65,72 @@ def test_all_basic_types(): ] +def test_nullable_types(): + """Tests nullable types (["null", "string"]), with and without default values. Also tests that nullable properties aren't + made double nullable if they're not required.""" + json_schema = """ + { + "type": "object", + "properties": { + "required_nullable_no_default": {"type": ["null", "string"]}, + "required_nullable_with_null_default": {"type": ["null", "string"], "default": null}, + "required_nullable_with__default": {"type": ["null", "string"], "default": "default_value"}, + "nullable_no_default": {"type": ["null", "string"]}, + "nullable_with_null_default": {"type": ["null", "string"], "default": null}, + "nullable_with__default": {"type": ["null", "string"], "default": "default_value"} + }, + "required": ["required_nullable_no_default", "required_nullable_with_null_default", "required_nullable_with__default"] + } + """ + Draft202012Validator.check_schema(loads(json_schema)) + struct_type = JSONSchemaConverter().to_recap(json_schema) + assert isinstance(struct_type, StructType) + assert struct_type.fields == [ + UnionType([NullType(), StringType()], name="required_nullable_no_default"), + UnionType( + [NullType(), StringType()], + name="required_nullable_with_null_default", + default=None, + ), + UnionType( + [NullType(), StringType()], + name="required_nullable_with__default", + default="default_value", + ), + UnionType([NullType(), StringType()], name="nullable_no_default"), + UnionType( + [NullType(), StringType()], + name="nullable_with_null_default", + default=None, + ), + UnionType( + [NullType(), StringType()], + name="nullable_with__default", + default="default_value", + ), + ] + + +def test_union_types(): + json_schema = """ + { + "type": "object", + "properties": { + "union": {"type": ["null", "string", "boolean", "number"]} + }, + "required": ["union"] + } + """ + Draft202012Validator.check_schema(loads(json_schema)) + struct_type = JSONSchemaConverter().to_recap(json_schema) + assert isinstance(struct_type, StructType) + assert struct_type.fields == [ + UnionType( + [NullType(), StringType(), BoolType(), FloatType(bits=64)], name="union" + ), + ] + + def test_nested_objects(): json_schema = """ { From 193e411f41d65dcbae30d4437d91156abd4e19ef Mon Sep 17 00:00:00 2001 From: Adrian Kreuziger Date: Tue, 6 Feb 2024 13:42:31 -0800 Subject: [PATCH 2/2] Fix optional logic --- recap/converters/json_schema.py | 25 ++++++++++++----------- tests/unit/converters/test_json_schema.py | 12 +++++------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/recap/converters/json_schema.py b/recap/converters/json_schema.py index 99379ff..ffdac9b 100644 --- a/recap/converters/json_schema.py +++ b/recap/converters/json_schema.py @@ -69,23 +69,24 @@ def _parse( self.json_registry = resource @ self.json_registry extra_attrs["alias"] = alias_strategy(resource_id) - # Special handling for "type" defined as a list of strings like - # {"type": ["string", "boolean"]} - if "type" in json_schema and isinstance(json_schema["type"], list): - types = [self._parse(s, alias_strategy) for s in json_schema["type"]] - return UnionType(types, **extra_attrs) - match json_schema: + # Special handling for "type" defined as a list of strings like + # {"type": ["string", "boolean"]} + case {"type": list(type_list)}: + types = [self._parse(s, alias_strategy) for s in type_list] + return UnionType(types, **extra_attrs) case {"type": "object", "properties": properties}: fields = [] for name, prop in properties.items(): field = self._parse(prop, alias_strategy) - # Make type nullable if it's not required, but avoid double nullable types - if ( - name not in json_schema.get("required", []) - and not field.is_nullable() - ): - field = field.make_nullable() + # If not explicitly required, make optional by ensuring the field is + # nullable, and has a default + if name not in json_schema.get("required", []): + if not field.is_nullable(): + field = field.make_nullable() + if "default" not in field.extra_attrs: + field.extra_attrs["default"] = None + field.extra_attrs["name"] = name fields.append(field) return StructType(fields, **extra_attrs) diff --git a/tests/unit/converters/test_json_schema.py b/tests/unit/converters/test_json_schema.py index 29a28cc..095e32b 100644 --- a/tests/unit/converters/test_json_schema.py +++ b/tests/unit/converters/test_json_schema.py @@ -74,12 +74,12 @@ def test_nullable_types(): "properties": { "required_nullable_no_default": {"type": ["null", "string"]}, "required_nullable_with_null_default": {"type": ["null", "string"], "default": null}, - "required_nullable_with__default": {"type": ["null", "string"], "default": "default_value"}, + "required_nullable_with_default": {"type": ["null", "string"], "default": "default_value"}, "nullable_no_default": {"type": ["null", "string"]}, "nullable_with_null_default": {"type": ["null", "string"], "default": null}, - "nullable_with__default": {"type": ["null", "string"], "default": "default_value"} + "nullable_with_default": {"type": ["null", "string"], "default": "default_value"} }, - "required": ["required_nullable_no_default", "required_nullable_with_null_default", "required_nullable_with__default"] + "required": ["required_nullable_no_default", "required_nullable_with_null_default", "required_nullable_with_default"] } """ Draft202012Validator.check_schema(loads(json_schema)) @@ -94,10 +94,10 @@ def test_nullable_types(): ), UnionType( [NullType(), StringType()], - name="required_nullable_with__default", + name="required_nullable_with_default", default="default_value", ), - UnionType([NullType(), StringType()], name="nullable_no_default"), + UnionType([NullType(), StringType()], name="nullable_no_default", default=None), UnionType( [NullType(), StringType()], name="nullable_with_null_default", @@ -105,7 +105,7 @@ def test_nullable_types(): ), UnionType( [NullType(), StringType()], - name="nullable_with__default", + name="nullable_with_default", default="default_value", ), ]