API Moving tests to use transactions

_config/tests.yml

@@ -8,6 +8,7 @@ SilverStripe\Core\Injector\Injector:
         globals: '%$SilverStripe\Dev\State\GlobalsTestState'
         extensions: '%$SilverStripe\Dev\State\ExtensionTestState'
         flushable: '%$SilverStripe\Dev\State\FlushableTestState'
+        fixtures: '%$SilverStripe\Dev\State\FixtureTestState'
         requirements: '%$SilverStripe\View\Dev\RequirementsTestState'
         ssviewer: '%$SilverStripe\View\Dev\SSViewerTestState'
 ---

src/Dev/FixtureFactory.php

@@ -41,7 +41,7 @@ class FixtureFactory
     protected $fixtures = array();
 
     /**
-     * @var array Callbacks
+     * @var FixtureBlueprint[] Callbacks
      */
     protected $blueprints = array();
 

src/Dev/SapphireTest.php

@@ -23,6 +23,7 @@ use SilverStripe\Core\Manifest\ClassLoader;
 use SilverStripe\Dev\Constraint\SSListContains;
 use SilverStripe\Dev\Constraint\SSListContainsOnly;
 use SilverStripe\Dev\Constraint\SSListContainsOnlyMatchingItems;
+use SilverStripe\Dev\State\FixtureTestState;
 use SilverStripe\Dev\State\SapphireTestState;
 use SilverStripe\Dev\State\TestState;
 use SilverStripe\i18n\i18n;
@@ -148,7 +149,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
     /**
      * State management container for SapphireTest
      *
-     * @var TestState
+     * @var SapphireTestState
      */
     protected static $state = null;
 
@@ -159,6 +160,17 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
      */
     protected static $tempDB = null;
 
+    /**
+     * @return TempDatabase
+     */
+    public static function tempDB()
+    {
+        if (!static::$tempDB) {
+            static::$tempDB = TempDatabase::create();
+        }
+        return static::$tempDB;
+    }
+
     /**
      * Gets illegal extensions for this class
      *
@@ -208,6 +220,22 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
         return static::$fixture_file;
     }
 
+    /**
+     * @return bool
+     */
+    public function getUsesDatabase()
+    {
+        return $this->usesDatabase;
+    }
+
+    /**
+     * @return array
+     */
+    public function getRequireDefaultRecordsFrom()
+    {
+        return $this->requireDefaultRecordsFrom;
+    }
+
     /**
      * Setup  the test.
      * Always sets up in order:
@@ -254,31 +282,10 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
 
         $fixtureFiles = $this->getFixturePaths();
 
-        // Set up fixture
         if ($this->shouldSetupDatabaseForCurrentTest($fixtureFiles)) {
-            if (!static::$tempDB->isUsed()) {
-                static::$tempDB->build();
-            }
-
-            DataObject::singleton()->flushCache();
-
-            static::$tempDB->clearAllData();
-
-            foreach ($this->requireDefaultRecordsFrom as $className) {
-                $instance = singleton($className);
-                if (method_exists($instance, 'requireDefaultRecords')) {
-                    $instance->requireDefaultRecords();
-                }
-                if (method_exists($instance, 'augmentDefaultRecords')) {
-                    $instance->augmentDefaultRecords();
-                }
-            }
-
-            foreach ($fixtureFiles as $fixtureFilePath) {
-                $fixture = YamlFixture::create($fixtureFilePath);
-                $fixture->writeInto($this->getFixtureFactory());
-            }
-
+            /** @var FixtureTestState $fixtureState */
+            $fixtureState = static::$state->getStateByName('fixtures');
+            $this->setFixtureFactory($fixtureState->getFixtureFactory(static::class));
             $this->logInWithPermission('ADMIN');
         }
 
