Browse Source

Split log formatter from engine into formatter.

Corey Taylor 4 years ago
parent
commit
6f0088a803

+ 20 - 0
psalm-baseline.xml

@@ -162,6 +162,26 @@
       <code>translate</code>
     </InternalMethod>
   </file>
+  <file src="src/Log/Engine/ArrayLog.php">
+    <DeprecatedMethod occurrences="1">
+      <code>_format</code>
+    </DeprecatedMethod>
+  </file>
+  <file src="src/Log/Engine/ConsoleLog.php">
+    <DeprecatedMethod occurrences="1">
+      <code>_format</code>
+    </DeprecatedMethod>
+  </file>
+  <file src="src/Log/Engine/FileLog.php">
+    <DeprecatedMethod occurrences="1">
+      <code>_format</code>
+    </DeprecatedMethod>
+  </file>
+  <file src="src/Log/Engine/SyslogLog.php">
+    <DeprecatedMethod occurrences="1">
+      <code>_format</code>
+    </DeprecatedMethod>
+  </file>
   <file src="src/ORM/Locator/LocatorAwareTrait.php">
     <DeprecatedClass occurrences="1">
       <code>$this</code>

+ 39 - 14
src/Log/Engine/BaseLog.php

@@ -18,7 +18,9 @@ namespace Cake\Log\Engine;
 
 use ArrayObject;
 use Cake\Core\InstanceConfigTrait;
-use DateTimeImmutable;
+use Cake\Log\Formatter\AbstractFormatter;
+use Cake\Log\Formatter\DefaultFormatter;
+use InvalidArgumentException;
 use JsonSerializable;
 use Psr\Log\AbstractLogger;
 use Serializable;
@@ -38,10 +40,15 @@ abstract class BaseLog extends AbstractLogger
     protected $_defaultConfig = [
         'levels' => [],
         'scopes' => [],
-        'dateFormat' => 'Y-m-d H:i:s',
+        'formatter' => DefaultFormatter::class,
     ];
 
     /**
+     * @var \Cake\Log\Formatter\AbstractFormatter
+     */
+    protected $formatter;
+
+    /**
      * __construct method
      *
      * @param array $config Configuration array
@@ -61,6 +68,23 @@ abstract class BaseLog extends AbstractLogger
         if (!empty($this->_config['types']) && empty($this->_config['levels'])) {
             $this->_config['levels'] = (array)$this->_config['types'];
         }
+
+        // Default to DefaultFormatter for BC with custom loggers that don't set `formatter` in $_defaultConfig
+        $formatterConfig = $this->_config['formatter'] ?? DefaultFormatter::class;
+        if (!is_array($formatterConfig)) {
+            $formatterConfig = ['className' => $formatterConfig];
+        }
+
+        /** @var class-string<\Cake\Log\Formatter\AbstractFormatter> $className */
+        $className = $formatterConfig['className'];
+        $this->formatter = new $className($formatterConfig);
+        if (!$this->formatter instanceof AbstractFormatter) {
+            throw new InvalidArgumentException(sprintf(
+                'Formatter must extend `%s`, got `%s` instead',
+                AbstractFormatter::class,
+                $className
+            ));
+        }
     }
 
     /**
@@ -92,9 +116,22 @@ abstract class BaseLog extends AbstractLogger
      * @param string $message The message to be formatted.
      * @param array $context Additional logging information for the message.
      * @return string
+     * @deprecated 4.3.0 Call `resolve()` directly from your log engine and format the message in a formatter.
      */
     protected function _format(string $message, array $context = []): string
     {
+        return $this->resolve($message, $context);
+    }
+
+    /**
+     * Resolves interpolation expressions in message string.
+     *
+     * @param string $message Interpolated message
+     * @param array $context Interpolation expression values
+     * @return string
+     */
+    protected function resolve(string $message, array $context = []): string
+    {
         if (strpos($message, '{') === false && strpos($message, '}') === false) {
             return $message;
         }
@@ -167,16 +204,4 @@ abstract class BaseLog extends AbstractLogger
         /** @psalm-suppress InvalidArgument */
         return str_replace(array_keys($replacements), $replacements, $message);
     }
-
-    /**
-     * Returns date formatted according to given `dateFormat` option format.
-     *
-     * This function affects `FileLog` or` ConsoleLog` datetime information format.
-     *
-     * @return string
-     */
-    protected function _getFormattedDate(): string
-    {
-        return (new DateTimeImmutable())->format($this->_config['dateFormat']);
-    }
 }

