Bound operations not appended to non-contained navigation property paths

[irvinesunday]

Related to #201

With this bound operation:

and given this non-contained navigation property:

and given a schema namespace value of microsoft.graph

We expect to generate paths similar to below:
.../followedSites/microsoft.graph.add

This is not the case with the current version because we check the containment of the navigation property here.
According to the spec. doc., see ref:

Actions and Functions MAY be bound to an entity type, primitive type, complex type, or a collection...
The namespace- or alias-qualified name of a bound operation may be appended to any URL that identifies 
a resource whose type matches, or is derived from, the type of the binding parameter. 

This may be applied to any navigation property segment as long as its type matches the type of the binding parameter.

[baywet]

Thanks for documenting this in details. I'm torn here.
Historically we've decided to only bind operations to contained navigation properties, entity sets or singletons. Or in other words to only have operations on the entities canonical path. This is to avoid expanding too many paths and growing the description too much. It makes sense since why would one call an operation on a "deeper path" when a shallower one is already available?

This case is an interesting example, and poor API design IMHO. The followed sites should instead be a collection of "followedSite", contained. FollowedSite is a new entity type that semantically belongs to the user (is contained), has a non-contained reference to site (existing entity), and potentially other properties like "followedDate", "followedReason", etc...

But since the API is not getting fixed any time soon, we probably out to make an opiniated decision here.
What would be the consequence of setting contains target true for that property?
Can we rely on an annotation/attribute that would signal "bind operations even though this is not contained"?
Do we have other cases like this one?

[baywet]

after a quick chat with @darrelmiller it looks like there's no such annotation. Can you have a look at the implications of changing contains target for that property? and see if we have other instances of that scenario?

[irvinesunday]

after a quick chat with @darrelmiller it looks like there's no such annotation. Can you have a look at the implications of changing contains target for that property? and see if we have other instances of that scenario?

Appending ContainsTarget=true generates 1042 more paths. I'm collating the list of other navigation props. that have a similar issue.

[irvinesunday]

The number of similar scenarios are quite significant. Below are just a couple of them:

** Non-containment Navigation properties:**

• followedSites --> Action Name="add"
• group --> Action Name="assignLicense"
• filesFolder --> Action Name="restore"
• createdOnBehalfOf --> Action Name="restore"/Action Name="follow"/Action Name="copy"
• manager --> ""
• owners --> Action Name="getByIds"
• directReports --> ""
• memberOf --> ""
• transitiveMemberOf --> ""
• registeredOwners --> ""
• members --> ""
• driveItem --> Action Name="copy" (binds to mailFolder, driveItem, message entities and root nav. prop)
• group --> Action Name="assignLicense" (binds to user and group entity)
• teachers --> Function Name="delta"
• schools --> Function Name="delta"
• user
• acceptedSenders
• calendar
• membersWithLicenseErrors
• parentNotebook
• parentSection
• baseTypes
• sites
• channel

Entities:

• directoryObject
  • directReports/{directoryObject-id}
  • memberOf/{directoryObject-id}
  • transitiveMemberOf/{directoryObject-id}
• driveItem
• contentType
• site

Clearly, setting their containment to true will be unfeasible, as the number of paths generated will just be humongous.

[baywet]

To provide an update to everyone here: Darrel brought it to the API council and it was decided to introduce a new attribute/annotation to say something like "target properties" which would contain list of non contained properties we want that method to target additionally to the contained properties/entity sets/singleton.
Next step is to get @mikepizzo to provide us guidance for that new attribute/annotation, but if you have suggestions, feel free to add them here.

[irvinesunday]

From discussions with @darrelmiller, the recommendation was to come up with a custom annotation that will be appended to a given operation. This annotation will list the non-contained navigation properties that should append paths to the given operation. The annotation would look something like:

[irvinesunday]

This was the proposal from Mike for the annotations to be used.

[irvinesunday]

After further discussions, the below annotation structure has been arrived at:

<Term Name="RequiresExplicitBinding" Type="Core.Tag" DefaultValue="true" AppliesTo="Action, Function">
  <Annotation Name="Description" Edm.String="This bound action or function is only available on model elements annotated with the ExplicitOperationBinding term."/>
</Term>

<Term Name="ExplicitOperationBindings" Type="Collection(Edm.String)"> 
   <Annotation Name="Description" Edm.String="The qualified names of explicitly bound operations that are supported on the target model element. These bindings are in addition to any bindings to the type of the target model element not annotated with RequiresExplicitBinding"/> 
</Term>

Example usage:

<Action Name="forward" IsBound="true" >
  <AnnotationTerm="Org.OData.Capabilities.V1.RequiresExplicitBinding"/>
  <ParameterName="bindingParameter" Type="self.email"/>
  <ParameterName="recipient" Type="Collection(self.recipent)"/>
  <ReturnTypeType="self.email" />
</Action>

<Annotations Target="self.Container/me/email">
  <Annotation Term="Org.OData.Capabilities.V1.ExplicitOperationBindings">
    <Collection>
      <String>self.forward</String>
    </Collection>
  </Annotation>
</Annotations>

Find further reading of the proposal here

[irvinesunday]

This issue is blocked while waiting for the resolution of: OData/odata.net#2543

[darrelmiller]

Can we confirm that we are no longer blocked?

[irvinesunday]

Can we confirm that we are no longer blocked?

This is now unblocked. The PR to resolve this: #378

[irvinesunday]

Resolved by: #378