Support multiple HTTP clients

[Ndiritu]

Come to a conclusion on how to support multiple HTTP clients

Some references:

• https://www.php-fig.org/psr/psr-18/
• https://docs.php-http.org/en/latest/index.html

This blocks GraphClientFactory work.
AB#9888

[Ndiritu]

Added to v2-preview release

[drupol]

I see that the v2-preview doesn't require guzzle by default, which is great.

However, it requires now microsoft/microsoft-graph-core (see composer.json) which depends on guzzlehttp/guzzle (see composer.json), so I don't get it at all.

How about removing any HTTP client library and uses PSR18 interfaces only in both projects?

[Ndiritu]

Thanks for feedback @drupol

Our intention was/is to support Guzzle by default for developers who don't care about the underlying HTTP client lib but also provide flexibility for devs who do.

[drupol]

Hi !

Got your message, but I have the feeling that you're relying on a concrete implementation instead of relying on interfaces, so, there is actually, no flexibility.

[Ndiritu]

We are providing flexibility using interfaces
We allow devs to plug in their PSR implementation when making requests here and for async requests, here

And also while creating a Graph client

(FYI, we will soon be breaking the latest v2-preview significantly and it will not include multiple HTTP client support just yet)

[drupol]

We are providing flexibility using interfaces

If that's the case, then guzzlehttp/guzzle should be moved to require-dev section of composer.json and you should use psr/http-client which provides the PSR18 interfaces.

And in order to let the user know that a PSR18 HTTP Client is needed, you may add psr/http-client-implementation in the require section.

This way, when the user run composer install, there will be a note saying that an HTTP Client is needed, with a list of potential libraries.

We allow devs to plug in their PSR implementation when making requests here and for async requests, here

I see indeed. IMHO, I found it a bit confusing to be able to inject different HTTP Clients. I think an HTTP Client is a service and should be injected in the constructor only and removed from the GraphRequest::execute method.

And also while creating a Graph client

(FYI, we will soon be breaking the latest v2-preview significantly and it will not include multiple HTTP client support just yet)

Oh ! Nice ! I wrote the previous comment without reaching this part yet :D

If you're in a middle of a refactoring, please remove any dependency to concrete implementation and use PSR interfaces.

An example project using this: https://github.com/ecphp/cas-lib (check the composer.json file).

[Ndiritu]

Thanks for the composer suggestions!

I see indeed. IMHO, I found it a bit confusing to be able to inject different HTTP Clients. I think an HTTP Client is a service and should be injected in the constructor only and removed from the GraphRequest::execute method.

I agree as well. The new release will initialise the HTTP client once via the constructor

[drupol]

Great, looking forward to the next version :)