diff --git a/Collector/ProfilePlugin.php b/Collector/ProfilePlugin.php index 96d5b817..bf5d54e8 100644 --- a/Collector/ProfilePlugin.php +++ b/Collector/ProfilePlugin.php @@ -65,27 +65,105 @@ public function handleRequest(RequestInterface $request, callable $next, callabl // wrap the next callback to profile the plugin request changes $wrappedNext = function (RequestInterface $request) use ($next, $profile) { - $profile->setRequest($this->formatter->formatRequest($request)); + $this->onOutgoingRequest($request, $profile); return $next($request); }; // wrap the first callback to profile the plugin request changes $wrappedFirst = function (RequestInterface $request) use ($first, $profile) { - $profile->setRequest($this->formatter->formatRequest($request)); + $this->onOutgoingRequest($request, $profile); return $first($request); }; - return $this->plugin->handleRequest($request, $wrappedNext, $wrappedFirst)->then(function (ResponseInterface $response) use ($profile) { - $profile->setResponse($this->formatter->formatResponse($response)); + try { + $promise = $this->plugin->handleRequest($request, $wrappedNext, $wrappedFirst); + } catch (Exception $e) { + $this->onException($request, $profile, $e, $stack); + + throw $e; + } + + return $promise->then(function (ResponseInterface $response) use ($profile, $request, $stack) { + $this->onOutgoingResponse($response, $profile, $request, $stack); return $response; - }, function (Exception $exception) use ($profile) { - $profile->setFailed(true); - $profile->setResponse($this->formatter->formatException($exception)); + }, function (Exception $exception) use ($profile, $request, $stack) { + $this->onException($request, $profile, $exception, $stack); throw $exception; }); } + + /** + * @param RequestInterface $request + * @param Profile $profile + * @param Exception $exception + * @param Stack $stack + */ + private function onException( + RequestInterface $request, + Profile $profile, + Exception $exception, + Stack $stack = null + ) { + $profile->setFailed(true); + $profile->setResponse($this->formatter->formatException($exception)); + $this->collectRequestInformation($request, $stack); + } + + /** + * @param RequestInterface $request + * @param Profile $profile + */ + private function onOutgoingRequest(RequestInterface $request, Profile $profile) + { + $profile->setRequest($this->formatter->formatRequest($request)); + } + + /** + * @param ResponseInterface $response + * @param Profile $profile + * @param RequestInterface $request + * @param Stack $stack + */ + private function onOutgoingResponse(ResponseInterface $response, Profile $profile, RequestInterface $request, Stack $stack = null) + { + $profile->setResponse($this->formatter->formatResponse($response)); + $this->collectRequestInformation($request, $stack); + } + + /** + * Collect request information when not already done by the HTTP client. This happens when using the CachePlugin + * and the cache is hit without re-validation. + * + * @param RequestInterface $request + * @param Stack|null $stack + */ + private function collectRequestInformation(RequestInterface $request, Stack $stack = null) + { + if (null === $stack) { + return; + } + + if (empty($stack->getRequestTarget())) { + $stack->setRequestTarget($request->getRequestTarget()); + } + if (empty($stack->getRequestMethod())) { + $stack->setRequestMethod($request->getMethod()); + } + if (empty($stack->getRequestScheme())) { + $stack->setRequestScheme($request->getUri()->getScheme()); + } + if (empty($stack->getRequestHost())) { + $stack->setRequestHost($request->getUri()->getHost()); + } + if (empty($stack->getClientRequest())) { + $stack->setClientRequest($this->formatter->formatRequest($request)); + } + if (empty($stack->getCurlCommand())) { + $stack->setCurlCommand($this->formatter->formatAsCurlCommand($request)); + } + } } diff --git a/Tests/Functional/ProfilingTest.php b/Tests/Functional/ProfilingTest.php new file mode 100644 index 00000000..1a06e535 --- /dev/null +++ b/Tests/Functional/ProfilingTest.php @@ -0,0 +1,130 @@ +collector = new Collector([]); + $this->formatter = new Formatter(new FullHttpMessageFormatter(), new CurlCommandFormatter()); + $this->stopwatch = new Stopwatch(); + } + + public function testProfilingWithCachePlugin() + { + $client = $this->createClient([ + new Plugin\CachePlugin(new ArrayAdapter(), StreamFactoryDiscovery::find(), [ + 'respect_response_cache_directives' => [], + 'default_ttl' => 86400, + ]), + ]); + + $client->sendRequest(new Request('GET', 'https://example.com')); + $client->sendRequest(new Request('GET', 'https://example.com')); + + $this->assertCount(2, $this->collector->getStacks()); + $stack = $this->collector->getStacks()[1]; + $this->assertEquals('GET', $stack->getRequestMethod()); + $this->assertEquals('https', $stack->getRequestScheme()); + $this->assertEquals('/', $stack->getRequestTarget()); + $this->assertEquals('example.com', $stack->getRequestHost()); + } + + public function testProfilingWhenPluginThrowException() + { + $client = $this->createClient([ + new ExceptionThrowerPlugin(), + ]); + + $this->setExpectedException(\Exception::class); + + try { + $client->sendRequest(new Request('GET', 'https://example.com')); + } finally { + $this->assertCount(1, $this->collector->getStacks()); + $stack = $this->collector->getStacks()[0]; + $this->assertEquals('GET', $stack->getRequestMethod()); + $this->assertEquals('https', $stack->getRequestScheme()); + $this->assertEquals('/', $stack->getRequestTarget()); + $this->assertEquals('example.com', $stack->getRequestHost()); + } + } + + public function testProfiling() + { + $client = $this->createClient([ + new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri('https://example.com')), + new Plugin\RedirectPlugin(), + new Plugin\RetryPlugin(), + ]); + + $client->sendRequest(new Request('GET', '/')); + + $this->assertCount(1, $this->collector->getStacks()); + $stack = $this->collector->getStacks()[0]; + $this->assertCount(3, $stack->getProfiles()); + $this->assertEquals('GET', $stack->getRequestMethod()); + $this->assertEquals('https', $stack->getRequestScheme()); + $this->assertEquals('/', $stack->getRequestTarget()); + $this->assertEquals('example.com', $stack->getRequestHost()); + } + + private function createClient(array $plugins, $clientName = 'Acme', array $clientOptions = []) + { + $plugins = array_map(function (Plugin $plugin) { + return new ProfilePlugin($plugin, $this->collector, $this->formatter, get_class($plugin)); + }, $plugins); + + array_unshift($plugins, new StackPlugin($this->collector, $this->formatter, $clientName)); + + $client = new Client(); + $client = new ProfileClient($client, $this->collector, $this->formatter, $this->stopwatch); + $client = new PluginClient($client, $plugins, $clientOptions); + + return $client; + } +} + +class ExceptionThrowerPlugin implements Plugin +{ + /** + * {@inheritdoc} + */ + public function handleRequest(RequestInterface $request, callable $next, callable $first) + { + throw new \Exception(); + } +} diff --git a/Tests/Unit/Collector/ProfilePluginTest.php b/Tests/Unit/Collector/ProfilePluginTest.php index 2e971a28..dbf7eac4 100644 --- a/Tests/Unit/Collector/ProfilePluginTest.php +++ b/Tests/Unit/Collector/ProfilePluginTest.php @@ -11,6 +11,8 @@ use Http\HttplugBundle\Collector\ProfilePlugin; 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; @@ -36,6 +38,11 @@ class ProfilePluginTest extends \PHPUnit_Framework_TestCase */ private $response; + /** + * @var Promise + */ + private $fulfilledPromise; + /** * @var Stack */ @@ -46,6 +53,11 @@ class ProfilePluginTest extends \PHPUnit_Framework_TestCase */ private $exception; + /** + * @var Promise + */ + private $rejectedPromise; + /** * @var Formatter */ @@ -62,8 +74,10 @@ public function setUp() $this->collector = $this->getMockBuilder(Collector::class)->disableOriginalConstructor()->getMock(); $this->request = new Request('GET', '/'); $this->response = new Response(); + $this->fulfilledPromise = new FulfilledPromise($this->response); $this->currentStack = new Stack('default', 'FormattedRequest'); $this->exception = new TransferException(); + $this->rejectedPromise = new RejectedPromise($this->exception); $this->formatter = $this->getMockBuilder(Formatter::class)->disableOriginalConstructor()->getMock(); $this->collector @@ -74,9 +88,7 @@ public function setUp() $this->plugin ->method('handleRequest') ->willReturnCallback(function ($request, $next, $first) { - $next($request); - - return new FulfilledPromise($this->response); + return $next($request); }) ; @@ -115,6 +127,7 @@ public function testCallDecoratedPlugin() ; $this->subject->handleRequest($this->request, function () { + return $this->fulfilledPromise; }, function () { }); } @@ -122,6 +135,7 @@ public function testCallDecoratedPlugin() public function testProfileIsInitialized() { $this->subject->handleRequest($this->request, function () { + return $this->fulfilledPromise; }, function () { }); @@ -133,6 +147,7 @@ public function testProfileIsInitialized() public function testCollectRequestInformations() { $this->subject->handleRequest($this->request, function () { + return $this->fulfilledPromise; }, function () { }); @@ -143,6 +158,7 @@ public function testCollectRequestInformations() public function testOnFulfilled() { $promise = $this->subject->handleRequest($this->request, function () { + return $this->fulfilledPromise; }, function () { }); @@ -156,7 +172,7 @@ public function testOnRejected() $this->setExpectedException(TransferException::class); $promise = $this->subject->handleRequest($this->request, function () { - throw new TransferException(); + return $this->rejectedPromise; }, function () { }); diff --git a/composer.json b/composer.json index 3376db0c..755129c3 100644 --- a/composer.json +++ b/composer.json @@ -37,11 +37,13 @@ "php-http/guzzle6-adapter": "^1.1.1", "php-http/react-adapter": "^0.2.1", "php-http/buzz-adapter": "^0.3", + "php-http/mock-client": "^1.0", "symfony/phpunit-bridge": "^3.2", "symfony/twig-bundle": "^2.8 || ^3.0", "symfony/twig-bridge": "^2.8 || ^3.0", "symfony/web-profiler-bundle": "^2.8 || ^3.0", "symfony/finder": "^2.7 || ^3.0", + "symfony/cache": "^3.1", "polishsymfonycommunity/symfony-mocker-container": "^1.0", "matthiasnoback/symfony-dependency-injection-test": "^1.0" },