@@ -393,24 +400,29 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
     }
 
     /**
-     * @return FixtureFactory
+     * @deprecated 4.0..5.0
+     * @return FixtureFactory|false
      */
     public function getFixtureFactory()
     {
-        if (!$this->fixtureFactory) {
-            $this->fixtureFactory = Injector::inst()->create(FixtureFactory::class);
-        }
-        return $this->fixtureFactory;
+        Deprecation::notice('5.0', __FUNCTION__ . ' is deprecated, use ' . FixtureTestState::class . ' instead');
+        /** @var FixtureTestState $state */
+        $state = static::$state->getStateByName('fixtures');
+        return $state->getFixtureFactory(static::class);
     }
 
     /**
      * Sets a new fixture factory
-     *
+     * @deprecated 4.0..5.0
      * @param FixtureFactory $factory
      * @return $this
      */
     public function setFixtureFactory(FixtureFactory $factory)
     {
+        Deprecation::notice('5.0', __FUNCTION__ . ' is deprecated, use ' . FixtureTestState::class . ' instead');
+        /** @var FixtureTestState $state */
+        $state = static::$state->getStateByName('fixtures');
+        $state->setFixtureFactory($factory, static::class);
         $this->fixtureFactory = $factory;
         return $this;
     }
@@ -476,11 +488,13 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
      * Load a YAML fixture file into the database.
      * Once loaded, you can use idFromFixture() and objFromFixture() to get items from the fixture.
      * Doesn't clear existing fixtures.
+     * @deprecated 4.0...5.0
      *
      * @param string $fixtureFile The location of the .yml fixture file, relative to the site base dir
      */
     public function loadFixture($fixtureFile)
     {
+        Deprecation::notice('5.0', __FUNCTION__ . ' is deprecated, use ' . FixtureTestState::class . ' instead');
         $fixture = Injector::inst()->create(YamlFixture::class, $fixtureFile);
         $fixture->writeInto($this->getFixtureFactory());
     }
@@ -973,7 +987,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
         // Register state
         static::$state = SapphireTestState::singleton();
         // Register temp DB holder
