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

json::serialization: don't raise errors on missing attributes with default values #2302

Merged
merged 8 commits into from
Sep 1, 2016

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Aug 30, 2016

This PR fits in the ongoing work to improve deserializing from plain JSON objects.

The JSON deserialization engine no longer raises errors on missing attributes when a default value is available. Attributes may be missing because the JSON object come from a third-party API or
when loading serialized data from a previous version of the software. Default values include simple default values (var x = 4), lazy attributes and nullable types (which are set to null). This does not yet include optional attributes, more work would be needed.

The test/example can be activated when #2296 is fixed.

Signed-off-by: Alexis Laferrière <[email protected]>
@xymus xymus force-pushed the deser-default-value branch from c4e2aac to 9576ee4 Compare August 30, 2016 15:24
@privat
Copy link
Member

privat commented Aug 30, 2016

nice
+1

@Morriar
Copy link
Member

Morriar commented Aug 31, 2016

Cool, +1

@ppepos
Copy link
Contributor

ppepos commented Aug 31, 2016

Is there a possibility to add something similar to how errors are managed to know if an attribute was missing (and has been set to the default value)? I feel like being capable of knowing if the object was exactly as expected has its benefits.

@xymus
Copy link
Contributor Author

xymus commented Aug 31, 2016

@ppepos There could be a Deserializer::warnings alongside errors for such cases. It could be useful at debugging or when loading data from a previous software version to detect possible errors and warn the user. This could easily be added if there's a need for it.

Another alternative would be an annotation to change the behavior per attribute and make then non-optional. (serialize_no_default? serialize@force?) This would apply to classes where the default values are really constructors.

@ppepos
Copy link
Contributor

ppepos commented Aug 31, 2016

Option 1 seems to cover my worries.

privat added a commit that referenced this pull request Sep 1, 2016
…with default values

This PR fits in the ongoing work to improve deserializing from plain JSON objects.

The JSON deserialization engine no longer raises errors on missing attributes when a default value is available. Attributes may be missing because the JSON object come from a third-party API or
when loading serialized data from a previous version of the software. Default values include simple default values (`var x = 4`), lazy attributes and nullable types (which are set to `null`). This does not yet include `optional` attributes, more work would be needed.

The test/example can be activated when #2296 is fixed.

Pull-Request: #2302
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
@privat privat merged commit 9576ee4 into nitlang:master Sep 1, 2016
@xymus xymus deleted the deser-default-value branch September 5, 2016 00:58
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.

4 participants