Skip to content

Commit e9cf10e

Browse files
committed
fix: generate body_content_type param when multiple content types share the same schema
1 parent 5fcaf72 commit e9cf10e

File tree

4 files changed

+134
-1
lines changed

4 files changed

+134
-1
lines changed
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
"""Functional tests for request body handling."""
2+
3+
import pytest
4+
5+
from end_to_end_tests.functional_tests.helpers import (
6+
with_generated_client_fixture,
7+
with_generated_code_import,
8+
with_generated_code_imports,
9+
)
10+
11+
_DUPLICATE_CONTENT_TYPES_SPEC = """
12+
components:
13+
schemas:
14+
MyBody:
15+
type: object
16+
properties:
17+
name:
18+
type: string
19+
required:
20+
- name
21+
paths:
22+
/my-endpoint:
23+
post:
24+
operationId: my_endpoint
25+
requestBody:
26+
content:
27+
application/json:
28+
schema:
29+
$ref: '#/components/schemas/MyBody'
30+
application/x-www-form-urlencoded:
31+
schema:
32+
$ref: '#/components/schemas/MyBody'
33+
multipart/form-data:
34+
schema:
35+
$ref: '#/components/schemas/MyBody'
36+
responses:
37+
'200':
38+
description: Success
39+
"""
40+
41+
42+
@with_generated_client_fixture(_DUPLICATE_CONTENT_TYPES_SPEC)
43+
@with_generated_code_imports(
44+
".models.MyBody",
45+
)
46+
@with_generated_code_import(".api.default.my_endpoint._get_kwargs", alias="get_kwargs")
47+
class TestDuplicateContentTypesUseSameRef:
48+
"""When all content types in a requestBody reference the same $ref schema,
49+
the generated code should use a body_content_type parameter for dispatch
50+
instead of isinstance checks (which would all pass for the same type)."""
51+
52+
def test_defaults_to_json(self, MyBody, get_kwargs):
53+
"""Without specifying body_content_type, the default content type (first in spec) is used."""
54+
body = MyBody(name="test")
55+
result = get_kwargs(body=body)
56+
assert "json" in result, "Expected JSON body by default"
57+
assert result.get("headers", {}).get("Content-Type") == "application/json"
58+
59+
def test_form_urlencoded(self, MyBody, get_kwargs):
60+
"""Passing body_content_type='application/x-www-form-urlencoded' sends form data."""
61+
body = MyBody(name="test")
62+
result = get_kwargs(body=body, body_content_type="application/x-www-form-urlencoded")
63+
assert "data" in result, "Expected form-urlencoded body"
64+
assert result.get("headers", {}).get("Content-Type") == "application/x-www-form-urlencoded"
65+
66+
def test_multipart(self, MyBody, get_kwargs):
67+
"""Passing body_content_type='multipart/form-data' sends multipart data."""
68+
body = MyBody(name="test")
69+
result = get_kwargs(body=body, body_content_type="multipart/form-data")
70+
assert "files" in result, "Expected multipart body"
71+
72+
def test_json_and_multipart_are_exclusive(self, MyBody, get_kwargs):
73+
"""JSON and multipart dispatches must be mutually exclusive (not both applied)."""
74+
body = MyBody(name="test")
75+
json_result = get_kwargs(body=body, body_content_type="application/json")
76+
assert "files" not in json_result
77+
assert "data" not in json_result
78+
79+
multipart_result = get_kwargs(body=body, body_content_type="multipart/form-data")
80+
assert "json" not in multipart_result
81+
assert "data" not in multipart_result

