Skip to content

Allow to configure plugins specifically for a client #112

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.

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 1 commit into from
Aug 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

### Added

- You can now also configure client specific plugins in the `plugins` option of a client.
Some plugins that you previously had to define in your own services can now be configured on the client.
- Support for BatchClient
- The stopwatch plugin in included by default when using profiling.

Expand Down
352 changes: 253 additions & 99 deletions DependencyInjection/Configuration.php

Large diffs are not rendered by default.

111 changes: 87 additions & 24 deletions DependencyInjection/HttplugExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
use Http\Message\Authentication\BasicAuth;
use Http\Message\Authentication\Bearer;
use Http\Message\Authentication\Wsse;
use Psr\Http\Message\UriInterface;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\DependencyInjection\Reference;
Expand Down Expand Up @@ -53,7 +55,7 @@ public function load(array $configs, ContainerBuilder $container)
}

// Configure toolbar
if ($config['profiling']['enabled']) {
if ($this->isConfigEnabled($container, $config['profiling'])) {
$loader->load('data-collector.xml');

if (!empty($config['profiling']['formatter'])) {
Expand All @@ -70,8 +72,8 @@ public function load(array $configs, ContainerBuilder $container)
;
}

$this->configurePlugins($container, $config['plugins']);
$this->configureClients($container, $config);
$this->configureSharedPlugins($container, $config['plugins']); // must be after clients, as clients.X.plugins might use plugins as templates that will be removed
$this->configureAutoDiscoveryClients($container, $config);
}

Expand All @@ -91,7 +93,7 @@ private function configureClients(ContainerBuilder $container, array $config)
$first = $name;
}

$this->configureClient($container, $name, $arguments, $config['profiling']['enabled']);
$this->configureClient($container, $name, $arguments, $this->isConfigEnabled($container, $config['profiling']));
}

