Allow overriding of HttpClient

[tchiotludo]

Overriding of the HttpClient is not really possible with the current implementation.
You set on protected function createGuzzleClient($proxyPort = null)

• default request headers
• base_url

If we want to have a custom HttpClient, we need to set all the headers & base url.
With this update, we can override the httpclient with set this values

[msftclas]

All CLA requirements met.

[curry684]

👎

Should just use HTTPlug and PSR standards instead of hacking it in locally.

[curry684]

http://docs.php-http.org/en/latest/

[tchiotludo]

Whatever the http client you use, the goal of this pull request is to let the sdk know the header needed and the base url.

In the current implementation, if you want a custom client, you need to pass a client with headers (auth) and base url already set to be working with microsoft api. So you need to know what header is needed.

The main goal of a sdk is to works without knowing how the auth works (just pass a accessToken and don't care if the token is a header and how to send it correctly).

I think you can use any http client, with the current implementation, you just can't send any valid request without setting headers needed, so the parameter $client is useless for my point of view and must be remove or changed

[curry684]

I'm just saying it's much easier to make everything pluggable with the PSR-7 compliant HTTPlug library, and then exposing the properties you need here, than to change internal APIs twice in different ways for the same result.

[caitlinrussell]

It looks like HTTPlug would be good to implement here, however, in the meantime I agree that not having access to the base URL and request headers is a bug in the current implementation.

[caitlinrussell]

If a client is passed in, we should not assume they are using Guzzle

[tchiotludo]

Don't really understand this comment.
We need to assume that people are using a compliant client that works with the SDK.

Since the http client interface is not standardize (like PSR7 for example), we need to declare the current implementation of a http client we are using.

From my point of view, we need to force an http client (Guzzle or HTTPlug or another one), the simpliest is Guzzle :

use GuzzleHttp\ClientInterface;

    /**
    * Executes the HTTP request using Guzzle
    *
    * @param ClientInterface $client The client to use in the request
    *
     * @throws GraphException if response is invalid
     *
    * @return mixed object or array of objects
    *         of class $returnType
    */
    public function execute(ClientInterface $client = null)
    {

If the people use HTTPlug, they can use the guzzle adapter to give to msgraph-sdk the client needed.

Or if you want to use HTTPlug to abstract completely, it's not the same work since it need to refractor the whole sdk since there is a lot of dependency with Guzzle (also on generated part on the sdk) and it will be breaking change and a major version bump

[Ndiritu]

I think we can work with Guzzle as a default for now as we prep for support PSR-18 HTTP clients in the future.

[Ndiritu]

@tchiotludo my apologies that this PR is being addressed years later. We've had a change in the team and will be supporting this SDK more...

I agree and like that this solves the issue of wiring up a custom Guzzle HTTP client.

A cleaner way to wire up these defaults is currently under design + pending prioritisation under #438. The scope of this should allow one to pass in a PSR-18 compliant HTTP client and set it up for use with Graph which should address @curry684's valid concerns.

@tchiotludo would you be interested in updating this PR with latest changes on dev?

[Ndiritu]

Could this be getDefaultGuzzleRequestOptions? Or basically add default somewhere in the naming? and without the proxyPort param passed in

[Ndiritu]

Closing this as support for PSR-18 and HTTPlug is being addressed in this PR