-        static::$tempDB = TempDatabase::create();
+        static::tempDB();
     }
 
     /**

src/Dev/State/FixtureTestState.php

@@ -0,0 +1,229 @@
+<?php
+
+namespace SilverStripe\Dev\State;
+
+use LogicException;
+use SilverStripe\Control\Director;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Manifest\ClassLoader;
+use SilverStripe\Dev\FixtureFactory;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Dev\YamlFixture;
+use SilverStripe\ORM\DataObject;
+
+class FixtureTestState implements TestState
+{
+
+    /**
+     * @var FixtureFactory[]
+     */
+    private $fixtureFactories = [];
+
+    /**
+     * Called on setup
+     *
+     * @param SapphireTest $test
+     */
+    public function setUp(SapphireTest $test)
+    {
+        if ($this->testNeedsDB($test)) {
+            $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();
+                    }
+                }
+                $this->loadFixtures($test);
+            }
+            $tmpDB->startTransaction();
+        }
+    }
+
+    /**
+     * Called on tear down
+     *
+     * @param SapphireTest $test
+     */
+    public function tearDown(SapphireTest $test)
+    {
+        if ($this->testNeedsDB($test)) {
+            $test::tempDB()->rollbackTransaction();
+        }
+    }
+
+    /**
+     * Called once on setup
+     *
+     * @param string $class Class being setup
+     */
+    public function setUpOnce($class)
+    {
+        $this->fixtureFactories[strtolower($class)] = Injector::inst()->create(FixtureFactory::class);
+    }
+
+    /**
+     * Called once on tear down
+     *
+     * @param string $class Class being torn down
+     */
+    public function tearDownOnce($class)
+    {
+        unset($this->fixtureFactories[strtolower($class)]);
+        $class::tempDB()->clearAllData();
+    }
+
+    /**
+     * @param string $class
+     *
+     * @return bool|FixtureFactory
+     */
+    public function getFixtureFactory($class)
+    {
+        $testClass = strtolower($class);
+        if (array_key_exists($testClass, $this->fixtureFactories)) {
+            return $this->fixtureFactories[$testClass];
+        }
+        return false;
+    }
+
+    /**
+     * @param FixtureFactory $factory
+     * @param string $class
+     */
+    public function setFixtureFactory(FixtureFactory $factory, $class)
+    {
+        $this->fixtureFactories[strtolower($class)] = $factory;
+    }
+
+    /**
+     * @param array $fixtures
+     *
+     * @param SapphireTest $test
+     *
+     * @return array
+     */
+    protected function getFixturePaths($fixtures, SapphireTest $test)
+    {
+        return array_map(function ($fixtureFilePath) use ($test) {
+            return $this->resolveFixturePath($fixtureFilePath, $test);
+        }, $fixtures);
+    }
+
+    /**
+     * @param SapphireTest $test
+     */
+    protected function loadFixtures(SapphireTest $test)
+    {
+        $fixtures = $test::get_fixture_file();
+        $fixtures = is_array($fixtures) ? $fixtures : [$fixtures];
+        $paths = $this->getFixturePaths($fixtures, $test);
+        foreach ($paths as $fixtureFile) {
+            $this->loadFixture($fixtureFile, $test);
+        }
+    }
+
+    /**
+     * @param string $fixtureFile
+     * @param SapphireTest $test
+     */
+    protected function loadFixture($fixtureFile, SapphireTest $test)
+    {
+        /** @var YamlFixture $fixture */
+        $fixture = Injector::inst()->create(YamlFixture::class, $fixtureFile);
+        $fixture->writeInto($this->getFixtureFactory(get_class($test)));
+    }
+
+    /**
+     * Map a fixture path to a physical file
+     *
+     * @param string $fixtureFilePath
+     * @param SapphireTest $test
+     *
+     * @return string
+     */
+    protected function resolveFixturePath($fixtureFilePath, SapphireTest $test)
+    {
+        // Support fixture paths relative to the test class, rather than relative to webroot
+        // String checking is faster than file_exists() calls.
+        $isRelativeToFile
+            = (strpos($fixtureFilePath, '/') === false)
+            || preg_match('/^(\.){1,2}/', $fixtureFilePath);
+
+        if ($isRelativeToFile) {
+            $resolvedPath = realpath($this->getTestAbsolutePath($test) . '/' . $fixtureFilePath);
+            if ($resolvedPath) {
+                return $resolvedPath;
+            }
+        }
+
+        // Check if file exists relative to base dir
+        $resolvedPath = realpath(Director::baseFolder() . '/' . $fixtureFilePath);
+        if ($resolvedPath) {
+            return $resolvedPath;
+        }
+
+        return $fixtureFilePath;
+    }
+
+    /**
+     * Useful for writing unit tests without hardcoding folder structures.
+     *
+     * @param SapphireTest $test
+     *
+     * @return string Absolute path to current class.
+     */
+    protected function getTestAbsolutePath(SapphireTest $test)
+    {
+        $filename = ClassLoader::inst()->getItemPath(get_class($test));
+        if (!$filename) {
+            throw new LogicException('getItemPath returned null for ' . static::class
+                . '. Try adding flush=1 to the test run.');
+        }
+        return dirname($filename);
+    }
+
+    /**
+     * @param SapphireTest $test
+     *
+     * @return bool
+     */
+    protected function testNeedsDB(SapphireTest $test)
+    {
+        $annotations = $test->getAnnotations();
+
+        // annotation explicitly disables the DB
+        if (array_key_exists('useDatabase', $annotations['method'])
+            && $annotations['method']['useDatabase'][0] === 'false') {
+            return false;
+        }
+
+        // annotation explicitly enables the DB
+        if (array_key_exists('useDatabase', $annotations['method'])
+            && $annotations['method']['useDatabase'][0] !== 'false') {
+            return true;
+        }
+
+        // test class explicitly enables DB
+        if ($test->getUsesDatabase()) {
+            return true;
+        }
+
+        // presence of fixture file implicitly enables DB
+        $fixtures = $test::get_fixture_file();
+        if (!empty($fixtures)) {
+            return true;
+        }
+
+        return false;
+    }
+}

