diff --git a/.travis.yml b/.travis.yml index 5238a8b7..8c24c0cf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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; diff --git a/CHANGELOG.md b/CHANGELOG.md index fb12f2bf..83bb9219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/ClientFactory/PluginClientFactory.php b/ClientFactory/PluginClientFactory.php index 9213f878..10092758 100644 --- a/ClientFactory/PluginClientFactory.php +++ b/ClientFactory/PluginClientFactory.php @@ -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 */ final class PluginClientFactory diff --git a/Collector/PluginClientFactory.php b/Collector/PluginClientFactory.php new file mode 100644 index 00000000..d98b0051 --- /dev/null +++ b/Collector/PluginClientFactory.php @@ -0,0 +1,77 @@ + + * + * @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); + } +} diff --git a/Collector/PluginClientFactoryListener.php b/Collector/PluginClientFactoryListener.php new file mode 100644 index 00000000..6dbf3056 --- /dev/null +++ b/Collector/PluginClientFactoryListener.php @@ -0,0 +1,54 @@ + + * + * @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], + ]; + } +} diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index 6f5df8f1..4d59f3be 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -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; @@ -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); } @@ -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 = []; @@ -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; } @@ -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; @@ -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) { @@ -310,9 +302,9 @@ function ($id) { $plugins ) ) - ->addArgument(new Reference($arguments['factory'])) - ->addArgument($arguments['config']) - ->addArgument($pluginClientOptions) + ->addArgument([ + 'client_name' => $clientName, + ]) ; /* @@ -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; - } } diff --git a/Resources/config/data-collector.xml b/Resources/config/data-collector.xml index 890e11c1..dd3f3b85 100644 --- a/Resources/config/data-collector.xml +++ b/Resources/config/data-collector.xml @@ -84,5 +84,17 @@ + + + + + + + + + + + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 1f82018a..23688423 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -44,6 +44,9 @@ + + + diff --git a/Tests/Functional/ServiceInstantiationTest.php b/Tests/Functional/ServiceInstantiationTest.php index e305ac0a..817d8aa4 100644 --- a/Tests/Functional/ServiceInstantiationTest.php +++ b/Tests/Functional/ServiceInstantiationTest.php @@ -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 @@ -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; + } } diff --git a/Tests/Unit/DependencyInjection/HttplugExtensionTest.php b/Tests/Unit/DependencyInjection/HttplugExtensionTest.php index af4dc686..21b7c8f1 100644 --- a/Tests/Unit/DependencyInjection/HttplugExtensionTest.php +++ b/Tests/Unit/DependencyInjection/HttplugExtensionTest.php @@ -3,6 +3,7 @@ namespace Http\HttplugBundle\Tests\Unit\DependencyInjection; use Http\Client\Common\PluginClient; +use Http\HttplugBundle\Collector\PluginClientFactoryListener; use Http\HttplugBundle\DependencyInjection\HttplugExtension; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; use Symfony\Component\DependencyInjection\Reference; @@ -116,15 +117,14 @@ public function testClientPlugins() ]); $plugins = [ - 'httplug.client.acme.plugin.stack', - 'httplug.client.acme.plugin.decoder.debug', - 'httplug.plugin.redirect.debug', - 'httplug.client.acme.plugin.add_host.debug', - 'httplug.client.acme.plugin.header_append.debug', - 'httplug.client.acme.plugin.header_defaults.debug', - 'httplug.client.acme.plugin.header_set.debug', - 'httplug.client.acme.plugin.header_remove.debug', - 'httplug.client.acme.authentication.my_basic.debug', + 'httplug.client.acme.plugin.decoder', + 'httplug.plugin.redirect', + 'httplug.client.acme.plugin.add_host', + 'httplug.client.acme.plugin.header_append', + 'httplug.client.acme.plugin.header_defaults', + 'httplug.client.acme.plugin.header_set', + 'httplug.client.acme.plugin.header_remove', + 'httplug.client.acme.authentication.my_basic', ]; $pluginReferences = array_map(function ($id) { return new Reference($id); @@ -134,7 +134,7 @@ public function testClientPlugins() foreach ($plugins as $id) { $this->assertContainerBuilderHasService($id); } - $this->assertContainerBuilderHasServiceDefinitionWithArgument('httplug.client.acme', 0, $pluginReferences); + $this->assertContainerBuilderHasServiceDefinitionWithArgument('httplug.client.acme', 1, $pluginReferences); } /** @@ -196,10 +196,7 @@ public function testProfilingWhenToolbarIsSpecificallyOn() ] ); - $def = $this->container->findDefinition('httplug.client'); - $arguments = $def->getArguments(); - - $this->assertTrue(isset($arguments[3])); + $this->assertContainerBuilderHasService(PluginClientFactoryListener::class); } public function testOverrideProfillingFormatter() diff --git a/composer.json b/composer.json index 173f421e..ad0fe1be 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "php": "^5.5 || ^7.0", "php-http/client-implementation": "^1.0", "php-http/message-factory": "^1.0.2", - "php-http/client-common": "^1.2", + "php-http/client-common": "^1.6", "php-http/cache-plugin": "^1.4", "php-http/logger-plugin": "^1.0", "php-http/stopwatch-plugin": "^1.0", @@ -29,7 +29,8 @@ "php-http/message": "^1.4", "php-http/discovery": "^1.0", "twig/twig": "^1.18 || ^2.0", - "symfony/asset": "^2.8 || ^3.0" + "symfony/asset": "^2.8 || ^3.0", + "symfony/dependency-injection": "^2.8.3 || ^3.0.3" }, "require-dev": { "phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.1", @@ -55,6 +56,8 @@ "conflict": { "php-http/guzzle6-adapter": "<1.1" }, + "minimum-stability": "dev", + "prefer-stable": true, "autoload": { "psr-4": { "Http\\HttplugBundle\\": "" @@ -71,7 +74,7 @@ }, "extra": { "branch-alias": { - "dev-master": "1.7-dev" + "dev-master": "1.8-dev" } } }