+ 11 - 4
src/Log/Engine/ConsoleLog.php

@@ -17,6 +17,7 @@ declare(strict_types=1);
 namespace Cake\Log\Engine;
 
 use Cake\Console\ConsoleOutput;
+use Cake\Log\Formatter\DefaultFormatter;
 use InvalidArgumentException;
 
 /**
@@ -34,7 +35,10 @@ class ConsoleLog extends BaseLog
         'levels' => null,
         'scopes' => [],
         'outputAs' => null,
-        'dateFormat' => 'Y-m-d H:i:s',
+        'formatter' => [
+            'className' => DefaultFormatter::class,
+            'includeTags' => true,
+        ],
     ];
 
     /**
@@ -74,6 +78,11 @@ class ConsoleLog extends BaseLog
         if (isset($config['outputAs'])) {
             $this->_output->setOutputAs($config['outputAs']);
         }
+
+        if (isset($this->_config['dateFormat'])) {
+            deprecationWarning('`dateFormat` option should now be set in the formatter options.');
+            $this->formatter->setConfig('dateFormat', $this->_config['dateFormat']);
+        }
     }
 
     /**
@@ -88,8 +97,6 @@ class ConsoleLog extends BaseLog
     public function log($level, $message, array $context = [])
     {
         $message = $this->_format($message, $context);
-        $output = $this->_getFormattedDate() . ' ' . ucfirst($level) . ': ' . $message;
-
-        $this->_output->write(sprintf('<%s>%s</%s>', $level, $output, $level));
+        $this->_output->write($this->formatter->format($level, $message, $context));
     }
 }

+ 13 - 4
src/Log/Engine/FileLog.php

@@ -17,6 +17,7 @@ declare(strict_types=1);
 namespace Cake\Log\Engine;
 
 use Cake\Core\Configure;
+use Cake\Log\Formatter\DefaultFormatter;
 use Cake\Utility\Text;
 
 /**
@@ -53,7 +54,9 @@ class FileLog extends BaseLog
         'rotate' => 10,
         'size' => 10485760, // 10MB
         'mask' => null,
-        'dateFormat' => 'Y-m-d H:i:s',
+        'formatter' => [
+            'className' => DefaultFormatter::class,
+        ],
     ];
 
     /**
@@ -105,6 +108,11 @@ class FileLog extends BaseLog
                 $this->_size = Text::parseFileSize($this->_config['size']);
             }
         }
+
+        if (isset($this->_config['dateFormat'])) {
+            deprecationWarning('`dateFormat` option should now be set in the formatter options.');
+            $this->formatter->setConfig('dateFormat', $this->_config['dateFormat']);
+        }
     }
 
     /**
@@ -119,7 +127,8 @@ class FileLog extends BaseLog
     public function log($level, $message, array $context = []): void
     {
         $message = $this->_format($message, $context);
-        $output = $this->_getFormattedDate() . ' ' . ucfirst($level) . ': ' . $message . "\n";
+        $message = $this->formatter->format($level, $message, $context);
+
         $filename = $this->_getFilename($level);
         if ($this->_size) {
             $this->_rotateFile($filename);
@@ -128,13 +137,13 @@ class FileLog extends BaseLog
         $pathname = $this->_path . $filename;
         $mask = $this->_config['mask'];
         if (!$mask) {
-            file_put_contents($pathname, $output, FILE_APPEND);
+            file_put_contents($pathname, $message . "\n", FILE_APPEND);
 
             return;
         }
 
         $exists = is_file($pathname);
-        file_put_contents($pathname, $output, FILE_APPEND);
+        file_put_contents($pathname, $message . "\n", FILE_APPEND);
         static $selfError = false;
 
         if (!$selfError && !$exists && !chmod($pathname, (int)$mask)) {

+ 21 - 5
src/Log/Engine/SyslogLog.php

@@ -16,6 +16,8 @@ declare(strict_types=1);
  */
 namespace Cake\Log\Engine;
 
+use Cake\Log\Formatter\SyslogFormatter;
+
 /**
  * Syslog stream for Logging. Writes logs to the system logger
  */
