Skip to content

Commit

Permalink
Start refactoring requestContext.
Browse files Browse the repository at this point in the history 
* Make requestContext less expensive to generate.
* Remove garbage methods in Router.
* Remove tests for garbage methods.
* Add methods to RouteCollection to facilitate testing.
* Update tests.
  • Loading branch information
@markstory
markstory committed Jul 4, 2012
1 parent 4ed0f54 commit 2f2466e
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 165 deletions.
20 changes: 19 additions & 1 deletion lib/Cake/Routing/RouteCollection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use Cake\Routing\Route\Route;

class RouteCollection {
class RouteCollection implements \Countable {

/**
* A hash table of routes indexed by route names.
Expand Down Expand Up @@ -165,4 +165,22 @@ public function promote($which) {
return true;
}

/**
* Get a route out of the collection.
*
* @param int $index The index of the route you want.
* @return mixed Either the route object or null.
*/
public function get($index) {
return isset($this->_routes[$index]) ? $this->_routes[$index] : null;
}

/**
* Part of the countable interface.
*
* @return integer The number of connected routes.
*/
public function count() {
return count($this->_routes);
}
}
126 changes: 64 additions & 62 deletions lib/Cake/Routing/Router.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,19 @@ class Router {
*/
protected static $_requests = array();

/**
* The top most request's context. Updated whenever
* requests are pushed/popped off the stack.
*
* @var array
*/
protected static $_requestContext = array(
'_base' => '',
'_port' => 80,
'_scheme' => 'http',
'_host' => 'localhost',
);

/**
* Initial state is populated the first time reload() is called which is at the bottom
* of this file. This is a cheat as get_class_vars() returns the value of static vars even if they
Expand Down Expand Up @@ -457,6 +470,16 @@ protected static function _parseExtension($url) {
return compact('ext', 'url');
}

/**
* Set the route collection object Router should use.
*
* @param Cake\Routing\RouteCollection $routes
* @return void
*/
public static function setRouteCollection(RouteCollection $routes) {
self::$_routes = $routes;
}

/**
* Takes parameter and path information back from the Dispatcher, sets these
* parameters as the current request parameters that are merged with url arrays
Expand All @@ -473,7 +496,7 @@ protected static function _parseExtension($url) {
*/
public static function setRequestInfo($request) {
if ($request instanceof Request) {
static::$_requests[] = $request;
static::pushRequest($request);
} else {
$requestData = $request;
$requestData += array(array(), array());
Expand All @@ -484,29 +507,61 @@ public static function setRequestInfo($request) {
);
$request = new Request();
$request->addParams($requestData[0])->addPaths($requestData[1]);
static::$_requests[] = $request;
static::pushRequest($request);
}
}

/**
* Set the route collection object Router should use.
* Push a request onto the request stack. Pushing a request
* sets the request context used when generating urls.
*
* @param Cake\Routing\RouteCollection $routes
* @param Cake\Network\Request $request
* @return void
*/
public static function setRouteCollection(RouteCollection $routes) {
self::$_routes = $routes;
public static function pushRequest(Request $request) {
self::$_requests[] = $request;
self::_setRequestContext($request);
}

/**
* Populate the request context used to generate URL's
* Generally set to the last/most recent request.
*
* @param Cake\Network\Request $request
* @return void
*/
protected static function _setRequestcontext(Request $request) {
self::$_requestContext = array(
'_base' => $request->base,
'_port' => $request->port(),
'_scheme' => $request->scheme(),
'_host' => $request->host()
);
}

/**
* Pops a request off of the request stack. Used when doing requestAction
*
* @return Cake\Network\Request The request removed from the stack.
* @see Router::setRequestInfo()
* @see Router::pushRequest()
* @see Object::requestAction()
*/
public static function popRequest() {
return array_pop(static::$_requests);
$removed = array_pop(static::$_requests);
$last = end(static::$_requests);
static::_setRequestContext($last);
reset(static::$_requests);
return $removed;
}

/**
* Fetch the current request context.
*
* @return array An array with the current request context.
*/
public function getRequestContext() {
return self::$_requestContext;
>>>>>>> Start refactoring requestContext.
}

/**
Expand Down Expand Up @@ -597,25 +652,13 @@ public static function url($url = null, $full = false) {
$request = static::getRequest(true);
if ($request) {
$params = $request->params;
$requestContext = array(
'_base' => $request->base,
'_port' => $request->port(),
'_scheme' => $request->scheme(),
'_host' => $request->host()
);
$here = $request->here;
} else {
$params = array(
'plugin' => null,
'controller' => null,
'action' => 'index'
);
$requestContext = array(
'_base' => '',
'_port' => 80,
'_scheme' => 'http',
'_host' => 'localhost',
);
$here = null;
}

Expand Down Expand Up @@ -672,6 +715,7 @@ public static function url($url = null, $full = false) {
'controller' => $params['controller'],
'plugin' => $params['plugin']
);
$requestContext = self::$_requestContext;
$output = self::$_routes->match($url, $params, $requestContext);
} else {
// String urls.
Expand Down Expand Up @@ -778,48 +822,6 @@ public static function normalize($url = '/') {
return $url;
}

/**
* Returns the route matching the current request URL.
*
* @return Cake\Routing\Route\Route Matching route object.
* @todo Remove? Not really that useful.
*/
public static function &requestRoute() {
return static::$_currentRoute[0];
}

/**
* Returns the route matching the current request (useful for requestAction traces)
*
* @return Cake\Routing\Route\Route Matching route object.
* @todo Remove? Not really that useful.
*/
public static function &currentRoute() {
return static::$_currentRoute[count(static::$_currentRoute) - 1];
}

/**
* Removes the plugin name from the base URL.
*
* @param string $base Base URL
* @param string $plugin Plugin name
* @return string base url with plugin name removed if present
* @todo Remove? Not really that useful.
*/
public static function stripPlugin($base, $plugin = null) {
if ($plugin != null) {
$base = preg_replace('/(?:' . $plugin . ')/', '', $base);
$base = str_replace('//', '', $base);
$pos1 = strrpos($base, '/');
$char = strlen($base) - 1;

if ($pos1 === $char) {
$base = substr($base, 0, $char);
}
}
return $base;
}

/**
* Instructs the router to parse out file extensions from the URL. For example,
* http://example.com/posts.rss would yield an file extension of "rss".
Expand Down
115 changes: 13 additions & 102 deletions lib/Cake/Test/TestCase/Routing/RouterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Cake\Core\Plugin;
use Cake\Network\Request;
use Cake\Routing\Route\Route;
use Cake\Routing\RouteCollection;
use Cake\Routing\Router;
use Cake\TestSuite\TestCase;

Expand Down Expand Up @@ -763,38 +764,6 @@ public function testUrlGenerationWithExtensions() {
$this->assertEquals($expected, $result);
}

/**
* testPluginUrlGeneration method
*
* @return void
*/
public function testUrlGenerationPlugins() {
$request = new Request();
Router::setRequestInfo(
$request->addParams(array(
'plugin' => 'test', 'controller' => 'controller', 'action' => 'index'
))->addPaths(array(
'base' => '/base', 'here' => '/clients/sage/portal/donations', 'webroot' => '/base/'
))
);

$this->assertEquals(Router::url('read/1'), '/base/test/controller/read/1');

Router::reload();
Router::connect('/:lang/:plugin/:controller/*', array('action' => 'index'));

$request = new Request();
Router::setRequestInfo(
$request->addParams(array(
'lang' => 'en',
'plugin' => 'shows', 'controller' => 'shows', 'action' => 'index',
'url' => array('url' => 'en/shows/'),
))->addPaths(array(
'base' => '', 'here' => '/en/shows', 'webroot' => '/'
))
);
}

/**
* test that you can leave active plugin routes with plugin = null
*
Expand Down Expand Up @@ -1751,67 +1720,6 @@ public function testRegexRouteMatching() {
$this->assertEquals($expected, $result);
}

/**
* testStripPlugin
*
* @return void
*/
public function testStripPlugin() {
$pluginName = 'forums';
$url = 'example.com/' . $pluginName . '/';
$expected = 'example.com';

$this->assertEquals($expected, Router::stripPlugin($url, $pluginName));
$this->assertEquals(Router::stripPlugin($url), $url);
$this->assertEquals(Router::stripPlugin($url, null), $url);
}

/**
* testCurrentRoute
*
* This test needs some improvement and actual requestAction() usage
*
* @return void
*/
public function testCurrentRoute() {
$this->markTestIncomplete('Fails due to changes for RouteCollection.');

$url = array('controller' => 'pages', 'action' => 'display', 'government');
Router::connect('/government', $url);
Router::parse('/government');
$route = Router::currentRoute();
$this->assertEquals(array_merge($url, array('plugin' => null)), $route->defaults);
}

/**
* testRequestRoute
*
* @return void
*/
public function testRequestRoute() {
$this->markTestIncomplete('Fails due to changes for RouteCollection.');

$url = array('controller' => 'products', 'action' => 'display', 5);
Router::connect('/government', $url);
Router::parse('/government');
$route = Router::requestRoute();
$this->assertEquals(array_merge($url, array('plugin' => null)), $route->defaults);

// test that the first route is matched
$newUrl = array('controller' => 'products', 'action' => 'display', 6);
Router::connect('/government', $url);
Router::parse('/government');
$route = Router::requestRoute();
$this->assertEquals(array_merge($url, array('plugin' => null)), $route->defaults);

// test that an unmatched route does not change the current route
$newUrl = array('controller' => 'products', 'action' => 'display', 6);
Router::connect('/actor', $url);
Router::parse('/government');
$route = Router::requestRoute();
$this->assertEquals(array_merge($url, array('plugin' => null)), $route->defaults);
}

/**
* test that connectDefaults() can disable default route connection
*
Expand Down Expand Up @@ -2137,21 +2045,24 @@ public function testResourceMap() {
* @return void
*/
public function testRouteRedirection() {
$this->markTestIncomplete('Fails due to changes for RouteCollection.');
$routes = new RouteCollection();
Router::setRouteCollection($routes);

Router::redirect('/blog', array('controller' => 'posts'), array('status' => 302));
$this->assertEquals(1, count(Router::$routes));
Router::$routes[0]->response = $this->getMock('Cake\Network\Response', array('_sendHeader'));
Router::$routes[0]->stop = false;
$this->assertEquals(302, Router::$routes[0]->options['status']);
$this->assertEquals(1, count($routes));

$routes->get(0)->response = $this->getMock('Cake\Network\Response', array('_sendHeader'));
$routes->get(0)->stop = false;
$this->assertEquals(302, $routes->get(0)->options['status']);

Router::parse('/blog');
$header = Router::$routes[0]->response->header();
$header = $routes->get(0)->response->header();
$this->assertEquals(Router::url('/posts', true), $header['Location']);
$this->assertEquals(302, Router::$routes[0]->response->statusCode());
$this->assertEquals(302, $routes->get(0)->response->statusCode());

Router::$routes[0]->response = $this->getMock('Cake\Network\Response', array('_sendHeader'));
$routes->get(0)->response = $this->getMock('Cake\Network\Response', array('_sendHeader'));
Router::parse('/not-a-match');
$this->assertEquals(array(), Router::$routes[0]->response->header());
$this->assertEquals(array(), $routes->get(0)->response->header());
}

/**
Expand Down

0 comments on commit 2f2466e

Please sign in to comment.