Skip to content

Refactor the HttplugExtension #97

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 8 commits into from
Jul 24, 2016
Merged

Refactor the HttplugExtension #97

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3280a1f
Refactor the HttplugExtension
Nyholm Jul 20, 2016
0619c0a
minor refactor
Nyholm Jul 20, 2016
b661a82
style fix
Nyholm Jul 20, 2016
3951006
typo
Nyholm Jul 21, 2016
a1aae48
Minor fixes
Nyholm Jul 21, 2016
4f76044
Merge Profiling back to main extension
sagikazarmark Jul 23, 2016
bf2a5a4
Merge pull request #100 from php-http/dev-issue95-2
Nyholm Jul 24, 2016
e25ca31
added note about plugin client
Nyholm Jul 24, 2016
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
20 changes: 14 additions & 6 deletions ClientFactory/PluginClientFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,23 @@
final class PluginClientFactory
{
/**
* @param Plugin[] $plugins
* @param ClientFactory $factory
* @param array $config config to the client factory
* @param array $pluginClientOptions config forwarded to the PluginClient
* @param Plugin[] $plugins
* @param ClientFactory|callable $factory
* @param array $config config to the client factory
* @param array $pluginClientOptions config forwarded to the PluginClient
*
* @return PluginClient
*/
public static function createPluginClient(array $plugins, ClientFactory $factory, array $config, array $pluginClientOptions = [])
public static function createPluginClient(array $plugins, $factory, array $config, array $pluginClientOptions = [])
{
return new PluginClient($factory->createClient($config), $plugins, $pluginClientOptions);
if ($factory instanceof ClientFactory) {
$client = $factory->createClient($config);
} elseif (is_callable($factory)) {
$client = $factory($config);
} else {
throw new \RuntimeException(sprintf('Second argument to PluginClientFactory::createPluginClient must be a "%s" or a callable.', ClientFactory::class));
}

return new PluginClient($client, $plugins, $pluginClientOptions);
}
}
195 changes: 76 additions & 119 deletions DependencyInjection/HttplugExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
use Http\Client\Common\FlexibleHttpClient;
use Http\Client\Common\HttpMethodsClient;
use Http\Client\Common\Plugin\AuthenticationPlugin;
use Http\Client\Common\PluginClient;
use Http\Discovery\HttpAsyncClientDiscovery;
use Http\Discovery\HttpClientDiscovery;
use Http\HttplugBundle\ClientFactory\DummyClient;
use Http\HttplugBundle\Collector\DebugPlugin;
use Http\HttplugBundle\ClientFactory\PluginClientFactory;
use Http\Message\Authentication\BasicAuth;
use Http\Message\Authentication\Bearer;
use Http\Message\Authentication\Wsse;
Expand Down Expand Up @@ -39,21 +38,6 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('services.xml');
$loader->load('plugins.xml');

$toolbar = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug');

if ($toolbar) {
$loader->load('data-collector.xml');

if (!empty($config['toolbar']['formatter'])) {
// Add custom formatter
$container->getDefinition('httplug.collector.debug_collector')
->replaceArgument(0, new Reference($config['toolbar']['formatter']));
}

$container->getDefinition('httplug.formatter.full_http_message')
->addArgument($config['toolbar']['captured_body_length']);
}

foreach ($config['classes'] as $service => $class) {
if (!empty($class)) {
$container->register(sprintf('httplug.%s.default', $service), $class);
Expand All @@ -66,45 +50,47 @@ public function load(array $configs, ContainerBuilder $container)
}

$this->configurePlugins($container, $config['plugins']);
$this->configureClients($container, $config, $toolbar);
$this->configureAutoDiscoveryClients($container, $config, $toolbar);
$serviceIds = $this->configureClients($container, $config);
$autoServiceIds = $this->configureAutoDiscoveryClients($container, $config);

$toolbar = is_bool($config['toolbar']['enabled']) ? $config['toolbar']['enabled'] : $container->hasParameter('kernel.debug') && $container->getParameter('kernel.debug');
Copy link
Member

Choose a reason for hiding this comment

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

Can we not directly set the default value of this parameter to %kernel.debug% in the configuration class ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you reference container parameters in the Configuration.php? If so, we should

if ($toolbar) {
(new ProfilerExtension())->load($config, $container, array_unique(array_merge($serviceIds, $autoServiceIds)));
}
}

