From 307431c042e6352034e03c9e2ef7af660ec9f9ba Mon Sep 17 00:00:00 2001 From: Packy Gallagher Date: Mon, 23 Nov 2020 17:48:43 -0800 Subject: [PATCH 1/5] Add allOf support for model definitions Combines the child elements into one, without class heirarchy, mixins, etc --- .../parser/properties/__init__.py | 19 ++++++- .../parser/properties/model_property.py | 55 ++++++++++++++++++- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index 4b71df5a2..73c91b065 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/openapi_python_client/parser/properties/__init__.py @@ -236,14 +236,27 @@ def build_model_property( required_properties: List[Property] = [] optional_properties: List[Property] = [] relative_imports: Set[str] = set() + references: List[oai.Reference] = [] class_name = data.title or name if parent_name: class_name = f"{utils.pascal_case(parent_name)}{utils.pascal_case(class_name)}" ref = Reference.from_ref(class_name) - for key, value in (data.properties or {}).items(): + all_props = data.properties or {} + if not isinstance(data, oai.Reference) and data.allOf: + for sub_prop in data.allOf: + if isinstance(sub_prop, oai.Reference): + references += [sub_prop] + else: + all_props.update(sub_prop.properties or {}) + required_set.update(sub_prop.required or []) + + for key, value in all_props.items(): prop_required = key in required_set + if not isinstance(value, oai.Reference) and value.allOf: + # resolved later + continue prop, schemas = property_from_data( name=key, required=prop_required, data=value, schemas=schemas, parent_name=class_name ) @@ -257,6 +270,7 @@ def build_model_property( prop = ModelProperty( reference=ref, + references=references, required_properties=required_properties, optional_properties=optional_properties, relative_imports=relative_imports, @@ -509,5 +523,6 @@ def build_schemas(*, components: Dict[str, Union[oai.Reference, oai.Schema]]) -> processing = True # We made some progress this round, do another after it's done to_process = next_round schemas.errors.extend(errors) - + for model in schemas.models.values(): + model.resolve_references(components=components, schemas=schemas) return schemas diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index ca36171af..52251a886 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -1,17 +1,25 @@ -from typing import ClassVar, List, Set +from __future__ import annotations + +from collections.abc import Iterable +from typing import TYPE_CHECKING, ClassVar, Dict, List, Set, Union import attr +from ... import schema as oai +from ..errors import ParseError from ..reference import Reference from .property import Property +if TYPE_CHECKING: + from .schemas import Schemas + @attr.s(auto_attribs=True, frozen=True) class ModelProperty(Property): """ A property which refers to another Schema """ reference: Reference - + references: List[oai.Reference] required_properties: List[Property] optional_properties: List[Property] description: str @@ -19,6 +27,49 @@ class ModelProperty(Property): template: ClassVar[str] = "model_property.pyi" + def resolve_references( + self, components: Dict[str, Union[oai.Reference, oai.Schema]], schemas: Schemas + ) -> Union[List[Property], ParseError]: + from ..properties import property_from_data + + required_set = set() + props = {} + while self.references: + reference = self.references.pop() + source_name = Reference.from_ref(reference.ref).class_name + referenced_prop = components[source_name] + assert isinstance(referenced_prop, oai.Schema) + for p, val in (referenced_prop.properties or {}).items(): + props[p] = (val, source_name) + for sub_prop in referenced_prop.allOf or []: + if isinstance(sub_prop, oai.Reference): + self.references.append(sub_prop) + else: + for p, val in (sub_prop.properties or {}).items(): + props[p] = (val, source_name) + if isinstance(referenced_prop.required, Iterable): + for sub_prop_name in referenced_prop.required: + required_set.add(sub_prop_name) + + for key, (value, source_name) in (props or {}).items(): + required = key in required_set + prop, _ = property_from_data( + name=key, required=required, data=value, schemas=schemas, parent_name=source_name + ) + if isinstance(prop, ParseError): + return prop + if required: + self.required_properties.append(prop) + # Remove the optional version + new_optional_props = [op for op in self.optional_properties if op.name != prop.name] + self.optional_properties.clear() + self.optional_properties.extend(new_optional_props) + elif not any(ep for ep in (self.optional_properties + self.required_properties) if ep.name == prop.name): + self.optional_properties.append(prop) + self.relative_imports.update(prop.get_imports(prefix="..")) + + return self.required_properties + self.optional_properties + def get_type_string(self, no_optional: bool = False) -> str: """ Get a string representation of type that should be used when declaring this property """ type_string = self.reference.class_name From e7c9a2ee506dd162a2a3eaeebde66ea8ad44c317 Mon Sep 17 00:00:00 2001 From: Packy Gallagher Date: Fri, 4 Dec 2020 13:52:52 -0800 Subject: [PATCH 2/5] Fix schemas object building --- openapi_python_client/parser/properties/__init__.py | 13 +++++++++++-- .../parser/properties/model_property.py | 10 +++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py index 73c91b065..499b52aec 100644 --- a/openapi_python_client/parser/properties/__init__.py +++ b/openapi_python_client/parser/properties/__init__.py @@ -522,7 +522,16 @@ def build_schemas(*, components: Dict[str, Union[oai.Reference, oai.Schema]]) -> schemas = schemas_or_err processing = True # We made some progress this round, do another after it's done to_process = next_round + + resolve_errors: List[PropertyError] = [] + models = list(schemas.models.values()) + for model in models: + schemas_or_err = model.resolve_references(components=components, schemas=schemas) + if isinstance(schemas_or_err, PropertyError): + resolve_errors.append(schemas_or_err) + else: + schemas = schemas_or_err + schemas.errors.extend(errors) - for model in schemas.models.values(): - model.resolve_references(components=components, schemas=schemas) + schemas.errors.extend(resolve_errors) return schemas diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index 52251a886..fbe257111 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -6,7 +6,7 @@ import attr from ... import schema as oai -from ..errors import ParseError +from ..errors import PropertyError from ..reference import Reference from .property import Property @@ -29,7 +29,7 @@ class ModelProperty(Property): def resolve_references( self, components: Dict[str, Union[oai.Reference, oai.Schema]], schemas: Schemas - ) -> Union[List[Property], ParseError]: + ) -> Union[Schemas, PropertyError]: from ..properties import property_from_data required_set = set() @@ -53,10 +53,10 @@ def resolve_references( for key, (value, source_name) in (props or {}).items(): required = key in required_set - prop, _ = property_from_data( + prop, schemas = property_from_data( name=key, required=required, data=value, schemas=schemas, parent_name=source_name ) - if isinstance(prop, ParseError): + if isinstance(prop, PropertyError): return prop if required: self.required_properties.append(prop) @@ -68,7 +68,7 @@ def resolve_references( self.optional_properties.append(prop) self.relative_imports.update(prop.get_imports(prefix="..")) - return self.required_properties + self.optional_properties + return schemas def get_type_string(self, no_optional: bool = False) -> str: """ Get a string representation of type that should be used when declaring this property """ From e0c734c1bae65415e0cc699e257f7dcd80c498f1 Mon Sep 17 00:00:00 2001 From: Packy Gallagher Date: Fri, 4 Dec 2020 13:21:06 -0800 Subject: [PATCH 3/5] Update existing tests --- tests/test_parser/test_properties/test_init.py | 3 +++ tests/test_parser/test_properties/test_model_property.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py index 96047c60f..324952a4e 100644 --- a/tests/test_parser/test_properties/test_init.py +++ b/tests/test_parser/test_properties/test_init.py @@ -584,6 +584,7 @@ def test_property_from_data_ref_model(self): nullable=False, default=None, reference=Reference(class_name=class_name, module_name="my_model"), + references=[], required_properties=[], optional_properties=[], description="", @@ -599,6 +600,7 @@ def test_property_from_data_ref_model(self): nullable=False, default=None, reference=Reference(class_name=class_name, module_name="my_model"), + references=[], required_properties=[], optional_properties=[], description="", @@ -1073,6 +1075,7 @@ def test_build_model_property(): nullable=False, default=None, reference=Reference(class_name="ParentMyModel", module_name="parent_my_model"), + references=[], required_properties=[StringProperty(name="req", required=True, nullable=False, default=None)], optional_properties=[DateTimeProperty(name="opt", required=False, nullable=False, default=None)], description=data.description, diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index 72c8f27f1..c3bec8e00 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -23,6 +23,7 @@ def test_get_type_string(no_optional, nullable, required, expected): nullable=nullable, default=None, reference=Reference(class_name="MyClass", module_name="my_module"), + references=[], description="", optional_properties=[], required_properties=[], @@ -41,6 +42,7 @@ def test_get_imports(): nullable=True, default=None, reference=Reference(class_name="MyClass", module_name="my_module"), + references=[], description="", optional_properties=[], required_properties=[], From abbcee79450a5825e663a3797f8c2b0ec2ab33ff Mon Sep 17 00:00:00 2001 From: Packy Gallagher Date: Fri, 4 Dec 2020 13:41:08 -0800 Subject: [PATCH 4/5] Add test for resolve_references --- .../test_properties/test_model_property.py | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index c3bec8e00..90395876a 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -57,3 +57,67 @@ def test_get_imports(): "from typing import Dict", "from typing import cast", } + + +def test_resolve_references(mocker): + import openapi_python_client.schema as oai + from openapi_python_client.parser.properties import build_model_property + + schemas = { + "RefA": oai.Schema.construct( + title=mocker.MagicMock(), + description=mocker.MagicMock(), + required=["String"], + properties={ + "String": oai.Schema.construct(type="string"), + "Enum": oai.Schema.construct(type="string", enum=["aValue"]), + "DateTime": oai.Schema.construct(type="string", format="date-time"), + }, + ), + "RefB": oai.Schema.construct( + title=mocker.MagicMock(), + description=mocker.MagicMock(), + required=["DateTime"], + properties={ + "Int": oai.Schema.construct(type="integer"), + "DateTime": oai.Schema.construct(type="string", format="date-time"), + "Float": oai.Schema.construct(type="number", format="float"), + }, + ), + # Intentionally no properties defined + "RefC": oai.Schema.construct( + title=mocker.MagicMock(), + description=mocker.MagicMock(), + ), + } + + model_schema = oai.Schema.construct( + allOf=[ + oai.Reference.construct(ref="#/components/schemas/RefA"), + oai.Reference.construct(ref="#/components/schemas/RefB"), + oai.Reference.construct(ref="#/components/schemas/RefC"), + oai.Schema.construct( + title=mocker.MagicMock(), + description=mocker.MagicMock(), + required=["Float"], + properties={ + "String": oai.Schema.construct(type="string"), + "Float": oai.Schema.construct(type="number", format="float"), + }, + ), + ] + ) + + components = {**schemas, "Model": model_schema} + + from openapi_python_client.parser.properties import Schemas + + schemas_holder = Schemas() + model, schemas_holder = build_model_property( + data=model_schema, name="Model", required=True, schemas=schemas_holder, parent_name=None + ) + model.resolve_references(components, schemas_holder) + assert sorted(p.name for p in model.required_properties) == ["DateTime", "Float", "String"] + assert all(p.required for p in model.required_properties) + assert sorted(p.name for p in model.optional_properties) == ["Enum", "Int"] + assert all(not p.required for p in model.optional_properties) From 729851ca3ff6db09f5725a54bf722a1930f8d755 Mon Sep 17 00:00:00 2001 From: Packy Gallagher Date: Fri, 4 Dec 2020 14:08:35 -0800 Subject: [PATCH 5/5] Update test_build_schemas for resolve_references on models --- .../test_parser/test_properties/test_init.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py index 324952a4e..39af21b35 100644 --- a/tests/test_parser/test_properties/test_init.py +++ b/tests/test_parser/test_properties/test_init.py @@ -986,19 +986,25 @@ def test__string_based_property_unsupported_format(self, mocker): def test_build_schemas(mocker): build_model_property = mocker.patch(f"{MODULE_NAME}.build_model_property") in_data = {"1": mocker.MagicMock(enum=None), "2": mocker.MagicMock(enum=None), "3": mocker.MagicMock(enum=None)} + model_1 = mocker.MagicMock() schemas_1 = mocker.MagicMock() model_2 = mocker.MagicMock() schemas_2 = mocker.MagicMock(errors=[]) - error = PropertyError() + schemas_2.models = {"1": model_1, "2": model_2} + error_1 = PropertyError() schemas_3 = mocker.MagicMock() + schemas_4 = mocker.MagicMock(errors=[]) + model_1.resolve_references.return_value = schemas_4 + error_2 = PropertyError() + model_2.resolve_references.return_value = error_2 # This loops through one for each, then again to retry the error build_model_property.side_effect = [ (model_1, schemas_1), (model_2, schemas_2), - (error, schemas_3), - (error, schemas_3), + (error_1, schemas_3), + (error_1, schemas_3), ] from openapi_python_client.parser.properties import Schemas, build_schemas @@ -1014,8 +1020,12 @@ def test_build_schemas(mocker): ] ) # schemas_3 was the last to come back from build_model_property, but it should be ignored because it's an error - assert result == schemas_2 - assert result.errors == [error] + model_1.resolve_references.assert_called_once_with(components=in_data, schemas=schemas_2) + # schemas_4 came from resolving model_1 + model_2.resolve_references.assert_called_once_with(components=in_data, schemas=schemas_4) + # resolving model_2 resulted in err, so no schemas_5 + assert result == schemas_4 + assert result.errors == [error_1, error_2] def test_build_parse_error_on_reference():