// If we have clients configured
Expand All @@ -108,7 +110,7 @@ private function configureClients(ContainerBuilder $container, array $config)
* @param ContainerBuilder $container
* @param array $config
*/
private function configurePlugins(ContainerBuilder $container, array $config)
private function configureSharedPlugins(ContainerBuilder $container, array $config)
{
if (!empty($config['authentication'])) {
$this->configureAuthentication($container, $config['authentication']);
Expand All @@ -118,21 +120,23 @@ private function configurePlugins(ContainerBuilder $container, array $config)
foreach ($config as $name => $pluginConfig) {
$pluginId = 'httplug.plugin.'.$name;

if ($pluginConfig['enabled']) {
if ($this->isConfigEnabled($container, $pluginConfig)) {
$def = $container->getDefinition($pluginId);
$this->configurePluginByName($name, $def, $pluginConfig);
$this->configurePluginByName($name, $def, $pluginConfig, $container, $pluginId);
} else {
$container->removeDefinition($pluginId);
}
}
}

/**
* @param string $name
* @param Definition $definition
* @param array $config
* @param string $name
* @param Definition $definition
* @param array $config
* @param ContainerBuilder $container In case we need to add additional services for this plugin
* @param string $serviceId Service id of the plugin, in case we need to add additional services for this plugin.
*/
private function configurePluginByName($name, Definition $definition, array $config)
private function configurePluginByName($name, Definition $definition, array $config, ContainerInterface $container, $serviceId)
{
switch ($name) {
case 'cache':
Expand Down Expand Up @@ -173,6 +177,23 @@ private function configurePluginByName($name, Definition $definition, array $con
$definition->replaceArgument(0, new Reference($config['stopwatch']));
break;

/* client specific plugins */

case 'add_host':
$uriService = $serviceId.'.host_uri';
$this->createUri($container, $uriService, $config['host']);
$definition->replaceArgument(0, new Reference($uriService));
$definition->replaceArgument(1, [
'replace' => $config['replace'],
]);
break;
case 'header_append':
case 'header_defaults':
case 'header_set':
case 'header_remove':
$definition->replaceArgument(0, $config['headers']);
break;

default:
throw new \InvalidArgumentException(sprintf('Internal exception: Plugin %s is not handled', $name));
}
Expand All @@ -181,11 +202,15 @@ private function configurePluginByName($name, Definition $definition, array $con
/**
* @param ContainerBuilder $container
* @param array $config
*
* @return array List of service ids for the authentication plugins.
*/
private function configureAuthentication(ContainerBuilder $container, array $config)
private function configureAuthentication(ContainerBuilder $container, array $config, $servicePrefix = 'httplug.plugin.authentication')
{
$pluginServices = [];

foreach ($config as $name => $values) {
$authServiceKey = sprintf('httplug.plugin.authentication.%s.auth', $name);
$authServiceKey = sprintf($servicePrefix.'.%s.auth', $name);
switch ($values['type']) {
case 'bearer':
$container->register($authServiceKey, Bearer::class)
Expand All @@ -208,33 +233,54 @@ private function configureAuthentication(ContainerBuilder $container, array $con
throw new \LogicException(sprintf('Unknown authentication type: "%s"', $values['type']));
}

$container->register('httplug.plugin.authentication.'.$name, AuthenticationPlugin::class)
->addArgument(new Reference($authServiceKey));
$pluginServiceKey = $servicePrefix.'.'.$name;
$container->register($pluginServiceKey, AuthenticationPlugin::class)
->addArgument(new Reference($authServiceKey))
;
$pluginServices[] = $pluginServiceKey;
}

return $pluginServices;
}

/**
* @param ContainerBuilder $container
* @param string $name
* @param string $clientName
* @param array $arguments
* @param bool $profiling
*/
private function configureClient(ContainerBuilder $container, $name, array $arguments, $profiling)
private function configureClient(ContainerBuilder $container, $clientName, array $arguments, $profiling)
{
$serviceId = 'httplug.client.'.$name;
$serviceId = 'httplug.client.'.$clientName;

$plugins = [];
foreach ($arguments['plugins'] as $plugin) {
list($pluginName, $pluginConfig) = each($plugin);
if ('reference' === $pluginName) {
$plugins[] = $pluginConfig['id'];
} elseif ('authentication' === $pluginName) {
$plugins = array_merge($plugins, $this->configureAuthentication($container, $pluginConfig, $serviceId.'.authentication'));
} else {
$pluginServiceId = $serviceId.'.plugin.'.$pluginName;
$def = clone $container->getDefinition('httplug.plugin'.'.'.$pluginName);
$def->setAbstract(false);
$this->configurePluginByName($pluginName, $def, $pluginConfig, $container, $pluginServiceId);
$container->setDefinition($pluginServiceId, $def);
$plugins[] = $pluginServiceId;
}
}

$pluginClientOptions = [];

if ($profiling) {
// Add the stopwatch plugin
if (!in_array('httplug.plugin.stopwatch', $arguments['plugins'])) {
// Add the stopwatch plugin
array_unshift($arguments['plugins'], 'httplug.plugin.stopwatch');
array_unshift($plugins, 'httplug.plugin.stopwatch');
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO: handle more plugins

// Tell the plugin journal what plugins we used
$container
->getDefinition('httplug.collector.plugin_journal')
->addMethodCall('setPlugins', [$name, $arguments['plugins']])
->addMethodCall('setPlugins', [$clientName, $plugins])
;

$debugPluginServiceId = $this->registerDebugPlugin($container, $serviceId);
Expand All @@ -250,7 +296,7 @@ private function configureClient(ContainerBuilder $container, $name, array $argu
function ($id) {
return new Reference($id);
},
$arguments['plugins']
$plugins
)
)
->addArgument(new Reference($arguments['factory']))
Expand Down Expand Up @@ -290,6 +336,23 @@ function ($id) {
}
}

/**
* Create a URI object with the default URI factory.
*
* @param ContainerBuilder $container
* @param string $serviceId Name of the private service to create
* @param string $uri String representation of the URI
*/
private function createUri(ContainerBuilder $container, $serviceId, $uri)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should we use an abstract service that we clone and update instead of this?

Copy link
Member

Choose a reason for hiding this comment

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

I do not mind either way. Let's leave it.

{
$container
->register($serviceId, UriInterface::class)
->setPublic(false)
->setFactory([new Reference('httplug.uri_factory'), 'createUri'])
->addArgument($uri)
;
}

/**
* Make the user can select what client is used for auto discovery. If none is provided, a service will be created
* by finding a client using auto discovery.
Expand All @@ -307,7 +370,7 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
$container,
'auto_discovered_client',
[HttpClientDiscovery::class, 'find'],
$config['profiling']['enabled']
$this->isConfigEnabled($container, $config['profiling'])
Copy link
Member

Choose a reason for hiding this comment

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

I've never seen this. Why is it needed? Is it to make sure we do not get "undefined key 'enabled'"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its the counterpart to canBeEnabled/canBeDisabled. i feel when we use canBeEnabled/Disabled we should also use this method.

Copy link
Member

Choose a reason for hiding this comment

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

Okey, Cool

);
}

Expand All @@ -322,7 +385,7 @@ private function configureAutoDiscoveryClients(ContainerBuilder $container, arra
$container,
'auto_discovered_async',
[HttpAsyncClientDiscovery::class, 'find'],
$config['profiling']['enabled']
$this->isConfigEnabled($container, $config['profiling'])
);
}

Expand Down
20 changes: 20 additions & 0 deletions Resources/config/plugins.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,25 @@
<service id="httplug.plugin.stopwatch" class="Http\Client\Common\Plugin\StopwatchPlugin" public="false">
<argument />
</service>

<!-- client specific plugin definition prototypes -->

<service id="httplug.plugin.add_host" class="Http\Client\Common\Plugin\AddHostPlugin" public="false" abstract="true">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

abstract=true makes symfony remove the definition template when the container is built.

<argument/>
<argument/>
</service>
<service id="httplug.plugin.header_append" class="Http\Client\Common\Plugin\HeaderAppendPlugin" public="false" abstract="true">
<argument/>
</service>
<service id="httplug.plugin.header_defaults" class="Http\Client\Common\Plugin\HeaderDefaultsPlugin" public="false" abstract="true">
<argument/>
</service>
<service id="httplug.plugin.header_set" class="Http\Client\Common\Plugin\HeaderSetPlugin" public="false" abstract="true">
<argument/>
</service>
<service id="httplug.plugin.header_remove" class="Http\Client\Common\Plugin\HeaderRemovePlugin" public="false" abstract="true">
<argument/>
</service>

</services>
</container>
3 changes: 3 additions & 0 deletions Tests/Functional/ServiceInstantiationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ public function testDebugToolbar()
$plugins = $journal->getPlugins('acme');
$this->assertEquals([
'httplug.plugin.stopwatch',
'httplug.client.acme.plugin.decoder',
'httplug.plugin.redirect',
'httplug.client.acme.plugin.add_host',
'httplug.client.acme.authentication.my_basic',
], $plugins);
}
}
37 changes: 37 additions & 0 deletions Tests/Resources/Fixtures/config/full.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,43 @@
'uri_factory' => 'Http\Message\UriFactory\GuzzleUriFactory',
'stream_factory' => 'Http\Message\StreamFactory\GuzzleStreamFactory',
],
'clients' => [
'test' => [
'factory' => 'httplug.factory.guzzle6',
'http_methods_client' => true,
'plugins' => [
'httplug.plugin.redirect',
[
'add_host' => [
'host' => 'http://localhost',
],
],
[
'header_set' => [
'headers' => [
'X-FOO' => 'bar',
],
],
],
[
'header_remove' => [
'headers' => [
'X-FOO',
],
],
],
[
'authentication' => [
'my_basic' => [
'type' => 'basic',
'username' => 'foo',
'password' => 'bar',
],
],
]
],
],
],
'profiling' => [
'enabled' => true,
'formatter' => 'my_toolbar_formatter',
Expand Down
21 changes: 21 additions & 0 deletions Tests/Resources/Fixtures/config/full.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,27 @@
<uri-factory>Http\Message\UriFactory\GuzzleUriFactory</uri-factory>
<stream-factory>Http\Message\StreamFactory\GuzzleStreamFactory</stream-factory>
</classes>
<client name="test" factory="httplug.factory.guzzle6" http-methods-client="true">
<plugin>httplug.plugin.redirect</plugin>
<plugin>
<add-host host="http://localhost"/>
</plugin>
<plugin>
<header-set>
<header name="X-FOO">bar</header>
</header-set>
</plugin>
<plugin>
<header-remove>
<header>X-FOO</header>
</header-remove>
</plugin>
<plugin>
<authentication>
<my_basic type="basic" username="foo" password="bar"/>
</authentication>
</plugin>
</client>
<profiling enabled="true" formatter="my_toolbar_formatter" captured_body_length="0"/>
<plugins>
<authentication>
Expand Down
22 changes: 22 additions & 0 deletions Tests/Resources/Fixtures/config/full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@ httplug:
message_factory: Http\Message\MessageFactory\GuzzleMessageFactory
uri_factory: Http\Message\UriFactory\GuzzleUriFactory
stream_factory: Http\Message\StreamFactory\GuzzleStreamFactory
clients:
test:
factory: httplug.factory.guzzle6
http_methods_client: true
plugins:
- 'httplug.plugin.redirect'
-
add_host:
host: http://localhost
-
header_set:
headers:
X-FOO: bar
-
header_remove:
headers: [X-FOO]
-
authentication:
my_basic:
type: basic
username: foo
password: bar
profiling:
enabled: true
formatter: my_toolbar_formatter
Expand Down
6 changes: 6 additions & 0 deletions Tests/Resources/Fixtures/config/invalid_plugin.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
httplug:
clients:
acme:
plugins:
- foobar:
baz: ~
Loading