Add support for autowiring

[csarrazi]

Currently, even when using a single service, one can not rely on autowiring to connect the defined service to another one. The only way to make this work currently is to make the service public, and inject the container using autowiring in the service.

The bundle should work in a similar manner, with or without autowiring.

[csarrazi]

@dunglas Do you have any pointers for that one?

[csarrazi]

From https://github.com/csarrazi/CsaGuzzleBundle/issues/217#issuecomment-351312848

They'll need to the $ syntax:

_defaults:
  autowire: true

Foo\Bar: {$client: 'guzzle_service'}

I guess the best approach would be to document this.

@dunglas: Is there any equivalent to that syntax using the container builder API?

[csarrazi]

Nvm, found it there: https://symfony.com/doc/current/service_container.html#services-manually-wire-args

Thanks for the pointer, @dunglas!

[BPScott]

Something that would help with autowiring and avoid the need for specifying explicit arguments (the $ syntax above), is to create aliases for GuzzleHttp\ClientInterface and GuzzleHttp\Client which refer to a particular client.

That way any service that typehints against GuzzleHttp\ClientInterface and GuzzleHttp\Client would use that "default" client, unless you specify an explicit one using the $ syntax.

Consider I have a trio of classes App\MyConsumerPrimary, App\MyConsumerSecondary and App\MyConsumerAutomatic, all of which take an instance of GuzzleHttp\ClientInterface as the "client" argument, and some configuration like:

csa_guzzle:
    clients:
        primary:
            alias: 'GuzzleHttp\ClientInterface' # This is the important line!
            config: [timeout: 5]
        secondary:
            config: [timeout: 10]

services:
    _defaults:
        autoconfigure: true
        autowire: true
        public: false

    App\MyConsumerPrimary:  {$client: '@csa_guzzle.client.primary'}
    App\MyConsumerSecondary: {$client: '@csa_guzzle.client.secondary'}
    App\MyConsumerAutomatic: ~

In this case App\MyConsumerPrimary and App\MyConsumerSecondary will get the explicitly defined client instances (as you've already noted above).

App\MyConsumerAutomatic meanwhile will look for a service named after the typehint of the argument - GuzzleHttp\ClientInterface - which does exist thanks to the alias line under the primary client. Thus the @csa_guzzle.client.primary service gets injected into this class.

---

Unfortunately people in userland might typehint against the the implementation (GuzzleHttp\Client) rather than the interface, so I think it would be best to provide some mechansim to register aliases for both Client and ClientInterface. I think this could be done in one of two ways:

1)" client->{name}->alias" needs to be expanded to allow multiple aliases. PROs: building on existing behaviour. CONs: people might attempt to alias multiple clients to the default name, leading to confusing errors. 2) a new property "client->autowired_client" that takes a service reference (e.g. '@csa_guzzle.client.primary'). PROS: a single place to set a value, it sits atop clients CONS: new config

After that you could automatically register the aliases if the user specified only a single client.

[csarrazi]

Hum, 1) Could be solved relatively easily by adding sanity checks in the DI extension, which will check for unicity of the service. We could also automatically add the alias should the service be the only one defined. 2) Having a new config option is fine too. It's not like the bundle has a huge list of configuration options anyways ;) And like the previous point, we can always define the auto wired service to be the only first service defined by default, and let the user override it.

I think I like option 2 better. I think it makes things less error prone, and I do think that offering the option to users to define multiple aliases for the same service may lead into unexpected errors (re-defining the alias multiple times, with different targets, which would make DI use the same client for all services relying on auto-wiring, without any control on which service would be the one defined).

[BPScott]

I agree option two is preferable as well 👍 I'm not 100% on the naming of the new config option, "autowired_client" was a hasty strawman to communicate the idea but I can't think of anything better atm

[csarrazi]

We could call that default_client, to keep the same terminology as in other projects

[BPScott]

Massive +1 for consistency with others

[csarrazi]

@BPScott Could you try branch feature/di-autowiring? It works flawlessly and tests seem to confirm that it works correctly, but I want to make sure that I'm not breaking anything.

[csarrazi]

Solved in #222