From 35132f8a81d832a523e95468cec0b3757fbe2994 Mon Sep 17 00:00:00 2001 From: Fabien Bourigault Date: Thu, 6 Jul 2017 14:05:56 +0200 Subject: [PATCH] display children requests as nested in the profiler --- Collector/Collector.php | 55 +++++++-- Collector/ProfileClient.php | 68 +++++------ Collector/ProfilePlugin.php | 10 +- Collector/Stack.php | 21 ++++ Collector/StackPlugin.php | 15 ++- Resources/public/style/httplug.css | 7 ++ Resources/views/stack.html.twig | 75 ++++++++++++ Resources/views/webprofiler.html.twig | 73 ++--------- Tests/Unit/Collector/CollectorTest.php | 51 ++++++++ Tests/Unit/Collector/ProfileClientTest.php | 135 +++++++++++++++++---- Tests/Unit/Collector/ProfilePluginTest.php | 2 +- Tests/Unit/Collector/StackPluginTest.php | 51 ++++++++ composer.json | 1 + 13 files changed, 420 insertions(+), 144 deletions(-) create mode 100644 Resources/views/stack.html.twig diff --git a/Collector/Collector.php b/Collector/Collector.php index a2eabddd..185e47b1 100644 --- a/Collector/Collector.php +++ b/Collector/Collector.php @@ -20,6 +20,11 @@ */ class Collector extends DataCollector { + /** + * @var Stack|null + */ + private $activeStack; + public function __construct() { $this->data['stacks'] = []; @@ -41,6 +46,38 @@ public function getName() return 'httplug'; } + /** + * Mark the stack as active. If a stack was already active, use it as parent for our stack. + * + * @param Stack $stack + */ + public function activateStack(Stack $stack) + { + if ($this->activeStack !== null) { + $stack->setParent($this->activeStack); + } + + $this->activeStack = $stack; + } + + /** + * Mark the stack as inactive. + * + * @param Stack $stack + */ + public function deactivateStack(Stack $stack) + { + $this->activeStack = $stack->getParent(); + } + + /** + * @return Stack|null + */ + public function getActiveStack() + { + return $this->activeStack; + } + /** * @param Stack $stack */ @@ -50,23 +87,23 @@ public function addStack(Stack $stack) } /** + * @param Stack $parent + * * @return Stack[] */ - public function getStacks() + public function getChildrenStacks(Stack $parent) { - return $this->data['stacks']; + return array_filter($this->data['stacks'], function (Stack $stack) use ($parent) { + return $stack->getParent() === $parent; + }); } /** - * @return Stack|null Return null there is no current stack. + * @return Stack[] */ - public function getCurrentStack() + public function getStacks() { - if (false === $stack = end($this->data['stacks'])) { - return null; - } - - return $stack; + return $this->data['stacks']; } /** diff --git a/Collector/ProfileClient.php b/Collector/ProfileClient.php index 93015b42..bc2693c3 100644 --- a/Collector/ProfileClient.php +++ b/Collector/ProfileClient.php @@ -75,23 +75,31 @@ public function __construct($client, Collector $collector, Formatter $formatter, */ public function sendAsyncRequest(RequestInterface $request) { - $stack = $this->collector->getCurrentStack(); + $stack = $this->collector->getActiveStack(); + $this->collectRequestInformations($request, $stack); $event = $this->stopwatch->start($this->getStopwatchEventName($request)); - return $this->client->sendAsyncRequest($request)->then( - function (ResponseInterface $response) use ($event, $stack) { - $event->stop(); - $this->collectResponseInformations($response, $event, $stack); + $onFulfilled = function (ResponseInterface $response) use ($event, $stack) { + $this->collectResponseInformations($response, $event, $stack); + + return $response; + }; + + $onRejected = function (\Exception $exception) use ($event, $stack) { + $this->collectExceptionInformations($exception, $event, $stack); + + throw $exception; + }; - return $response; - }, function (\Exception $exception) use ($event, $stack) { - $event->stop(); - $this->collectExceptionInformations($exception, $event, $stack); + $this->collector->deactivateStack($stack); - throw $exception; - } - ); + try { + return $this->client->sendAsyncRequest($request)->then($onFulfilled, $onRejected); + } finally { + $event->stop(); + $this->collector->activateStack($stack); + } } /** @@ -99,35 +107,35 @@ function (ResponseInterface $response) use ($event, $stack) { */ public function sendRequest(RequestInterface $request) { - $stack = $this->collector->getCurrentStack(); + $stack = $this->collector->getActiveStack(); + $this->collectRequestInformations($request, $stack); $event = $this->stopwatch->start($this->getStopwatchEventName($request)); try { $response = $this->client->sendRequest($request); - $event->stop(); - $this->collectResponseInformations($response, $event, $stack); return $response; } catch (\Exception $e) { - $event->stop(); $this->collectExceptionInformations($e, $event, $stack); throw $e; + } catch (\Throwable $e) { + $this->collectExceptionInformations($e, $event, $stack); + + throw $e; + } finally { + $event->stop(); } } /** * @param RequestInterface $request - * @param Stack|null $stack + * @param Stack $stack */ - private function collectRequestInformations(RequestInterface $request, Stack $stack = null) + private function collectRequestInformations(RequestInterface $request, Stack $stack) { - if (null === $stack) { - return; - } - $stack->setRequestTarget($request->getRequestTarget()); $stack->setRequestMethod($request->getMethod()); $stack->setRequestScheme($request->getUri()->getScheme()); @@ -139,14 +147,10 @@ private function collectRequestInformations(RequestInterface $request, Stack $st /** * @param ResponseInterface $response * @param StopwatchEvent $event - * @param Stack|null $stack + * @param Stack $stack */ - private function collectResponseInformations(ResponseInterface $response, StopwatchEvent $event, Stack $stack = null) + private function collectResponseInformations(ResponseInterface $response, StopwatchEvent $event, Stack $stack) { - if (null === $stack) { - return; - } - $stack->setDuration($event->getDuration()); $stack->setResponseCode($response->getStatusCode()); $stack->setClientResponse($this->formatter->formatResponse($response)); @@ -155,18 +159,14 @@ private function collectResponseInformations(ResponseInterface $response, Stopwa /** * @param \Exception $exception * @param StopwatchEvent $event - * @param Stack|null $stack + * @param Stack $stack */ - private function collectExceptionInformations(\Exception $exception, StopwatchEvent $event, Stack $stack = null) + private function collectExceptionInformations(\Exception $exception, StopwatchEvent $event, Stack $stack) { if ($exception instanceof HttpException) { $this->collectResponseInformations($exception->getResponse(), $event, $stack); } - if (null === $stack) { - return; - } - $stack->setDuration($event->getDuration()); $stack->setClientException($this->formatter->formatException($exception)); } diff --git a/Collector/ProfilePlugin.php b/Collector/ProfilePlugin.php index e202de5a..c18ac3ec 100644 --- a/Collector/ProfilePlugin.php +++ b/Collector/ProfilePlugin.php @@ -51,10 +51,8 @@ public function handleRequest(RequestInterface $request, callable $next, callabl { $profile = new Profile(get_class($this->plugin)); - $stack = $this->collector->getCurrentStack(); - if (null !== $stack) { - $stack->addProfile($profile); - } + $stack = $this->collector->getActiveStack(); + $stack->addProfile($profile); // wrap the next callback to profile the plugin request changes $wrappedNext = function (RequestInterface $request) use ($next, $profile) { @@ -136,10 +134,6 @@ private function onOutgoingResponse(ResponseInterface $response, Profile $profil */ private function collectRequestInformation(RequestInterface $request, Stack $stack = null) { - if (null === $stack) { - return; - } - if (empty($stack->getRequestTarget())) { $stack->setRequestTarget($request->getRequestTarget()); } diff --git a/Collector/Stack.php b/Collector/Stack.php index 8ee7112b..a5d467ce 100644 --- a/Collector/Stack.php +++ b/Collector/Stack.php @@ -16,6 +16,11 @@ final class Stack */ private $client; + /** + * @var Stack + */ + private $parent; + /** * @var Profile[] */ @@ -104,6 +109,22 @@ public function getClient() return $this->client; } + /** + * @return Stack + */ + public function getParent() + { + return $this->parent; + } + + /** + * @param Stack $parent + */ + public function setParent(Stack $parent) + { + $this->parent = $parent; + } + /** * @param Profile $profile */ diff --git a/Collector/StackPlugin.php b/Collector/StackPlugin.php index a08b65e4..5e2da321 100644 --- a/Collector/StackPlugin.php +++ b/Collector/StackPlugin.php @@ -52,16 +52,25 @@ public function handleRequest(RequestInterface $request, callable $next, callabl $stack = new Stack($this->client, $this->formatter->formatRequest($request)); $this->collector->addStack($stack); + $this->collector->activateStack($stack); - return $next($request)->then(function (ResponseInterface $response) use ($stack) { + $onFulfilled = function (ResponseInterface $response) use ($stack) { $stack->setResponse($this->formatter->formatResponse($response)); return $response; - }, function (Exception $exception) use ($stack) { + }; + + $onRejected = function (Exception $exception) use ($stack) { $stack->setResponse($this->formatter->formatException($exception)); $stack->setFailed(true); throw $exception; - }); + }; + + try { + return $next($request)->then($onFulfilled, $onRejected); + } finally { + $this->collector->deactivateStack($stack); + } } } diff --git a/Resources/public/style/httplug.css b/Resources/public/style/httplug.css index 88cb6cee..860d7eee 100644 --- a/Resources/public/style/httplug.css +++ b/Resources/public/style/httplug.css @@ -74,6 +74,13 @@ display: flex; } +/** + * Stack + */ +.httplug-stack>.httplug-stack { + margin-left: 2.5em; +} + /** * Stack header */ diff --git a/Resources/views/stack.html.twig b/Resources/views/stack.html.twig new file mode 100644 index 00000000..d366439b --- /dev/null +++ b/Resources/views/stack.html.twig @@ -0,0 +1,75 @@ +
+
+ {% if stack.failed %} + ✘ + {% else %} + ✔ + {% endif %} + {{ stack.requestMethod }} +
+
+ {{ stack.requestScheme }}:// + {{ stack.requestHost }} + {{ stack.requestTarget }} +
+
+ {{ stack.duration|number_format }} ms + {% if stack.responseCode >= 400 and stack.responseCode <= 599 %} + {{ stack.responseCode }} + {% elseif stack.responseCode >= 300 and stack.responseCode <= 399 %} + {{ stack.responseCode }} + {% else %} + {{ stack.responseCode }} + {% endif %} +
+
+
+
+
+ + +
+ + +
+
+
+

Request

+ {{ stack.clientRequest|httplug_markup|nl2br }} +
+
+

Response

+ {{ stack.clientResponse|httplug_markup|nl2br }} +
+
+ {% if stack.profiles %} +
+ {% for profile in stack.profiles %} +

{{ profile.plugin }}

+
+
+

Request

+ {{ profile.request|httplug_markup|nl2br }} +
+
+

Response

+ {{ profile.response|httplug_markup|nl2br }} +
+
+ {% if not loop.last %} +
+ {% endif %} + {% endfor %} +
+ {% endif %} +
+{% for child in collector.childrenStacks(stack) %} +
+ {% include 'HttplugBundle::stack.html.twig' with { + 'collector': collector, + 'client': client, + 'stack': child, + 'id': id ~ '-' ~ loop.index + } only %} +
+{% endfor %} diff --git a/Resources/views/webprofiler.html.twig b/Resources/views/webprofiler.html.twig index 9d0ff3d4..302f345d 100644 --- a/Resources/views/webprofiler.html.twig +++ b/Resources/views/webprofiler.html.twig @@ -78,71 +78,14 @@ These messages are sent by client named "{{ client }}".

- {% for stack in collector.clientStacks(client) %} -
-
- {% if stack.failed %} - ✘ - {% else %} - ✔ - {% endif %} - {{ stack.requestMethod }} -
-
- {{ stack.requestScheme }}:// - {{ stack.requestHost }} - {{ stack.requestTarget }} -
-
- {{ stack.duration|number_format }} ms - {% if stack.responseCode >= 400 and stack.responseCode <= 599 %} - {{ stack.responseCode }} - {% elseif stack.responseCode >= 300 and stack.responseCode <= 399 %} - {{ stack.responseCode }} - {% else %} - {{ stack.responseCode }} - {% endif %} -
-
-
-
-
- - -
- - -
-
-
-

Request

- {{ stack.clientRequest|httplug_markup|nl2br }} -
-
-

Response

- {{ stack.clientResponse|httplug_markup|nl2br }} -
-
- {% if stack.profiles %} -
- {% for profile in stack.profiles %} -

{{ profile.plugin }}

-
-
-

Request

- {{ profile.request|httplug_markup|nl2br }} -
-
-

Response

- {{ profile.response|httplug_markup|nl2br }} -
-
- {% if not loop.last %} -
- {% endif %} - {% endfor %} -
- {% endif %} + {% for stack in collector.clientStacks(client) if not stack.parent %} +
+ {% include 'HttplugBundle::stack.html.twig' with { + 'collector': collector, + 'client': client, + 'stack': stack, + 'id': loop.index + } only %}
{% endfor %}
diff --git a/Tests/Unit/Collector/CollectorTest.php b/Tests/Unit/Collector/CollectorTest.php index 3d853495..bb4445cf 100644 --- a/Tests/Unit/Collector/CollectorTest.php +++ b/Tests/Unit/Collector/CollectorTest.php @@ -17,4 +17,55 @@ public function testCollectClientNames() $this->assertEquals(['default', 'acme'], $collector->getClients()); } + + public function testActivateStack() + { + $parent = new Stack('acme', 'GET / HTTP/1.1'); + $stack = new Stack('acme', 'GET / HTTP/1.1'); + + $collector = new Collector(); + + $collector->activateStack($parent); + $collector->activateStack($stack); + + $this->assertEquals($parent, $stack->getParent()); + $this->assertEquals($stack, $collector->getActiveStack()); + } + + public function testDeactivateStack() + { + $stack = new Stack('acme', 'GET / HTTP/1.1'); + $collector = new Collector(); + + $collector->activateStack($stack); + $this->assertNotNull($collector->getActiveStack()); + + $collector->deactivateStack($stack); + $this->assertNull($collector->getActiveStack()); + } + + public function testDeactivateStackSetParentAsActiveStack() + { + $parent = new Stack('acme', 'GET / HTTP/1.1'); + $stack = new Stack('acme', 'GET / HTTP/1.1'); + + $collector = new Collector(); + + $collector->activateStack($parent); + $collector->activateStack($stack); + $collector->deactivateStack($stack); + + $this->assertEquals($parent, $collector->getActiveStack()); + } + + public function testAddStack() + { + $stack = new Stack('acme', 'GET / HTTP/1.1'); + $collector = new Collector(); + + $collector->addStack($stack); + + $this->assertEquals(['acme'], $collector->getClients()); + $this->assertEquals([$stack], $collector->getClientStacks('acme')); + } } diff --git a/Tests/Unit/Collector/ProfileClientTest.php b/Tests/Unit/Collector/ProfileClientTest.php index 095cf912..9dbe1896 100644 --- a/Tests/Unit/Collector/ProfileClientTest.php +++ b/Tests/Unit/Collector/ProfileClientTest.php @@ -13,10 +13,12 @@ use Http\HttplugBundle\Collector\Stack; use Http\Promise\FulfilledPromise; use Http\Promise\Promise; +use Http\Promise\RejectedPromise; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; use Symfony\Component\Stopwatch\Stopwatch; +use Symfony\Component\Stopwatch\StopwatchEvent; class ProfileClientTest extends \PHPUnit_Framework_TestCase { @@ -28,7 +30,7 @@ class ProfileClientTest extends \PHPUnit_Framework_TestCase /** * @var Stack */ - private $currentStack; + private $activeStack; /** * @var HttpClient @@ -50,6 +52,11 @@ class ProfileClientTest extends \PHPUnit_Framework_TestCase */ private $stopwatch; + /** + * @var StopwatchEvent + */ + private $stopwatchEvent; + /** * @var ProfileClient */ @@ -63,7 +70,17 @@ class ProfileClientTest extends \PHPUnit_Framework_TestCase /** * @var Promise */ - private $promise; + private $fulfilledPromise; + + /** + * @var \Exception + */ + private $exception; + + /** + * @var RejectedPromise + */ + private $rejectedPromise; /** * @var UriInterface @@ -73,60 +90,130 @@ class ProfileClientTest extends \PHPUnit_Framework_TestCase public function setUp() { $this->collector = $this->getMockBuilder(Collector::class)->disableOriginalConstructor()->getMock(); - $this->currentStack = new Stack('default', 'FormattedRequest'); + $this->activeStack = new Stack('default', 'FormattedRequest'); $this->client = $this->getMockBuilder(ClientInterface::class)->getMock(); $this->uri = new Uri('https://example.com/target'); $this->request = new Request('GET', $this->uri); $this->formatter = $this->getMockBuilder(Formatter::class)->disableOriginalConstructor()->getMock(); - $this->stopwatch = new Stopwatch(); + $this->stopwatch = $this->getMockBuilder(Stopwatch::class)->disableOriginalConstructor()->getMock(); + $this->stopwatchEvent = $this->getMockBuilder(StopwatchEvent::class)->disableOriginalConstructor()->getMock(); $this->subject = new ProfileClient($this->client, $this->collector, $this->formatter, $this->stopwatch); $this->response = new Response(); - $this->promise = new FulfilledPromise($this->response); + $this->exception = new \Exception(); + $this->fulfilledPromise = new FulfilledPromise($this->response); + $this->rejectedPromise = new RejectedPromise($this->exception); + + $this->collector->method('getActiveStack')->willReturn($this->activeStack); + $this->formatter + ->method('formatResponse') + ->with($this->response) + ->willReturn('FormattedResponse') + ; + $this->formatter + ->method('formatException') + ->with($this->exception) + ->willReturn('FormattedException') + ; - $this->client->method('sendRequest')->willReturn($this->response); - $this->client->method('sendAsyncRequest')->will($this->returnCallback(function () { - return $this->promise; - })); + $this->stopwatch + ->method('start') + ->willReturn($this->stopwatchEvent) + ; - $this->collector->method('getCurrentStack')->willReturn($this->currentStack); + $this->stopwatchEvent + ->method('getDuration') + ->willReturn(42) + ; } - public function testCallDecoratedClient() + public function testSendRequest() { $this->client ->expects($this->once()) ->method('sendRequest') ->with($this->identicalTo($this->request)) + ->willReturn($this->response) ; + $response = $this->subject->sendRequest($this->request); + + $this->assertEquals($this->response, $response); + $this->assertEquals('GET', $this->activeStack->getRequestMethod()); + $this->assertEquals('/target', $this->activeStack->getRequestTarget()); + $this->assertEquals('example.com', $this->activeStack->getRequestHost()); + $this->assertEquals('https', $this->activeStack->getRequestScheme()); + } + + public function testSendAsyncRequest() + { $this->client ->expects($this->once()) ->method('sendAsyncRequest') ->with($this->identicalTo($this->request)) + ->willReturn($this->fulfilledPromise) ; - $this->assertEquals($this->response, $this->subject->sendRequest($this->request)); - $this->assertEquals($this->promise, $this->subject->sendAsyncRequest($this->request)); + $this->collector + ->expects($this->once()) + ->method('deactivateStack') + ; + + $promise = $this->subject->sendAsyncRequest($this->request); + + $this->assertEquals($this->fulfilledPromise, $promise); + $this->assertEquals('GET', $this->activeStack->getRequestMethod()); + $this->assertEquals('/target', $this->activeStack->getRequestTarget()); + $this->assertEquals('example.com', $this->activeStack->getRequestHost()); + $this->assertEquals('https', $this->activeStack->getRequestScheme()); } - public function testCollectRequestInformations() + public function testOnFulfilled() { - $this->subject->sendRequest($this->request); + $this->collector + ->expects($this->once()) + ->method('activateStack') + ->with($this->activeStack) + ; + + $this->stopwatchEvent + ->expects($this->once()) + ->method('stop') + ; + + $this->client + ->method('sendAsyncRequest') + ->willReturn($this->fulfilledPromise) + ; + + $this->subject->sendAsyncRequest($this->request); - $this->assertEquals('GET', $this->currentStack->getRequestMethod()); - $this->assertEquals('/target', $this->currentStack->getRequestTarget()); - $this->assertEquals('example.com', $this->currentStack->getRequestHost()); - $this->assertEquals('https', $this->currentStack->getRequestScheme()); + $this->assertEquals(42, $this->activeStack->getDuration()); + $this->assertEquals(200, $this->activeStack->getResponseCode()); + $this->assertEquals('FormattedResponse', $this->activeStack->getClientResponse()); } - public function testCollectAsyncRequestInformations() + public function testOnRejected() { + $this->collector + ->expects($this->once()) + ->method('activateStack') + ->with($this->activeStack) + ; + + $this->stopwatchEvent + ->expects($this->once()) + ->method('stop') + ; + + $this->client + ->method('sendAsyncRequest') + ->willReturn($this->rejectedPromise) + ; + $this->subject->sendAsyncRequest($this->request); - $this->assertEquals('GET', $this->currentStack->getRequestMethod()); - $this->assertEquals('/target', $this->currentStack->getRequestTarget()); - $this->assertEquals('example.com', $this->currentStack->getRequestHost()); - $this->assertEquals('https', $this->currentStack->getRequestScheme()); + $this->assertEquals(42, $this->activeStack->getDuration()); + $this->assertEquals('FormattedException', $this->activeStack->getClientException()); } } diff --git a/Tests/Unit/Collector/ProfilePluginTest.php b/Tests/Unit/Collector/ProfilePluginTest.php index 1492e328..d82eb291 100644 --- a/Tests/Unit/Collector/ProfilePluginTest.php +++ b/Tests/Unit/Collector/ProfilePluginTest.php @@ -81,7 +81,7 @@ public function setUp() $this->formatter = $this->getMockBuilder(Formatter::class)->disableOriginalConstructor()->getMock(); $this->collector - ->method('getCurrentStack') + ->method('getActiveStack') ->willReturn($this->currentStack) ; diff --git a/Tests/Unit/Collector/StackPluginTest.php b/Tests/Unit/Collector/StackPluginTest.php index 21f10640..a2a0e826 100644 --- a/Tests/Unit/Collector/StackPluginTest.php +++ b/Tests/Unit/Collector/StackPluginTest.php @@ -88,6 +88,10 @@ public function testStackIsInitialized() return true; })) ; + $this->collector + ->expects($this->once()) + ->method('activateStack') + ; $next = function () { return new FulfilledPromise($this->response); @@ -109,6 +113,10 @@ public function testOnFulfilled() return true; })) ; + $this->collector + ->expects($this->once()) + ->method('deactivateStack') + ; $next = function () { return new FulfilledPromise($this->response); @@ -134,6 +142,10 @@ public function testOnRejected() return true; })) ; + $this->collector + ->expects($this->once()) + ->method('deactivateStack') + ; $this->setExpectedException(Exception::class); @@ -149,4 +161,43 @@ public function testOnRejected() $this->assertEquals('FormattedException', $currentStack->getResponse()); $this->assertTrue($currentStack->isFailed()); } + + public function testOnException() + { + $this->collector + ->expects($this->once()) + ->method('deactivateStack') + ; + + $this->setExpectedException(\Exception::class); + + $next = function () { + throw new \Exception(); + }; + + $this->subject->handleRequest($this->request, $next, function () { + }); + } + + public function testOnError() + { + if (!interface_exists(\Throwable::class)) { + $this->markTestSkipped(); + } + + $this->collector + ->expects($this->once()) + ->method('deactivateStack') + ; + + //PHPUnit wrap any \Error into a \PHPUnit_Framework_Error. So we are expecting the + $this->setExpectedException(\PHPUnit_Framework_Error::class); + + $next = function () { + return 2 / 0; + }; + + $this->subject->handleRequest($this->request, $next, function () { + }); + } } diff --git a/composer.json b/composer.json index 755129c3..f62afcde 100644 --- a/composer.json +++ b/composer.json @@ -32,6 +32,7 @@ }, "require-dev": { "phpunit/phpunit": "^4.5 || ^5.4", + "phpunit/php-token-stream": "^1.1.8", "php-http/curl-client": "^1.0", "php-http/socket-client": "^1.0", "php-http/guzzle6-adapter": "^1.1.1",