From 0625068bf0a6667d18cca284eda1ba40907d2538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 31 May 2017 17:19:21 +0200 Subject: [PATCH] Added a new ResettableInterface and implemented it where possible. When one use Monolog in a long process like an AMQP worker with a `FingersCrossedHandler` or `BufferHandler` there is a drawback: as soon as there is an AMQP message that generate a log >= error (for example), all next AMQP messages will output logs, even if theses messages don't generate log where level >= error. In the same context there is a drawback for processor that add an UUID to the logs. The UUID should change for each AMQP messages. --- This patch address this issue with a new interface: `ResettableInterface` interface. Side note: `reset()`, `flush()`, `clear()`, are already used in Monolog. So basically, one can use the `reset()` on the `Logger` and on some `Handler`s / `Processor`s. It's especially useful for * the `FingersCrossedHandler`: it `close()` the buffer, then it `clear()` the buffer. * the `BufferHandler`: it `flush()` the buffer, then it `clear()` the buffer. * the `UidProcessor`: it renew the `uid`. --- CHANGELOG.md | 2 + src/Monolog/Handler/AbstractHandler.php | 16 +++- .../Handler/AbstractProcessingHandler.php | 2 + src/Monolog/Handler/BrowserConsoleHandler.php | 9 ++- src/Monolog/Handler/BufferHandler.php | 10 +++ src/Monolog/Handler/FingersCrossedHandler.php | 8 ++ src/Monolog/Handler/GroupHandler.php | 12 +++ src/Monolog/Handler/HandlerWrapper.php | 10 ++- src/Monolog/Logger.php | 17 ++++- src/Monolog/Processor/UidProcessor.php | 17 ++++- src/Monolog/ResettableInterface.php | 25 +++++++ .../Handler/BrowserConsoleHandlerTest.php | 2 +- .../Monolog/Handler/ChromePHPHandlerTest.php | 4 +- .../Handler/FingersCrossedHandlerTest.php | 4 +- tests/Monolog/Handler/FirePHPHandlerTest.php | 4 +- tests/Monolog/LoggerTest.php | 73 +++++++++++++++++++ 16 files changed, 200 insertions(+), 15 deletions(-) create mode 100644 src/Monolog/ResettableInterface.php diff --git a/CHANGELOG.md b/CHANGELOG.md index a69362b68..75ac442b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ * Fixed table row styling issues in HtmlFormatter * Fixed RavenHandler dropping the message when logging exception * Fixed WhatFailureGroupHandler skipping processors when using handleBatch + * Added a `ResettableInterface` in order to reset/reset/clear/flush handlers and processors + and implement it where possible ### 1.23.0 (2017-06-19) diff --git a/src/Monolog/Handler/AbstractHandler.php b/src/Monolog/Handler/AbstractHandler.php index bf56549d5..b286a01c0 100644 --- a/src/Monolog/Handler/AbstractHandler.php +++ b/src/Monolog/Handler/AbstractHandler.php @@ -11,16 +11,17 @@ namespace Monolog\Handler; -use Monolog\Logger; use Monolog\Formatter\FormatterInterface; use Monolog\Formatter\LineFormatter; +use Monolog\Logger; +use Monolog\ResettableInterface; /** * Base Handler class providing the Handler structure * * @author Jordi Boggiano */ -abstract class AbstractHandler implements HandlerInterface +abstract class AbstractHandler implements HandlerInterface, ResettableInterface { protected $level = Logger::DEBUG; protected $bubble = true; @@ -174,6 +175,17 @@ public function __destruct() } } + public function reset() + { + $this->close(); + + foreach ($this->processors as $processor) { + if ($processor instanceof ResettableInterface) { + $processor->reset(); + } + } + } + /** * Gets the default formatter. * diff --git a/src/Monolog/Handler/AbstractProcessingHandler.php b/src/Monolog/Handler/AbstractProcessingHandler.php index 6f18f72e1..e1e89530a 100644 --- a/src/Monolog/Handler/AbstractProcessingHandler.php +++ b/src/Monolog/Handler/AbstractProcessingHandler.php @@ -11,6 +11,8 @@ namespace Monolog\Handler; +use Monolog\ResettableInterface; + /** * Base Handler class providing the Handler structure * diff --git a/src/Monolog/Handler/BrowserConsoleHandler.php b/src/Monolog/Handler/BrowserConsoleHandler.php index 0225ee714..14fa0dc70 100644 --- a/src/Monolog/Handler/BrowserConsoleHandler.php +++ b/src/Monolog/Handler/BrowserConsoleHandler.php @@ -69,14 +69,19 @@ public static function send() } elseif ($format === 'js') { static::writeOutput(static::generateScript()); } - static::reset(); + static::resetStatic(); } } + public function reset() + { + self::resetStatic(); + } + /** * Forget all logged records */ - public static function reset() + public static function resetStatic() { static::$records = array(); } diff --git a/src/Monolog/Handler/BufferHandler.php b/src/Monolog/Handler/BufferHandler.php index c15e28a24..ca2c7a799 100644 --- a/src/Monolog/Handler/BufferHandler.php +++ b/src/Monolog/Handler/BufferHandler.php @@ -12,6 +12,7 @@ namespace Monolog\Handler; use Monolog\Logger; +use Monolog\ResettableInterface; /** * Buffers all records until closing the handler and then pass them as batch. @@ -114,4 +115,13 @@ public function clear() $this->bufferSize = 0; $this->buffer = array(); } + + public function reset() + { + parent::reset(); + + if ($this->handler instanceof ResettableInterface) { + $this->handler->reset(); + } + } } diff --git a/src/Monolog/Handler/FingersCrossedHandler.php b/src/Monolog/Handler/FingersCrossedHandler.php index d8026be6f..a2d85dde6 100644 --- a/src/Monolog/Handler/FingersCrossedHandler.php +++ b/src/Monolog/Handler/FingersCrossedHandler.php @@ -14,6 +14,7 @@ use Monolog\Handler\FingersCrossed\ErrorLevelActivationStrategy; use Monolog\Handler\FingersCrossed\ActivationStrategyInterface; use Monolog\Logger; +use Monolog\ResettableInterface; /** * Buffers all records until a certain level is reached @@ -147,7 +148,14 @@ public function close() */ public function reset() { + parent::reset(); + + $this->buffer = array(); $this->buffering = true; + + if ($this->handler instanceof ResettableInterface) { + $this->handler->reset(); + } } /** diff --git a/src/Monolog/Handler/GroupHandler.php b/src/Monolog/Handler/GroupHandler.php index c38508c25..28e5c5648 100644 --- a/src/Monolog/Handler/GroupHandler.php +++ b/src/Monolog/Handler/GroupHandler.php @@ -12,6 +12,7 @@ namespace Monolog\Handler; use Monolog\Formatter\FormatterInterface; +use Monolog\ResettableInterface; /** * Forwards records to multiple handlers @@ -90,6 +91,17 @@ public function handleBatch(array $records) } } + public function reset() + { + parent::reset(); + + foreach ($this->handlers as $handler) { + if ($handler instanceof ResettableInterface) { + $handler->reset(); + } + } + } + /** * {@inheritdoc} */ diff --git a/src/Monolog/Handler/HandlerWrapper.php b/src/Monolog/Handler/HandlerWrapper.php index e540d80f4..55e649868 100644 --- a/src/Monolog/Handler/HandlerWrapper.php +++ b/src/Monolog/Handler/HandlerWrapper.php @@ -11,6 +11,7 @@ namespace Monolog\Handler; +use Monolog\ResettableInterface; use Monolog\Formatter\FormatterInterface; /** @@ -30,7 +31,7 @@ * * @author Alexey Karapetov */ -class HandlerWrapper implements HandlerInterface +class HandlerWrapper implements HandlerInterface, ResettableInterface { /** * @var HandlerInterface @@ -105,4 +106,11 @@ public function getFormatter() { return $this->handler->getFormatter(); } + + public function reset() + { + if ($this->handler instanceof ResettableInterface) { + return $this->handler->reset(); + } + } } diff --git a/src/Monolog/Logger.php b/src/Monolog/Logger.php index 5034ead19..d998bb655 100644 --- a/src/Monolog/Logger.php +++ b/src/Monolog/Logger.php @@ -25,7 +25,7 @@ * * @author Jordi Boggiano */ -class Logger implements LoggerInterface +class Logger implements LoggerInterface, ResettableInterface { /** * Detailed debug information @@ -354,6 +354,21 @@ public function addRecord($level, $message, array $context = array()) return true; } + public function reset() + { + foreach ($this->handlers as $handler) { + if ($handler instanceof ResettableInterface) { + $handler->reset(); + } + } + + foreach ($this->processors as $processor) { + if ($processor instanceof ResettableInterface) { + $processor->reset(); + } + } + } + /** * Adds a log record at the DEBUG level. * diff --git a/src/Monolog/Processor/UidProcessor.php b/src/Monolog/Processor/UidProcessor.php index 812707cdb..47f13f007 100644 --- a/src/Monolog/Processor/UidProcessor.php +++ b/src/Monolog/Processor/UidProcessor.php @@ -11,12 +11,14 @@ namespace Monolog\Processor; +use Monolog\ResettableInterface; + /** * Adds a unique identifier into records * * @author Simon Mönch */ -class UidProcessor +class UidProcessor implements ResettableInterface { private $uid; @@ -26,7 +28,8 @@ public function __construct($length = 7) throw new \InvalidArgumentException('The uid length must be an integer between 1 and 32'); } - $this->uid = substr(hash('md5', uniqid('', true)), 0, $length); + + $this->uid = $this->generateUid($length); } public function __invoke(array $record) @@ -43,4 +46,14 @@ public function getUid() { return $this->uid; } + + public function reset() + { + $this->uid = $this->generateUid(strlen($this->uid)); + } + + private function generateUid($length) + { + return substr(hash('md5', uniqid('', true)), 0, $length); + } } diff --git a/src/Monolog/ResettableInterface.php b/src/Monolog/ResettableInterface.php new file mode 100644 index 000000000..5e7bd6f3a --- /dev/null +++ b/src/Monolog/ResettableInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Monolog; + +/** + * Handler or Processor implementing this interface will be reset when Logger::reset() is called. + * + * Resetting an Handler or a Processor usually means cleaning all buffers or + * resetting in its internal state. + * + * @author Grégoire Pineau + */ +interface ResettableInterface +{ + public function reset(); +} diff --git a/tests/Monolog/Handler/BrowserConsoleHandlerTest.php b/tests/Monolog/Handler/BrowserConsoleHandlerTest.php index ffb1d746a..ffe45da2f 100644 --- a/tests/Monolog/Handler/BrowserConsoleHandlerTest.php +++ b/tests/Monolog/Handler/BrowserConsoleHandlerTest.php @@ -21,7 +21,7 @@ class BrowserConsoleHandlerTest extends TestCase { protected function setUp() { - BrowserConsoleHandler::reset(); + BrowserConsoleHandler::resetStatic(); } protected function generateScript() diff --git a/tests/Monolog/Handler/ChromePHPHandlerTest.php b/tests/Monolog/Handler/ChromePHPHandlerTest.php index 0449f8b1a..421cc4918 100644 --- a/tests/Monolog/Handler/ChromePHPHandlerTest.php +++ b/tests/Monolog/Handler/ChromePHPHandlerTest.php @@ -21,7 +21,7 @@ class ChromePHPHandlerTest extends TestCase { protected function setUp() { - TestChromePHPHandler::reset(); + TestChromePHPHandler::resetStatic(); $_SERVER['HTTP_USER_AGENT'] = 'Monolog Test; Chrome/1.0'; } @@ -136,7 +136,7 @@ class TestChromePHPHandler extends ChromePHPHandler { protected $headers = array(); - public static function reset() + public static function resetStatic() { self::$initialized = false; self::$overflowed = false; diff --git a/tests/Monolog/Handler/FingersCrossedHandlerTest.php b/tests/Monolog/Handler/FingersCrossedHandlerTest.php index b92bf437b..0ec36531a 100644 --- a/tests/Monolog/Handler/FingersCrossedHandlerTest.php +++ b/tests/Monolog/Handler/FingersCrossedHandlerTest.php @@ -58,7 +58,7 @@ public function testHandleStopsBufferingAfterTrigger() * @covers Monolog\Handler\FingersCrossedHandler::activate * @covers Monolog\Handler\FingersCrossedHandler::reset */ - public function testHandleRestartBufferingAfterReset() + public function testHandleResetBufferingAfterReset() { $test = new TestHandler(); $handler = new FingersCrossedHandler($test); @@ -76,7 +76,7 @@ public function testHandleRestartBufferingAfterReset() * @covers Monolog\Handler\FingersCrossedHandler::handle * @covers Monolog\Handler\FingersCrossedHandler::activate */ - public function testHandleRestartBufferingAfterBeingTriggeredWhenStopBufferingIsDisabled() + public function testHandleResetBufferingAfterBeingTriggeredWhenStopBufferingIsDisabled() { $test = new TestHandler(); $handler = new FingersCrossedHandler($test, Logger::WARNING, 0, false, false); diff --git a/tests/Monolog/Handler/FirePHPHandlerTest.php b/tests/Monolog/Handler/FirePHPHandlerTest.php index 0eb10a63f..7a404e660 100644 --- a/tests/Monolog/Handler/FirePHPHandlerTest.php +++ b/tests/Monolog/Handler/FirePHPHandlerTest.php @@ -21,7 +21,7 @@ class FirePHPHandlerTest extends TestCase { public function setUp() { - TestFirePHPHandler::reset(); + TestFirePHPHandler::resetStatic(); $_SERVER['HTTP_USER_AGENT'] = 'Monolog Test; FirePHP/1.0'; } @@ -77,7 +77,7 @@ class TestFirePHPHandler extends FirePHPHandler { protected $headers = array(); - public static function reset() + public static function resetStatic() { self::$initialized = false; self::$sendHeaders = true; diff --git a/tests/Monolog/LoggerTest.php b/tests/Monolog/LoggerTest.php index f938ea028..442e87dea 100644 --- a/tests/Monolog/LoggerTest.php +++ b/tests/Monolog/LoggerTest.php @@ -614,4 +614,77 @@ public function testCustomHandleException() $logger->pushHandler($handler); $logger->info('test'); } + + public function testReset() + { + $logger = new Logger('app'); + + $testHandler = new Handler\TestHandler(); + $bufferHandler = new Handler\BufferHandler($testHandler); + $groupHandler = new Handler\GroupHandler(array($bufferHandler)); + $fingersCrossedHandler = new Handler\FingersCrossedHandler($groupHandler); + + $logger->pushHandler($fingersCrossedHandler); + + $processorUid1 = new Processor\UidProcessor(10); + $uid1 = $processorUid1->getUid(); + $groupHandler->pushProcessor($processorUid1); + + $processorUid2 = new Processor\UidProcessor(5); + $uid2 = $processorUid2->getUid(); + $logger->pushProcessor($processorUid2); + + $getProperty = function ($object, $property) { + $reflectionProperty = new \ReflectionProperty(get_class($object), $property); + $reflectionProperty->setAccessible(true); + + return $reflectionProperty->getValue($object); + }; + $that = $this; + $assertBufferOfBufferHandlerEmpty = function () use ($getProperty, $bufferHandler, $that) { + $that->assertEmpty($getProperty($bufferHandler, 'buffer')); + }; + $assertBuffersEmpty = function() use ($assertBufferOfBufferHandlerEmpty, $getProperty, $fingersCrossedHandler, $that) { + $assertBufferOfBufferHandlerEmpty(); + $that->assertEmpty($getProperty($fingersCrossedHandler, 'buffer')); + }; + + $logger->debug('debug'); + $logger->reset(); + $assertBuffersEmpty(); + $this->assertFalse($testHandler->hasDebugRecords()); + $this->assertFalse($testHandler->hasErrorRecords()); + $this->assertNotSame($uid1, $uid1 = $processorUid1->getUid()); + $this->assertNotSame($uid2, $uid2 = $processorUid2->getUid()); + + $logger->debug('debug'); + $logger->error('error'); + $logger->reset(); + $assertBuffersEmpty(); + $this->assertTrue($testHandler->hasDebugRecords()); + $this->assertTrue($testHandler->hasErrorRecords()); + $this->assertNotSame($uid1, $uid1 = $processorUid1->getUid()); + $this->assertNotSame($uid2, $uid2 = $processorUid2->getUid()); + + $logger->info('info'); + $this->assertNotEmpty($getProperty($fingersCrossedHandler, 'buffer')); + $assertBufferOfBufferHandlerEmpty(); + $this->assertFalse($testHandler->hasInfoRecords()); + + $logger->reset(); + $assertBuffersEmpty(); + $this->assertFalse($testHandler->hasInfoRecords()); + $this->assertNotSame($uid1, $uid1 = $processorUid1->getUid()); + $this->assertNotSame($uid2, $uid2 = $processorUid2->getUid()); + + $logger->notice('notice'); + $logger->emergency('emergency'); + $logger->reset(); + $assertBuffersEmpty(); + $this->assertFalse($testHandler->hasInfoRecords()); + $this->assertTrue($testHandler->hasNoticeRecords()); + $this->assertTrue($testHandler->hasEmergencyRecords()); + $this->assertNotSame($uid1, $processorUid1->getUid()); + $this->assertNotSame($uid2, $processorUid2->getUid()); + } }