diff --git a/core/model/SiteTree.php b/core/model/SiteTree.php index 99e6ce2c9..4348a8283 100755 --- a/core/model/SiteTree.php +++ b/core/model/SiteTree.php @@ -1288,19 +1288,13 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid DataObject::set_context_obj($this); - // Ensure URLSegment is unique - $count = 1; - $otherpage = false; - while((class_exists($this->URLSegment) && is_subclass_of($this->URLSegment, 'RequestHandler')) || $otherpage = SiteTree::get_by_link($this->URLSegment)) { - if($otherpage && $otherpage->ID == $this->ID) { - break; - } - - $otherpage = false; + // Ensure that this object has a non-conflicting URLSegment value. + $count = 2; + while(!$this->validURLSegment()) { + $this->URLSegment = preg_replace('/-[0-9]+$/', null, $this->URLSegment) . '-' . $count; $count++; - $this->URLSegment = ereg_replace('-[0-9]+$','', $this->URLSegment) . "-$count"; } - + DataObject::set_context_obj(null); parent::onBeforeWrite(); @@ -1338,8 +1332,58 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid parent::onAfterDelete(); } - - + + /** + * Returns TRUE if this object has a URLSegment value that does not conflict with any other objects. This methods + * checks for: + * - A page with the same URLSegment that has a conflict. + * - Conflicts with actions on the parent page. + * - A conflict caused by a root page having the same URLSegment as a class name. + * - Conflicts with action-specific templates on the parent page. + * + * @return bool + */ + public function validURLSegment() { + if(self::nested_urls() && $parent = $this->Parent()) { + if($this->URLSegment == 'index') return false; + + if($controller = ModelAsController::controller_for($parent)) { + $actions = Object::combined_static($controller->class, 'allowed_actions', 'RequestHandler'); + + // check for a conflict with an entry in $allowed_actions + if(is_array($actions)) { + if(array_key_exists($this->URLSegment, $actions) || in_array($this->URLSegment, $actions)) { + return false; + } + } + + // check for a conflict with an action-specific template + if($controller->hasMethod('hasActionTemplate') && $controller->hasActionTemplate($this->URLSegment)) { + return false; + } + } + } + + if(!self::nested_urls() || !$this->ParentID) { + if(class_exists($this->URLSegment) && is_subclass_of($this->URLSegment, 'RequestHandler')) return false; + } + + $IDFilter = ($this->ID) ? "AND \"SiteTree\".\"ID\" <> $this->ID" : null; + $parentFilter = null; + + if(self::nested_urls()) { + if($this->ParentID) { + $parentFilter = " AND \"SiteTree\".\"ParentID\" = $this->ParentID"; + } else { + $parentFilter = ' AND "SiteTree"."ParentID" = 0'; + } + } + + return DB::query ( + "SELECT COUNT(ID) FROM \"SiteTree\" WHERE \"URLSegment\" = '$this->URLSegment' $IDFilter $parentFilter" + )->value() < 1; + } + /** * Generate a URL segment based on the title provided. * @param string $title Page title. diff --git a/tests/SiteTreeTest.php b/tests/SiteTreeTest.php index 949e24bd7..3fb203b03 100755 --- a/tests/SiteTreeTest.php +++ b/tests/SiteTreeTest.php @@ -476,11 +476,94 @@ class SiteTreeTest extends SapphireTest { $this->assertTrue($ceo->isSection()); } + /** + * @covers SiteTree::validURLSegment() + */ + public function testValidURLSegmentURLSegmentConflicts() { + $sitetree = new SiteTree(); + SiteTree::disable_nested_urls(); + + $sitetree->URLSegment = 'home'; + $this->assertFalse($sitetree->validURLSegment(), 'URLSegment conflicts are recognised'); + $sitetree->URLSegment = 'home-noconflict'; + $this->assertTrue($sitetree->validURLSegment()); + + $sitetree->ParentID = $this->idFromFixture('Page', 'about'); + $sitetree->URLSegment = 'home'; + $this->assertFalse($sitetree->validURLSegment(), 'Conflicts are still recognised with a ParentID value'); + + SiteTree::enable_nested_urls(); + + $sitetree->ParentID = 0; + $sitetree->URLSegment = 'home'; + $this->assertFalse($sitetree->validURLSegment(), 'URLSegment conflicts are recognised'); + + $sitetree->ParentID = $this->idFromFixture('Page', 'about'); + $this->assertTrue($sitetree->validURLSegment(), 'URLSegments can be the same across levels'); + + $sitetree->URLSegment = 'my-staff'; + $this->assertFalse($sitetree->validURLSegment(), 'Nested URLSegment conflicts are recognised'); + $sitetree->URLSegment = 'my-staff-noconflict'; + $this->assertTrue($sitetree->validURLSegment()); + } + + /** + * @covers SiteTree::validURLSegment() + */ + public function testValidURLSegmentClassNameConflicts() { + $sitetree = new SiteTree(); + $sitetree->URLSegment = 'Controller'; + + $this->assertFalse($sitetree->validURLSegment(), 'Class name conflicts are recognised'); + } + + /** + * @covers SiteTree::validURLSegment() + */ + public function testValidURLSegmentControllerConflicts() { + SiteTree::enable_nested_urls(); + + $sitetree = new SiteTree(); + $sitetree->ParentID = $this->idFromFixture('SiteTreeTest_Conflicted', 'parent'); + + $sitetree->URLSegment = 'index'; + $this->assertFalse($sitetree->validURLSegment(), 'index is not a valid URLSegment'); + + $sitetree->URLSegment = 'conflicted-action'; + $this->assertFalse($sitetree->validURLSegment(), 'allowed_actions conflicts are recognised'); + + $sitetree->URLSegment = 'conflicted-template'; + $this->assertFalse($sitetree->validURLSegment(), 'Action-specific template conflicts are recognised'); + + $sitetree->URLSegment = 'valid'; + $this->assertTrue($sitetree->validURLSegment(), 'Valid URLSegment values are allowed'); + } + } -// We make these extend page since that's what all page types are expected to do +/**#@+ + * @ignore + */ + class SiteTreeTest_PageNode extends Page implements TestOnly { } -class SiteTreeTest_PageNode_Controller extends Page_Controller implements TestOnly { +class SiteTreeTest_PageNode_Controller extends Page_Controller implements TestOnly { } -?> +class SiteTreeTest_Conflicted extends Page implements TestOnly { } +class SiteTreeTest_Conflicted_Controller extends Page_Controller implements TestOnly { + + public static $allowed_actions = array ( + 'conflicted-action' + ); + + public function hasActionTemplate($template) { + if($template == 'conflicted-template') { + return true; + } else { + return parent::hasActionTemplate($template); + } + } + +} + +/**#@-*/ diff --git a/tests/SiteTreeTest.yml b/tests/SiteTreeTest.yml index 1031ebc0d..f4fe72751 100755 --- a/tests/SiteTreeTest.yml +++ b/tests/SiteTreeTest.yml @@ -71,7 +71,11 @@ Page: Title: Controller numericonly: Title: 1930 - + +SiteTreeTest_Conflicted: + parent: + Title: Parent + ErrorPage: 404: Title: Page not Found