-
Notifications
You must be signed in to change notification settings - Fork 161
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
Resolve ContentID in odata.bind annotation #643
Resolve ContentID in odata.bind annotation #643
Conversation
@gathogojr is this valid |
src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceDeserializer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataResourceDeserializer.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.AspNetCore.OData.E2E.Tests/Batch/ContentIdToLocationMappingTests.cs
Show resolved
Hide resolved
Uri resolvedId = id; | ||
|
||
string idOriginalString = id.OriginalString; | ||
if (ContentIdReferencePattern.IsMatch(idOriginalString)) |
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.
is this Regex Matching? Is it possible to use substring
or indexOf
to do the matching instead of Regex
. I remember some discussions where this Regex
was being discouraged because of performance reasons.
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.
@ElizabethOkerio We use similar logic in ODataMessageWrapper
class. We're looking for a very particular pattern here - a $
followed by an integer. I think using Substring
and Index
would result into more complex logic
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.
On an unrelated note, let's avoid substring in favor Span<char>.Slice()
where possible, to avoid string allocations.
@ElizabethOkerio Yes. Consider the sample payload below that the customer provided on the issue. You should be able to establish a parent-child relationship in the manner illustrated
|
Issues
This pull request fixes OData/odata.net#2442.
Description
Resolve ContentID in odata.bind annotation
Checklist (Uncheck if it is not completed)
Additional work necessary
If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.