Skip to content

Commit f5c7f53

Browse files
🐛 Fix serializing collection request body.
1 parent 1c2ae70 commit f5c7f53

File tree

4 files changed

+191
-23
lines changed

4 files changed

+191
-23
lines changed

ChangeLog.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,16 @@ All notable changes to this project will be documented in this file.
55
This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html),
66
and the format of this file is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
77

8+
89
## [Unreleased]
910

1011
### Added
11-
1212
- Accept session_factory in `ClientBase.__init__`.
13-
- Python 3.13 is not supported (after upgrading mimeparse to 2.0.0rc1).
13+
14+
### Fixed
15+
- Handling collections in request bodies.
16+
- Dropped dependency on std package cgi, which allows running under python 3.13 .
17+
1418

1519
## [0.11.0] - 2024-08-13
1620

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Serializing request body
2+
3+
## The Current solution (0.11.0)
4+
5+
Currently lapidary checks type of value of the body parameter and tries to match against the type map passed to the `Body` annotation.
6+
7+
Problem starts when body is a collection: `type()` returns the type of only the collection (e.g. `list`), so the type matching fails.
8+
9+
## Possible alternatives
10+
11+
1. Check the type of the first item, but there's never a guarantee that the passed collection is homogenic.
12+
Both JSON Schema and python typing support heterogenic collections.
13+
14+
2. Check type of all items is out of the question for performance reasons, and pydantic does it anyway during serialization.
15+
16+
3. Try to serialize the value with a TypeAdapter for each type in the type map. The first successful attempt also determines the body content type.
17+
4. Either accept extra parameter `body_type: type` or accept body as tuple with the type explicitly declared: `body: T | Union[T, type]`.
18+
19+
The last two solutions seem feasible.
20+
Trying every type would incur a performance hit for unions of complex types, but
21+
- it would handle simpler cases well,
22+
- keep lapidary compatible with lapidary-render,
23+
24+
Lapidary could still accept optional type parameter but use the other method as a fallback for when user doesn't pass the type.
25+
26+
# Accepted solution
27+
28+
Lapidary will not check the type of body value, instead it will try serializing it with every type mentioned in the type map in `Body` annotation.
29+
30+
Lapidary should implement an optional explicit body type parameter in a future version, either as a separate parameter, or as a tuple together with the body value.

