Add compatibility with curl-client v2 (now relying on PSR17 factories)

[shulard]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	yes
Deprecations?	no
Related tickets	fix #338
License	MIT

What's in this PR?

This PR contains the compatibility with php-http/curl-client v2.

Why?

Installing the bundle with the curl-client will trigger that error now :

Argument 1 passed to Http\Client\Curl\Client::__construct() must be an instance of Psr\Http\Message\ResponseFactoryInterface or null, instance of Nyholm\Psr7\Factory\HttplugFactory given, called in /vagrant/vendor/php-http/httplug-bundle/src/ClientFactory/CurlFactory.php on line 5

Checklist

• Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• Documentation pull request created (if not simply a bugfix)

To Do

• Handle compatibility with curl-client v1 in parallel of v2

[dbu]

i did #340 to remove all the old stuff that is breaking the build (as well as fixing deprecation notices from symfony 4.3)

[dbu]

can you please rebase on master to hopefully get the build fixed?

[shulard]

Yes I've just made the rebase.

[deguif]

Is there anything I can do to help merging this one?

[shulard]

@deguif maybe yes ! Currently, we can't find the root cause of the travis failed build...

[deguif]

I think it's the vcr-plugin which requires php-client-common ^2.0. Removing it from composer.json should fix the issue, but maybe not wanted ;)
https://github.com/php-http/vcr-plugin/blob/master/composer.json#L16

[dbu]

hm, but the bundle allows client-common 2.0. is there another dependency in the same thing that is limited to client-common 1.0?

[deguif]

Let's investigate this issue more in details ;)

[deguif]

I think the issue is when requiring php-http/buzz-adapter:^1.0 php-http/guzzle6-adapter:^1.1.1 php-http/react-adapter:^0.2.1 php-http/socket-client:^1.0, httplug:^1.0 is required.

So it's not possible to use php-http/buzz-adapter:^1.0 php-http/guzzle6-adapter:^1.1.1 php-http/react-adapter:^0.2.1 php-http/socket-client:^1.0 and the vcr-plugin which requires explictly php-http/client-common:^2.0 as it would need httplug:^2.0, which is not possible as it was required by some packages at ^1.0

[dbu]

thanks for the analysis!

then i guess we should remove vcr-plugin from require-dev and only require it in some builds of the matrix (and skip related tests in the other builds by checking for existence of the vcr plugin class) @shulard do you want to try to do that?

[shulard]

Yes I try to do that asap.

[deguif]

I can also do the PR @shulard if you're busy at the moment.
It will probably be longer as I don't know the project currently ')

[shulard]

@deguif I just pushed a first try 😄

[deguif]

Nice all green ;)
This was quick. Thanks @shulard

[shulard]

Too quick, there was a typo in the .travis.yml file so I just forced push a fix 😄.

[shulard]

@dbu it seems ok now. I've added the vcr-plugin in the travis matrix during the tests on httplug 2.x clients. Do you think it's the only build on which it's required ?

[dbu]

awesome!

can you please add the vcr-plugin to the dev stability and the code-coverage build? the others are fine as is (and for lowest, it could be hiding problems if we have the plugin as it prevents installing with old versions)

[shulard]

I've updated the matrix, it still seems ok 😄.

[dbu]

awesome, thank you!

[dbu]

great, i am preparing a release now.

[deguif]

It seems I got problem now with CachePlugin, as it doesn't support Psr\Http\Message\StreamFactoryInterface

[shulard]

@deguif so after the update, you still got the error regarding HttplugFactory ? Same on my side I think...

[shulard]

We were a bit too enthousiast regarding that PR...

[deguif]

With the HttplugFactory probably as it doesn't return the PSR interfaces that were added here.
But on my side it doesn't work either with the Psr17Factory as the cache plugin doesn't support PSR interfaces but the Httplug ones.

[deguif]

Maybe we could change the constructor and its methods on the CachePlugin to take a Http\Message\StreamFactory or Psr\Http\Message\StreamFactoryInterface as they both return a Psr\Http\Message\StreamInterface with their createStream method.

And given the class is final, the backward compatibility for type-hint is not necessary ;)

What do you think @dbu?

[deguif]

@shulard @dbu I just opened: php-http/cache-plugin#56 so we can discuss on the issue I previously talked about.