From a00cf3e8fbe20a8a0923cca58fed185a0ded6179 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Sun, 4 Oct 2026 21:23:47 +0400 Subject: [PATCH] fix(DatabaseManager): make setLogger() reach every driver test(DatabaseManager): cover setLogger() with factory, added drivers and databases, clones, reconnect and transactions A logger set explicitly now takes precedence over the LoggerFactory for existing and future drivers, and is passed to drivers registered via addDriver() and addDatabase(). Drivers cloned by Database::withoutCache() are not tracked by the manager and keep the logger they had when cloned. Refs cycle/orm#60 Assisted-By: Claude Opus 5.5 --- src/DatabaseManager.php | 48 ++- .../Unit/DatabaseManagerLoggerTest.php | 334 ++++++++++++++++++ .../Unit/Driver/DatabaseManagerTest.php | 21 +- 3 files changed, 387 insertions(+), 16 deletions(-) create mode 100644 tests/Database/Unit/DatabaseManagerLoggerTest.php diff --git a/src/DatabaseManager.php b/src/DatabaseManager.php index 2e9077fd..09155eca 100644 --- a/src/DatabaseManager.php +++ b/src/DatabaseManager.php @@ -44,17 +44,20 @@ public function __construct( ) {} /** - * Set logger for all drivers + * Takes precedence over the logger factory, for drivers created later too. + * Drivers cloned by {@see Database::withoutCache()} before this call keep their previous logger. */ public function setLogger(LoggerInterface $logger): void { $this->logger = $logger; - // Assign the logger to all initialized drivers foreach ($this->drivers as $driver) { - if ($driver instanceof LoggerAwareInterface) { - $driver->setLogger($this->logger); - } + $this->applyLogger($driver, $logger); + } + + foreach ($this->databases as $database) { + $this->applyLogger($database->getDriver(DatabaseInterface::WRITE), $logger); + $this->applyLogger($database->getDriver(DatabaseInterface::READ), $logger); } } @@ -120,6 +123,11 @@ public function addDatabase(Database $database): void ); $this->databases[$database->getName()] = $database; + + if ($this->logger !== null) { + $this->applyLogger($database->getDriver(DatabaseInterface::WRITE), $this->logger); + $this->applyLogger($database->getDriver(DatabaseInterface::READ), $this->logger); + } } /** @@ -160,11 +168,9 @@ public function driver(string $driver): DriverInterface $driverObject = $this->config->getDriver($driver); $this->drivers[$driver] = $driverObject; - if ($driverObject instanceof LoggerAwareInterface) { - $logger = $this->getLoggerForDriver($driverObject); - if (!$logger instanceof NullLogger) { - $driverObject->setLogger($logger); - } + $logger = $this->getLoggerForDriver($driverObject); + if ($logger !== null) { + $this->applyLogger($driverObject, $logger); } return $this->drivers[$driver]; @@ -183,6 +189,10 @@ public function addDriver(string $name, DriverInterface $driver): self $this->drivers[$name] = $driver; + if ($this->logger !== null) { + $this->applyLogger($driver, $this->logger); + } + return $this; } @@ -199,12 +209,22 @@ private function makeDatabase(DatabasePartial $database): Database ); } - private function getLoggerForDriver(DriverInterface $driver): LoggerInterface + private function getLoggerForDriver(DriverInterface $driver): ?LoggerInterface { - if (!$this->loggerFactory) { - return $this->logger ??= new NullLogger(); + if ($this->logger !== null) { + return $this->logger; } - return $this->loggerFactory->getLogger($driver); + $logger = $this->loggerFactory?->getLogger($driver); + + // A driver without a logger skips building the log context of every query. + return $logger instanceof NullLogger ? null : $logger; + } + + private function applyLogger(DriverInterface $driver, LoggerInterface $logger): void + { + if ($driver instanceof LoggerAwareInterface) { + $driver->setLogger($logger); + } } } diff --git a/tests/Database/Unit/DatabaseManagerLoggerTest.php b/tests/Database/Unit/DatabaseManagerLoggerTest.php new file mode 100644 index 00000000..90f6dd34 --- /dev/null +++ b/tests/Database/Unit/DatabaseManagerLoggerTest.php @@ -0,0 +1,334 @@ +createManager(); + $dbal->setLogger($logger = new RecordingLogger()); + + $dbal->database('default')->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testSetLoggerAfterDatabaseIsCreated(): void + { + $dbal = $this->createManager(); + $db = $dbal->database('default'); + $dbal->setLogger($logger = new RecordingLogger()); + + $db->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testSetLoggerAfterDriverIsCreated(): void + { + $dbal = $this->createManager(); + $driver = $dbal->driver('write'); + $dbal->setLogger($logger = new RecordingLogger()); + + $driver->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testSetLoggerAfterGetDrivers(): void + { + $dbal = $this->createManager(); + $drivers = $dbal->getDrivers(); + $dbal->setLogger($logger = new RecordingLogger()); + + foreach ($drivers as $driver) { + $driver->query('SELECT 1')->fetchAll(); + } + + $this->assertSame(\count($drivers), $logger->count('SELECT 1')); + } + + public function testSetLoggerReachesReadDriver(): void + { + $dbal = $this->createManager(); + $db = $dbal->database('default'); + $dbal->setLogger($logger = new RecordingLogger()); + + $db->getDriver(DatabaseInterface::READ)->query('SELECT 1')->fetchAll(); + + $this->assertNotSame($db->getDriver(DatabaseInterface::READ), $db->getDriver(DatabaseInterface::WRITE)); + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testRepeatedSetLoggerReplacesLoggerEverywhere(): void + { + $dbal = $this->createManager(); + $db = $dbal->database('default'); + $dbal->setLogger($first = new RecordingLogger()); + $dbal->setLogger($second = new RecordingLogger()); + + $db->query('SELECT 1')->fetchAll(); + $dbal->database('other')->query('SELECT 2')->fetchAll(); + + $this->assertSame([], $first->messages); + $this->assertLogged($second, 'SELECT 1'); + $this->assertLogged($second, 'SELECT 2'); + } + + public function testSetLoggerOverridesLoggerSetOnDriver(): void + { + $dbal = $this->createManager(); + $driver = $dbal->driver('write'); + $driver->setLogger($own = new RecordingLogger()); + $dbal->setLogger($global = new RecordingLogger()); + + $driver->query('SELECT 1')->fetchAll(); + + $this->assertSame([], $own->messages); + $this->assertLogged($global, 'SELECT 1'); + } + + public function testLoggerFactoryIsUsedForNewDrivers(): void + { + $dbal = $this->createManager($factory = new RecordingLoggerFactory()); + + $dbal->database('default')->query('SELECT 1')->fetchAll(); + + $this->assertLogged($factory->loggers['read'], 'SELECT 1'); + } + + public function testSetLoggerWithLoggerFactoryBeforeDriversAreCreated(): void + { + $dbal = $this->createManager(new RecordingLoggerFactory()); + $dbal->setLogger($logger = new RecordingLogger()); + + $dbal->database('default')->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testSetLoggerWithLoggerFactoryAfterDriversAreCreated(): void + { + $dbal = $this->createManager(new RecordingLoggerFactory()); + $db = $dbal->database('default'); + $dbal->setLogger($logger = new RecordingLogger()); + + $db->query('SELECT 1')->fetchAll(); + $dbal->database('other')->query('SELECT 2')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + $this->assertLogged($logger, 'SELECT 2'); + } + + public function testDriverAddedAfterSetLogger(): void + { + $dbal = $this->createManager(); + $dbal->setLogger($logger = new RecordingLogger()); + $dbal->addDriver('manual', $driver = $this->createDriver()); + + $driver->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testDriverAddedBeforeSetLogger(): void + { + $dbal = $this->createManager(); + $dbal->addDriver('manual', $driver = $this->createDriver()); + $dbal->setLogger($logger = new RecordingLogger()); + + $driver->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testDatabaseAddedBeforeSetLogger(): void + { + $dbal = $this->createManager(); + $dbal->addDatabase(new Database('manual', '', $this->createDriver())); + $dbal->setLogger($logger = new RecordingLogger()); + + $dbal->database('manual')->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testDatabaseAddedAfterSetLogger(): void + { + $dbal = $this->createManager(); + $dbal->setLogger($logger = new RecordingLogger()); + $dbal->addDatabase(new Database('manual', '', $this->createDriver())); + + $dbal->database('manual')->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testDatabaseWithoutCacheCreatedBeforeSetLoggerKeepsPreviousLogger(): void + { + $dbal = $this->createManager(); + $dbal->setLogger($previous = new RecordingLogger()); + $db = $dbal->database('default')->withoutCache(); + $dbal->setLogger($logger = new RecordingLogger()); + + $db->query('SELECT 1')->fetchAll(); + + $this->assertSame([], $logger->messages); + $this->assertLogged($previous, 'SELECT 1'); + } + + public function testDatabaseWithoutCacheCreatedAfterSetLogger(): void + { + $dbal = $this->createManager(); + $dbal->setLogger($logger = new RecordingLogger()); + $db = $dbal->database('default')->withoutCache(); + + $db->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testDatabaseWithPrefixCreatedBeforeSetLogger(): void + { + $dbal = $this->createManager(); + $db = $dbal->database('default')->withPrefix('p_'); + $dbal->setLogger($logger = new RecordingLogger()); + + $db->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testSetLoggerSurvivesReconnect(): void + { + $dbal = $this->createManager(); + $driver = $dbal->driver('write'); + $driver->query('SELECT 0')->fetchAll(); + $dbal->setLogger($logger = new RecordingLogger()); + + $driver->disconnect(); + $driver->query('SELECT 1')->fetchAll(); + + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testSetLoggerLogsTransactions(): void + { + $dbal = $this->createManager(); + $db = $dbal->database('default'); + $dbal->setLogger($logger = new RecordingLogger()); + + $db->transaction(static function (Database $db): void { + $db->transaction(static fn(Database $db) => $db->query('SELECT 1')->fetchAll()); + }); + $db->begin(); + $db->rollback(); + + $this->assertLogged($logger, 'Begin transaction'); + $this->assertLogged($logger, "Transaction: new savepoint 'SVP2'"); + $this->assertLogged($logger, "Transaction: release savepoint 'SVP2'"); + $this->assertLogged($logger, 'Commit transaction'); + $this->assertLogged($logger, 'Rollback transaction'); + $this->assertLogged($logger, 'SELECT 1'); + } + + public function testNullLoggerSilencesDrivers(): void + { + $dbal = $this->createManager(); + $db = $dbal->database('default'); + $dbal->setLogger($logger = new RecordingLogger()); + $dbal->setLogger(new NullLogger()); + + $db->query('SELECT 1')->fetchAll(); + $dbal->database('other')->query('SELECT 2')->fetchAll(); + + $this->assertSame([], $logger->messages); + } + + public function testNullLoggerIsPassedToNewDrivers(): void + { + $dbal = $this->createManager(); + $dbal->setLogger($logger = new NullLogger()); + + $driver = $dbal->driver('write'); + + $this->assertSame($logger, (fn() => $this->logger)->call($driver)); + } + + private function createManager(?LoggerFactoryInterface $factory = null): DatabaseManager + { + return new DatabaseManager( + new DatabaseConfig([ + 'default' => 'default', + 'databases' => [ + 'default' => ['write' => 'write', 'read' => 'read'], + 'other' => ['driver' => 'other'], + ], + 'connections' => [ + 'write' => new SQLiteDriverConfig(connection: new MemoryConnectionConfig()), + 'read' => new SQLiteDriverConfig(connection: new MemoryConnectionConfig()), + 'other' => new SQLiteDriverConfig(connection: new MemoryConnectionConfig()), + ], + ]), + $factory, + ); + } + + private function createDriver(): DriverInterface + { + return SQLiteDriver::create(new SQLiteDriverConfig(connection: new MemoryConnectionConfig())); + } + + private function assertLogged(RecordingLogger $logger, string $message): void + { + $this->assertGreaterThan( + 0, + $logger->count($message), + \sprintf("Message '%s' is not logged. Logged: %s", $message, \json_encode($logger->messages)), + ); + } +} + +final class RecordingLogger extends AbstractLogger +{ + /** @var list */ + public array $messages = []; + + public function log($level, \Stringable|string $message, array $context = []): void + { + $this->messages[] = (string) $message; + } + + public function count(string $message): int + { + return \count(\array_filter($this->messages, static fn(string $m) => \trim($m) === $message)); + } +} + +final class RecordingLoggerFactory implements LoggerFactoryInterface +{ + /** @var array */ + public array $loggers = []; + + public function getLogger(?DriverInterface $driver = null): LoggerInterface + { + return $this->loggers[$driver?->getName() ?? ''] ??= new RecordingLogger(); + } +} diff --git a/tests/Database/Unit/Driver/DatabaseManagerTest.php b/tests/Database/Unit/Driver/DatabaseManagerTest.php index 4c4dbca0..078a83b0 100644 --- a/tests/Database/Unit/Driver/DatabaseManagerTest.php +++ b/tests/Database/Unit/Driver/DatabaseManagerTest.php @@ -91,7 +91,25 @@ public function testDatabaseManagerWithLoggerAndWithoutLoggerFactoryShouldReturn $this->assertSame($this->logger, $property->getValue($driver)); } - public function testDatabaseManagerWithLoggerAndWithLoggerFactoryShouldReturnLoggerFromFactory(): void + public function testDatabaseManagerWithLoggerAndWithLoggerFactoryShouldReturnLogger(): void + { + $manager = new DatabaseManager( + $this->getDatabaseConfig(), + $this->loggerFactory, + ); + + $this->loggerFactory->expects($this->never())->method('getLogger'); + + $manager->setLogger($this->logger); + $driver = $manager->driver('test'); + + $refl = new \ReflectionClass($driver); + $property = $refl->getProperty('logger'); + $property->setAccessible(true); + $this->assertSame($this->logger, $property->getValue($driver)); + } + + public function testDatabaseManagerWithoutLoggerAndWithLoggerFactoryShouldReturnLoggerFromFactory(): void { $manager = new DatabaseManager( $this->getDatabaseConfig(), @@ -105,7 +123,6 @@ public function testDatabaseManagerWithLoggerAndWithLoggerFactoryShouldReturnLog ->with($this->isInstanceOf(TestDriver::class)) ->willReturn($loggerFromFactory); - $manager->setLogger($this->logger); $driver = $manager->driver('test'); $refl = new \ReflectionClass($driver);