openapi_python_client/parser/openapi.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,14 +483,46 @@ def iter_all_parameters(self) -> Iterator[tuple[oai.ParameterLocation, Property]
483483
yield from ((oai.ParameterLocation.HEADER, param) for param in self.header_parameters)
484484
yield from ((oai.ParameterLocation.COOKIE, param) for param in self.cookie_parameters)
485485

486+
@property
487+
def bodies_with_content_type_dispatch(self) -> bool:
488+
"""True when multiple bodies share the same Python type, making isinstance dispatch non-functional.
489+
490+
When all content types reference the same $ref schema they resolve to the same Python type.
491+
isinstance checks would all pass, causing every branch to execute (last one wins).
492+
In this case the generated code uses a ``body_content_type`` string parameter instead.
493+
"""
494+
type_strings = [body.prop.get_type_string() for body in self.bodies]
495+
return len(type_strings) != len(set(type_strings))
496+
497+
@property
498+
def unique_body_types(self) -> list[Body]:
499+
"""Bodies with duplicate Python types removed (first occurrence kept).
500+
501+
Used for type annotations when bodies_with_content_type_dispatch is True.
502+
"""
503+
seen: set[str] = set()
504+
result = []
505+
for body in self.bodies:
506+
ts = body.prop.get_type_string()
507+
if ts not in seen:
508+
seen.add(ts)
509+
result.append(body)
510+
return result
511+
486512
def list_all_parameters(self) -> list[Property]:
487513
"""Return a list of all the parameters of this endpoint"""
514+
body_props: list[Property]
515+
if self.bodies_with_content_type_dispatch:
516+
# Deduplicate: multiple bodies share a type, only emit each unique type once
517+
body_props = [body.prop for body in self.unique_body_types]
518+
else:
519+
body_props = [body.prop for body in self.bodies]
488520
return (
489521
self.path_parameters
490522
+ self.query_parameters
491523
+ self.header_parameters
492524
+ self.cookie_parameters
493-
+ [body.prop for body in self.bodies]
525+
+ body_props
494526
)
495527

496528

openapi_python_client/templates/endpoint_macros.py.jinja

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,21 @@ client: AuthenticatedClient | Client,
122122
{% if endpoint.bodies | length == 1 %}
123123
body: {{ endpoint.bodies[0].prop.get_type_string() }}{% if not endpoint.bodies[0].prop.required %} = UNSET{% endif %},
124124
{% elif endpoint.bodies | length > 1 %}
125+
{% if endpoint.bodies_with_content_type_dispatch %}
126+
body:
127+
{%- for body in endpoint.unique_body_types -%}{% set body_required = body_required and body.prop.required %}
128+
{{ body.prop.get_type_string(no_optional=True) }} {% if not loop.last %} | {% endif %}
129+
{%- endfor -%}{% if not body_required %} | Unset = UNSET{% endif %}
130+
,
131+
body_content_type: str = "{{ endpoint.bodies[0].content_type }}",
132+
{% else %}
125133
body:
126134
{%- for body in endpoint.bodies -%}{% set body_required = body_required and body.prop.required %}
127135
{{ body.prop.get_type_string(no_optional=True) }} {% if not loop.last %} | {% endif %}
128136
{%- endfor -%}{% if not body_required %} | Unset = UNSET{% endif %}
129137
,
130138
{% endif %}
139+
{% endif %}
131140
{# query parameters #}
132141
{% for parameter in endpoint.query_parameters %}
133142
{{ parameter.to_string() }},
@@ -151,6 +160,9 @@ client=client,
151160
{% endif %}
152161
{% if endpoint.bodies | length > 0 %}
153162
body=body,
163+
{% if endpoint.bodies_with_content_type_dispatch %}
164+
body_content_type=body_content_type,
165+
{% endif %}
154166
{% endif %}
155167
{% for parameter in endpoint.query_parameters %}
156168
{{ parameter.python_name }}={{ parameter.python_name }},

openapi_python_client/templates/endpoint_module.py.jinja

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,19 @@ def _get_kwargs(
4747
}
4848

4949
{% if endpoint.bodies | length > 1 %}
50+
{% if endpoint.bodies_with_content_type_dispatch %}
51+
{% for body in endpoint.bodies %}
52+
{{ "if" if loop.first else "elif" }} body_content_type == "{{ body.content_type }}":
53+
{{ body_to_kwarg(body) | indent(8) }}
54+
headers["Content-Type"] = "{{ body.content_type }}"
55+
{% endfor %}
56+
{% else %}
5057
{% for body in endpoint.bodies %}
5158
if isinstance(body, {{body.prop.get_type_string(no_optional=True) }}):
5259
{{ body_to_kwarg(body) | indent(8) }}
5360
headers["Content-Type"] = "{{ body.content_type }}"
5461
{% endfor %}
62+
{% endif %}
5563
{% elif endpoint.bodies | length == 1 %}
5664
{% set body = endpoint.bodies[0] %}
5765
{{ body_to_kwarg(body) | indent(4) }}

0 commit comments

Comments
 (0)