Convert PsrLogAdapter to a singleton and revert Client changes

Author: jmikola

composer.json

@@ -22,6 +22,7 @@
     "require-dev": {
         "doctrine/coding-standard": "^11.1",
         "phpbench/phpbench": "^1.2",
+        "psr/log": "^1.1.4",
         "rector/rector": "^0.16.0",
         "squizlabs/php_codesniffer": "^3.7",
         "symfony/phpunit-bridge": "^5.2",

src/Client.php

@@ -37,14 +37,12 @@
 use MongoDB\Operation\ListDatabaseNames;
 use MongoDB\Operation\ListDatabases;
 use MongoDB\Operation\Watch;
-use Psr\Log\LoggerAwareInterface;
-use Psr\Log\LoggerInterface;
 use Throwable;
 
 use function is_array;
 use function is_string;
 
-class Client implements LoggerAwareInterface
+class Client
 {
     public const DEFAULT_URI = 'mongodb://127.0.0.1/';
 
@@ -331,19 +329,6 @@ public function selectDatabase(string $databaseName, array $options = [])
         return new Database($this->manager, $databaseName, $options);
     }
 
-    public function setLogger(LoggerInterface $logger): void
-    {
-        /* TODO: lazily initialize a PsrLogAdapter for this client and register
-         * it with PHPC. Consider what happens if multiple clients instances in
-         * an app register the same PSR logger. That logger will likely receive
-         * duplicate messages, since PHPC's own checks would only prevent the
-         * same PsrLogAdapter instance from being registered multiple times.
-         *
-         * Perhaps we can work around this by making PsrLogAdapter a singleton
-         * or utilize a static map of PSR loggers.
-         */
-    }
-
     /**
      * Start a new client session.
      *

src/PsrLogAdapter.php

@@ -17,56 +17,80 @@
 
 namespace MongoDB;
 
-use MongoDB\Driver\Logging\Logger as DriverLoggerInterface;
-use Psr\Log\LoggerInterface as PsrLoggerInterface;
+use MongoDB\Driver\Logging\Logger as DriverLogger;
+use Psr\Log\LoggerInterface as PsrLogger;
 use Psr\Log\LogLevel as PsrLogLevel;
+use SplObjectStorage;
 
-class PsrLogAdapter implements DriverLoggerInterface
+use function MongoDB\Driver\Logging\addLogger;
+use function MongoDB\Driver\Logging\removeLogger;
+
+final class PsrLogAdapter implements DriverLogger
 {
-    private PsrLoggerInterface $psrLogger;
+    private static ?self $instance = null;
+
+    /** @psalm-var SplObjectStorage<PsrLogger, null> */
+    private SplObjectStorage $psrLoggers;
+
+    private const PSR_LEVELS = [
+        DriverLogger::LEVEL_ERROR => PsrLogLevel::ERROR,
+        /* libmongoc considers "critical" less severe than "error" so map it to
+         * "error" in the PSR logger. */
+        DriverLogger::LEVEL_CRITICAL => PsrLogLevel::ERROR,
+        DriverLogger::LEVEL_WARNING => PsrLogLevel::WARNING,
+        DriverLogger::LEVEL_MESSAGE => PsrLogLevel::NOTICE,
+        DriverLogger::LEVEL_INFO => PsrLogLevel::INFO,
+        DriverLogger::LEVEL_DEBUG => PsrLogLevel::DEBUG,
+        /* PSR does not define a "trace" level, so map it to "debug" in the PSR
+         * logger. That said, trace logging is very verbose and should not be
+         * encouraged when using a PSR logger. */
+        DriverLogger::LEVEL_TRACE => PsrLogLevel::DEBUG,
+    ];
+
+    public static function addLogger(PsrLogger $psrLogger): void
+    {
+        $instance = self::getInstance();
+
+        $instance->psrLoggers->attach($psrLogger);
+
+        addLogger($instance);
+    }
 
-    public function __construct(PsrLoggerInterface $psrLogger)
+    public static function getInstance(): self
     {
-        $this->psrLogger = $psrLogger;
+        if (self::$instance === null) {
+            self::$instance = new self();
+        }
+
+        return self::$instance;
     }
 
     public function log(int $level, string $domain, string $message): void
     {
-        $this->psrLogger->log(
-            $this->driverLevelToPsrLevel($level),
-            $domain . ': ' . $message,
-        );
+        $instance = self::getInstance();
+        /* A default case is intentionally not handled since libmongoc and
+         * PHPC should avoid emitting bogus levels; however, we can consider
+         * throwing an UnexpectedValueException here. */
+        $psrLevel = self::PSR_LEVELS[$level];
+        $context = ['domain' => $domain];
+
+        foreach ($instance->psrLoggers as $psrLogger) {
+            $psrLogger->log($psrLevel, $message, $context);
+        }
     }
 
-    // TODO: Refactor this method as a private const array map on PsrLogAdapter
-    private function driverLevelToPsrLevel(int $level): string
+    public static function removeLogger(PsrLogger $psrLogger): void
     {
-        switch ($level) {
-            /* libmongoc considers "critical" less severe than "error" so route
-             * both levels to "error" in the PSR logger. */
-            case DriverLoggerInterface::LEVEL_ERROR:
-            case DriverLoggerInterface::LEVEL_CRITICAL:
-                return PsrLogLevel::ERROR;
-
-            case DriverLoggerInterface::LEVEL_WARNING:
-                return PsrLogLevel::WARNING;
-
-            case DriverLoggerInterface::LEVEL_MESSAGE:
-                return PsrLogLevel::NOTICE;
-
-            case DriverLoggerInterface::LEVEL_INFO:
-                return PsrLogLevel::INFO;
-
-            /* PSR does not define a "trace" level, so route both levels to
-             * "debug". That said, trace logging is very verbose and should not
-             * be encouraged when using a PSR logger. */
-            case DriverLoggerInterface::LEVEL_DEBUG:
-            case DriverLoggerInterface::LEVEL_TRACE:
-                return PsrLogLevel::DEBUG;
-
-            /* A default case is intentionally not handled since libmongoc and
-             * PHPC should avoid emitting bogus levels; however, we can consider
-             * throwing an UnexpectedValueException here. */
+        $instance = self::getInstance();
+        $instance->psrLoggers->detach($psrLogger);
+
+        if ($instance->psrLoggers->count() === 0) {
+            removeLogger($instance);
         }
     }
+
+    private function __construct()
+    {
+        $this->psrLoggers = new SplObjectStorage();
+    }
 }

tests/PsrLogAdapterTest.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace MongoDB\Tests;
+
+use MongoDB\Driver\Logging\Logger as DriverLogger;
+use MongoDB\PsrLogAdapter;
+use PHPUnit\Framework\TestCase as BaseTestCase;
+use Psr\Log\AbstractLogger;
+use Psr\Log\LoggerInterface;
+use Psr\Log\LogLevel;
+
+use function func_get_args;
+use function MongoDB\Driver\Logging\log;
+
+class PsrLogAdapterTest extends BaseTestCase
+{
+    private PsrLogAdapter $adapter;
+
+    private LoggerInterface $logger;
+
+    public function setUp(): void
+    {
+        $this->logger = $this->createTestPsrLogger();
+    }
+
+    public function tearDown(): void
+    {
+        PsrLogAdapter::removeLogger($this->logger);
+    }
+
+    public function testDriverLogLevelsAreMappedToPsrLogLevels(): void
+    {
+        PsrLogAdapter::addLogger($this->logger);
+
+        log(DriverLogger::LEVEL_ERROR, 'error');
+        log(DriverLogger::LEVEL_CRITICAL, 'critical');
+        log(DriverLogger::LEVEL_WARNING, 'warning');
+        log(DriverLogger::LEVEL_MESSAGE, 'message');
+        log(DriverLogger::LEVEL_INFO, 'info');
+        log(DriverLogger::LEVEL_DEBUG, 'debug');
+        log(DriverLogger::LEVEL_TRACE, 'trace');
+
+        $context = ['domain' => 'php'];
+
+        $expectedLogs = [
+            [LogLevel::ERROR, 'error', $context],
+            [LogLevel::ERROR, 'critical', $context],
+            [LogLevel::WARNING, 'warning', $context],
+            [LogLevel::NOTICE, 'message', $context],
+            [LogLevel::INFO, 'info', $context],
+            [LogLevel::DEBUG, 'debug', $context],
+            [LogLevel::DEBUG, 'trace', $context],
+        ];
+
+        $this->assertSame($this->logger->logs, $expectedLogs);
+    }
+
+    private function createTestPsrLogger(): LoggerInterface
+    {
+        return new class extends AbstractLogger {
+            public $logs = [];
+
+            public function log($level, $message, array $context = []): void
+            {
+                $this->logs[] = func_get_args();
+            }
+        };
+    }
+}