Refactor to separate 'applyState' and 'saveState' functions

`appyState` will now only read the test session data and update the environment to relect that state. This is intended for things like database connections and mailer classes. Actions that need to alter state to apply it (eg creating a temp db) should be done in other actions such as 'start' `saveState` will only take an existing state object and persist it to a file Also addresses: [Connect to test database on session load](#63) [Skip Temp DB creation if state has no database prop](#64) Prevent re-creating session file on end (original issue for this PR)
silverstripe · May 5, 2019 · 92a4de4 · 92a4de4
1 parent 61d12ec
commit 92a4de4
Show file tree

Hide file tree

Showing 8 changed files with 624 additions and 81 deletions.
diff --git a/src/TestSessionController.php b/src/TestSessionController.php
@@ -311,7 +311,7 @@ public function clear()
 
         $this->extend('onBeforeClear');
 
-        $tempDB = new TempDatabase();
+        $tempDB = TempDatabase::create();
         if ($tempDB->isUsed()) {
             $tempDB->clearAllData();
         }

diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php
@@ -7,10 +7,11 @@
 use InvalidArgumentException;
 use LogicException;
 use SilverStripe\Assets\Filesystem;
-use SilverStripe\Core\Environment;
 use SilverStripe\Control\Director;
+use SilverStripe\Control\Email\Email;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Environment;
 use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Core\Injector\Injector;
@@ -163,7 +164,9 @@ public function startTestSession($state = null, $id = null)
         $json = json_encode($state, JSON_FORCE_OBJECT);
         $state = json_decode($json);
 
-        $this->applyState($state);
+        $state = $this->createDatabase($state);
+        $state = $this->applyState($state);
+        $this->saveState($state);
 
         // Back up /assets folder
         $this->backupAssets();
@@ -180,6 +183,7 @@ public function updateTestSession($state)
         $state = json_decode($json);
 
         $this->applyState($state);
+        $this->saveState($state);
 
         $this->extend('onAfterUpdateTestSession');
     }
@@ -252,26 +256,14 @@ protected function moveRecursive($src, $dest, $ignore = [])
     }
 
     /**
-     * Assumes the database has already been created in startTestSession(), as this method can be called from
-     * _config.php where we don't yet have a DB connection.
+     * takes a state object (stdClass) and merges the existing state from getState()
+     * NB new state will not be overwritten
      *
-     * Persists the state to the filesystem.
-     *
-     * You can extend this by creating an Extension object and implementing either onBeforeApplyState() or
-     * onAfterApplyState() to add your own test state handling in.
-     *
-     * @param mixed $state
-     * @throws LogicException
-     * @throws InvalidArgumentException
+     * @param stdClass $state
+     * @return void
      */
-    public function applyState($state)
+    public function mergeWithExistingState($state)
     {
-        $this->extend('onBeforeApplyState', $state);
-
-        // back up source
-        $databaseConfig = DB::getConfig();
-        $this->oldDatabaseName = $databaseConfig['database'];
-
         // Load existing state from $this->state into $state, if there is any
         $oldState = $this->getState();
 
@@ -282,43 +274,78 @@ public function applyState($state)
                 }
             }
         }
+        return $state;
+    }
 
-        // ensure we have a connection to the database
-        $this->connectToDatabase($state);
+    /**
+     * Creates a TempDB
+     * Will modify state if the created database has a different name from what was in state
+     *
+     * @param stdClass $state
+     * @return stdClass $state
+     */
+    public function createDatabase($state)
+    {
+        $dbCreate = isset($state->createDatabase) ? (bool) $state->createDatabase : false;
+        $dbName = (isset($state->database)) ? $state->database : null;
 
-        // Database
-        if (!$this->isRunningTests()) {
-            $dbName = (isset($state->database)) ? $state->database : null;
+        if ($dbName) {
+            $dbExists = DB::get_conn()->databaseExists($dbName);
+        } else {
+            $dbExists = false;
+        }
 
-            if ($dbName) {
-                $dbExists = DB::get_conn()->databaseExists($dbName);
-            } else {
-                $dbExists = false;
+        if (!$dbExists && $dbCreate) {
+            $databaseConfig = DB::getConfig();
+            $this->oldDatabaseName = $databaseConfig['database'];
+            // Create a new one with a randomized name
+            $tempDB = new TempDatabase();
+            $dbName = $tempDB->build();
+
+            $state->database = $dbName; // In case it's changed by the call to SapphireTest::create_temp_db();
+
+            // Set existing one, assumes it already has been created
+            $prefix = Environment::getEnv('SS_DATABASE_PREFIX') ?: 'ss_';
+            $pattern = strtolower(sprintf('#^%stmpdb.*#', preg_quote($prefix, '#')));
+            if (!preg_match($pattern, $dbName)) {
+                throw new InvalidArgumentException("Invalid database name format");
             }
 
-            if (!$dbExists) {
-                // Create a new one with a randomized name
-                $tempDB = new TempDatabase();
-                $dbName = $tempDB->build();
+            $databaseConfig['database'] = $dbName; // Instead of calling DB::set_alternative_db_name();
 
-                $state->database = $dbName; // In case it's changed by the call to SapphireTest::create_temp_db();
+            // Connect to the new database, overwriting the old DB connection (if any)
+            DB::connect($databaseConfig);
+        }
 
-                // Set existing one, assumes it already has been created
-                $prefix = Environment::getEnv('SS_DATABASE_PREFIX') ?: 'ss_';
-                $pattern = strtolower(sprintf('#^%stmpdb.*#', preg_quote($prefix, '#')));
-                if (!preg_match($pattern, $dbName)) {
-                    throw new InvalidArgumentException("Invalid database name format");
-                }
+        return $state;
+    }
 
-                $this->oldDatabaseName = $databaseConfig['database'];
-                $databaseConfig['database'] = $dbName; // Instead of calling DB::set_alternative_db_name();
+    /**
+     * Takes a state object and applies environment transformations to current application environment
+     *
+     * Does not persist any changes to the the state object - @see saveState for persistence
+     *
+     * Assumes the database has already been created in startTestSession(), as this method can be called from
+     * _config.php where we don't yet have a DB connection.
+     *
+     * You can extend this by creating an Extension object and implementing either onBeforeApplyState() or
+     * onAfterApplyState() to add your own test state handling in.
+     *
+     * @param mixed $state
+     * @throws LogicException
+     * @throws InvalidArgumentException
+     * @return mixed $state
+     */
+    public function applyState($state)
+    {
+        $this->extend('onBeforeApplyState', $state);
 
-                // Connect to the new database, overwriting the old DB connection (if any)
-                DB::connect($databaseConfig);
-            }
+        // back up source
+        $databaseConfig = DB::getConfig();
+        $this->oldDatabaseName = $databaseConfig['database'];
 
-            TestSessionState::create()->write();  // initialize the session state
-        }
+        // ensure we have a connection to the database
+        $this->connectToDatabase($state);
 
         // Mailer
         $mailer = (isset($state->mailer)) ? $state->mailer : null;
@@ -330,6 +357,8 @@ public function applyState($state)
                     $mailer
                 ));
             }
