From 4c5de82625f6705834052cc79533cb2a41f31e28 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Wed, 11 Jun 2014 16:14:56 +1200
Subject: [PATCH] Versioned no longer sets redundant session data

---
 model/Versioned.php           | 34 ++++++++++++++++++++++++----------
 tests/model/VersionedTest.php |  7 +++++++
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/model/Versioned.php b/model/Versioned.php
index 03567f95c..8db06e8d0 100644
--- a/model/Versioned.php
+++ b/model/Versioned.php
@@ -27,6 +27,11 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
 	 */
 	protected $liveStage;
 	
+	/**
+	 * The default reading mode
+	 */
+	const DEFAULT_MODE = 'Stage.Live';
+	
 	/**
 	 * A version that a DataObject should be when it is 'migrating',
 	 * that is, when it is in the process of moving from one stage to another.
@@ -924,6 +929,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
 		
 	//-----------------------------------------------------------------------------------------------//
 	
+	
 	/**
 	 * Choose the stage the site is currently on.
 	 *
@@ -939,28 +945,36 @@ class Versioned extends DataExtension implements TemplateGlobalProvider {
 	 * @param Session $session Optional session within which to store the resulting stage
 	 */
 	public static function choose_site_stage($session = null) {
-		// Default to Live
-		$mode = 'Stage.Live';
+		// Check any pre-existing session mode
+		$preexistingMode = $session
+			? $session->inst_get('readingMode')
+			: Session::get('readingMode');
 		
-		// Get reading mode
+		// Determine the reading mode
 		if(isset($_GET['stage'])) {
 			$stage = ucfirst(strtolower($_GET['stage']));
 			if(!in_array($stage, array('Stage', 'Live'))) $stage = 'Live';
 			$mode = 'Stage.' . $stage;
 		} elseif (isset($_GET['archiveDate']) && strtotime($_GET['archiveDate'])) {
 			$mode = 'Archive.' . $_GET['archiveDate'];
-		} elseif($session) {
-			$mode = $session->inst_get('readingMode') ?: $mode;
+		} elseif($preexistingMode) {
+			$mode = $preexistingMode;
 		} else {
-			$mode = Session::get('readingMode') ?: $mode;
+			$mode = self::DEFAULT_MODE;
 		}
 		
 		// Save reading mode
 		Versioned::set_reading_mode($mode);
-		if($session) {
-			$session->inst_set('readingMode', $mode);
-		} else {
-			Session::set('readingMode', $mode);
+		
+		// Try not to store the mode in the session if not needed
+		if(($preexistingMode && $preexistingMode !== $mode)
+			|| (!$preexistingMode && $mode !== self::DEFAULT_MODE)
+		) {
+			if($session) {
+				$session->inst_set('readingMode', $mode);
+			} else {
+				Session::set('readingMode', $mode);
+			}
 		}
 
 		if(!headers_sent() && !Director::is_cli()) {
diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php
index b8330d874..0b7246cac 100644
--- a/tests/model/VersionedTest.php
+++ b/tests/model/VersionedTest.php
@@ -567,6 +567,13 @@ class VersionedTest extends SapphireTest {
 			'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 = new Session(array());
+		Director::test('/', null, $session2);
+		$this->assertEmpty($session2->inst_changedData());
+		Director::test('/?stage=Live', null, $session2);
+		$this->assertEmpty($session2->inst_changedData());
+		
 		// Test choose_site_stage
 		Session::set('readingMode', 'Stage.Stage');
 		Versioned::choose_site_stage();