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

[FEATURE] Prevent deserialization when required field is missing (C# Generator) #2114

Open
2 tasks done
fr-th opened this issue Oct 25, 2024 · 2 comments
Open
2 tasks done
Labels
enhancement New feature or request

Comments

@fr-th
Copy link

fr-th commented Oct 25, 2024

Why do we need this improvement?

I think that, since the c# presets for deserialization create custom JsonConverters and do not use the Deserialization methods provided by packages like Newtonsoft, we are missing some options.

For instance, you would expect fields marked as "required" in a yaml contract to be annotate with Required.Always attribute like below. This attribute would normally prevent deserialization and throw an exception because the message do not respect the contract
image

I think the generated models should behave like this because it is what you would expect from json deserialization. In the current state , required field are "ignored" and there is no way to know that a field is missing without some additional code.

How will this change help?

When deserializing incoming yaml files, if a required field is missing, you'll know immediately which one it is.
Without this, your field can be null or at default value and you either have to add custom validation or lose time debugging why your code is not behaving as intended.

Screenshots

No response

How could it be implemented/designed?

At first I would like to introduce a new CSharpOptions called enforceRequired. It would be a boolean set to false by default, to avoid breaking changes for people using the current implementation.
Since the newtonsoft attributes for deserialization are not used I would mimic the behavior and use this new 'enforceRequired' option in the Newtonsoft preset to add a check: if it is set to true and if the property is required, then I throw a JsonSerializerException if I do not find the property. In any other case it's business as usual :
image

🚧 Breaking changes

No

👀 Have you checked for similar open issues?

  • I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue?

Yes I am willing to submit a PR!

@fr-th fr-th added the enhancement New feature or request label Oct 25, 2024
@jonaslagoni
Copy link
Member

Makes sense @fr-th, let me know if you encounter any problems while creating the PR 🙏

@fr-th
Copy link
Author

fr-th commented Oct 29, 2024

@jonaslagoni I created the PR here : #2117
It's my first contribution so please don't hesitate if I missed some stuff :)

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

No branches or pull requests

2 participants