+            Injector::inst()->registerService(new $mailer(), Mailer::class);
+            Email::config()->set("send_all_emails_to", null);
         }
 
         // Date and time
@@ -343,11 +372,24 @@ public function applyState($state)
                     $state->datetime
                 ));
             }
+            DBDatetime::set_mock_now($state->datetime);
         }
 
-        $this->saveState($state);
+        // Allows inclusion of a PHP file, usually with procedural commands
+        // to set up required test state. The file can be generated
+        // through {@link TestSessionStubCodeWriter}, and the session state
+        // set through {@link TestSessionController->set()} and the
+        // 'testsession.stubfile' state parameter.
+        if (isset($state->stubfile)) {
+            $file = $state->stubfile;
+            if (!Director::isLive() && $file && file_exists($file)) {
+                include_once($file);
+            }
+        }
 
-        $this->extend('onAfterApplyState');
+        $this->extend('onAfterApplyState', $state);
+
+        return $state;
     }
 
     /**
@@ -477,6 +519,8 @@ public function endTestSession()
     /**
      * Loads a YAML fixture into the database as part of the {@link TestSessionController}.
      *
+     * Writes loaded fixture files to $state->fixtures
+     *
      * @param string $fixtureFile The .yml file to load
      * @return FixtureFactory The loaded fixture
      * @throws LogicException
@@ -501,7 +545,7 @@ public function loadFixtureIntoDb($fixtureFile)
 
         $state = $this->getState();
         $state->fixtures[] = $fixtureFile;
-        $this->applyState($state);
+        $this->saveState($state);
 
         return $fixture;
     }
@@ -545,10 +589,12 @@ protected function getAssetsBackupfolder()
 
     /**
      * Ensure that there is a connection to the database
-     * 
+     *
      * @param mixed $state
+     * @return void
      */
-    public function connectToDatabase($state = null) {
+    public function connectToDatabase($state = null)
+    {
         if ($state == null) {
             $state = $this->getState();
         }

diff --git a/src/TestSessionHTTPMiddleware.php b/src/TestSessionHTTPMiddleware.php
@@ -57,41 +57,26 @@ public function process(HTTPRequest $request, callable $delegate)
      */
     protected function loadTestState(HTTPRequest $request)
     {
-        $testState = $this->testSessionEnvironment->getState();
-
-        // Date and time
-        if (isset($testState->datetime)) {
-            DBDatetime::set_mock_now($testState->datetime);
-        }
-
-        // Register mailer
-        if (isset($testState->mailer)) {
-            $mailer = $testState->mailer;
-            Injector::inst()->registerService(new $mailer(), Mailer::class);
-            Email::config()->set("send_all_emails_to", null);
-        }
-
-        // Allows inclusion of a PHP file, usually with procedural commands
-        // to set up required test state. The file can be generated
-        // through {@link TestSessionStubCodeWriter}, and the session state
-        // set through {@link TestSessionController->set()} and the
-        // 'testsession.stubfile' state parameter.
-        if (isset($testState->stubfile)) {
-            $file = $testState->stubfile;
-            if (!Director::isLive() && $file && file_exists($file)) {
-                // Connect to the database so the included code can interact with it
-                $this->testSessionEnvironment->connectToDatabase();
-
-                include_once($file);
-            }
-        }
+        $state = $this->testSessionEnvironment->getState();
+        $this->testSessionEnvironment->applyState($state);
     }
 
+    /**
+     * @param HTTPRequest $request
+     * @return void
+     */
     protected function restoreTestState(HTTPRequest $request)
     {
         // Store PHP session
         $state = $this->testSessionEnvironment->getState();
         $state->session = $request->getSession()->getAll();
-        $this->testSessionEnvironment->applyState($state);
+
+        // skip saving file if the session is being closed (all test properties are removed except session)
+        $keys = get_object_vars($state);
+        if (count($keys) <= 1) {
+            return;
+        }
+
+        $this->testSessionEnvironment->saveState($state);
     }
 }
diff --git a/tests/stubs/_manifest_exclude b/tests/stubs/_manifest_exclude
diff --git a/tests/stubs/teststub.php b/tests/stubs/teststub.php
@@ -0,0 +1,3 @@
+<?php
+
+define('TESTSESSION_STUBFILE', false);