php-http/HttplugBundle

AutoWiring and HttpMethodsClient

soullivaneuh opened this issue · 6 comments

Q A
Bug? no
New Feature? yes
Version 1.10.0

With the following configuration:

httplug:
    clients:
        default:
            factory: 'httplug.factory.guzzle6'
            config:
                timeout: 5

The bundle provide autowiring for the default http client:

  Http\Client\HttpAsyncClient                                                                     
      alias to httplug.async_client.default                                                       
  Http\Client\HttpClient                                                                          
      alias to httplug.client.default                                                             

It's also possible to enable the http methods client:

httplug:
    clients:
        default:
            factory: 'httplug.factory.guzzle6'
            config:
                timeout: 5
            http_methods_client: true

But the autowiring still rely on HttpClient interface:

  Http\Client\HttpAsyncClient                                                                     
      alias to httplug.async_client.default                                                       
  Http\Client\HttpClient                                                                          
      alias to httplug.client.default.http_methods                                                

So, we of course have a HttpMethodsClient, but the following code sample does not rely on anything:

public function faviconRedirectAction(string $domain, int $minSize, HttpClient $client): Response
{
    dump($client);die;

And we have to check this with instanceof in order to not make some tools like PHPStan crying a lot.

It would be wonderful to be able to do something like this:

public function faviconRedirectAction(string $domain, int $minSize, HttpMethodsClient $client): Response
{
    dump($client);die;

And directly have autowiring working.

What do you think?

The workaround for people interested like me is pretty simple, one line on your services definition:

services:
    Http\Client\Common\HttpMethodsClient: '@httplug.client.default.http_methods'
dbu commented

i agree that we should do an autowire alias for HttpMethodsClient. that client is not a standard but a specific thing, and if you want to use that, you should typehint it like that, not as HttpClient and just hope that you get the right one.

do you want to do a pull request to add that service alias in the bundle, @soullivaneuh ?

To make things cleaner, it would be better to create a HttpMethodsClientInterface first.

WDYT?

I’m not sure an interface is needed.

dbu commented

agree with tobias, the methods client is a pure convenience layer and not a generic interface. i'd go for the concrete class in that case.

I'm fine with the idea.

do you want to do a pull request to add that service alias in the bundle

@dbu To be honest, I don't know when I'll have the time to add the service and the test.

I opened the issue for the idea, this is not very urgent to me. 😉

An interface is now added in client-common: 2.0