src/Dev/State/SapphireTestState.php

@@ -22,6 +22,36 @@ class SapphireTestState implements TestState
         return $this->states;
     }
 
+    /**
+     * @param string $name
+     *
+     * @return bool|TestState
+     */
+    public function getStateByName($name)
+    {
+        $states = $this->getStates();
+        if (array_key_exists($name, $states)) {
+            return $states[$name];
+        }
+        return false;
+    }
+
+    /**
+     * @param string $class
+     *
+     * @return bool|TestState
+     */
+    public function getStateByClass($class)
+    {
+        $lClass = strtolower($class);
+        foreach ($this->getStates() as $state) {
+            if ($lClass === strtolower(get_class($state))) {
+                return $state;
+            }
+        }
+        return false;
+    }
+
     /**
      * @param TestState[] $states
      * @return $this

src/ORM/Connect/TempDatabase.php

@@ -23,6 +23,11 @@ class TempDatabase
      */
     protected $name = null;
 
+    /**
+     * @var bool If a transaction has been started
+     */
+    protected $hasStarted = false;
+
     /**
      * Create a new temp database
      *
@@ -68,6 +73,46 @@ class TempDatabase
         return $this->isDBTemp($selected);
     }
 
+    /**
+     * @return bool
+     */
+    public function hasStarted()
+    {
+        return $this->hasStarted;
+    }
+
+    /**
+     * @return bool
+     */
+    public function supportsTransactions()
+    {
+        return static::getConn()->supportsTransactions();
+    }
+
+    /**
+     * Start a transaction for easy rollback after tests
+     */
+    public function startTransaction()
+    {
+        $this->hasStarted = true;
+        if (static::getConn()->supportsTransactions()) {
+            static::getConn()->transactionStart();
+        }
+    }
+
+    /**
+     * Rollback a transaction (or trash all data if the DB doesn't support databases
+     */
+    public function rollbackTransaction()
+    {
+        if (static::getConn()->supportsTransactions()) {
+            static::getConn()->transactionRollback();
+        } else {
+            $this->hasStarted = false;
+            static::clearAllData();
+        }
+    }
+
     /**
      * Destroy the current temp database
      */
@@ -102,6 +147,7 @@ class TempDatabase
      */
     public function clearAllData()
     {
+        $this->hasStarted = false;
         if (!$this->isUsed()) {
             return;
         }
@@ -183,6 +229,11 @@ class TempDatabase
      */
     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()) {
+            $this->rollbackTransaction();
+        }
         if (!$this->isUsed()) {
             return;
         }

src/ORM/DataObjectSchema.php

@@ -100,6 +100,7 @@ class DataObjectSchema
      * @param string $class Class name (not a table).
      * @param string $field Name of field that belongs to this class (or a parent class)
      * @param string $tablePrefix Optional prefix for table (alias)
+     *
      * @return string The SQL identifier string for the corresponding column for this field
      */
     public function sqlColumnForField($class, $field, $tablePrefix = null)
@@ -118,6 +119,7 @@ class DataObjectSchema
      * the name that would be used if this table did exist.
      *
      * @param string $class
+     *
      * @return string Returns the table name, or null if there is no table
      */
     public function tableName($class)
@@ -135,6 +137,7 @@ class DataObjectSchema
      * passed class.
      *
      * @param string|object $class
+     *
      * @return string
      * @throws InvalidArgumentException
      */
@@ -155,6 +158,7 @@ class DataObjectSchema
      * Get the base table
      *
      * @param string|object $class
+     *
      * @return string
      */
     public function baseDataTable($class)
@@ -185,6 +189,7 @@ class DataObjectSchema
      *  - UNINHERITED Limit to only this table
      *  - DB_ONLY Exclude virtual fields (such as composite fields), and only include fields with a db column.
      *  - INCLUDE_CLASS Prefix the field specification with the class name in RecordClass.Column(spec) format.
