fix: correctly resolve references to a type that is itself just a single allOf reference

Author: eli-bl

openapi_python_client/parser/bodies.py

@@ -117,6 +117,7 @@ def body_from_data(
                     **schemas.classes_by_name,
                     prop.class_info.name: prop,
                 },
+                models_to_process=[*schemas.models_to_process, prop],
             )
         bodies.append(
             Body(

openapi_python_client/parser/properties/__init__.py

@@ -126,7 +126,7 @@ def _property_from_ref(
     return prop, schemas
 
 
-def property_from_data(  # noqa: PLR0911
+def property_from_data(  # noqa: PLR0911, PLR0912
     name: str,
     required: bool,
     data: oai.Reference | oai.Schema,
@@ -153,7 +153,7 @@ def property_from_data(  # noqa: PLR0911
     sub_data: list[oai.Schema | oai.Reference] = data.allOf + data.anyOf + data.oneOf
     # A union of a single reference should just be passed through to that reference (don't create copy class)
     if len(sub_data) == 1 and isinstance(sub_data[0], oai.Reference):
-        return _property_from_ref(
+        prop, schemas = _property_from_ref(
             name=name,
             required=required,
             parent=data,
@@ -162,6 +162,17 @@ def property_from_data(  # noqa: PLR0911
             config=config,
             roots=roots,
         )
+        if not isinstance(prop, PropertyError):
+            # We won't be generating a separate Python class for this schema - references to it will just use
+            # the class for the schema it's referencing - so we don't add it to classes_by_name; but we do
+            # add it to models_to_process, if it's a model, because its properties still need to be resolved.
+            if isinstance(prop, ModelProperty):
+                schemas = evolve(
+                    schemas,
+                    models_to_process=[*schemas.models_to_process, prop],
+                )
+        return prop, schemas
+
     if data.type == oai.DataType.BOOLEAN:
         return (
             BooleanProperty.build(
@@ -341,7 +352,7 @@ def _process_model_errors(
 
 
 def _process_models(*, schemas: Schemas, config: Config) -> Schemas:
-    to_process = (prop for prop in schemas.classes_by_name.values() if isinstance(prop, ModelProperty))
+    to_process = schemas.models_to_process
     still_making_progress = True
     final_model_errors: list[tuple[ModelProperty, PropertyError]] = []
     latest_model_errors: list[tuple[ModelProperty, PropertyError]] = []
@@ -368,12 +379,11 @@ def _process_models(*, schemas: Schemas, config: Config) -> Schemas:
                 continue
             schemas = schemas_or_err
             still_making_progress = True
-        to_process = (prop for prop in next_round)
+        to_process = next_round
 
     final_model_errors.extend(latest_model_errors)
     errors = _process_model_errors(final_model_errors, schemas=schemas)
-    schemas.errors.extend(errors)
-    return schemas
+    return evolve(schemas, errors=[*schemas.errors, *errors], models_to_process=to_process)
 
 
 def build_schemas(

openapi_python_client/parser/properties/model_property.py

@@ -119,7 +119,11 @@ def build(
             )
             return error, schemas
 
-        schemas = evolve(schemas, classes_by_name={**schemas.classes_by_name, class_info.name: prop})
+        schemas = evolve(
+            schemas,
+            classes_by_name={**schemas.classes_by_name, class_info.name: prop},
+            models_to_process=[*schemas.models_to_process, prop],
+        )
         return prop, schemas
 
     @classmethod

openapi_python_client/parser/properties/schemas.py

@@ -22,8 +22,10 @@
 from ..errors import ParameterError, ParseError, PropertyError
 
 if TYPE_CHECKING:  # pragma: no cover
+    from .model_property import ModelProperty
     from .property import Property
 else:
+    ModelProperty = "ModelProperty"
     Property = "Property"
 
 
@@ -77,6 +79,7 @@ class Schemas:
     classes_by_reference: Dict[ReferencePath, Property] = field(factory=dict)
     dependencies: Dict[ReferencePath, Set[Union[ReferencePath, ClassName]]] = field(factory=dict)
     classes_by_name: Dict[ClassName, Property] = field(factory=dict)
+    models_to_process: List[ModelProperty] = field(factory=list)
     errors: List[ParseError] = field(factory=list)
 
     def add_dependencies(self, ref_path: ReferencePath, roots: Set[Union[ReferencePath, ClassName]]) -> None:

tests/test_parser/test_properties/test_init.py

@@ -530,6 +530,7 @@ def test_property_from_data_ref_enum_with_overridden_default(self, enum_property
         prop, new_schemas = property_from_data(
             name=name, required=required, data=data, schemas=schemas, parent_name="", config=config
         )
+        new_schemas = attr.evolve(new_schemas, models_to_process=[])  # intermediate state irrelevant to this test
 
         assert prop == enum_property_factory(
             name="some_enum",
@@ -918,15 +919,18 @@ def test_retries_failing_models_while_making_progress(
 
         first_model = model_property_factory()
         second_class_name = ClassName("second", "")
+        second_model = model_property_factory()
+        any_prop = any_property_factory()
         schemas = Schemas(
             classes_by_name={
                 ClassName("first", ""): first_model,
-                second_class_name: model_property_factory(),
-                ClassName("non-model", ""): any_property_factory(),
-            }
+                second_class_name: second_model,
+                ClassName("non-model", ""): any_prop,
+            },
+            models_to_process=[first_model, second_model],
         )
         process_model = mocker.patch(
-            f"{MODULE_NAME}.process_model", side_effect=[PropertyError(), Schemas(), PropertyError()]
+            f"{MODULE_NAME}.process_model", side_effect=[PropertyError(), schemas, PropertyError()]
         )
         process_model_errors = mocker.patch(f"{MODULE_NAME}._process_model_errors", return_value=["error"])
 
@@ -935,8 +939,8 @@ def test_retries_failing_models_while_making_progress(
         process_model.assert_has_calls(
             [
                 call(first_model, schemas=schemas, config=config),
-                call(schemas.classes_by_name[second_class_name], schemas=schemas, config=config),
-                call(first_model, schemas=result, config=config),
+                call(second_model, schemas=schemas, config=config),
+                call(first_model, schemas=schemas, config=config),
             ]
         )
         assert process_model_errors.was_called_once_with([(first_model, PropertyError())])
@@ -950,14 +954,16 @@ def test_detect_recursive_allof_reference_no_retry(self, mocker, model_property_
         recursive_model = model_property_factory(
             class_info=Class(name=class_name, module_name=PythonIdentifier("module_name", ""))
         )
+        second_model = model_property_factory()
         schemas = Schemas(
             classes_by_name={
                 "recursive": recursive_model,
-                "second": model_property_factory(),
-            }
+                "second": second_model,
+            },
+            models_to_process=[recursive_model, second_model],
         )
         recursion_error = PropertyError(data=Reference.model_construct(ref=f"#/{class_name}"))
-        process_model = mocker.patch(f"{MODULE_NAME}.process_model", side_effect=[recursion_error, Schemas()])
+        process_model = mocker.patch(f"{MODULE_NAME}.process_model", side_effect=[recursion_error, schemas])
         process_model_errors = mocker.patch(f"{MODULE_NAME}._process_model_errors", return_value=["error"])
 
         result = _process_models(schemas=schemas, config=config)
@@ -972,6 +978,58 @@ def test_detect_recursive_allof_reference_no_retry(self, mocker, model_property_
         assert all(error in result.errors for error in process_model_errors.return_value)
         assert "\n\nRecursive allOf reference found" in recursion_error.detail
 
+    def test_resolve_reference_to_single_allof_reference(self, config, model_property_factory):
+        # test for https://github.com/openapi-generators/openapi-python-client/issues/1091
+        from openapi_python_client.parser.properties import Schemas, build_schemas
+
+        components = {
+            "Model1": oai.Schema.model_construct(
+                type="object",
+                properties={
+                    "prop1": oai.Schema.model_construct(type="string"),
+                },
+            ),
+            "Model2": oai.Schema.model_construct(
+                allOf=[
+                    oai.Reference.model_construct(ref="#/components/schemas/Model1"),
+                ]
+            ),
+            "Model3": oai.Schema.model_construct(
+                allOf=[
+                    oai.Reference.model_construct(ref="#/components/schemas/Model2"),
+                    oai.Schema.model_construct(
+                        type="object",
+                        properties={
+                            "prop2": oai.Schema.model_construct(type="string"),
+                        },
+                    ),
+                ],
+            ),
+        }
+        schemas = Schemas()
+
+        result = build_schemas(components=components, schemas=schemas, config=config)
+
+        assert result.errors == []
+        assert result.models_to_process == []
+
+        # Classes should only be generated for Model1 and Model3
+        assert result.classes_by_name.keys() == {"Model1", "Model3"}
+
+        # References to Model2 should be resolved to the same class as Model1
+        assert result.classes_by_reference.keys() == {
+            "/components/schemas/Model1",
+            "/components/schemas/Model2",
+            "/components/schemas/Model3",
+        }
+        assert (
+            result.classes_by_reference["/components/schemas/Model2"].class_info
+            == result.classes_by_reference["/components/schemas/Model1"].class_info
+        )
+
+        # Verify that Model3 extended the properties from Model1
+        assert [p.name for p in result.classes_by_name["Model3"].optional_properties] == ["prop1", "prop2"]
+
 
 class TestPropogateRemoval:
     def test_propogate_removal_class_name(self):