From 35a54efbbc2e25c7a474d5172bce1d84e3f8bd3f Mon Sep 17 00:00:00 2001 From: Jake Selig Date: Tue, 21 Jun 2022 16:36:34 -0600 Subject: [PATCH 1/3] fix: explicitly track first if-branch usage in union template --- .../templates/property_templates/union_property.py.jinja | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openapi_python_client/templates/property_templates/union_property.py.jinja b/openapi_python_client/templates/property_templates/union_property.py.jinja index 8a7d506d6..62ad57409 100644 --- a/openapi_python_client/templates/property_templates/union_property.py.jinja +++ b/openapi_python_client/templates/property_templates/union_property.py.jinja @@ -66,10 +66,13 @@ elif {{ source }} is None: {% else %} {% set ns.contains_modified_properties = true %} {% endif %} + {% set has_first_clause = false %} {% if loop.first and property.required and not property.nullable %}{# No if UNSET or if None statement before this #} if isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}): + {% set has_first_clause = true %} {% elif not loop.last or ns.contains_properties_without_transform %} -elif isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}): +{% if not has_first_clause %}if{% else %}elif{% endif %} isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}): + {% set has_first_clause = true %} {% else %} else: {% endif %} From 11a561bd6221b1293cc07122c01284dc06de74f9 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sat, 2 Jul 2022 17:19:08 -0600 Subject: [PATCH 2/3] fix: Template logic for tracking ifs in unions --- .../property_templates/union_property.py.jinja | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/openapi_python_client/templates/property_templates/union_property.py.jinja b/openapi_python_client/templates/property_templates/union_property.py.jinja index 62ad57409..fb686d0c6 100644 --- a/openapi_python_client/templates/property_templates/union_property.py.jinja +++ b/openapi_python_client/templates/property_templates/union_property.py.jinja @@ -40,24 +40,26 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri {% endmacro %} {% macro transform(property, source, destination, declare_type=True, multipart=False) %} +{% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required, has_if = false) %} {% if not property.required or property.nullable %} {{ destination }}{% if declare_type %}: {{ property.get_type_string(json=True) }}{% endif %} {% if not property.required %} if isinstance({{ source }}, Unset): {{ destination }} = UNSET + {% set ns.has_if = true %} {% endif %} {% endif %} {% if property.nullable %} - {% if property.required %} -if {{ source }} is None: - {% else %}{# There's an if UNSET statement before this #} + {% if ns.has_if %} elif {{ source }} is None: + {% else %} +if {{ source }} is None: + {% set ns.has_if = true %} {% endif %} {{ destination }} = None {% endif %} -{% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required) %} {% for inner_property in property.inner_properties %} {% import "property_templates/" + inner_property.template as inner_template %} {% if not inner_template.transform %} @@ -66,12 +68,11 @@ elif {{ source }} is None: {% else %} {% set ns.contains_modified_properties = true %} {% endif %} - {% set has_first_clause = false %} - {% if loop.first and property.required and not property.nullable %}{# No if UNSET or if None statement before this #} + {% if not ns.has_if %} if isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}): - {% set has_first_clause = true %} + {% set ns.has_if = true %} {% elif not loop.last or ns.contains_properties_without_transform %} -{% if not has_first_clause %}if{% else %}elif{% endif %} isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}): +elif isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}): {% set has_first_clause = true %} {% else %} else: From cea08ca5fdfe588ab6d1302df9807c9ded073b5a Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sat, 2 Jul 2022 17:39:13 -0600 Subject: [PATCH 3/3] fix: More bad template logic --- .../my_test_api_client/api/__init__.py | 5 + .../api/responses/__init__.py | 14 +++ .../api/responses/__init__.py | 0 ..._responses_unions_simple_before_complex.py | 118 ++++++++++++++++++ .../api/tests/defaults_tests_defaults_post.py | 2 + .../api/tests/get_user_list.py | 2 + .../my_test_api_client/models/__init__.py | 4 + .../my_test_api_client/models/a_model.py | 3 + ...ions_simple_before_complex_response_200.py | 79 ++++++++++++ ...ple_before_complex_response_200a_type_1.py | 44 +++++++ end_to_end_tests/openapi.json | 27 ++++ .../templates/model.py.jinja | 2 +- .../union_property.py.jinja | 4 +- 13 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/api/responses/__init__.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py index 3295d210c..cd12c2407 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py @@ -5,6 +5,7 @@ from .default import DefaultEndpoints from .location import LocationEndpoints from .parameters import ParametersEndpoints +from .responses import ResponsesEndpoints from .tag1 import Tag1Endpoints from .tests import TestsEndpoints from .true_ import True_Endpoints @@ -15,6 +16,10 @@ class MyTestApiClientApi: def tests(cls) -> Type[TestsEndpoints]: return TestsEndpoints + @classmethod + def responses(cls) -> Type[ResponsesEndpoints]: + return ResponsesEndpoints + @classmethod def default(cls) -> Type[DefaultEndpoints]: return DefaultEndpoints diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py new file mode 100644 index 000000000..c2e39c16a --- /dev/null +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py @@ -0,0 +1,14 @@ +""" Contains methods for accessing the API Endpoints """ + +import types + +from . import post_responses_unions_simple_before_complex + + +class ResponsesEndpoints: + @classmethod + def post_responses_unions_simple_before_complex(cls) -> types.ModuleType: + """ + Regression test for #603 + """ + return post_responses_unions_simple_before_complex diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/responses/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/api/responses/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py b/end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py new file mode 100644 index 000000000..9dd058470 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py @@ -0,0 +1,118 @@ +from typing import Any, Dict, Optional + +import httpx + +from ...client import Client +from ...models.post_responses_unions_simple_before_complex_response_200 import ( + PostResponsesUnionsSimpleBeforeComplexResponse200, +) +from ...types import Response + + +def _get_kwargs( + *, + client: Client, +) -> Dict[str, Any]: + url = "{}/responses/unions/simple_before_complex".format(client.base_url) + + headers: Dict[str, str] = client.get_headers() + cookies: Dict[str, Any] = client.get_cookies() + + return { + "method": "post", + "url": url, + "headers": headers, + "cookies": cookies, + "timeout": client.get_timeout(), + } + + +def _parse_response(*, response: httpx.Response) -> Optional[PostResponsesUnionsSimpleBeforeComplexResponse200]: + if response.status_code == 200: + response_200 = PostResponsesUnionsSimpleBeforeComplexResponse200.from_dict(response.json()) + + return response_200 + return None + + +def _build_response(*, response: httpx.Response) -> Response[PostResponsesUnionsSimpleBeforeComplexResponse200]: + return Response( + status_code=response.status_code, + content=response.content, + headers=response.headers, + parsed=_parse_response(response=response), + ) + + +def sync_detailed( + *, + client: Client, +) -> Response[PostResponsesUnionsSimpleBeforeComplexResponse200]: + """Regression test for #603 + + Returns: + Response[PostResponsesUnionsSimpleBeforeComplexResponse200] + """ + + kwargs = _get_kwargs( + client=client, + ) + + response = httpx.request( + verify=client.verify_ssl, + **kwargs, + ) + + return _build_response(response=response) + + +def sync( + *, + client: Client, +) -> Optional[PostResponsesUnionsSimpleBeforeComplexResponse200]: + """Regression test for #603 + + Returns: + Response[PostResponsesUnionsSimpleBeforeComplexResponse200] + """ + + return sync_detailed( + client=client, + ).parsed + + +async def asyncio_detailed( + *, + client: Client, +) -> Response[PostResponsesUnionsSimpleBeforeComplexResponse200]: + """Regression test for #603 + + Returns: + Response[PostResponsesUnionsSimpleBeforeComplexResponse200] + """ + + kwargs = _get_kwargs( + client=client, + ) + + async with httpx.AsyncClient(verify=client.verify_ssl) as _client: + response = await _client.request(**kwargs) + + return _build_response(response=response) + + +async def asyncio( + *, + client: Client, +) -> Optional[PostResponsesUnionsSimpleBeforeComplexResponse200]: + """Regression test for #603 + + Returns: + Response[PostResponsesUnionsSimpleBeforeComplexResponse200] + """ + + return ( + await asyncio_detailed( + client=client, + ) + ).parsed diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py index 6bcd59c8c..6c30b939d 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py +++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py @@ -51,6 +51,8 @@ def _get_kwargs( params["list_prop"] = json_list_prop + json_union_prop: Union[float, str] + json_union_prop = union_prop params["union_prop"] = json_union_prop diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py index 29dd706e8..e4ed22231 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py +++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py @@ -47,6 +47,8 @@ def _get_kwargs( params["an_enum_value_with_only_null"] = json_an_enum_value_with_only_null + json_some_date: str + if isinstance(some_date, datetime.date): json_some_date = some_date.isoformat() else: diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py index 85e5243ec..f34a171f6 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py @@ -40,6 +40,10 @@ from .model_with_union_property_inlined_fruit_type_0 import ModelWithUnionPropertyInlinedFruitType0 from .model_with_union_property_inlined_fruit_type_1 import ModelWithUnionPropertyInlinedFruitType1 from .none import None_ +from .post_responses_unions_simple_before_complex_response_200 import PostResponsesUnionsSimpleBeforeComplexResponse200 +from .post_responses_unions_simple_before_complex_response_200a_type_1 import ( + PostResponsesUnionsSimpleBeforeComplexResponse200AType1, +) from .test_inline_objects_json_body import TestInlineObjectsJsonBody from .test_inline_objects_response_200 import TestInlineObjectsResponse200 from .validation_error import ValidationError diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py index d52001229..0cf302e56 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py @@ -71,6 +71,8 @@ def to_dict(self) -> Dict[str, Any]: an_allof_enum_with_overridden_default = self.an_allof_enum_with_overridden_default.value + a_camel_date_time: str + if isinstance(self.a_camel_date_time, datetime.datetime): a_camel_date_time = self.a_camel_date_time.isoformat() @@ -79,6 +81,7 @@ def to_dict(self) -> Dict[str, Any]: a_date = self.a_date.isoformat() required_not_nullable = self.required_not_nullable + one_of_models: Union[Any, Dict[str, Any]] if isinstance(self.one_of_models, FreeFormModel): one_of_models = self.one_of_models.to_dict() diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py new file mode 100644 index 000000000..6ce78fcd5 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py @@ -0,0 +1,79 @@ +from typing import Any, Dict, List, Type, TypeVar, Union, cast + +import attr + +from ..models.post_responses_unions_simple_before_complex_response_200a_type_1 import ( + PostResponsesUnionsSimpleBeforeComplexResponse200AType1, +) + +T = TypeVar("T", bound="PostResponsesUnionsSimpleBeforeComplexResponse200") + + +@attr.s(auto_attribs=True) +class PostResponsesUnionsSimpleBeforeComplexResponse200: + """ + Attributes: + a (Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str]): + """ + + a: Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str] + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + a: Union[Dict[str, Any], str] + + if isinstance(self.a, PostResponsesUnionsSimpleBeforeComplexResponse200AType1): + a = self.a.to_dict() + + else: + a = self.a + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update( + { + "a": a, + } + ) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + + def _parse_a(data: object) -> Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str]: + try: + if not isinstance(data, dict): + raise TypeError() + a_type_1 = PostResponsesUnionsSimpleBeforeComplexResponse200AType1.from_dict(data) + + return a_type_1 + except: # noqa: E722 + pass + return cast(Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str], data) + + a = _parse_a(d.pop("a")) + + post_responses_unions_simple_before_complex_response_200 = cls( + a=a, + ) + + post_responses_unions_simple_before_complex_response_200.additional_properties = d + return post_responses_unions_simple_before_complex_response_200 + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py new file mode 100644 index 000000000..6b9ae2484 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py @@ -0,0 +1,44 @@ +from typing import Any, Dict, List, Type, TypeVar + +import attr + +T = TypeVar("T", bound="PostResponsesUnionsSimpleBeforeComplexResponse200AType1") + + +@attr.s(auto_attribs=True) +class PostResponsesUnionsSimpleBeforeComplexResponse200AType1: + """ """ + + additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + post_responses_unions_simple_before_complex_response_200a_type_1 = cls() + + post_responses_unions_simple_before_complex_response_200a_type_1.additional_properties = d + return post_responses_unions_simple_before_complex_response_200a_type_1 + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index b958e530b..dc74e033c 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -713,6 +713,33 @@ } } }, + "/responses/unions/simple_before_complex": { + "post": { + "tags": ["responses"], + "description": "Regression test for #603", + "responses": { + "200": { + "description": "A union with simple types before complex ones.", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["a"], + "properties": { + "a": { + "oneOf": [ + {"type": "string"}, + {"type": "object"} + ] + } + } + } + } + } + } + } + } + }, "/auth/token_with_cookie": { "get": { "tags": [ diff --git a/openapi_python_client/templates/model.py.jinja b/openapi_python_client/templates/model.py.jinja index 07f929d66..dc033c41a 100644 --- a/openapi_python_client/templates/model.py.jinja +++ b/openapi_python_client/templates/model.py.jinja @@ -85,7 +85,7 @@ field_dict: Dict[str, Any] = {} {% endif %} {% if prop_template and prop_template.transform %} for prop_name, prop in self.additional_properties.items(): - {{ prop_template.transform(model.additional_properties, "prop", "field_dict[prop_name]", multipart=multipart) | indent(4) }} + {{ prop_template.transform(model.additional_properties, "prop", "field_dict[prop_name]", multipart=multipart, declare_type=false) | indent(4) }} {% elif multipart %} field_dict.update({ key: (None, str(value).encode(), "text/plain") diff --git a/openapi_python_client/templates/property_templates/union_property.py.jinja b/openapi_python_client/templates/property_templates/union_property.py.jinja index fb686d0c6..039709369 100644 --- a/openapi_python_client/templates/property_templates/union_property.py.jinja +++ b/openapi_python_client/templates/property_templates/union_property.py.jinja @@ -41,15 +41,13 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri {% macro transform(property, source, destination, declare_type=True, multipart=False) %} {% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required, has_if = false) %} -{% if not property.required or property.nullable %} -{{ destination }}{% if declare_type %}: {{ property.get_type_string(json=True) }}{% endif %} +{% if declare_type %}{{ destination }}: {{ property.get_type_string(json=True) }}{% endif %} {% if not property.required %} if isinstance({{ source }}, Unset): {{ destination }} = UNSET {% set ns.has_if = true %} {% endif %} -{% endif %} {% if property.nullable %} {% if ns.has_if %} elif {{ source }} is None: