Skip to content

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

Merged
merged 26 commits into from
Feb 23, 2022

Conversation

irvinesunday
Copy link
Contributor

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 the Org.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 for Get 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 for Org.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.

irvinesunday and others added 8 commits February 21, 2022 10:57
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>
@darrelmiller
Copy link
Member

@irvinesunday Can you confirm that this code is dependent on the ongoing work in ApiDoctor OneDrive/apidoctor#116 that isn't yet merged?

@irvinesunday
Copy link
Contributor Author

@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.

@xuzhg
Copy link
Contributor

xuzhg commented Feb 22, 2022

Looks good to me, :shipit:

darrelmiller
darrelmiller previously approved these changes Feb 23, 2022
@darrelmiller
Copy link
Member

@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.

irvinesunday and others added 6 commits February 23, 2022 18:25
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>
Copy link
Member

@baywet baywet left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use descriptions from restrictions annotations for operation descriptions
4 participants