no need for LoggedStatementInterface after all, and increase test cov…

…erage
atlasphp · Dec 31, 2020 · a0749b1 · a0749b1
1 parent bd90206
commit a0749b1
Show file tree

Hide file tree

Showing 8 changed files with 318 additions and 337 deletions.
diff --git a/src/Connection.php b/src/Connection.php
@@ -129,10 +129,16 @@ public function prepare(
         $sth = $this->pdo->prepare($statement, $driverOptions);
 
         if ($this->logQueries && $this->persistent) {
-            $sth = PersistentLoggedStatement::new($sth);
+            $sth = PersistentLoggedStatement::new(
+                $sth,
+                function (array $entry) : void {
+                    $this->addLogEntry($entry);
+                },
+                $this->newLogEntry($statement)
+            );
         }
 
-        if ($sth instanceof LoggedStatementInterface) {
+        if ($sth instanceof LoggedStatement) {
             $sth->setLogEntry($this->newLogEntry($statement));
             $sth->setQueryLogger(function (array $entry) : void {
                 $this->addLogEntry($entry);

diff --git a/src/LoggedStatement.php b/src/LoggedStatement.php
@@ -13,7 +13,7 @@
 use PDO;
 use PDOStatement;
 
-class LoggedStatement extends PDOStatement implements LoggedStatementInterface
+class LoggedStatement extends PDOStatement
 {
     private $logEntry;
 

diff --git a/src/LoggedStatementInterface.php b/src/LoggedStatementInterface.php
diff --git a/src/PersistentLoggedStatement.php b/src/PersistentLoggedStatement.php
@@ -14,18 +14,23 @@
 use PDO;
 use PDOStatement;
 
-class PersistentLoggedStatement extends PDOStatement implements LoggedStatementInterface
+class PersistentLoggedStatement extends PDOStatement
 {
-    private $logEntry;
+    private $parent;
 
     private $queryLogger;
 
-    private $parent;
+    private $logEntry;
 
-    public static function new(PDOStatement $parent = null)
-    {
+    public static function new(
+        PDOStatement $parent,
+        callable $queryLogger,
+        array $logEntry
+    ) {
         $sth = new self();
         $sth->parent = $parent;
+        $sth->queryLogger = $queryLogger;
+        $sth->logEntry = $logEntry;
         return $sth;
     }
 
@@ -66,7 +71,7 @@ public function bindValue(
     {
         $result = $this->parent->bindValue($parameter, $value, $dataType);
 
-        if ($result && $this->logEntry !== null) {
+        if ($result) {
             $this->logEntry['values'][$parameter] = $value;
         }
 
@@ -155,24 +160,8 @@ public function nextRowset()
         return $this->parent->nextRowset();
     }
 
-    /* Logging */
-
-    public function setLogEntry(array $logEntry) : void
-    {
-        $this->logEntry = $logEntry;
-    }
-
-    public function setQueryLogger(callable $queryLogger) : void
-    {
-        $this->queryLogger = $queryLogger;
-    }
-
     private function log($inputParameters) : void
     {
-        if ($this->queryLogger === null || $this->logEntry === null) {
-            return;
-        }
-
         if ($inputParameters !== null) {
             $this->logEntry['values'] = array_replace(
                 $this->logEntry['values'],

diff --git a/tests/ConnectionLocatorTest.php b/tests/ConnectionLocatorTest.php
@@ -236,6 +236,66 @@ public function testQueryLogging()
         $this->assertSame($expect, $labels);
     }
 
+    public function testQueryLoggingOnExistingInstances()
+    {
+        $locator = $this->newLocator($this->read, $this->write);
+        $stm = "SELECT * FROM sqlite_master";
+
+        // create instances before logging is turned on
+        $locator->getDefault();
+        $types = ['read', 'write'];
+        foreach ($types as $type) {
+            for ($i = 1; $i <= 3; $i ++) {
+                $name = $type . $i;
+                $locator->get(strtoupper($type), $name);
+            }
+        }
+
+        // now turn on query logging
+        $locator->logQueries(true);
+
+        // default connection
+        $connection = $locator->getDefault();
+        $sth = $connection->perform($stm);
+        $this->assertInstanceOf(PDOStatement::CLASS, $sth);
+
+        // read and write connections
+        $types = ['read', 'write'];
+        foreach ($types as $type) {
+            for ($i = 1; $i <= 3; $i ++) {
+                $name = $type . $i;
+                $connection = $locator->get(strtoupper($type), $name);
+                $sth = $connection->perform($stm);
+                $this->assertInstanceOf(PDOStatement::CLASS, $sth);
+            }
+        }
+
+        $queries = $locator->getQueries();
+        $this->assertCount(7, $queries);
+
+        $labels = [];
+        foreach ($queries as $query) {
+            $labels[] = $query['connection'];
+            $this->assertTrue($query['start'] > 0);
+            $this->assertTrue($query['finish'] > $query['start']);
+            $this->assertTrue($query['duration'] > 0);
+            $this->assertTrue($query['statement'] == 'SELECT * FROM sqlite_master');
+            $this->assertTrue($query['values'] === []);
+            $this->assertTrue($query['trace'] != '');
+        }
+
+        $expect = [
+            'DEFAULT',
+            'READ:read1',
+            'READ:read2',
+            'READ:read3',
+            'WRITE:write1',
+            'WRITE:write2',
+            'WRITE:write3',
+        ];
+        $this->assertSame($expect, $labels);
+    }
+
     public function testSetQueryLogger()
     {
         $entries = [];

diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php
@@ -55,6 +55,14 @@ public function testFactory()
         $this->assertInstanceOf(Connection::CLASS, $connection);
     }
 
+    public function test__call()
+    {
+        $this->assertSame(
+            'sqlite',
+            $this->connection->getAttribute(PDO::ATTR_DRIVER_NAME)
+        );
+    }
+
     public function testGetDriverName()
     {
         $this->assertSame('sqlite', $this->connection->getDriverName());
@@ -420,22 +428,29 @@ public function testPersistent()
         $persistent->logQueries(true);
 
         // when prepared from native PDO, should be PDOStatement
-        $sth = $persistent->getPdo()->prepare("SELECT id FROM pdotest WHERE id = 0");
+        $sth = $persistent->getPdo()->prepare("SELECT id FROM pdotest WHERE id = :id");
         $this->assertInstanceOf(PDOStatement::CLASS, $sth);
         $this->assertNotInstanceOf(PersistentLoggedStatement::CLASS, $sth);
 
         // should not log, because prepared directly from PDO
-        $this->assertTrue($sth->execute());
+        $this->assertTrue($sth->execute(['id' => 0]));
         $queries = $this->connection->getQueries();
         $this->assertCount(0, $queries);
 
         // when prepared from Connection, should be LoggedStatement
-        $sth = $persistent->prepare("SELECT id FROM pdotest WHERE id = 0");
+        $sth = $persistent->prepare("SELECT id FROM pdotest WHERE id = :id");
         $this->assertInstanceOf(PDOStatement::CLASS, $sth);
         $this->assertInstanceOf(PersistentLoggedStatement::CLASS, $sth);
 
         // should log, because prepared from Connection
-        $this->assertTrue($sth->execute());
+        $this->assertTrue($sth->execute(['id' => 0]));
+        $queries = $persistent->getQueries();
+        $this->assertCount(1, $queries);
+
+        // try again without logging; should still be only 1 query
+        $persistent->logQueries(false);
+        $sth = $persistent->prepare("SELECT id FROM pdotest WHERE id = :id");
+        $this->assertTrue($sth->execute(['id' => 0]));
         $queries = $persistent->getQueries();
         $this->assertCount(1, $queries);
     }