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: 5The 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: trueBut 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'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.
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