src/lapidary/runtime/model/request.py

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from ..annotations import Body, Cookie, Header, Metadata, Param, Path, Query, WebArg
1313
from ..http_consts import ACCEPT, CONTENT_TYPE, MIME_JSON
1414
from ..metattype import is_array_like, make_not_optional
15-
from ..mime import find_mime
1615
from ..types_ import Dumper, MimeType, RequestFactory, SecurityRequirements
1716
from .annotations import (
1817
find_annotation,
@@ -23,6 +22,9 @@
2322
if typing.TYPE_CHECKING:
2423
from ..client_base import ClientBase
2524
from ..operation import Operation
25+
import logging
26+
27+
logger = logging.getLogger(__name__)
2628

2729

2830
@dc.dataclass
@@ -174,37 +176,41 @@ def update_builder(self, builder: RequestBuilder, free_params: Mapping[str, typi
174176

175177
@dc.dataclass
176178
class BodyContributor:
177-
dumpers: Mapping[type, Mapping[MimeType, Dumper]]
179+
serializers: list[tuple[pydantic.TypeAdapter, str]]
178180

179181
def update_builder(self, builder: 'RequestBuilder', value: typing.Any, media_type: MimeType = MIME_JSON) -> None:
180-
matched_media_type, dump = self.find_dumper(type(value), media_type)
182+
matched_media_type, content = self._dump(value, media_type)
181183
builder.headers[CONTENT_TYPE] = matched_media_type
182-
builder.content = dump(value)
184+
builder.content = content
183185

184-
def find_dumper(self, typ: type, media_type: MimeType) -> tuple[MimeType, Dumper]:
185-
for base in typ.__mro__[:-1]:
186+
def _dump(self, value: typing.Any, media_type: MimeType = MIME_JSON):
187+
for type_adapter, media_type_ in self.serializers:
188+
if not BodyContributor._media_matches(media_type_, media_type):
189+
logger.debug('Ignoring unsupported media_type: %s', media_type)
190+
continue
186191
try:
187-
media_dumpers = self.dumpers[base]
188-
matched_media_type = find_mime(media_dumpers.keys(), media_type)
189-
if not matched_media_type:
190-
raise ValueError('Unsupported media_type', media_type)
191-
return matched_media_type, media_dumpers[matched_media_type]
192-
except KeyError:
192+
raw = type_adapter.dump_json(value, exclude_unset=True, by_alias=True)
193+
return media_type_, raw
194+
except pydantic.ValidationError:
193195
continue
194-
raise TypeError(f'Unsupported type: {typ.__name__}')
196+
else:
197+
raise ValueError('Unsupported value')
195198

196199
@classmethod
197200
def for_parameter(cls, annotation: type) -> typing.Self:
198-
dumpers: dict[type, dict[MimeType, Dumper]] = {}
199201
body: Body
200202
_, body = find_annotation(annotation, Body)
201-
for media_type, typ in body.content.items():
202-
m_type, m_subtype, _ = mimeparse.parse_media_range(media_type)
203-
if f'{m_type}/{m_subtype}' != MIME_JSON:
204-
raise TypeError(f'Unsupported media type: {media_type}')
205-
type_dumpers = dumpers.setdefault(typ, {})
206-
type_dumpers[media_type] = mk_pydantic_dumper(typ)
207-
return cls(dumpers=dumpers)
203+
serializers = [
204+
(pydantic.TypeAdapter(python_type), media_type)
205+
for media_type, python_type in body.content.items()
206+
if BodyContributor._media_matches(media_type)
207+
]
208+
return cls(serializers)
209+
210+
@staticmethod
211+
def _media_matches(media_type: str, match: str = MIME_JSON) -> bool:
212+
m_type, m_subtype, _ = mimeparse.parse_media_range(media_type)
213+
return f'{m_type}/{m_subtype}' == match
208214

209215

210216
@dc.dataclass

tests/test_body_serialization.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
from typing import Annotated, Generic, Optional, Union
2+
3+
from client import ClientTestBase
4+
from httpx import AsyncClient
5+
from typing_extensions import Self, TypeVar
6+
7+
from lapidary.runtime import Body, ModelBase, Responses, get
8+
from lapidary.runtime.model.op import process_operation_method
9+
10+
11+
class BodyModel(ModelBase):
12+
a: Optional[str]
13+
14+
15+
def test_serialize_str():
16+
class Client(ClientTestBase):
17+
def op(self: Self, body: Annotated[str, Body({'application/json': str})]) -> Annotated[None, Responses({})]:
18+
pass
19+
20+
client = ClientTestBase(AsyncClient())
21+
22+
adapter, response = process_operation_method(Client.op, get('/path'))
23+
request, auth = adapter.build_request(client, dict(body='a'))
24+
25+
assert request.content == b'"a"'
26+
27+
28+
def test_serialize_obj():
29+
class Client(ClientTestBase):
30+
def op(self: Self, body: Annotated[BodyModel, Body({'application/json': BodyModel})]) -> Annotated[None, Responses({})]:
31+
pass
32+
33+
client = ClientTestBase(AsyncClient())
34+
35+
adapter, response = process_operation_method(Client.op, get('/path'))
36+
request, auth = adapter.build_request(client, dict(body=BodyModel(a='a')))
37+
38+
assert request.content == b'{"a":"a"}'
39+
40+
41+
def test_serialize_list():
42+
class Client(ClientTestBase):
43+
def op(self: Self, body: Annotated[list[BodyModel], Body({'application/json': list[BodyModel]})]) -> Annotated[None, Responses({})]:
44+
pass
45+
46+
client = ClientTestBase(AsyncClient())
47+
48+
adapter, response = process_operation_method(Client.op, get('/path'))
49+
request, auth = adapter.build_request(client, dict(body=[BodyModel(a='a')]))
50+
51+
assert request.content == b'[{"a":"a"}]'
52+
53+
54+
T = TypeVar('T')
55+
56+
57+
class GenericBodyModel(ModelBase, Generic[T]):
58+
a: T
59+
60+
61+
def test_serialize_generic_str():
62+
class Client(ClientTestBase):
63+
def op(
64+
self: Self, body: Annotated[list[GenericBodyModel[str]], Body({'application/json': list[GenericBodyModel[str]]})]
65+
) -> Annotated[None, Responses({})]:
66+
pass
67+
68+
client = ClientTestBase(AsyncClient())
69+
70+
adapter, response = process_operation_method(Client.op, get('/path'))
71+
request, auth = adapter.build_request(client, dict(body=[GenericBodyModel(a='a')]))
72+
73+
assert request.content == b'[{"a":"a"}]'
74+
75+
76+
def test_serialize_generic_int():
77+
class Client(ClientTestBase):
78+
def op(
79+
self: Self, body: Annotated[list[GenericBodyModel[int]], Body({'application/json': list[GenericBodyModel[int]]})]
80+
) -> Annotated[None, Responses({})]:
81+
pass
82+
83+
client = ClientTestBase(AsyncClient())
84+
85+
adapter, response = process_operation_method(Client.op, get('/path'))
86+
request, auth = adapter.build_request(client, dict(body=[GenericBodyModel(a=1)]))
87+
88+
assert request.content == b'[{"a":1}]'
89+
90+
91+
def test_serialize_generic_obj():
92+
class Client(ClientTestBase):
93+
def op(
94+
self: Self, body: Annotated[list[GenericBodyModel[BodyModel]], Body({'application/json': list[GenericBodyModel[BodyModel]]})]
95+
) -> Annotated[None, Responses({})]:
96+
pass
97+
98+
client = ClientTestBase(AsyncClient())
99+
100+
adapter, response = process_operation_method(Client.op, get('/path'))
101+
request, auth = adapter.build_request(client, dict(body=[GenericBodyModel(a=BodyModel(a='a'))]))
102+
103+
assert request.content == b'[{"a":{"a":"a"}}]'
104+
105+
106+
def test_serialize_generic_union():
107+
class Client(ClientTestBase):
108+
def op(
109+
self: Self,
110+
body: Annotated[
111+
Union[list[GenericBodyModel[BodyModel]], GenericBodyModel[BodyModel], BodyModel],
112+
Body({'application/json': Union[list[GenericBodyModel[BodyModel]], GenericBodyModel[BodyModel], BodyModel]}),
113+
],
114+
) -> Annotated[None, Responses({})]:
115+
pass
116+
117+
client = ClientTestBase(AsyncClient())
118+
119+
adapter, _ = process_operation_method(Client.op, get('/path'))
120+
121+
request, _ = adapter.build_request(client, dict(body=[GenericBodyModel(a=BodyModel(a='a'))]))
122+
assert request.content == b'[{"a":{"a":"a"}}]'
123+
124+
request, _ = adapter.build_request(client, dict(body=GenericBodyModel(a=BodyModel(a='a'))))
125+
assert request.content == b'{"a":{"a":"a"}}'
126+
127+
request, _ = adapter.build_request(client, dict(body=BodyModel(a='a')))
128+
assert request.content == b'{"a":"a"}'

0 commit comments

Comments
 (0)