From 51cd8a74ff175c6d86402986c28c146ee11fedac Mon Sep 17 00:00:00 2001 From: Fabien Bourigault Date: Wed, 8 Feb 2017 23:35:48 +0100 Subject: [PATCH] rewrite all profiler logic --- CHANGELOG.md | 9 + Collector/Collector.php | 111 +++++++++++ Collector/DebugPlugin.php | 63 ------ Collector/DebugPluginCollector.php | 181 ----------------- Collector/Formatter.php | 69 +++++++ Collector/PluginJournal.php | 54 ----- Collector/Profile.php | 91 +++++++++ Collector/ProfilePlugin.php | 74 +++++++ Collector/RequestStackProvider.php | 141 ------------- Collector/Stack.php | 112 +++++++++++ Collector/StackPlugin.php | 67 +++++++ DependencyInjection/HttplugExtension.php | 76 ++++--- Resources/config/data-collector.xml | 12 ++ Resources/views/webprofiler.html.twig | 95 +++++---- Tests/Functional/ServiceInstantiationTest.php | 15 +- Tests/Unit/Collector/FormatterTest.php | 91 +++++++++ Tests/Unit/Collector/ProfilePluginTest.php | 185 ++++++++++++++++++ Tests/Unit/Collector/StackPluginTest.php | 175 +++++++++++++++++ .../HttplugExtensionTest.php | 3 +- 19 files changed, 1078 insertions(+), 546 deletions(-) create mode 100644 Collector/Collector.php delete mode 100644 Collector/DebugPlugin.php delete mode 100644 Collector/DebugPluginCollector.php create mode 100644 Collector/Formatter.php delete mode 100644 Collector/PluginJournal.php create mode 100644 Collector/Profile.php create mode 100644 Collector/ProfilePlugin.php delete mode 100644 Collector/RequestStackProvider.php create mode 100644 Collector/Stack.php create mode 100644 Collector/StackPlugin.php create mode 100644 Tests/Unit/Collector/FormatterTest.php create mode 100644 Tests/Unit/Collector/ProfilePluginTest.php create mode 100644 Tests/Unit/Collector/StackPluginTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 823ea692..5a8a7923 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Change Log +## 1.4.0 - 2017-XX-XX + +### Changed + +- The profiler collector has been rewritten to use objects instead of arrays. + +### Fixed + +- WebProfiler is no longer broken when there was a redirect. ## 1.3.0 - 2016-08-16 diff --git a/Collector/Collector.php b/Collector/Collector.php new file mode 100644 index 00000000..60111434 --- /dev/null +++ b/Collector/Collector.php @@ -0,0 +1,111 @@ + + * + * @internal + */ +class Collector extends DataCollector +{ + /** + * @param array $clients + */ + public function __construct(array $clients) + { + $this->data['stacks'] = []; + $this->data['clients'] = $clients; + } + + /** + * {@inheritdoc} + */ + public function collect(Request $request, Response $response, Exception $exception = null) + { + // We do not need to collect any data from the Symfony Request and Response + } + + /** + * {@inheritdoc} + */ + public function getName() + { + return 'httplug'; + } + + /** + * @param Stack $stack + */ + public function addStack(Stack $stack) + { + $this->data['stacks'][] = $stack; + } + + /** + * @return Stack[] + */ + public function getStacks() + { + return $this->data['stacks']; + } + + /** + * @return Stack + */ + public function getCurrentStack() + { + return end($this->data['stacks']); + } + + /** + * @return Stack[] + */ + public function getSuccessfulStacks() + { + return array_filter($this->data['stacks'], function (Stack $stack) { + return !$stack->isFailed(); + }); + } + + /** + * @return Stack[] + */ + public function getFailedStacks() + { + return array_filter($this->data['stacks'], function (Stack $stack) { + return $stack->isFailed(); + }); + } + + /** + * @return array + */ + public function getClients() + { + return $this->data['clients']; + } + + /** + * @param $client + * + * @return Stack[] + */ + public function getClientStacks($client) + { + return array_filter($this->data['stacks'], function (Stack $stack) use ($client) { + return $stack->getClient() == $client; + }); + } +} diff --git a/Collector/DebugPlugin.php b/Collector/DebugPlugin.php deleted file mode 100644 index 224b3bef..00000000 --- a/Collector/DebugPlugin.php +++ /dev/null @@ -1,63 +0,0 @@ - - */ -final class DebugPlugin implements Plugin -{ - /** - * @var DebugPluginCollector - */ - private $collector; - - /** - * @var string - */ - private $clientName; - - /** - * @var int - */ - private $depth = -1; - - /** - * @param DebugPluginCollector $collector - * @param string $clientName - */ - public function __construct(DebugPluginCollector $collector, $clientName) - { - $this->collector = $collector; - $this->clientName = $clientName; - } - - /** - * {@inheritdoc} - */ - public function handleRequest(RequestInterface $request, callable $next, callable $first) - { - $collector = $this->collector; - $clientName = $this->clientName; - $depth = &$this->depth; - - $collector->addRequest($request, $clientName, ++$depth); - - return $next($request)->then(function (ResponseInterface $response) use ($collector, $clientName, &$depth) { - $collector->addResponse($response, $clientName, $depth--); - - return $response; - }, function (Exception $exception) use ($collector, $clientName, &$depth) { - $collector->addFailure($exception, $clientName, $depth--); - - throw $exception; - }); - } -} diff --git a/Collector/DebugPluginCollector.php b/Collector/DebugPluginCollector.php deleted file mode 100644 index 9693b234..00000000 --- a/Collector/DebugPluginCollector.php +++ /dev/null @@ -1,181 +0,0 @@ - - */ -final class DebugPluginCollector extends DataCollector -{ - /** - * @var Formatter - */ - private $formatter; - - /** - * @var PluginJournal - */ - private $journal; - - /** - * @param Formatter $formatter - * @param PluginJournal $journal - */ - public function __construct(Formatter $formatter, PluginJournal $journal) - { - $this->formatter = $formatter; - $this->journal = $journal; - } - - /** - * @param RequestInterface $request - * @param string $clientName - * @param int $depth - */ - public function addRequest(RequestInterface $request, $clientName, $depth) - { - $this->data[$clientName]['request'][$depth][] = $this->formatter->formatRequest($request); - } - - /** - * @param ResponseInterface $response - * @param string $clientName - * @param int $depth - */ - public function addResponse(ResponseInterface $response, $clientName, $depth) - { - $this->data[$clientName]['response'][$depth][] = $this->formatter->formatResponse($response); - $this->data[$clientName]['failure'][$depth][] = false; - } - - /** - * @param Exception $exception - * @param string $clientName - * @param int $depth - */ - public function addFailure(Exception $exception, $clientName, $depth) - { - if ($exception instanceof Exception\HttpException) { - $formattedResponse = $this->formatter->formatResponse($exception->getResponse()); - } elseif ($exception instanceof Exception\TransferException) { - $formattedResponse = $exception->getMessage(); - } else { - $formattedResponse = sprintf('Unexpected exception of type "%s"', get_class($exception)); - } - - $this->data[$clientName]['response'][$depth][] = $formattedResponse; - $this->data[$clientName]['failure'][$depth][] = true; - } - - /** - * Returns the successful request-response pairs. - * - * @return int - */ - public function getSuccessfulRequests() - { - $count = 0; - foreach ($this->data as $client) { - if (isset($client['failure'])) { - foreach ($client['failure'][0] as $failure) { - if (!$failure) { - ++$count; - } - } - } - } - - return $count; - } - - /** - * Returns the failed request-response pairs. - * - * @return int - */ - public function getFailedRequests() - { - $count = 0; - foreach ($this->data as $client) { - if (isset($client['failure'])) { - foreach ($client['failure'][0] as $failure) { - if ($failure) { - ++$count; - } - } - } - } - - return $count; - } - - /** - * Returns the total number of request made. - * - * @return int - */ - public function getTotalRequests() - { - return $this->getSuccessfulRequests() + $this->getFailedRequests(); - } - - /** - * Return a RequestStackProvider for each client. - * - * @return RequestStackProvider[] - */ - public function getClients() - { - return RequestStackProvider::createFromCollectedData($this->data); - } - - /** - * @return PluginJournal - */ - public function getJournal() - { - return $this->journal; - } - - /** - * {@inheritdoc} - */ - public function collect(Request $request, Response $response, \Exception $exception = null) - { - // We do not need to collect any data from the Symfony Request and Response - } - - /** - * {@inheritdoc} - */ - public function getName() - { - return 'httplug'; - } - - /** - * {@inheritdoc} - */ - public function serialize() - { - return serialize([$this->data, $this->journal]); - } - - /** - * {@inheritdoc} - */ - public function unserialize($data) - { - list($this->data, $this->journal) = unserialize($data); - } -} diff --git a/Collector/Formatter.php b/Collector/Formatter.php new file mode 100644 index 00000000..0bb706fd --- /dev/null +++ b/Collector/Formatter.php @@ -0,0 +1,69 @@ + + * + * @internal + */ +class Formatter implements MessageFormatter +{ + /** + * @var MessageFormatter + */ + private $formatter; + + /** + * @param MessageFormatter $formatter + */ + public function __construct(MessageFormatter $formatter) + { + $this->formatter = $formatter; + } + + /** + * Formats an exception. + * + * @param Exception $exception + * + * @return string + */ + public function formatException(Exception $exception) + { + if ($exception instanceof HttpException) { + return $this->formatter->formatResponse($exception->getResponse()); + } + + if ($exception instanceof TransferException) { + return sprintf('Transfer error: %s', $exception->getMessage()); + } + + return sprintf('Unexpected exception of type "%s": %s', get_class($exception), $exception->getMessage()); + } + + /** + * {@inheritdoc} + */ + public function formatRequest(RequestInterface $request) + { + return $this->formatter->formatRequest($request); + } + + /** + * {@inheritdoc} + */ + public function formatResponse(ResponseInterface $response) + { + return $this->formatter->formatResponse($response); + } +} diff --git a/Collector/PluginJournal.php b/Collector/PluginJournal.php deleted file mode 100644 index cfc40e4d..00000000 --- a/Collector/PluginJournal.php +++ /dev/null @@ -1,54 +0,0 @@ - - */ -final class PluginJournal -{ - /** - * @var array ['clientName'=>['index' => 'PluginName'] - */ - private $data; - - /** - * @param string $clientName - * - * @return array - */ - public function getPlugins($clientName) - { - if (isset($this->data[$clientName])) { - return $this->data[$clientName]; - } - - return []; - } - - /** - * @param string $clientName - * @param int $idx - * - * @return string|null - */ - public function getPluginName($clientName, $idx) - { - if (isset($this->data[$clientName][$idx])) { - return $this->data[$clientName][$idx]; - } - - return; - } - - /** - * @param string $clientName - * @param array $plugins - */ - public function setPlugins($clientName, array $plugins) - { - $this->data[$clientName] = $plugins; - } -} diff --git a/Collector/Profile.php b/Collector/Profile.php new file mode 100644 index 00000000..b9ff0e5d --- /dev/null +++ b/Collector/Profile.php @@ -0,0 +1,91 @@ + + * + * @internal + */ +final class Profile +{ + /** + * @var string + */ + private $plugin; + + /** + * @var string + */ + private $request; + + /** + * @var string + */ + private $response; + + /** + * @var bool + */ + private $failed = false; + + /** + * @param string $plugin + * @param string $request + */ + public function __construct($plugin, $request) + { + $this->plugin = $plugin; + $this->request = $request; + } + + /** + * @return string + */ + public function getPlugin() + { + return $this->plugin; + } + + /** + * @return string + */ + public function getRequest() + { + return $this->request; + } + + /** + * @return string + */ + public function getResponse() + { + return $this->response; + } + + /** + * @param string $response + */ + public function setResponse($response) + { + $this->response = $response; + } + + /** + * @return bool + */ + public function isFailed() + { + return $this->failed; + } + + /** + * @param bool $failed + */ + public function setFailed($failed) + { + $this->failed = $failed; + } +} diff --git a/Collector/ProfilePlugin.php b/Collector/ProfilePlugin.php new file mode 100644 index 00000000..4517f318 --- /dev/null +++ b/Collector/ProfilePlugin.php @@ -0,0 +1,74 @@ + + * + * @internal + */ +class ProfilePlugin implements Plugin +{ + /** + * @var Plugin + */ + private $plugin; + + /** + * @var Collector + */ + private $collector; + + /** + * @var Formatter + */ + private $formatter; + + /** + * @var string + */ + private $pluginName; + + /** + * @param Plugin $plugin + * @param Collector $collector + * @param Formatter $formatter + * @param string $pluginName + */ + public function __construct(Plugin $plugin, Collector $collector, Formatter $formatter, $pluginName) + { + $this->plugin = $plugin; + $this->collector = $collector; + $this->formatter = $formatter; + $this->pluginName = $pluginName; + } + + /** + * {@inheritdoc} + */ + public function handleRequest(RequestInterface $request, callable $next, callable $first) + { + $profile = new Profile($this->pluginName, $this->formatter->formatRequest($request)); + + $this->collector->getCurrentStack()->addProfile($profile); + + return $this->plugin->handleRequest($request, $next, $first)->then(function (ResponseInterface $response) use ($profile) { + $profile->setResponse($this->formatter->formatResponse($response)); + + return $response; + }, function (Exception $exception) use ($profile) { + $profile->setFailed(true); + $profile->setResponse($this->formatter->formatException($exception)); + + throw $exception; + }); + } +} diff --git a/Collector/RequestStackProvider.php b/Collector/RequestStackProvider.php deleted file mode 100644 index ef280658..00000000 --- a/Collector/RequestStackProvider.php +++ /dev/null @@ -1,141 +0,0 @@ - - */ -final class RequestStackProvider -{ - /** - * Array that tell if a request errored or not. true = success, false = failure. - * - * @var array - */ - private $failures; - - /** - * A multidimensional array with requests. - * $requests[0][0] is the first request before all plugins. - * $requests[0][1] is the first request after the first plugin. - * - * @var array - */ - private $requests; - - /** - * A multidimensional array with responses. - * $responses[0][0] is the first responses before all plugins. - * $responses[0][1] is the first responses after the first plugin. - * - * @var array - */ - private $responses; - - /** - * @param array $failures if the response was successful or not - * @param array $requests - * @param array $responses - */ - public function __construct(array $failures, array $requests, array $responses) - { - $this->failures = $failures; - $this->requests = $requests; - $this->responses = $responses; - } - - /** - * Create an array of ClientDataCollector from collected data. - * - * @param array $data - * - * @return RequestStackProvider[] - */ - public static function createFromCollectedData(array $data) - { - $clientData = []; - foreach ($data as $clientName => $messages) { - $clientData[$clientName] = static::createOne($messages); - } - - return $clientData; - } - - /** - * @param array $messages is an array with keys 'failure', 'request' and 'response' which hold requests for each call to - * sendRequest and for each depth - * - * @return RequestStackProvider - */ - private static function createOne($messages) - { - $orderedFailure = []; - $orderedRequests = []; - $orderedResponses = []; - - foreach ($messages['failure'] as $depth => $failures) { - foreach ($failures as $idx => $failure) { - $orderedFailure[$idx][$depth] = $failure; - } - } - - foreach ($messages['request'] as $depth => $requests) { - foreach ($requests as $idx => $request) { - $orderedRequests[$idx][$depth] = $request; - } - } - - foreach ($messages['response'] as $depth => $responses) { - foreach ($responses as $idx => $response) { - $orderedResponses[$idx][$depth] = $response; - } - } - - return new self($orderedFailure, $orderedRequests, $orderedResponses); - } - - /** - * Get the index keys for the request and response stacks. - * - * @return array - */ - public function getStackIndexKeys() - { - return array_keys($this->requests); - } - - /** - * @param int $idx - * - * @return array - */ - public function getRequestStack($idx) - { - return $this->requests[$idx]; - } - - /** - * @param int $idx - * - * @return array - */ - public function getResponseStack($idx) - { - return $this->responses[$idx]; - } - - /** - * @param int $idx - * - * @return array - */ - public function getFailureStack($idx) - { - return $this->failures[$idx]; - } -} diff --git a/Collector/Stack.php b/Collector/Stack.php new file mode 100644 index 00000000..d4788396 --- /dev/null +++ b/Collector/Stack.php @@ -0,0 +1,112 @@ + + * + * @internal + */ +final class Stack +{ + /** + * @var string + */ + private $client; + + /** + * @var Profile[] + */ + private $profiles = []; + + /** + * @var string + */ + private $request; + + /** + * @var string + */ + private $response; + + /** + * @var bool + */ + private $failed = false; + + /** + * @param string $client + * @param string $request + */ + public function __construct($client, $request) + { + $this->client = $client; + $this->request = $request; + } + + /** + * @return string + */ + public function getClient() + { + return $this->client; + } + + /** + * @param Profile $profile + */ + public function addProfile(Profile $profile) + { + $this->profiles[] = $profile; + } + + /** + * @return Profile[] + */ + public function getProfiles() + { + return $this->profiles; + } + + /** + * @return string + */ + public function getRequest() + { + return $this->request; + } + + /** + * @return string + */ + public function getResponse() + { + return $this->response; + } + + /** + * @param string $response + */ + public function setResponse($response) + { + $this->response = $response; + } + + /** + * @return bool + */ + public function isFailed() + { + return $this->failed; + } + + /** + * @param bool $failed + */ + public function setFailed($failed) + { + $this->failed = $failed; + } +} diff --git a/Collector/StackPlugin.php b/Collector/StackPlugin.php new file mode 100644 index 00000000..a08b65e4 --- /dev/null +++ b/Collector/StackPlugin.php @@ -0,0 +1,67 @@ + + * + * @internal + */ +class StackPlugin implements Plugin +{ + /** + * @var Collector + */ + private $collector; + + /** + * @var string + */ + private $client; + + /** + * @var Formatter + */ + private $formatter; + + /** + * @param Collector $collector + * @param Formatter $formatter + * @param string $client + */ + public function __construct(Collector $collector, Formatter $formatter, $client) + { + $this->collector = $collector; + $this->formatter = $formatter; + $this->client = $client; + } + + /** + * {@inheritdoc} + */ + public function handleRequest(RequestInterface $request, callable $next, callable $first) + { + $stack = new Stack($this->client, $this->formatter->formatRequest($request)); + + $this->collector->addStack($stack); + + return $next($request)->then(function (ResponseInterface $response) use ($stack) { + $stack->setResponse($this->formatter->formatResponse($response)); + + return $response; + }, function (Exception $exception) use ($stack) { + $stack->setResponse($this->formatter->formatException($exception)); + $stack->setFailed(true); + + throw $exception; + }); + } +} diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index baeff099..80386be4 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -10,7 +10,7 @@ use Http\Discovery\HttpClientDiscovery; use Http\HttplugBundle\ClientFactory\DummyClient; use Http\HttplugBundle\ClientFactory\PluginClientFactory; -use Http\HttplugBundle\Collector\DebugPlugin; +use Http\HttplugBundle\Collector\ProfilePlugin; 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); + $this->configureClients($container, $config, $this->isConfigEnabled($container, $config['profiling'])); $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); } @@ -82,10 +82,12 @@ public function load(array $configs, ContainerBuilder $container) * * @param ContainerBuilder $container * @param array $config + * @param bool $profiling */ - private function configureClients(ContainerBuilder $container, array $config) + private function configureClients(ContainerBuilder $container, array $config, $profiling) { $first = null; + $clients = []; foreach ($config['clients'] as $name => $arguments) { if ($first === null) { @@ -94,6 +96,7 @@ private function configureClients(ContainerBuilder $container, array $config) } $this->configureClient($container, $name, $arguments, $this->isConfigEnabled($container, $config['profiling'])); + $clients[] = $name; } // If we have clients configured @@ -104,6 +107,12 @@ private function configureClients(ContainerBuilder $container, array $config) $container->setAlias('httplug.client.default', 'httplug.client.'.$first); } } + + if ($profiling) { + $container->getDefinition('httplug.collector.collector') + ->setArguments([$clients]) + ; + } } /** @@ -262,10 +271,10 @@ private function configureClient(ContainerBuilder $container, $clientName, array $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); + $definition = clone $container->getDefinition('httplug.plugin'.'.'.$pluginName); + $definition->setAbstract(false); + $this->configurePluginByName($pluginName, $definition, $pluginConfig, $container, $pluginServiceId); + $container->setDefinition($pluginServiceId, $definition); $plugins[] = $pluginServiceId; } } @@ -277,15 +286,26 @@ private function configureClient(ContainerBuilder $container, $clientName, array array_unshift($plugins, 'httplug.plugin.stopwatch'); } - // Tell the plugin journal what plugins we used - $container - ->getDefinition('httplug.collector.plugin_journal') - ->addMethodCall('setPlugins', [$clientName, $plugins]) - ; - - $debugPluginServiceId = $this->registerDebugPlugin($container, $serviceId); + //Decorate each plugin with a ProfilePlugin instance. + foreach ($plugins as $pluginServiceId) { + $container->register($pluginServiceId.'.debug', ProfilePlugin::class) + ->setDecoratedService($pluginServiceId) + ->setArguments([ + new Reference($pluginServiceId.'.debug.inner'), + new Reference('httplug.collector.collector'), + new Reference('httplug.collector.formatter'), + $pluginServiceId, + ]) + ->setPublic(false) + ; + } - $pluginClientOptions['debug_plugins'] = [new Reference($debugPluginServiceId)]; + // Add the newstack plugin + $definition = clone $container->getDefinition('httplug.plugin.stack'); + $definition->setAbstract(false); + $definition->addArgument($clientName); + $container->setDefinition($serviceId.'.plugin.newstack', $definition); + array_unshift($plugins, $serviceId.'.plugin.newstack'); } $container @@ -420,10 +440,6 @@ private function registerAutoDiscoverableClient(ContainerBuilder $container, $na ->getDefinition('httplug.collector.plugin_journal') ->addMethodCall('setPlugins', [$name, ['httplug.plugin.stopwatch']]) ; - - $debugPluginServiceId = $this->registerDebugPlugin($container, $serviceId); - - $pluginClientOptions['debug_plugins'] = [new Reference($debugPluginServiceId)]; } $container @@ -435,28 +451,6 @@ private function registerAutoDiscoverableClient(ContainerBuilder $container, $na return $serviceId; } - /** - * Create a new plugin service for this client. - * - * @param ContainerBuilder $container - * @param string $serviceId - * - * @return string - */ - private function registerDebugPlugin(ContainerBuilder $container, $serviceId) - { - $serviceIdDebugPlugin = $serviceId.'.debug_plugin'; - - $container - ->register($serviceIdDebugPlugin, DebugPlugin::class) - ->addArgument(new Reference('httplug.collector.debug_collector')) - ->addArgument(substr($serviceId, strrpos($serviceId, '.') + 1)) - ->setPublic(false) - ; - - return $serviceIdDebugPlugin; - } - /** * {@inheritdoc} */ diff --git a/Resources/config/data-collector.xml b/Resources/config/data-collector.xml index d42e93e5..a7e2b0d0 100644 --- a/Resources/config/data-collector.xml +++ b/Resources/config/data-collector.xml @@ -8,12 +8,24 @@ + + + + + + + + + + + + diff --git a/Resources/views/webprofiler.html.twig b/Resources/views/webprofiler.html.twig index 899ea500..e7af78af 100644 --- a/Resources/views/webprofiler.html.twig +++ b/Resources/views/webprofiler.html.twig @@ -3,28 +3,28 @@ {% import _self as macro %} {% block toolbar %} - {% if collector.totalRequests > 0 %} + {% if collector.stacks|length > 0 %} {% set icon %} {% if constant('Symfony\\Component\\HttpKernel\\Kernel::VERSION_ID') >= 20800 %} {{ include('@Httplug/Icon/httplug.svg') }} {% else %} {{ include('@Httplug/Icon/httplug_old.svg') }} {% endif %} - {{ collector.totalRequests }} + {{ collector.stacks|length }} req. {% endset %} {% set text %}
Successful requests - {{ collector.sucessfulRequests }} + {{ collector.successfulStacks|length }}
Failed requests - {{ collector.failedRequests }} + {{ collector.failedStacks|length }}
- {% endset %} + {% include 'WebProfilerBundle:Profiler:toolbar_item.html.twig' with { 'link': profiler_url } %} {% endif %} {% endblock %} @@ -37,7 +37,7 @@ {% block menu %} {# This left-hand menu appears when using the full-screen profiler. #} - + {% if constant('Symfony\\Component\\HttpKernel\\Kernel::VERSION_ID') >= 20800 %} {{ include('@Httplug/Icon/httplug.svg') }} @@ -54,24 +54,23 @@
- {% for name, client in collector.clients %} + {% for client in collector.clients %}
-

{{ name }} {{ client.stackIndexKeys|length }}

+

{{ client }} {{ collector.clientStacks(client)|length }}

- These messages are sent by client named "{{ name }}". + These messages are sent by client named "{{ client }}".

- {% for stackIndex in client.stackIndexKeys %} - {% set failureStack = client.failureStack(stackIndex) %} + {% for stack in collector.clientStacks(client) %}

- Request #{{ stackIndex }} - {% if failureStack[0] %} + Request #{{ loop.index }} + {% if stack.failed %} - Errored {% endif %}

- {{ macro.printMessages(client.requstStack(stackIndex), client.responseStack(stackIndex), failureStack, collector.journal.plugins(name)) }} + {{ macro.printMessages(stack) }} {% endfor %}
@@ -85,7 +84,7 @@ {% endblock %} -{% macro printMessages(requestStack, responseStack, failureStack, pluginNames) %} +{% macro printMessages(stack) %} @@ -93,51 +92,45 @@ - - + + - {% if requestStack|length > 1 %} + {% if stack.profiles %} - {% for idx in 0..requestStack|length-1 %} - {% if loop.first %} - {# We do not have a plugin at the first entry in the stack #} - - - - - {% else %} - - - - - {% endif %} + + + + + {% for profile in stack.profiles %} - - + + + + + + - {% if loop.last %} - - - - - {% endif %} {% endfor %} + + + + {% endif %}
Request
{{ requestStack[responseStack|length-1]|httplug_markup|nl2br }}{{ responseStack[0]|httplug_markup|nl2br }}{{ stack.request|httplug_markup|nl2br }}{{ stack.response|httplug_markup|nl2br }}
↓ Start - End - {% if failureStack[idx] %} - ☓ - {% endif %} -
↓ {{ pluginNames[idx-1] }} ↑ - {% if failureStack[idx-1] %} - ☓ - {% endif %} -
↓ Start - End + {% if stack.failed %} + ☓ + {% endif %} +
{{ requestStack[idx]|httplug_markup|nl2br }}{{ responseStack[idx]|httplug_markup|nl2br }}↓ {{ profile.plugin }} ↑ + {% if profile.failed %} + ☓ + {% endif %} +
{{ profile.request|httplug_markup|nl2br }}{{ profile.response|httplug_markup|nl2br }}
⟶ HTTP client↑ - {% if failureStack[idx-1] %} - ☓ - {% endif %} -
⟶ HTTP client↑ + {#{% if profile.failed %}#} + {#☓#} + {#{% endif %}#} +
{% endmacro %} diff --git a/Tests/Functional/ServiceInstantiationTest.php b/Tests/Functional/ServiceInstantiationTest.php index 8d07db67..f68cc5bc 100644 --- a/Tests/Functional/ServiceInstantiationTest.php +++ b/Tests/Functional/ServiceInstantiationTest.php @@ -3,8 +3,7 @@ namespace Http\HttplugBundle\Tests\Functional; use Http\Client\HttpClient; -use Http\HttplugBundle\Collector\DebugPluginCollector; -use Http\HttplugBundle\Collector\PluginJournal; +use Http\HttplugBundle\Collector\Collector; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\HttpKernel\Profiler\Profiler; @@ -37,16 +36,6 @@ public function testDebugToolbar() $this->assertInstanceOf(Profiler::class, $profiler); $this->assertTrue($profiler->has('httplug')); $collector = $profiler->get('httplug'); - $this->assertInstanceOf(DebugPluginCollector::class, $collector); - /** @var PluginJournal $journal */ - $journal = $collector->getJournal(); - $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); + $this->assertInstanceOf(Collector::class, $collector); } } diff --git a/Tests/Unit/Collector/FormatterTest.php b/Tests/Unit/Collector/FormatterTest.php new file mode 100644 index 00000000..defb81b8 --- /dev/null +++ b/Tests/Unit/Collector/FormatterTest.php @@ -0,0 +1,91 @@ +formatter = $this->getMockBuilder(MessageFormatter::class)->getMock(); + + $this->subject = new Formatter($this->formatter); + } + + public function testFormatRequest() + { + $request = $this->getMockBuilder(RequestInterface::class)->getMock(); + + $this->formatter + ->expects($this->once()) + ->method('formatRequest') + ->with($this->identicalTo($request)) + ; + + $this->subject->formatRequest($request); + } + + public function testFormatResponse() + { + $response = $this->getMockBuilder(ResponseInterface::class)->getMock(); + + $this->formatter + ->expects($this->once()) + ->method('formatResponse') + ->with($this->identicalTo($response)) + ; + + $this->subject->formatResponse($response); + } + + public function testFormatHttpException() + { + $response = $this->getMockBuilder(ResponseInterface::class)->getMock(); + $exception = $this->getMockBuilder(HttpException::class)->disableOriginalConstructor()->getMock(); + $exception + ->method('getResponse') + ->willReturn($response) + ; + + $this->formatter + ->expects($this->once()) + ->method('formatResponse') + ->with($this->identicalTo($response)) + ->willReturn('FormattedException') + ; + + $this->assertEquals('FormattedException', $this->subject->formatException($exception)); + } + + public function testFormatTransferException() + { + $exception = $this->getMockBuilder(TransferException::class) + ->setConstructorArgs(['ExceptionMessage']) + ->getMock() + ; + + $this->assertEquals('Transfer error: ExceptionMessage', $this->subject->formatException($exception)); + } + + public function testFormatException() + { + $exception = new \RuntimeException('Unexpected error'); + $this->assertEquals('Unexpected exception of type "RuntimeException": Unexpected error', $this->subject->formatException($exception)); + } +} diff --git a/Tests/Unit/Collector/ProfilePluginTest.php b/Tests/Unit/Collector/ProfilePluginTest.php new file mode 100644 index 00000000..12912a03 --- /dev/null +++ b/Tests/Unit/Collector/ProfilePluginTest.php @@ -0,0 +1,185 @@ +plugin = $this->getMockBuilder(Plugin::class)->getMock(); + $this->collector = $this->getMockBuilder(Collector::class)->disableOriginalConstructor()->getMock(); + $this->request = $this->getMockBuilder(RequestInterface::class)->getMock(); + $this->response = $this->getMockBuilder(ResponseInterface::class)->getMock(); + $this->currentStack = new Stack('default', 'FormattedRequest'); + $this->promise = $this->getMockBuilder(Promise::class)->getMock(); + $this->exception = $this->getMockBuilder(Exception::class)->disableOriginalConstructor()->getMock(); + $this->formatter = $this->getMockBuilder(Formatter::class)->disableOriginalConstructor()->getMock(); + + $this->collector + ->method('getCurrentStack') + ->willReturn($this->currentStack) + ; + + $this->plugin + ->method('handleRequest') + ->willReturn($this->promise) + ; + + $this->formatter + ->method('formatRequest') + ->with($this->identicalTo($this->request)) + ->willReturn('FormattedRequest') + ; + + $this->formatter + ->method('formatResponse') + ->with($this->identicalTo($this->response)) + ->willReturn('FormattedResponse') + ; + + $this->formatter + ->method('formatException') + ->with($this->identicalTo($this->exception)) + ->willReturn('FormattedException') + ; + + $this->subject = new ProfilePlugin( + $this->plugin, + $this->collector, + $this->formatter, + 'http.plugin.mock' + ); + } + + public function testCallDecoratedPlugin() + { + $this->plugin + ->expects($this->once()) + ->method('handleRequest') + ->with($this->request) + ; + + $this->subject->handleRequest($this->request, function () { + }, function () { + }); + } + + public function testProfileIsInitialized() + { + $this->subject->handleRequest($this->request, function () { + }, function () { + }); + + $this->assertCount(1, $this->currentStack->getProfiles()); + $profile = $this->currentStack->getProfiles()[0]; + $this->assertEquals('http.plugin.mock', $profile->getPlugin()); + $this->assertEquals('FormattedRequest', $profile->getRequest()); + } + + public function testOnFulfilled() + { + $this->promise + ->method('then') + ->will($this->returnCallback(function (callable $onFulfilled) { + $fulfilled = $this->getMockBuilder(Promise::class)->getMock(); + $fulfilled + ->method('wait') + ->with(true) + ->willReturn($onFulfilled($this->response)) + ; + + return $fulfilled; + })) + ; + + $promise = $this->subject->handleRequest($this->request, function () { + }, function () { + }); + + $this->assertEquals($this->response, $promise->wait()); + $profile = $this->currentStack->getProfiles()[0]; + $this->assertEquals('FormattedResponse', $profile->getResponse()); + } + + public function testOnRejected() + { + $this->setExpectedException(Exception::class); + + $this->promise + ->method('then') + ->will($this->returnCallback(function (callable $onFulfilled, callable $onRejected) { + $rejected = $this->getMockBuilder(Promise::class)->getMock(); + $rejected + ->method('wait') + ->with(true) + ->willReturn($onRejected($this->exception)) + ; + + return $rejected; + })) + ; + + $promise = $this->subject->handleRequest($this->request, function () { + }, function () { + }); + + $this->assertEquals($this->exception, $promise->wait()); + $profile = $this->currentStack->getProfiles()[0]; + $this->assertEquals('FormattedException', $profile->getResponse()); + } +} diff --git a/Tests/Unit/Collector/StackPluginTest.php b/Tests/Unit/Collector/StackPluginTest.php new file mode 100644 index 00000000..448ff061 --- /dev/null +++ b/Tests/Unit/Collector/StackPluginTest.php @@ -0,0 +1,175 @@ +collector = $this->getMockBuilder(Collector::class)->disableOriginalConstructor()->getMock(); + $this->formatter = $this->getMockBuilder(Formatter::class)->disableOriginalConstructor()->getMock(); + $this->request = $this->getMockBuilder(RequestInterface::class)->getMock(); + $this->response = $this->getMockBuilder(ResponseInterface::class)->getMock(); + $this->exception = $this->getMockBuilder(Exception::class)->disableOriginalConstructor()->getMock(); + + $this->formatter + ->method('formatRequest') + ->with($this->request) + ->willReturn('FormattedRequest') + ; + + $this->formatter + ->method('formatResponse') + ->with($this->response) + ->willReturn('FormattedResponse') + ; + + $this->formatter + ->method('formatException') + ->with($this->exception) + ->willReturn('FormattedException') + ; + + $this->subject = new StackPlugin($this->collector, $this->formatter, 'default'); + } + + public function testStackIsInitialized() + { + $this->collector + ->expects($this->once()) + ->method('addStack') + ->with($this->callback(function (Stack $stack) { + $this->assertEquals('default', $stack->getClient()); + $this->assertEquals('FormattedRequest', $stack->getRequest()); + + return true; + })) + ; + + $next = function () { + return $this->getMockBuilder(Promise::class)->getMock(); + }; + + $this->subject->handleRequest($this->request, $next, function () { + }); + } + + public function testOnFulfilled() + { + //Capture the current stack + $currentStack = null; + $this->collector + ->method('addStack') + ->with($this->callback(function (Stack $stack) use (&$currentStack) { + $currentStack = $stack; + + return true; + })) + ; + + $next = function () { + $promise = $this->getMockBuilder(Promise::class)->getMock(); + $promise->method('then') + ->will($this->returnCallback(function (callable $onFulfilled) { + $fulfilled = $this->getMockBuilder(Promise::class)->getMock(); + $fulfilled + ->method('wait') + ->with(true) + ->willReturn($onFulfilled($this->response)) + ; + + return $fulfilled; + })) + ; + + return $promise; + }; + + $promise = $this->subject->handleRequest($this->request, $next, function () { + }); + + $this->assertEquals($this->response, $promise->wait()); + $this->assertInstanceOf(Stack::class, $currentStack); + $this->assertEquals('FormattedResponse', $currentStack->getResponse()); + } + + public function testOnRejected() + { + //Capture the current stack + $currentStack = null; + $this->collector + ->method('addStack') + ->with($this->callback(function (Stack $stack) use (&$currentStack) { + $currentStack = $stack; + + return true; + })) + ; + + $this->setExpectedException(Exception::class); + + $next = function () { + $promise = $this->getMockBuilder(Promise::class)->getMock(); + $promise + ->method('then') + ->will($this->returnCallback(function (callable $onFulfilled, callable $onRejected) { + $rejected = $this->getMockBuilder(Promise::class)->getMock(); + $rejected + ->method('wait') + ->with(true) + ->willReturn($onRejected($this->exception)); + + return $rejected; + })); + + return $promise; + }; + + $promise = $this->subject->handleRequest($this->request, $next, function () { + }); + + $this->assertEquals($this->exception, $promise->wait()); + $this->assertInstanceOf(Stack::class, $currentStack); + $this->assertEquals('FormattedException', $currentStack->getResponse()); + $this->assertTrue($currentStack->isFailed()); + } +} diff --git a/Tests/Unit/DependencyInjection/HttplugExtensionTest.php b/Tests/Unit/DependencyInjection/HttplugExtensionTest.php index d9704bd1..b6910358 100644 --- a/Tests/Unit/DependencyInjection/HttplugExtensionTest.php +++ b/Tests/Unit/DependencyInjection/HttplugExtensionTest.php @@ -116,6 +116,7 @@ public function testClientPlugins() ]); $plugins = [ + 'httplug.client.acme.plugin.newstack', 'httplug.plugin.stopwatch', 'httplug.client.acme.plugin.decoder', 'httplug.plugin.redirect', @@ -194,8 +195,6 @@ public function testProfilingWhenToolbarIsSpecificallyOn() $arguments = $def->getArguments(); $this->assertTrue(isset($arguments[3])); - $this->assertTrue(isset($arguments[3]['debug_plugins'])); - $this->assertNotEmpty($arguments[3]['debug_plugins']); } private function verifyProfilingDisabled()