correctly resolve references to a type that is itself just a single allOf reference (openapi-generators#1103)

Fixes openapi-generators#1091

Example of a valid spec that triggered this bug:
https://gist.github.com/eli-bl/8f5c7d1d872d9fda5379fa6370dab6a8

In this spec, CreateCat contains only an `allOf` with a single reference
to CreateAnimal. The current behavior of `openapi-python-client` in such
a case is that it treats CreateCat as simply an alias for CreateAnimal;
any references to it are treated as references to CreateAnimal, and it
doesn't bother creating a model class for CreateCat. And if the spec
contained only those two types, then this would be successful.

(Note, the term "alias" doesn't exist in OpenAPI/JSON Schema, but I'm
using it here to mean "a type that extends one other type with `allOf`,
with no changes." Whether that should be treated as a separate thing in
any way is not really a concern of OpenAPI; it's an issue for us only
because we are generating code for model classes. See also:
openapi-generators#1104)

Anyway, in this case the spec also contains UpdateCat, which extends
CreateCat with an additional property. This _should_ be exactly the same
as extending CreateAnimal... but, prior to this fix, it resulted in a
parsing error. The problem happened like this:

1. In `_create_schemas`, we create a ModelProperty for each of the three
schemas.
* The one for CreateCat is handled slightly differently: its `data`
attribute points to the exact same schema as CreateAnimal, and we do not
add it into `schemas.classes_by_name` because we don't want to generate
a separate Python class for it.
2. In `_process_models`, we're attempting to finish filling in the
property list for each model.
* That might not be possible right away because there might be a
reference to another model that hasn't been fully processed yet. So we
iterate as many times as necessary until they're all fully resolved.
However...
* What we are iterating over is `schemas.classes_by_name`. There's an
incorrect assumption that every named model is included in that dict; in
this case, CreateCat is not in it.
* Therefore, CreateCat remains in an invalid state, and the reference
from CreateAnimal to CreateCat causes an error.

My solution is to use `classes_by_name` only for the purpose of
determining what Python classes to generate, and add a new collection,
`models_to_process`, which includes _every_ ModelProperty including ones
that are aliases.

After the fix, generating a client from the example spec succeeds. The
only Python model classes created are CreateAnimal and UpdateCat; the
`post` endpoint that referenced CreateCat uses the CreateAnimal class.
Again, that's consistent with how `openapi-python-client` currently
handles these type aliases; the difference is just that it no longer
fails when it sees a reference _to_ the alias.

---------

Co-authored-by: Dylan Anthony <[email protected]>

Author: 2 people

.changeset/correctly_resolve_references_to_a_type_that_is_itself_just_a_single_allof_reference.md

@@ -0,0 +1,7 @@
+---
+default: patch
+---
+
+# Correctly resolve references to a type that is itself just a single allOf reference
+
+PR #1103 fixed issue #1091. Thanks @eli-bl!

end_to_end_tests/baseline_openapi_3.0.json

@@ -1680,6 +1680,33 @@
           }
         }
       }
