silverstripe · chillu · Jun 22, 2017 · Jun 8, 2017 · Jun 12, 2017 · Jun 13, 2017
diff --git a/_config/versionedtests.yml 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
@@ -0,0 +1,36 @@
+<?php
+
+namespace SilverStripe\Versioned\Dev;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Dev\State\TestState;
+use SilverStripe\Versioned\Versioned;
+
+/**
+ * Decorate sapphire test with versioning
+ */
+class VersionedTestState implements TestState
+{
+    /**
+     * @var string
+     */
+    protected $readingmode = null;
+
+    public function setUp(SapphireTest $test)
+    {
+        $this->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/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;
@@ -244,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');
     }
 
     /**
@@ -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,35 +2024,44 @@ 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;
         }
 
         // Save reading mode
         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
@@ -8,22 +8,18 @@
 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;
 
 /**
  * Initialises the versioned stage when a request is made.
  */
 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()->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;
     }

diff --git 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,

diff --git 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,57 +970,57 @@ 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'
         );
 
-        // Test live persists
+        // Doesn't store default stage in session if not necessary
         Director::test('/?stage=Live', null, $session);
-        $this->assertEquals(
-            'Stage.Live',
-            $session->inst_get('readingMode'),
+        $this->assertNull(
+            $session->get('readingMode'),
             'Check querystring changes reading mode to Live'
         );
         Director::test('/', null, $session);
-        $this->assertEquals(
-            'Stage.Live',
-            $session->inst_get('readingMode'),
+        $this->assertNull(
+            $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 +1030,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 +1064,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 +1126,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
@@ -4,6 +4,9 @@
 
 use SilverStripe\Dev\TestOnly;
 
+/**
+ * @property string $AnotherField
+ */
 class AnotherSubclass extends TestObject implements TestOnly
 {
     private static $table_name = 'VersionedTest_AnotherSubclass';