From e2114f0e10d872914a16086f89368f447f822478 Mon Sep 17 00:00:00 2001 From: EnesKartal Date: Sun, 15 Nov 2026 02:16:17 +0300 Subject: [PATCH] fix: avoid orphan else in optional union JSON body transform --- .../union_property.py.jinja | 13 ++- tests/test_templates/test_endpoint_macros.py | 83 ++++++++++++++++++- 2 files changed, 93 insertions(+), 3 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 45f350f5e..f7a4d7228 100644 --- a/openapi_python_client/templates/property_templates/union_property.py.jinja +++ b/openapi_python_client/templates/property_templates/union_property.py.jinja @@ -40,7 +40,7 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri {% endmacro %} {% macro transform(property, source, destination, declare_type=True, skip_unset=False) %} -{% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required, has_if = false) %} +{% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required and not skip_unset, has_if = false) %} {% if declare_type %}{{ destination }}: {{ property.get_type_string(json=True) | as_unembedded_code }}{% endif %} {% if not property.required and not skip_unset %} @@ -59,7 +59,7 @@ if isinstance({{ source }}, Unset): {% if not ns.has_if %} if isinstance({{ source }}, {{ inner_property.get_instance_type_string() | as_unembedded_code }}): {% set ns.has_if = true %} - {% elif not loop.last or ns.contains_properties_without_transform %} + {% elif not loop.last or ns.contains_properties_without_transform or (not property.required and skip_unset) %} elif isinstance({{ source }}, {{ inner_property.get_instance_type_string() | as_unembedded_code }}): {% else %} else: @@ -67,10 +67,19 @@ else: {{ inner_template.transform(inner_property, source, destination, declare_type=False) | indent(4) }} {% endfor %} {% if ns.contains_properties_without_transform and ns.contains_modified_properties %} + {%- if not property.required and skip_unset %} +elif not isinstance({{ source }}, Unset): + {%- else %} else: + {%- endif %} {{ destination }} = {{ source }} {%- elif ns.contains_properties_without_transform %} + {%- if not property.required and skip_unset %} +if not isinstance({{ source }}, Unset): + {{ destination }} = {{ source }} + {%- else %} {{ destination }} = {{ source }} + {%- endif %} {%- endif %} {% endmacro %} diff --git a/tests/test_templates/test_endpoint_macros.py b/tests/test_templates/test_endpoint_macros.py index c601e0e30..888d70f53 100644 --- a/tests/test_templates/test_endpoint_macros.py +++ b/tests/test_templates/test_endpoint_macros.py @@ -1,6 +1,22 @@ +import ast from types import SimpleNamespace -from openapi_python_client.strings import PythonCode +from jinja2 import Environment, PackageLoader + +from openapi_python_client import TEMPLATE_FILTERS +from openapi_python_client.parser.properties import AnyProperty, DateTimeProperty, UnionProperty +from openapi_python_client.strings import PythonCode, PythonIdentifier + + +def _template_env() -> Environment: + env = Environment( + loader=PackageLoader("openapi_python_client"), + trim_blocks=True, + lstrip_blocks=True, + extensions=["jinja2.ext.loopcontrols"], + ) + env.filters.update(TEMPLATE_FILTERS) + return env def _render_docstring(env, return_string, is_detailed: bool) -> str: @@ -34,3 +50,68 @@ def test_endpoint_docstring_unwraps_python_code_return_string(env) -> None: assert "MyModel | None" in result assert "object at 0x" not in result + + +def test_json_body_optional_union_without_transform_emits_valid_python() -> None: + """Optional union JSON body with only non-transformed members must not emit an orphan else.""" + template = _template_env().get_template("endpoint_macros.py.jinja") + prop = UnionProperty( + name="body", + required=False, + default=None, + description=None, + example=None, + inner_properties=[ + AnyProperty( + name="body_type_0", + required=True, + default=None, + description=None, + example=None, + python_name=PythonIdentifier(value="body_type_0", prefix="field_"), + ) + ], + python_name=PythonIdentifier(value="body", prefix="field_"), + ) + body = SimpleNamespace(prop=prop) + + rendered = template.module.json_body(body).strip() + ast.parse(rendered) + assert rendered == 'if not isinstance(body, Unset):\n _kwargs["json"] = body' + + +def test_json_body_optional_union_with_mixed_transform_guards_unset() -> None: + """Optional union JSON body with transformed and non-transformed members must not fall into else for Unset.""" + template = _template_env().get_template("endpoint_macros.py.jinja") + prop = UnionProperty( + name="body", + required=False, + default=None, + description=None, + example=None, + inner_properties=[ + DateTimeProperty( + name="body_type_0", + required=True, + default=None, + description=None, + example=None, + python_name=PythonIdentifier(value="body_type_0", prefix="field_"), + ), + AnyProperty( + name="body_type_1", + required=True, + default=None, + description=None, + example=None, + python_name=PythonIdentifier(value="body_type_1", prefix="field_"), + ), + ], + python_name=PythonIdentifier(value="body", prefix="field_"), + ) + body = SimpleNamespace(prop=prop) + + rendered = template.module.json_body(body).strip() + ast.parse(rendered) + assert "elif not isinstance(body, Unset):" in rendered + assert "\nelse:" not in rendered