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

Fix bug #3 and add corresponding test case #4

Merged
merged 3 commits into from
Aug 18, 2016
Merged

Fix bug #3 and add corresponding test case #4

merged 3 commits into from
Aug 18, 2016

Conversation

blumu
Copy link
Contributor

@blumu blumu commented Aug 12, 2016

This is a quick fix for #2.
This change enables the Json.Net MissingMemberHandling option which makes the Compact deserializer strict. This means that deserialization will faill if there is any member in the Json that is not present in the F# type. This is not technically required for Compact deserialization but it is needed to make the current implementation of the BackwardConmpatible serializer pass all the unit tests. In particular without this setting the following code

type NestedStructure = { subField : int}
type NestedOptionStructure = NestedStructure option 

let x= Some {subField = 456 }
let z= Default.serialize x 
z|> Compact.deserialize<NestedOptionStructure>

would deserialize to Some {subField = 0;} instead of throwing, which would then confuse the backward-compatible deserializer.

@blumu blumu changed the title Fix bug #2 and add corresponding test case Fix bug #3 and add corresponding test case Aug 12, 2016
@blumu blumu force-pushed the wiblum/2 branch 3 times, most recently from 61d21b6 to 89f297a Compare August 16, 2016 16:26
…lead to ambiguities. (Better and more complete fix for #3)

This ambiguity occurs for instance for discriminated union or record types that happen to have a field named 'Some'
@blumu
Copy link
Contributor Author

blumu commented Aug 18, 2016

Note: there could be further ambiguities when deserializing type of the form Option<'a> where type 'a is a record or discriminated union with a field named Some. The latest iteration of this PR adds unit tests to cover this case and resolves the ambiguity by enforcing Json boxing of option types whenever the underlying type has an attribute named Some.

@blumu blumu merged commit c1181c2 into master Aug 18, 2016
@blumu blumu deleted the wiblum/2 branch August 18, 2016 18:34
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.

None yet

1 participant