@@ -52,10 +54,12 @@ class SyslogLog extends BaseLog
     protected $_defaultConfig = [
         'levels' => [],
         'scopes' => [],
-        'format' => '%s: %s',
         'flag' => LOG_ODELAY,
         'prefix' => '',
         'facility' => LOG_USER,
+        'formatter' => [
+            'className' => SyslogFormatter::class,
+        ],
     ];
 
     /**
@@ -82,6 +86,19 @@ class SyslogLog extends BaseLog
     protected $_open = false;
 
     /**
+     * @inheritDoc
+     */
+    public function __construct(array $config = [])
+    {
+        parent::__construct($config);
+
+        if (isset($this->_config['format'])) {
+            deprecationWarning('`format` option should now be set in the formatter options.');
+            $this->formatter->setConfig('format', $this->_config['format']);
+        }
+    }
+
+    /**
      * Writes a message to syslog
      *
      * Map the $level back to a LOG_ constant value, split multi-line messages into multiple
@@ -106,10 +123,9 @@ class SyslogLog extends BaseLog
             $priority = $this->_levelMap[$level];
         }
 
-        $messages = explode("\n", $this->_format($message, $context));
-        foreach ($messages as $message) {
-            $message = sprintf($this->_config['format'], $level, $message);
-            $this->_write($priority, $message);
+        $lines = explode("\n", $this->_format($message, $context));
+        foreach ($lines as $line) {
+            $this->_write($priority, $this->formatter->format($level, $line, $context));
         }
     }
 

+ 50 - 0
src/Log/Formatter/AbstractFormatter.php

@@ -0,0 +1,50 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) :  Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakefoundation.org CakePHP(tm) Project
+ * @since         4.3.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Log\Formatter;
+
+use Cake\Core\InstanceConfigTrait;
+
+abstract class AbstractFormatter
+{
+    use InstanceConfigTrait;
+
+    /**
+     * Default config for this class
+     *
+     * @var array
+     */
+    protected $_defaultConfig = [
+    ];
+
+    /**
+     * @param array $config Config options
+     */
+    public function __construct(array $config = [])
+    {
+        $this->setConfig($config);
+    }
+
+    /**
+     * Formats message.
+     *
+     * @param mixed $level Logging level
+     * @param string $message Message string
+     * @param array $context Mesage context
+     * @return string Formatted message
+     */
+    abstract public function format($level, string $message, array $context = []): string;
+}

+ 51 - 0
src/Log/Formatter/DefaultFormatter.php

@@ -0,0 +1,51 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) :  Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakefoundation.org CakePHP(tm) Project
+ * @since         4.3.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Log\Formatter;
+
+class DefaultFormatter extends AbstractFormatter
+{
+    /**
+     * Default config for this class
+     *
+     * @var array
+     */
+    protected $_defaultConfig = [
+        'dateFormat' => 'Y-m-d H:i:s',
+        'includeTags' => false,
+    ];
+
+    /**
+     * @param array $config Formatter config
+     */
+    public function __construct(array $config = [])
+    {
+        $this->setConfig($config);
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function format($level, string $message, array $context = []): string
+    {
+        $message = sprintf('%s %s: %s', date($this->_config['dateFormat']), ucfirst($level), $message);
+        if ($this->_config['includeTags']) {
+            $message = sprintf('<%s>%s</%s>', $level, $message, $level);
+        }
+
+        return $message;
+    }
+}

+ 45 - 0
src/Log/Formatter/SyslogFormatter.php

@@ -0,0 +1,45 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) :  Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakefoundation.org CakePHP(tm) Project
+ * @since         4.3.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Log\Formatter;
+
+class SyslogFormatter extends AbstractFormatter
+{
+    /**
+     * Default config for this class
+     *
+     * @var array
+     */
+    protected $_defaultConfig = [
+        'format' => '%s: %s',
+    ];
+
+    /**
+     * @param array $config Formatter config
+     */
+    public function __construct(array $config = [])
+    {
+        $this->setConfig($config);
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function format($level, string $message, array $context = []): string
+    {
+        return sprintf($this->getConfig('format'), $level, $message);
+    }
+}

+ 13 - 0
tests/TestCase/Log/Engine/BaseLogTest.php

@@ -21,8 +21,10 @@ use Cake\Datasource\ConnectionManager;
 use Cake\Http\Response;
 use Cake\ORM\Entity;
 use Cake\TestSuite\TestCase;
+use InvalidArgumentException;
 use Psr\Log\LogLevel;
 use TestApp\Log\Engine\TestBaseLog;
+use TestApp\Log\Formatter\InvalidFormatter;
 
 class BaseLogTest extends TestCase
 {
@@ -144,4 +146,15 @@ class BaseLogTest extends TestCase
         );
         $this->assertSame('val', $this->logger->getMessage());
     }
