Skip to content

Add PluginClientFactory support #202

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 13 commits into from
Sep 5, 2017
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
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ matrix:

before_install:
- if [ "$SYMFONY_VERSION" != "" ]; then composer require "symfony/symfony:${SYMFONY_VERSION}" --no-update; fi;
- if [ "$DEPENDENCIES" = "dev" ]; then sed -i '/prefer-stable/d' composer.json; fi;
- if [ "$DEPENDENCIES" = "dev" ]; then perl -pi -e 's/^}$/,"minimum-stability":"dev"}/' composer.json; fi;
- if [ $COVERAGE != true ]; then phpenv config-rm xdebug.ini; fi;

Expand Down
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,23 @@

The change log describes what is "Added", "Removed", "Changed" or "Fixed" between each release.

## Unreleased

### Added

- Any third party library using `Http\Client\Common\PluginClientFactory` to create `Http\Client\Common\PluginClient`
instances now gets zero config profiling.
- `Http\Client\Common\PluginClientFactory` factory service.

### Changed

- `ProfilePlugin` and `StackPlugin` are no longer registered as (private) services decorators. Those decorators are now
created through the `Http\HttplugBundle\Collector\PluginClientFactory`.

### Deprecated

- The `Http\HttplugBundle\ClientFactory\PluginClientFactory` class.

## 1.7.1 - 2017-08-04

### Fixed
Expand Down
4 changes: 4 additions & 0 deletions ClientFactory/PluginClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

namespace Http\HttplugBundle\ClientFactory;

@trigger_error('The '.__NAMESPACE__.'\PluginClientFactory class is deprecated since version 1.8 and will be removed in 2.0. Use Http\Client\Common\PluginClientFactory instead.', E_USER_DEPRECATED);

use Http\Client\Common\Plugin;
use Http\Client\Common\PluginClient;

/**
* This factory creates a PluginClient.
*
* @deprecated
*
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
final class PluginClientFactory
Expand Down
77 changes: 77 additions & 0 deletions Collector/PluginClientFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php

namespace Http\HttplugBundle\Collector;

use Http\Client\Common\Plugin;
use Http\Client\Common\PluginClient;
use Http\Client\HttpAsyncClient;
use Http\Client\HttpClient;
use Symfony\Component\Stopwatch\Stopwatch;

/**
* This factory is used as a replacement for Http\Client\Common\PluginClientFactory when profiling is enabled. It
* creates PluginClient instances with all profiling decorators and extra plugins.
*
* @author Fabien Bourigault <bourigaultfabien@gmail.com>
*
* @internal
*/
final class PluginClientFactory
{
/**
* @var Collector
*/
private $collector;

/**
* @var Formatter
*/
private $formatter;

/**
* @var Stopwatch
*/
private $stopwatch;

/**
* @param Collector $collector
* @param Formatter $formatter
* @param Stopwatch $stopwatch
*/
public function __construct(Collector $collector, Formatter $formatter, Stopwatch $stopwatch)
{
$this->collector = $collector;
$this->formatter = $formatter;
$this->stopwatch = $stopwatch;
}

/**
* @param HttpClient|HttpAsyncClient $client
* @param Plugin[] $plugins
* @param array $options {
*
* @var string $client_name to give client a name which may be used when displaying client information like in
* the HTTPlugBundle profiler.
* }
*
* @see PluginClient constructor for PluginClient specific $options.
*
* @return PluginClient
*/
public function createClient($client, array $plugins = [], array $options = [])
{
$plugins = array_map(function (Plugin $plugin) {
return new ProfilePlugin($plugin, $this->collector, $this->formatter);
}, $plugins);

$clientName = isset($options['client_name']) ? $options['client_name'] : 'Default';
array_unshift($plugins, new StackPlugin($this->collector, $this->formatter, $clientName));
unset($options['client_name']);

if (!$client instanceof ProfileClient) {
$client = new ProfileClient($client, $this->collector, $this->formatter, $this->stopwatch);
}

return new PluginClient($client, $plugins, $options);
}
}
54 changes: 54 additions & 0 deletions Collector/PluginClientFactoryListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

namespace Http\HttplugBundle\Collector;

