From 95bcac796a3bcb65e7d9fc55e0af0b9d17407938 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Mon, 18 Jun 2018 14:44:31 +1200
Subject: [PATCH 1/8] ENHANCEMENT Ensure test DB is flushed on either DDL or
 transaction-disabled tests Fixes #8182

---
 src/Dev/SapphireTest.php           | 18 +++++++-
 src/Dev/State/FixtureTestState.php | 66 +++++++++++++++++++++---------
 src/ORM/Connect/Database.php       | 22 ++++++----
 src/ORM/Connect/MySQLDatabase.php  | 62 ++++++++++++++++++++++++----
 src/ORM/Connect/TempDatabase.php   | 25 ++++++++---
 5 files changed, 152 insertions(+), 41 deletions(-)

diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php
index c4c39ddd4..988f7f087 100644
--- a/src/Dev/SapphireTest.php
+++ b/src/Dev/SapphireTest.php
@@ -78,6 +78,14 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
      */
     protected $usesDatabase = null;
 
+    /**
+     * This test will cleanup its state via transactions.
+     * If set to false a full schema is forced between tests, but at a performance cost.
+     *
+     * @var bool
+     */
+    protected $usesTransactions = true;
+
     /**
      * @var bool
      */
@@ -228,6 +236,14 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
         return $this->usesDatabase;
     }
 
+    /**
+     * @return bool
+     */
+    public function getUsesTransactions()
+    {
+        return $this->usesTransactions;
+    }
+
     /**
      * @return array
      */
@@ -1188,7 +1204,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
         if (strpos($fixtureFilePath, ':') !== false) {
             return ModuleResourceLoader::singleton()->resolvePath($fixtureFilePath);
         }
-        
+
         // Support fixture paths relative to the test class, rather than relative to webroot
         // String checking is faster than file_exists() calls.
         $resolvedPath = realpath($this->getCurrentAbsolutePath() . '/' . $fixtureFilePath);
