From fdacc9de1612c03c9462363c1bc05c2232dbfae8 Mon Sep 17 00:00:00 2001 From: mark_story Date: Fri, 12 Aug 2011 21:21:05 -0400 Subject: [PATCH] Fixing issues with request stack not being used correctly when there are requestAction requests being performed. Adding Router::popRequest() to allow manipulation of request stack so nested requestAction or serial requestAction calls work correctly. Fixes #1906 --- lib/Cake/Core/Object.php | 4 +++- lib/Cake/Routing/Router.php | 21 +++++++++++----- lib/Cake/Test/Case/Core/ObjectTest.php | 6 ++++- lib/Cake/Test/Case/Routing/RouterTest.php | 29 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/Cake/Core/Object.php b/lib/Cake/Core/Object.php index 237b74593..252604586 100644 --- a/lib/Cake/Core/Object.php +++ b/lib/Cake/Core/Object.php @@ -87,7 +87,9 @@ class Object { } $dispatcher = new Dispatcher(); - return $dispatcher->dispatch($request, new CakeResponse(), $extra); + $result = $dispatcher->dispatch($request, new CakeResponse(), $extra); + Router::popRequest(); + return $result; } /** diff --git a/lib/Cake/Routing/Router.php b/lib/Cake/Routing/Router.php index 11cf19537..c7f4f0abe 100644 --- a/lib/Cake/Routing/Router.php +++ b/lib/Cake/Routing/Router.php @@ -517,6 +517,9 @@ class Router { * parameters as the current request parameters that are merged with url arrays * created later in the request. * + * Nested requests will create a stack of requests. You can remove requests using + * Router::popRequest(). This is done automatically when using Object::requestAction(). + * * Will accept either a CakeRequest object or an array of arrays. Support for * accepting arrays may be removed in the future. * @@ -535,6 +538,17 @@ class Router { } } +/** + * Pops a request off of the request stack. Used when doing requestAction + * + * @return CakeRequest The request removed from the stack. + * @see Router::setRequestInfo() + * @see Object::requestAction() + */ + public static function popRequest() { + return array_pop(self::$_requests); + } + /** * Get the either the current request object, or the first one. * @@ -675,12 +689,7 @@ class Router { $path = array('base' => null); if (!empty(self::$_requests)) { - $currentRequest = self::$_requests[count(self::$_requests) - 1]; - if (!empty($currentRequest->params['requested'])) { - $request = self::$_requests[0]; - } else { - $request = $currentRequest; - } + $request = self::$_requests[count(self::$_requests) - 1]; $params = $request->params; $path = array('base' => $request->base, 'here' => $request->here); } diff --git a/lib/Cake/Test/Case/Core/ObjectTest.php b/lib/Cake/Test/Case/Core/ObjectTest.php index a46ee6aaa..0e318ff56 100644 --- a/lib/Cake/Test/Case/Core/ObjectTest.php +++ b/lib/Cake/Test/Case/Core/ObjectTest.php @@ -463,12 +463,14 @@ class ObjectTest extends CakeTestCase { 'views' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS), 'controllers' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Controller' . DS) ), true); + $this->assertNull(Router::getRequest(), 'request stack should be empty.'); + $result = $this->object->requestAction(''); $this->assertFalse($result); $result = $this->object->requestAction('/request_action/test_request_action'); $expected = 'This is a test'; - $this->assertEqual($expected, $result);; + $this->assertEqual($expected, $result); $result = $this->object->requestAction('/request_action/another_ra_test/2/5'); $expected = 7; @@ -488,6 +490,8 @@ class ObjectTest extends CakeTestCase { $result = $this->object->requestAction('/request_action/normal_request_action'); $expected = 'Hello World'; $this->assertEqual($expected, $result); + + $this->assertNull(Router::getRequest(), 'requests were not popped off the stack, this will break url generation'); } /** diff --git a/lib/Cake/Test/Case/Routing/RouterTest.php b/lib/Cake/Test/Case/Routing/RouterTest.php index 88d8110b7..f1088093a 100644 --- a/lib/Cake/Test/Case/Routing/RouterTest.php +++ b/lib/Cake/Test/Case/Routing/RouterTest.php @@ -2298,6 +2298,35 @@ class RouterTest extends CakeTestCase { $this->assertEqual($result->webroot, '/'); } +/** + * Test that Router::url() uses the first request + */ + public function testUrlWithRequestAction() { + $firstRequest = new CakeRequest('/posts/index'); + $firstRequest->addParams(array( + 'plugin' => null, + 'controller' => 'posts', + 'action' => 'index' + ))->addPaths(array('base' => '')); + + $secondRequest = new CakeRequest('/posts/index'); + $secondRequest->addParams(array( + 'requested' => 1, + 'plugin' => null, + 'controller' => 'comments', + 'action' => 'listing' + ))->addPaths(array('base' => '')); + + Router::setRequestInfo($firstRequest); + Router::setRequestInfo($secondRequest); + + $result = Router::url(array('base' => false)); + $this->assertEquals('/comments/listing', $result, 'with second requests, the last should win.'); + + Router::popRequest(); + $result = Router::url(array('base' => false)); + $this->assertEquals('/posts', $result, 'with second requests, the last should win.'); + } /** * test that a route object returning a full url is not modified. *