+     *
      * @return array List of fields, where the key is the field name and the value is the field specification.
      */
     public function fieldSpecs($classOrInstance, $options = 0)
@@ -235,6 +240,7 @@ class DataObjectSchema
      *  - UNINHERITED Limit to only this table
      *  - DB_ONLY Exclude virtual fields (such as composite fields), and only include fields with a db column.
      *  - INCLUDE_CLASS Prefix the field specification with the class name in RecordClass.Column(spec) format.
+     *
      * @return string|null Field will be a string in FieldClass(args) format, or
      * RecordClass.FieldClass(args) format if using INCLUDE_CLASS. Will be null if no field is found.
      */
@@ -248,6 +254,7 @@ class DataObjectSchema
      * Find the class for the given table
      *
      * @param string $table
+     *
      * @return string|null The FQN of the class, or null if not found
      */
     public function tableClass($table)
@@ -303,6 +310,7 @@ class DataObjectSchema
      * See dev/build errors for details in case of table name violation.
      *
      * @param string $class
+     *
      * @return string
      */
     protected function buildTableName($class)
@@ -334,6 +342,7 @@ class DataObjectSchema
      *
      * @param string $class Class name to query from
      * @param bool $aggregated Include fields in entire hierarchy, rather than just on this table
+     *
      * @return array Map of fieldname to specification, similiar to {@link DataObject::$db}.
      */
     public function databaseFields($class, $aggregated = true)
@@ -360,6 +369,7 @@ class DataObjectSchema
      * @param string $class Class name to query from
      * @param string $field Field name
      * @param bool $aggregated Include fields in entire hierarchy, rather than just on this table
+     *
      * @return string|null Field specification, or null if not a field
      */
     public function databaseField($class, $field, $aggregated = true)
@@ -392,6 +402,7 @@ class DataObjectSchema
      * Check if the given class has a table
      *
      * @param string $class
+     *
      * @return bool
      */
     public function classHasTable($class)
@@ -415,6 +426,7 @@ class DataObjectSchema
      *
      * @param string $class Name of class to check
      * @param bool $aggregated Include fields in entire hierarchy, rather than just on this table
+     *
      * @return array List of composite fields and their class spec
      */
     public function compositeFields($class, $aggregated = true)
@@ -442,6 +454,7 @@ class DataObjectSchema
      * @param string $class Class name to query from
      * @param string $field Field name
      * @param bool $aggregated Include fields in entire hierarchy, rather than just on this table
+     *
      * @return string|null Field specification, or null if not a field
      */
     public function compositeField($class, $field, $aggregated = true)
@@ -535,6 +548,7 @@ class DataObjectSchema
      * Get "default" database indexable field types
      *
      * @param  string $class
+     *
      * @return array
      */
     protected function cacheDefaultDatabaseIndexes($class)
@@ -559,6 +573,7 @@ class DataObjectSchema
      * Look for custom indexes declared on the class
      *
      * @param  string $class
+     *
      * @return array
      * @throws InvalidArgumentException If an index already exists on the class
      * @throws InvalidArgumentException If a custom index format is not valid
@@ -637,6 +652,7 @@ class DataObjectSchema
      * Parses a specified column into a sort field and direction
      *
      * @param string $column String to parse containing the column name
+     *
      * @return array Resolved table and column.
      */
     protected function parseSortColumn($column)
@@ -659,6 +675,7 @@ class DataObjectSchema
      *
      * @param string $candidateClass
      * @param string $fieldName
+     *
      * @return string
      */
     public function tableForField($candidateClass, $fieldName)
@@ -677,6 +694,7 @@ class DataObjectSchema
      *
      * @param string $candidateClass
      * @param string $fieldName
+     *
      * @return string
      */
     public function classForField($candidateClass, $fieldName)
@@ -720,8 +738,10 @@ class DataObjectSchema
      *                          If the class name is 'ManyManyThroughList' then this is the name of the
      *                          has_many relation.
      * )
