From c2a8eec43c91995c89701ae2e55816458ae01486 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 27 Aug 2012 10:56:59 +1200 Subject: [PATCH] APICHANGE: Changed behaviour of HTTP_Request::params to include route table params (as per 2.4 behaviour, see FIX: below). ADDED: HTTP_Request::params() to retrieve all (shifted) params used in the request FIXED: Issue where route-table level arguments would not be accessible without using non-deprecated API. ADDED: Test case to test the above items UPDATED: Extended Director::test to allow for the retrieval of the request object UPDATED: Deprecated notice on Director::urlParam and Director::urlParams REMOVED: Unused variable FIXED: Coding convention conformity --- control/Director.php | 18 ++++++++------- control/HTTPRequest.php | 41 ++++++++++++++++++++++++++++------ tests/control/DirectorTest.php | 27 +++++++++++++++++++++- 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/control/Director.php b/control/Director.php index bc3973503..8734d69d6 100644 --- a/control/Director.php +++ b/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); - if($headers) foreach($headers as $k => $v) $req->addHeader($k, $v); + $request = new SS_HTTPRequest($httpMethod, $url, $getVars, $postVars, $body); + 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; } diff --git a/control/HTTPRequest.php b/control/HTTPRequest.php index 59c8a5875..0e1cc252d 100644 --- a/control/HTTPRequest.php +++ b/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])) - return $this->latestParams[$name]; - else - return null; + if(isset($this->latestParams[$name])) return $this->latestParams[$name]; + 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; } diff --git a/tests/control/DirectorTest.php b/tests/control/DirectorTest.php index 3d68179f3..bfd555749 100644 --- a/tests/control/DirectorTest.php +++ b/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' + ) )); } @@ -204,6 +208,27 @@ class DirectorTest extends SapphireTest { Deprecation::restore_settings($originalDeprecation); } + + /** + * Tests that additional parameters specified in the routing table are + * saved in the request + */ + function testRouteParams() { + Deprecation::notification_version('2.4'); + + 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';