Skip to content

Commit cf181c2

Browse files
eli-bldbanty
eli-bl
and
dbanty
authored
correctly resolve references to a type that is itself just a single allOf reference (#1103)
Fixes #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: #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 <dbanty@users.noreply.github.com>
1 parent 5f7c9a5 commit cf181c2

File tree

14 files changed

+932
-59
lines changed

14 files changed

+932
-59
lines changed

‎.changeset/correctly_resolve_references_to_a_type_that_is_itself_just_a_single_allof_reference.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
default: patch
3+
---
4+
5+
# Correctly resolve references to a type that is itself just a single allOf reference
6+
7+
PR #1103 fixed issue #1091. Thanks @eli-bl!

‎end_to_end_tests/baseline_openapi_3.0.json‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,33 @@
16291629
}
16301630
}
16311631
}
1632+
},
1633+
"/models/allof": {
1634+
"get": {
1635+
"responses": {
1636+
"200": {
1637+
"description": "OK",
1638+
"content": {
1639+
"application/json": {
1640+
"schema": {
1641+
"type": "object",
1642+
"properties": {
1643+
"aliased": {
1644+
"$ref": "#/components/schemas/Aliased"
1645+
},
1646+
"extended": {
1647+
"$ref": "#/components/schemas/Extended"
1648+
},
1649+
"model": {
1650+
"$ref": "#/components/schemas/AModel"
1651+
}
1652+
}
1653+
}
1654+
}
1655+
}
1656+
}
1657+
}
1658+
}
16321659
}
16331660
},
16341661
"components": {
@@ -1647,6 +1674,23 @@
16471674
"an_required_field"
16481675
]
16491676
},
1677+
"Aliased":{
1678+
"allOf": [
1679+
{"$ref": "#/components/schemas/AModel"}
1680+
]
1681+
},
1682+
"Extended": {
1683+
"allOf": [
1684+
{"$ref": "#/components/schemas/Aliased"},
1685+
{"type": "object",
1686+
"properties": {
1687+
"fromExtended": {
1688+
"type": "string"
1689+
}
1690+
}
1691+
}
1692+
]
1693+
},
16501694
"AModel": {
16511695
"title": "AModel",
16521696
"required": [

‎end_to_end_tests/baseline_openapi_3.1.yaml‎

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,7 +1619,34 @@ info:
16191619
}
16201620
}
16211621
}
1622-
}
1622+
},
1623+
"/models/allof": {
1624+
"get": {
1625+
"responses": {
1626+
"200": {
1627+
"description": "OK",
1628+
"content": {
1629+
"application/json": {
1630+
"schema": {
1631+
"type": "object",
1632+
"properties": {
1633+
"aliased": {
1634+
"$ref": "#/components/schemas/Aliased"
1635+
},
1636+
"extended": {
1637+
"$ref": "#/components/schemas/Extended"
1638+
},
1639+
"model": {
1640+
"$ref": "#/components/schemas/AModel"
1641+
}
1642+
}
1643+
}
1644+
}
1645+
}
1646+
}
1647+
}
1648+
}
1649+
},
16231650
}
16241651
"components":
16251652
"schemas": {
@@ -1637,6 +1664,23 @@ info:
16371664
"an_required_field"
16381665
]
16391666
},
1667+
"Aliased": {
1668+
"allOf": [
1669+
{ "$ref": "#/components/schemas/AModel" }
1670+
]
1671+
},
1672+
"Extended": {
1673+
"allOf": [
1674+
{ "$ref": "#/components/schemas/Aliased" },
1675+
{ "type": "object",
1676+
"properties": {
1677+
"fromExtended": {
1678+
"type": "string"
1679+
}
1680+
}
1681+
}
1682+
]
1683+
},
16401684
"AModel": {
16411685
"title": "AModel",
16421686
"required": [
@@ -1667,11 +1711,7 @@ info:
16671711
"default": "overridden_default"
16681712
},
16691713
"an_optional_allof_enum": {
1670-
"allOf": [
1671-
{
1672-
"$ref": "#/components/schemas/AnAllOfEnum"
1673-
}
1674-
]
1714+
"$ref": "#/components/schemas/AnAllOfEnum",
16751715
},
16761716
"nested_list_of_enums": {
16771717
"title": "Nested List Of Enums",
@@ -1808,11 +1848,7 @@ info:
18081848
]
18091849
},
18101850
"model": {
1811-
"allOf": [
1812-
{
1813-
"$ref": "#/components/schemas/ModelWithUnionProperty"
1814-
}
1815-
]
1851+
"$ref": "#/components/schemas/ModelWithUnionProperty"
18161852
},
18171853
"nullable_model": {
18181854
"oneOf": [
@@ -1825,11 +1861,7 @@ info:
18251861
]
18261862
},
18271863
"not_required_model": {
1828-
"allOf": [
1829-
{
1830-
"$ref": "#/components/schemas/ModelWithUnionProperty"
1831-
}
1832-
]
1864+
"$ref": "#/components/schemas/ModelWithUnionProperty"
18331865
},
18341866
"not_required_nullable_model": {
18351867
"oneOf": [

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import types
44

5-
from . import get_common_parameters, post_common_parameters, reserved_parameters
5+
from . import get_common_parameters, get_models_allof, post_common_parameters, reserved_parameters
66

77

88
class DefaultEndpoints:
@@ -17,3 +17,7 @@ def post_common_parameters(cls) -> types.ModuleType:
1717
@classmethod
1818
def reserved_parameters(cls) -> types.ModuleType:
1919
return reserved_parameters
20+
21+
@classmethod
22+
def get_models_allof(cls) -> types.ModuleType:
23+
return get_models_allof

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
from http import HTTPStatus
2+
from typing import Any, Dict, Optional, Union
3+
4+
import httpx
5+
6+
from ... import errors
7+
from ...client import AuthenticatedClient, Client
8+
from ...models.get_models_allof_response_200 import GetModelsAllofResponse200
9+
from ...types import Response
10+
11+
12+
def _get_kwargs() -> Dict[str, Any]:
13+
_kwargs: Dict[str, Any] = {
14+
"method": "get",
15+
"url": "/models/allof",
16+
}
17+
18+
return _kwargs
19+
20+
21+
def _parse_response(
22+
*, client: Union[AuthenticatedClient, Client], response: httpx.Response
23+
) -> Optional[GetModelsAllofResponse200]:
24+
if response.status_code == HTTPStatus.OK:
25+
response_200 = GetModelsAllofResponse200.from_dict(response.json())
26+
27+
return response_200
28+
if client.raise_on_unexpected_status:
29+
raise errors.UnexpectedStatus(response.status_code, response.content)
30+
else:
31+
return None
32+
33+
34+
def _build_response(
35+
*, client: Union[AuthenticatedClient, Client], response: httpx.Response
36+
) -> Response[GetModelsAllofResponse200]:
37+
return Response(
38+
status_code=HTTPStatus(response.status_code),
39+
content=response.content,
40+
headers=response.headers,
41+
parsed=_parse_response(client=client, response=response),
42+
)
43+
44+
45+
def sync_detailed(
46+
*,
47+
client: Union[AuthenticatedClient, Client],
48+
) -> Response[GetModelsAllofResponse200]:
49+
"""
50+
Raises:
51+
errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
52+
httpx.TimeoutException: If the request takes longer than Client.timeout.
53+
54+
Returns:
55+
Response[GetModelsAllofResponse200]
56+
"""
57+
58+
kwargs = _get_kwargs()
59+
60+
response = client.get_httpx_client().request(
61+
**kwargs,
62+
)
63+
64+
return _build_response(client=client, response=response)
65+
66+
67+
def sync(
68+
*,
69+
client: Union[AuthenticatedClient, Client],
70+
) -> Optional[GetModelsAllofResponse200]:
71+
"""
72+
Raises:
73+
errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
74+
httpx.TimeoutException: If the request takes longer than Client.timeout.
75+
76+
Returns:
77+
GetModelsAllofResponse200
78+
"""
79+
80+
return sync_detailed(
81+
client=client,
82+
).parsed
83+
84+
85+
async def asyncio_detailed(
86+
*,
87+
client: Union[AuthenticatedClient, Client],
88+
) -> Response[GetModelsAllofResponse200]:
89+
"""
90+
Raises:
91+
errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
92+
httpx.TimeoutException: If the request takes longer than Client.timeout.
93+
94+
Returns:
95+
Response[GetModelsAllofResponse200]
96+
"""
97+
98+
kwargs = _get_kwargs()
99+
100+
response = await client.get_async_httpx_client().request(**kwargs)
101+
102+
return _build_response(client=client, response=response)
103+
104+
105+
async def asyncio(
106+
*,
107+
client: Union[AuthenticatedClient, Client],
108+
) -> Optional[GetModelsAllofResponse200]:
109+
"""
110+
Raises:
111+
errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
112+
httpx.TimeoutException: If the request takes longer than Client.timeout.
113+
114+
Returns:
115+
GetModelsAllofResponse200
116+
"""
117+
118+
return (
119+
await asyncio_detailed(
120+
client=client,
121+
)
122+
).parsed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,11 @@
3434
from .body_upload_file_tests_upload_post_some_object import BodyUploadFileTestsUploadPostSomeObject
3535
from .body_upload_file_tests_upload_post_some_optional_object import BodyUploadFileTestsUploadPostSomeOptionalObject
3636
from .different_enum import DifferentEnum
37+
from .extended import Extended
3738
from .free_form_model import FreeFormModel
3839
from .get_location_header_types_int_enum_header import GetLocationHeaderTypesIntEnumHeader
3940
from .get_location_header_types_string_enum_header import GetLocationHeaderTypesStringEnumHeader
41+
from .get_models_allof_response_200 import GetModelsAllofResponse200
4042
from .http_validation_error import HTTPValidationError
4143
from .import_ import Import
4244
from .json_like_body import JsonLikeBody
@@ -111,9 +113,11 @@
111113
"BodyUploadFileTestsUploadPostSomeObject",
112114
"BodyUploadFileTestsUploadPostSomeOptionalObject",
113115
"DifferentEnum",
116+
"Extended",
114117
"FreeFormModel",
115118
"GetLocationHeaderTypesIntEnumHeader",
116119
"GetLocationHeaderTypesStringEnumHeader",
120+
"GetModelsAllofResponse200",
117121
"HTTPValidationError",
118122
"Import",
119123
"JsonLikeBody",

0 commit comments

Comments
 (0)