From 1d8f112dd847b818076c4f724d553534d41ccda2 Mon Sep 17 00:00:00 2001 From: Bernard Hamlin Date: Wed, 13 Mar 2019 09:15:10 +1300 Subject: [PATCH] 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](https://github.com/silverstripe/silverstripe-testsession/pull/63) [Skip Temp DB creation if state has no database prop](https://github.com/silverstripe/silverstripe-testsession/pull/64) Prevent re-creating session file on end (original issue for this PR) --- src/TestSessionController.php | 2 +- src/TestSessionEnvironment.php | 140 ++++++---- src/TestSessionHTTPMiddleware.php | 43 +-- tests/stubs/_manifest_exclude | 0 tests/stubs/teststub.php | 3 + tests/unit/TestSessionControllerTest.php | 278 +++++++++++++++++++ tests/unit/TestSessionEnvironmentTest.php | 162 +++++++++++ tests/unit/TestSessionHTTPMiddlewareTest.php | 103 +++++++ 8 files changed, 645 insertions(+), 86 deletions(-) create mode 100644 tests/stubs/_manifest_exclude create mode 100644 tests/stubs/teststub.php create mode 100644 tests/unit/TestSessionControllerTest.php create mode 100644 tests/unit/TestSessionEnvironmentTest.php create mode 100644 tests/unit/TestSessionHTTPMiddlewareTest.php diff --git a/src/TestSessionController.php b/src/TestSessionController.php index 4a53bca..2b32819 100644 --- a/src/TestSessionController.php +++ b/src/TestSessionController.php @@ -311,7 +311,7 @@ class TestSessionController extends Controller $this->extend('onBeforeClear'); - $tempDB = new TempDatabase(); + $tempDB = TempDatabase::create(); if ($tempDB->isUsed()) { $tempDB->clearAllData(); } diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php index e0989e5..e8e8bf0 100644 --- a/src/TestSessionEnvironment.php +++ b/src/TestSessionEnvironment.php @@ -7,10 +7,12 @@ use Exception; use InvalidArgumentException; use LogicException; use SilverStripe\Assets\Filesystem; -use SilverStripe\Core\Environment; use SilverStripe\Control\Director; +use SilverStripe\Control\Email\Email; +use SilverStripe\Control\Email\Mailer; 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; @@ -20,6 +22,7 @@ use SilverStripe\ORM\Connect\TempDatabase; use SilverStripe\ORM\DatabaseAdmin; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\TestSession\TestSessionState; use SilverStripe\Versioned\Versioned; use stdClass; @@ -162,7 +165,9 @@ class TestSessionEnvironment $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(); @@ -179,6 +184,7 @@ class TestSessionEnvironment $state = json_decode($json); $this->applyState($state); + $this->saveState($state); $this->extend('onAfterUpdateTestSession'); } @@ -251,17 +257,65 @@ class TestSessionEnvironment } /** + * 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; + + if ($dbName) { + $dbExists = DB::get_conn()->databaseExists($dbName); + } else { + $dbExists = false; + } + + $databaseConfig = DB::getConfig(); + if (!$dbExists && $dbCreate) { + $this->oldDatabaseName = $databaseConfig['database']; + // Create a new one with a randomized name + $tempDB = TempDatabase::create(); + $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 || $dbCreate) { + // Connect to the new database, overwriting the old DB connection (if any) + $databaseConfig['database'] = $dbName; // Instead of calling DB::set_alternative_db_name(); + DB::connect($databaseConfig); + TestSessionState::create()->write(); // initialize the session state + } + + return $state; + } + + /** + * 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. * - * 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 + * @return mixed $state */ public function applyState($state) { @@ -271,64 +325,21 @@ class TestSessionEnvironment $databaseConfig = DB::getConfig(); $this->oldDatabaseName = $databaseConfig['database']; - // Load existing state from $this->state into $state, if there is any - $oldState = $this->getState(); - - if ($oldState) { - foreach ($oldState as $k => $v) { - if (!isset($state->$k)) { - $state->$k = $v; // Don't overwrite stuff in $state, as that's the new state - } - } - } - // ensure we have a connection to the database $this->connectToDatabase($state); - // Database - if (!$this->isRunningTests()) { - $dbName = (isset($state->database)) ? $state->database : null; - - if ($dbName) { - $dbExists = DB::get_conn()->databaseExists($dbName); - } else { - $dbExists = false; - } - - if (!$dbExists) { - // 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"); - } - - $this->oldDatabaseName = $databaseConfig['database']; - $databaseConfig['database'] = $dbName; // Instead of calling DB::set_alternative_db_name(); - - // Connect to the new database, overwriting the old DB connection (if any) - DB::connect($databaseConfig); - } - - TestSessionState::create()->write(); // initialize the session state - } - // Mailer $mailer = (isset($state->mailer)) ? $state->mailer : null; if ($mailer) { - if (!class_exists($mailer) || !is_subclass_of($mailer, 'SilverStripe\\Control\\Email\\Mailer')) { + if (!class_exists($mailer) || !is_subclass_of($mailer, Mailer::class)) { throw new InvalidArgumentException(sprintf( 'Class "%s" is not a valid class, or subclass of Mailer', $mailer )); } + Injector::inst()->registerService(new $mailer(), Mailer::class); + Email::config()->set("send_all_emails_to", null); } // Date and time @@ -342,11 +353,24 @@ class TestSessionEnvironment $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; } /** @@ -454,7 +478,7 @@ class TestSessionEnvironment $this->restoreAssets(); // Reset DB - $tempDB = new TempDatabase(); + $tempDB = TempDatabase::create(); if ($tempDB->isUsed()) { $state = $this->getState(); $dbConn = DB::get_schema(); @@ -476,6 +500,8 @@ class TestSessionEnvironment /** * 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 @@ -500,7 +526,7 @@ class TestSessionEnvironment $state = $this->getState(); $state->fixtures[] = $fixtureFile; - $this->applyState($state); + $this->saveState($state); return $fixture; } @@ -544,10 +570,12 @@ class TestSessionEnvironment /** * 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 index c031558..51568d6 100644 --- a/src/TestSessionHTTPMiddleware.php +++ b/src/TestSessionHTTPMiddleware.php @@ -57,41 +57,26 @@ class TestSessionHTTPMiddleware implements HTTPMiddleware */ 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); - } - - // Connect to the test session database - $this->testSessionEnvironment->connectToDatabase(); - - // 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)) { - 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 new file mode 100644 index 0000000..e69de29 diff --git a/tests/stubs/teststub.php b/tests/stubs/teststub.php new file mode 100644 index 0000000..59a87d1 --- /dev/null +++ b/tests/stubs/teststub.php @@ -0,0 +1,3 @@ +unregisterNamedObject(TestSessionEnvironment::class); + Injector::inst()->registerService( + $this->createMock(TestSessionEnvironment::class), + TestSessionEnvironment::class + ); + $this->testSessionEnvironment = TestSessionEnvironment::singleton(); + } + + public function testIndex() + { + $controller = new TestSessionController(); + $this->assertContains('Start a new test session', (string) $controller->index()); + + $env = $this->testSessionEnvironment; + $env->method('isRunningTests') + ->willReturn(true); + + $state = new stdClass(); + $state->showme = 'test'; + $env->method('getState') + ->willReturn($state); + + $html = (string) $controller->index(); + $this->assertContains('Test session in progress.', $html); + $this->assertContains('showme:', $html); + } + + public function testStartNonGlobalImport() + { + DBDatetime::set_mock_now(1552525373); + $params = [ + 'datetime' => [ + 'date' => DBDatetime::now()->Date(), + ], + 'importDatabasePath' => '/somepath', + 'importDatabaseFilename' => 'bigOldb.sql', + 'requireDefaultRecords' => '1', + 'fixture' => 'fixture.yml', + ]; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $env = $this->testSessionEnvironment; + $state = new stdClass(); + + $env->method('getState') + ->willReturn($state); + + $env->expects($this->once()) + ->method('startTestSession') + ->with( + [ + 'datetime' => 'Mar 14, 2019 00:00:00', + 'importDatabasePath' => '/somepath', + 'importDatabaseFilename' => 'bigOldb.sql', + 'requireDefaultRecords' => '1', + 'fixture' => 'fixture.yml', + ], + $this->isType('string') + ); + + // cannot do an import and default records + $env->expects($this->never()) + ->method('requireDefaultRecords'); + + $env->expects($this->once()) + ->method('loadFixtureIntoDb') + ->with('fixture.yml'); + + $html = (string) $controller->start(); + $this->assertContains('Test session in progress.', $html); + + $this->assertNotNull($request->getSession()->get('TestSessionId')); + }, 'dev/testsession/start', $params); + + DBDatetime::clear_mock_now(); + } + + public function testStartNonGlobalDefaultRecords() + { + $params = [ + 'requireDefaultRecords' => '1', + ]; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $env = $this->testSessionEnvironment; + $state = new stdClass(); + + $env->method('getState') + ->willReturn($state); + + $env->expects($this->once()) + ->method('startTestSession') + ->with( + [ + 'requireDefaultRecords' => '1', + ], + $this->isType('string') + ); + + $env->expects($this->once()) + ->method('requireDefaultRecords') + ->with(); + + $controller->start(); + }, 'dev/testsession/start', $params); + } + + public function testSetNotRunning() + { + $this->expectException(LogicException::class); + $params = []; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $controller->set(); + }, 'dev/testsession/set', $params); + } + + public function testSetRunning() + { + DBDatetime::set_mock_now(1552525373); + $params = [ + 'datetime' => [ + 'date' => DBDatetime::now()->Date(), + ], + ]; + + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $env = $this->testSessionEnvironment; + $state = new stdClass(); + + $env->method('isRunningTests') + ->willReturn(true); + + $env->method('getState') + ->willReturn($state); + + $env->expects($this->once()) + ->method('updateTestSession') + ->with( + [ + 'datetime' => 'Mar 14, 2019 00:00:00', + ] + ); + + $html = (string) $controller->set(); + $this->assertContains('Test session in progress.', $html); + }, 'dev/testsession/set', $params); + + DBDatetime::clear_mock_now(); + } + + public function testClearNotRunning() + { + $this->expectException(LogicException::class); + $params = []; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $controller->clear(); + }, 'dev/testsession/clear', $params); + } + + public function testClearRunning() + { + + $params = []; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $env = $this->testSessionEnvironment; + $state = new stdClass(); + + $env->method('isRunningTests') + ->willReturn(true); + + $env->method('getState') + ->willReturn($state); + + Config::nest(); + Config::modify()->set(TempDatabase::class, 'factory', new class { + public function create() + { + $mockdb = $this->createMock(TempDatabase::class); + $mockdb->method('isUsed')->willReturn(true); + $mockdb->expects($this->once()) + ->method('clearAllData'); + return $mockdb; + } + }); + + $msg = $controller->clear(); + Config::unnest(); + $this->assertEquals('Cleared database and test state', $msg); + }, 'dev/testsession/clear', $params); + } + + public function testEndNotRunning() + { + $this->expectException(LogicException::class); + $params = []; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $controller->end(); + }, 'dev/testsession/end', $params); + } + + public function testEndRunning() + { + $params = []; + Director::mockRequest(function ($request) { + $controller = new TestSessionController(); + $controller->setRequest($request); + + $env = $this->testSessionEnvironment; + $state = new stdClass(); + + $env->method('isRunningTests') + ->willReturn(true); + + $env->method('getState') + ->willReturn($state); + + $env->expects($this->once()) + ->method('endTestSession'); + + $html = (string) $controller->end(); + $this->assertContains('Test session ended', $html); + + $this->assertNull($request->getSession()->get('TestSessionId')); + }, 'dev/testsession/end', $params); + } +} diff --git a/tests/unit/TestSessionEnvironmentTest.php b/tests/unit/TestSessionEnvironmentTest.php new file mode 100644 index 0000000..cd85f95 --- /dev/null +++ b/tests/unit/TestSessionEnvironmentTest.php @@ -0,0 +1,162 @@ +unregisterNamedObject(TestSessionEnvironment::class); + Injector::inst()->registerService( + new TestSessionEnvironment, + TestSessionEnvironment::class + ); + $this->testSessionEnvironment = TestSessionEnvironment::singleton(); + } + + public function testApplyStateNoModification() + { + $state = new stdClass(); + $state->testprop = ''; + + $env = new TestSessionEnvironment(); + $afterApply = $env->applyState($state); + + $this->assertSame($state, $afterApply); + } + + public function testApplyStateDBConnect() + { + $state = new stdClass(); + $state->testprop = ''; + + $env = $this->getMockBuilder(TestSessionEnvironment::class) + ->setMethodsExcept(['applyState']) + ->getMock(); + + $env->expects($this->once()) + ->method('connectToDatabase'); + + $env->applyState($state); + } + + public function testApplyStateDatetime() + { + $state = new stdClass(); + $now = new DateTime(); + $now->modify('-1 day'); + $state->datetime = $now->format('Y-m-d H:i:s'); + + $env = new TestSessionEnvironment(); + $env->applyState($state); + $date = new DateTime(); + $this->assertNotEquals($date->getTimestamp(), DBDatetime::now()->getTimestamp()); + $this->assertEquals($now->getTimestamp(), DBDatetime::now()->getTimestamp()); + + $state->datetime = 'invalid format'; + $this->expectExceptionMessage('Invalid date format "invalid format", use yyyy-MM-dd HH:mm:ss'); + $env->applyState($state); + } + + public function testAppyStateMailer() + { + + $state = new stdClass(); + $state->mailer = TestMailer::class; + $env = new TestSessionEnvironment(); + + $env->applyState($state); + + $mailer = Injector::inst()->get(Mailer::class); + $this->assertEquals(TestMailer::class, get_class($mailer)); + + $state->mailer = 'stdClass'; + $this->expectExceptionMessage('Class "stdClass" is not a valid class, or subclass of Mailer'); + $env->applyState($state); + } + + public function testAppyStateStub() + { + + $module = ModuleLoader::getModule('silverstripe/testsession'); + $state = new stdClass(); + + $state->stubfile = $module->getPath() . DIRECTORY_SEPARATOR . 'tests/stubs/teststub.php'; + + $env = new TestSessionEnvironment(); + + $this->assertFalse(defined('TESTSESSION_STUBFILE')); + + $env->applyState($state); + + $this->assertTrue(defined('TESTSESSION_STUBFILE')); + } + + public function testConnectToDatabase() + { + $env = new TestSessionEnvironment(); + $dbExisting = DB::get_conn()->getSelectedDatabase(); + + $env->connectToDatabase(new stdClass); + $db = DB::get_conn()->getSelectedDatabase(); + $this->assertEquals($dbExisting, $db); + + $tempDB = new TempDatabase(); + $dbName = $tempDB->build(); + + $state = new stdClass(); + $state->database = $dbName; + $env->connectToDatabase($state); + $db = DB::get_conn()->getSelectedDatabase(); + $this->assertEquals($dbName, $db); + + $tempDB->kill(); + } + + public function testCreateDatabase() + { + $env = new TestSessionEnvironment(); + $dbExisting = DB::get_conn()->getSelectedDatabase(); + + $state = new stdClass; + + $env->createDatabase($state); + $db = DB::get_conn()->getSelectedDatabase(); + $this->assertEquals($dbExisting, $db); + + + $state = new stdClass(); + $state->createDatabase = true; + $env->createDatabase($state); + $db = DB::get_conn()->getSelectedDatabase(); + $this->assertNotEquals($dbExisting, $db); + + // check the TestSessionState object has been created + $stateObj = TestSessionState::get()->first(); + $this->assertNotNull($stateObj); + $this->assertTrue($stateObj->exists()); + + $tempDB = new TempDatabase('default'); + $tempDB->kill(); + + $state = new stdClass(); + $state->database = $dbExisting; + + $env->connectToDatabase($state); + } +} diff --git a/tests/unit/TestSessionHTTPMiddlewareTest.php b/tests/unit/TestSessionHTTPMiddlewareTest.php new file mode 100644 index 0000000..325de56 --- /dev/null +++ b/tests/unit/TestSessionHTTPMiddlewareTest.php @@ -0,0 +1,103 @@ +registerService( + $this->createMock(TestSessionEnvironment::class), + TestSessionEnvironment::class + ); + $this->testSessionEnvironment = TestSessionEnvironment::singleton(); + } + + public function testProcessNoTestRunning() + { + $env = $this->testSessionEnvironment; + + // setup expected calls on environment + $env->expects($this->never()) + ->method('getState'); + + $env->expects($this->never()) + ->method('applyState'); + + Injector::nest(); + + Injector::inst()->registerService( + $env, + TestSessionEnvironment::class + ); + + $middleware = new TestSessionHTTPMiddleware(); + // Mock request + $session = new Session([]); + $request = new HTTPRequest('GET', '/'); + $request->setSession($session); + $delegate = function () { + // noop + }; + $middleware->process($request, $delegate); + + Injector::unnest(); + } + + public function testProcessTestsRunning() + { + + $state = new stdClass(); + $env = $this->testSessionEnvironment; + + $env->method('isRunningTests') + ->willReturn(true); + + // setup expected calls on environment + $env->expects($this->exactly(2)) + ->method('getState') + ->willReturn($state); + + $env->expects($this->once()) + ->method('applyState'); + + Injector::nest(); + + Injector::inst()->registerService( + $env, + TestSessionEnvironment::class + ); + + $middleware = new TestSessionHTTPMiddleware(); + // Mock request + $session = new Session([]); + $request = new HTTPRequest('GET', '/'); + $request->setSession($session); + $delegate = function () { + // noop + }; + $middleware->process($request, $delegate); + + Injector::unnest(); + } +}