FIX Add Nested DB transaction support (#7848)

* TEST Prove nested transactions break

* Add nested transaction support

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,6 +305,9 @@ class MySQLDatabase extends Database
 
     public function transactionStart($transactionMode = false, $sessionCharacteristics = false)
     {
+        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);
@@ -311,6 +319,8 @@ class MySQLDatabase extends Database
                 $this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics);
             }
         }
+        ++$this->transactionNesting;
+    }
 
     public function transactionSavepoint($savepoint)
     {
@@ -321,15 +331,24 @@ class MySQLDatabase extends Database
     {
         if ($savepoint) {
             $this->query('ROLLBACK TO ' . $savepoint);
+        } else {
+            --$this->transactionNesting;
+            if ($this->transactionNesting > 0) {
+                $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting);
             } else {
                 $this->query('ROLLBACK');
             }
         }
+    }
 
     public function transactionEnd($chain = false)
     {
+        --$this->transactionNesting;
+        if ($this->transactionNesting <= 0) {
+            $this->transactionNesting = 0;
             $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
         }
+    }
 
     public function comparisonClause(
         $field,

tests/php/ORM/TransactionTest.php

@@ -5,36 +5,70 @@ 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(
+    protected static $extra_dataobjects = [
-        TransactionTest\TestObject::class
+        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()
     {
-
-        if (DB::get_conn()->supportsTransactions()==true) {
         DB::get_conn()->transactionStart();
-            $obj=new TransactionTest\TestObject();
+        $obj = new TransactionTest\TestObject();
-            $obj->Title='First page';
+        $obj->Title = 'First page';
         $obj->write();
 
-            $obj=new TransactionTest\TestObject();
+        $obj = new TransactionTest\TestObject();
-            $obj->Title='Second page';
+        $obj->Title = 'Second page';
         $obj->write();
 
         //Create a savepoint here:
         DB::get_conn()->transactionSavepoint('rollback');
 
-            $obj=new TransactionTest\TestObject();
+        $obj = new TransactionTest\TestObject();
-            $obj->Title='Third page';
+        $obj->Title = 'Third page';
         $obj->write();
 
-            $obj=new TransactionTest\TestObject();
+        $obj = new TransactionTest\TestObject();
-            $obj->Title='Fourth page';
+        $obj->Title = 'Fourth page';
         $obj->write();
 
         //Revert to a savepoint:
@@ -42,10 +76,10 @@ class TransactionTest extends SapphireTest
 
         DB::get_conn()->transactionEnd();
 
-            $first=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='First page'");
+        $first = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='First page'");
-            $second=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Second page'");
+        $second = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Second page'");
-            $third=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Third page'");
+        $third = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Third page'");
-            $fourth=DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Fourth page'");
+        $fourth = DataObject::get(TransactionTest\TestObject::class, "\"Title\"='Fourth page'");
 
         //These pages should be in the system
         $this->assertTrue(is_object($first) && $first->exists());
@@ -54,8 +88,5 @@ class TransactionTest extends SapphireTest
         //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');
-        }
     }
 }