Split interfaces and base/abstract classes into more sensible pieces

UPGRADE.md

@@ -6,3 +6,12 @@
 
 - Removed non-PSR-3 methods to add records, all the `add*` (e.g. `addWarning`)
   methods as well as `emerg`, `crit`, `err` and `warn`.
+
+- `HandlerInterface` has been split off and two new interfaces now exist for
+  more granular controls: `ProcessableHandlerInterface` and
+  `FormattableHandlerInterface`. Handlers not extending `AbstractHandler`
+  should make sure to implement the relevant interfaces.
+
+- `HandlerInterface` now requires the `close` method to be implemented. This
+  only impacts you if you implement the interface yourself, but you can extend
+  the new `Monolog\Handler\Handler` base class.

src/Monolog/Handler/AbstractHandler.php

@@ -12,25 +12,17 @@
 namespace Monolog\Handler;
 
 use Monolog\Logger;
-use Monolog\Formatter\FormatterInterface;
-use Monolog\Formatter\LineFormatter;
 
 /**
- * Base Handler class providing the Handler structure
+ * Base Handler class providing basic level/bubble support
  *
  * @author Jordi Boggiano <j.boggiano@seld.be>
  */
-abstract class AbstractHandler implements HandlerInterface
+abstract class AbstractHandler extends Handler
 {
     protected $level = Logger::DEBUG;
     protected $bubble = true;
 
-    /**
-     * @var FormatterInterface
-     */
-    protected $formatter;
-    protected $processors = array();
-
     /**
      * @param int     $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
@@ -49,72 +41,6 @@ abstract class AbstractHandler implements HandlerInterface
         return $record['level'] >= $this->level;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function handleBatch(array $records)
-    {
-        foreach ($records as $record) {
-            $this->handle($record);
-        }
-    }
-
-    /**
-     * Closes the handler.
-     *
-     * This will be called automatically when the object is destroyed
-     */
-    public function close()
-    {
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function pushProcessor($callback)
-    {
-        if (!is_callable($callback)) {
-            throw new \InvalidArgumentException('Processors must be valid callables (callback or object with an __invoke method), '.var_export($callback, true).' given');
-        }
-        array_unshift($this->processors, $callback);
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function popProcessor()
-    {
-        if (!$this->processors) {
-            throw new \LogicException('You tried to pop from an empty processor stack.');
-        }
-
-        return array_shift($this->processors);
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function setFormatter(FormatterInterface $formatter)
-    {
-        $this->formatter = $formatter;
-
-        return $this;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function getFormatter()
-    {
-        if (!$this->formatter) {
-            $this->formatter = $this->getDefaultFormatter();
-        }
-
-        return $this->formatter;
-    }
-
     /**
      * Sets minimum logging level at which this handler will be triggered.
      *
@@ -162,23 +88,4 @@ abstract class AbstractHandler implements HandlerInterface
     {
         return $this->bubble;
     }
-
-    public function __destruct()
-    {
-        try {
-            $this->close();
-        } catch (\Exception $e) {
-            // do nothing
-        }
-    }
-
-    /**
-     * Gets the default formatter.
-     *
-     * @return FormatterInterface
-     */
-    protected function getDefaultFormatter()
-    {
-        return new LineFormatter();
-    }
 }

src/Monolog/Handler/AbstractProcessingHandler.php

@@ -19,8 +19,11 @@ namespace Monolog\Handler;
  * @author Jordi Boggiano <j.boggiano@seld.be>
  * @author Christophe Coevoet <stof@notk.org>
  */
-abstract class AbstractProcessingHandler extends AbstractHandler
+abstract class AbstractProcessingHandler extends AbstractHandler implements ProcessableHandlerInterface, FormattableHandlerInterface
 {
+    use ProcessableHandlerTrait;
+    use FormattableHandlerTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -30,7 +33,9 @@ abstract class AbstractProcessingHandler extends AbstractHandler
             return false;
         }
 
-        $record = $this->processRecord($record);
+        if ($this->processors) {
+            $record = $this->processRecord($record);
+        }
 
         $record['formatted'] = $this->getFormatter()->format($record);
 
@@ -46,21 +51,4 @@ abstract class AbstractProcessingHandler extends AbstractHandler
      * @return void
      */
     abstract protected function write(array $record);
-
-    /**
-     * Processes a record.
-     *
-     * @param  array $record
-     * @return array
-     */
-    protected function processRecord(array $record)
-    {
-        if ($this->processors) {
-            foreach ($this->processors as $processor) {
-                $record = call_user_func($processor, $record);
-            }
-        }
-
-        return $record;
-    }
 }

src/Monolog/Handler/BufferHandler.php

@@ -21,8 +21,10 @@ use Monolog\Logger;
  *
  * @author Christophe Coevoet <stof@notk.org>
  */
-class BufferHandler extends AbstractHandler
+class BufferHandler extends AbstractHandler implements ProcessableHandlerInterface
 {
+    use ProcessableHandlerTrait;
+
     protected $handler;
     protected $bufferSize = 0;
     protected $bufferLimit;
@@ -70,9 +72,7 @@ class BufferHandler extends AbstractHandler
         }
 
         if ($this->processors) {
-            foreach ($this->processors as $processor) {
+            $record = $this->processRecord($record);
-                $record = call_user_func($processor, $record);
-            }
         }
 
         $this->buffer[] = $record;

src/Monolog/Handler/FilterHandler.php

@@ -21,8 +21,10 @@ use Monolog\Logger;
  * @author Hennadiy Verkh
  * @author Jordi Boggiano <j.boggiano@seld.be>
  */
-class FilterHandler extends AbstractHandler
+class FilterHandler extends Handler implements ProcessableHandlerInterface
 {
+    use ProcessableHandlerTrait;
+
     /**
      * Handler or factory callable($record, $this)
      *
@@ -113,9 +115,7 @@ class FilterHandler extends AbstractHandler
         }
 
         if ($this->processors) {
-            foreach ($this->processors as $processor) {
+            $record = $this->processRecord($record);
-                $record = call_user_func($processor, $record);
-            }
         }
 
         $this->handler->handle($record);

src/Monolog/Handler/FingersCrossedHandler.php

@@ -27,8 +27,10 @@ use Monolog\Logger;
  *
  * @author Jordi Boggiano <j.boggiano@seld.be>
  */
-class FingersCrossedHandler extends AbstractHandler
+class FingersCrossedHandler extends Handler implements ProcessableHandlerInterface
 {
+    use ProcessableHandlerTrait;
+
     protected $handler;
     protected $activationStrategy;
     protected $buffering = true;
@@ -85,9 +87,7 @@ class FingersCrossedHandler extends AbstractHandler
     public function handle(array $record)
     {
         if ($this->processors) {
-            foreach ($this->processors as $processor) {
+            $record = $this->processRecord($record);
-                $record = call_user_func($processor, $record);
-            }
         }
 
         if ($this->buffering) {

src/Monolog/Handler/FormattableHandlerInterface.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\Formatter\FormatterInterface;
+
+/**
+ * Interface to describe loggers that have a formatter
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+interface FormattableHandlerInterface
+{
+    /**
+     * Sets the formatter.
+     *
+     * @param  FormatterInterface $formatter
+     * @return self
+     */
+    public function setFormatter(FormatterInterface $formatter);
+
+    /**
+     * Gets the formatter.
+     *
+     * @return FormatterInterface
+     */
+    public function getFormatter();
+}

src/Monolog/Handler/FormattableHandlerTrait.php

@@ -0,0 +1,61 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\Logger;
+use Monolog\Formatter\FormatterInterface;
+use Monolog\Formatter\LineFormatter;
+
+/**
+ * Helper trait for implementing FormattableInterface
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+trait FormattableHandlerTrait
+{
+    /**
+     * @var FormatterInterface
+     */
+    protected $formatter;
+
+    /**
+     * {@inheritdoc}
+     */
+    public function setFormatter(FormatterInterface $formatter)
+    {
+        $this->formatter = $formatter;
+
+        return $this;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getFormatter()
+    {
+        if (!$this->formatter) {
+            $this->formatter = $this->getDefaultFormatter();
+        }
+
+        return $this->formatter;
+    }
+
+    /**
+     * Gets the default formatter.
+     *
+     * @return FormatterInterface
+     */
+    protected function getDefaultFormatter()
+    {
+        return new LineFormatter();
+    }
+}

src/Monolog/Handler/GroupHandler.php

@@ -16,8 +16,10 @@ namespace Monolog\Handler;
  *
  * @author Lenar Lõhmus <lenar@city.ee>
  */
-class GroupHandler extends AbstractHandler
+class GroupHandler extends Handler implements ProcessableHandlerInterface
 {
+    use ProcessableHandlerTrait;
+
     protected $handlers;
 
     /**
@@ -56,9 +58,7 @@ class GroupHandler extends AbstractHandler
     public function handle(array $record)
     {
         if ($this->processors) {
-            foreach ($this->processors as $processor) {
+            $record = $this->processRecord($record);
-                $record = call_user_func($processor, $record);
-            }
         }
 
         foreach ($this->handlers as $handler) {

src/Monolog/Handler/Handler.php

@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\Logger;
+
+/**
+ * Base Handler class providing basic close() support as well as handleBatch
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+abstract class Handler implements HandlerInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function handleBatch(array $records)
+    {
+        foreach ($records as $record) {
+            $this->handle($record);
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function close()
+    {
+    }
+
+    public function __destruct()
+    {
+        try {
+            $this->close();
+        } catch (\Exception $e) {
+            // do nothing
+        }
+    }
+}

src/Monolog/Handler/HandlerInterface.php

@@ -11,8 +11,6 @@
 
 namespace Monolog\Handler;
 
-use Monolog\Formatter\FormatterInterface;
-
 /**
  * Interface that all Monolog Handlers must implement
  *
@@ -59,32 +57,12 @@ interface HandlerInterface
     public function handleBatch(array $records);
 
     /**
-     * Adds a processor in the stack.
+     * Closes the handler.
      *
-     * @param  callable $callback
+     * This will be called automatically when the object is destroyed if you extend Monolog\Handler\Handler
-     * @return self
-     */
-    public function pushProcessor($callback);
-
-    /**
-     * Removes the processor on top of the stack and returns it.
      *
-     * @return callable
+     * Implementations have to be indempotent (i.e. it should be possible to call close several times without breakage)
+     * and ideally handlers should be able to reopen themselves on handle() after they have been closed.
      */
-    public function popProcessor();
+    public function close();
-
-    /**
-     * Sets the formatter.
-     *
-     * @param  FormatterInterface $formatter
-     * @return self
-     */
-    public function setFormatter(FormatterInterface $formatter);
-
-    /**
-     * Gets the formatter.
-     *
-     * @return FormatterInterface
-     */
-    public function getFormatter();
 }

src/Monolog/Handler/NullHandler.php

@@ -21,14 +21,24 @@ use Monolog\Logger;
  *
  * @author Jordi Boggiano <j.boggiano@seld.be>
  */
-class NullHandler extends AbstractHandler
+class NullHandler extends Handler
 {
+    private $level;
+
     /**
      * @param int $level The minimum logging level at which this handler will be triggered
      */
-    public function __construct($level = Logger::DEBUG)
+    public function __construct(int $level = Logger::DEBUG)
     {
-        parent::__construct($level, false);
+        $this->level = $level;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isHandling(array $record)
+    {
+        return $record['level'] >= $this->level;
     }
 
     /**

src/Monolog/Handler/ProcessableHandlerInterface.php

@@ -0,0 +1,35 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+/**
+ * Interface to describe loggers that have processors
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+interface ProcessableHandlerInterface
+{
+    /**
+     * Adds a processor in the stack.
+     *
+     * @param  callable $callback
+     * @return self
+     */
+    public function pushProcessor(callable $callback);
+
+    /**
+     * Removes the processor on top of the stack and returns it.
+     *
+     * @return callable
+     */
+    public function popProcessor();
+}

src/Monolog/Handler/ProcessableHandlerTrait.php

@@ -0,0 +1,67 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\Logger;
+
+/**
+ * Helper trait for implementing ProcessableInterface
+ *
+ * @author Jordi Boggiano <j.boggiano@seld.be>
+ */
+trait ProcessableHandlerTrait
+{
+    /**
+     * @var callable[]
+     */
+    protected $processors = [];
+
+    /**
+     * {@inheritdoc}
+     */
+    public function pushProcessor(callable $callback)
+    {
+        if (!is_callable($callback)) {
+            throw new \InvalidArgumentException('Processors must be valid callables (callback or object with an __invoke method), '.var_export($callback, true).' given');
+        }
+        array_unshift($this->processors, $callback);
+
+        return $this;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function popProcessor()
+    {
+        if (!$this->processors) {
+            throw new \LogicException('You tried to pop from an empty processor stack.');
+        }
+
+        return array_shift($this->processors);
+    }
+
+    /**
+     * Processes a record.
+     *
+     * @param  array $record
+     * @return array
+     */
+    protected function processRecord(array $record)
+    {
+        foreach ($this->processors as $processor) {
+            $record = $processor($record);
+        }
+
+        return $record;
+    }
+}

src/Monolog/Handler/SamplingHandler.php

@@ -25,8 +25,10 @@ namespace Monolog\Handler;
  * @author Bryan Davis <bd808@wikimedia.org>
  * @author Kunal Mehta <legoktm@gmail.com>
  */
-class SamplingHandler extends AbstractHandler
+class SamplingHandler extends AbstractHandler implements ProcessableHandlerInterface
 {
+    use ProcessableHandlerTrait;
+
     /**
      * @var callable|HandlerInterface $handler
      */
@@ -69,9 +71,7 @@ class SamplingHandler extends AbstractHandler
             }
 
             if ($this->processors) {
-                foreach ($this->processors as $processor) {
+                $record = $this->processRecord($record);
-                    $record = call_user_func($processor, $record);
-                }
             }
 
             $this->handler->handle($record);

tests/Monolog/Handler/AbstractHandlerTest.php

@@ -13,7 +13,6 @@ namespace Monolog\Handler;
 
 use Monolog\TestCase;
 use Monolog\Logger;
-use Monolog\Formatter\LineFormatter;
 use Monolog\Processor\WebProcessor;
 
 class AbstractHandlerTest extends TestCase
@@ -24,8 +23,6 @@ class AbstractHandlerTest extends TestCase
      * @covers Monolog\Handler\AbstractHandler::setLevel
      * @covers Monolog\Handler\AbstractHandler::getBubble
      * @covers Monolog\Handler\AbstractHandler::setBubble
-     * @covers Monolog\Handler\AbstractHandler::getFormatter
-     * @covers Monolog\Handler\AbstractHandler::setFormatter
      */
     public function testConstructAndGetSet()
     {
@@ -35,10 +32,8 @@ class AbstractHandlerTest extends TestCase
 
         $handler->setLevel(Logger::ERROR);
         $handler->setBubble(true);
-        $handler->setFormatter($formatter = new LineFormatter);
         $this->assertEquals(Logger::ERROR, $handler->getLevel());
         $this->assertEquals(true, $handler->getBubble());
-        $this->assertSame($formatter, $handler->getFormatter());
     }
 
     /**
@@ -72,44 +67,4 @@ class AbstractHandlerTest extends TestCase
         $handler->setLevel('debug');
         $this->assertTrue($handler->isHandling($this->getRecord(Logger::DEBUG)));
     }
-
-    /**
-     * @covers Monolog\Handler\AbstractHandler::getFormatter
-     * @covers Monolog\Handler\AbstractHandler::getDefaultFormatter
-     */
-    public function testGetFormatterInitializesDefault()
-    {
-        $handler = $this->getMockForAbstractClass('Monolog\Handler\AbstractHandler');
-        $this->assertInstanceOf('Monolog\Formatter\LineFormatter', $handler->getFormatter());
-    }
-
-    /**
-     * @covers Monolog\Handler\AbstractHandler::pushProcessor
-     * @covers Monolog\Handler\AbstractHandler::popProcessor
-     * @expectedException LogicException
-     */
-    public function testPushPopProcessor()
-    {
-        $logger = $this->getMockForAbstractClass('Monolog\Handler\AbstractHandler');
-        $processor1 = new WebProcessor;
-        $processor2 = new WebProcessor;
-
-        $logger->pushProcessor($processor1);
-        $logger->pushProcessor($processor2);
-
-        $this->assertEquals($processor2, $logger->popProcessor());
-        $this->assertEquals($processor1, $logger->popProcessor());
-        $logger->popProcessor();
-    }
-
-    /**
-     * @covers Monolog\Handler\AbstractHandler::pushProcessor
-     * @expectedException InvalidArgumentException
-     */
-    public function testPushProcessorWithNonCallable()
-    {
-        $handler = $this->getMockForAbstractClass('Monolog\Handler\AbstractHandler');
-
-        $handler->pushProcessor(new \stdClass());
-    }
 }

tests/Monolog/Handler/AbstractProcessingHandlerTest.php

@@ -14,9 +14,21 @@ namespace Monolog\Handler;
 use Monolog\TestCase;
 use Monolog\Logger;
 use Monolog\Processor\WebProcessor;
+use Monolog\Formatter\LineFormatter;
 
 class AbstractProcessingHandlerTest extends TestCase
 {
+    /**
+     * @covers Monolog\Handler\FormattableHandlerTrait::getFormatter
+     * @covers Monolog\Handler\FormattableHandlerTrait::setFormatter
+     */
+    public function testConstructAndGetSet()
+    {
+        $handler = $this->getMockForAbstractClass('Monolog\Handler\AbstractProcessingHandler', array(Logger::WARNING, false));
+        $handler->setFormatter($formatter = new LineFormatter);
+        $this->assertSame($formatter, $handler->getFormatter());
+    }
+
     /**
      * @covers Monolog\Handler\AbstractProcessingHandler::handle
      */
@@ -77,4 +89,44 @@ class AbstractProcessingHandlerTest extends TestCase
         $handler->handle($this->getRecord());
         $this->assertEquals(6, count($handledRecord['extra']));
     }
+
+    /**
+     * @covers Monolog\Handler\ProcessableHandlerTrait::pushProcessor
+     * @covers Monolog\Handler\ProcessableHandlerTrait::popProcessor
+     * @expectedException LogicException
+     */
+    public function testPushPopProcessor()
+    {
+        $logger = $this->getMockForAbstractClass('Monolog\Handler\AbstractProcessingHandler');
+        $processor1 = new WebProcessor;
+        $processor2 = new WebProcessor;
+
+        $logger->pushProcessor($processor1);
+        $logger->pushProcessor($processor2);
+
+        $this->assertEquals($processor2, $logger->popProcessor());
+        $this->assertEquals($processor1, $logger->popProcessor());
+        $logger->popProcessor();
+    }
+
+    /**
+     * @covers Monolog\Handler\ProcessableHandlerTrait::pushProcessor
+     * @expectedException TypeError
+     */
+    public function testPushProcessorWithNonCallable()
+    {
+        $handler = $this->getMockForAbstractClass('Monolog\Handler\AbstractProcessingHandler');
+
+        $handler->pushProcessor(new \stdClass());
+    }
+
+    /**
+     * @covers Monolog\Handler\FormattableHandlerTrait::getFormatter
+     * @covers Monolog\Handler\FormattableHandlerTrait::getDefaultFormatter
+     */
+    public function testGetFormatterInitializesDefault()
+    {
+        $handler = $this->getMockForAbstractClass('Monolog\Handler\AbstractProcessingHandler');
+        $this->assertInstanceOf(LineFormatter::class, $handler->getFormatter());
+    }
 }