From 79615fbced1996abf2c183ca93288b9909f6c442 Mon Sep 17 00:00:00 2001 From: Robert-Jan de Dreu Date: Sun, 30 Nov 2014 19:34:06 +0100 Subject: [PATCH 1/2] Load controllers with FQCN. Add documentation on how the use FQCN routing Moved check for FQCN and directory and added tests Forgot some of the test description --- classes/Kohana/Request/Client/Internal.php | 6 ++++++ classes/Kohana/Route.php | 8 ++++++++ guide/kohana/routing.md | 20 ++++++++++++++++++++ tests/kohana/RouteTest.php | 18 ++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/classes/Kohana/Request/Client/Internal.php b/classes/Kohana/Request/Client/Internal.php index 773c0dcac..c4d45a151 100644 --- a/classes/Kohana/Request/Client/Internal.php +++ b/classes/Kohana/Request/Client/Internal.php @@ -45,6 +45,12 @@ public function execute_request(Request $request, Response $response) $prefix .= str_replace(array('\\', '/'), '_', trim($directory, '/')).'_'; } + // Use the FQCN to load the Controller + if (substr($controller, 0, 1) === '\\') + { + $prefix = ''; + } + if (Kohana::$profiling) { // Set the benchmark name diff --git a/classes/Kohana/Route.php b/classes/Kohana/Route.php index 66940ea96..a59e9723b 100644 --- a/classes/Kohana/Route.php +++ b/classes/Kohana/Route.php @@ -351,6 +351,14 @@ public function defaults(array $defaults = NULL) return $this->_defaults; } + if (isset($defaults['controller']) AND substr($defaults['controller'], 0, 1) === '\\') + { + if (isset($defaults['directory'])) + { + throw new Kohana_Exception('Route directory should not be set when the controller is a FQCN.'); + } + } + $this->_defaults = $defaults; return $this; diff --git a/guide/kohana/routing.md b/guide/kohana/routing.md index bbf1dcac5..586f547e0 100644 --- a/guide/kohana/routing.md +++ b/guide/kohana/routing.md @@ -68,6 +68,8 @@ If a key in a route is optional (or not present in the route), you can provide a [!!] The `controller` and `action` key must always have a value, so they either need to be required in your route (not inside of parentheses) or have a default value provided. +[!!] When using the `Fully Qualified Class Name` or `FQCN` for the `controller` you aren't allowed to set `directory`. The reason is that when using the `FQCN` the autoloader will look for the right location of the file. + [!!] Kohana automatically converts controllers to follow the standard naming convention. For example /blog/view/123 would look for the controller Controller_Blog in classes/Controller/Blog.php and trigger the action_view() method on it. In the default route, all the keys are optional, and the controller and action are given a default. If we called an empty url, the defaults would fill in and `Controller_Welcome::action_index()` would be called. If we called `foobar` then only the default for action would be used, so it would call `Controller_Foobar::action_index()` and finally, if we called `foobar/baz` then neither default would be used and `Controller_Foobar::action_baz()` would be called. @@ -80,6 +82,24 @@ TODO: example of either using directory or controller where it isn't in the rout ### Directory +### Selecting controllers by their FQCN + +You can also use the `Fully Qualified Class Name` to find the controller. It will enable you to store your controllers at the location of your choosing as long as the autoloader is able to find it. + +When using `FQCN` controllers the controllers will not use any prefixed or affixes so `\Acme\Module\Controller\Demo` will point to `class Demo {}` and `\Acme\Module\Controller\DemoController` will point to `class DemoController {}`. You will have full flexibility on how you name you classes. + +Because of the autoloading the `directory` default value will not work and setting it will result in a `Exception` being thrown. + +An example of a `FQCN` route: + + /* + * Routes using FQCN + */ + Route::set('default', 'controller-name/') + ->defaults(array( + 'controller' => '\Acme\Module\Controller\DemoController' + )); + ## Route Filters In 3.3, you can specify advanced routing schemes by using filter callbacks. When you need to match a route based on more than just the URI of a request, for example, based on the method request (GET/POST/DELETE), a filter will allow you to do so. These filters will receive the `Route` object being tested, the currently matched `$params` array, and the `Request` object as the three parameters. Here's a simple example: diff --git a/tests/kohana/RouteTest.php b/tests/kohana/RouteTest.php index a6f6d57be..389c1519d 100644 --- a/tests/kohana/RouteTest.php +++ b/tests/kohana/RouteTest.php @@ -521,6 +521,24 @@ public function test_defaults_are_not_used_if_param_is_identical() $this->assertSame('welcome2', $route->uri(array('controller' => 'welcome2'))); } + /** + * When using a FQCN test if the directory is not set else throw an exception + * + * @test + * @covers Route::defaults + */ + public function test_defaults_throws_exception_when_setting_fqcn_and_directory() + { + $this->setExpectedException('Kohana_Exception', 'Route directory should not be set when the controller is a FQCN.'); + + $route = new Route('((/(/)))'); + $route->defaults(array( + 'directory' => 'directory/path', + 'controller' => '\FQCN\Class\Name', + 'action' => 'index' + )); + } + /** * Provider for test_required_parameters_are_needed * From f3a20814a0013cada83c2d31108071f9e9b528e8 Mon Sep 17 00:00:00 2001 From: Andrew Coulton Date: Mon, 12 Jan 2015 13:44:15 +0000 Subject: [PATCH 2/2] Tests for Request_Client_Internal mapping of request params to controller class Adds tests to cover the new FQCN controller class mapping as well as legacy behaviour with and without directory prefixes. --- tests/kohana/request/client/InternalTest.php | 129 ++++++++++++++++--- 1 file changed, 108 insertions(+), 21 deletions(-) diff --git a/tests/kohana/request/client/InternalTest.php b/tests/kohana/request/client/InternalTest.php index 434862bc0..2a2424b59 100644 --- a/tests/kohana/request/client/InternalTest.php +++ b/tests/kohana/request/client/InternalTest.php @@ -1,4 +1,7 @@ getMock('Request', array('directory', 'controller', 'action', 'uri', 'response'), array($uri)); + $request = new FixedParametersRequestStub(array( + 'directory' => $directory, + 'controller' => $controller, + 'action' => $action, + 'uri' => $uri, + )); - $request->expects($this->any()) - ->method('directory') - ->will($this->returnValue($directory)); + $internal_client = new Request_Client_Internal; + $response = $internal_client->execute($request); + $this->assertSame($expected, $response->status()); + } - $request->expects($this->any()) - ->method('controller') - ->will($this->returnValue($controller)); + /** + * @return array + */ + public function provider_controller_class_mapping() + { + return array( + array( + array('controller' => 'RequestClientInternalTestControllerDummy', 'directory' => ''), + 'Controller_RequestClientInternalTestControllerDummy', + ), + array( + array('controller' => 'ControllerDummy', 'directory' => 'RequestClientInternalTest'), + 'Controller_RequestClientInternalTest_ControllerDummy', + ), + array( + array('controller' => '\RequestClientInternalTestControllerDummy', 'directory' => ''), + '\RequestClientInternalTestControllerDummy', + ), + array( + array('controller' => '\test\RequestClientInternalTest\ControllerDummy', 'directory' => ''), + '\test\RequestClientInternalTest\ControllerDummy', + ), + ); + } - $request->expects($this->any()) - ->method('action') - ->will($this->returnValue($action)); + /** + * @param array $request_params + * @param string $expect_class + * + * @dataProvider provider_controller_class_mapping + */ + public function test_maps_request_params_to_controller_class($request_params, $expect_class) + { + $client = new Request_Client_Internal; + $request = new FixedParametersRequestStub($request_params); + $response = new ControllerCapturingResponseStub; + $client->execute_request($request, $response); - $request->expects($this->any()) - ->method('uri') - ->will($this->returnValue($uri)); + $this->assertInstanceOf($expect_class, $response->getController()); + } +} - $request->expects($this->any()) - ->method('response') - ->will($this->returnValue($this->getMock('Response'))); + class RequestClientInternalTestControllerDummy extends Controller { - $internal_client = new Request_Client_Internal; + public function execute() + { + if ($this->response instanceof ControllerCapturingResponseStub) + { + $this->response->setController($this); + } + return $this->response; + } - $response = $internal_client->execute($request); + } + + class Controller_RequestClientInternalTest_ControllerDummy extends \RequestClientInternalTestControllerDummy {} + class Controller_RequestClientInternalTestControllerDummy extends \RequestClientInternalTestControllerDummy {} + +} // End of global namespace + +namespace test\RequestClientInternalTest { + + class FixedParametersRequestStub extends \Request { + + public function __construct($params) + { + $params = array_merge( + array( + 'directory' => '', + 'controller' => '', + 'action' => 'index', + 'uri' => '/', + ), + $params + ); + $this->_directory = $params['directory']; + $this->_controller = $params['controller']; + $this->_action = $params['action']; + $this->_uri = $params['uri']; + } - $this->assertSame($expected, $response->status()); } -} \ No newline at end of file + + class ControllerCapturingResponseStub extends \Response { + + protected $controller; + + public function __construct() {} + + public function setController(\Controller $controller) + { + $this->controller = $controller; + } + + public function getController() + { + return $this->controller; + } + } + + class ControllerDummy extends \RequestClientInternalTestControllerDummy {} +}