Skip to content
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

[Feature] Adds support for RequiresExplicitBinding and ExplicitOperationBindings annotations for operations #378

Merged
merged 11 commits into from
May 2, 2023

Conversation

irvinesunday
Copy link
Collaborator

@irvinesunday irvinesunday commented Apr 28, 2023

Fixes #323, #232

This PR:

  • Uses the annotations Org.OData.Core.V1.RequiresExplicitBinding in operations and Org.OData.Core.V1.ExplicitOperationBindings in model elements to associate which operations are to be generated for which elements.
  • Updates the unit and integration tests.
  • Updates the release notes.

How it works

Take for example the operation:

<Action Name="getMemberObjects" IsBound="true">
  <Annotation Term="Org.OData.Core.V1.RequiresExplicitBinding"/>
  <Parameter Name="bindingParameter" Type="graph.directoryObject" Nullable="false" />
  <Parameter Name="securityEnabledOnly" Type="Edm.Boolean" />
  <ReturnType Type="Collection(Edm.String)" Nullable="false" Unicode="false" />
</Action>

The bindingParameter is of type graph.directoryObject. This means that all model elements of this type (including all of its derived types) will have the operation getMemberObjects appended to their paths. In order to restrict the generation of this operation only to certain instances of the binding type - directoryObject, we append the <Annotation Term="Org.OData.Core.V1.RequiresExplicitBinding"/> annotation to the operation (as shown above).

All the operations which bind to directoryObject, for example, may need to be annotated with this annotation to avoid generating irrelevant operations to all of its derived types..

The navigation property deletedItems is an instance of type Collection(graph.directoryObject):
<NavigationProperty Name="deletedItems" Type="Collection(graph.directoryObject)" ContainsTarget="true" />
Ordinarily it would have the path /directory/deletedItems/{directoryObject-id}/microsoft.graph.getMemberGroups which is invalid. The only valid operation for this navigation property path would be /directory/deletedItems/{directoryObject-id}/microsoft.graph.restore.

To restrict all the operations bound to instances of directoryObject from being generated as operations on the deletedItems navigation property and only allow the restore operation, we would create an ExplicitOperationBindings to the navigation property as such:

<NavigationProperty Name="deletedItems" Type="Collection(graph.directoryObject)" ContainsTarget="true">
  <Annotation Term="Org.OData.Core.V1.ExplicitOperationBindings">
    <Collection>
      <String>microsoft.graph.restore</String>
    </Collection>
  </Annotation>
</NavigationProperty>

The entity type directoryObject will need to invoke all the operations that are bound to it and since all the operations which bind to directoryObject have been annotated with the annotation Org.OData.Core.V1.RequiresExplicitBinding, a corresponding Org.OData.Core.V1.ExplicitOperationBindings annotations with the collection of allowable operations need to be specified. Specifying this annotation on the entity type itself would automatically propagate to all its derived types. Which is not what we want. Therefore, this corresponding annotation will need to be annotated on the navigation source instance of this entity type. For example:

<Annotations Target="microsoft.graph.GraphService/directoryObjects">
  <Annotation Term="Org.OData.Core.V1.ExplicitOperationBindings">
    <Collection>
      <String>microsoft.graph.checkMemberGroups</String>
      <String>microsoft.graph.checkMemberObjects</String>
      <String>microsoft.graph.getMemberGroups</String>
      <String>microsoft.graph.getMemberObjects</String>
    </Collection>
  </Annotation>
</Annotations>

The above will yield the paths:

/directoryObjects/{directoryObject-id}/microsoft.graph.checkMemberGroups
/directoryObjects/{directoryObject-id}/microsoft.graph.checkMemberObjects
/directoryObjects/{directoryObject-id}/microsoft.graph.getMemberGroups
/directoryObjects/{directoryObject-id}/microsoft.graph.getMemberObjects

Unless a corresponding derived type of directoryObject gets annotated with the Org.OData.Core.V1.ExplicitOperationBindings, these operations will not be created for paths of this derived type.

The net result from all this should be a significant reduction in the number of invalid operation paths and a reduction in the size of the OpenAPI document and consequently our SDKs. The appropriate annotations will need to be added to the CSDL through XSLT initially and later by workload teams to realise this.

NB: The default behaviour is preserved (all operations generated for the bound entity types) when none of these annotations are specified anywhere.

@irvinesunday irvinesunday marked this pull request as ready for review April 28, 2023 12:36
Copy link
Member

@andrueastman andrueastman 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 this @irvinesunday

Co-authored-by: Eastman <andrueastman@users.noreply.github.com>
@sonarcloud
Copy link

sonarcloud bot commented May 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

94.8% 94.8% Coverage
0.0% 0.0% Duplication

@irvinesunday irvinesunday merged commit 08e5738 into master May 2, 2023
@irvinesunday irvinesunday deleted the feature/is/explicit-ops-binding branch May 2, 2023 13:37
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.

Missing paths on derived/odata cast paths
2 participants