From 682da19a1aec7388fcbc7fdac9f646f9bad54814 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 8 Jun 2017 18:03:51 +1200 Subject: [PATCH 1/6] App object refactor --- src/Versioned.php | 44 +++++++++++++++++++++------------- src/VersionedRequestFilter.php | 21 +++++----------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/src/Versioned.php b/src/Versioned.php index 954b2469..8acf7baa 100644 --- a/src/Versioned.php +++ b/src/Versioned.php @@ -2,6 +2,7 @@ namespace SilverStripe\Versioned; +use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Session; use SilverStripe\Control\Director; @@ -1365,7 +1366,7 @@ public function canViewVersioned($member = null) } // Bypass if site is unsecured - if (Session::get('unsecuredDraftSite')) { + if (Controller::has_curr() && Controller::curr()->getRequest()->getSession()->get('unsecuredDraftSite')) { return true; } @@ -2023,24 +2024,33 @@ public static function can_choose_site_stage($request) * * If neither of these are set, it checks the session, otherwise the stage * is set to 'Live'. + * @param HTTPRequest $request */ - public static function choose_site_stage() + public static function choose_site_stage(HTTPRequest $request) { // Check any pre-existing session mode - $preexistingMode = Session::get('readingMode'); - - // Determine the reading mode - if (isset($_GET['stage'])) { - $stage = ucfirst(strtolower($_GET['stage'])); - if (!in_array($stage, [static::DRAFT, static::LIVE])) { + $preexistingMode = $request->getSession()->get('readingMode') ?: static::DEFAULT_MODE; + $mode = $preexistingMode; + + // Check reading mode + $getStage = $request->getVar('stage'); + if ($getStage) { + if (strcasecmp($getStage, static::DRAFT) === 0) { + $stage = static::DRAFT; + } else { $stage = static::LIVE; } $mode = 'Stage.' . $stage; - } elseif (isset($_GET['archiveDate']) && strtotime($_GET['archiveDate'])) { - $mode = 'Archive.' . $_GET['archiveDate']; - } elseif ($preexistingMode) { - $mode = $preexistingMode; - } else { + } + + // Check archived date + $getArchived = $request->getVar('archiveDate'); + if ($getArchived && strtotime($getArchived)) { + $mode = 'Archive.' . $getArchived; + } + + // Fallback + if (!$mode) { $mode = static::DEFAULT_MODE; } @@ -2048,10 +2058,10 @@ public static function choose_site_stage() Versioned::set_reading_mode($mode); // Try not to store the mode in the session if not needed - if (($preexistingMode && $preexistingMode !== $mode) - || (!$preexistingMode && $mode !== static::DEFAULT_MODE) - ) { - Session::set('readingMode', $mode); + if ($mode === static::DEFAULT_MODE) { + $request->getSession()->clear('readingMode'); + } else { + $request->getSession()->set('readingMode', $mode); } if (!headers_sent() && !Director::is_cli()) { diff --git a/src/VersionedRequestFilter.php b/src/VersionedRequestFilter.php index 2f3b4575..4ac83267 100644 --- a/src/VersionedRequestFilter.php +++ b/src/VersionedRequestFilter.php @@ -8,10 +8,7 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\RequestFilter; -use SilverStripe\Control\Session; use SilverStripe\Core\Convert; -use SilverStripe\Dev\SapphireTest; -use SilverStripe\ORM\DataModel; use SilverStripe\Security\Security; /** @@ -19,11 +16,10 @@ */ class VersionedRequestFilter implements RequestFilter { - public function preRequest(HTTPRequest $request, Session $session, DataModel $model) + public function preRequest(HTTPRequest $request) { - // Bootstrap session so that Session::get() accesses the right instance + // Ensure Controller::curr() is available $dummyController = new Controller(); - $dummyController->setSession($session); $dummyController->setRequest($request); $dummyController->pushCurrent(); @@ -39,22 +35,17 @@ public function preRequest(HTTPRequest $request, Session $session, DataModel $mo // Force output since RequestFilter::preRequest doesn't support response overriding $response = Security::permissionFailure($dummyController, $permissionMessage); - $session->inst_save(); + $request->getSession()->inst_save(); $dummyController->popCurrent(); - // Prevent output in testing - if (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test()) { - throw new HTTPResponse_Exception($response); - } - $response->output(); - die; + throw new HTTPResponse_Exception($response); } - Versioned::choose_site_stage(); + Versioned::choose_site_stage($request); $dummyController->popCurrent(); return true; } - public function postRequest(HTTPRequest $request, HTTPResponse $response, DataModel $model) + public function postRequest(HTTPRequest $request, HTTPResponse $response) { return true; } From 5924fa9d989f930adc1d2655810714fbfa2d5eba Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 12 Jun 2017 15:00:48 +1200 Subject: [PATCH 2/6] Update session usage --- src/Versioned.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Versioned.php b/src/Versioned.php index 8acf7baa..9fe01dad 100644 --- a/src/Versioned.php +++ b/src/Versioned.php @@ -245,7 +245,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider, Resetta public static function reset() { self::$reading_mode = ''; - Session::clear('readingMode'); + Controller::curr()->getRequest()->getSession()->clear('readingMode'); } /** From 75ab11e68fbb67dfeef6564e2c89059c0148019e Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 13 Jun 2017 14:12:05 +1200 Subject: [PATCH 3/6] API Versioned test state --- _config/versionedtests.yml | 10 +++++++ src/Dev/VersionedTestState.php | 36 ++++++++++++++++++++++++++ tests/php/VersionedLazyLoadingTest.php | 2 +- 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 _config/versionedtests.yml create mode 100644 src/Dev/VersionedTestState.php diff --git a/_config/versionedtests.yml b/_config/versionedtests.yml new file mode 100644 index 00000000..f40ed415 --- /dev/null +++ b/_config/versionedtests.yml @@ -0,0 +1,10 @@ +--- +Name: versionedtests +After: + - sapphiretest +--- +SilverStripe\Core\Injector\Injector: + SilverStripe\Dev\SapphireTestState: + properties: + States: + versioned: %$SilverStripe\Versioned\Dev\VersionedTestState diff --git a/src/Dev/VersionedTestState.php b/src/Dev/VersionedTestState.php new file mode 100644 index 00000000..d4641e8a --- /dev/null +++ b/src/Dev/VersionedTestState.php @@ -0,0 +1,36 @@ +readingmode = Versioned::get_reading_mode(); + } + + public function tearDown(SapphireTest $test) + { + Versioned::set_reading_mode($this->readingmode); + } + + public function setUpOnce($class) + { + } + + public function tearDownOnce($class) + { + } +} diff --git a/tests/php/VersionedLazyLoadingTest.php b/tests/php/VersionedLazyLoadingTest.php index 7a65eaf4..02825c8e 100644 --- a/tests/php/VersionedLazyLoadingTest.php +++ b/tests/php/VersionedLazyLoadingTest.php @@ -23,7 +23,7 @@ class VersionedLazyLoadingTest extends SapphireTest 'VersionedTest.yml' ]; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( VersionedTest::$extra_dataobjects, From eff5b596f94d19fb9ee1d7f618a905c73a42cb80 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 19 Jun 2017 13:00:12 +1200 Subject: [PATCH 4/6] Update session references --- src/VersionedRequestFilter.php | 2 +- tests/php/VersionedTest.php | 61 +++++++++++---------- tests/php/VersionedTest/AnotherSubclass.php | 3 + 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/VersionedRequestFilter.php b/src/VersionedRequestFilter.php index 4ac83267..9a4fd5c4 100644 --- a/src/VersionedRequestFilter.php +++ b/src/VersionedRequestFilter.php @@ -35,7 +35,7 @@ public function preRequest(HTTPRequest $request) // Force output since RequestFilter::preRequest doesn't support response overriding $response = Security::permissionFailure($dummyController, $permissionMessage); - $request->getSession()->inst_save(); + $request->getSession()->save(); $dummyController->popCurrent(); throw new HTTPResponse_Exception($response); } diff --git a/tests/php/VersionedTest.php b/tests/php/VersionedTest.php index daff24a5..ca1ef761 100644 --- a/tests/php/VersionedTest.php +++ b/tests/php/VersionedTest.php @@ -2,22 +2,22 @@ namespace SilverStripe\Versioned\Tests; +use DateTime; +use SilverStripe\Control\Director; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse_Exception; -use SilverStripe\ORM\DataObjectSchema; -use SilverStripe\ORM\DB; -use SilverStripe\Versioned\Versioned; -use SilverStripe\Versioned\ChangeSet; -use SilverStripe\Versioned\ChangeSetItem; -use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\FieldType\DBDatetime; -use SilverStripe\Core\Convert; +use SilverStripe\Control\Session; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Control\Director; -use SilverStripe\Control\Session; -use DateTime; +use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; +use SilverStripe\ORM\DB; +use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\Versioned\ChangeSet; +use SilverStripe\Versioned\Versioned; class VersionedTest extends SapphireTest { @@ -970,21 +970,21 @@ public function testLazyLoadFieldsRetrieval() */ public function testReadingPersistent() { - $session = Injector::inst()->create('SilverStripe\\Control\\Session', []); + $session = new Session([]); $adminID = $this->logInWithPermission('ADMIN'); - $session->inst_set('loggedInAs', $adminID); + $session->set('loggedInAs', $adminID); // Set to stage Director::test('/?stage=Stage', null, $session); $this->assertEquals( 'Stage.Stage', - $session->inst_get('readingMode'), + $session->get('readingMode'), 'Check querystring changes reading mode to Stage' ); Director::test('/', null, $session); $this->assertEquals( 'Stage.Stage', - $session->inst_get('readingMode'), + $session->get('readingMode'), 'Check that subsequent requests in the same session remain in Stage mode' ); @@ -992,35 +992,37 @@ public function testReadingPersistent() Director::test('/?stage=Live', null, $session); $this->assertEquals( 'Stage.Live', - $session->inst_get('readingMode'), + $session->get('readingMode'), 'Check querystring changes reading mode to Live' ); Director::test('/', null, $session); $this->assertEquals( 'Stage.Live', - $session->inst_get('readingMode'), + $session->get('readingMode'), 'Check that subsequent requests in the same session remain in Live mode' ); // Test that session doesn't redundantly store the default stage if it doesn't need to - $session2 = Injector::inst()->create('SilverStripe\\Control\\Session', []); - $session2->inst_set('loggedInAs', $adminID); + $session2 = new Session([]); + $session2->set('loggedInAs', $adminID); Director::test('/', null, $session2); - $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData()); + $this->assertArrayNotHasKey('readingMode', $session2->changedData()); Director::test('/?stage=Live', null, $session2); - $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData()); + $this->assertArrayNotHasKey('readingMode', $session2->changedData()); // Test choose_site_stage unset($_GET['stage']); unset($_GET['archiveDate']); - Session::set('readingMode', 'Stage.Stage'); - Versioned::choose_site_stage(); + $request = new HTTPRequest('GET', '/'); + $request->setSession(new Session([])); + $request->getSession()->set('readingMode', 'Stage.Stage'); + Versioned::choose_site_stage($request); $this->assertEquals('Stage.Stage', Versioned::get_reading_mode()); - Session::set('readingMode', 'Archive.2014-01-01'); - Versioned::choose_site_stage(); + $request->getSession()->set('readingMode', 'Archive.2014-01-01'); + Versioned::choose_site_stage($request); $this->assertEquals('Archive.2014-01-01', Versioned::get_reading_mode()); - Session::clear('readingMode'); - Versioned::choose_site_stage(); + $request->getSession()->clear('readingMode'); + Versioned::choose_site_stage($request); $this->assertEquals('Stage.Live', Versioned::get_reading_mode()); } @@ -1030,7 +1032,7 @@ public function testReadingPersistent() public function testReadingModeSecurity() { $this->logOut(); - $this->setExpectedException(HTTPResponse_Exception::class); + $this->expectException(HTTPResponse_Exception::class); $session = Injector::inst()->create(Session::class, []); Director::test('/?stage=Stage', null, $session); } @@ -1064,12 +1066,14 @@ public function testStageCascadeOnRelations() // Stage record - 2 children Versioned::set_stage(Versioned::DRAFT); + /** @var VersionedTest\TestObject $draftPage */ $draftPage = $this->objFromFixture(VersionedTest\TestObject::class, 'page2'); $draftPage->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); $this->assertEquals(2, $draftPage->Children()->Count()); // Live record - no children Versioned::set_stage(Versioned::LIVE); + /** @var VersionedTest\TestObject $livePage */ $livePage = $this->objFromFixture(VersionedTest\TestObject::class, 'page2'); $this->assertEquals(0, $livePage->Children()->Count()); @@ -1124,6 +1128,7 @@ public function testWriteToStage() $this->assertRecordHasLatestVersion($record, 2); // Test subclass with versioned extension only added to the base clases + /** @var VersionedTest\AnotherSubclass $record */ $record = VersionedTest\AnotherSubclass::create(); $record->Title = "Test A"; $record->AnotherField = "Test A"; diff --git a/tests/php/VersionedTest/AnotherSubclass.php b/tests/php/VersionedTest/AnotherSubclass.php index 08b526ae..90d07177 100644 --- a/tests/php/VersionedTest/AnotherSubclass.php +++ b/tests/php/VersionedTest/AnotherSubclass.php @@ -4,6 +4,9 @@ use SilverStripe\Dev\TestOnly; +/** + * @property string $AnotherField + */ class AnotherSubclass extends TestObject implements TestOnly { private static $table_name = 'VersionedTest_AnotherSubclass'; From 91fb952dc49f0639d6462c58700f7f58bb0e5b9a Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 20 Jun 2017 17:17:43 +1200 Subject: [PATCH 5/6] Update namespace --- src/Dev/VersionedTestState.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dev/VersionedTestState.php b/src/Dev/VersionedTestState.php index d4641e8a..2b7e2a59 100644 --- a/src/Dev/VersionedTestState.php +++ b/src/Dev/VersionedTestState.php @@ -3,7 +3,7 @@ namespace SilverStripe\Versioned\Dev; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Dev\TestState; +use SilverStripe\Dev\State\TestState; use SilverStripe\Versioned\Versioned; /** From 449ff3b678622277c1ce1d9951fa12ca06a74956 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 21 Jun 2017 17:00:27 +1200 Subject: [PATCH 6/6] Fix versioned tests --- tests/php/VersionedTest.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/php/VersionedTest.php b/tests/php/VersionedTest.php index ca1ef761..1e1e2b0d 100644 --- a/tests/php/VersionedTest.php +++ b/tests/php/VersionedTest.php @@ -988,16 +988,14 @@ public function testReadingPersistent() 'Check that subsequent requests in the same session remain in Stage mode' ); - // Test live persists + // Doesn't store default stage in session if not necessary Director::test('/?stage=Live', null, $session); - $this->assertEquals( - 'Stage.Live', + $this->assertNull( $session->get('readingMode'), 'Check querystring changes reading mode to Live' ); Director::test('/', null, $session); - $this->assertEquals( - 'Stage.Live', + $this->assertNull( $session->get('readingMode'), 'Check that subsequent requests in the same session remain in Live mode' );