From 3620f43a9e7480cfcd726dfc6c2147d4006da96b Mon Sep 17 00:00:00 2001 From: Patrick Weyck Date: Mon, 26 Jun 2023 16:55:46 +0200 Subject: [PATCH] NEXT-26632 - Refactor annotate package into a processor --- ...ndler.php => AnnotatePackageProcessor.php} | 29 ++---- ...t.php => AnnotatePackageProcessorTest.php} | 95 +++++++------------ 2 files changed, 45 insertions(+), 79 deletions(-) rename src/Core/Framework/Log/Monolog/{AnnotatePackageHandler.php => AnnotatePackageProcessor.php} (79%) rename tests/unit/php/Core/Framework/Log/Monolog/{AnnotatePackageHandlerTest.php => AnnotatePackageProcessorTest.php} (80%) diff --git a/src/Core/Framework/Log/Monolog/AnnotatePackageHandler.php b/src/Core/Framework/Log/Monolog/AnnotatePackageProcessor.php similarity index 79% rename from src/Core/Framework/Log/Monolog/AnnotatePackageHandler.php rename to src/Core/Framework/Log/Monolog/AnnotatePackageProcessor.php index 49744c54606..daf19fe0301 100644 --- a/src/Core/Framework/Log/Monolog/AnnotatePackageHandler.php +++ b/src/Core/Framework/Log/Monolog/AnnotatePackageProcessor.php @@ -2,9 +2,8 @@ namespace Shopware\Core\Framework\Log\Monolog; -use Monolog\Handler\AbstractHandler; -use Monolog\Handler\HandlerInterface; use Monolog\LogRecord; +use Monolog\Processor\ProcessorInterface; use Shopware\Core\Framework\Log\Package; use Symfony\Component\Console\Command\Command; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -14,26 +13,27 @@ * @internal */ #[Package('core')] -class AnnotatePackageHandler extends AbstractHandler +class AnnotatePackageProcessor implements ProcessorInterface { /** * @internal */ public function __construct( - private readonly HandlerInterface $handler, private readonly RequestStack $requestStack, private readonly ContainerInterface $container, ) { - parent::__construct(); } - public function handle(LogRecord $record): bool + /** + * {@inheritdoc} + */ + public function __invoke(LogRecord $record) { $packages = []; $exception = $record->context['exception'] ?? null; if ($exception instanceof \ErrorException && str_starts_with($exception->getMessage(), 'User Deprecated:')) { - return $this->handler->handle($record); + return $record; } if ($controllerPackage = $this->getControllerPackage()) { @@ -55,21 +55,10 @@ public function handle(LogRecord $record): bool } if ($packages !== []) { - $context = $record->context; - $context[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = $packages; - - $record = new LogRecord( - $record->datetime, - $record->channel, - $record->level, - $record->message, - $context, - $record->extra, - $record->formatted - ); + $record->extra[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = $packages; } - return $this->handler->handle($record); + return $record; } private function getControllerPackage(): ?string diff --git a/tests/unit/php/Core/Framework/Log/Monolog/AnnotatePackageHandlerTest.php b/tests/unit/php/Core/Framework/Log/Monolog/AnnotatePackageProcessorTest.php similarity index 80% rename from tests/unit/php/Core/Framework/Log/Monolog/AnnotatePackageHandlerTest.php rename to tests/unit/php/Core/Framework/Log/Monolog/AnnotatePackageProcessorTest.php index 125e31941ba..a7a5e38bbe8 100644 --- a/tests/unit/php/Core/Framework/Log/Monolog/AnnotatePackageHandlerTest.php +++ b/tests/unit/php/Core/Framework/Log/Monolog/AnnotatePackageProcessorTest.php @@ -6,7 +6,7 @@ use Monolog\Level; use Monolog\LogRecord; use PHPUnit\Framework\TestCase; -use Shopware\Core\Framework\Log\Monolog\AnnotatePackageHandler; +use Shopware\Core\Framework\Log\Monolog\AnnotatePackageProcessor; use Shopware\Core\Framework\Log\Package; use Shopware\Core\Framework\ShopwareHttpException; use Symfony\Component\Console\Command\Command; @@ -18,19 +18,19 @@ use Symfony\Component\HttpFoundation\Response; /** - * @covers \Shopware\Core\Framework\Log\Monolog\AnnotatePackageHandler + * @covers \Shopware\Core\Framework\Log\Monolog\AnnotatePackageProcessor * * @internal */ #[Package('cause')] -class AnnotatePackageHandlerTest extends TestCase +class AnnotatePackageProcessorTest extends TestCase { public function testOnlyController(): void { $requestStack = new RequestStack(); $inner = $this->createMock(AbstractHandler::class); $container = $this->createMock(ContainerInterface::class); - $handler = new AnnotatePackageHandler($inner, $requestStack, $container); + $handler = new AnnotatePackageProcessor($requestStack, $container); $request = new Request(); $request->attributes->set('_controller', TestController::class . '::load'); @@ -43,22 +43,18 @@ public function testOnlyController(): void 'Some message' ); - $context[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ - 'entrypoint' => 'controller', - ]; - $expected = new LogRecord( $record->datetime, $record->channel, $record->level, $record->message, - $context + [] ); + $expected->extra[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ + 'entrypoint' => 'controller', + ]; - $inner->expects(static::once()) - ->method('handle') - ->with($expected); - $handler->handle($record); + static::assertEquals($expected, $handler($record)); } public function testOnlyControllerWithNonClassServiceId(): void @@ -66,7 +62,7 @@ public function testOnlyControllerWithNonClassServiceId(): void $requestStack = new RequestStack(); $inner = $this->createMock(AbstractHandler::class); $container = $this->createMock(ContainerInterface::class); - $handler = new AnnotatePackageHandler($inner, $requestStack, $container); + $handler = new AnnotatePackageProcessor($requestStack, $container); $request = new Request(); $request->attributes->set('_controller', 'test.controller::load'); @@ -83,22 +79,18 @@ public function testOnlyControllerWithNonClassServiceId(): void 'Some message' ); - $context[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ - 'entrypoint' => 'controller', - ]; - $expected = new LogRecord( $record->datetime, $record->channel, $record->level, $record->message, - $context + [] ); + $expected->extra[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ + 'entrypoint' => 'controller', + ]; - $inner->expects(static::once()) - ->method('handle') - ->with($expected); - $handler->handle($record); + static::assertEquals($expected, $handler($record)); } public function testOnlyControllerWithInvalidServiceId(): void @@ -106,7 +98,7 @@ public function testOnlyControllerWithInvalidServiceId(): void $requestStack = new RequestStack(); $inner = $this->createMock(AbstractHandler::class); $container = $this->createMock(ContainerInterface::class); - $handler = new AnnotatePackageHandler($inner, $requestStack, $container); + $handler = new AnnotatePackageProcessor($requestStack, $container); $request = new Request(); $request->attributes->set('_controller', 'test.controller::load'); @@ -123,10 +115,7 @@ public function testOnlyControllerWithInvalidServiceId(): void 'Some message' ); - $inner->expects(static::once()) - ->method('handle') - ->with($record); - $handler->handle($record); + static::assertEquals($record, $handler($record)); } public function testExceptionInController(): void @@ -134,7 +123,7 @@ public function testExceptionInController(): void $requestStack = new RequestStack(); $inner = $this->createMock(AbstractHandler::class); $container = $this->createMock(ContainerInterface::class); - $handler = new AnnotatePackageHandler($inner, $requestStack, $container); + $handler = new AnnotatePackageProcessor($requestStack, $container); $request = new Request(); $request->attributes->set('_controller', TestController::class . '::load'); @@ -158,12 +147,6 @@ public function testExceptionInController(): void $context ); - $context[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ - 'entrypoint' => 'controller', - 'exception' => 'exception', - 'causingClass' => 'cause', - ]; - $expected = new LogRecord( $record->datetime, $record->channel, @@ -171,11 +154,13 @@ public function testExceptionInController(): void $record->message, $context ); + $expected->extra[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ + 'entrypoint' => 'controller', + 'exception' => 'exception', + 'causingClass' => 'cause', + ]; - $inner->expects(static::once()) - ->method('handle') - ->with($expected); - $handler->handle($record); + static::assertEquals($expected, $handler($record)); } public function testNoPackageAttributes(): void @@ -183,7 +168,7 @@ public function testNoPackageAttributes(): void $requestStack = new RequestStack(); $inner = $this->createMock(AbstractHandler::class); $container = $this->createMock(ContainerInterface::class); - $handler = new AnnotatePackageHandler($inner, $requestStack, $container); + $handler = new AnnotatePackageProcessor($requestStack, $container); $request = new Request(); $request->attributes->set('_controller', TestControllerNoPackage::class . '::load'); @@ -207,10 +192,6 @@ public function testNoPackageAttributes(): void $context ); - $context[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ - 'causingClass' => 'cause', - ]; - $expected = new LogRecord( $record->datetime, $record->channel, @@ -218,11 +199,11 @@ public function testNoPackageAttributes(): void $record->message, $context ); + $expected->extra[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ + 'causingClass' => 'cause', + ]; - $inner->expects(static::once()) - ->method('handle') - ->with($expected); - $handler->handle($record); + static::assertEquals($expected, $handler($record)); } public function testAnnotateCommand(): void @@ -238,7 +219,7 @@ public function testAnnotateCommand(): void $inner = $this->createMock(AbstractHandler::class); $container = $this->createMock(ContainerInterface::class); - $handler = new AnnotatePackageHandler($inner, $this->createMock(RequestStack::class), $container); + $handler = new AnnotatePackageProcessor($this->createMock(RequestStack::class), $container); $context = [ 'exception' => $exception, @@ -252,12 +233,6 @@ public function testAnnotateCommand(): void $context ); - $context[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ - 'entrypoint' => 'command', - 'exception' => 'exception', - 'causingClass' => 'command', - ]; - $expected = new LogRecord( $record->datetime, $record->channel, @@ -265,11 +240,13 @@ public function testAnnotateCommand(): void $record->message, $context ); + $expected->extra[Package::PACKAGE_TRACE_ATTRIBUTE_KEY] = [ + 'entrypoint' => 'command', + 'exception' => 'exception', + 'causingClass' => 'command', + ]; - $inner->expects(static::once()) - ->method('handle') - ->with($expected); - $handler->handle($record); + static::assertEquals($expected, $handler($record)); } }