-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add support for paths with alternate keys #308
Conversation
576970e
to
a10262b
Compare
a10262b
to
400e122
Compare
test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs
Outdated
Show resolved
Hide resolved
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 also believe we should introduce a setting for that new behavior
src/Microsoft.OpenApi.OData.Reader/Operation/EntityDeleteOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/EntityGetOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/EntityUpdateOperationHandler.cs
Outdated
Show resolved
Hide resolved
I had a discussion with @darrelmiller regarding this, and we thought that if there are alternate keys defined in the metadata then we ought to generate paths that utilize them as well. Do you think the presence of these paths is going to affect any downstream tools? CC @peombwa |
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
It might increase significantly the size of the SDKs, especially for sets that are fairly "close to the root" of the API paths tree since the logic might go ahead and expand all the paths we already had under the main key. |
@baywet We decided not to expand the paths using alternate keys. I will be adding the setting. |
Thanks, that's what I suspected from the code but I wasn't sure whether it was a final decision or a stopgap. |
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.
thanks for making the changes!
As @baywet said above, we may see a significant growth in paths. However, having the setting to control how paths with alternate keys are generated is enough to warrant any concerns with downstream tools. |
Fixes #120
This PR: