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

Implement ToClone method on RequestBuilder #3199

Closed
darrelmiller opened this issue Aug 25, 2023 · 7 comments · Fixed by #3212
Closed

Implement ToClone method on RequestBuilder #3199

darrelmiller opened this issue Aug 25, 2023 · 7 comments · Fixed by #3212
Assignees
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Milestone

Comments

@darrelmiller
Copy link
Member

darrelmiller commented Aug 25, 2023

Related to #2597.

Currently the devx is not great for handling Delta calls. When the user gets the deltaLink back they need to create a new instance of the RequestBuilder but it is not obvious what the type is. By implementing Clone the developer can create a new instance of the Requestbuilder based on the provided path rather than the default one provided by the hierarchy.

e.g.

   var changes = graphClient.Users.ToClone(deltaLink).GetAsync()
@darrelmiller darrelmiller added the generator Issues or improvements relater to generation capabilities. label Aug 25, 2023
@darrelmiller darrelmiller added this to the Kiota v1.6 milestone Aug 25, 2023
@baywet baywet added the enhancement New feature or request label Aug 25, 2023
@baywet baywet added this to Kiota Aug 25, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Aug 25, 2023
@baywet
Copy link
Member

baywet commented Aug 25, 2023

@sebastienlevert we just need you to tell us whether you're happy with the pattern and if you can come up with a better name

@baywet
Copy link
Member

baywet commented Aug 26, 2023

Actually I think I just came up with a better name "withUrl" what do you think @darrelmiller ?

@baywet
Copy link
Member

baywet commented Aug 28, 2023

additional notes:

  • using withUrl will ignore any other query or path parameters. (no using sub code path on the fluent API, no using the query parameters, etc...)
  • we should only add this method when at least one operation is present.
  • should the path segment be deprecated, this method should be deprecated as well.

@sebastienlevert
Copy link
Contributor

So to confirm my understanding:

  • As a developer, I get an URL that is opaque, I pass it to the WithUrl() method and I can get any more control over the call (no options for middlewares, for instance)
  • It does the call and gets the data back with the associated models from the path

Are there scenarios where this URL would NOT be used from the same path? I remember some Planner APIs on Graph that returns a URL but on the Groups endpoint... How would I use it? How do I know which one is it if we treat this URL as opaque?

@baywet
Copy link
Member

baywet commented Aug 28, 2023

No, once you pass a raw URL the only things you "loose" are what would rely on the URI template (path parameters, query parameters) in the fluent API. But you retain the ability to set headers, request options etc...

You'd have to match the path segments in the URI with the fluent API path segments. Which works well because they map. Today you already need to do that work + figure out the imports, pass the request adapter etc... We're effectively simplifying the pattern.

@baywet baywet assigned baywet and unassigned sebastienlevert Aug 28, 2023
@baywet baywet moved this from Todo to In Progress in Kiota Aug 28, 2023
@sebastienlevert
Copy link
Contributor

sebastienlevert commented Aug 29, 2023

Love it! Ship it! 🚀

So this effectively provides the "arbitrary" URL pattern, just that you need to pass it to the right request builder. Love it 🥳 Are there scenarios where an API could provide an URL in another endpoint, where you might not have generated a request builder for?

For example, this is the response for a plan in Planner:

{
    "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#planner/plans/$entity",
    "@odata.etag": "W/\"JzEtUGxhbiAgQEBAQEBAQEBAQEBAQEBBQCc=\"",
    "createdDateTime": "2017-09-13T21:58:50.7250508Z",
    "owner": "1e770bc2-3c5f-487f-871f-16fbdf1c8ed8",
    "title": "Activewear",
    "id": "CONGZUWfGUu4msTgNP66e2UAAySi",
    "createdBy": {
        "user": {
            "displayName": null,
            "id": "48d31887-5fad-4d73-a9f5-3c356e68a038"
        },
        "application": {
            "displayName": null,
            "id": "09abbdfd-ed23-44ee-a2d9-a627aa1c90f3"
        }
    },
    "container": {
        "containerId": "1e770bc2-3c5f-487f-871f-16fbdf1c8ed8",
        "type": "group",
        "url": "https://graph.microsoft.com/v1.0/groups/1e770bc2-3c5f-487f-871f-16fbdf1c8ed8"
    }
}

You see the url property. If you don't have the groups request builder, you wouldn't be able to use it. I'm OK with it being a limitation, but I don't know if APIs could generate dynamic URLs to "unknown" areas of an API... and then it would become tricky a little bit.

Just needs your thoughts @darrelmiller and @baywet

@baywet
Copy link
Member

baywet commented Aug 29, 2023

if you generated a self-served client and you haven't included the groups endpoints, no you wouldn't be able to query the groups endpoints with the arbitrary URL because your client would be missing models and fluent API paths.
However in your specific example in under /groups/planner/plans so you might or might not have that path depending on your include filters.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Kiota Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants