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

Serde deserialize support #205

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

farazfazli
Copy link

@farazfazli farazfazli commented Mar 23, 2023

Closes #203. WIP.

@farazfazli farazfazli marked this pull request as draft March 23, 2023 04:30
@farazfazli
Copy link
Author

I believe the struct fields that aren't being deserialized will have to be wrapped with Option, to ensure that deserialization is performed properly. We can't use #[serde(skip_deserializing)] because certain types which lack a default implementation are defined in a different package.

One example is of this is OffsetDateTime.

@farazfazli
Copy link
Author

@LouisGariepy Please let me know your thoughts on this whenever you get a chance.

@LouisGariepy
Copy link
Member

Hi Faraz! Sorry for the delay.

I do believe it would be possible to implement Deserialize for the "owned" variant of generated types.

I'm not sure about this comment though

I believe the struct fields that aren't being deserialized will have to be wrapped with Option, to ensure that deserialization is performed properly. We can't use #[serde(skip_deserializing)] because certain types which lack a default implementation are defined in a different package.

Could you clarify what you mean by "fields that aren't being deserialized"? Do you mean that you would want you api to accept missing fields?

In the structs generated by Cornucopia, Option means that the attribute is nullable in the database. We can't make fields optional based on whether they are Default or not.

In my opinion, the Serialize and Deserialize implementations are provided as conveniences for simple cases. If you need more complex Serialization/Deserialization (for example, making some fields skippable during deserialization), you should consider:

  1. Making a deserialization wrapper that handles the deserialization logic, or
  2. Marking the relevant types as nullable in your database.

@farazfazli Maybe I'm misunderstood what you meant though. Please let me know and don't hesitate to correct me if that's the case.

Thanks for your contributions!

@Occuros
Copy link

Occuros commented Sep 3, 2024

Is this PR still considered?

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

Successfully merging this pull request may close these issues.

Flag for generating Serde deserialize trait
3 participants