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

Expanding navigation properties on alternate keys #459

Open
andrueastman opened this issue Dec 6, 2023 · 2 comments
Open

Expanding navigation properties on alternate keys #459

andrueastman opened this issue Dec 6, 2023 · 2 comments
Assignees
Labels
priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days status:needs-discussion An issue that requires more discussion internally before resolving type:enhancement Enhancement request targeting an existing experience.

Comments

@andrueastman
Copy link
Member

Related to microsoftgraph/msgraph-sdk-dotnet#2243

From the documentation at https://learn.microsoft.com/en-us/graph/api/directoryrole-list-members?view=graph-rest-1.0&tabs=http#example-2-get-the-members-of-a-directory-role-using-roletemplateid

It should be possible to list the members of a directoryRole using id or using the roleTemplateId alternate key.

GET /directoryRoles/{role-id}/members
GET /directoryRoles(roleTemplateId='{roleTemplateId}')/members

At moment we do not expand paths with alternate keys even though they may have contained navigation properties. As these maybe be valid paths, we will need to investigate the validity and impact on path expansion across other paths.

@andrueastman andrueastman added type:enhancement Enhancement request targeting an existing experience. priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days labels Dec 6, 2023
@irvinesunday
Copy link
Contributor

Preliminary analysis of possible outcome of this would mean expanding /directoryRoles(roleTemplateId='{roleTemplateId}') into similar expanded paths for /directoryRoles/{directoryRole-id}

/directoryRoles/{directoryRole-id}/members
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/$ref
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/microsoft.graph.application
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/microsoft.graph.device
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/microsoft.graph.group
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/microsoft.graph.orgContact
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/microsoft.graph.servicePrincipal
/directoryRoles/{directoryRole-id}/members/{directoryObject-id}/microsoft.graph.user
/directoryRoles/{directoryRole-id}/members/$count
/directoryRoles/{directoryRole-id}/members/$ref
/directoryRoles/{directoryRole-id}/members/microsoft.graph.application
/directoryRoles/{directoryRole-id}/members/microsoft.graph.application/$count
/directoryRoles/{directoryRole-id}/members/microsoft.graph.device
/directoryRoles/{directoryRole-id}/members/microsoft.graph.device/$count
/directoryRoles/{directoryRole-id}/members/microsoft.graph.group
/directoryRoles/{directoryRole-id}/members/microsoft.graph.group/$count
/directoryRoles/{directoryRole-id}/members/microsoft.graph.orgContact
/directoryRoles/{directoryRole-id}/members/microsoft.graph.orgContact/$count
/directoryRoles/{directoryRole-id}/members/microsoft.graph.servicePrincipal
/directoryRoles/{directoryRole-id}/members/microsoft.graph.servicePrincipal/$count
/directoryRoles/{directoryRole-id}/members/microsoft.graph.user
/directoryRoles/{directoryRole-id}/members/microsoft.graph.user/$count
/directoryRoles/{directoryRole-id}/microsoft.graph.checkMemberGroups
/directoryRoles/{directoryRole-id}/microsoft.graph.checkMemberObjects
/directoryRoles/{directoryRole-id}/microsoft.graph.getMemberGroups
/directoryRoles/{directoryRole-id}/microsoft.graph.getMemberObjects
/directoryRoles/{directoryRole-id}/microsoft.graph.restore
/directoryRoles/{directoryRole-id}/scopedMembers
/directoryRoles/{directoryRole-id}/scopedMembers/{scopedRoleMembership-id}
/directoryRoles/{directoryRole-id}/scopedMembers/$count

This would considerably increase our OpenAPI paths footprint. To try and contain this, would we want to have expansion rules that enforces:

  1. Expanding only the immediate navigation property (and no indexing)?
  2. No expansion into type casts?
  3. No generation of bound operations?

@irvinesunday irvinesunday self-assigned this Dec 7, 2023
@irvinesunday irvinesunday added the status:needs-discussion An issue that requires more discussion internally before resolving label Dec 7, 2023
@irvinesunday irvinesunday added this to the OData: 1.6 milestone Dec 7, 2023
@andrueastman andrueastman self-assigned this Jan 24, 2024
@baywet
Copy link
Member

baywet commented Apr 15, 2024

Taking this example again, all the members operations are available under the "main id" path segment.
The vast majority of APIs in Microsoft Graph only support a canonical path for operations, or relatively few alternatives (like /me instead of /users/id, etc...)
This is in a effort to limit the API size that needs to be understood by the customers and maintained by the service.
The size of the packages has been a common concern across languages.
Additionally, there are alternatives to making 2 requests if the client application is performance sensitive: use the withUrl approach.

I suggest we DO NOT expand those paths using the conversion library.

Maybe better steps here would be to:

  1. document the "withUrl" pattern in the graph conceptual documentation.
  2. amend the snippets generation process so it leverages the withUrl approach on operations that are under an alternate key.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days status:needs-discussion An issue that requires more discussion internally before resolving type:enhancement Enhancement request targeting an existing experience.
Projects
None yet
Development

No branches or pull requests

3 participants