Moving to using the asynchronous client by default (request for feedback)

[sreeise]

There are many ways in which the current setup makes it difficult to handle multiple use cases while still supporting both blocking and asynchronous requests. After research and working on this I have come to the conclusion that the best way to handle this is by simplifying the client and making use of the Clone abilities of Arc (std::sync::Arc).

Using the reqwest crate we get the benefit of being able to clone using Arc without having to implement Arc ourselves. This is because reqwest handles the Arc wrapping of the client.

The first question is how would this work?

Here is an example of the client (naming not important here)

#[derive(Clone)]
pub struct Client {
    access_token: String,
    inner: reqwest::Client,
}

impl Client {
    // Build method to set defaults or customize the client such as connection timeout.
    pub fn build() -> Client {...}

}

// Simplified version 
pub struct GraphClient {
    http_client: Client
}

What we have is two clients. The first is the actual wrapper of reqwest::Client with the access token and the second is the graph client itself.

Briefly I want to mention something further down the road. Instead of access token being stored in that field in the future I want to change this to be more in line with authentication types that are similar to how we see in other graph SDKs. As an example consider how some OAuth Credential types can refresh access tokens using a refresh token. By using types to represent tokens we can provide a mechanism to automatically renew access tokens while using the client. It will also be more in line with what people are use to using with other languages.

Additionally it would provide a way to easily add more credential types such as Azure credentials and possibly using cloud services such as Azure Key Vault to enable cloud integration and authentication via the cloud.

pub trait TokenCredential {...}

pub struct AuthorizationCodeCredential {...}

impl TokenCredential for AuthorizationCodeCredential {}

// As an example. Not saying this has to be a trait object here.
pub struct HttpClient {
    credential: Box<dyn TokenCredential>
    inner: reqwest::Client
}

All of that will come later on. For now I want to move on to how the client will work. Basically we have our base GraphClient and that client will have methods to choose what api to call just as it does now. When the api is chosen, instead of taking a reference to the http client, we will clone the http client to an api client. That api client doesnt and will never care about the graph client. Each api client will be there own entities and can be created and called without the GraphClient.

Here is an example of the admin api client and what that would look like.

pub struct AdminClient {
  http_client: Client // Our http client fully owned
}

impl AdminClient {
  // Example only... I didnt look to see what the names now.
  pub fn get_admins(self) -> ResponseHandler {...} 
}

Lastly, each api call method will return a ResponseHandler (still working out the names). In general, the response handler will store a request or request builder but not the http client itself. This is because the response handler needs to or at least probably should act in a limited lifetime scope just as the request would before its called.

pub struct ResponseHandler {
  request_builder: RequestBuilder
}

impl ResponseHandler {
  pub async fn send(&self) -> Result<Response, Error> {...}
}

In addition to handling the request chain this way I am considering that most requests will just be returning a reqwest::Response instead of a wrapped response by default. This is because the response that reqwest returns can handle all of the use cases that are currently implemented. There will still be valid use cases that need to be be handled differently, such as downloading files or multi-part uploads, but those will be as a add on for convenience and not as a barrier or a wrapper. It will also allow using http response specific methods and data that are provided by the reqwest crate because its meant for that use case. There will still be conveniance methods but they will be more focused on providing additional functionality and not basic http functionality.

For blocking requests, I believe the best way to do it going forward would to use tokio, or possibly even a built in mechanism from the reqwest crate, to provide blocking on requests as an optional method call instead of the normal send. I am still working on this and how it will work.

I am also considering looking at making it easy just to get the response without needing to call send() every time for those who just want to call the api as quickly as possible. But that is more of a maybe and based solely on how much time I have to do extra things (which is almost never).

To finish, this is a high level overview of what I am working on. I would like to get some feedback and comments from those who have contributed to this project and those that are using it. Id also like to get your ideas and have a discussion so that this project can be successful and useful for everyone. There are several feature requests that I have received, that are perfectly valid and useful, but that are more difficult to implement with how the client handles requests/responses currently. So I think this is a really important step to take going forward that will make a huge difference.