[WIP] Implement Guzzle 6 adapter

[ddeboer]

No description provided.

[sagikazarmark]

Thanks. I will take a look at it in the evening.

Don't care about the failing tests: they are because diactoros require PHP 5.5 and we still test again 5.4. If we can decouple from a specific message implementation then this issue will go away.

[sagikazarmark]

Is that really a Friends of Symfony class in Guzzle 6?

[sagikazarmark]

Please check the guzzle5-adapter package to see how integration test should be added.

[ddeboer]

This should be changed to ~6.0 when Guzzle 6 has been released.

[ddeboer]

Sorry, accidentally committed some WIP stuff. Fixed that.

Please check the guzzle5-adapter package to see how integration test should be added.

I only see an HttpAdapterTest; which test class should I use for testing an PSR HTTP adapter?

[sagikazarmark]

See this file. The two methods you have to implement: testGetName and createHttpAdapter.

AFAIK Guzzle 6 uses the same ring architecture, so you should probably create multiple tests to test with various handlers.

[ddeboer]

Thanks. I meant: that’s the HttpAdapterTest, but I only implemented the minimal Psr7Adapter, for which I can find no test.

[sagikazarmark]

Ah, I missed that, I thought you extended the CoreHttpAdapter. Adapters provided by us should extend that class so that it is both a PsrHttpAdapter and an HttpAdapter. However it Guzzle6 is going to be the first adapter which is PSR compatible. It is still undecided how we should proceed in that case: force CoreHttpAdapter or have another implementation which helps implementing HttpAdapter interface.

[ddeboer]

Yes, things becomes different in the case of Guzzle 6, which does PSR-7 natively.

My intuition is that we don’t really need CoreHttpAdapter in this case. I like the minimalist PsrHttpAdapter interface, which just takes PSR-7 request objects. Now PSR-7 has been accepted, more and more users will probably start working with those objects in their projects. This makes the HttpAdapter (with get()/post() etc. convenience methods) less relevant here, as constructing a request object that is native to the adapter is no longer necessary. Do you think those convenience methods still add enough value to maintain them?

In any case, I do think we need a new ConfigurablePsrHttpAdapter to use sendRequest(s) with an $option argument.

[sagikazarmark]

I am not entirely sure that HttpAdapter should always be implemented.

However extending CoreHttpAdapter is extremely easy: replace sendRequest and sendRequests methods with their sendInternal* methods.

I am waiting for some feedback from @egeloen with the upcoming interface separation. After that we can discuss how we should proceed with PSR7 clients (since it is the first one).

[ddeboer]

But do we still need the distinction between internal and PSR-7 requests when building adapters for PSR-7 libraries?

Okay, I’ll wait with adding tests until the interface segregation has been finished. Where is the discussion taking place?

[sagikazarmark]

There isn't really a discussion, because @egeloen is disappeared. I invited you to the Gitter room.

Internal requests are not just there to provide a bridge between PSR7 and non-PSR7 clients. They are also used for special type bodies, like files, or arrays.

[sagikazarmark]

Guzzle 6 is released

[ddeboer]

Thanks, I had already changed the version constraint.

[sagikazarmark]

I've been thinking about this whole thing with PSR and not PSR. I think the right choice would be to have a Client implementation which relies on adapters. This way adapters can remain PSR wrappers and the client implements the callable methods AND most of the internal request things.

[ddeboer]

I’m not sure I understand. Do you mean you want to keep the internal/PSR request separation and the HttpAdapter’s convenience methods?

[sagikazarmark]

I mean I want to move them to a separate implementation (Client) and use adapters there. All the internal request and convenience methods would be part of the HTTP Client implementation. Adapters would only be PSR wrapper this way (basically accepting requests and returning responses).

[ddeboer]

I like it, as it keeps the adapters lean and pure PSR-7. I guess adapters for non PSR-7-compliant HTTP clients such as Guzzle 5 would then convert between the PSR request object as produced by Http\Client and the Guzzle 5 request object (and vice versa for the response).

[sagikazarmark]

Exactly.

[sagikazarmark]

@ddeboer Thanks for your contribution. I am going to merge this and finish tests.