From 2cfdc48e650c3e6ba220dd9321ba468d30a87749 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 26 Feb 2019 17:24:12 +0100 Subject: [PATCH 1/4] Add SubRequestListener --- src/EventListener/SubRequestListener.php | 38 +++++++++ test/EventListener/SubRequestListenerTest.php | 83 +++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 src/EventListener/SubRequestListener.php create mode 100644 test/EventListener/SubRequestListenerTest.php diff --git a/src/EventListener/SubRequestListener.php b/src/EventListener/SubRequestListener.php new file mode 100644 index 00000000..80c473c8 --- /dev/null +++ b/src/EventListener/SubRequestListener.php @@ -0,0 +1,38 @@ +isMasterRequest()) { + return; + } + + Hub::getCurrent()->pushScope(); + } + + /** + * Pops a {@see Scope} for each finished SubRequest + * + * @param FinishRequestEvent $event + */ + public function onKernelFinishRequest(FinishRequestEvent $event): void + { + if ($event->isMasterRequest()) { + return; + } + + Hub::getCurrent()->popScope(); + } +} diff --git a/test/EventListener/SubRequestListenerTest.php b/test/EventListener/SubRequestListenerTest.php new file mode 100644 index 00000000..cb4db719 --- /dev/null +++ b/test/EventListener/SubRequestListenerTest.php @@ -0,0 +1,83 @@ +currentHub = $this->prophesize(HubInterface::class); + + Hub::setCurrent($this->currentHub->reveal()); + } + + public function testOnKernelRequestWithMasterRequest(): void + { + $listener = new SubRequestListener(); + + $subRequestEvent = $this->prophesize(GetResponseEvent::class); + $subRequestEvent->isMasterRequest() + ->willReturn(true); + + $this->currentHub->pushScope() + ->shouldNotBeCalled(); + + $listener->onKernelRequest($subRequestEvent->reveal()); + } + + public function testOnKernelRequestWithSubRequest(): void + { + $listener = new SubRequestListener(); + + $subRequestEvent = $this->prophesize(GetResponseEvent::class); + $subRequestEvent->isMasterRequest() + ->willReturn(false); + + $this->currentHub->pushScope() + ->shouldBeCalledTimes(1) + ->willReturn(new Scope()); + + $listener->onKernelRequest($subRequestEvent->reveal()); + } + + public function testOnKernelFinishRequestWithMasterRequest(): void + { + $listener = new SubRequestListener(); + + $subRequestEvent = $this->prophesize(FinishRequestEvent::class); + $subRequestEvent->isMasterRequest() + ->willReturn(true); + + $this->currentHub->popScope() + ->shouldNotBeCalled(); + + $listener->onKernelFinishRequest($subRequestEvent->reveal()); + } + + public function testOnKernelFinishRequestWithSubRequest(): void + { + $listener = new SubRequestListener(); + + $subRequestEvent = $this->prophesize(FinishRequestEvent::class); + $subRequestEvent->isMasterRequest() + ->willReturn(false); + + $this->currentHub->popScope() + ->shouldBeCalledTimes(1) + ->willReturn(true); + + $listener->onKernelFinishRequest($subRequestEvent->reveal()); + } +} From 52284d5b7da3a46ab098b0334a2c32b267bcb481 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 26 Feb 2019 17:28:22 +0100 Subject: [PATCH 2/4] Remove unused public aliases in tests --- test/DependencyInjection/SentryExtensionTest.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index e9177c77..9e1c9c6d 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -7,8 +7,6 @@ use Sentry\Event; use Sentry\Options; use Sentry\SentryBundle\DependencyInjection\SentryExtension; -use Sentry\SentryBundle\EventListener\ConsoleListener; -use Sentry\SentryBundle\EventListener\RequestListener; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -19,8 +17,6 @@ class SentryExtensionTest extends TestCase { - private const REQUEST_LISTENER_TEST_PUBLIC_ALIAS = 'sentry.request_listener.public_alias'; - private const CONSOLE_LISTENER_TEST_PUBLIC_ALIAS = 'sentry.console_listener.public_alias'; private const OPTIONS_TEST_PUBLIC_ALIAS = 'sentry.options.public_alias'; public function testDataProviderIsMappingTheRightNumberOfOptions(): void @@ -299,8 +295,6 @@ private function getContainer(array $configuration = []): Container $containerBuilder->set('request_stack', $mockRequestStack); $containerBuilder->set('event_dispatcher', $mockEventDispatcher); $containerBuilder->setAlias(self::OPTIONS_TEST_PUBLIC_ALIAS, new Alias(Options::class, true)); - $containerBuilder->setAlias(self::REQUEST_LISTENER_TEST_PUBLIC_ALIAS, new Alias(RequestListener::class, true)); - $containerBuilder->setAlias(self::CONSOLE_LISTENER_TEST_PUBLIC_ALIAS, new Alias(ConsoleListener::class, true)); $beforeSend = new Definition('callable'); $beforeSend->setFactory([CallbackMock::class, 'createCallback']); From 3933c8a28de911462204fe7b8cd856b666b75237 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 26 Feb 2019 17:30:16 +0100 Subject: [PATCH 3/4] Configure the SubRequestListener --- src/DependencyInjection/Configuration.php | 1 + src/Resources/config/services.xml | 5 +++ .../DependencyInjection/ConfigurationTest.php | 1 + test/SentryBundleTest.php | 33 +++++++++++++++++-- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 97583099..ceba0ba8 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -120,6 +120,7 @@ public function getConfigTreeBuilder() ->addDefaultsIfNotSet() ->children() ->scalarNode('request')->defaultValue(1)->end() + ->scalarNode('sub_request')->defaultValue(1)->end() ->scalarNode('console')->defaultValue(1)->end() ->end() ->end(); diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index ab43c4a7..cb55102e 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -38,5 +38,10 @@ + + + + + diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index a2d5bd46..647d82ac 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -45,6 +45,7 @@ public function testConfigurationDefaults(): void 'dsn' => null, 'listener_priorities' => [ 'request' => 1, + 'sub_request' => 1, 'console' => 1, ], 'options' => [ diff --git a/test/SentryBundleTest.php b/test/SentryBundleTest.php index 221d28c2..e94c4166 100644 --- a/test/SentryBundleTest.php +++ b/test/SentryBundleTest.php @@ -6,9 +6,12 @@ use Sentry\SentryBundle\DependencyInjection\SentryExtension; use Sentry\SentryBundle\EventListener\ConsoleListener; use Sentry\SentryBundle\EventListener\RequestListener; +use Sentry\SentryBundle\EventListener\SubRequestListener; +use Symfony\Component\Console\ConsoleEvents; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\HttpKernel\KernelEvents; class SentryBundleTest extends TestCase { @@ -21,7 +24,7 @@ public function testContainerHasConsoleListenerConfiguredCorrectly(): void $expectedTag = [ 'kernel.event_listener' => [ [ - 'event' => 'console.command', + 'event' => ConsoleEvents::COMMAND, 'method' => 'onConsoleCommand', 'priority' => '%sentry.listener_priorities.console%', ], @@ -40,12 +43,12 @@ public function testContainerHasRequestListenerConfiguredCorrectly(): void $expectedTag = [ 'kernel.event_listener' => [ [ - 'event' => 'kernel.request', + 'event' => KernelEvents::REQUEST, 'method' => 'onKernelRequest', 'priority' => '%sentry.listener_priorities.request%', ], [ - 'event' => 'kernel.controller', + 'event' => KernelEvents::CONTROLLER, 'method' => 'onKernelController', 'priority' => '%sentry.listener_priorities.request%', ], @@ -55,6 +58,30 @@ public function testContainerHasRequestListenerConfiguredCorrectly(): void $this->assertSame($expectedTag, $consoleListener->getTags()); } + public function testContainerHasSubRequestListenerConfiguredCorrectly(): void + { + $container = $this->getContainer(); + + $consoleListener = $container->getDefinition(SubRequestListener::class); + + $expectedTag = [ + 'kernel.event_listener' => [ + [ + 'event' => KernelEvents::REQUEST, + 'method' => 'onKernelRequest', + 'priority' => '%sentry.listener_priorities.sub_request%', + ], + [ + 'event' => KernelEvents::FINISH_REQUEST, + 'method' => 'onKernelFinishRequest', + 'priority' => '%sentry.listener_priorities.sub_request%', + ], + ], + ]; + + $this->assertSame($expectedTag, $consoleListener->getTags()); + } + private function getContainer(array $configuration = []): ContainerBuilder { $containerBuilder = new ContainerBuilder(); From a57fb741c524fd1a077b6dc75a715fb11cad7ac9 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Tue, 26 Feb 2019 17:42:14 +0100 Subject: [PATCH 4/4] Add tests for listener priorities --- test/DependencyInjection/SentryExtensionTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/DependencyInjection/SentryExtensionTest.php b/test/DependencyInjection/SentryExtensionTest.php index 9e1c9c6d..61bec66b 100644 --- a/test/DependencyInjection/SentryExtensionTest.php +++ b/test/DependencyInjection/SentryExtensionTest.php @@ -49,9 +49,25 @@ public function testOptionsDefaultValues(): void $this->assertSame(['var/cache', $vendorDir], $options->getInAppExcludedPaths()); $this->assertSame(1, $container->getParameter('sentry.listener_priorities.request')); + $this->assertSame(1, $container->getParameter('sentry.listener_priorities.sub_request')); $this->assertSame(1, $container->getParameter('sentry.listener_priorities.console')); } + public function testListenerPriorities(): void + { + $container = $this->getContainer([ + 'listener_priorities' => [ + 'request' => 123, + 'sub_request' => 456, + 'console' => 789, + ], + ]); + + $this->assertSame(123, $container->getParameter('sentry.listener_priorities.request')); + $this->assertSame(456, $container->getParameter('sentry.listener_priorities.sub_request')); + $this->assertSame(789, $container->getParameter('sentry.listener_priorities.console')); + } + /** * @dataProvider optionsValueProvider */