From 5ed6910e1cca13c3fea677a564dc13a36533f50c Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Mon, 8 May 2017 10:31:34 +0200 Subject: [PATCH] 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 | 2 +- framework/db/Connection.php | 14 +++++++++ framework/log/DbTarget.php | 4 +-- tests/framework/db/ConnectionTest.php | 41 +++++++++++++++++++++++++-- tests/framework/log/DbTargetTest.php | 23 +++++++-------- 5 files changed, 67 insertions(+), 17 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 50147f8fcf..2a72d435be 100644 --- a/framework/CHANGELOG.md +++ b/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) diff --git a/framework/db/Connection.php b/framework/db/Connection.php index ceebcc93f5..7531644cc1 100644 --- a/framework/db/Connection.php +++ b/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; + } } diff --git a/framework/log/DbTarget.php b/framework/log/DbTarget.php index 922a9cd787..0392f5e402 100644 --- a/framework/log/DbTarget.php +++ b/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); diff --git a/tests/framework/db/ConnectionTest.php b/tests/framework/db/ConnectionTest.php index df694f7e4b..7d6a1121ce 100644 --- a/tests/framework/db/ConnectionTest.php +++ b/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); + } } diff --git a/tests/framework/log/DbTargetTest.php b/tests/framework/log/DbTargetTest.php index 812bf98ea4..26b1e8ae38 100644 --- a/tests/framework/log/DbTargetTest.php +++ b/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); } -} \ No newline at end of file +}