refactor: allOf support to happen in build_model_property

Author: dbanty

end_to_end_tests/golden-record-custom/custom_e2e/models/model_from_all_of.py

@@ -11,34 +11,34 @@
 class ModelFromAllOf:
     """  """
 
-    another_sub_property: Union[Unset, str] = UNSET
     a_sub_property: Union[Unset, str] = UNSET
+    another_sub_property: Union[Unset, str] = UNSET
     additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
 
     def to_dict(self) -> Dict[str, Any]:
-        another_sub_property = self.another_sub_property
         a_sub_property = self.a_sub_property
+        another_sub_property = self.another_sub_property
 
         field_dict: Dict[str, Any] = {}
         field_dict.update(self.additional_properties)
         field_dict.update({})
-        if another_sub_property is not UNSET:
-            field_dict["another_sub_property"] = another_sub_property
         if a_sub_property is not UNSET:
             field_dict["a_sub_property"] = a_sub_property
+        if another_sub_property is not UNSET:
+            field_dict["another_sub_property"] = another_sub_property
 
         return field_dict
 
     @classmethod
     def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
         d = src_dict.copy()
-        another_sub_property = d.pop("another_sub_property", UNSET)
-
         a_sub_property = d.pop("a_sub_property", UNSET)
 
+        another_sub_property = d.pop("another_sub_property", UNSET)
+
         model_from_all_of = cls(
-            another_sub_property=another_sub_property,
             a_sub_property=a_sub_property,
+            another_sub_property=another_sub_property,
         )
 
         model_from_all_of.additional_properties = d

end_to_end_tests/golden-record/my_test_api_client/models/model_from_all_of.py

@@ -11,34 +11,34 @@
 class ModelFromAllOf:
     """  """
 
-    another_sub_property: Union[Unset, str] = UNSET
     a_sub_property: Union[Unset, str] = UNSET
+    another_sub_property: Union[Unset, str] = UNSET
     additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
 
     def to_dict(self) -> Dict[str, Any]:
-        another_sub_property = self.another_sub_property
         a_sub_property = self.a_sub_property
+        another_sub_property = self.another_sub_property
 
         field_dict: Dict[str, Any] = {}
         field_dict.update(self.additional_properties)
         field_dict.update({})
-        if another_sub_property is not UNSET:
-            field_dict["another_sub_property"] = another_sub_property
         if a_sub_property is not UNSET:
             field_dict["a_sub_property"] = a_sub_property
+        if another_sub_property is not UNSET:
+            field_dict["another_sub_property"] = another_sub_property
 
         return field_dict
 
     @classmethod
     def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
         d = src_dict.copy()
-        another_sub_property = d.pop("another_sub_property", UNSET)
-
         a_sub_property = d.pop("a_sub_property", UNSET)
 
+        another_sub_property = d.pop("another_sub_property", UNSET)
+
         model_from_all_of = cls(
-            another_sub_property=another_sub_property,
             a_sub_property=a_sub_property,
+            another_sub_property=another_sub_property,
         )
 
         model_from_all_of.additional_properties = d

openapi_python_client/parser/properties/__init__.py

@@ -250,21 +250,25 @@ 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)
 
     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 sub_prop in data.allOf or []:
+        if isinstance(sub_prop, oai.Reference):
+            source_name = Reference.from_ref(sub_prop.ref).class_name
+            sub_model = schemas.models.get(source_name)
+            if sub_model is None:
+                return PropertyError(f"Reference {sub_prop.ref} not found"), schemas
+            required_properties.extend(sub_model.required_properties)
+            optional_properties.extend(sub_model.optional_properties)
+            relative_imports.update(sub_model.relative_imports)
+        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
@@ -302,7 +306,6 @@ def build_model_property(
 
     prop = ModelProperty(
         reference=ref,
-        references=references,
         required_properties=required_properties,
         optional_properties=optional_properties,
         relative_imports=relative_imports,
@@ -555,15 +558,5 @@ 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
 
-    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)
-    schemas.errors.extend(resolve_errors)
     return schemas

openapi_python_client/parser/properties/model_property.py

@@ -1,21 +1,16 @@
-from collections.abc import Iterable
-from typing import ClassVar, Dict, List, Set, Union
+from typing import ClassVar, List, Set, Union
 
 import attr
 
-from ... import schema as oai
-from ..errors import PropertyError
 from ..reference import Reference
 from .property import Property
-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
@@ -24,49 +19,6 @@ class ModelProperty(Property):
 
     template: ClassVar[str] = "model_property.pyi"
 
-    def resolve_references(
-        self, components: Dict[str, Union[oai.Reference, oai.Schema]], schemas: Schemas
-    ) -> Union[Schemas, PropertyError]:
-        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, schemas = property_from_data(
-                name=key, required=required, data=value, schemas=schemas, parent_name=source_name
-            )
-            if isinstance(prop, PropertyError):
-                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 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 """
         type_string = self.reference.class_name

openapi_python_client/parser/properties/schemas.py

@@ -5,16 +5,14 @@
 import attr
 
 from ..errors import ParseError
-
-# Avoid circular import with forward reference
-from . import model_property
 from .enum_property import EnumProperty
+from .model_property import ModelProperty
 
 
 @attr.s(auto_attribs=True, frozen=True)
 class Schemas:
     """ Structure for containing all defined, shareable, and resuabled schemas (attr classes and Enums) """
 
     enums: Dict[str, EnumProperty] = attr.ib(factory=dict)
-    models: Dict[str, "model_property.ModelProperty"] = attr.ib(factory=dict)
+    models: Dict[str, ModelProperty] = attr.ib(factory=dict)
     errors: List[ParseError] = attr.ib(factory=list)

tests/test_parser/test_properties/test_init.py

@@ -586,7 +586,6 @@ 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="",
@@ -603,7 +602,6 @@ 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="",
@@ -990,25 +988,19 @@ 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=[])
-    schemas_2.models = {"1": model_1, "2": model_2}
-    error_1 = PropertyError()
+    error = 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_1, schemas_3),
-        (error_1, schemas_3),
+        (error, schemas_3),
+        (error, schemas_3),
     ]
 
     from openapi_python_client.parser.properties import Schemas, build_schemas
@@ -1024,12 +1016,8 @@ 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
-    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]
+    assert result == schemas_2
+    assert result.errors == [error]
 
 
 def test_build_parse_error_on_reference():
@@ -1103,7 +1091,6 @@ def test_build_model_property(additional_properties_schema, expected_additional_
         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,