-
Notifications
You must be signed in to change notification settings - Fork 352
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
added logic to treat a dynamic property that contains a json array as… #2547
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me. Requested some clarifications on a few minor comments.
...tionalTests/Microsoft.OData.Client.Tests/DataServiceContextHttpClientHandlerProviderTests.cs
Outdated
Show resolved
Hide resolved
...tionalTests/Microsoft.OData.Client.Tests/DataServiceContextHttpClientHandlerProviderTests.cs
Outdated
Show resolved
Hide resolved
...icrosoft.OData.Core.Tests/JsonLight/ODataJsonLightEntryAndFeedDeserializerUndeclaredTests.cs
Show resolved
Hide resolved
… a collection of untyped values instead of an untyped value
… explaining the change
497532c
to
60b48fa
Compare
…g that clients are not relying on exceptions that result from us not conforming with the standard
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
… a collection of untyped values instead of an untyped value
Issues
This pull request fixes #2499.
Description
*Dynamic properties that don't have an odata type are currently being parsed as untyped. The materializer assumes that these untyped properties are primitive, so the logic it uses during deserialization fails if the property is instead something like a collection. This change updates the parser to check if the dynamic property contains a json array and, if it does, treats the property as a collection of untyped values. The materializer treats this collection correctly. *
Checklist (Uncheck if it is not completed)
Additional work necessary
I discovered many issues around the treatment of open types in model parsing as well as deserialization. I will create follow-up work items for those issues.