-
-
Notifications
You must be signed in to change notification settings - Fork 86
More on JSON serialization #71
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
Conversation
…Fixed item list unit test to reflect new semantics.
|
We use a tool (Schema.NET.Tool) to generate all Schema.NET types from the schema.org schema file that we download. Changing the enums manually and re-ordering the types (i.e. IThing) is going to get overwritten. Take a look at |
…unit tests to reflect semantics.
|
@RehanSaeed Actually, I'd already fixed up Enumeration.cs to generate the enums like that (maybe a week ago or so?) but just needed to unit test the changes, so did them manually for now, knowing they'll get overwritten when you next run the tool. I see how ItemListElement matched schema.org, and maybe that's just a glitch with the order they've defined things. In most cases, the most "specific" interface match is listed last, but in this case, it's first. I've reverted it back to the original (schema.org) order, and fixed up the related unit tests, to look for IThing not IListItem. So, now if you stick a IListItem in ItemListElement, it'll come back out as an IThing - not a IListItem. Not optimal, but it seems like that is the only odd case. Currently we try and assign the best match from right-to-left (or last-to-first, in schema.org terms). Another approach may be to keep a "weight" on each type, and sort the Values<> types by priority, and try to match in that order. So any primitive types are tried last, and any interface at the top of the type hierarchy is tried first. Not sure if this overcomplicates things though? |
…rray deserialization always returns list of interfaces not class instances.
|
I made some changes which introduced a merge conflict. I used the new GitHub feature to resolve it which was cool. I saw your comment in #75. Are you planning on more changes? |
|
Nope, no more changes. All checked in.
…On Fri, Jun 28, 2019 at 3:30 AM Muhammad Rehan Saeed < ***@***.***> wrote:
I made some changes which introduced a merge conflict. I used the new
GitHub feature to resolve it which was cool. I saw your comment in #75
<#75>. Are you planning on
more changes?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#71?email_source=notifications&email_token=ADHW7NUXL2DETEJWMMVYUDLP4XR4RA5CNFSM4H3EQ5M2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYZW4WA#issuecomment-506687064>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADHW7NQAD4242N5EKQF4XLTP4XR4RANCNFSM4H3EQ5MQ>
.
|
…oblems anyway). VS added nuget.config.
|
Noticed a build failure with previous checkin, so fixed that. Now, no more changes planned. Would love to get a formal release of this soon. |
|
Merged. Thanks for your hard work! |
I've added in all the StringEnumConverter attributes, so everything serializes properly.
Also, I've flipped IThing and IListItem in the Values<> of ListItem, since with this new behavior it should deserialize as the most specific type (right to left).
I had to fix the ListItem test for the new semantics where it won't deserialize duplicate objects. All the other unit tests still worked fine, so I think this will be OK.
I refactored the default JSON serializer settings into TestDefaults class just to simplify things a bit.