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

Implemented ability to deserialize discriminated unions regardless of union tag position #95

Merged
merged 6 commits into from
May 26, 2021

Conversation

xperiandri
Copy link
Contributor

I create a snapshot of a reader (a copy of structure) and parse it into a JsonDocument (which is a lazy operation) and do a union case lookup.
I have not added new tests but played with modification of existing (not committed) so add tests as appropriate.

Fixes #93

@xperiandri
Copy link
Contributor Author

Unfortunately, I was not managed to design changes that would allow doing ValueEquals because readExternalTag requires reading a field name. But all the other cases operate with JsonElement and its value.
Hence I had to get a string instance.

@xperiandri
Copy link
Contributor Author

@Tarmil could you have a look?

@xperiandri
Copy link
Contributor Author

@Tarmil

@Tarmil
Copy link
Owner

Tarmil commented Apr 16, 2021

I've been pretty busy this week but I'll take a look this weekend.

@Tarmil
Copy link
Owner

Tarmil commented Apr 18, 2021

This looks pretty good! I was unsure whether saving and restoring a Utf8JsonReader like this would work, but it seems officially supported, so that's nice.

I added some tests, and fixed an issue in the AdjacentTag case. I'll definitely add a flag to determine whether to use it or not, because despite the low allocation we're still re-parsing the object, and some people may want to ensure they don't do that. But I think I'll make it enabled by default.

@xperiandri
Copy link
Contributor Author

Cool! Thanks!

@xperiandri
Copy link
Contributor Author

@Tarmil is there something left preventing this to be merged?

@Tarmil
Copy link
Owner

Tarmil commented May 26, 2021

Just my schedule 🙂 I'll add the flag and release it now.

@Tarmil Tarmil merged commit dfc6eb4 into Tarmil:master May 26, 2021
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.

Union deserialiser cannot handle the internal tag not being the first property
2 participants