From da4b65c749c983d6c08f0fa3642e017e6499efc7 Mon Sep 17 00:00:00 2001 From: Andrew Short Date: Sun, 11 Oct 2009 00:07:23 +0000 Subject: [PATCH] FEATURE: Added RequestHandler->hasAction() and Controller->hasAction() to check if a specific action is defined on a controller. ENHANCEMENT: Updated ContentController->handleRequest() to use Controller->hasAction() to check whether to fall over to a child page, rather than relying on an error response from Controller->handleRequest(). From: Andrew Short git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@88505 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- core/control/ContentController.php | 33 ++++++++++++++++-------------- core/control/Controller.php | 12 ++++++----- core/control/RequestHandler.php | 25 ++++++++++++++++++++++ core/model/SiteTree.php | 17 +-------------- tests/ControllerTest.php | 26 ++++++++++++++++++++++- 5 files changed, 76 insertions(+), 37 deletions(-) mode change 100644 => 100755 core/control/RequestHandler.php diff --git a/core/control/ContentController.php b/core/control/ContentController.php index 25a79abb0..8d50ea5f0 100755 --- a/core/control/ContentController.php +++ b/core/control/ContentController.php @@ -168,30 +168,33 @@ class ContentController extends Controller { * @return HTTPResponse */ public function handleRequest(HTTPRequest $request) { - Director::set_current_page($this->data()); - $response = parent::handleRequest($request); + $child = null; + $action = $request->param('Action'); - // If the default handler returns an error, due to the action not existing, attempt to fall over to a child - // SiteTree object, then use its corresponding ContentController to handle the request. This allows for the - // building of nested chains of controllers corresponding to a nested URL. - if(SiteTree::nested_urls() && $response instanceof HTTPResponse && $response->isError()) { + // If nested URLs are enabled, and there is no action handler for the current request then attempt to pass + // control to a child controller. This allows for the creation of chains of controllers which correspond to a + // nested URL. + if($action && SiteTree::nested_urls() && !$this->hasAction($action)) { Translatable::disable_locale_filter(); $child = DataObject::get_one('SiteTree', sprintf ( - "\"ParentID\" = %s AND \"URLSegment\" = '%s'", - $this->ID, - Convert::raw2sql($request->param('Action')) + "\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql($action) )); Translatable::enable_locale_filter(); - - if($child && $child->canView()) { - $request->shiftAllParams(); - return ModelAsController::controller_for($child)->handleRequest($request); - } } - Director::set_current_page(null); + if($child) { + $request->shiftAllParams(); + $request->shift(); + + $response = ModelAsController::controller_for($child)->handleRequest($request); + } else { + Director::set_current_page($this->data()); + $response = parent::handleRequest($request); + Director::set_current_page(null); + } + return $response; } diff --git a/core/control/Controller.php b/core/control/Controller.php index e6960bee8..5d8783c82 100755 --- a/core/control/Controller.php +++ b/core/control/Controller.php @@ -198,7 +198,7 @@ class Controller extends RequestHandler { return $result; } } else { - if($this->action == 'index' || $this->hasActionTemplate($this->action)) { + if($this->action == 'index' || $this->hasAction($this->action)) { return $this->getViewer($this->action)->process($this); } else { return $this->httpError(404, "The action '$this->action' does not exist in class $this->class"); @@ -305,16 +305,18 @@ class Controller extends RequestHandler { } return new SSViewer($templates); } - + + public function hasAction($action) { + return parent::hasAction($action) || $this->hasActionTemplate($action); + } + /** * Returns TRUE if this controller has a template that is specifically designed to handle a specific action. * * @param string $action * @return bool */ - public function hasActionTemplate($action = null) { - if(!$action) $action = $this->action; - + public function hasActionTemplate($action) { if(isset($this->templates[$action])) return true; $parentClass = $this->class; diff --git a/core/control/RequestHandler.php b/core/control/RequestHandler.php old mode 100644 new mode 100755 index 296e2bde0..0dd431cb4 --- a/core/control/RequestHandler.php +++ b/core/control/RequestHandler.php @@ -176,6 +176,31 @@ class RequestHandler extends ViewableData { return $this; } + /** + * Checks if this request handler has a specific action (even if the current user cannot access it). + * + * @param string $action + * @return bool + */ + public function hasAction($action) { + if($action == 'index') return true; + + $action = strtolower($action); + $actions = Object::combined_static($this->class, 'allowed_actions', 'RequestHandler'); + + if(is_array($actions)) { + if(array_key_exists($action, $actions) || in_array($action, $actions)) { + return true; + } + } + + if(!is_array($actions) || !$this->uninherited('allowed_actions')) { + if($action != 'init' && $action != 'run' && method_exists($this, $action)) return true; + } + + return false; + } + /** * Check that the given action is allowed to be called from a URL. * It will interrogate {@link self::$allowed_actions} to determine this. diff --git a/core/model/SiteTree.php b/core/model/SiteTree.php index 4db028cd3..4ce1f77a6 100755 --- a/core/model/SiteTree.php +++ b/core/model/SiteTree.php @@ -1343,28 +1343,13 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * - 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($controller instanceof Controller && $controller->hasAction($this->URLSegment)) return false; } } diff --git a/tests/ControllerTest.php b/tests/ControllerTest.php index c7b0438ce..b3edca49a 100755 --- a/tests/ControllerTest.php +++ b/tests/ControllerTest.php @@ -103,6 +103,18 @@ class ControllerTest extends FunctionalTest { $this->assertEquals('/admin/action', Controller::join_links('/admin', 'action')); } + + /** + * @covers Controller::hasAction() + */ + public function testHasAction() { + $controller = new ControllerTest_HasAction(); + + $this->assertFalse($controller->hasAction('undefined'), 'undefined actions do not exist'); + $this->assertTrue($controller->hasAction('allowed_action'), 'allowed actions are recognised'); + $this->assertTrue($controller->hasAction('template_action'), 'action-specific templates are recognised'); + } + } /** @@ -164,4 +176,16 @@ class ControllerTest_FullSecuredController extends Controller { } } -class ControllerTest_UnsecuredController extends ControllerTest_SecuredController {} \ No newline at end of file +class ControllerTest_UnsecuredController extends ControllerTest_SecuredController {} + +class ControllerTest_HasAction extends Controller { + + public static $allowed_actions = array ( + 'allowed_action' + ); + + protected $templates = array ( + 'template_action' => 'template' + ); + +} \ No newline at end of file