-
Notifications
You must be signed in to change notification settings - Fork 65
Uses descriptions from CRUD restrictions annotations for OpenAPI operation descriptions #178
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
The description is basically just a description of the element and doesn't really add any more info. about the operation
src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyDeleteOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/EntitySetGetOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/EntityUpdateOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/RefGetOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/SingletonGetOperationHandler.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/SingletonPatchOperationHandler.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…rationHandler.cs Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…tionHandler.cs Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…tionHandler.cs Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…tionHandler.cs Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…ationHandler.cs Co-authored-by: Vincent Biret <vibiret@microsoft.com>
…ndler.cs Co-authored-by: Vincent Biret <vibiret@microsoft.com>
@irvinesunday Can you confirm that this code is dependent on the ongoing work in ApiDoctor OneDrive/apidoctor#116 that isn't yet merged? |
@darrelmiller they are related but independent work items. This code checks for the existence of said CRUD restriction annotations in the CSDL that the code in Api Doctor will add in that mentioned ongoing work. The structure of those annotations is already pre-determined. The absence of descriptions in the CRUD restrictions annotations, or absence of the CRUD restriction annotations themselves in the CSDL should not break the OpenAPI generation process (other than the missing operation descriptions that are being targeted). Therefore, this PR, once reviewed and validated should be able to be merged whilst the Api Doctor work is still in progress. And vice-versa I believe. |
Use CRUD restrictions annotations to get the descriptions
Looks good to me, |
@irvinesunday Yes, the description fields are well defined. However, another element of the ApiDoctor work was to include the URL to the reference documentation. The way we incorporate that into the CSDL has not been determined. I guess we can add support for that in a separate set of PRs. |
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
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 taking the time to make the changes
Fixes #177
This PR proposes:
Reading in descriptions for operations from the below CRUD restrictions annotations:
Org.OData.Capabilities.V1.ReadRestrictions
Org.OData.Capabilities.V1.UpdateRestrictions
Org.OData.Capabilities.V1.InsertRestrictions
Org.OData.Capabilities.V1.DeleteRestrictions
For
Get
operations, if the above are unavailable, we fallback to theOrg.OData.Core.V1.Description
annotation for that particular EDM element. This is the standard annotation that we've been using for our descriptions, which we've used for all operations. We will use this default description annotation only forGet
operations as they don't really make sense for other operations since they are mostly just descriptive of a given EDM element and don't really provide any meaningful info. about a particular operation.Since
Org.OData.Capabilities.V1.NavigationRestrictions
can be defined at the navigation source (entity set/singleton) level as well as in-lined at the navigation property level, we are checking for descriptions at the aforementioned annotatable targets when populating the operation descriptions for navigation properties and$ref
paths.Updating tests and integration files to validate the above.
Updating the checks for navigation property
Navigability
to check forOrg.OData.Capabilities.V1.NavigationRestrictions
at both the navigation source level and navigation property level.Refactoring out some common code into a common helper or utility class for reuse.
Open question: Is there a way to annotate structural/complex properties with CRUD restrictions annotations? In the absence of this, we aren't able to populate meaningful descriptions for complex properties.