/**
* Configure client services.
*
* @param ContainerBuilder $container
* @param array $config
* @param bool $enableCollector
*
* @return array with client service names
*/
private function configureClients(ContainerBuilder $container, array $config, $enableCollector)
private function configureClients(ContainerBuilder $container, array $config)
{
// If we have a client named 'default'
$first = isset($config['clients']['default']) ? 'default' : null;
$serviceIds = [];
$first = null;

foreach ($config['clients'] as $name => $arguments) {
if ($first === null) {
// Save the name of the first configurated client.
$first = $name;
}

$this->configureClient($container, $name, $arguments, $enableCollector);
$serviceIds[] = $this->configureClient($container, $name, $arguments);
}

// If we have clients configured
if ($first !== null) {
if ($first !== 'default') {
// If we do not have a client named 'default'
if (!isset($config['clients']['default'])) {
// Alias the first client to httplug.client.default
$container->setAlias('httplug.client.default', 'httplug.client.'.$first);
}
} elseif ($enableCollector) {
$serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'default');
// No client was configured. Make sure to configure the auto discovery client with the PluginClient.
$container->register('httplug.client', PluginClient::class)
->addArgument(new Reference('httplug.client.default'))
->addArgument([])
->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]);
}

return $serviceIds;
}

/**
Expand Down Expand Up @@ -212,46 +198,34 @@ private function configureAuthentication(ContainerBuilder $container, array $con
* @param ContainerBuilder $container
* @param string $name
* @param array $arguments
* @param bool $enableCollector
*
* @return string The service id of the client.
*/
private function configureClient(ContainerBuilder $container, $name, array $arguments, $enableCollector)
private function configureClient(ContainerBuilder $container, $name, array $arguments)
{
$serviceId = 'httplug.client.'.$name;
$def = $container->register($serviceId, DummyClient::class);

// If there are no plugins nor should we use the data collector
if (empty($arguments['plugins']) && !$enableCollector) {
$def->setFactory([new Reference($arguments['factory']), 'createClient'])
->addArgument($arguments['config']);
} else {
$def
->setFactory('Http\HttplugBundle\ClientFactory\PluginClientFactory::createPluginClient')
->addArgument(
array_map(
function ($id) {
return new Reference($id);
},
$arguments['plugins']
)
$definition = $container->register($serviceId, DummyClient::class);
$definition->setFactory([PluginClientFactory::class, 'createPluginClient'])
->addArgument(
array_map(
function ($id) {
return new Reference($id);
},
$arguments['plugins']
)
->addArgument(new Reference($arguments['factory']))
->addArgument($arguments['config'])
;

if ($enableCollector) {
$serviceIdDebugPlugin = $this->registerDebugPlugin($container, $name);
$def->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]);
)
->addArgument(new Reference($arguments['factory']))
->addArgument($arguments['config'])
->addArgument([])
;

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

Choose a reason for hiding this comment

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

IMO this is still toolbar dependent. Service only declared in data_collector.xml?

->addMethodCall('setPlugins', [$name, $arguments['plugins']]);

/*
* Decorate the client with clients from client-common
*/

if ($arguments['flexible_client']) {
$container->register($serviceId.'.flexible', FlexibleHttpClient::class)
->addArgument(new Reference($serviceId.'.flexible.inner'))
Expand All @@ -265,91 +239,74 @@ function ($id) {
->setPublic(false)
->setDecoratedService($serviceId);
}
}

/**
* Create a new plugin service for this client.
*
* @param ContainerBuilder $container
* @param string $name
*
* @return string
*/
private function registerDebugPlugin(ContainerBuilder $container, $name)
{
$serviceIdDebugPlugin = 'httplug.client.'.$name.'.debug_plugin';
$container->register($serviceIdDebugPlugin, DebugPlugin::class)
->addArgument(new Reference('httplug.collector.debug_collector'))
->addArgument($name)
->setPublic(false);

return $serviceIdDebugPlugin;
return $serviceId;
}

/**
* Make sure we inject the debug plugin for clients found by auto discovery.
* 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.
*
* @param ContainerBuilder $container
* @param array $config
* @param bool $enableCollector
*
* @return array of service ids.
*/
private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config, $enableCollector)
private function configureAutoDiscoveryClients(ContainerBuilder $container, array $config)
{
$serviceIds = [];

$httpClient = $config['discovery']['client'];
if ($httpClient === 'auto') {
$httpClient = $this->registerAutoDiscoverableClientWithDebugPlugin(
$container,
'client',
[HttpClientDiscovery::class, 'find'],
$enableCollector
);
} elseif ($httpClient) {
if (!empty($httpClient)) {
if ($httpClient === 'auto') {
$httpClient = $this->registerAutoDiscoverableClient(
$container,
'auto_discovered_client',
[HttpClientDiscovery::class, 'find']
);
}

$serviceIds[] = $httpClient;
$httpClient = new Reference($httpClient);
}

$asyncHttpClient = $config['discovery']['async_client'];
if ($asyncHttpClient === 'auto') {
$asyncHttpClient = $this->registerAutoDiscoverableClientWithDebugPlugin(
$container,
'async_client',
[HttpAsyncClientDiscovery::class, 'find'],
$enableCollector
);
} elseif ($asyncHttpClient) {
if (!empty($asyncHttpClient)) {
if ($asyncHttpClient === 'auto') {
$asyncHttpClient = $this->registerAutoDiscoverableClient(
$container,
'auto_discovered_async',
[HttpAsyncClientDiscovery::class, 'find']
);
}
$serviceIds[] = $asyncHttpClient;
$asyncHttpClient = new Reference($httpClient);
}

$container->getDefinition('httplug.strategy')
->addArgument($httpClient)
->addArgument($asyncHttpClient);

return $serviceIds;
}

/**
* Find a client with auto discovery and return a service Reference to it.
*
* @param ContainerBuilder $container
* @param string $name
* @param callable $factory
* @param bool $enableCollector
*
* @return Reference
* @return string service id
*/
private function registerAutoDiscoverableClientWithDebugPlugin(ContainerBuilder $container, $name, $factory, $enableCollector)
private function registerAutoDiscoverableClient(ContainerBuilder $container, $name, $factory)
{
$definition = $container->register('httplug.auto_discovery_'.$name.'.pure', DummyClient::class);
$definition->setPublic(false);
$definition->setFactory($factory);

$pluginDefinition = $container
->register('httplug.auto_discovery_'.$name.'.plugin', PluginClient::class)
->setPublic(false)
->addArgument(new Reference('httplug.auto_discovery_'.$name.'.pure'))
->addArgument([])
;

if ($enableCollector) {
$serviceIdDebugPlugin = $this->registerDebugPlugin($container, 'auto_discovery_'.$name);
$pluginDefinition->addArgument(['debug_plugins' => [new Reference($serviceIdDebugPlugin)]]);
}
$serviceId = 'httplug.auto_discovery.'.$name;
$definition = $container->register($serviceId, DummyClient::class);
$definition
->setFactory([PluginClientFactory::class, 'createPluginClient'])
->setArguments([[], $factory, [], []]);

return new Reference('httplug.auto_discovery_'.$name.'.plugin');
return $serviceId;
}
}
Loading