-
Notifications
You must be signed in to change notification settings - Fork 179
Set read untyped as string using reflection #1534
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
…ed in the next version. However, the ASP.NET Core OData library is used in OData.NET E2E test cases for ODL 8.x and ODL 9.x. To keep the existing behavior, we still need to set ReadUntypedAsString=false via reflection.
|
I don't understand why we need this -- ReadUntypedAsString="false" should be the default. That is, once we fully deprecate read untyped as string, ODL should never read untyped as string -- it should always read as structured. So why do we need to set a flag through reflection to do what should be the default? |
mikepizzo
left a comment
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.
ReadUntypedAsString=false should be the default when we remove the property from ODL. We shouldn't need to set it to false.
ASP.NET Core OData 9.x depends on ODL 8.x version, ODL 8.x version has:
I think that's why we still need 'ReadUntypedAsString="false"' in the code. |
@mikepizzo So, do you think 'https://github.com/OData/odata.net/blob/main/src/Microsoft.OData.Core/ODataMessageReaderSettings.cs#L56' is wrong? |
|
/AzurePipelines run |
|
No pipelines are associated with this pull request. |
@xuzhg -- Yes -- as part of deprecating ReadUntypedAsString, we should always default to false (that is, we should always read untyped as structured values). We should never read untyped values as raw strings, regardless the OData version. Whether or not to read as string really isn't dependent upon the version -- we just added that so that, at least for new payload versions, we would never have the problem. Deprecating was removing that behavior from all versions. |
Yes, That's the 'fix' required in ODL. Before that fix, this is the workaround to set the 'ReadUntypedAsString' = false always and we can use the latest ASP.NET Core OData version to develop ODL 8.x and ODL 9.x version. |
ODL will remove 'ReadUntypedAsString',
However, ASP.NET Core OData is used in OData.Net E2E test case which 'temporally' set the 'ReadUntypedAsString=false'.
For back-compatibility, We don't remove it completely in current version and let's use the reflection to replace the property setting directly.
In the next major version, let's remove such codes.
I don't think it's a bigger performance.
I do update the version to 9.4.1 and will use this version in the OData.NET E2E test cases.