use Http\Client\Common\PluginClientFactory;
use Http\HttplugBundle\Collector\PluginClientFactory as CollectorPluginClientFactory;
use Symfony\Component\EventDispatcher\Event;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* This subscriber ensures that every PluginClient created when using Http\Client\Common\PluginClientFactory without
* using the Symfony dependency injection container uses the Http\HttplugBundle\Collector\PluginClientFactory factory
* when profiling is enabled. This allows 0 config profiling of third party libraries which use HTTPlug.
*
* @author Fabien Bourigault <bourigaultfabien@gmail.com>
*
* @internal
*/
final class PluginClientFactoryListener implements EventSubscriberInterface
{
/**
* @var CollectorPluginClientFactory
*/
private $factory;

/**
* @param CollectorPluginClientFactory $factory
*/
public function __construct(CollectorPluginClientFactory $factory)
{
$this->factory = $factory;
}

/**
* Make sure to profile clients created using PluginClientFactory.
*
* @param Event $e
*/
public function onEvent(Event $e)
{
PluginClientFactory::setFactory([$this->factory, 'createClient']);
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents()
{
return [
'kernel.request' => ['onEvent', 1024],
'console.command' => ['onEvent', 1024],
];
}
}
82 changes: 17 additions & 65 deletions DependencyInjection/HttplugExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
use Http\Client\Common\HttpMethodsClient;
use Http\Client\Common\Plugin\AuthenticationPlugin;
use Http\Client\Common\PluginClient;
use Http\HttplugBundle\ClientFactory\PluginClientFactory;
use Http\HttplugBundle\Collector\ProfilePlugin;
use Http\Client\Common\PluginClientFactory;
use Http\Client\HttpClient;
use Http\Message\Authentication\BasicAuth;
use Http\Message\Authentication\Bearer;
use Http\Message\Authentication\Wsse;
Expand Down Expand Up @@ -72,7 +72,7 @@ public function load(array $configs, ContainerBuilder $container)
;
}

