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 <andrewjshort@gmail.com>

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@88505 467b73ca-7a2a-4603-9d3b-597d59a354a9

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());
+		$child  = null;
-		$response = parent::handleRequest($request);
+		$action = $request->param('Action');
 		
-		// If the default handler returns an error, due to the action not existing, attempt to fall over to a child
+		// If nested URLs are enabled, and there is no action handler for the current request then attempt to pass
-		// SiteTree object, then use its corresponding ContentController to handle the request. This allows for the
+		// control to a child controller. This allows for the creation of chains of controllers which correspond to a
-		// building of nested chains of controllers corresponding to a nested URL.
+		// nested URL.
-		if(SiteTree::nested_urls() && $response instanceof HTTPResponse && $response->isError()) {
+		if($action && SiteTree::nested_urls() && !$this->hasAction($action)) {
 			Translatable::disable_locale_filter();
 			
 			$child = DataObject::get_one('SiteTree', sprintf (
-				"\"ParentID\" = %s AND \"URLSegment\" = '%s'",
+				"\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql($action)
-				$this->ID,
-				Convert::raw2sql($request->param('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;
 	}
 	

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");
@@ -306,15 +306,17 @@ 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) {
+	public function hasActionTemplate($action) {
-		if(!$action) $action = $this->action;
-		
 		if(isset($this->templates[$action])) return true;
 		
 		$parentClass = $this->class;

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.

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');
+				if($controller instanceof Controller && $controller->hasAction($this->URLSegment)) return false;
-				
-				// 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;
-				}
 			}
 		}
 		

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');
+	}
+	
 }
 
 /**
@@ -165,3 +177,15 @@ class ControllerTest_FullSecuredController extends Controller {
 }
 
 class ControllerTest_UnsecuredController extends ControllerTest_SecuredController {}
+
+class ControllerTest_HasAction extends Controller {
+	
+	public static $allowed_actions = array (
+		'allowed_action'
+	);
+	
+	protected $templates = array (
+		'template_action' => 'template'
+	);
+	
+}