+    },
+    "/models/allof": {
+      "get": {
+        "responses": {
+          "200": {
+            "description": "OK",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "object",
+                  "properties": {
+                    "aliased": {
+                      "$ref": "#/components/schemas/Aliased"
+                    },
+                    "extended": {
+                      "$ref": "#/components/schemas/Extended"
+                    },
+                    "model": {
+                      "$ref": "#/components/schemas/AModel"
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
     }
   },
   "components": {
@@ -1698,6 +1725,23 @@
           "an_required_field"
         ]
       },
+      "Aliased":{
+        "allOf": [
+          {"$ref":  "#/components/schemas/AModel"}
+        ]
+      },
+      "Extended": {
+        "allOf": [
+          {"$ref": "#/components/schemas/Aliased"},
+          {"type": "object",
+            "properties": {
+              "fromExtended": {
+                "type": "string"
+              }
+            }
+          }
+        ]
+      },
       "AModel": {
         "title": "AModel",
         "required": [

end_to_end_tests/baseline_openapi_3.1.yaml

@@ -1670,7 +1670,34 @@ info:
         }
       }
     }
-  }
+  },
+  "/models/allof": {
+    "get": {
+      "responses": {
+        "200": {
+          "description": "OK",
+          "content": {
+            "application/json": {
+              "schema": {
+                "type": "object",
+                "properties": {
+                  "aliased": {
+                    "$ref": "#/components/schemas/Aliased"
+                  },
+                  "extended": {
+                    "$ref": "#/components/schemas/Extended"
+                  },
+                  "model": {
+                    "$ref": "#/components/schemas/AModel"
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  },
 }
 "components":
   "schemas": {
@@ -1688,6 +1715,23 @@ info:
         "an_required_field"
       ]
     },
+    "Aliased": {
+      "allOf": [
+        { "$ref": "#/components/schemas/AModel" }
+      ]
+    },
+    "Extended": {
+      "allOf": [
+        { "$ref": "#/components/schemas/Aliased" },
+        { "type": "object",
+          "properties": {
+            "fromExtended": {
+              "type": "string"
+            }
+          }
+        }
+      ]
+    },
     "AModel": {
       "title": "AModel",
       "required": [
@@ -1718,11 +1762,7 @@ info:
           "default": "overridden_default"
         },
         "an_optional_allof_enum": {
-          "allOf": [
-            {
-              "$ref": "#/components/schemas/AnAllOfEnum"
-            }
-          ]
+          "$ref": "#/components/schemas/AnAllOfEnum",
         },
         "nested_list_of_enums": {
           "title": "Nested List Of Enums",
@@ -1859,11 +1899,7 @@ info:
           ]
         },
         "model": {
-          "allOf": [
-            {
-              "$ref": "#/components/schemas/ModelWithUnionProperty"
-            }
-          ]
+          "$ref": "#/components/schemas/ModelWithUnionProperty"
         },
         "nullable_model": {
           "oneOf": [
@@ -1876,11 +1912,7 @@ info:
           ]
         },
         "not_required_model": {
-          "allOf": [
-            {
-              "$ref": "#/components/schemas/ModelWithUnionProperty"
-            }
-          ]
+          "$ref": "#/components/schemas/ModelWithUnionProperty"
         },
         "not_required_nullable_model": {
           "oneOf": [

end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/default/__init__.py

@@ -2,7 +2,7 @@
 
 import types
 
-from . import get_common_parameters, post_common_parameters, reserved_parameters
+from . import get_common_parameters, get_models_allof, post_common_parameters, reserved_parameters
 
 
 class DefaultEndpoints:
@@ -17,3 +17,7 @@ def post_common_parameters(cls) -> types.ModuleType:
     @classmethod
     def reserved_parameters(cls) -> types.ModuleType:
         return reserved_parameters
+
+    @classmethod
+    def get_models_allof(cls) -> types.ModuleType:
+        return get_models_allof

end_to_end_tests/golden-record/my_test_api_client/api/default/get_models_allof.py

@@ -0,0 +1,122 @@
+from http import HTTPStatus
+from typing import Any, Dict, Optional, Union
+
+import httpx
+
+from ... import errors
+from ...client import AuthenticatedClient, Client
+from ...models.get_models_allof_response_200 import GetModelsAllofResponse200
+from ...types import Response
+
+
+def _get_kwargs() -> Dict[str, Any]:
+    _kwargs: Dict[str, Any] = {
+        "method": "get",
+        "url": "/models/allof",
+    }
+
+    return _kwargs
+
+
+def _parse_response(
+    *, client: Union[AuthenticatedClient, Client], response: httpx.Response
+) -> Optional[GetModelsAllofResponse200]:
+    if response.status_code == HTTPStatus.OK:
+        response_200 = GetModelsAllofResponse200.from_dict(response.json())
+
+        return response_200
+    if client.raise_on_unexpected_status:
+        raise errors.UnexpectedStatus(response.status_code, response.content)
+    else:
+        return None
+
+
+def _build_response(
+    *, client: Union[AuthenticatedClient, Client], response: httpx.Response
+) -> Response[GetModelsAllofResponse200]:
+    return Response(
+        status_code=HTTPStatus(response.status_code),
+        content=response.content,
+        headers=response.headers,
+        parsed=_parse_response(client=client, response=response),
+    )
+
+
+def sync_detailed(
+    *,
+    client: Union[AuthenticatedClient, Client],
+) -> Response[GetModelsAllofResponse200]:
+    """
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        Response[GetModelsAllofResponse200]
+    """
+
+    kwargs = _get_kwargs()
+
+    response = client.get_httpx_client().request(
+        **kwargs,
+    )
+
+    return _build_response(client=client, response=response)
+
+
+def sync(
+    *,
+    client: Union[AuthenticatedClient, Client],
+) -> Optional[GetModelsAllofResponse200]:
+    """
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        GetModelsAllofResponse200
+    """
+
+    return sync_detailed(
+        client=client,
+    ).parsed
+
+
+async def asyncio_detailed(
+    *,
+    client: Union[AuthenticatedClient, Client],
+) -> Response[GetModelsAllofResponse200]:
+    """
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        Response[GetModelsAllofResponse200]
+    """
+
+    kwargs = _get_kwargs()
+
+    response = await client.get_async_httpx_client().request(**kwargs)
+
+    return _build_response(client=client, response=response)
+
+
+async def asyncio(
+    *,
+    client: Union[AuthenticatedClient, Client],
+) -> Optional[GetModelsAllofResponse200]:
+    """
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        GetModelsAllofResponse200
+    """
+
+    return (
+        await asyncio_detailed(
+            client=client,
+        )
+    ).parsed

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

@@ -34,9 +34,11 @@
 from .body_upload_file_tests_upload_post_some_object import BodyUploadFileTestsUploadPostSomeObject
 from .body_upload_file_tests_upload_post_some_optional_object import BodyUploadFileTestsUploadPostSomeOptionalObject
 from .different_enum import DifferentEnum
+from .extended import Extended
 from .free_form_model import FreeFormModel
 from .get_location_header_types_int_enum_header import GetLocationHeaderTypesIntEnumHeader
 from .get_location_header_types_string_enum_header import GetLocationHeaderTypesStringEnumHeader
+from .get_models_allof_response_200 import GetModelsAllofResponse200
 from .http_validation_error import HTTPValidationError
 from .import_ import Import
 from .json_like_body import JsonLikeBody
@@ -112,9 +114,11 @@
     "BodyUploadFileTestsUploadPostSomeObject",
     "BodyUploadFileTestsUploadPostSomeOptionalObject",
     "DifferentEnum",
+    "Extended",
     "FreeFormModel",
     "GetLocationHeaderTypesIntEnumHeader",
     "GetLocationHeaderTypesStringEnumHeader",
+    "GetModelsAllofResponse200",
     "HTTPValidationError",
     "Import",
     "JsonLikeBody",