asyncapi / modelina

A library for generating typed models based on inputs such as AsyncAPI, OpenAPI, and JSON Schema documents with high customization
https://modelina.org
Apache License 2.0
312 stars 182 forks source link

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

Open fr-th opened 5 days ago

fr-th commented 5 days ago

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?

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue?

Yes I am willing to submit a PR!

jonaslagoni commented 5 days ago

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

fr-th commented 2 days ago

@jonaslagoni I created the PR here : https://github.com/asyncapi/modelina/pull/2117 It's my first contribution so please don't hesitate if I missed some stuff :)