microsoft / OpenAPI.NET.OData

Generates OpenAPI document from OData CSDL
MIT License
207 stars 63 forks source link

Support POST by ref and POST by object for api enpoints that allow both #395

Open ramsessanchez opened 1 year ago

ramsessanchez commented 1 year ago

An issue was recently raised for the current Java sdk in which a user wanted to create a group while simultaneously posting to the '/directory/administrativeUnit/members' enpoint. The documentation states that this is possible: https://learn.microsoft.com/en-us/graph/api/administrativeunit-post-members?view=graph-rest-1.0&tabs=http However, our current java SDK only creates a referenceRequestBuilder for this scenario due to a limitation in the current generation. We acknowledge that this is due to a 'Contains-Target' annotation being absent in the metadata file, this however is not incorrect since the service allows for both posting by reference and object so we cannot simply add 'Contains-Target' as this would remove the other functionality. In short, we need to support both. This relates to Kiota because while investigating the issue Mustafa and I discovered that the code snippet generated for C# in the documentation provided above does not compile in Raptor, specifically because the method 'postAsync' does not exist for the following case:

graphClient.Directory.AdministrativeUnits["{administrativeUnit-id}"].Members.PostAsync(requestBody);

but the following is available:

graphClient.Directory.AdministrativeUnits["{administrativeUnit-id}"].Members.Ref.PostAsync(requestBody);

While it seems that our intention is to support both scenarios given the snippets generated, the reality is that the Kiota generation is not actually generating the code to provide functionality for both scenarios. Is the expectation that Kiota should cover scenarios in which we can post by reference or object to a single endpoint? Or are we overlooking this scenario?

andrueastman commented 1 year ago

Thanks for raising this @ramsessanchez,

This is a metadata issue (possibly a conversion library change) as the path is missing from the openAPI reference and will need to be added for Kiota to generate it.

Any chance you can create the issue at https://github.com/microsoftgraph/msgraph-metadata and for it to be investigated for both paths to be generated simultaneously? @irvinesunday May be able to help if the conversion library is the cause.

ramsessanchez commented 1 year ago

Hey @andrueastman, I also believe it's a metadata issue, though I'm a bit curious as to how this behavior should be specified. We are currently generating the 'withReference' methods because the members property is missing 'OpenType="true"', this is correct behavior though since this is allowed. Including 'OpenType="true"' would also result in correct behavior though, how do we define the need for both behaviors?

<EntityType Name="administrativeUnit" BaseType="graph.directoryObject" OpenType="true">
~
        <NavigationProperty Name="members" Type="Collection(graph.directoryObject)" />
~
baywet commented 1 year ago

@ramsessamchez I believe the scenario you're referring to (not the $ref) is called a deep insert in odata terms.

I don't believe the open type has any impact on your scenario.

And yes this is a metadata conversion issue. I don't think the conversion library supports that scenario today.

It'd be great if you could create an issue over there and close this one once it's done. Kiota doesn't know anything about odata so it's not a kiota concern.

One thing to consider though, odata probably assumes all APIs support deep insert by default which would grow our metadata significantly. We should probably have 2 settings: generate deep insert endpoints (boolean, generates all when true unless an annotation disables it for a given endpoint), require annotation for deep insert (boolean, requires deep insert annotation to true before projecting the endpoint). This will limit the growth, similar to what we did for raw count and other scenarios.

For the annotation, Irvine can help identify one and if none exist work with Mike to define one.

I hope this helps.

microsoft-github-policy-service[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.