Permalink
Browse files

BUGFIX Renamed Nested URLs are automatically redirected to their new …

…location with 301 HTTP status code in ModelAsController/ContentController (fixes #5393, thanks cbarberis) (from r103461)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@112144 467b73ca-7a2a-4603-9d3b-597d59a354a9
  • Loading branch information...
sminnee committed Oct 13, 2010
1 parent da671c6 commit 752869e23b62d39da8976415126ca90d2451b0f6
Showing with 211 additions and 28 deletions.
  1. +28 −1 core/control/ContentController.php
  2. +31 −11 core/control/ModelAsController.php
  3. +152 −16 tests/control/ModelAsControllerTest.php
@@ -134,7 +134,7 @@ public function init() {
*
* @return SS_HTTPResponse
*/
- public function handleRequest(SS_HTTPRequest $request) {
+ public function handleRequest(SS_HTTPRequest $request) {
$child = null;
$action = $request->param('Action');
@@ -144,12 +144,39 @@ public function handleRequest(SS_HTTPRequest $request) {
if($action && SiteTree::nested_urls() && !$this->hasAction($action)) {
// See ModelAdController->getNestedController() for similar logic
Translatable::disable_locale_filter();
+ // look for a page with this URLSegment
$child = DataObject::get_one('SiteTree', sprintf (
"\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql($action)
));
Translatable::enable_locale_filter();
+
+ // if we can't find a page with this URLSegment try to find one that used to have
+ // that URLSegment but changed. See ModelAsController->getNestedController() for similiar logic.
+ if(!$child){
+ $child = ModelAsController::find_old_page($action,$this->ID);
+ if($child){
+ $response = new SS_HTTPResponse();
+ $params = $request->getVars();
+ if(isset($params['url'])) unset($params['url']);
+ $response->redirect(
+ Controller::join_links(
+ $child->Link(
+ Controller::join_links(
+ $request->param('ID'), // 'ID' is the new 'URLSegment', everything shifts up one position
+ $request->param('OtherID')
+ )
+ ),
+ // Needs to be in separate join links to avoid urlencoding
+ ($params) ? '?' . http_build_query($params) : null
+ ),
+ 301
+ );
+ return $response;
+ }
+ }
}
+ // we found a page with this URLSegment.
if($child) {
$request->shiftAllParams();
$request->shift();
@@ -62,7 +62,7 @@ public function handleRequest(SS_HTTPRequest $request) {
try {
$result = $this->getNestedController();
-
+
if($result instanceof RequestHandler) {
$result = $result->handleRequest($this->request);
} else if(!($result instanceof SS_HTTPResponse)) {
@@ -86,7 +86,7 @@ public function getNestedController() {
if(!$URLSegment = $request->param('URLSegment')) {
throw new Exception('ModelAsController->getNestedController(): was not passed a URLSegment value.');
}
-
+
// Find page by link, regardless of current locale settings
Translatable::disable_locale_filter();
$sitetree = DataObject::get_one(
@@ -101,11 +101,26 @@ public function getNestedController() {
if(!$sitetree) {
// If a root page has been renamed, redirect to the new location.
- if($redirect = $this->findOldPage($URLSegment)) {
+ // See ContentController->handleRequest() for similiar logic.
+ $redirect = self::find_old_page($URLSegment);
+ if($redirect = self::find_old_page($URLSegment)) {
+ $params = $request->getVars();
+ if(isset($params['url'])) unset($params['url']);
$this->response = new SS_HTTPResponse();
- $this->response->redirect($redirect->Link (
- Controller::join_links($request->param('Action'), $request->param('ID'), $request->param('OtherID'))
- ));
+ $this->response->redirect(
+ Controller::join_links(
+ $redirect->Link(
+ Controller::join_links(
+ $request->param('Action'),
+ $request->param('ID'),
+ $request->param('OtherID')
+ )
+ ),
+ // Needs to be in separate join links to avoid urlencoding
+ ($params) ? '?' . http_build_query($params) : null
+ ),
+ 301
+ );
return $this->response;
}
@@ -128,22 +143,27 @@ public function getNestedController() {
}
/**
- * @param string $URLSegment
+ * @param string $URLSegment A subset of the url. i.e in /home/contact/ home and contact are URLSegment.
+ * @param int $parentID The ID of the parent of the page the URLSegment belongs to.
* @return SiteTree
*/
- protected function findOldPage($URLSegment) {
+ static function find_old_page($URLSegment,$parentID = 0) {
$URLSegment = Convert::raw2sql($URLSegment);
// First look for a non-nested page that has a unique URLSegment and can be redirected to.
- if(SiteTree::nested_urls() && $pages = DataObject::get('SiteTree', "\"URLSegment\" = '$URLSegment'")) {
- if($pages->Count() == 1) return $pages->First();
+ if(SiteTree::nested_urls()) {
+ $pages = DataObject::get(
+ 'SiteTree',
+ "\"URLSegment\" = '$URLSegment'" . ((SiteTree::nested_urls()) ? ' AND "ParentID" = ' . (int)$parentID : '')
+ );
+ if($pages && $pages->Count() == 1) return $pages->First();
}
// Get an old version of a page that has been renamed.
$query = new SQLQuery (
'"RecordID"',
'"SiteTree_versions"',
- "\"URLSegment\" = '$URLSegment' AND \"WasPublished\" = 1" . (SiteTree::nested_urls() ? ' AND "ParentID" = 0' : null),
+ "\"URLSegment\" = '$URLSegment' AND \"WasPublished\" = 1" . (SiteTree::nested_urls() ? ' AND "ParentID" = ' . (int)$parentID : ''),
'"LastEdited" DESC',
null,
null,
@@ -3,32 +3,168 @@
* @package sapphire
* @subpackage tests
*/
-class ModelAsControllerTest extends SapphireTest {
+class ModelAsControllerTest extends FunctionalTest {
protected $usesDatabase = true;
- public function testFindOldPage() {
+ protected $autoFollowRedirection = false;
+
+ protected function generateNestedPagesFixture() {
+ $level1 = new Page();
+ $level1->Title = 'First Level';
+ $level1->URLSegment = 'level1';
+ $level1->write();
+ $level1->publish('Stage', 'Live');
+
+ $level1->URLSegment = 'newlevel1';
+ $level1->write();
+ $level1->publish('Stage', 'Live');
+
+ $level2 = new Page();
+ $level2->Title = 'Second Level';
+ $level2->URLSegment = 'level2';
+ $level2->ParentID = $level1->ID;
+ $level2->write();
+ $level2->publish('Stage', 'Live');
+
+ $level2->URLSegment = 'newlevel2';
+ $level2->write();
+ $level2->publish('Stage', 'Live');
+
+ $level3 = New Page();
+ $level3->Title = "Level 3";
+ $level3->URLSegment = 'level3';
+ $level3->ParentID = $level2->ID;
+ $level3->write();
+ $level3->publish('Stage','Live');
+
+ $level3->URLSegment = 'newlevel3';
+ $level3->write();
+ $level3->publish('Stage','Live');
+ }
+
+ /**
+ * We're building up a page hierarchy ("nested URLs") and rename
+ * all the individual pages afterwards. The assumption is that
+ * all pages will be found by their old segments.
+ *
+ * Original: level1/level2/level3
+ * Republished as: newlevel1/newlevel2/newlevel3
+ */
+ public function testRedirectsNestedRenamedPages(){
+ $this->generateNestedPagesFixture();
+
+ // check a first level URLSegment
+ $response = $this->get('level1/action');
+ $this->assertEquals($response->getStatusCode(),301);
+ $this->assertEquals(
+ Controller::join_links(Director::baseURL() . 'newlevel1/action'),
+ $response->getHeader('Location')
+ );
+
+ // check second level URLSegment
+ $response = $this->get('newlevel1/level2');
+ $this->assertEquals($response->getStatusCode(),301 );
+ $this->assertEquals(
+ Controller::join_links(Director::baseURL() . 'newlevel1/newlevel2/'),
+ $response->getHeader('Location')
+ );
+
+ // check third level URLSegment
+ $response = $this->get('newlevel1/newlevel2/level3');
+ $this->assertEquals($response->getStatusCode(), 301);
+ $this->assertEquals(
+ Controller::join_links(Director::baseURL() . 'newlevel1/newlevel2/newlevel3/'),
+ $response->getHeader('Location')
+ );
+ }
+
+ function testDoesntRedirectToNestedChildrenOutsideOfOwnHierarchy() {
+ $this->generateNestedPagesFixture();
+
+ $otherParent = new Page(array(
+ 'URLSegment' => 'otherparent'
+ ));
+ $otherParent->write();
+ $otherParent->publish('Stage', 'Live');
+
+ $response = $this->get('level1/otherparent');
+ $this->assertEquals($response->getStatusCode(), 301);
+
+ $response = $this->get('newlevel1/otherparent');
+ $this->assertEquals(
+ $response->getStatusCode(),
+ 404,
+ 'Requesting an unrelated page on a renamed parent should be interpreted as a missing action, not a redirect'
+ );
+ }
+
+ function testRedirectsNestedRenamedPagesWithGetParameters() {
+ $this->generateNestedPagesFixture();
+
+ // check third level URLSegment
+ $response = $this->get('newlevel1/newlevel2/level3/?foo=bar&test=test');
+ $this->assertEquals($response->getStatusCode(), 301);
+ $this->assertEquals(
+ Controller::join_links(Director::baseURL() . 'newlevel1/newlevel2/newlevel3/', '?foo=bar&test=test'),
+ $response->getHeader('Location')
+ );
+ }
+
+ function testDoesntRedirectToNestedRenamedPageWhenNewExists() {
+ $this->generateNestedPagesFixture();
+
+ $otherLevel1 = new Page(array(
+ 'Title' => "Other Level 1",
+ 'URLSegment' => 'level1'
+ ));
+ $otherLevel1->write();
+ $otherLevel1->publish('Stage', 'Live');
+
+ $response = $this->get('level1');
+ $this->assertEquals(
+ $response->getStatusCode(),
+ 200
+ );
+
+ $response = $this->get('level1/newlevel2');
+ $this->assertEquals(
+ $response->getStatusCode(),
+ 404,
+ 'The old newlevel2/ URLSegment is checked as an action on the new page, which shouldnt exist.'
+ );
+ }
+
+ function testFindOldPage(){
$page = new Page();
- $page->Title = 'Test Page';
- $page->URLSegment = 'test-page';
+ $page->Title = 'First Level';
+ $page->URLSegment = 'oldurl';
$page->write();
$page->publish('Stage', 'Live');
- $page->URLSegment = 'test';
+ $page->URLSegment = 'newurl';
$page->write();
$page->publish('Stage', 'Live');
- $router = new ModelAsController();
- $request = new SS_HTTPRequest(
- 'GET', 'test-page/action/id/otherid'
- );
- $request->match('$URLSegment/$Action/$ID/$OtherID');
- $response = $router->handleRequest($request);
+ $response = ModelAsController::find_old_page('oldurl');
+ $this->assertEquals('First Level',$response->Title);
- $this->assertEquals (
- $response->getHeader('Location'),
- Controller::join_links(Director::baseURL() . 'test/action/id/otherid')
- );
+ $page2 = new Page();
+ $page2->Title = 'Second Level Page';
+ $page2->URLSegment = 'oldpage2';
+ $page2->ParentID = $page->ID;
+ $page2->write();
+ $page2->publish('Stage', 'Live');
+
+ $page2->URLSegment = 'newpage2';
+ $page2->write();
+ $page2->publish('Stage', 'Live');
+
+ $response = ModelAsController::find_old_page('oldpage2',$page2->ParentID);
+ $this->assertEquals('Second Level Page',$response->Title);
+
+ $response = ModelAsController::find_old_page('oldpage2',$page2->ID);
+ $this->assertEquals(false, $response );
}
-}
+}

0 comments on commit 752869e

Please sign in to comment.