Added support for cloning a db connection

improved fix #14020
fixes #13890

https://github.com/yiisoft/yii2/pull/14020/files#r115185865

close #14121

framework/CHANGELOG.md

@@ -46,7 +46,7 @@ Yii Framework 2 Change Log
 - Bug #13790: Fixed error in `\yii\widgets\MaskedInput` JavaScript by raising version required (samdark)
 - Bug #13807: Fixed `yii\db\QueryBuilder` to inherit subquery params when building a `INSERT INTO ... SELECT` query (sergeymakinen)
 - Bug #13848: `yii\di\Instance::ensure()` wasn't throwing an exception when `$type` is specified and `$reference` object isn't instance of `$type` (c-jonua)
-- Bug #13890: DbTarget log transaction bug (shirase)
+- Bug #13890: `yii\log\DbTarget` log messages where not written when a database transaction was rolled back, added support for cloning a `yii\db\Connection` (shirase, cebe)
 - Bug #13901: Fixed passing unused parameter to `formatMessage()` call in `\yii\validators\IpValidator` (Kolyunya)
 - Bug #13961: Fixed `unserialize()` error during RBAC rule retrieving from PostgreSQL DBMS (vsguts)
 - Bug #14012: `yii\db\pgsql\Schema::findViewNames()` was skipping materialized views (insolita)

framework/db/Connection.php

@@ -1080,4 +1080,18 @@ class Connection extends Component
         $this->close();
         return array_keys((array) $this);
     }
+
+    /**
+     * Reset the connection after cloning.
+     */
+    public function __clone()
+    {
+        parent::__clone();
+
+        $this->_master = false;
+        $this->_slave = false;
+        $this->pdo = null;
+        $this->_schema = null;
+        $this->_transaction = null;
+    }
 }

framework/log/DbTarget.php

@@ -61,9 +61,9 @@ class DbTarget extends Target
     public function export()
     {
         if ($this->db->getTransaction()) {
+            // create new database connection, if there is an open transaction
+            // to ensure insert statement is not affected by a rollback
             $this->db = clone $this->db;
-            $this->db->pdo = null;
-            $this->db->open();
         }
 
         $tableName = $this->db->quoteTableName($this->logTable);

tests/framework/db/ConnectionTest.php

@@ -316,7 +316,7 @@ abstract class ConnectionTest extends DatabaseTestCase
         $thrown = false;
         try {
             $connection->createCommand('INSERT INTO qlog1(a) VALUES(:a);', [':a' => 1])->execute();
-        } catch(\yii\db\Exception $e) {
+        } catch (\yii\db\Exception $e) {
             $this->assertContains('INSERT INTO qlog1(a) VALUES(1);', $e->getMessage(), 'Exception message should contain raw SQL query: ' . (string)$e);
             $thrown = true;
         }
@@ -325,10 +325,47 @@ abstract class ConnectionTest extends DatabaseTestCase
         $thrown = false;
         try {
             $connection->createCommand('SELECT * FROM qlog1 WHERE id=:a ORDER BY nonexistingcolumn;', [':a' => 1])->queryAll();
-        } catch(\yii\db\Exception $e) {
+        } catch (\yii\db\Exception $e) {
             $this->assertContains('SELECT * FROM qlog1 WHERE id=1 ORDER BY nonexistingcolumn;', $e->getMessage(), 'Exception message should contain raw SQL query: ' . (string)$e);
             $thrown = true;
         }
         $this->assertTrue($thrown, 'An exception should have been thrown by the command.');
     }
+
+    /**
+     * Ensure database connection is reset on when a connection is cloned.
+     * Make sure each connection element has its own PDO instance i.e. own connection to the DB.
+     * Also transaction elements should not be shared between two connections.
+     */
+    public function testClone()
+    {
+        $connection = $this->getConnection(true, false);
+        $this->assertNull($connection->transaction);
+        $this->assertNull($connection->pdo);
+        $connection->open();
+        $this->assertNull($connection->transaction);
+        $this->assertNotNull($connection->pdo);
+
+        $conn2 = clone $connection;
+        $this->assertNull($connection->transaction);
+        $this->assertNotNull($connection->pdo);
+
+        $this->assertNull($conn2->transaction);
+        $this->assertNull($conn2->pdo);
+
+        $connection->beginTransaction();
+
+        $this->assertNotNull($connection->transaction);
+        $this->assertNotNull($connection->pdo);
+
+        $this->assertNull($conn2->transaction);
+        $this->assertNull($conn2->pdo);
+
+        $conn3 = clone $connection;
+
+        $this->assertNotNull($connection->transaction);
+        $this->assertNotNull($connection->pdo);
+        $this->assertNull($conn3->transaction);
+        $this->assertNull($conn3->pdo);
+    }
 }

tests/framework/log/DbTargetTest.php

@@ -37,7 +37,7 @@ abstract class DbTargetTest extends TestCase
                     'db' => static::getConnection(),
                     'log' => [
                         'targets' => [
-                            [
+                            'db' => [
                                 'class' => 'yii\log\DbTarget',
                                 'levels' => ['warning'],
                                 'logTable' => self::$logTable,
@@ -58,9 +58,9 @@ abstract class DbTargetTest extends TestCase
         }
     }
 
-    public static function setUpBeforeClass()
+    public function setUp()
     {
-        parent::setUpBeforeClass();
+        parent::setUp();
         $databases = static::getParam('databases');
         static::$database = $databases[static::$driverName];
         $pdo_database = 'pdo_' . static::$driverName;
@@ -72,20 +72,14 @@ abstract class DbTargetTest extends TestCase
         static::runConsoleAction('migrate/up', ['migrationPath' => '@yii/log/migrations/', 'interactive' => false]);
     }
 
-    public static function tearDownAfterClass()
+    public function tearDown()
     {
+        self::getConnection()->createCommand()->truncateTable(self::$logTable)->execute();
         static::runConsoleAction('migrate/down', ['migrationPath' => '@yii/log/migrations/', 'interactive' => false]);
         if (static::$db) {
             static::$db->close();
         }
-        Yii::$app = null;
-        parent::tearDownAfterClass();
-    }
-
-    protected function tearDown()
-    {
         parent::tearDown();
-        self::getConnection()->createCommand()->truncateTable(self::$logTable)->execute();
     }
 
     /**
@@ -159,10 +153,15 @@ abstract class DbTargetTest extends TestCase
         $logger->messages[] = $messsageData;
         $logger->flush(true);
 
+        // current db connection should still have a transaction
+        $this->assertNotNull($db->transaction);
+        // log db connection should not have transaction
+        $this->assertNull(Yii::$app->log->targets['db']->db->transaction);
+
         $tx->rollBack();
 
         $query = (new Query())->select('COUNT(*)')->from(self::$logTable)->where(['category' => 'test', 'message' => 'test']);
         $count = $query->createCommand($db)->queryScalar();
         static::assertEquals(1, $count);
     }
-}
+}