HttpMethodsClient not working with PSR-17

[jdecool]

Description
The HttpMethodsClient constructor allows to get an RequestFactory or RequestFactoryInterface as second parameter.

Both declare the createRequest but don't have the same argument.

RequestFactoryInterface::createRequest only have two parameters method and uri.

The HttpMethodsClient::send method which is used to make the HTTP call have two other parameters. So when using a PSR-17 implementation, the code will not work as the body and/or headers won't be sent.

How to reproduce

$http = new HttpMethodsClient(HttpClientDiscovery::find(), Psr17FactoryDiscovery::findRequestFactory());
$response = $http->get($uri, $headers);

Headers will not be sent.

[GrahamCampbell]

I cannot replicate this. Which particular client and request factory are you discovering. Maybe they have a bug?

EDIT: I couldn't replicate this because my request factory happened to implement both interfaces.

[jdecool]

I use guzzle6-adapter as it doesn't provide a PSR-17 implementation, I use it with nyholm/psr7.

But beyond that, if we look at the interfaces definitions:

namespace Http\Message;

interface RequestFactory
{
    public function createRequest($method, $uri, array $headers = [], $body = null, $protocolVersion = '1.1');
}

namespace Psr\Http\Message;

interface RequestFactoryInterface
{
    public function createRequest(string $method, $uri): RequestInterface;
}

And how the send method is implemented:

public function send(string $method, $uri, array $headers = [], $body = null): ResponseInterface
{
    return $this->sendRequest($this->requestFactory->createRequest(
        $method,
        $uri,
        $headers,
        $body
    ));
}

There is a problem.

Do you have an example with a working PSR-17 implementation ?

[GrahamCampbell]

php-http/guzzle6-adapter:^2.0.1 http-interop/http-factory-guzzle:^1.0 should work correctly. You don't need two different psr7 implementations (Guzzle already provides one).

NB You can also use Guzzle 7: guzzlehttp/guzzle:^7.0.1 http-interop/http-factory-guzzle:^1.0.

[jdecool]

Not working.

I use those dependencies:

"require": {
        "php": "^7.3",
        "ext-json": "*",
        "php-http/client-common": "^2.2",
        "php-http/client-implementation": "^1.0",
        "php-http/discovery": "^1.6",
        "php-http/httplug": "^2.0",
        "http-interop/http-factory-guzzle": "^1.0",
        "php-http/guzzle6-adapter": "^2.0"
},

With this code:

$http = new Http\Client\Common\HttpMethodsClient(
    Http\Discovery\HttpClientDiscovery::find(),
    Http\Discovery\Psr17FactoryDiscovery::findRequestFactory(),
);

$response = $http->get('http://foo/bar', ['My-Header' => 'Value']);

A debug in HttpMethodsClient::send:

And in it's implementation:

Headers are not set.

[jdecool]

I don't understand how the code could work as the two interfaces allowed in the HttpMethodsClient don't have the same contract.

[GrahamCampbell]

Thanks for those details. Very helpful. :)

    "php-http/client-implementation": "^1.0",

You don't need this line, in fact you should not add it.

Headers are not set.

Let me take a look at this in more detail...

[GrahamCampbell]

I think you absolutely are right, and the reason I hadn't seen this bug before is because I happened to be using something that was also a PHP-HTTP request factory, and not just a PSR one. There is definitely a bug here. I will prepare a PR to fix this. :)

[GrahamCampbell]

@jdecool Please let me know what you think of #202.

[jdecool]

Please let me know what you think of #202.

Look good to me