Skip to content

Use ClientInterface instead of HttpClient interface when registering aliases #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 26, 2021
Merged

Use ClientInterface instead of HttpClient interface when registering aliases #394

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a7d6c8
Register client alias for `Psr\Http\Client\ClientInterface`
ruudk Jul 12, 2021
85f511d
Deprecate client alias registration on HttpClient interface
ruudk Jul 12, 2021
0006ec2
Check if HttpClient implements ClientInterface
ruudk Jul 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/DependencyInjection/HttplugExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Http\Message\Authentication\QueryParam;
use Http\Message\Authentication\Wsse;
use Http\Mock\Client as MockClient;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\UriInterface;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Alias;
Expand Down Expand Up @@ -382,7 +383,14 @@ private function configureClient(ContainerBuilder $container, $clientName, array
$serviceId = 'httplug.client.'.$clientName;

if (method_exists($container, 'registerAliasForArgument')) {
$container->registerAliasForArgument($serviceId, HttpClient::class, $clientName);
$alias = $container->registerAliasForArgument($serviceId, HttpClient::class, $clientName);

$interfaces = class_implements(HttpClient::class) ?? [];
if (isset($interfaces[ClientInterface::class])) {
$alias->setDeprecated('php-http/httplug-bundle', '1.22', 'The "%alias_id%" alias is deprecated, use "Psr\Http\Client\ClientInterface" instead.');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruudk somebody reported that symfony complains about the deprecation warning, saying it needs an %alias_id% (though i see that here)... https://twitter.com/PP_Vortex/status/1419599426084032519

can you please check if the deprecation warning works correctly for you and there is no symfony warning about alias_id?

Copy link

@franziskahahn franziskahahn Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some deep dive into vendors and found out the following fact:

the method setDeprecated in Alias.php changed in symfony 5.3 compared to symfony 4.4:
https://github.com/symfony/dependency-injection/blob/5.3/Alias.php
https://github.com/symfony/dependency-injection/blob/4.4/Alias.php

In symfony 4.4, the second parameter is the template. So when using http plug bundle with symfony 4.4 executing assets:install interpretes this line as "deprecation template is '1.22'" and this string obviously doesn't contain %alias_id%.

$alias->setDeprecated('php-http/httplug-bundle', '1.22', 'The "%alias_id%" alias is deprecated, use "Psr\Http\Client\ClientInterface" instead.'); is fine for symfony 5.3 but not for symfony 4.4

My conclusion: Currently, php-http/httplug-bundle 1.22 is not compatible with symfony 4.4 but seems to be compatible with symfony 5.3

What would be the next steps here? Would you like to be compatible with both versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check it out and make a PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #396

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for reporting the issue and for the analysis @franziskahahn. the deprecation is removed in https://github.com/php-http/HttplugBundle/releases/tag/1.22.1


$container->registerAliasForArgument($serviceId, ClientInterface::class, $clientName);
}
}

$plugins = [];
Expand Down