diff --git a/_config/versionedstate.yml b/_config/versionedstate.yml new file mode 100644 index 000000000..fa415ad50 --- /dev/null +++ b/_config/versionedstate.yml @@ -0,0 +1,9 @@ +--- +Name: versionedstate +--- +RequestHandler: + extensions: + - VersionedStateExtension +DataObject: + extensions: + - VersionedStateExtension diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 16219d5f5..fb9a49043 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -460,8 +460,11 @@ class LeftAndMain extends Controller implements PermissionProvider { // replaced TableListField.ss or Form.ss. Config::inst()->update('SSViewer', 'theme_enabled', false); - //set the reading mode for the admin to stage - Versioned::reading_stage('Stage'); + // Set the current reading mode + Versioned::reading_stage(Versioned::DRAFT); + + // Set default reading mode to suppress ?stage=Stage querystring params in CMS + Versioned::set_default_reading_mode(Versioned::get_reading_mode()); } public function handleRequest(SS_HTTPRequest $request, DataModel $model = null) { @@ -560,7 +563,7 @@ class LeftAndMain extends Controller implements PermissionProvider { '/', // trailing slash needed if $action is null! "$action" ); - $this->extend('updateLink', $link); + $this->extend('updateLink', $link, $action); return $link; } diff --git a/cache/Cache.php b/cache/Cache.php index 88015c096..672355261 100644 --- a/cache/Cache.php +++ b/cache/Cache.php @@ -146,7 +146,7 @@ class SS_Cache { * @param string $frontend (optional) The type of Zend_Cache frontend * @param array $frontendOptions (optional) Any frontend options to use. * - * @return Zend_Cache_Frontend The cache object + * @return Zend_Cache_Core The cache object */ public static function factory($for, $frontend='Output', $frontendOptions=null) { self::init(); diff --git a/control/Controller.php b/control/Controller.php index 9a3012d49..1d9e4a9a7 100644 --- a/control/Controller.php +++ b/control/Controller.php @@ -89,13 +89,6 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { $this->baseInitCalled = true; } - /** - * Returns a link to this controller. Overload with your own Link rules if they exist. - */ - public function Link() { - return get_class($this) .'/'; - } - /** * Executes this controller, and return an {@link SS_HTTPResponse} object with the result. * diff --git a/control/Director.php b/control/Director.php index 1e8fd439b..aef7f868f 100644 --- a/control/Director.php +++ b/control/Director.php @@ -222,7 +222,7 @@ class Director implements TemplateGlobalProvider { // These are needed so that calling Director::test() doesnt muck with whoever is calling it. // Really, it's some inappropriate coupling and should be resolved by making less use of statics - $oldStage = Versioned::current_stage(); + $oldMode = Versioned::get_reading_mode(); $getVars = array(); if(!$httpMethod) $httpMethod = ($postVars || is_array($postVars)) ? "POST" : "GET"; @@ -248,7 +248,7 @@ class Director implements TemplateGlobalProvider { // Set callback to invoke prior to return $onCleanup = function() use( $existingRequestVars, $existingGetVars, $existingPostVars, $existingSessionVars, - $existingCookies, $existingServer, $existingRequirementsBackend, $oldStage + $existingCookies, $existingServer, $existingRequirementsBackend, $oldMode ) { // Restore the superglobals $_REQUEST = $existingRequestVars; @@ -262,7 +262,7 @@ class Director implements TemplateGlobalProvider { // These are needed so that calling Director::test() doesnt muck with whoever is calling it. // Really, it's some inappropriate coupling and should be resolved by making less use of statics - Versioned::reading_stage($oldStage); + Versioned::set_reading_mode($oldMode); Injector::unnest(); // Restore old CookieJar, etc Config::unnest(); diff --git a/control/RequestHandler.php b/control/RequestHandler.php index 429c2aa28..0f94bd3eb 100644 --- a/control/RequestHandler.php +++ b/control/RequestHandler.php @@ -36,6 +36,14 @@ class RequestHandler extends ViewableData { /** + * Optional url_segment for this request handler + * + * @config + * @var string|null + */ + private static $url_segment = null; + + /** * @var SS_HTTPRequest $request The request object that the controller was called with. * Set in {@link handleRequest()}. Useful to generate the {} */ @@ -496,4 +504,20 @@ class RequestHandler extends ViewableData { public function setRequest($request) { $this->request = $request; } + + /** + * Returns a link to this controller. Overload with your own Link rules if they exist. + * + * @param string $action Optional action (soft-supported via func_get_args) + * @return string + */ + public function Link() { + $action = func_num_args() ? func_get_arg(0) : null; + $urlSegment = $this->config()->get('url_segment') ?: get_class($this); + $link = Controller::join_links($urlSegment, $action, '/'); + + // Give extensions the chance to modify by reference + $this->extend('updateLink', $link); + return $link; + } } diff --git a/control/VersionedRequestFilter.php b/control/VersionedRequestFilter.php index 40b1b05a8..605b24b54 100644 --- a/control/VersionedRequestFilter.php +++ b/control/VersionedRequestFilter.php @@ -38,7 +38,7 @@ class VersionedRequestFilter implements RequestFilter { die; } - Versioned::choose_site_stage(); + Versioned::choose_site_stage($request); $dummyController->popCurrent(); return true; } diff --git a/core/Object.php b/core/Object.php index e321d9f24..0a28bae0a 100644 --- a/core/Object.php +++ b/core/Object.php @@ -572,13 +572,6 @@ abstract class SS_Object { Config::inst()->extraConfigSourcesChanged($class); Injector::inst()->unregisterNamedObject($class); - - // load statics now for DataObject classes - if(is_subclass_of($class, 'DataObject')) { - if(!is_subclass_of($extensionClass, 'DataExtension')) { - user_error("$extensionClass cannot be applied to $class without being a DataExtension", E_USER_ERROR); - } - } } @@ -653,7 +646,11 @@ abstract class SS_Object { // -------------------------------------------------------------------------------------------------------------- - private static $unextendable_classes = array('SS_Object', 'Object', 'ViewableData', 'RequestHandler'); + private static $unextendable_classes = array( + 'SS_Object', + 'Object', + 'ViewableData', + ); static public function get_extra_config_sources($class = null) { if($class === null) $class = get_called_class(); diff --git a/dev/FunctionalTest.php b/dev/FunctionalTest.php index 1ad9471bb..aef70cf96 100644 --- a/dev/FunctionalTest.php +++ b/dev/FunctionalTest.php @@ -381,10 +381,12 @@ class FunctionalTest extends SapphireTest { if($enabled) { $this->session()->inst_set('readingMode', 'Stage.Stage'); $this->session()->inst_set('unsecuredDraftSite', true); + Versioned::set_draft_site_secured(false); } else { $this->session()->inst_set('readingMode', 'Stage.Live'); $this->session()->inst_set('unsecuredDraftSite', false); + Versioned::set_draft_site_secured(true); } } diff --git a/forms/Form.php b/forms/Form.php index 096e1a723..03c8fb93c 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -1070,11 +1070,18 @@ class Form extends RequestHandler { public function FormAction() { if ($this->formActionPath) { return $this->formActionPath; - } elseif($this->controller->hasMethod("FormObjectLink")) { - return $this->controller->FormObjectLink($this->name); - } else { - return Controller::join_links($this->controller->Link(), $this->name); } + + // Respect FormObjectLink() method + if($this->controller->hasMethod("FormObjectLink")) { + $link = $this->controller->FormObjectLink($this->getName()); + } else { + $link = Controller::join_links($this->controller->Link(), $this->getName()); + } + + // Join with action and decorate + $this->extend('updateLink', $link); + return $link; } /** diff --git a/forms/FormField.php b/forms/FormField.php index 044de543b..9c280d4c3 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -274,7 +274,9 @@ class FormField extends RequestHandler { * @return string */ public function Link($action = null) { - return Controller::join_links($this->form->FormAction(), 'field/' . $this->name, $action); + $link = Controller::join_links($this->form->FormAction(), 'field/' . $this->name, $action); + $this->extend('updateLink', $link, $action); + return $link; } /** @@ -470,7 +472,7 @@ class FormField extends RequestHandler { if($this->Message()) { $classes[] .= 'holder-' . $this->MessageType(); } - + $this->extend('updateExtraClass', $classes); return implode(' ', $classes); diff --git a/model/Versioned.php b/model/Versioned.php index d9b8e12cc..ff0209389 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -4,17 +4,15 @@ * The Versioned extension allows your DataObjects to have several versions, allowing you to rollback changes and view * history. An example of this is the pages used in the CMS. * - * @property int $Version - * * @package framework * @subpackage model * - * @property DataObject owner - * @property int RecordID - * @property int Version - * @property bool WasPublished - * @property int AuthorID - * @property int PublisherID + * @property DataObject $owner + * @property int $RecordID + * @property int $Version + * @property bool $WasPublished + * @property int $AuthorID + * @property int $PublisherID */ class Versioned extends DataExtension implements TemplateGlobalProvider { @@ -41,6 +39,16 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { */ const DEFAULT_MODE = 'Stage.Live'; + /** + * The Public stage. + */ + const LIVE = 'Live'; + + /** + * The draft (default) stage + */ + const DRAFT = 'Stage'; + /** * 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. @@ -54,9 +62,37 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { */ protected static $cache_versionnumber; - /** @var string */ + /** + * Set if draft site is secured or not. Fails over to + * $draft_site_secured if unset + * + * @var bool|null + */ + protected static $is_draft_site_secured = null; + + /** + * Default config for $is_draft_site_secured + * + * @config + * @var bool + */ + private static $draft_site_secured = true; + + /** + * Current reading mode + * + * @var string + */ protected static $reading_mode = null; + /** + * Default reading mode, if none set. + * Any modes which differ to this value should be assigned via querystring / session (if enabled) + * + * @var null + */ + protected static $default_reading_mode = self::DEFAULT_MODE; + /** * Flag which is temporarily changed during the write() process to influence augmentWrite() behaviour. If set to * true, no new version will be created for the following write. Needs to be public as other classes introspect this @@ -150,11 +186,24 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { */ private static $non_live_permissions = array('CMS_ACCESS_LeftAndMain', 'CMS_ACCESS_CMSMain', 'VIEW_DRAFT_CONTENT'); + /** + * Use PHP's session storage for the "reading mode" and "unsecuredDraftSite", + * instead of explicitly relying on the "stage" query parameter. + * This is considered bad practice, since it can cause draft content + * to leak under live URLs to unauthorised users, depending on HTTP cache settings. + * + * @config + * @var bool + */ + private static $use_session = true; + /** * Reset static configuration variables to their default values. */ public static function reset() { - self::$reading_mode = ''; + self::set_reading_mode(null); + self::set_default_reading_mode(null); + self::set_draft_site_secured(null); Session::clear('readingMode'); } @@ -184,18 +233,13 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @param DataQuery $dataQuery */ public function augmentDataQueryCreation(SQLQuery &$query, DataQuery &$dataQuery) { - $parts = explode('.', Versioned::get_reading_mode()); - - if($parts[0] == 'Archive') { - $dataQuery->setQueryParam('Versioned.mode', 'archive'); - $dataQuery->setQueryParam('Versioned.date', $parts[1]); - - } else if($parts[0] == 'Stage' && $parts[1] != $this->defaultStage - && array_search($parts[1],$this->stages) !== false) { - - $dataQuery->setQueryParam('Versioned.mode', 'stage'); - $dataQuery->setQueryParam('Versioned.stage', $parts[1]); - } + // Convert reading mode to dataquery params and assign + $args = VersionedReadingMode::toDataQueryParams(Versioned::get_reading_mode()); + if ($args) { + foreach ($args as $key => $value) { + $dataQuery->setQueryParam($key, $value); + } + } } /** @@ -253,24 +297,30 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { // Reading a specific stage (Stage or Live) case 'stage': + // Check stage is available on object $stage = $dataQuery->getQueryParam('Versioned.stage'); - if($stage && ($stage != $this->defaultStage)) { - foreach($query->getFrom() as $table => $dummy) { - // Only rewrite table names that are actually part of the subclass tree - // This helps prevent rewriting of other tables that get joined in, in - // particular, many_many tables - if(class_exists($table) && ($table == $this->owner->class - || is_subclass_of($table, $this->owner->class) - || is_subclass_of($this->owner->class, $table))) { - $query->renameTable($table, $table . '_' . $stage); - } + if (!$stage || !in_array($stage, $this->stages) || $stage === $this->defaultStage) { + break; + } + foreach ($query->getFrom() as $table => $dummy) { + // Only rewrite table names that are actually part of the subclass tree + // This helps prevent rewriting of other tables that get joined in, in + // particular, many_many tables + if (class_exists($table) && ($table == $this->owner->class + || is_subclass_of($table, $this->owner->class) + || is_subclass_of($this->owner->class, $table))) { + $query->renameTable($table, $table . '_' . $stage); } } break; // Reading a specific stage, but only return items that aren't in any other stage case 'stage_unique': + // Check stage is available on object $stage = $dataQuery->getQueryParam('Versioned.stage'); + if (!$stage || !in_array($stage, $this->stages) || $stage === $this->defaultStage) { + break; + } // Recurse to do the default stage behavior (must be first, we rely on stage renaming happening before // below) @@ -791,15 +841,15 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return bool False is returned if the current viewing mode denies visibility */ public function canViewVersioned($member = null) { - // Bypass when live stage - $mode = $this->owner->getSourceQueryParam("Versioned.mode"); - $stage = $this->owner->getSourceQueryParam("Versioned.stage"); - if ($mode === 'stage' && $stage === static::get_live_stage()) { + // Bypass if site is unsecured + if (!self::get_draft_site_secured()) { return true; } - // Bypass if site is unsecured - if (Session::get('unsecuredDraftSite')) { + // Bypass when live stage + $mode = $this->owner->getSourceQueryParam("Versioned.mode"); + $stage = $this->owner->getSourceQueryParam("Versioned.stage"); + if ($mode === 'stage' && $stage === self::LIVE) { return true; } @@ -1055,6 +1105,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $query = $list->dataQuery()->query(); + $baseTable = null; foreach($query->getFrom() as $table => $tableJoin) { if(is_string($tableJoin) && $tableJoin[0] == '"') { $baseTable = str_replace('"','',$tableJoin); @@ -1139,6 +1190,16 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return true; } + // Request is allowed if unsecuredDraftSite is enabled + if (!static::get_draft_site_secured()) { + return true; + } + + // Predict if choose_site_stage() will allow unsecured draft assignment by session + if (Config::inst()->get('Versioned', 'use_session') && Session::get('unsecuredDraftSite')) { + return true; + } + // Check permissions with member ID in session. $member = Member::currentUser(); $permissions = Config::inst()->get(get_called_class(), 'non_live_permissions'); @@ -1150,36 +1211,45 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * - If $_GET['stage'] is set, then it will use that stage, and store it in the session. * - If $_GET['archiveDate'] is set, it will use that date, and store it in the session. * - If neither of these are set, it checks the session, otherwise the stage is set to 'Live'. + * + * @param SS_HTTPRequest|null $request */ - public static function choose_site_stage() { - // 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, array('Stage', 'Live'))) $stage = 'Live'; - $mode = 'Stage.' . $stage; - } elseif (isset($_GET['archiveDate']) && strtotime($_GET['archiveDate'])) { - $mode = 'Archive.' . $_GET['archiveDate']; - } elseif($preexistingMode) { - $mode = $preexistingMode; - } else { - $mode = self::DEFAULT_MODE; + public static function choose_site_stage(SS_HTTPRequest $request = null) { + if (!$request) { + throw new InvalidArgumentException("Request not found"); } + $mode = static::get_default_reading_mode(); - // Save reading mode - Versioned::set_reading_mode($mode); + // Check any pre-existing session mode + $useSession = Config::inst()->get('Versioned', 'use_session'); + $updateSession = false; + if ($useSession) { + // Boot reading mode from session + $mode = Session::get('readingMode') ?: $mode; - // Try not to store the mode in the session if not needed - if(($preexistingMode && $preexistingMode !== $mode) - || (!$preexistingMode && $mode !== self::DEFAULT_MODE) - ) { - Session::set('readingMode', $mode); - } + // Set draft site security if disabled for this session + if (Session::get('unsecuredDraftSite')) { + static::set_draft_site_secured(false); + } + } + + // Verify if querystring contains valid reading mode + $queryMode = VersionedReadingMode::fromQueryString($request->getVars()); + if ($queryMode) { + $mode = $queryMode; + $updateSession = true; + } + + // Save reading mode + Versioned::set_reading_mode($mode); + + // Set mode if session enabled + if ($useSession && $updateSession) { + Session::set('readingMode', $mode); + } if(!headers_sent() && !Director::is_cli()) { - if(Versioned::current_stage() == 'Live') { + if(Versioned::current_stage() === self::LIVE) { // clear the cookie if it's set if(Cookie::get('bypassStaticCache')) { Cookie::force_expiry('bypassStaticCache', null, null, false, true /* httponly */); @@ -1217,7 +1287,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return string */ public static function get_live_stage() { - return "Live"; + return self::LIVE; } /** @@ -1249,9 +1319,52 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @param string $stage */ public static function reading_stage($stage) { + VersionedReadingMode::validateStage($stage); Versioned::set_reading_mode('Stage.' . $stage); } + /** + * Replace default mode. + * An non-default mode should be specified via querystring arguments. + * + * @param string $mode + */ + public static function set_default_reading_mode($mode) { + self::$default_reading_mode = $mode; + } + + /** + * Get default reading mode + * + * @return string + */ + public static function get_default_reading_mode() { + return self::$default_reading_mode ?: self::DEFAULT_MODE; + } + + /** + * Check if draft site should be secured. + * Can be turned off if draft site unauthenticated + * + * @return bool + */ + public static function get_draft_site_secured() { + if (isset(static::$is_draft_site_secured)) { + return (bool)static::$is_draft_site_secured; + } + // Config default + return (bool)Config::inst()->get('Versioned', 'draft_site_secured'); + } + + /** + * Set if the draft site should be secured or not + * + * @param bool $secured + */ + public static function set_draft_site_secured($secured) { + static::$is_draft_site_secured = $secured; + } + /** * Set the reading archive date. * diff --git a/model/VersionedReadingMode.php b/model/VersionedReadingMode.php new file mode 100644 index 000000000..eb381fa58 --- /dev/null +++ b/model/VersionedReadingMode.php @@ -0,0 +1,144 @@ + 'archive', + 'Versioned.date' => $parts[1], + ); + case 'Stage': + self::validateStage($parts[1]); + return array( + 'Versioned.mode' => 'stage', + 'Versioned.stage' => $parts[1], + ); + default: + // Unsupported mode + return null; + } + } + + /** + * Converts dataquery params to original reading mode. + * Only supports stage / archive + * + * @param array $params + * @return string|null + */ + public static function fromDataQueryParams($params) + { + // Switch on reading mode + if (empty($params["Versioned.mode"])) { + return null; + } + + switch ($params["Versioned.mode"]) { + case 'archive': + return 'Archive.' . $params['Versioned.date']; + case 'stage': + return 'Stage.' . $params['Versioned.stage']; + default: + return null; + } + } + + /** + * Convert querystring arguments to reading mode. + * Only supports stage / archive mode + * + * @param array|string $query Querystring arguments (array or string) + * @return string|null Reading mode, or null if not found / supported + */ + public static function fromQueryString($query) + { + if (is_string($query)) { + parse_str($query, $query); + } + if (empty($query)) { + return null; + } + + // Archive date is specified + if (isset($query['archiveDate']) && strtotime($query['archiveDate'])) { + return 'Archive.' . $query['archiveDate']; + } + + // Stage is specified by itself + if (isset($query['stage']) && strcasecmp($query['stage'], Versioned::DRAFT) === 0) { + return 'Stage.' . Versioned::DRAFT; + } + if (isset($query['stage']) && strcasecmp($query['stage'], Versioned::LIVE) === 0) { + return 'Stage.' . Versioned::LIVE; + } + + // Unsupported query mode + return null; + } + + /** + * Build querystring arguments for current reading mode. + * Supports stage / archive only. + * + * @param string $mode + * @return array List of querystring arguments as an array + */ + public static function toQueryString($mode) + { + if (empty($mode)) { + return null; + } + if (!is_string($mode)) { + throw new InvalidArgumentException("mode must be a string"); + } + $parts = explode('.', $mode); + switch ($parts[0]) { + case 'Archive': + return array( + 'archiveDate' => $parts[1], + ); + case 'Stage': + self::validateStage($parts[1]); + return array( + 'stage' => $parts[1], + ); + default: + // Unsupported mode + return null; + } + } + + /** + * Validate the stage is valid, throwing an exception if it's not + * + * @param string $stage + */ + public static function validateStage($stage) + { + // Any stage is allowed in 3.x. Note that 4.x only allows Stage / Live + // Any string that contains no dots is ok. + if (empty($stage) || !preg_match('/^([^.]+)$/', $stage)) { + throw new InvalidArgumentException("Invalid stage name \"{$stage}\""); + } + } +} diff --git a/model/VersionedStateExtension.php b/model/VersionedStateExtension.php new file mode 100644 index 000000000..be5fbbecb --- /dev/null +++ b/model/VersionedStateExtension.php @@ -0,0 +1,89 @@ +hasVersionedQuery($link)) { + return; + } + + // Skip if current mode matches default mode + // See LeftAndMain::init() for example of this being overridden. + $readingMode = $this->getReadingmode(); + if ($readingMode === Versioned::get_default_reading_mode()) { + return; + } + + // Determine if query args are supported for the current mode + $queryargs = VersionedReadingMode::toQueryString($readingMode); + if (!$queryargs) { + return; + } + + // Decorate + $link = Controller::join_links( + $link, + '?' . http_build_query($queryargs) + ); + } + + /** + * Check if link contains versioned queryargs + * + * @param string $link + * @return bool + */ + protected function hasVersionedQuery($link) + { + // Find querystrings + $parts = explode('?', $link, 2); + if (count($parts) < 2) { + return false; + } + + // Parse args + $readingMode = VersionedReadingMode::fromQueryString($parts[1]); + return !empty($readingMode); + } + + /** + * Get reading mode for the record / controller being decorated + * + * @return string + */ + protected function getReadingmode() + { + $default = Versioned::get_reading_mode(); + + // Non dataobjects use global mode + if (! $this->owner instanceof DataObject) { + return $default; + } + + // Respect source query params (so records selected from live will have live urls) + $queryParams = $this->owner->getSourceQueryParams(); + return VersionedReadingMode::fromDataQueryParams($queryParams) + // Fall back to default otherwise + ?: $default; + } +} diff --git a/security/CMSSecurity.php b/security/CMSSecurity.php index 851be1d49..69f6e9fc5 100644 --- a/security/CMSSecurity.php +++ b/security/CMSSecurity.php @@ -43,7 +43,10 @@ class CMSSecurity extends Security { } public function Link($action = null) { - return Controller::join_links(Director::baseURL(), "CMSSecurity", $action); + $link = Controller::join_links(Director::baseURL(), "CMSSecurity", $action); + // Give extensions the chance to modify by reference + $this->extend('updateLink', $link, $action); + return $link; } /** diff --git a/security/Security.php b/security/Security.php index a97426543..877c161b0 100644 --- a/security/Security.php +++ b/security/Security.php @@ -399,7 +399,11 @@ class Security extends Controller implements TemplateGlobalProvider { * @return string Returns the link to the given action */ public function Link($action = null) { - return Controller::join_links(Director::baseURL(), "Security", $action); + $link = Controller::join_links(Director::baseURL(), "Security", $action); + + // Give extensions the chance to modify by reference + $this->extend('updateLink', $link, $action); + return $link; } /** diff --git a/tests/model/VersionedReadingModeTest.php b/tests/model/VersionedReadingModeTest.php new file mode 100644 index 000000000..6967674a8 --- /dev/null +++ b/tests/model/VersionedReadingModeTest.php @@ -0,0 +1,175 @@ +assertEquals( + $dataQuery, + VersionedReadingMode::toDataQueryParams($readingMode), + "Convert {$readingMode} to dataquery parameters" + ); + } + /** + * @dataProvider provideReadingModes() + * + * @param string $readingMode + * @param array $dataQuery + * @param array $queryStringArray + * @param string $queryString + */ + public function testFromDataQueryParameters($readingMode, $dataQuery, $queryStringArray, $queryString) + { + $this->assertEquals( + $readingMode, + VersionedReadingMode::fromDataQueryParams($dataQuery), + "Convert {$readingMode} from dataquery parameters" + ); + } + + /** + * @dataProvider provideReadingModes() + * + * @param string $readingMode + * @param array $dataQuery + * @param array $queryStringArray + * @param string $queryString + */ + public function testToQueryString($readingMode, $dataQuery, $queryStringArray, $queryString) + { + $this->assertEquals( + $queryStringArray, + VersionedReadingMode::toQueryString($readingMode), + "Convert {$readingMode} to querystring array" + ); + } + + /** + * @dataProvider provideReadingModes() + * + * @param string $readingMode + * @param array $dataQuery + * @param array $queryStringArray + * @param string $queryString + */ + public function testFromQueryString($readingMode, $dataQuery, $queryStringArray, $queryString) + { + $this->assertEquals( + $readingMode, + VersionedReadingMode::fromQueryString($queryStringArray), + "Convert {$readingMode} from querystring array" + ); + $this->assertEquals( + $readingMode, + VersionedReadingMode::fromQueryString($queryString), + "Convert {$readingMode} from querystring encoded string" + ); + } + + /** + * Return list of reading modes in order: + * - reading mode string + * - dataquery params array + * - query string array + * - query string (string) + * @return array + */ + public function provideReadingModes() + { + return array( + // Draft + array( + 'Stage.Stage', + array( + 'Versioned.mode' => 'stage', + 'Versioned.stage' => 'Stage', + ), + array( + 'stage' => 'Stage', + ), + 'stage=Stage' + ), + // Live + array( + 'Stage.Live', + array( + 'Versioned.mode' => 'stage', + 'Versioned.stage' => 'Live', + ), + array( + 'stage' => 'Live', + ), + 'stage=Live' + ), + // Draft archive + array( + 'Archive.2017-11-15 11:31:42', + array( + 'Versioned.mode' => 'archive', + 'Versioned.date' => '2017-11-15 11:31:42', + ), + array( + 'archiveDate' => '2017-11-15 11:31:42', + ), + 'archiveDate=2017-11-15+11%3A31%3A42', + ), + // Live archive + array( + 'Archive.2017-11-15 11:31:42', + array( + 'Versioned.mode' => 'archive', + 'Versioned.date' => '2017-11-15 11:31:42', + ), + array( + 'archiveDate' => '2017-11-15 11:31:42', + ), + 'archiveDate=2017-11-15+11%3A31%3A42', + ), + ); + } + + /** + * @dataProvider provideTestInvalidStage + * @param string $stage + */ + public function testInvalidStage($stage) + { + $this->setExpectedException('InvalidArgumentException'); + VersionedReadingMode::validateStage($stage); + } + + public function provideTestInvalidStage() + { + return array( + array(''), + array('Stage.stage'), + ); + } + + /** + * @dataProvider provideTestValidStage + * @param string $stage + */ + public function testValidStage($stage) + { + VersionedReadingMode::validateStage($stage); + $this->assertTrue(true, 'Stage is valid'); + } + + public function provideTestValidStage() + { + return array( + array('anything'), + array(Versioned::DRAFT), + array(Versioned::LIVE), + ); + } +} diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index ecad71788..31fe97306 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -296,7 +296,7 @@ class VersionedTest extends SapphireTest { } public function testWritingNewToStage() { - $origStage = Versioned::current_stage(); + $origMode = Versioned::get_reading_mode(); Versioned::reading_stage("Stage"); $page = new VersionedTest_DataObject(); @@ -315,7 +315,7 @@ class VersionedTest extends SapphireTest { $this->assertEquals(1, $stage->count()); $this->assertEquals($stage->First()->Title, 'testWritingNewToStage'); - Versioned::reading_stage($origStage); + Versioned::set_reading_mode($origMode); } /** @@ -325,7 +325,7 @@ class VersionedTest extends SapphireTest { * the VersionedTest_DataObject record though. */ public function testWritingNewToLive() { - $origStage = Versioned::current_stage(); + $origMode = Versioned::get_reading_mode(); Versioned::reading_stage("Live"); $page = new VersionedTest_DataObject(); @@ -344,7 +344,7 @@ class VersionedTest extends SapphireTest { )); $this->assertEquals(0, $stage->count()); - Versioned::reading_stage($origStage); + Versioned::set_reading_mode($origMode); } /** @@ -635,63 +635,91 @@ class VersionedTest extends SapphireTest { Versioned::set_reading_mode($originalMode); } - /** - * Tests that reading mode persists between requests - */ - public function testReadingPersistent() { - $session = Injector::inst()->create('Session', array()); - $adminID = $this->logInWithPermission('ADMIN'); - $session->inst_set('loggedInAs', $adminID); + public function testReadingNotPersistentWhenUseSessionFalse() + { + Config::inst()->update('Versioned', 'use_session', false); - // Set to stage - Director::test('/?stage=Stage', null, $session); - $this->assertEquals( - 'Stage.Stage', - $session->inst_get('readingMode'), - 'Check querystring changes reading mode to Stage' - ); - Director::test('/', null, $session); - $this->assertEquals( - 'Stage.Stage', - $session->inst_get('readingMode'), - 'Check that subsequent requests in the same session remain in Stage mode' - ); + $session = new Session(array()); + $adminID = $this->logInWithPermission('ADMIN'); + $session->inst_set('loggedInAs', $adminID); - // Test live persists - Director::test('/?stage=Live', null, $session); - $this->assertEquals( - 'Stage.Live', - $session->inst_get('readingMode'), - 'Check querystring changes reading mode to Live' - ); - Director::test('/', null, $session); - $this->assertEquals( - 'Stage.Live', - $session->inst_get('readingMode'), - 'Check that subsequent requests in the same session remain in Live mode' - ); + Director::test('/?stage=Stage', null, $session); + $this->assertNull( + $session->inst_get('readingMode'), + 'Check querystring does not change reading mode' + ); - // Test that session doesn't redundantly store the default stage if it doesn't need to - $session2 = Injector::inst()->create('Session', array()); - $session2->inst_set('loggedInAs', $adminID); - Director::test('/', null, $session2); - $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData()); - Director::test('/?stage=Live', null, $session2); - $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData()); + Director::test('/', null, $session); + $this->assertNull( + $session->inst_get('readingMode'), + 'Check that subsequent requests in the same session do not have a changed reading mode' + ); + } - // Test choose_site_stage - unset($_GET['stage']); - unset($_GET['archiveDate']); - Session::set('readingMode', 'Stage.Stage'); - Versioned::choose_site_stage(); - $this->assertEquals('Stage.Stage', Versioned::get_reading_mode()); - Session::set('readingMode', 'Archive.2014-01-01'); - Versioned::choose_site_stage(); - $this->assertEquals('Archive.2014-01-01', Versioned::get_reading_mode()); - Session::clear('readingMode'); - Versioned::choose_site_stage(); - $this->assertEquals('Stage.Live', Versioned::get_reading_mode()); - } + /** + * Tests that reading mode persists between requests + */ + public function testReadingPersistentWhenUseSessionTrue() + { + Config::inst()->update('Versioned', 'use_session', true); + + $session = new Session(array()); + $adminID = $this->logInWithPermission('ADMIN'); + $session->inst_set('loggedInAs', $adminID); + // Set to stage + Director::test('/?stage=Stage', null, $session); + $this->assertEquals( + 'Stage.Stage', + $session->inst_get('readingMode'), + 'Check querystring changes reading mode to Stage' + ); + Director::test('/', null, $session); + $this->assertEquals( + 'Stage.Stage', + $session->inst_get('readingMode'), + 'Check that subsequent requests in the same session remain in Stage mode' + ); + // Default stage stored anyway (in case default changes) + Director::test('/?stage=Live', null, $session); + $this->assertEquals( + 'Stage.Live', + $session->inst_get('readingMode'), + 'Check querystring changes reading mode to Live' + ); + Director::test('/', null, $session); + $this->assertEquals( + 'Stage.Live', + $session->inst_get('readingMode'), + 'Check that subsequent requests in the same session remain in Live mode' + ); + // Test that session doesn't redundantly modify session stage without querystring args + $session2 = new Session(array()); + $session2->inst_set('loggedInAs', $adminID); + Director::test('/', null, $session2); + $this->assertArrayNotHasKey('readingMode', $session2->inst_changedData()); + Director::test('/?stage=Live', null, $session2); + $this->assertArrayHasKey('readingMode', $session2->inst_changedData()); + // Test choose_site_stage + unset($_GET['stage']); + unset($_GET['archiveDate']); + $request = new SS_HTTPRequest('GET', '/'); + Session::clear_all(); + Session::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); + $this->assertEquals('Archive.2014-01-01', Versioned::get_reading_mode()); + Session::clear('readingMode'); + Versioned::choose_site_stage($request); + $this->assertEquals('Stage.Live', Versioned::get_reading_mode()); + // Ensure stage is reset to Live when logging out + Session::set('readingMode', 'Stage.Stage'); + Versioned::choose_site_stage($request); + Session::clear_all(); + Versioned::choose_site_stage($request); + $this->assertSame('Stage.Live', Versioned::get_reading_mode()); + } /** * Test that stage parameter is blocked by non-administrative users diff --git a/tests/view/SSViewerCacheBlockTest.php b/tests/view/SSViewerCacheBlockTest.php index 57c4243e6..5e3fef377 100644 --- a/tests/view/SSViewerCacheBlockTest.php +++ b/tests/view/SSViewerCacheBlockTest.php @@ -145,7 +145,7 @@ class SSViewerCacheBlockTest extends SapphireTest { public function testVersionedCache() { - $origStage = Versioned::current_stage(); + $origMode = Versioned::get_reading_mode(); // Run without caching in stage to prove data is uncached $this->_reset(false); @@ -211,7 +211,7 @@ class SSViewerCacheBlockTest extends SapphireTest { $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) ); - Versioned::reading_stage($origStage); + Versioned::set_reading_mode($origMode); } /**