From 1ab843776d2bf514f41c32cedca9367fe3fb059d Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 8 Jun 2017 18:01:46 +1200 Subject: [PATCH 1/5] App object refactor --- .travis.yml | 34 --------------- src/TestSessionController.php | 25 ++++++----- src/TestSessionEnvironment.php | 11 +++-- src/TestSessionRequestFilter.php | 18 ++++---- tests/TestSessionStubCodeWriterTest.php | 55 ------------------------- 5 files changed, 29 insertions(+), 114 deletions(-) delete mode 100644 .travis.yml delete mode 100644 tests/TestSessionStubCodeWriterTest.php diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index a6d3823..0000000 --- a/.travis.yml +++ /dev/null @@ -1,34 +0,0 @@ -language: php - -sudo: false - -env: - global: - - COMPOSER_ROOT_VERSION=2.0.x-dev - - CORE_RELEASE=master - -matrix: - include: - - php: 5.6 - env: - - PHPUNIT_TEST=1 - - DB=PGSQL - - php: 5.6 - env: - - PHPUNIT_TEST=1 - - DB=MYSQL - - php: 5.6 - env: - - PHPCS_TEST=1 - - DB=MYSQL - -before_script: - - if [[ $PHPCS_TEST ]]; then pyrus install pear/PHP_CodeSniffer; fi - - phpenv rehash - - git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support - - php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss - - cd ~/builds/ss - -script: - - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit testsession/tests/; fi - - if [[ $PHPCS_TEST ]]; then (cd testsession && composer run-script lint); fi diff --git a/src/TestSessionController.php b/src/TestSessionController.php index 9ec1112..f8579bd 100644 --- a/src/TestSessionController.php +++ b/src/TestSessionController.php @@ -4,10 +4,8 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\Director; -use SilverStripe\Control\Session; use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Injector\Injector; -use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\CheckboxField; use SilverStripe\Forms\DatetimeField; @@ -104,14 +102,14 @@ public function index() */ public function start() { - $params = $this->request->requestVars(); + $params = $this->getRequest()->requestVars(); if (!empty($params['globalTestSession'])) { $id = null; } else { $generator = Injector::inst()->get(RandomGenerator::class); $id = substr($generator->randomToken(), 0, 10); - Session::set('TestSessionId', $id); + $this->getRequest()->getSession()->set('TestSessionId', $id); } // Convert datetime from form object into a single string @@ -181,14 +179,15 @@ public function browsersessionstate($request) throw new LogicException('No query parameters detected'); } - $sessionStates = (array)Session::get('_TestSessionController.BrowserSessionState'); + $session = $this->getRequest()->getSession(); + $sessionStates = (array)$session->get('_TestSessionController.BrowserSessionState'); foreach ($newSessionStates as $k => $v) { - Session::set($k, $v); + $session->set($k, $v); } // Track which state we're setting so we can unset later in end() - Session::set('_TestSessionController.BrowserSessionState', array_merge($sessionStates, $newSessionStates)); + $session->set('_TestSessionController.BrowserSessionState', array_merge($sessionStates, $newSessionStates)); } public function StartForm() @@ -341,17 +340,17 @@ public function end() } $this->environment->endTestSession(); - Session::clear('TestSessionId'); + $session = Controller::curr()->getRequest()->getSession(); + $session->clear('TestSessionId'); // Clear out all PHP session states which have been set previously - if ($sessionStates = Session::get('_TestSessionController.BrowserSessionState')) { + if ($sessionStates = $session->get('_TestSessionController.BrowserSessionState')) { foreach ($sessionStates as $k => $v) { - Session::clear($k); + $session->clear($k); } - Session::clear('_TestSessionController'); + $session->clear('_TestSessionController'); } - return $this->renderWith('TestSession_end'); } @@ -394,7 +393,7 @@ protected function getDatabaseTemplates($path = null) $templates = array(); if (!$path) { - $path = $this->config()->database_templates_path; + $path = $this->config()->get('database_templates_path'); } // TODO Remove once we can set BASE_PATH through the config layer diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php index dcf2183..3a15f3c 100644 --- a/src/TestSessionEnvironment.php +++ b/src/TestSessionEnvironment.php @@ -3,6 +3,7 @@ namespace SilverStripe\TestSession; use SilverStripe\Control\Director; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Session; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Extensible; @@ -77,11 +78,15 @@ class TestSessionEnvironment public function __construct($id = null) { $this->constructExtensions(); - if ($id) { $this->id = $id; - } else { - Session::start(); + } + } + + public function init(HTTPRequest $request) + { + if (!$this->id) { + $request->getSession()->start(); // $_SESSION != Session::get() in some execution paths, suspect Controller->pushCurrent() // as part of the issue, easiest resolution is to use session directly for now $this->id = (isset($_SESSION['TestSessionId'])) ? $_SESSION['TestSessionId'] : null; diff --git a/src/TestSessionRequestFilter.php b/src/TestSessionRequestFilter.php index bbf3bb3..fe48628 100644 --- a/src/TestSessionRequestFilter.php +++ b/src/TestSessionRequestFilter.php @@ -2,17 +2,16 @@ namespace SilverStripe\TestSession; +use SilverStripe\Control\Director; use SilverStripe\Control\Email\Email; use SilverStripe\Control\Email\Mailer; -use SilverStripe\ORM\DataModel; -use SilverStripe\ORM\FieldType\DBDatetime; -use SilverStripe\ORM\DB; -use SilverStripe\Core\Injector\Injector; -use SilverStripe\Control\Session; -use SilverStripe\Control\Director; -use SilverStripe\Control\RequestFilter; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; +use SilverStripe\Control\RequestFilter; +use SilverStripe\Control\Session; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\ORM\DB; +use SilverStripe\ORM\FieldType\DBDatetime; /** * Sets state previously initialized through {@link TestSessionController}. @@ -29,9 +28,10 @@ public function __construct() $this->testSessionEnvironment = TestSessionEnvironment::singleton(); } - public function preRequest(HTTPRequest $request, Session $session, DataModel $model) + public function preRequest(HTTPRequest $request) { $isRunningTests = $this->testSessionEnvironment->isRunningTests(); + $this->testSessionEnvironment->init($request); if (!$isRunningTests) { return; } @@ -68,7 +68,7 @@ public function preRequest(HTTPRequest $request, Session $session, DataModel $mo } } - public function postRequest(HTTPRequest $request, HTTPResponse $response, DataModel $model) + public function postRequest(HTTPRequest $request, HTTPResponse $response) { if (!$this->testSessionEnvironment->isRunningTests()) { return; diff --git a/tests/TestSessionStubCodeWriterTest.php b/tests/TestSessionStubCodeWriterTest.php deleted file mode 100644 index 96acf7c..0000000 --- a/tests/TestSessionStubCodeWriterTest.php +++ /dev/null @@ -1,55 +0,0 @@ -write('foo();', false); - $this->assertFileExists($file); - $this->assertEquals( - file_get_contents($writer->getFilePath()), - "write('foo();', false); - $writer->write('bar();', false); - $this->assertFileExists($file); - $this->assertEquals( - file_get_contents($writer->getFilePath()), - "write('foo();', false); - $this->assertFileExists($file); - $writer->reset(); - $this->assertFileNotExists($file); - } -} From 9ec863f91777e7ff2fa90a17750ecb5ce197c539 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 12 Jun 2017 14:59:20 +1200 Subject: [PATCH 2/5] Fix session accessors --- src/TestSessionEnvironment.php | 4 ++-- src/TestSessionRequestFilter.php | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php index 3a15f3c..17b49ba 100644 --- a/src/TestSessionEnvironment.php +++ b/src/TestSessionEnvironment.php @@ -86,10 +86,10 @@ public function __construct($id = null) public function init(HTTPRequest $request) { if (!$this->id) { - $request->getSession()->start(); + $request->getSession()->init(); // $_SESSION != Session::get() in some execution paths, suspect Controller->pushCurrent() // as part of the issue, easiest resolution is to use session directly for now - $this->id = (isset($_SESSION['TestSessionId'])) ? $_SESSION['TestSessionId'] : null; + $this->id = $request->getSession()->get('TestSessionId'); } } diff --git a/src/TestSessionRequestFilter.php b/src/TestSessionRequestFilter.php index fe48628..7999aaf 100644 --- a/src/TestSessionRequestFilter.php +++ b/src/TestSessionRequestFilter.php @@ -8,7 +8,6 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\RequestFilter; -use SilverStripe\Control\Session; use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; @@ -76,7 +75,7 @@ public function postRequest(HTTPRequest $request, HTTPResponse $response) // Store PHP session $state = $this->testSessionEnvironment->getState(); - $state->session = Session::get_all(); + $state->session = $request->getSession()->getAll(); $this->testSessionEnvironment->applyState($state); } } From 1651d5695a857d306f9e6b23f1e8ce24f40e4a62 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 13 Jun 2017 15:00:32 +1200 Subject: [PATCH 3/5] Update testsession for SapphireTest changes --- src/TestSessionEnvironment.php | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php index 17b49ba..ccc395a 100644 --- a/src/TestSessionEnvironment.php +++ b/src/TestSessionEnvironment.php @@ -2,22 +2,21 @@ namespace SilverStripe\TestSession; +use Exception; +use InvalidArgumentException; +use LogicException; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; -use SilverStripe\Control\Session; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\FixtureFactory; use SilverStripe\Dev\SapphireTest; -use SilverStripe\ORM\DB; use SilverStripe\ORM\DatabaseAdmin; +use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Versioned\Versioned; -use InvalidArgumentException; -use LogicException; -use Exception; use stdClass; /** @@ -94,7 +93,7 @@ public function init(HTTPRequest $request) } /** - * @return String Absolute path to the file persisting our state. + * @return string Absolute path to the file persisting our state. */ public function getFilePath() { @@ -401,8 +400,6 @@ public function endTestSession() } // End test session mode $this->resetDatabaseName(); - - SapphireTest::set_is_running_test(false); } $this->removeStateFile(); From f5ef9f4fbf0b80ab384e9a65fe869f528513cc38 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Jun 2017 21:04:10 +1200 Subject: [PATCH 4/5] API Use new TempDatabase service --- src/TestSessionController.php | 9 ++++++--- src/TestSessionEnvironment.php | 19 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/TestSessionController.php b/src/TestSessionController.php index f8579bd..1f862f5 100644 --- a/src/TestSessionController.php +++ b/src/TestSessionController.php @@ -16,6 +16,7 @@ use SilverStripe\Forms\HiddenField; use SilverStripe\Forms\TextField; use SilverStripe\ORM\ArrayList; +use SilverStripe\ORM\Connect\TempDatabase; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\Security\Permission; @@ -315,8 +316,9 @@ public function clear() $this->extend('onBeforeClear'); - if (SapphireTest::using_temp_db()) { - SapphireTest::empty_temp_db(); + $tempDB = new TempDatabase(); + if ($tempDB->isUsed()) { + $tempDB->clearAllData(); } if (isset($_SESSION['_testsession_codeblocks'])) { @@ -359,7 +361,8 @@ public function end() */ public function isTesting() { - return SapphireTest::using_temp_db(); + $tempDB = new TempDatabase(); + return $tempDB->isUsed(); } /** diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php index ccc395a..fb74c9e 100644 --- a/src/TestSessionEnvironment.php +++ b/src/TestSessionEnvironment.php @@ -12,7 +12,7 @@ use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\FixtureFactory; -use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\Connect\TempDatabase; use SilverStripe\ORM\DatabaseAdmin; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; @@ -195,7 +195,7 @@ public function applyState($state) $this->extend('onBeforeApplyState', $state); // back up source - global $databaseConfig; + $databaseConfig = DB::getConfig(); $this->oldDatabaseName = $databaseConfig['database']; // Load existing state from $this->state into $state, if there is any @@ -243,7 +243,8 @@ public function applyState($state) if (!$dbExists) { // Create a new one with a randomized name - $dbName = SapphireTest::create_temp_db(); + $tempDB = new TempDatabase(); + $dbName = $tempDB->build(); $state->database = $dbName; // In case it's changed by the call to SapphireTest::create_temp_db(); @@ -355,9 +356,12 @@ public function loadFromFile() $this->applyState($json); } catch (Exception $e) { - throw new \Exception("A test session appears to be in progress, but we can't retrieve the details. " - . "Try removing the " . $this->getFilePath() . " file. Inner " - . "error: " . $e->getMessage()); + throw new Exception( + "A test session appears to be in progress, but we can't retrieve the details.\n" + . "Try removing the " . $this->getFilePath() . " file.\n" + . "Inner error: " . $e->getMessage() . "\n" + . "Stacktrace: " . $e->getTraceAsString() + ); } } } @@ -389,7 +393,8 @@ public function endTestSession() { $this->extend('onBeforeEndTestSession'); - if (SapphireTest::using_temp_db()) { + $tempDB = new TempDatabase(); + if ($tempDB->isUsed()) { $state = $this->getState(); $dbConn = DB::get_schema(); $dbExists = $dbConn->databaseExists($state->database); From 2868e6bd3e5b1f102e87fe775c01ae80d750b00f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 22 Jun 2017 15:15:17 +1200 Subject: [PATCH 5/5] Update references to deprecated global --- _config.php | 28 ++++++++++++++-------------- src/TestSessionEnvironment.php | 4 ++-- src/TestSessionRequestFilter.php | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/_config.php b/_config.php index 7bd9a34..a372c88 100644 --- a/_config.php +++ b/_config.php @@ -10,21 +10,21 @@ * This closure will run every time a Resque_Event is forked (just before it is forked, so it applies to the parent * and child process). */ -if(class_exists('Resque_Event') && class_exists('SSResqueRun')) { - Resque_Event::listen('beforeFork', function($data) { - global $databaseConfig; +if (class_exists('Resque_Event') && class_exists('SSResqueRun')) { + Resque_Event::listen('beforeFork', function ($data) { + $databaseConfig = DB::getConfig(); - // Reconnect to the database - this may connect to the old DB first, but is required because these processes - // are long-lived, and MySQL connections often get closed in between worker runs. We need to connect before - // calling {@link TestSessionEnvironment::loadFromFile()}. - DB::connect($databaseConfig); + // Reconnect to the database - this may connect to the old DB first, but is required because these processes + // are long-lived, and MySQL connections often get closed in between worker runs. We need to connect before + // calling {@link TestSessionEnvironment::loadFromFile()}. + DB::connect($databaseConfig); - $testEnv = TestSessionEnvironment::singleton(); + $testEnv = TestSessionEnvironment::singleton(); - if($testEnv->isRunningTests()) { - $testEnv->loadFromFile(); - } else { - $testEnv->endTestSession(); - } - }); + if ($testEnv->isRunningTests()) { + $testEnv->loadFromFile(); + } else { + $testEnv->endTestSession(); + } + }); } diff --git a/src/TestSessionEnvironment.php b/src/TestSessionEnvironment.php index fb74c9e..c4e60ca 100644 --- a/src/TestSessionEnvironment.php +++ b/src/TestSessionEnvironment.php @@ -450,9 +450,9 @@ public function loadFixtureIntoDb($fixtureFile) public function resetDatabaseName() { if ($this->oldDatabaseName) { - global $databaseConfig; - + $databaseConfig = DB::getConfig(); $databaseConfig['database'] = $this->oldDatabaseName; + DB::setConfig($databaseConfig); $conn = DB::get_conn(); diff --git a/src/TestSessionRequestFilter.php b/src/TestSessionRequestFilter.php index 7999aaf..cda508c 100644 --- a/src/TestSessionRequestFilter.php +++ b/src/TestSessionRequestFilter.php @@ -58,7 +58,7 @@ public function preRequest(HTTPRequest $request) $file = $testState->stubfile; if (!Director::isLive() && $file && file_exists($file)) { // Connect to the database so the included code can interact with it - global $databaseConfig; + $databaseConfig = DB::getConfig(); if ($databaseConfig) { DB::connect($databaseConfig); }