From c30a4ee92db5071f030aee40653d2c07398a4e0b Mon Sep 17 00:00:00 2001 From: Temporary Date: Mon, 16 Jan 2017 16:19:57 -0800 Subject: [PATCH 1/4] Altered PdoConnection to allow nested transaction. Created/altered tests to account for this. Fixes #4 --- src/PdoConnection.php | 36 ++++++++++++++++++-- tests/Unit/PdoConnectionTest.php | 57 ++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/PdoConnection.php b/src/PdoConnection.php index 6bafd6e..73f0d07 100644 --- a/src/PdoConnection.php +++ b/src/PdoConnection.php @@ -57,6 +57,16 @@ class PdoConnection */ private $reuseConnection = true; + /** + * @var int Number of open transactions + */ + private $transactionCount = 0; + + /** + * @var bool Flag that tells whether a rollback is needed + */ + private $rollbackNeeded = false; + /** * Creates a new Model object that establishes a new PDO connection using * the given database info, or the default configured info set in the database @@ -228,7 +238,11 @@ public function prepare($sql, $fetchMode = null) */ public function begin() { - return $this->connect()->beginTransaction(); + if (!$this->transactionCount++) { + return $this->connect()->beginTransaction(); + } + + return ($this->transactionCount >= 0); } /** @@ -238,7 +252,13 @@ public function begin() */ public function rollBack() { - return $this->connect()->rollBack(); + if (--$this->transactionCount === 0) { + $this->rollbackNeeded = false; + return $this->connect()->rollBack(); + } + + $this->rollbackNeeded = true; + return false; } /** @@ -248,7 +268,17 @@ public function rollBack() */ public function commit() { - return $this->connect()->commit(); + if (--$this->transactionCount === 0) { + if (!$this->rollbackNeeded) { + return $this->connect()->commit(); + } + + $this->rollbackNeeded = false; + $this->connect()->rollback(); + return false; + } + + return ($this->transactionCount >= 0 && !$this->rollbackNeeded); } /** diff --git a/tests/Unit/PdoConnectionTest.php b/tests/Unit/PdoConnectionTest.php index 2fdffb8..c74c8fd 100644 --- a/tests/Unit/PdoConnectionTest.php +++ b/tests/Unit/PdoConnectionTest.php @@ -140,8 +140,13 @@ public function testBegin() */ public function testRollBack() { + // begin and rollback transaction $this->connection->setConnection($this->mockConnection('rollBack', true)); + $this->connection->begin(); $this->assertTrue($this->connection->rollBack()); + + // Since there is no open transaction, this should return false + $this->assertFalse($this->connection->rollBack()); } /** @@ -153,8 +158,60 @@ public function testRollBack() */ public function testCommit() { + // begin and commit transaction $this->connection->setConnection($this->mockConnection('commit', true)); + $this->connection->begin(); $this->assertTrue($this->connection->commit()); + + // Since there is no open transaction, this should return false + $this->assertFalse($this->connection->commit()); + } + + public function nestedTransactionData() + { + return array( + array(array('commit', 'commit', 'commit'), true), + array(array('rollback', 'rollback', 'rollback'), true), + array(array('commit', 'commit', 'rollback'), true), + array(array('rollback', 'commit', 'rollback'), true), + array(array('rollback', 'commit'), false), + array(array('commit', 'rollback', 'commit'), false) + ); + } + + /** + * @covers ::__construct + * @covers ::begin + * @covers ::commit + * @covers ::rollback + * @covers ::connect + * @covers ::getConnection + * @covers ::setConnection + * @dataProvider nestedTransactionData + * + * @param array $actions + * @param bool $return + */ + public function testNestedTransactions(array $actions, $return) + { + $this->connection->setConnection($this->mockConnection('beginTransaction', true)); + foreach ($actions as $action) { + $this->assertTrue($this->connection->begin()); + } + + end($actions); + $end_index = key($actions); + reset($actions); + foreach ($actions as $index => $action) { + if ($index === $end_index) { + if ($return) { + $this->connection->setConnection($this->mockConnection($action, true)); + } + $this->assertEquals($return, $this->connection->{$action}()); + } else { + $this->connection->{$action}(); + } + } } /** From 6ef6c0a462d9dd6111c5187d7cf368acf779e28e Mon Sep 17 00:00:00 2001 From: Temporary Date: Tue, 17 Jan 2017 10:06:11 -0800 Subject: [PATCH 2/4] Added/removed/updated comments. Changed some formatting. Updated variable name. Restructured PdoConnection::commit and PdoConnectionTest::testNestedTransactions. --- src/PdoConnection.php | 29 +++++++++++++++++------------ tests/Unit/PdoConnectionTest.php | 27 +++++++++++++++------------ 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/PdoConnection.php b/src/PdoConnection.php index 73f0d07..1a126c6 100644 --- a/src/PdoConnection.php +++ b/src/PdoConnection.php @@ -242,16 +242,18 @@ public function begin() return $this->connect()->beginTransaction(); } - return ($this->transactionCount >= 0); + return $this->transactionCount >= 0; } /** - * Rolls back and closes the transaction + * Rolls back and closes the transaction or mark this transaction set for rollback * - * @return boolean True if the transaction was successfully rolled back and closed, false otherwise + * @return boolean True if this is the outermost transaction and it was successfully rolled + * back and closed, false otherwise */ public function rollBack() { + // Rollback if this is the outermost transaction, otherwise mark the set as erroring if (--$this->transactionCount === 0) { $this->rollbackNeeded = false; return $this->connect()->rollBack(); @@ -262,23 +264,26 @@ public function rollBack() } /** - * Commits a transaction + * Commits a transaction or rolls back an erroring transaction set * - * @return boolean True if the transaction was successfully commited and closed, false otherwise + * @return boolean True if the transaction was successfully commited, closed, and this is the outermost + * transaction or this is a non-erroring inner transaction, false otherwise */ public function commit() { + $return = null; if (--$this->transactionCount === 0) { - if (!$this->rollbackNeeded) { - return $this->connect()->commit(); + // Rollback if this transaction set is erroring, commit otherwise + if ($this->rollbackNeeded) { + $this->rollbackNeeded = false; + $this->connect()->rollback(); + $return = false; + } else { + $return = $this->connect()->commit(); } - - $this->rollbackNeeded = false; - $this->connect()->rollback(); - return false; } - return ($this->transactionCount >= 0 && !$this->rollbackNeeded); + return (isset($return) ? $return : $this->transactionCount >= 0 && !$this->rollbackNeeded); } /** diff --git a/tests/Unit/PdoConnectionTest.php b/tests/Unit/PdoConnectionTest.php index c74c8fd..8189b8c 100644 --- a/tests/Unit/PdoConnectionTest.php +++ b/tests/Unit/PdoConnectionTest.php @@ -134,6 +134,7 @@ public function testBegin() /** * @covers ::rollBack * @covers ::__construct + * @covers ::begin * @covers ::getConnection * @covers ::setConnection * @covers ::connect @@ -152,21 +153,25 @@ public function testRollBack() /** * @covers ::commit * @covers ::__construct + * @covers ::begin * @covers ::getConnection * @covers ::setConnection * @covers ::connect */ public function testCommit() { - // begin and commit transaction $this->connection->setConnection($this->mockConnection('commit', true)); $this->connection->begin(); $this->assertTrue($this->connection->commit()); - // Since there is no open transaction, this should return false $this->assertFalse($this->connection->commit()); } + /** + * Passes a list of data sets to pass to testNestedTransacitons + * + * @return array A list of data sets to pass to testNestedTransacitons + */ public function nestedTransactionData() { return array( @@ -199,17 +204,15 @@ public function testNestedTransactions(array $actions, $return) $this->assertTrue($this->connection->begin()); } - end($actions); - $end_index = key($actions); - reset($actions); + $end = count($actions) - 1; foreach ($actions as $index => $action) { - if ($index === $end_index) { - if ($return) { - $this->connection->setConnection($this->mockConnection($action, true)); - } - $this->assertEquals($return, $this->connection->{$action}()); - } else { - $this->connection->{$action}(); + if ($index === $end && $return) { + $this->connection->setConnection($this->mockConnection($action, true)); + } + + $actual = $this->connection->{$action}(); + if ($index === $end) { + $this->assertEquals($return, $actual); } } } From 0e622e4a00b9817428e4aced2e79ad5638af683f Mon Sep 17 00:00:00 2001 From: Temporary Date: Tue, 17 Jan 2017 10:08:16 -0800 Subject: [PATCH 3/4] Removed comments. --- tests/Unit/PdoConnectionTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Unit/PdoConnectionTest.php b/tests/Unit/PdoConnectionTest.php index 8189b8c..63d53be 100644 --- a/tests/Unit/PdoConnectionTest.php +++ b/tests/Unit/PdoConnectionTest.php @@ -141,12 +141,10 @@ public function testBegin() */ public function testRollBack() { - // begin and rollback transaction $this->connection->setConnection($this->mockConnection('rollBack', true)); $this->connection->begin(); $this->assertTrue($this->connection->rollBack()); - // Since there is no open transaction, this should return false $this->assertFalse($this->connection->rollBack()); } From e70746e4779a75ec84df0ea188513fae29c30805 Mon Sep 17 00:00:00 2001 From: Temporary Date: Tue, 17 Jan 2017 10:33:49 -0800 Subject: [PATCH 4/4] Updated PdoConnectionTest::testNestedConnections to include 'begin' as one of the actions given by the data provider and added test cases. --- tests/Unit/PdoConnectionTest.php | 36 ++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/Unit/PdoConnectionTest.php b/tests/Unit/PdoConnectionTest.php index 63d53be..aee05a1 100644 --- a/tests/Unit/PdoConnectionTest.php +++ b/tests/Unit/PdoConnectionTest.php @@ -173,12 +173,19 @@ public function testCommit() public function nestedTransactionData() { return array( - array(array('commit', 'commit', 'commit'), true), - array(array('rollback', 'rollback', 'rollback'), true), - array(array('commit', 'commit', 'rollback'), true), - array(array('rollback', 'commit', 'rollback'), true), - array(array('rollback', 'commit'), false), - array(array('commit', 'rollback', 'commit'), false) + array(array('begin', 'begin', 'begin', 'commit', 'commit', 'commit'), true), + array(array('begin', 'begin', 'begin', 'rollback', 'rollback', 'rollback'), true), + array(array('begin', 'begin', 'begin', 'commit', 'commit', 'rollback'), true), + array(array('begin', 'begin', 'begin', 'rollback', 'commit', 'rollback'), true), + array(array('commit', 'begin', 'begin', 'commit'), true), + array(array('rollback', 'begin', 'begin', 'rollback'), true), + array(array('rollback', 'rollback', 'begin', 'begin'), true), + array(array('commit', 'commit', 'begin', 'begin'), true), + array(array('begin', 'begin', 'rollback', 'commit'), false), + array(array('begin', 'begin', 'begin', 'commit', 'rollback', 'commit'), false), + array(array('rollback', 'begin', 'begin', 'commit'), false), + array(array('begin', 'commit', 'commit'), false), + array(array('begin', 'rollback', 'rollback'), false) ); } @@ -197,15 +204,18 @@ public function nestedTransactionData() */ public function testNestedTransactions(array $actions, $return) { - $this->connection->setConnection($this->mockConnection('beginTransaction', true)); - foreach ($actions as $action) { - $this->assertTrue($this->connection->begin()); - } - + $transactions = 0; $end = count($actions) - 1; foreach ($actions as $index => $action) { - if ($index === $end && $return) { - $this->connection->setConnection($this->mockConnection($action, true)); + if ($action == 'begin') { + if ($transactions++ === 0) { + $this->connection->setConnection($this->mockConnection('beginTransaction', true)); + } + } else { + $transactions--; + if ($index === $end && $return) { + $this->connection->setConnection($this->mockConnection($action, true)); + } } $actual = $this->connection->{$action}();