From 81df2d2c12917ec9d442f2f5362f0e5d609a830b Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sat, 17 Aug 2024 00:25:08 +0200 Subject: [PATCH] feat(PsrLoggerAdapter): Allow to use `Psr\Log\LogLevel` for `log` method There is the `Psr\Log\LogLevel` class defining loglevel constants, to be fully compatible we should at least support those logging levels. Moreover this is the last part that was still required from `ILogger` interface, as we did not have alternatives for the loglevel constants. Signed-off-by: Ferdinand Thiessen --- lib/private/Log/PsrLoggerAdapter.php | 119 +++++++------------------ tests/lib/Log/PsrLoggerAdapterTest.php | 88 ++++++++++++++++++ 2 files changed, 118 insertions(+), 89 deletions(-) create mode 100644 tests/lib/Log/PsrLoggerAdapterTest.php diff --git a/lib/private/Log/PsrLoggerAdapter.php b/lib/private/Log/PsrLoggerAdapter.php index 57a67bd67a793..79e9d4d96212a 100644 --- a/lib/private/Log/PsrLoggerAdapter.php +++ b/lib/private/Log/PsrLoggerAdapter.php @@ -14,6 +14,7 @@ use OCP\Log\IDataLogger; use Psr\Log\InvalidArgumentException; use Psr\Log\LoggerInterface; +use Psr\Log\LogLevel; use Throwable; use function array_key_exists; use function array_merge; @@ -24,6 +25,20 @@ public function __construct( ) { } + public static function logLevelToInt(string $level): int { + return match ($level) { + LogLevel::ALERT => ILogger::ERROR, + LogLevel::CRITICAL => ILogger::ERROR, + LogLevel::DEBUG => ILogger::DEBUG, + LogLevel::EMERGENCY => ILogger::FATAL, + LogLevel::ERROR => ILogger::ERROR, + LogLevel::INFO => ILogger::INFO, + LogLevel::NOTICE => ILogger::INFO, + LogLevel::WARNING => ILogger::WARN, + default => throw new InvalidArgumentException('Unsupported custom log level'), + }; + } + public function setEventDispatcher(IEventDispatcher $eventDispatcher): void { $this->logger->setEventDispatcher($eventDispatcher); } @@ -39,17 +54,7 @@ private function containsThrowable(array $context): bool { * @param mixed[] $context */ public function emergency($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::FATAL, - ], - $context - )); - } else { - $this->logger->emergency((string)$message, $context); - } + $this->log(LogLevel::EMERGENCY, (string)$message, $context); } /** @@ -62,17 +67,7 @@ public function emergency($message, array $context = []): void { * @param mixed[] $context */ public function alert($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::ERROR, - ], - $context - )); - } else { - $this->logger->alert((string)$message, $context); - } + $this->log(LogLevel::ALERT, (string)$message, $context); } /** @@ -84,17 +79,7 @@ public function alert($message, array $context = []): void { * @param mixed[] $context */ public function critical($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::ERROR, - ], - $context - )); - } else { - $this->logger->critical((string)$message, $context); - } + $this->log(LogLevel::CRITICAL, (string)$message, $context); } /** @@ -105,17 +90,7 @@ public function critical($message, array $context = []): void { * @param mixed[] $context */ public function error($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::ERROR, - ], - $context - )); - } else { - $this->logger->error((string)$message, $context); - } + $this->log(LogLevel::ERROR, (string)$message, $context); } /** @@ -128,17 +103,7 @@ public function error($message, array $context = []): void { * @param mixed[] $context */ public function warning($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::WARN, - ], - $context - )); - } else { - $this->logger->warning((string)$message, $context); - } + $this->log(LogLevel::WARNING, (string)$message, $context); } /** @@ -148,17 +113,7 @@ public function warning($message, array $context = []): void { * @param mixed[] $context */ public function notice($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::INFO, - ], - $context - )); - } else { - $this->logger->notice((string)$message, $context); - } + $this->log(LogLevel::NOTICE, (string)$message, $context); } /** @@ -170,17 +125,7 @@ public function notice($message, array $context = []): void { * @param mixed[] $context */ public function info($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::INFO, - ], - $context - )); - } else { - $this->logger->info((string)$message, $context); - } + $this->log(LogLevel::INFO, (string)$message, $context); } /** @@ -190,17 +135,7 @@ public function info($message, array $context = []): void { * @param mixed[] $context */ public function debug($message, array $context = []): void { - if ($this->containsThrowable($context)) { - $this->logger->logException($context['exception'], array_merge( - [ - 'message' => (string)$message, - 'level' => ILogger::DEBUG, - ], - $context - )); - } else { - $this->logger->debug((string)$message, $context); - } + $this->log(LogLevel::DEBUG, (string)$message, $context); } /** @@ -213,8 +148,14 @@ public function debug($message, array $context = []): void { * @throws InvalidArgumentException */ public function log($level, $message, array $context = []): void { + if (is_string($level)) { + $level = self::logLevelToInt($level); + } + if (isset($context['level']) && is_string($context['level'])) { + $context['level'] = self::logLevelToInt($context['level']); + } if (!is_int($level) || $level < ILogger::DEBUG || $level > ILogger::FATAL) { - throw new InvalidArgumentException('Nextcloud allows only integer log levels'); + throw new InvalidArgumentException('Unsupported custom log level'); } if ($this->containsThrowable($context)) { $this->logger->logException($context['exception'], array_merge( diff --git a/tests/lib/Log/PsrLoggerAdapterTest.php b/tests/lib/Log/PsrLoggerAdapterTest.php new file mode 100644 index 0000000000000..cc9ddea378a07 --- /dev/null +++ b/tests/lib/Log/PsrLoggerAdapterTest.php @@ -0,0 +1,88 @@ +logger = $this->createMock(Log::class); + $this->loggerAdapter = new PsrLoggerAdapter($this->logger); + } + + /** + * @dataProvider dataPsrLoggingLevels + */ + public function testLoggingWithPsrLogLevels(string $level, int $expectedLevel): void { + $this->logger->expects(self::once()) + ->method('log') + ->with($expectedLevel, 'test message', ['app' => 'test']); + $this->loggerAdapter->log($level, 'test message', ['app' => 'test']); + } + + /** + * @dataProvider dataPsrLoggingLevels + */ + public function testLogLevelToInt(string $level, int $expectedLevel): void { + $this->assertEquals($expectedLevel, PsrLoggerAdapter::logLevelToInt($level)); + } + + public function dataPsrLoggingLevels(): array { + return [ + [LogLevel::ALERT, ILogger::ERROR], + [LogLevel::CRITICAL, ILogger::ERROR], + [LogLevel::DEBUG, ILogger::DEBUG], + [LogLevel::EMERGENCY, ILogger::FATAL], + [LogLevel::ERROR, ILogger::ERROR], + [LogLevel::INFO, ILogger::INFO], + [LogLevel::NOTICE, ILogger::INFO], + [LogLevel::WARNING, ILogger::WARN], + ]; + } + + /** + * @dataProvider dataInvalidLoggingLevel + */ + public function testInvalidLoggingLevel($level): void { + $this->logger->expects(self::never()) + ->method('log'); + $this->expectException(InvalidArgumentException::class); + + $this->loggerAdapter->log($level, 'valid message'); + } + + public function dataInvalidLoggingLevel(): array { + return [ + // invalid string + ['this is not a level'], + // int out of range + [ILogger::DEBUG - 1], + [ILogger::FATAL + 1], + // float is not allowed + [1.2345], + // boolean is not a level + [true], + [false], + // + [null], + ]; + } +}