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

Error during serialization using Json serializer #68

Open
Simcon opened this issue Oct 2, 2018 · 1 comment
Open

Error during serialization using Json serializer #68

Simcon opened this issue Oct 2, 2018 · 1 comment
Labels

Comments

@Simcon
Copy link
Contributor

Simcon commented Oct 2, 2018

I've found an issue where a null value in an 'exotic' type from a third party library doesn't get deserialized correctly due to the settings for Json.NET.

I've posted a spike to my fork:

https://github.com/Simcon/memstate/commit/71870e91aad95a5a3d23bf6803b5b47b56c87501

Check out the addition of NullValueHandling = NullValueHandling.Ignore to the JsonSerializerAdapter constructor. This fixes the issue in the corresponding test.

The simple fix is to add the fix to mainline. Would this be possible, @rofr ?

The 'right' fix would be to allow specifying serialization settings but this non-trivial as far as I can see.

@rofr rofr added the bug label Oct 2, 2018
@rofr
Copy link
Member

rofr commented Oct 2, 2018

If Ignore is the sensible default, then we should go with that for sure. And it seems that this indeed is the case. If I understand correctly, null members won't be written upon serialization and then just keep defaults when being deserialized. So deserialized objects will have the same properties, use less space in the journal and marginally incur less I/O.

So the fix is trivial (one-liner) but a potential PR should include a simpler test. Feel free to have a go if you wish!

Supplying custom settings is doable, we have a pattern for this. But let's save that for later as the config api isn't frozen yet.

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

No branches or pull requests

2 participants