-
Notifications
You must be signed in to change notification settings - Fork 358
Fixes #3454: support the index after the collection in the path #3462
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
base: dev-9.x
Are you sure you want to change the base?
Conversation
|
/AzurePipelines run |
| /// <exception cref="System.ArgumentNullException">Throws if the input segment is null.</exception> | ||
| public override IEdmNavigationSource Translate(IndexSegment segment) | ||
| { | ||
| ExceptionUtils.CheckArgumentNotNull(segment, "segment"); |
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.
Why are we returning null here. #Pending
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.
I am think any reason to return the navigation source for a single item of collection?
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.
#Won't Fix to keep it returning null now. We can fix it if we do need the Navigation source for an index segment. But, so far, it's not.
| } | ||
|
|
||
| if (previousSegment is PropertySegment propertySegment && | ||
| long.TryParse(segmentText, out var index)) |
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.
| long.TryParse(segmentText, out var index)) | |
| long.TryParse(segmentText, out long index)) |
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.
#Resolved.
| BatchSegment segment3 = BatchSegment.Instance; | ||
| Assert.False(segment1.Equals(segment2)); | ||
| Assert.False(segment1.Equals(segment3)); | ||
| } |
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.
You could add a test for index -1, ^1
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.
Actually, in the ODataUriParserTests, there are -1 tests added.
Of course, I can add more here.
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.
#Resolved.
| /// </summary> | ||
| /// <param name="index">The index value.</param> | ||
| /// <param name="targetEdmType">The target type of the segment.</param> | ||
| public IndexSegment(long index, IEdmType targetEdmType) |
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.
Are we supporting From-end indexing with Index (^) #WontFix
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.
it's minus syntax.
for example: -1 means the last one.
So far, OData spec doesn't mention about "^" syntax.
|
/AzurePipelines run |
|
/AzurePipelines run |
Issues
This pull request fixes #3454.
Description
https://docs.oasis-open.org/odata/odata/v4.02/csd01/part1-protocol/odata-v4.02-csd01-part1-protocol.html#RequestinganIndividualMemberofanOrderedCollection
11.2.6.8 Requesting an Individual Member of an Ordered Collection
Individual members of collections of primitive and complex types annotated with the Ordered term (see OData-VocCore) are addressable by appending a segment containing the zero-based ordinal to the URL of the collection. A negative ordinal indexes from the end of the collection, with -1 representing the last item in the collection.
Entities are stably addressable using their canonical URL and are not accessible using an ordinal index.
Example 64: the first address in a list of addresses for MainSupplier
GET http://host/service/MainSupplier/Addresses/0
This PR is to fill the gap.
It's not a breaking change.
Briefly describe the changes of this pull request.
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.
Repository notes
Team members can start a CI build by adding a comment with the text
/AzurePipelines runto a PR. A bot may respond indicating that there is no pipeline associated with the pull request. This can be ignored if the build is triggered.Team members should not trigger a build this way for pull requests coming from forked repositories. They should instead trigger the build manually by setting the "branch" to
refs/pull/{prId}/mergewhere{prId}is the ID of the PR.