+     *
      * @param string $class Name of class to get component for
      * @param string $component The component name
+     *
      * @return array|null
      */
     public function manyManyComponent($class, $component)
@@ -768,6 +788,7 @@ class DataObjectSchema
      * @param string $parentClass Name of class
      * @param string $component Name of relation on class
      * @param string $specification specification for this belongs_many_many
+     *
      * @return array Array with child class and relation name
      */
     protected function parseBelongsManyManyComponent($parentClass, $component, $specification)
@@ -802,7 +823,7 @@ class DataObjectSchema
         // Return relatios
         return [
             'childClass' => $childClass,
-            'relationName' => $relationName
+            'relationName' => $relationName,
         ];
     }
 
@@ -811,6 +832,7 @@ class DataObjectSchema
      *
      * @param string $class
      * @param string $component
+     *
      * @return array|null
      */
     public function manyManyExtraFieldsForComponent($class, $component)
@@ -845,6 +867,7 @@ class DataObjectSchema
      * @param string $component
      * @param bool $classOnly If this is TRUE, than any has_many relationships in the form
      * "ClassName.Field" will have the field data stripped off. It defaults to TRUE.
+     *
      * @return string|null
      */
     public function hasManyComponent($class, $component, $classOnly = true)
@@ -868,6 +891,7 @@ class DataObjectSchema
      *
      * @param string $class
      * @param string $component
+     *
      * @return string|null
      */
     public function hasOneComponent($class, $component)
@@ -890,6 +914,7 @@ class DataObjectSchema
      * @param string $component
      * @param bool $classOnly If this is TRUE, than any has_many relationships in the
      * form "ClassName.Field" will have the field data stripped off. It defaults to TRUE.
+     *
      * @return string|null
      */
     public function belongsToComponent($class, $component, $classOnly = true)
@@ -912,8 +937,10 @@ class DataObjectSchema
      * Check class for any unary component
      *
      * Alias for hasOneComponent() ?: belongsToComponent()
+     *
      * @param string $class
      * @param string $component
+     *
      * @return string|null
      */
     public function unaryComponent($class, $component)
@@ -926,6 +953,7 @@ class DataObjectSchema
      * @param string $parentClass Parent class name
      * @param string $component ManyMany name
      * @param string|array $specification Declaration of many_many relation type
+     *
      * @return array
      */
     protected function parseManyManyComponent($parentClass, $component, $specification)
@@ -975,6 +1003,7 @@ class DataObjectSchema
      *
      * @param string $childClass
      * @param string $parentClass
+     *
      * @return string|null
      */
     protected function getManyManyInverseRelationship($childClass, $parentClass)
@@ -992,10 +1021,10 @@ class DataObjectSchema
             if (is_array($manyManySpec)) {
                 $toClass = $this->hasOneComponent($manyManySpec['through'], $manyManySpec['to']);
                 if ($toClass === $parentClass) {
-                return $inverseComponentName;
+                    return $inverseComponentName;
+                }
             }
         }
-        }
         return null;
     }
 
@@ -1011,6 +1040,7 @@ class DataObjectSchema
      * remote object.
      * @param string $type the join type - either 'has_many' or 'belongs_to'
      * @param boolean $polymorphic Flag set to true if the remote join field is polymorphic.
+     *
      * @return string
      * @throws Exception
      */
@@ -1091,6 +1121,7 @@ class DataObjectSchema
      * @param string $joinClass Class for the joined table
      * @param array $specification Complete many_many specification
      * @param string $key Name of key to check ('from' or 'to')
+     *
      * @return string Class that matches the given relation
      * @throws InvalidArgumentException
      */
@@ -1152,6 +1183,7 @@ class DataObjectSchema
      * @param string $parentClass Name of parent class
      * @param string $component Name of many_many component
      * @param array $specification Complete many_many specification
+     *
      * @return string Name of join class
      */
     protected function checkManyManyJoinClass($parentClass, $component, $specification)