Merge pull request #740 from tractorcow/3.0-route-fixes-alt

FIXED: Issue where route table arguments would not be accessible without using non deprecated API

control/Director.php

@@ -167,12 +167,13 @@ class Director implements TemplateGlobalProvider {
 	 * @param string $body The HTTP body
 	 * @param array $headers HTTP headers with key-value pairs
 	 * @param array $cookies to populate $_COOKIE
+	 * @param HTTP_Request $request The {@see HTTP_Request} object generated as a part of this request
 	 * @return SS_HTTPResponse
 	 * 
 	 * @uses getControllerForURL() The rule-lookup logic is handled by this.
 	 * @uses Controller::run() Controller::run() handles the page logic for a Director::direct() call.
 	 */
-	static function test($url, $postVars = null, $session = null, $httpMethod = null, $body = null, $headers = null, $cookies = null) {
+	static function test($url, $postVars = null, $session = null, $httpMethod = null, $body = null, $headers = null, $cookies = null, &$request = null) {
 		// These are needed so that calling Director::test() doesnt muck with whoever is calling it.
 		// Really, it's some inappropriate coupling and should be resolved by making less use of statics
 		$oldStage = Versioned::current_stage();
@@ -217,10 +218,10 @@ class Director implements TemplateGlobalProvider {
 		$_COOKIE = (array) $cookies;
 		$_SERVER['REQUEST_URI'] = Director::baseURL() . $urlWithQuerystring;
 
-		$req = new SS_HTTPRequest($httpMethod, $url, $getVars, $postVars, $body);
+		$request = new SS_HTTPRequest($httpMethod, $url, $getVars, $postVars, $body);
-		if($headers) foreach($headers as $k => $v) $req->addHeader($k, $v);
+		if($headers) foreach($headers as $k => $v) $request->addHeader($k, $v);
 		// TODO: Pass in the DataModel
-		$result = Director::handleRequest($req, $session, DataModel::inst());
+		$result = Director::handleRequest($request, $session, DataModel::inst());
 		
 		// Restore the superglobals
 		$_REQUEST = $existingRequestVars; 
@@ -257,6 +258,7 @@ class Director implements TemplateGlobalProvider {
 			}
 
 			if(($arguments = $request->match($pattern, true)) !== false) {
+				$request->setRouteParams($controllerOptions);
 				// controllerOptions provide some default arguments
 				$arguments = array_merge($controllerOptions, $arguments);
 
@@ -294,20 +296,20 @@ class Director implements TemplateGlobalProvider {
 	/**
 	 * Returns the urlParam with the given name
 	 * 
-	 * @deprecated 3.0 Use SS_HTTPRequest->latestParam()
+	 * @deprecated 3.0 Use SS_HTTPRequest->param()
 	 */
 	static function urlParam($name) {
-		Deprecation::notice('3.0', 'Use SS_HTTPRequest->latestParam() instead.');
+		Deprecation::notice('3.0', 'Use SS_HTTPRequest->param() instead.');
 		if(isset(Director::$urlParams[$name])) return Director::$urlParams[$name];
 	}
 	
 	/**
 	 * Returns an array of urlParams.
 	 * 
-	 * @deprecated 3.0 Use SS_HTTPRequest->latestParams()
+	 * @deprecated 3.0 Use SS_HTTPRequest->params()
 	 */
 	static function urlParams() {
-		Deprecation::notice('3.0', 'Use SS_HTTPRequest->latestParams() instead.');
+		Deprecation::notice('3.0', 'Use SS_HTTPRequest->params() instead.');
 		return Director::$urlParams;
 	}
 

control/HTTPRequest.php

@@ -82,6 +82,21 @@ class SS_HTTPRequest implements ArrayAccess {
 	 */
 	protected $latestParams = array();
 	
+	/**
+	 * @var array $routeParams Contains an associative array of all arguments
+	 * explicitly set in the route table for the current request.
+	 * Useful for passing generic arguments via custom routes.
+	 * 
+	 * E.g. The "Locale" parameter would be assigned "en_NZ" below
+	 * 
+	 * Director:
+	 *   rules:
+	 *     'en_NZ/$URLSegment!//$Action/$ID/$OtherID':
+	 *       Controller: 'ModelAsController'
+	 *       Locale: 'en_NZ'
+	 */
+	protected $routeParams = array();
+	
 	protected $unshiftedButParsedParts = 0;
 	
 	/**
@@ -364,7 +379,6 @@ class SS_HTTPRequest implements ArrayAccess {
 			$shiftCount = sizeof($patternParts);
 		}
 
-		$matched = true;
 		$arguments = array();
 		foreach($patternParts as $i => $part) {
 			$part = trim($part);
@@ -447,22 +461,35 @@ class SS_HTTPRequest implements ArrayAccess {
 	function latestParams() {
 		return $this->latestParams;
 	}
+	
 	function latestParam($name) {
-		if(isset($this->latestParams[$name]))
+		if(isset($this->latestParams[$name])) return $this->latestParams[$name];
-			return $this->latestParams[$name];
+		else return null;
-		else
+	}
-			return null;
+	
+	function routeParams() {
+		return $this->routeParams;
+	}
+	
+	function setRouteParams($params) {
+		$this->routeParams = $params;
+	}
+	
+	function params()
+	{
+		return array_merge($this->allParams, $this->routeParams);
 	}
 	
 	/**
 	 * Finds a named URL parameter (denoted by "$"-prefix in $url_handlers)
-	 * from the full URL.
+	 * from the full URL, or a parameter specified in the route table
 	 * 
 	 * @param string $name
 	 * @return string Value of the URL parameter (if found)
 	 */
 	function param($name) {
-		if(isset($this->allParams[$name])) return $this->allParams[$name];
+		$params = $this->params();
+		if(isset($params[$name])) return $params[$name];
 		else return null;
 	}
 	

tests/control/DirectorTest.php

@@ -18,7 +18,11 @@ class DirectorTest extends SapphireTest {
 		}
 		
 		Config::inst()->update('Director', 'rules', array(
-			'DirectorTestRule/$Action/$ID/$OtherID' => 'DirectorTestRequest_Controller'
+			'DirectorTestRule/$Action/$ID/$OtherID' => 'DirectorTestRequest_Controller',
+			'en-nz/$Action/$ID/$OtherID' => array(
+				'Controller' => 'DirectorTestRequest_Controller',
+				'Locale' => 'en_NZ'
+			)
 		));
 	}
 	
@@ -205,6 +209,25 @@ class DirectorTest extends SapphireTest {
 		Deprecation::restore_settings($originalDeprecation);
 	}
 	
+	/**
+	 * Tests that additional parameters specified in the routing table are 
+	 * saved in the request 
+	 */
+	function testRouteParams() {
+		Director::test('en-nz/myaction/myid/myotherid', null, null, null, null, null, null, $request);
+		
+		$this->assertEquals(
+			$request->params(), 
+			array(
+				'Controller' => 'DirectorTestRequest_Controller',
+				'Action' => 'myaction', 
+				'ID' => 'myid', 
+				'OtherID' => 'myotherid',
+				'Locale' => 'en_NZ'
+			)
+		);
+	}
+
 	function testForceSSLProtectsEntireSite() {
 		$_SERVER['REQUEST_URI'] = Director::baseURL() . 'admin';
 		$output = Director::forceSSL();