#6622 Logger refactoring: Turned the Exception (text) formatter into a processor + added tests

Monolog handlers support 1 formatter. Using processors instead of nested formatters leads to much simpler code.

4 files changed, 95 insertions, 12 deletions

diff --git a/config/global.php b/config/global.php
index e0353fbeb6..933c28daad 100644
--- a/config/global.php
+++ b/config/global.php
@@ -66,10 +66,12 @@ return array(
     ),
     'Piwik\Log\Handler\FileHandler' => DI\object()
         ->constructor(DI\link('log.file.filename'), DI\link('log.level'))
-        ->method('setFormatter', DI\link('Piwik\Log\Formatter\ExceptionTextFormatter')),
+        ->method('setFormatter', DI\link('Piwik\Log\Formatter\LineMessageFormatter'))
+        ->method('pushProcessor', DI\link('Piwik\Log\Processor\ExceptionToTextProcessor')),
     'Piwik\Log\Handler\DatabaseHandler' => DI\object()
         ->constructor(DI\link('log.level'))
-        ->method('setFormatter', DI\link('Piwik\Log\Formatter\ExceptionTextFormatter')),
+        ->method('setFormatter', DI\link('Piwik\Log\Formatter\LineMessageFormatter'))
+        ->method('pushProcessor', DI\link('Piwik\Log\Processor\ExceptionToTextProcessor')),
     'log.handlers.stdout' => DI\object('Monolog\Handler\StreamHandler')
         ->constructor('php://output', DI\link('log.level'))
         ->method('setFormatter', DI\link('Piwik\Log\Formatter\ExceptionHtmlFormatter')),
@@ -121,8 +123,6 @@ return array(
 
         return $logPath;
     }),
-    'Piwik\Log\Formatter\ExceptionTextFormatter' => DI\object()
-        ->constructor(DI\link('Piwik\Log\Formatter\LineMessageFormatter')),
     'Piwik\Log\Formatter\ExceptionHtmlFormatter' => DI\object()
         ->constructor(DI\link('Piwik\Log\Formatter\LineMessageFormatter')),
     'Piwik\Log\Formatter\LineMessageFormatter' => DI\object()
diff --git a/core/Log/Formatter/Formatter.php b/core/Log/Formatter/Formatter.php
index a6f602ebbd..6f5ac67e64 100644
--- a/core/Log/Formatter/Formatter.php
+++ b/core/Log/Formatter/Formatter.php
@@ -13,8 +13,7 @@ use Monolog\Formatter\FormatterInterface;
 /**
  * Formats a log message.
  *
- * Follows the Chain of responsibility design pattern, so don't forget to call `$this->next(...)`
- * at the end of the `format()` method.
+ * Follows the Chain of responsibility design pattern.
  */
 abstract class Formatter implements FormatterInterface
 {
diff --git a/core/Log/Formatter/ExceptionTextFormatter.php b/core/Log/Processor/ExceptionToTextProcessor.php
index e71b5f982a..ea3e3aec45 100644
--- a/core/Log/Formatter/ExceptionTextFormatter.php
+++ b/core/Log/Processor/ExceptionToTextProcessor.php
@@ -6,20 +6,20 @@
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
  */
 
-namespace Piwik\Log\Formatter;
+namespace Piwik\Log\Processor;
 
 use Piwik\Error;
 use Piwik\Log;
 
 /**
- * Formats a log message containing an exception object into a textual string.
+ * Process a log record containing an exception to generate a textual message.
  */
-class ExceptionTextFormatter extends Formatter
+class ExceptionToTextProcessor
 {
-    public function format(array $record)
+    public function __invoke(array $record)
     {
         if (! $this->contextContainsException($record)) {
-            return $this->next($record);
+            return $record;
         }
 
         /** @var \Exception $exception */
@@ -33,7 +33,7 @@ class ExceptionTextFormatter extends Formatter
             $this->getStackTrace($exception)
         );
 
-        return $this->next($record);
+        return $record;
     }
 
     private function contextContainsException($record)
diff --git a/tests/PHPUnit/Unit/Log/Processor/ExceptionToTextProcessorTest.php b/tests/PHPUnit/Unit/Log/Processor/ExceptionToTextProcessorTest.php
new file mode 100644
index 0000000000..398229d98d
--- /dev/null
+++ b/tests/PHPUnit/Unit/Log/Processor/ExceptionToTextProcessorTest.php
@@ -0,0 +1,84 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Tests\Unit\Log\Processor;
+
+use Piwik\Log;
+use Piwik\Log\Processor\ExceptionToTextProcessor;
+
+/**
+ * @covers \Piwik\Log\Processor\ExceptionToTextProcessor
+ */
+class ExceptionToTextProcessorTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @test
+     */
+    public function it_should_skip_if_no_exception()
+    {
+        $processor = new ExceptionToTextProcessor();
+
+        $record = array('message' => 'Hello world');
+
+        $this->assertEquals($record, $processor($record));
+    }
+
+    /**
+     * @test
+     */
+    public function it_should_replace_message_with_formatted_exception()
+    {
+        $processor = new ExceptionToTextProcessor();
+        Log::$debugBacktraceForTests = '[stack trace]';
+
+        $exception = new \Exception('Hello world');
+        $record = array(
+            'context' => array(
+                'exception' => $exception,
+            ),
+        );
+
+        $result = $processor($record);
+
+        $expected = array(
+            'message' => __FILE__ . "(39): Hello world\n[stack trace]",
+            'context' => array(
+                'exception' => $exception,
+            ),
+        );
+
+        $this->assertEquals($expected, $result);
+    }
+
+    /**
+     * @test
+     */
+    public function it_should_add_severity_for_errors()
+    {
+        $processor = new ExceptionToTextProcessor();
+        Log::$debugBacktraceForTests = '[stack trace]';
+
+        $exception = new \ErrorException('Hello world', 0, 1, 'file.php', 123);
+        $record = array(
+            'context' => array(
+                'exception' => $exception,
+            ),
+        );
+
+        $result = $processor($record);
+
+        $expected = array(
+            'message' => "file.php(123): Error - Hello world\n[stack trace]",
+            'context' => array(
+                'exception' => $exception,
+            ),
+        );
+
+        $this->assertEquals($expected, $result);
+    }
+}