Skip to content

Commit

Permalink
BUGFIX Renamed Nested URLs are automatically redirected to their new …
Browse files Browse the repository at this point in the history 
…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
Sam Minnee committed Oct 13, 2010
1 parent da671c6 commit 752869e
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 28 deletions.
29 changes: 28 additions & 1 deletion core/control/ContentController.php
View file
Expand Up @@ -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');

Expand All @@ -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();
Expand Down
42 changes: 31 additions & 11 deletions core/control/ModelAsController.php
View file
Expand Up @@ -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)) {
Expand All @@ -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(
Expand All @@ -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;
}
Expand All @@ -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,
Expand Down
168 changes: 152 additions & 16 deletions tests/control/ModelAsControllerTest.php
View file
Expand Up @@ -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.