$this->configureClients($container, $config, $profilingEnabled);
$this->configureClients($container, $config);
$this->configurePlugins($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 @@ -82,9 +82,8 @@ public function load(array $configs, ContainerBuilder $container)
*
* @param ContainerBuilder $container
* @param array $config
* @param bool $profiling
*/
private function configureClients(ContainerBuilder $container, array $config, $profiling)
private function configureClients(ContainerBuilder $container, array $config)
{
$first = null;
$clients = [];
Expand All @@ -95,7 +94,7 @@ private function configureClients(ContainerBuilder $container, array $config, $p
$first = $name;
}

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

Expand Down Expand Up @@ -266,9 +265,8 @@ private function configureAuthentication(ContainerBuilder $container, array $con
* @param ContainerBuilder $container
* @param string $clientName
* @param array $arguments
* @param bool $profiling
*/
private function configureClient(ContainerBuilder $container, $clientName, array $arguments, $profiling)
private function configureClient(ContainerBuilder $container, $clientName, array $arguments)
{
$serviceId = 'httplug.client.'.$clientName;

Expand All @@ -285,23 +283,17 @@ private function configureClient(ContainerBuilder $container, $clientName, array
}
}

$pluginClientOptions = [];
if ($profiling) {
//Decorate each plugin with a ProfilePlugin instance.
$plugins = array_map(function ($pluginServiceId) use ($container) {
$this->decoratePluginWithProfilePlugin($container, $pluginServiceId);

return $pluginServiceId.'.debug';
}, $plugins);

// To profile the requests, add a StackPlugin as first plugin in the chain.
$stackPluginId = $this->configureStackPlugin($container, $clientName, $serviceId);
array_unshift($plugins, $stackPluginId);
}
$container
->register($serviceId.'.client', HttpClient::class)
->setFactory([new Reference($arguments['factory']), 'createClient'])
->addArgument($arguments['config'])
->setPublic(false)
;

$container
->register($serviceId, PluginClient::class)
->setFactory([PluginClientFactory::class, 'createPluginClient'])
->setFactory([new Reference(PluginClientFactory::class), 'createClient'])
->addArgument(new Reference($serviceId.'.client'))
->addArgument(
array_map(
function ($id) {
Expand All @@ -310,9 +302,9 @@ function ($id) {
$plugins
)
)
->addArgument(new Reference($arguments['factory']))
->addArgument($arguments['config'])
->addArgument($pluginClientOptions)
->addArgument([
'client_name' => $clientName,
])
;

/*
Expand Down Expand Up @@ -432,44 +424,4 @@ private function configurePlugin(ContainerBuilder $container, $serviceId, $plugi

return $pluginServiceId;
}

/**
* Decorate the plugin service with a ProfilePlugin service.
*
* @param ContainerBuilder $container
* @param string $pluginServiceId
*/
private function decoratePluginWithProfilePlugin(ContainerBuilder $container, $pluginServiceId)
{
$container->register($pluginServiceId.'.debug', ProfilePlugin::class)
->setArguments([
new Reference($pluginServiceId),
new Reference('httplug.collector.collector'),
new Reference('httplug.collector.formatter'),
])
->setPublic(false);
}

/**
* Configure a StackPlugin for a client.
*
* @param ContainerBuilder $container
* @param string $clientName Client name to display in the profiler.
* @param string $serviceId Client service id. Used as base for the StackPlugin service id.
*
* @return string configured StackPlugin service id
*/
private function configureStackPlugin(ContainerBuilder $container, $clientName, $serviceId)
{
$pluginServiceId = $serviceId.'.plugin.stack';

$definition = class_exists(ChildDefinition::class)
? new ChildDefinition('httplug.plugin.stack')
: new DefinitionDecorator('httplug.plugin.stack');

$definition->addArgument($clientName);
$container->setDefinition($pluginServiceId, $definition);

return $pluginServiceId;
}
}
12 changes: 12 additions & 0 deletions Resources/config/data-collector.xml
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,17 @@
<argument type="service" id="httplug.collector.formatter"/>
<argument type="service" id="debug.stopwatch"/>
</service>

<service id="Http\Client\Common\PluginClientFactory" class="Http\HttplugBundle\Collector\PluginClientFactory" public="false">
<argument type="service" id="httplug.collector.collector"/>
<argument type="service" id="httplug.collector.formatter"/>
<argument type="service" id="debug.stopwatch"/>
</service>

<service id="Http\HttplugBundle\Collector\PluginClientFactoryListener" class="Http\HttplugBundle\Collector\PluginClientFactoryListener">
<argument type="service" id="Http\Client\Common\PluginClientFactory" />

<tag name="kernel.event_subscriber" />
</service>
</services>
</container>
3 changes: 3 additions & 0 deletions Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
</service>
<service id="Http\Client\HttpClient" alias="httplug.client" public="false" />

<!-- PluginClientFactory -->
<service id="Http\Client\Common\PluginClientFactory" class="Http\Client\Common\PluginClientFactory" public="false" />

<!-- ClientFactories -->
<service id="httplug.factory.auto" class="Http\HttplugBundle\ClientFactory\AutoDiscoveryFactory" public="false" />
<service id="httplug.factory.buzz" class="Http\HttplugBundle\ClientFactory\BuzzFactory" public="false">
Expand Down
22 changes: 22 additions & 0 deletions Tests/Functional/ServiceInstantiationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
use Http\HttplugBundle\Collector\StackPlugin;
use Nyholm\NSA;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Profiler\Profiler;

class ServiceInstantiationTest extends WebTestCase
Expand Down Expand Up @@ -71,4 +76,21 @@ public function testProfilingDecoration()
$this->assertInstanceOf(ProfilePlugin::class, $plugins[3]);
$this->assertInstanceOf(ProfilePlugin::class, $plugins[4]);
}

/**
* {@inheritdoc}
*/
protected static function bootKernel(array $options = [])
{
parent::bootKernel($options);

/** @var EventDispatcherInterface $dispatcher */
$dispatcher = static::$kernel->getContainer()->get('event_dispatcher');

$event = new GetResponseEvent(static::$kernel, new Request(), HttpKernelInterface::MASTER_REQUEST);

$dispatcher->dispatch(KernelEvents::REQUEST, $event);

return static::$kernel;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this might be confusing because the base test cases don't do it in older symfony versions. but i think its still a good idea. assigning the bootKernel return value, then accessing static::$kernel and in the end return the variable would be weird.

}
}
Loading