Add default_client configuration option

[ruudk]

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

What's in this PR?

Added default_client configuration option to disable assigning the first client as default client and to remove the default client service.

Why?

Today we had an outage because we removed our the default client from our list of clients (multiple).

Some service was stil referencing httplug.client.default but this was not producing an error on CI.

It turns out this is a feature, which should be called a foot gun in my opinion:

HttplugBundle/src/DependencyInjection/HttplugExtension.php

Lines 144 to 153 in 51173c3

	// If we do not have a client named 'default'
	if (!array_key_exists('default', $config['clients'])) {
	$serviceId = 'httplug.client.'.$first;
	// Alias the first client to httplug.client.default
	$container->setAlias('httplug.client.default', $serviceId);
	$default = $first;
	} else {
	$default = 'default';
	$serviceId = 'httplug.client.'.$default;
	}

The logic of this is flawed. If one changes the order of the clients list, suddenly another client will become the default.

In my opinion we should completely remove this default auto selection mode, but that might be a breaking change no one wants to do right now.

So in this PR, I propose we add a configuration option to disable this logic.

Checklist

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

[ruudk]

/cc @dbu whenever you have time 🙏

[dbu]

picking the first service as default if no default is specified explicitly seems to be common practice in symfony, e.g. doctrine orm does it as well.

it is convenient when you have only one connection. but with multiple, i see the problem that implicit default is very non-obvious.

i would suggest we do not add a new configuration option - default_client_autowiring already allows to prevent autowiring.

instead, we should trigger a deprecation when more than 1 client is defined and default_client_autowiring is set to true and main_alias.client is not explicitly set, to ask people to explicitly chose which client they want to autowire when the service does not specify which client it wants.

[ruudk]

instead, we should trigger a deprecation when more than 1 client is defined and default_client_autowiring is set to true and main_alias.client is not explicitly set, to ask people to explicitly chose which client they want to autowire when the service does not specify which client it wants.

In our project, we don't want to have a default, because we want developers to set specific clients per use case. So that every use case can be configured on their own without impacting other use cases. How to deal with that?

[ruudk]

it is convenient when you have only one connection

It could still be done when there is only 1 client defined, mark that as default.

If you add more, stop doing it.

[dbu]

i don't think we should remove this definition. it has no semantic meaning for symfony, and if i define a client that i call default it would be very unexpected to just nuke my definition.

however, we should additionally remove the httplug.psr18_client.default alias (from the main_classes)

[ruudk]

That's a good catch indeed. It's naive for me to just remove this, because I assume it's the default one created by the bundle.

[ruudk]

What if we create a new major version, where we do the following:

• Remove default client
• Remove auto wiring for default client
• Make it possible to set a given client as default: true.
• When there is a default configured client, we enable auto wire for that specific client

[dbu]

the default_client_autowiring: false was already meant to do this, it removes the aliasing for psr18 ClientInterface and the httplug async client interface. however, it does not prevent creating the alias for a client named default. which seems weird to do when we don't want autowiring.

to avoid BC breaks, i suggest we add default_client which if set to false would do the same as default_client_autowiring: false plus not create/remove the alias for httplug.client.default and httplug.psr18_client.default.

if main_alias.client or main_alias.psr18_client is explicitly configured but default_client: false, we should report an error. it makes no sense to configure those but then disable autowiring. (we should have done that check for default_client_autowiring too)

i want to keep the default to do autowiring. this seems more in line with what symfony does, e.g. the symfony http client.