From d3278d5470165bba14ee5026453ec7d529901f42 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Thu, 8 Feb 2018 21:28:32 +0000 Subject: [PATCH] FIX Add Nested DB transaction support (#7848) * TEST Prove nested transactions break * Add nested transaction support --- src/ORM/Connect/MySQLDatabase.php | 37 ++++++++--- tests/php/ORM/TransactionTest.php | 103 +++++++++++++++++++----------- 2 files changed, 95 insertions(+), 45 deletions(-) diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php index 051ab965f..fa3672db5 100644 --- a/src/ORM/Connect/MySQLDatabase.php +++ b/src/ORM/Connect/MySQLDatabase.php @@ -57,6 +57,11 @@ class MySQLDatabase extends Database */ private static $collation = 'utf8_general_ci'; + /** + * @var bool + */ + protected $transactionNesting = 0; + public function connect($parameters) { // Ensure that driver is available (required by PDO) @@ -300,16 +305,21 @@ class MySQLDatabase extends Database public function transactionStart($transactionMode = false, $sessionCharacteristics = false) { - // This sets the isolation level for the NEXT transaction, not the current one. - if ($transactionMode) { - $this->query('SET TRANSACTION ' . $transactionMode); - } + if ($this->transactionNesting > 0) { + $this->transactionSavepoint('NESTEDTRANSACTION' . $this->transactionNesting); + } else { + // This sets the isolation level for the NEXT transaction, not the current one. + if ($transactionMode) { + $this->query('SET TRANSACTION ' . $transactionMode); + } - $this->query('START TRANSACTION'); + $this->query('START TRANSACTION'); - if ($sessionCharacteristics) { - $this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics); + if ($sessionCharacteristics) { + $this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics); + } } + ++$this->transactionNesting; } public function transactionSavepoint($savepoint) @@ -322,13 +332,22 @@ class MySQLDatabase extends Database if ($savepoint) { $this->query('ROLLBACK TO ' . $savepoint); } else { - $this->query('ROLLBACK'); + --$this->transactionNesting; + if ($this->transactionNesting > 0) { + $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting); + } else { + $this->query('ROLLBACK'); + } } } public function transactionEnd($chain = false) { - $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN'); + --$this->transactionNesting; + if ($this->transactionNesting <= 0) { + $this->transactionNesting = 0; + $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN'); + } } public function comparisonClause( diff --git a/tests/php/ORM/TransactionTest.php b/tests/php/ORM/TransactionTest.php index 9c4bc5541..6a0d2b122 100644 --- a/tests/php/ORM/TransactionTest.php +++ b/tests/php/ORM/TransactionTest.php @@ -5,57 +5,88 @@ namespace SilverStripe\ORM\Tests; use SilverStripe\ORM\DB; use SilverStripe\ORM\DataObject; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\Tests\TransactionTest\TestObject; class TransactionTest extends SapphireTest { + protected $usesDatabase = true; - protected static $extra_dataobjects = array( - TransactionTest\TestObject::class - ); + protected static $extra_dataobjects = [ + TransactionTest\TestObject::class, + ]; + + public static function setUpBeforeClass() + { + parent::setUpBeforeClass(); + if (!DB::get_conn()->supportsTransactions()) { + static::markTestSkipped('Current database does not support transactions'); + } + } + + public function testNestedTransaction() + { + $this->assertCount(0, TestObject::get()); + try { + DB::get_conn()->withTransaction(function () { + $obj = TransactionTest\TestObject::create(); + $obj->Title = 'Test'; + $obj->write(); + + $this->assertCount(1, TestObject::get()); + + DB::get_conn()->withTransaction(function () { + $obj = TransactionTest\TestObject::create(); + $obj->Title = 'Test2'; + $obj->write(); + $this->assertCount(2, TestObject::get()); + }); + + throw new \Exception('roll back transaction'); + }); + } catch (\Exception $e) { + $this->assertEquals('roll back transaction', $e->getMessage()); + } + $this->assertCount(0, TestObject::get()); + } public function testCreateWithTransaction() { + DB::get_conn()->transactionStart(); + $obj = new TransactionTest\TestObject(); + $obj->Title = 'First page'; + $obj->write(); - if (DB::get_conn()->supportsTransactions()==true) { - DB::get_conn()->transactionStart(); - $obj=new TransactionTest\TestObject(); - $obj->Title='First page'; - $obj->write(); + $obj = new TransactionTest\TestObject(); + $obj->Title = 'Second page'; + $obj->write(); - $obj=new TransactionTest\TestObject(); - $obj->Title='Second page'; - $obj->write(); + //Create a savepoint here: + DB::get_conn()->transactionSavepoint('rollback'); - //Create a savepoint here: - DB::get_conn()->transactionSavepoint('rollback'); + $obj = new TransactionTest\TestObject(); + $obj->Title = 'Third page'; + $obj->write(); - $obj=new TransactionTest\TestObject(); - $obj->Title='Third page'; - $obj->write(); + $obj = new TransactionTest\TestObject(); + $obj->Title = 'Fourth page'; + $obj->write(); - $obj=new TransactionTest\TestObject(); - $obj->Title='Fourth page'; - $obj->write(); + //Revert to a savepoint: + DB::get_conn()->transactionRollback('rollback'); - //Revert to a savepoint: - DB::get_conn()->transactionRollback('rollback'); + DB::get_conn()->transactionEnd(); - DB::get_conn()->transactionEnd(); + $first = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='First page'"); + $second = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Second page'"); + $third = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Third page'"); + $fourth = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Fourth page'"); - $first=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='First page'"); - $second=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Second page'"); - $third=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Third page'"); - $fourth=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Fourth page'"); + //These pages should be in the system + $this->assertTrue(is_object($first) && $first->exists()); + $this->assertTrue(is_object($second) && $second->exists()); - //These pages should be in the system - $this->assertTrue(is_object($first) && $first->exists()); - $this->assertTrue(is_object($second) && $second->exists()); - - //These pages should NOT exist, we reverted to a savepoint: - $this->assertFalse(is_object($third) && $third->exists()); - $this->assertFalse(is_object($fourth) && $fourth->exists()); - } else { - $this->markTestSkipped('Current database does not support transactions'); - } + //These pages should NOT exist, we reverted to a savepoint: + $this->assertFalse(is_object($third) && $third->exists()); + $this->assertFalse(is_object($fourth) && $fourth->exists()); } }