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

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

Open
ramsessanchez opened this issue May 24, 2023 · 4 comments
Assignees
Labels
blocked dependencies Pull requests that update a dependency file

Comments

@ramsessanchez
Copy link

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?

@ramsessanchez ramsessanchez added the .NET Pull requests that update .net code label May 24, 2023
@andrueastman
Copy link
Member

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
Copy link
Author

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
Copy link
Member

baywet commented Jun 7, 2023

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

@baywet baywet added dependencies Pull requests that update a dependency file blocked and removed .NET Pull requests that update .net code labels Jun 7, 2023
@microsoft-github-policy-service

This comment was marked as outdated.

@baywet baywet reopened this Jun 15, 2023
@baywet baywet transferred this issue from microsoft/kiota Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

3 participants