diff --git a/src/Dev/State/FixtureTestState.php b/src/Dev/State/FixtureTestState.php
index ccbe67f1a..0272ebfc4 100644
--- a/src/Dev/State/FixtureTestState.php
+++ b/src/Dev/State/FixtureTestState.php
@@ -26,25 +26,31 @@ class FixtureTestState implements TestState
      */
     public function setUp(SapphireTest $test)
     {
-        if ($this->testNeedsDB($test)) {
-            $tmpDB = $test::tempDB();
-            if (!$tmpDB->isUsed()) {
-                $tmpDB->build();
-            }
-            DataObject::singleton()->flushCache();
+        if (!$this->testNeedsDB($test)) {
+            return;
+        }
+        $tmpDB = $test::tempDB();
+        if (!$tmpDB->isUsed()) {
+            $tmpDB->build();
+        }
+        DataObject::singleton()->flushCache();
 
-            if (!$tmpDB->hasStarted()) {
-                foreach ($test->getRequireDefaultRecordsFrom() as $className) {
-                    $instance = singleton($className);
-                    if (method_exists($instance, 'requireDefaultRecords')) {
-                        $instance->requireDefaultRecords();
-                    }
-                    if (method_exists($instance, 'augmentDefaultRecords')) {
-                        $instance->augmentDefaultRecords();
-                    }
+        // Ensure DB is built and populated
+        if (!$tmpDB->hasStarted()) {
+            foreach ($test->getRequireDefaultRecordsFrom() as $className) {
+                $instance = singleton($className);
+                if (method_exists($instance, 'requireDefaultRecords')) {
+                    $instance->requireDefaultRecords();
+                }
+                if (method_exists($instance, 'augmentDefaultRecords')) {
+                    $instance->augmentDefaultRecords();
                 }
-                $this->loadFixtures($test);
             }
+            $this->loadFixtures($test);
+        }
+
+        // Begin transactions if enabled
+        if ($test->getUsesTransactions()) {
             $tmpDB->startTransaction();
         }
     }
@@ -56,9 +62,21 @@ class FixtureTestState implements TestState
      */
     public function tearDown(SapphireTest $test)
     {
-        if ($this->testNeedsDB($test)) {
-            $test::tempDB()->rollbackTransaction();
+        if (!$this->testNeedsDB($test)) {
+            return;
         }
+
+        // For transactional states, rollback if possible
+        if ($test->getUsesTransactions()) {
+            $success = $test::tempDB()->rollbackTransaction();
+            if ($success) {
+                return;
+            }
+        }
+
+        // Force reset if transaction failed, or disabled
+        $test::tempDB()->kill();
+        $this->resetFixtureFactory(get_class($test));
     }
 
     /**
@@ -68,7 +86,7 @@ class FixtureTestState implements TestState
      */
     public function setUpOnce($class)
     {
-        $this->fixtureFactories[strtolower($class)] = Injector::inst()->create(FixtureFactory::class);
+        $this->resetFixtureFactory($class);
     }
 
     /**
@@ -220,4 +238,14 @@ class FixtureTestState implements TestState
 
         return false;
     }
+
+    /**
+     * Bootstrap a clean fixture factory for the given class
+     *
+     * @param string $class
+     */
+    protected function resetFixtureFactory($class)
+    {
+        $this->fixtureFactories[strtolower($class)] = Injector::inst()->create(FixtureFactory::class);
+    }
 }
diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php
index 3e2910522..add4d6948 100644
--- a/src/ORM/Connect/Database.php
+++ b/src/ORM/Connect/Database.php
@@ -654,13 +654,17 @@ abstract class Database
      *
      * @param string|boolean $savepoint Name of savepoint, or leave empty to rollback
      * to last savepoint
+     * @return bool|null Boolean is returned if success state is known, or null if
+     * unknown. Note: For error checking purposes null should not be treated as error.
      */
     abstract public function transactionRollback($savepoint = false);
 
     /**
      * Commit everything inside this transaction so far
      *
-     * @param boolean $chain
+     * @param bool $chain
+     * @return bool|null Boolean is returned if success state is known, or null if
+     * unknown. Note: For error checking purposes null should not be treated as error.
      */
     abstract public function transactionEnd($chain = false);
 
@@ -669,7 +673,7 @@ abstract class Database
      * which is different from table- or row-level locking.
      * See {@link getLock()} for details.
      *
-     * @return boolean Flag indicating that locking is available
+     * @return bool Flag indicating that locking is available
      */
     public function supportsLocks()
     {
@@ -681,7 +685,7 @@ abstract class Database
      * See {@link supportsLocks()} to check if locking is generally supported.
      *
      * @param string $name Name of the lock
-     * @return boolean
+     * @return bool
      */
     public function canLock($name)
     {
@@ -703,7 +707,7 @@ abstract class Database
      *
      * @param string $name Name of lock
      * @param integer $timeout Timeout in seconds
-     * @return boolean
+     * @return bool
      */
     public function getLock($name, $timeout = 5)
     {
@@ -715,7 +719,7 @@ abstract class Database
      * (if the execution aborts (e.g. due to an error) all locks are automatically released).
      *
      * @param string $name Name of the lock
-     * @return boolean Flag indicating whether the lock was successfully released
+     * @return bool Flag indicating whether the lock was successfully released
      */
     public function releaseLock($name)
     {
@@ -756,7 +760,7 @@ abstract class Database
      * Determine if the database with the specified name exists
      *
      * @param string $name Name of the database to check for
-     * @return boolean Flag indicating whether this database exists
+     * @return bool Flag indicating whether this database exists
      */
     public function databaseExists($name)
     {
@@ -778,12 +782,12 @@ abstract class Database
      * database if it doesn't exist in the current schema.
      *
      * @param string $name Name of the database
-     * @param boolean $create Flag indicating whether the database should be created
+     * @param bool $create Flag indicating whether the database should be created
      * if it doesn't exist. If $create is false and the database doesn't exist
      * then an error will be raised
-     * @param int|boolean $errorLevel The level of error reporting to enable for the query, or false if no error
+     * @param int|bool $errorLevel The level of error reporting to enable for the query, or false if no error
      * should be raised
-     * @return boolean Flag indicating success
+     * @return bool Flag indicating success
      */
     public function selectDatabase($name, $create = false, $errorLevel = E_USER_ERROR)
     {
diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php
index fa3672db5..59c0c2b6c 100644
--- a/src/ORM/Connect/MySQLDatabase.php
+++ b/src/ORM/Connect/MySQLDatabase.php
@@ -329,25 +329,73 @@ class MySQLDatabase extends Database
 
     public function transactionRollback($savepoint = false)
     {
+        // Named transaction
         if ($savepoint) {
             $this->query('ROLLBACK TO ' . $savepoint);
-        } else {
-            --$this->transactionNesting;
-            if ($this->transactionNesting > 0) {
-                $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting);
-            } else {
-                $this->query('ROLLBACK');
-            }
+            return true;
         }
+
+        // Fail if transaction isn't available
+        if (!$this->transactionNesting) {
+            return false;
+        }
+        --$this->transactionNesting;
+        if ($this->transactionNesting > 0) {
+            $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting);
+        } else {
+            $this->query('ROLLBACK');
+        }
+        return true;
     }
 
     public function transactionEnd($chain = false)
     {
+        // Fail if transaction isn't available
+        if (!$this->transactionNesting) {
+            return false;
+        }
         --$this->transactionNesting;
         if ($this->transactionNesting <= 0) {
             $this->transactionNesting = 0;
             $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
         }
+        return true;
+    }
+
+    /**
+     * In error condition, set transactionNesting to zero
+     */
+    protected function discardTransactions()
+    {
+        $this->transactionNesting = 0;
+    }
+
+    public function query($sql, $errorLevel = E_USER_ERROR)
+    {
+        $this->inspectQuery($sql);
+        return parent::query($sql, $errorLevel);
+    }
+
+    public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR)
+    {
+        $this->inspectQuery($sql);
+        return parent::preparedQuery($sql, $parameters, $errorLevel);
+    }
+
+    /**
+     * Inspect a SQL query prior to execution
+     *
+     * @param string $sql
+     */
+    protected function inspectQuery($sql)
+    {
+        // Any DDL discards transactions.
+        // See https://dev.mysql.com/doc/internals/en/transactions-notes-on-ddl-and-normal-transaction.html
+        // on why we need to be over-eager
+        $isDDL = $this->getConnector()->isQueryDDL($sql);
+        if ($isDDL) {
+            $this->discardTransactions();
+        }
     }
 
     public function comparisonClause(
diff --git a/src/ORM/Connect/TempDatabase.php b/src/ORM/Connect/TempDatabase.php
index 832acd606..b1fbecddf 100644
--- a/src/ORM/Connect/TempDatabase.php
+++ b/src/ORM/Connect/TempDatabase.php
@@ -102,15 +102,28 @@ class TempDatabase
 
     /**
      * Rollback a transaction (or trash all data if the DB doesn't support databases
+     *
+     * @return bool True if successfully rolled back, false otherwise. On error the DB is
+     * killed and must be re-created. Note that calling rollbackTransaction() when there
+     * is no transaction is counted as a failure, and will kill the DB.
      */
     public function rollbackTransaction()
     {
-        if (static::getConn()->supportsTransactions()) {
-            static::getConn()->transactionRollback();
-        } else {
-            $this->hasStarted = false;
-            static::clearAllData();
+        $success = $this->hasStarted() && static::getConn()->supportsTransactions();
+        if ($success) {
+            try {
+                // Explicit false = gnostic error from transactionRollback
+                if (static::getConn()->transactionRollback() === false) {
+                    $success = false;
+                }
+            } catch (DatabaseException $ex) {
+                $success = false;
+            }
         }
+        if (!$success) {
+            static::kill();
+        }
+        return $success;
     }
 
     /**
@@ -118,6 +131,8 @@ class TempDatabase
      */
     public function kill()
     {
+        $this->hasStarted = false;
+
         // Delete our temporary database
         if (!$this->isUsed()) {
             return;

From 11e0a3de43d7e05b7df63d52b0238e58baff3c2a Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Mon, 18 Jun 2018 17:34:05 +1200
Subject: [PATCH 2/8] BUG Ensure that build includes extra classes

---
 src/Dev/State/FixtureTestState.php | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/Dev/State/FixtureTestState.php b/src/Dev/State/FixtureTestState.php
index 0272ebfc4..a4df4d513 100644
--- a/src/Dev/State/FixtureTestState.php
+++ b/src/Dev/State/FixtureTestState.php
@@ -31,7 +31,14 @@ class FixtureTestState implements TestState
         }
         $tmpDB = $test::tempDB();
         if (!$tmpDB->isUsed()) {
+            // Build base db
             $tmpDB->build();
+
+            // Reset schema
+            $extraObjects = $test->getExtraDataObjects();
+            if ($extraObjects) {
+                $tmpDB->resetDBSchema($extraObjects);
+            }
         }
         DataObject::singleton()->flushCache();
 

From 225e61dc67f5ab550d41c42fa8c80b50b71db5b1 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Mon, 18 Jun 2018 17:56:42 +1200
Subject: [PATCH 3/8] BUG FIx manual resetDBSchema() calls breaking the
 database

---
 src/ORM/Connect/TempDatabase.php | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/ORM/Connect/TempDatabase.php b/src/ORM/Connect/TempDatabase.php
index b1fbecddf..ce9065bbb 100644
--- a/src/ORM/Connect/TempDatabase.php
+++ b/src/ORM/Connect/TempDatabase.php
@@ -105,7 +105,8 @@ class TempDatabase
      *
      * @return bool True if successfully rolled back, false otherwise. On error the DB is
      * killed and must be re-created. Note that calling rollbackTransaction() when there
-     * is no transaction is counted as a failure, and will kill the DB.
+     * is no transaction is counted as a failure, user code should either kill or flush the DB
+     * as necessary
      */
     public function rollbackTransaction()
     {
@@ -120,9 +121,6 @@ class TempDatabase
                 $success = false;
             }
         }
-        if (!$success) {
-            static::kill();
-        }
         return $success;
     }
 
@@ -247,7 +245,12 @@ class TempDatabase
         // pgsql doesn't allow schema updates inside transactions
         // so we need to rollback any transactions before commencing a schema reset
         if ($this->hasStarted()) {
-            $this->rollbackTransaction();
+            // Sometimes transactions fail, rebuild
+            $success = $this->rollbackTransaction();
+            if (!$success) {
+                $this->kill();
+                $this->build();
+            }
         }
         if (!$this->isUsed()) {
             return;

From 8ea3bb36a0d599c85e66b4441bc862b5e7c98d02 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Tue, 19 Jun 2018 16:16:50 +1200
Subject: [PATCH 4/8] Maybe fix it?

---
 src/Dev/State/FixtureTestState.php |  26 +++++-
 src/ORM/Connect/Database.php       |  14 ++++
 src/ORM/Connect/MySQLDatabase.php  |   9 ++-
 src/ORM/Connect/TempDatabase.php   | 126 ++++++++++++++---------------
 tests/php/Security/GroupTest.php   |   5 ++
 5 files changed, 113 insertions(+), 67 deletions(-)

diff --git a/src/Dev/State/FixtureTestState.php b/src/Dev/State/FixtureTestState.php
index a4df4d513..ec5a32d32 100644
--- a/src/Dev/State/FixtureTestState.php
+++ b/src/Dev/State/FixtureTestState.php
@@ -19,6 +19,13 @@ class FixtureTestState implements TestState
      */
     private $fixtureFactories = [];
 
+    /**
+     * Set if fixtures have been loaded
+     *
+     * @var bool
+     */
+    protected $loaded = [];
+
     /**
      * Called on setup
      *
@@ -29,6 +36,8 @@ class FixtureTestState implements TestState
         if (!$this->testNeedsDB($test)) {
             return;
         }
+
+        // Ensure DB is built
         $tmpDB = $test::tempDB();
         if (!$tmpDB->isUsed()) {
             // Build base db
@@ -40,10 +49,11 @@ class FixtureTestState implements TestState
                 $tmpDB->resetDBSchema($extraObjects);
             }
         }
+
         DataObject::singleton()->flushCache();
 
         // Ensure DB is built and populated
-        if (!$tmpDB->hasStarted()) {
+        if (!$this->getIsLoaded(get_class($test))) {
             foreach ($test->getRequireDefaultRecordsFrom() as $className) {
                 $instance = singleton($className);
                 if (method_exists($instance, 'requireDefaultRecords')) {
@@ -155,6 +165,8 @@ class FixtureTestState implements TestState
         foreach ($paths as $fixtureFile) {
             $this->loadFixture($fixtureFile, $test);
         }
+        // Flag as loaded
+        $this->loaded[get_class($test)] = true;
     }
 
     /**
@@ -254,5 +266,17 @@ class FixtureTestState implements TestState
     protected function resetFixtureFactory($class)
     {
         $this->fixtureFactories[strtolower($class)] = Injector::inst()->create(FixtureFactory::class);
+        $this->loaded[$class] = false;
+    }
+
+    /**
+     * Check if fixtures need to be loaded for this class
+     *
+     * @param string $class Name of test to check
+     * @return bool
+     */
+    protected function getIsLoaded($class)
+    {
+        return !empty($this->loaded[$class]);
     }
 }
diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php
index add4d6948..4c60ed085 100644
--- a/src/ORM/Connect/Database.php
+++ b/src/ORM/Connect/Database.php
@@ -668,6 +668,20 @@ abstract class Database
      */
     abstract public function transactionEnd($chain = false);
 
+    /**
+     * Return depth of current transaction
+     *
+     * @return int Nesting level, or 0 if not in a transaction
+     */
+    public function transactionDepth()
+    {
+        // Placeholder error for transactional DBs that don't expose depth
+        if ($this->supportsTransactions()) {
+            user_error(get_class($this) . " does not support transactionDepth", E_USER_WARNING);
+        }
+        return 0;
+    }
+
     /**
      * Determines if the used database supports application-level locks,
      * which is different from table- or row-level locking.
diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php
index 59c0c2b6c..49f6dd62d 100644
--- a/src/ORM/Connect/MySQLDatabase.php
+++ b/src/ORM/Connect/MySQLDatabase.php
@@ -348,6 +348,11 @@ class MySQLDatabase extends Database
         return true;
     }
 
+    public function transactionDepth()
+    {
+        return $this->transactionNesting;
+    }
+
     public function transactionEnd($chain = false)
     {
         // Fail if transaction isn't available
@@ -365,7 +370,7 @@ class MySQLDatabase extends Database
     /**
      * In error condition, set transactionNesting to zero
      */
-    protected function discardTransactions()
+    protected function resetTransactionNesting()
     {
         $this->transactionNesting = 0;
     }
@@ -394,7 +399,7 @@ class MySQLDatabase extends Database
         // on why we need to be over-eager
         $isDDL = $this->getConnector()->isQueryDDL($sql);
         if ($isDDL) {
-            $this->discardTransactions();
+            $this->resetTransactionNesting();
         }
     }
 
diff --git a/src/ORM/Connect/TempDatabase.php b/src/ORM/Connect/TempDatabase.php
index ce9065bbb..dce37959f 100644
--- a/src/ORM/Connect/TempDatabase.php
+++ b/src/ORM/Connect/TempDatabase.php
@@ -23,11 +23,6 @@ class TempDatabase
      */
     protected $name = null;
 
-    /**
-     * @var bool If a transaction has been started
-     */
-    protected $hasStarted = false;
-
     /**
      * Create a new temp database
      *
@@ -73,14 +68,6 @@ class TempDatabase
         return $this->isDBTemp($selected);
     }
 
-    /**
-     * @return bool
-     */
-    public function hasStarted()
-    {
-        return $this->hasStarted;
-    }
-
     /**
      * @return bool
      */
@@ -94,7 +81,6 @@ class TempDatabase
      */
     public function startTransaction()
     {
-        $this->hasStarted = true;
         if (static::getConn()->supportsTransactions()) {
             static::getConn()->transactionStart();
         }
@@ -110,18 +96,21 @@ class TempDatabase
      */
     public function rollbackTransaction()
     {
-        $success = $this->hasStarted() && static::getConn()->supportsTransactions();
-        if ($success) {
-            try {
-                // Explicit false = gnostic error from transactionRollback
-                if (static::getConn()->transactionRollback() === false) {
-                    $success = false;
-                }
-            } catch (DatabaseException $ex) {
-                $success = false;
-            }
+        // Ensure a rollback can be performed
+        $success = static::getConn()->supportsTransactions()
+            && static::getConn()->transactionDepth();
+        if (!$success) {
+            return false;
+        }
+        try {
+            // Explicit false = gnostic error from transactionRollback
+            if (static::getConn()->transactionRollback() === false) {
+                return false;
+            }
+            return true;
+        } catch (DatabaseException $ex) {
+            return false;
         }
-        return $success;
     }
 
     /**
@@ -129,13 +118,14 @@ class TempDatabase
      */
     public function kill()
     {
-        $this->hasStarted = false;
-
-        // Delete our temporary database
+        // Nothing to kill
         if (!$this->isUsed()) {
             return;
         }
 
+        // Rollback any transactions (note: Success ignored)
+        $this->rollbackTransaction();
+
         // Check the database actually exists
         $dbConn = $this->getConn();
         $dbName = $dbConn->getSelectedDatabase();
@@ -160,7 +150,6 @@ class TempDatabase
      */
     public function clearAllData()
     {
-        $this->hasStarted = false;
         if (!$this->isUsed()) {
             return;
         }
@@ -218,44 +207,12 @@ class TempDatabase
     }
 
     /**
-     * Clear all temp DBs on this connection
+     * Rebuild all database tables
      *
-     * Note: This will output results to stdout unless suppressOutput
-     * is set on the current db schema
+     * @param array $extraDataObjects
      */
-    public function deleteAll()
+    protected function rebuildTables($extraDataObjects = [])
     {
-        $schema = $this->getConn()->getSchemaManager();
-        foreach ($schema->databaseList() as $dbName) {
-            if ($this->isDBTemp($dbName)) {
-                $schema->dropDatabase($dbName);
-                $schema->alterationMessage("Dropped database \"$dbName\"", 'deleted');
-                flush();
-            }
-        }
-    }
-
-    /**
-     * Reset the testing database's schema.
-     *
-     * @param array $extraDataObjects List of extra dataobjects to build
-     */
-    public function resetDBSchema(array $extraDataObjects = [])
-    {
-        // pgsql doesn't allow schema updates inside transactions
-        // so we need to rollback any transactions before commencing a schema reset
-        if ($this->hasStarted()) {
-            // Sometimes transactions fail, rebuild
-            $success = $this->rollbackTransaction();
-            if (!$success) {
-                $this->kill();
-                $this->build();
-            }
-        }
-        if (!$this->isUsed()) {
-            return;
-        }
-
         DataObject::reset();
 
         // clear singletons, they're caching old extension info which is used in DatabaseAdmin->doBuild()
@@ -291,4 +248,45 @@ class TempDatabase
         ClassInfo::reset_db_cache();
         DataObject::singleton()->flushCache();
     }
+
+    /**
+     * Clear all temp DBs on this connection
+     *
+     * Note: This will output results to stdout unless suppressOutput
+     * is set on the current db schema
+     */
+    public function deleteAll()
+    {
+        $schema = $this->getConn()->getSchemaManager();
+        foreach ($schema->databaseList() as $dbName) {
+            if ($this->isDBTemp($dbName)) {
+                $schema->dropDatabase($dbName);
+                $schema->alterationMessage("Dropped database \"$dbName\"", 'deleted');
+                flush();
+            }
+        }
+    }
+
+    /**
+     * Reset the testing database's schema.
+     *
+     * @param array $extraDataObjects List of extra dataobjects to build
+     */
+    public function resetDBSchema(array $extraDataObjects = [])
+    {
+        // Skip if no DB
+        if (!$this->isUsed()) {
+            return;
+        }
+
+        try {
+            $this->rebuildTables($extraDataObjects);
+        } catch (DatabaseException $ex) {
+            // In case of error during build force a hard reset
+            // e.g. pgsql doesn't allow schema updates inside transactions
+            $this->kill();
+            $this->build();
+            $this->rebuildTables($extraDataObjects);
+        }
+    }
 }
diff --git a/tests/php/Security/GroupTest.php b/tests/php/Security/GroupTest.php
index 163257602..076e04584 100644
--- a/tests/php/Security/GroupTest.php
+++ b/tests/php/Security/GroupTest.php
@@ -20,6 +20,11 @@ class GroupTest extends FunctionalTest
         TestMember::class
     ];
 
+    protected function setUp()
+    {
+        parent::setUp();
+    }
+
     public function testGroupCodeDefaultsToTitle()
     {
         $g1 = new Group();

From 1d06dcdd28feb0c6a215bd55aa90cfc0fd4f3a4e Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Tue, 19 Jun 2018 17:00:23 +1200
Subject: [PATCH 5/8] Use 2.1 of postgres

---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 70e8be4a5..25b1b1b91 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -67,7 +67,7 @@ before_script:
 # Install composer dependencies
   - composer validate
   - mkdir ./public
-  - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.0.x-dev --no-update; fi
+  - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.1.x-dev --no-update; fi
   - if [[ $DB == SQLITE ]]; then composer require silverstripe/sqlite3:2.0.x-dev --no-update; fi
   - composer require silverstripe/recipe-testing:^1 silverstripe/recipe-core:4.2.x-dev silverstripe/admin:1.2.x-dev silverstripe/versioned:1.2.x-dev --no-update
   - if [[ $PHPUNIT_TEST == cms ]]; then composer require silverstripe/recipe-cms:4.2.x-dev --no-update; fi

From cbdf547c1b0f891f378e8d2edc6fb51b356f0edc Mon Sep 17 00:00:00 2001
From: Daniel Hensby <git@hens.by>
Date: Tue, 19 Jun 2018 18:31:59 +0100
Subject: [PATCH 6/8] Address feedback

---
 src/Dev/State/FixtureTestState.php | 7 ++++---
 src/ORM/Connect/Database.php       | 2 +-
 src/ORM/Connect/MySQLDatabase.php  | 5 +----
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/Dev/State/FixtureTestState.php b/src/Dev/State/FixtureTestState.php
index ec5a32d32..80ac22e4f 100644
--- a/src/Dev/State/FixtureTestState.php
+++ b/src/Dev/State/FixtureTestState.php
@@ -166,7 +166,7 @@ class FixtureTestState implements TestState
             $this->loadFixture($fixtureFile, $test);
         }
         // Flag as loaded
-        $this->loaded[get_class($test)] = true;
+        $this->loaded[strtolower(get_class($test))] = true;
     }
 
     /**
@@ -265,7 +265,8 @@ class FixtureTestState implements TestState
      */
     protected function resetFixtureFactory($class)
     {
-        $this->fixtureFactories[strtolower($class)] = Injector::inst()->create(FixtureFactory::class);
+        $class = strtolower($class);
+        $this->fixtureFactories[$class] = Injector::inst()->create(FixtureFactory::class);
         $this->loaded[$class] = false;
     }
 
@@ -277,6 +278,6 @@ class FixtureTestState implements TestState
      */
     protected function getIsLoaded($class)
     {
-        return !empty($this->loaded[$class]);
+        return !empty($this->loaded[strtolower($class)]);
     }
 }
diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php
index 4c60ed085..670b07b5b 100644
--- a/src/ORM/Connect/Database.php
+++ b/src/ORM/Connect/Database.php
@@ -676,7 +676,7 @@ abstract class Database
     public function transactionDepth()
     {
         // Placeholder error for transactional DBs that don't expose depth
-        if ($this->supportsTransactions()) {
+        if (!$this->supportsTransactions()) {
             user_error(get_class($this) . " does not support transactionDepth", E_USER_WARNING);
         }
         return 0;
diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php
index 49f6dd62d..0477591de 100644
--- a/src/ORM/Connect/MySQLDatabase.php
+++ b/src/ORM/Connect/MySQLDatabase.php
@@ -360,10 +360,7 @@ class MySQLDatabase extends Database
             return false;
         }
         --$this->transactionNesting;
-        if ($this->transactionNesting <= 0) {
-            $this->transactionNesting = 0;
-            $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
-        }
+        $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
         return true;
     }
 

From 1048520fbe809954852483f74fc91314a29edca5 Mon Sep 17 00:00:00 2001
From: Robbie Averill <robbie@silverstripe.com>
Date: Wed, 20 Jun 2018 10:41:36 +1200
Subject: [PATCH 7/8] Restore check for zero or negative transaction nesting

---
 src/ORM/Connect/MySQLDatabase.php | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php
index 0477591de..49f6dd62d 100644
--- a/src/ORM/Connect/MySQLDatabase.php
+++ b/src/ORM/Connect/MySQLDatabase.php
@@ -360,7 +360,10 @@ class MySQLDatabase extends Database
             return false;
         }
         --$this->transactionNesting;
-        $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
+        if ($this->transactionNesting <= 0) {
+            $this->transactionNesting = 0;
+            $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
+        }
         return true;
     }
 

From 793aafae917aa190e711ae5c7a18ed59c2812634 Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Thu, 21 Jun 2018 14:26:21 +0100
Subject: [PATCH 8/8] FIX Transaction depth should error if not implemented by
 child classes

---
 src/ORM/Connect/Database.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php
index 670b07b5b..4c60ed085 100644
--- a/src/ORM/Connect/Database.php
+++ b/src/ORM/Connect/Database.php
@@ -676,7 +676,7 @@ abstract class Database
     public function transactionDepth()
     {
         // Placeholder error for transactional DBs that don't expose depth
-        if (!$this->supportsTransactions()) {
+        if ($this->supportsTransactions()) {
             user_error(get_class($this) . " does not support transactionDepth", E_USER_WARNING);
         }
         return 0;