+
+    /**
+     * Test creating log engine with invalid formatter.
+     *
+     * @return void
+     */
+    public function testInvalidFormatter()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        new TestBaseLog(['formatter' => InvalidFormatter::class]);
+    }
 }

+ 21 - 1
tests/TestCase/Log/Engine/ConsoleLogTest.php

@@ -88,11 +88,31 @@ class ConsoleLogTest extends TestCase
         $filename = tempnam(sys_get_temp_dir(), 'cake_log_test');
         $log = new ConsoleLog([
             'stream' => $filename,
-            'dateFormat' => 'c',
+            'formatter.dateFormat' => 'c',
         ]);
         $log->log('error', 'oh noes');
         $fh = fopen($filename, 'r');
         $line = fgets($fh);
         $this->assertMatchesRegularExpression('/2[0-9]{3}-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+\+\d{2}:\d{2} Error: oh noes/', $line);
     }
+
+    /**
+     * Test deprecated dateFormat option
+     *
+     * @return void
+     */
+    public function testDeprecatedDateFormat()
+    {
+        $this->deprecated(function () {
+            $filename = tempnam(sys_get_temp_dir(), 'cake_log_test');
+            $log = new ConsoleLog([
+                'stream' => $filename,
+                'dateFormat' => 'c',
+            ]);
+            $log->log('error', 'oh noes');
+            $fh = fopen($filename, 'r');
+            $line = fgets($fh);
+            $this->assertMatchesRegularExpression('/2[0-9]{3}-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+\+\d{2}:\d{2} Error: oh noes/', $line);
+        });
+    }
 }

+ 19 - 3
tests/TestCase/Log/Engine/FileLogTest.php

@@ -208,13 +208,29 @@ class FileLogTest extends TestCase
     {
         $this->_deleteLogs(LOGS);
 
-        // original 'Y-m-d H:i:s' format test was testLogFileWriting() method
-
         // 'c': ISO 8601 date (added in PHP 5)
-        $log = new FileLog(['path' => LOGS, 'dateFormat' => 'c']);
+        $log = new FileLog(['path' => LOGS, 'formatter.dateFormat' => 'c']);
         $log->log('warning', 'Test warning');
 
         $result = file_get_contents(LOGS . 'error.log');
         $this->assertMatchesRegularExpression('/^2[0-9]{3}-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+\+\d{2}:\d{2} Warning: Test warning/', $result);
     }
+
+    /**
+     * Test deprecated dateFormat option
+     *
+     * @return void
+     */
+    public function testDeprecatedDateFormat()
+    {
+        $this->deprecated(function () {
+            $this->_deleteLogs(LOGS);
+
+            $log = new FileLog(['path' => LOGS, 'dateFormat' => 'c']);
+            $log->log('warning', 'Test warning');
+
+            $result = file_get_contents(LOGS . 'error.log');
+            $this->assertMatchesRegularExpression('/^2[0-9]{3}-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+\+\d{2}:\d{2} Warning: Test warning/', $result);
+        });
+    }
 }

+ 22 - 0
tests/TestCase/Log/Engine/SyslogLogTest.php

@@ -89,6 +89,28 @@ class SyslogLogTest extends TestCase
     }
 
     /**
+     * Test deprecated format option
+     *
+     * @return void
+     */
+    public function testDeprecatedFormat()
+    {
+        $this->deprecated(function () {
+            $log = $this->getMockBuilder(SyslogLog::class)
+                ->setConstructorArgs(['config' => ['format' => 'custom %s: %s']])
+                ->onlyMethods(['_open', '_write'])
+                ->getMock();
+            $log->expects($this->exactly(2))
+                ->method('_write')
+                ->withConsecutive(
+                    [LOG_DEBUG, 'custom debug: Foo'],
+                    [LOG_DEBUG, 'custom debug: Bar']
+                );
+            $log->log('debug', "Foo\nBar");
+        });
+    }
+
+    /**
      * Data provider for the write function test
      *
      * @return array

+ 8 - 0
tests/test_app/TestApp/Log/Formatter/InvalidFormatter.php

@@ -0,0 +1,8 @@
+<?php
+declare(strict_types=1);
+
+namespace TestApp\Log\Formatter;
+
+class InvalidFormatter
+{
+}