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.

Sign up for GitHub

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

Jump to bottom

GraphClientsDesign.md #570

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

GraphClientsDesign.md #570

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d92a7b
Create GraphClientsDesign.md
nikithauc Jan 18, 2022
1b4bcbb
Merge branch 'dev' into nikithauc/design-graph-service-clients
nikithauc Jan 19, 2022
612d9b2
Update GraphClientsDesign.md
nikithauc Jan 19, 2022
2fe5322
Merge branch 'dev' into nikithauc/design-graph-service-clients
nikithauc Jan 19, 2022
d8f6f7f
Merge branch 'dev' into nikithauc/design-graph-service-clients
nikithauc Feb 15, 2022
f5545d7
Merge branch 'dev' into nikithauc/design-graph-service-clients
nikithauc Feb 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions design/GraphClientsDesign.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@

### Graph JS Deliverables:

Note: Package names yet to be decided. The names are just examples and `service` and `core` prefix in the package names are for clarity purposes.

1. `microsoftgraph/microsoft-graph-javascript-service`:
nikithauc marked this conversation as resolved.
Show resolved Hide resolved

- Generated library which the request builders. Generated using Kiota builder.

- Will have dependencies on :
- @microsoftgraph/microsoft-graph-javascript-core
- @microsoft/kiota-abstractions
- @microsoft/kiota-http-fetchlibrary
- @microsoft/kiota-serialization-json

2. `microsoftgraph/microsoft-graph-javascript-core`:

- Contains Graph customizations and tasks such as PageIteration, LargeFileUpload.

- Will have dependencies on :
- @microsoft/kiota-abstractions
- @microsoft/kiota-http-fetchlibrary

### Usage of the two libraries :

As mentioned in PR: #558

Also, tasks constructors such as `PageIterator` and `LargeFileUpload` tasks should accept both `GraphServiceClient` and `GraphCoreClient`

```
// both should work
const pageIterator = PageIterator(GraphServiceClient, options);
or
const pageIterator = PageIterator(GraphCoreClient, options);
```

Goals:

- A Graph JS SDK user should not be required to create separate client instances for Graph Service library or the Graph Core library.
- To achieve this, the `Graph Service Client` should also expose the features of `Graph Core Client`.

Design:

1. `Graph Service client` should extend from `Graph Core Client`:

```

import { Client as GraphCoreClient } from `@microsoft/microsoft-graph-javascript-core`;


class GraphServiceClient extends GraphCoreClient {

public api(){
super.api();
}
}

```

- To acheive the above design we will need to customize the auto-generated `GraphServiceClient`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could achieve that by adding the API method to a base request adapter in the core repo. This way your code doesn't need to be duplicated between v1 and beta

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean exactly? How extensible is the base request adapter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't exist yet, but this is how I handled it in Go. kiota defines an http request adapter. Go core a BaseGraphRequestAdapter, which adds a bit of customization for graph on top of kiota. In this case the customization could be the API method to allow execution of arbitrary requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following are reasons to use an inheritance:

  • Tasks constructors such as PageIterator and LargeFileUpload tasks should accept both GraphServiceClient and GraphCoreClient
// both should work
const pageIterator = PageIterator(GraphServiceClient, options);
or
const pageIterator = PageIterator(GraphCoreClient, options);
  • Using inheritance will help in extending features from core to service.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my point being that the page iterator doesn't need the client but just the request adapter to be able to execute requests.
And today we don't have a mechanism in Kiota to make the client inherit from something.
You can have a look at the implementation of Page Iterator for Go microsoftgraph/msgraph-sdk-go-core#27

You could use the RequestAdapter property on the client, like the dotnet implementation, but I believe this will change to use the request adapter straight away because of the inheritance limitation.
https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/blob/5ad51c9fca2e4f28a5dd68bdc4e528d7abb0daab/src/Microsoft.Graph.Core/Tasks/PageIterator.cs#L154

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@baywet, what would look like a call using the request adapter. Do we really want the complexity to land of the developer? Is that even complex?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

const pageIterator = PageIterator(GraphServiceClient.requestAdapter, options);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The discussion around bringing your own client emphasized that a user should be able to access the core features and the service library with a same client instance. This leads to the option for inheritance so that we can couple the core client and the service client.

  2. it doesn't exist yet, but this is how I handled it in Go. kiota defines an http request adapter. Go core a BaseGraphRequestAdapter, which adds a bit of customization for graph on top of kiota. In this case the customization could be the API method to allow execution of arbitrary requests.

Even with the BaseGraphRequestAdapter we would need to customize the GraphServiceClient by adding a api() public method GraphServiceClient so that the user do something like GraphServiceClient.api();

  1. I am also thinking whether we should be abstracting the RequestAdapter and the Kiota surfaces from the user in an example like const pageIterator = PageIterator(GraphServiceClient.requestAdapter, options); @sebastienlevert I think we discussed about the abstraction of Kiota libraries in the Graph layers. What do you think about this?

  2. const pageIterator = PageIterator(GraphServiceClient.requestAdapter, options); would be similar to const pageIterator = PageIterator(GraphCoreClient.api(), options);`

I can see the value of a extending the GraphRequestAdapter in the Graph SDKs which will allow extensibility and easy customization. This should be plugged in optimally.