From 1c83dab189591d0d3e034f2ddd5b822b222825fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 3 Oct 2024 22:00:59 +0200 Subject: [PATCH 1/2] PHPORM-248 register command subscriber only when logs are enabled --- src/Connection.php | 34 +++++++++++++++++++++++++++++++--- tests/ConnectionTest.php | 15 +++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index a76ddc010..fcf932580 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -48,7 +48,7 @@ class Connection extends BaseConnection */ protected $connection; - private ?CommandSubscriber $commandSubscriber; + private ?CommandSubscriber $commandSubscriber = null; /** * Create a new database connection instance. @@ -65,8 +65,6 @@ public function __construct(array $config) // Create the connection $this->connection = $this->createConnection($dsn, $config, $options); - $this->commandSubscriber = new CommandSubscriber($this); - $this->connection->addSubscriber($this->commandSubscriber); // Select database $this->db = $this->connection->selectDatabase($this->getDefaultDatabaseName($dsn, $config)); @@ -141,6 +139,36 @@ public function getDatabaseName() return $this->getMongoDB()->getDatabaseName(); } + public function enableQueryLog() + { + parent::enableQueryLog(); + + $this->commandSubscriber = new CommandSubscriber($this); + $this->connection->addSubscriber($this->commandSubscriber); + } + + public function disableQueryLog() + { + parent::disableQueryLog(); // TODO: Change the autogenerated stub + + $this->connection->removeSubscriber($this->commandSubscriber); + $this->commandSubscriber = null; + } + + protected function withFreshQueryLog($callback) + { + try { + return parent::withFreshQueryLog($callback); + } finally { + // The parent method enable query log using enableQueryLog() + // but disables it by setting $loggingQueries to false. We need to + // remove the subscriber for performance. + if (! $this->loggingQueries) { + $this->disableQueryLog(); + } + } + } + /** * Get the name of the default database based on db config or try to detect it from dsn. * diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 4f9dfa10c..7a3364fb0 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -277,6 +277,21 @@ public function testQueryLog() } } + public function testDisableQueryLog() + { + // Disabled by default + DB::table('items')->get(); + $this->assertCount(0, DB::getQueryLog()); + + DB::enableQueryLog(); + DB::table('items')->get(); + $this->assertCount(1, DB::getQueryLog()); + + DB::disableQueryLog(); + DB::table('items')->get(); + $this->assertCount(1, DB::getQueryLog()); + } + public function testSchemaBuilder() { $schema = DB::connection('mongodb')->getSchemaBuilder(); From 28185389f4f0354937aa122626998c00525f1667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 4 Oct 2024 09:46:40 +0200 Subject: [PATCH 2/2] Fix multiple enable/disable --- src/Connection.php | 17 ++++++++++------- tests/ConnectionTest.php | 12 +++++++++++- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index fcf932580..84ca97aba 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -143,16 +143,20 @@ public function enableQueryLog() { parent::enableQueryLog(); - $this->commandSubscriber = new CommandSubscriber($this); - $this->connection->addSubscriber($this->commandSubscriber); + if (! $this->commandSubscriber) { + $this->commandSubscriber = new CommandSubscriber($this); + $this->connection->addSubscriber($this->commandSubscriber); + } } public function disableQueryLog() { - parent::disableQueryLog(); // TODO: Change the autogenerated stub + parent::disableQueryLog(); - $this->connection->removeSubscriber($this->commandSubscriber); - $this->commandSubscriber = null; + if ($this->commandSubscriber) { + $this->connection->removeSubscriber($this->commandSubscriber); + $this->commandSubscriber = null; + } } protected function withFreshQueryLog($callback) @@ -231,8 +235,7 @@ public function ping(): void /** @inheritdoc */ public function disconnect() { - $this->connection?->removeSubscriber($this->commandSubscriber); - $this->commandSubscriber = null; + $this->disableQueryLog(); $this->connection = null; } diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 7a3364fb0..fe3272943 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -287,9 +287,19 @@ public function testDisableQueryLog() DB::table('items')->get(); $this->assertCount(1, DB::getQueryLog()); + // Enable twice should only log once + DB::enableQueryLog(); + DB::table('items')->get(); + $this->assertCount(2, DB::getQueryLog()); + DB::disableQueryLog(); DB::table('items')->get(); - $this->assertCount(1, DB::getQueryLog()); + $this->assertCount(2, DB::getQueryLog()); + + // Disable twice should not log + DB::disableQueryLog(); + DB::table('items')->get(); + $this->assertCount(2, DB::getQueryLog()); } public function testSchemaBuilder()