From 050694291e38ca010f577a45c98c8f9e4866499e Mon Sep 17 00:00:00 2001 From: Matt Wells Date: Sun, 12 Jul 2015 17:26:16 +0100 Subject: [PATCH 1/4] Adds ability to cap log size in redis 100% backward compatible, defaults to not capping collection size. Additional constructor param of $capSize added which will ensure logs list is treated as a FILO queue Test coverage on new functionality --- src/Monolog/Handler/RedisHandler.php | 45 ++++++++++++++--- tests/Monolog/Handler/RedisHandlerTest.php | 57 ++++++++++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/Monolog/Handler/RedisHandler.php b/src/Monolog/Handler/RedisHandler.php index ee8c2363..7dcd8485 100644 --- a/src/Monolog/Handler/RedisHandler.php +++ b/src/Monolog/Handler/RedisHandler.php @@ -29,14 +29,16 @@ class RedisHandler extends AbstractProcessingHandler { private $redisClient; private $redisKey; + protected $capSize; /** - * @param \Predis\Client|\Redis $redis The redis instance - * @param string $key The key name to push records to - * @param integer $level The minimum logging level at which this handler will be triggered - * @param boolean $bubble Whether the messages that are handled can bubble up the stack or not + * @param \Predis\Client|\Redis $redis The redis instance + * @param string $key The key name to push records to + * @param integer $level The minimum logging level at which this handler will be triggered + * @param boolean $bubble Whether the messages that are handled can bubble up the stack or not + * @param integer $capSize Number of entries to limit list size to */ - public function __construct($redis, $key, $level = Logger::DEBUG, $bubble = true) + public function __construct($redis, $key, $level = Logger::DEBUG, $bubble = true, $capSize = false) { if (!(($redis instanceof \Predis\Client) || ($redis instanceof \Redis))) { throw new \InvalidArgumentException('Predis\Client or Redis instance required'); @@ -44,13 +46,44 @@ class RedisHandler extends AbstractProcessingHandler $this->redisClient = $redis; $this->redisKey = $key; + $this->capSize = $capSize; parent::__construct($level, $bubble); } + /** + * {@inheritDoc} + */ protected function write(array $record) { - $this->redisClient->rpush($this->redisKey, $record["formatted"]); + if ($this->capSize) + { + $this->writeCapped($record); + } else { + $this->redisClient->rpush($this->redisKey, $record["formatted"]); + } + } + + /** + * Write and cap the collection + * Writes the record to the redis list and caps its + * + * @param array $record associative record array + * @return void + */ + protected function writeCapped(array $record) + { + if($this->redisClient instanceof \Redis) { + $this->redisClient->multi() + ->lpush($this->redisKey, $record["formatted"]) + ->ltrim($this->redisKey, 0, $this->capSize) + ->execute(); + } else { + $this->redisClient->transaction(function($tx) use($record) { + $tx->lpush($this->redisKey, $record["formatted"]); + $tx->ltrim($this->redisKey, 0, $this->capSize); + }); + } } /** diff --git a/tests/Monolog/Handler/RedisHandlerTest.php b/tests/Monolog/Handler/RedisHandlerTest.php index 3629f8a2..5a4591c3 100644 --- a/tests/Monolog/Handler/RedisHandlerTest.php +++ b/tests/Monolog/Handler/RedisHandlerTest.php @@ -68,4 +68,61 @@ class RedisHandlerTest extends TestCase $handler->setFormatter(new LineFormatter("%message%")); $handler->handle($record); } + + public function testRedisHandleCapped() + { + $redis = $this->getMock('Redis', array('multi', 'lpush', 'ltrim', 'execute')); + + // Redis uses multi + $redis->expects($this->once()) + ->method('multi') + ->will($this->returnSelf()); + + $redis->expects($this->once()) + ->method('lpush') + ->will($this->returnSelf()); + + $redis->expects($this->once()) + ->method('ltrim') + ->will($this->returnSelf()); + + $redis->expects($this->once()) + ->method('execute') + ->will($this->returnSelf()); + + $record = $this->getRecord(Logger::WARNING, 'test', array('data' => new \stdClass, 'foo' => 34)); + + $handler = new RedisHandler($redis, 'key', Logger::DEBUG, true, 10); + $handler->setFormatter(new LineFormatter("%message%")); + $handler->handle($record); + } + + public function testPredisHandleCapped() + { + $redis = $this->getMock('Predis\Client', array('transaction')); + + // Redis uses multi + $redis->expects($this->once()) + ->method('transaction') + ->will($this->returnCallback(function($cb){ + + $redisTransaction = $this->getMock('Predis\Client', array('lpush', 'ltrim')); + + $redisTransaction->expects($this->once()) + ->method('lpush') + ->will($this->returnSelf()); + + $redisTransaction->expects($this->once()) + ->method('ltrim') + ->will($this->returnSelf()); + + $cb($redisTransaction); + })); + + $record = $this->getRecord(Logger::WARNING, 'test', array('data' => new \stdClass, 'foo' => 34)); + + $handler = new RedisHandler($redis, 'key', Logger::DEBUG, true, 10); + $handler->setFormatter(new LineFormatter("%message%")); + $handler->handle($record); + } } From 4d592818685a2b1cae95d661180ce29de5f70379 Mon Sep 17 00:00:00 2001 From: Matt Wells Date: Sun, 12 Jul 2015 19:21:53 +0100 Subject: [PATCH 2/4] Fixes 5.3 compatibility in test cases --- tests/Monolog/Handler/RedisHandlerTest.php | 23 +++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/Monolog/Handler/RedisHandlerTest.php b/tests/Monolog/Handler/RedisHandlerTest.php index 5a4591c3..088cef82 100644 --- a/tests/Monolog/Handler/RedisHandlerTest.php +++ b/tests/Monolog/Handler/RedisHandlerTest.php @@ -101,21 +101,20 @@ class RedisHandlerTest extends TestCase { $redis = $this->getMock('Predis\Client', array('transaction')); + $redisTransaction = $this->getMock('Predis\Client', array('lpush', 'ltrim')); + + $redisTransaction->expects($this->once()) + ->method('lpush') + ->will($this->returnSelf()); + + $redisTransaction->expects($this->once()) + ->method('ltrim') + ->will($this->returnSelf()); + // Redis uses multi $redis->expects($this->once()) ->method('transaction') - ->will($this->returnCallback(function($cb){ - - $redisTransaction = $this->getMock('Predis\Client', array('lpush', 'ltrim')); - - $redisTransaction->expects($this->once()) - ->method('lpush') - ->will($this->returnSelf()); - - $redisTransaction->expects($this->once()) - ->method('ltrim') - ->will($this->returnSelf()); - + ->will($this->returnCallback(function($cb) use ($redisTransaction){ $cb($redisTransaction); })); From ae2b2d0de16a968fcd72d2968dfbbff6ec89c89b Mon Sep 17 00:00:00 2001 From: Matt Wells Date: Sun, 12 Jul 2015 19:28:05 +0100 Subject: [PATCH 3/4] fixes reference for 5.3 in predis callback --- src/Monolog/Handler/RedisHandler.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Monolog/Handler/RedisHandler.php b/src/Monolog/Handler/RedisHandler.php index 7dcd8485..22ba4994 100644 --- a/src/Monolog/Handler/RedisHandler.php +++ b/src/Monolog/Handler/RedisHandler.php @@ -79,9 +79,11 @@ class RedisHandler extends AbstractProcessingHandler ->ltrim($this->redisKey, 0, $this->capSize) ->execute(); } else { - $this->redisClient->transaction(function($tx) use($record) { - $tx->lpush($this->redisKey, $record["formatted"]); - $tx->ltrim($this->redisKey, 0, $this->capSize); + $redisKey = $this->redisKey; + $capSize = $this->capSize; + $this->redisClient->transaction(function($tx) use($record, $redisKey, $capSize) { + $tx->lpush($redisKey, $record["formatted"]); + $tx->ltrim($redisKey, 0, $capSize); }); } } From 12711d133afd156e05080e5e1bedd3a281de99c4 Mon Sep 17 00:00:00 2001 From: Matt Wells Date: Mon, 13 Jul 2015 19:40:51 +0100 Subject: [PATCH 4/4] Switch back to using rpush Switches back to rpush to keep list order consistent with non capped collections --- src/Monolog/Handler/RedisHandler.php | 8 ++++---- tests/Monolog/Handler/RedisHandlerTest.php | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Monolog/Handler/RedisHandler.php b/src/Monolog/Handler/RedisHandler.php index 22ba4994..dd0e747e 100644 --- a/src/Monolog/Handler/RedisHandler.php +++ b/src/Monolog/Handler/RedisHandler.php @@ -75,15 +75,15 @@ class RedisHandler extends AbstractProcessingHandler { if($this->redisClient instanceof \Redis) { $this->redisClient->multi() - ->lpush($this->redisKey, $record["formatted"]) - ->ltrim($this->redisKey, 0, $this->capSize) + ->rpush($this->redisKey, $record["formatted"]) + ->ltrim($this->redisKey, -$this->capSize, -1) ->execute(); } else { $redisKey = $this->redisKey; $capSize = $this->capSize; $this->redisClient->transaction(function($tx) use($record, $redisKey, $capSize) { - $tx->lpush($redisKey, $record["formatted"]); - $tx->ltrim($redisKey, 0, $capSize); + $tx->rpush($redisKey, $record["formatted"]); + $tx->ltrim($redisKey, -$capSize, -1); }); } } diff --git a/tests/Monolog/Handler/RedisHandlerTest.php b/tests/Monolog/Handler/RedisHandlerTest.php index 088cef82..c415c722 100644 --- a/tests/Monolog/Handler/RedisHandlerTest.php +++ b/tests/Monolog/Handler/RedisHandlerTest.php @@ -71,7 +71,7 @@ class RedisHandlerTest extends TestCase public function testRedisHandleCapped() { - $redis = $this->getMock('Redis', array('multi', 'lpush', 'ltrim', 'execute')); + $redis = $this->getMock('Redis', array('multi', 'rpush', 'ltrim', 'execute')); // Redis uses multi $redis->expects($this->once()) @@ -79,7 +79,7 @@ class RedisHandlerTest extends TestCase ->will($this->returnSelf()); $redis->expects($this->once()) - ->method('lpush') + ->method('rpush') ->will($this->returnSelf()); $redis->expects($this->once()) @@ -101,10 +101,10 @@ class RedisHandlerTest extends TestCase { $redis = $this->getMock('Predis\Client', array('transaction')); - $redisTransaction = $this->getMock('Predis\Client', array('lpush', 'ltrim')); + $redisTransaction = $this->getMock('Predis\Client', array('rpush', 'ltrim')); $redisTransaction->expects($this->once()) - ->method('lpush') + ->method('rpush') ->will($this->returnSelf()); $redisTransaction->expects($this->once())