Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why not rely on serde for poem_openapi::Object serializations instead of a custom implementation? #723

Open
kik4444 opened this issue Jan 8, 2024 · 3 comments
Labels
question Further information is requested

Comments

@kik4444
Copy link
Contributor

kik4444 commented Jan 8, 2024

Recently I was evaluating using poem-openapi instead of axum with utoipa, but ultimately went with the latter because of utoipa's integration with serde.

Just as an example, I have a struct which is parsed from a toml file, so it must derive serde's Deserialize. I also want to return this struct from an API in JSON format, but I want a specific field to be serialized differently to hide the actual value. Normally I would also derive serde's Serialize and put a #[serde(serialize_with = "...")] field parameter above the struct's field. Since I want to return this struct from an API and generate OpenAPI documentation for it, I also derive poem_openapi::Object.

Unfortunately, this led me to realize that Object implements its own serialization which does not rely on serde, meaning that my serialize_with was ignored.

This also means that if I have a struct which I want to serialize and deserialize with serde (e.g. between yaml, toml) and return from an API with poem-openapi, I would have to duplicate macro field parameters like #[serde(default = "abc")] with #[oai(default = "abc")] and if there is no oai equivalent like for serialize_with then I'm screwed.

So why doesn't poem_openapi::Object integrate with serde instead of doing its own thing and re-implementing some of its functionality? For example utoipa provides serde integration and if a struct field has #[serde(default)] on it then the field is marked as not required in the OpenAPI documentation (though if you want the documentation to show what the default value is you'd have to additionally write #[schema(default = "...")]).

I believe such an integration would also solve #585, at least as far as parsing is concerned. The actual OpenAPI documentation would likely still require additional impls

@kik4444 kik4444 added the question Further information is requested label Jan 8, 2024
@NexRX
Copy link
Contributor

NexRX commented Jan 13, 2024

I've wondered to if its possible but i've assumed its because some parts of serde can't be supported because they would create differences between the OpenAPI doc generated and the data serialized. E.g. #[serde(serialize_with = "...")] would change the field's serialized type and it might not be easy (assuming possible) to follow the attribute path and use the MetaSchema for the serialized type instead.

@attila-lin
Copy link
Collaborator

maybe #362 (comment)

@BatmanAoD
Copy link

It seems that the deserialize_with/serialize_with attributes effectively make it possible to make Object defer to serde; the only thing lacking, as afar as I know, is a way to opt into using serde from the "top-level" of an Object, rather than for individual fields.

@sunli829 would you be open to a PR making serialize_with/deserialize_with applicable to an entire struct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants