Removing dependency on Guzzlehttp

[Nyholm]

This PR is starting to look like the final implementation. Before this can be merged we need to:

• Make sure Httplug is stable (hopefully before the end of the year)
• Fix the tests
• Resolve/Discuss the timeout FIXME

I added a new dependency on the Guzzle PSR7 implementation. But I removed any dependency to any transport library.

[bshaffer]

@Nyholm will you be continuing work on this? Since the tests are failing, and it's WIP, I'm not exactly sure what to do with this.

[Nyholm]

Yes. I will. I’ll try to find some time this week. Thank you for the reminder.

On 16 nov. 2015, at 21:02, Brent Shaffer [email protected] wrote:

@Nyholm https://github.com/Nyholm will you be continuing work on this? Since the tests are failing, and it's WIP, I'm not exactly sure what to do with this.

—
Reply to this email directly or view it on GitHub #81 (comment).

[Nyholm]

I've done some updates. Im using plugins instead of Guzzle's events.
I've left a few FIXME's. They should be discussed and resolved before merge.

Question: Do we want to support PSR-1 and PSR-2?

[bshaffer]

@Nyholm thank you for your work here! It really helps to see where you're going for with the PHP-HTTP integration.

We are planning to refactor the library so the minimum amount of logic possible is dependent on the HTTP client, i.e. other classes handle the caching, fetching of tokens, etc. Then we will have a Guzzle5 handler (to add subscribers) and a PSR-7 handler (implemented like a psr7 middleware) that does the minimal logic to add credentials to the request.

These refactorings will make adding PHP-HTTP support (via another handler) much simpler. We think this is the best way to add support because we do not want to force our users to depend solely on PHP-HTTP. Yes, we are forcing the use of Guzzle currently, but Guzzle has been the standard HTTP library in the community for some time.

[Nyholm]

@Nyholm thank you for your work here! It really helps to see where you're going for with the PHP-HTTP integration.

Im happy to help =)

We are planning to refactor the library so the minimum amount of logic possible is dependent on the HTTP client, i.e. other classes handle the caching, fetching of tokens, etc.

That is exactly what I've done in this PR. You got some type hints for a HttpClient but the only place we create a request or a client is in the HttpFactory.

Then we will have a Guzzle5 handler (to add subscribers) and a PSR-7 handler (implemented like a psr7 middleware) that does the minimal logic to add credentials to the request.

That is exactly what I've done in this PR. It also lets the user decide if it should use Guzzle6 or Guzzle5.

These refactorings will make adding PHP-HTTP support (via another handler) much simpler. We think this is the best way to add support because we do not want to force our users to depend solely on PHP-HTTP.

So you want to create an adapter for an adapter? To create an abstraction for an abstraction? So you want to create and maintain 3 adapters instead of relying on an interface? Just to be clear, you want a dependency graph looking like option 1 instead of option 2?

Wouldn't it be better if we (Goggle) focusing on doing stuff related to the Google api and authentication and let an other library build those adapters?

Yes, we are forcing the use of Guzzle currently, but Guzzle has been the standard HTTP library in the community for some time.

I really want to make an argument that this solution is not the best one. You should not be forcing any HTTP library. Yes, many other libraries are doing this today but it has some drawbacks:

• Google API will be tightly coupled with Guzzle
• It violates the common closure principle
• You will have more code to maintain, more logic and more possible bugs
• You do not help the php community by adapting to the new standard of HTTP clients

I have seen the comments in #83. Ping @sagikazarmark or @dbu, is there anything I'm failing to communicate?

[dbu]

I think this is well explained. Just to be sure the initial idea is clear: PHP-HTTP provides a generic abstraction for HTTP clients. Basically what your current Adapter is, but as a separate composer package.

We hope it to make its way into a PSR at some point.

[sagikazarmark]

One more thing to add here (besides PHP-HTTP being an organization and HTTPlug being the product): HTTPlug is a contract for generic HttpClients. Basically it is an interface for what you call Adapter/Handler here.

We also have powerful plugin library, which allows adding authentication info to the request, logging, etc.

[sagikazarmark]

I wonder if we could implement this library as an authentication provider in PHP HTTP:

https://github.com/php-http/authentication

[Nyholm]

I discovered https://github.com/php-http/authentication shortly after I made this implementation. I believe it is a good idea and I would be happy to try. I just want to see some positive response from Google before I do that though... I feel there is a good change they close this PR and create their own adapters from Guzzle5 and Guzzle6.

[bshaffer]

@Nyholm I am closing this in favor of #84, mostly because it has a smaller dependency tree and passing tests. HttpPlug handlers are still supported by passing them in as callables, and we will document this fact in the README. I would love your help providing examples also.

Thanks for your hard work and feedback on this.