Commit b65b30e7 authored by Fabien Potencier's avatar Fabien Potencier

merged branch fabpot/route-factory (PR #376)

Commits
-------

c463a6e1 added some unit tests
e29c91ed added an exception when a method does not exist to ease debugging
3fd92830 made controller methods (before, after, ...) extensible

Discussion
----------

made controller methods (before, after, ...) extensible

You can now add your own methods on controllers (like the built-in
after, before, convert, ...) by defining your own Route class:

    class MyRoute extends Route
    {
        public function secure($roles, $app)
        {
            // do something
        }
    }

and then change the "route_factory" accordingly:

    $this['route_factory'] = function () {
        return new MyRoute();
    };

If you want to benefit from the new methods in a custom controller
collection, pass an instance of your Route to the ControllerCollection
constructor:

    $controllers = new ControllerCollection(new MyRoute());

or even better, use the "controllers_factory" service:

    $controllers = $app['controllers_factory'];

The limitation is that you can only have one custom Route class in an
Application. But this PR is just the first step. Thanks to PHP 5.4 and
the new traits support, the next pull request (#378) uses this new
refactoring to provide traits that you can add to your
custom Application Route class.

---------------------------------------------------------------------------

by igorw at 2012-06-18T12:13:33Z

Looks good to me. One thing, what about a method_exists() in `__call` to give a better error message?

---------------------------------------------------------------------------

by fabpot at 2012-06-18T17:41:34Z

I've added an exception for when the method does not exist.

---------------------------------------------------------------------------

by fabpot at 2012-06-18T18:03:00Z

And now with some unit tests.

---------------------------------------------------------------------------

by igorw at 2012-06-18T18:33:33Z

👍
parents 7e603906 c463a6e1
......@@ -70,9 +70,18 @@ class Application extends \Pimple implements HttpKernelInterface, EventSubscribe
});
$this['controllers'] = $this->share(function () use ($app) {
return new ControllerCollection();
return $app['controllers_factory'];
});
$this['controllers_factory'] = function () use ($app) {
return new ControllerCollection($app['route_factory']);
};
$this['route_class'] = 'Silex\\Route';
$this['route_factory'] = function () use ($app) {
return new $app['route_class']();
};
$this['exception_handler'] = $this->share(function () {
return new ExceptionHandler();
});
......@@ -119,7 +128,7 @@ class Application extends \Pimple implements HttpKernelInterface, EventSubscribe
}
foreach ((array) $route->getOption('_before_middlewares') as $callback) {
$ret = call_user_func($callback, $request);
$ret = call_user_func($callback, $request, $app);
if ($ret instanceof Response) {
$event->setResponse($ret);
......
......@@ -72,113 +72,13 @@ class Controller
return $this;
}
/**
* Sets the requirement for a route variable.
*
* @param string $variable The variable name
* @param string $regexp The regexp to apply
*
* @return Controller $this The current Controller instance
*/
public function assert($variable, $regexp)
{
$this->route->assert($variable, $regexp);
return $this;
}
/**
* Sets the default value for a route variable.
*
* @param string $variable The variable name
* @param mixed $default The default value
*
* @return Controller $this The current Controller instance
*/
public function value($variable, $default)
{
$this->route->value($variable, $default);
return $this;
}
/**
* Sets a converter for a route variable.
*
* @param string $variable The variable name
* @param mixed $callback A PHP callback that converts the original value
*
* @return Controller $this The current Controller instance
*/
public function convert($variable, $callback)
public function __call($method, $arguments)
{
$this->route->convert($variable, $callback);
return $this;
if (!method_exists($this->route, $method)) {
throw new \BadMethodCallException(sprintf('Method "%s::%s" does not exist.', get_class($this->route), $method));
}
/**
* Sets the requirement for the HTTP method.
*
* @param string $method The HTTP method name. Multiple methods can be supplied, delimited by a pipe character '|', eg. 'GET|POST'
*
* @return Controller $this The current Controller instance
*/
public function method($method)
{
$this->route->method($method);
return $this;
}
/**
* Sets the requirement of HTTP (no HTTPS) on this controller.
*
* @return Controller $this The current Controller instance
*/
public function requireHttp()
{
$this->route->requireHttp();
return $this;
}
/**
* Sets the requirement of HTTPS on this controller.
*
* @return Controller $this The current Controller instance
*/
public function requireHttps()
{
$this->route->requireHttps();
return $this;
}
/**
* Sets a callback to handle before triggering the route callback.
*
* @param mixed $callback A PHP callback to be triggered when the Route is matched, just before the route callback
*
* @return Controller $this The current Controller instance
*/
public function before($callback)
{
$this->route->before($callback);
return $this;
}
/**
* Sets a callback to handle after the route callback.
*
* @param mixed $callback A PHP callback to be triggered after the route callback
*
* @return Controller $this The current Controller instance
*/
public function after($callback)
{
$this->route->after($callback);
call_user_func_array(array($this->route, $method), $arguments);
return $this;
}
......
......@@ -32,9 +32,9 @@ class ControllerCollection
/**
* Constructor.
*/
public function __construct()
public function __construct(Route $defaultRoute)
{
$this->defaultRoute = new Route('');
$this->defaultRoute = $defaultRoute;
}
/**
......@@ -110,144 +110,16 @@ class ControllerCollection
return $this->match($pattern, $to)->method('DELETE');
}
/**
* Sets the requirement for a route variable.
*
* @param string $variable The variable name
* @param string $regexp The regexp to apply
*
* @return ControllerCollection $this The current Controller instance
*/
public function assert($variable, $regexp)
{
$this->defaultRoute->assert($variable, $regexp);
foreach ($this->controllers as $controller) {
$controller->assert($variable, $regexp);
}
return $this;
}
/**
* Sets the default value for a route variable.
*
* @param string $variable The variable name
* @param mixed $default The default value
*
* @return ControllerCollection $this The current Controller instance
*/
public function value($variable, $default)
{
$this->defaultRoute->value($variable, $default);
foreach ($this->controllers as $controller) {
$controller->value($variable, $default);
}
return $this;
}
/**
* Sets a converter for a route variable.
*
* @param string $variable The variable name
* @param mixed $callback A PHP callback that converts the original value
*
* @return ControllerCollection $this The current Controller instance
*/
public function convert($variable, $callback)
{
$this->defaultRoute->convert($variable, $callback);
foreach ($this->controllers as $controller) {
$controller->convert($variable, $callback);
}
return $this;
}
/**
* Sets the requirement for the HTTP method.
*
* @param string $method The HTTP method name. Multiple methods can be supplied, delimited by a pipe character '|', eg. 'GET|POST'
*
* @return ControllerCollection $this The current Controller instance
*/
public function method($method)
{
$this->defaultRoute->method($method);
foreach ($this->controllers as $controller) {
$controller->method($method);
}
return $this;
}
/**
* Sets the requirement of HTTP (no HTTPS) on this controller.
*
* @return ControllerCollection $this The current Controller instance
*/
public function requireHttp()
{
$this->defaultRoute->requireHttp();
foreach ($this->controllers as $controller) {
$controller->requireHttp();
}
return $this;
}
/**
* Sets the requirement of HTTPS on this controller.
*
* @return ControllerCollection $this The current Controller instance
*/
public function requireHttps()
{
$this->defaultRoute->requireHttps();
foreach ($this->controllers as $controller) {
$controller->requireHttps();
}
return $this;
}
/**
* Sets a callback to handle before triggering the route callback.
*
* @param mixed $callback A PHP callback to be triggered when the Route is matched, just before the route callback
*
* @return ControllerCollection $this The current ControllerCollection instance
*/
public function before($callback)
public function __call($method, $arguments)
{
$this->defaultRoute->before($callback);
foreach ($this->controllers as $controller) {
$controller->before($callback);
}
return $this;
if (!method_exists($this->defaultRoute, $method)) {
throw new \BadMethodCallException(sprintf('Method "%s::%s" does not exist.', get_class($this->defaultRoute), $method));
}
/**
* Sets a callback to handle after the route callback.
*
* @param mixed $callback A PHP callback to be triggered after the route callback
*
* @return ControllerCollection $this The current ControllerCollection instance
*/
public function after($callback)
{
$this->defaultRoute->after($callback);
call_user_func_array(array($this->defaultRoute, $method), $arguments);
foreach ($this->controllers as $controller) {
$controller->after($callback);
call_user_func_array(array($controller, $method), $arguments);
}
return $this;
......
......@@ -20,6 +20,11 @@ use Symfony\Component\Routing\Route as BaseRoute;
*/
class Route extends BaseRoute
{
public function __construct($pattern = '', array $defaults = array(), array $requirements = array(), array $options = array())
{
parent::__construct($pattern, $defaults, $requirements, $options);
}
/**
* Sets the requirement for a route variable.
*
......
......@@ -25,14 +25,14 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
{
public function testGetRouteCollectionWithNoRoutes()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$routes = $controllers->flush();
$this->assertEquals(0, count($routes->all()));
}
public function testGetRouteCollectionWithRoutes()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->match('/foo', function () {});
$controllers->match('/bar', function () {});
......@@ -42,7 +42,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testControllerFreezing()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$fooController = $controllers->match('/foo', function () {})->bind('foo');
$barController = $controllers->match('/bar', function () {})->bind('bar');
......@@ -64,7 +64,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testConflictingRouteNames()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$mountedRootController = $controllers->match('/', function () {});
......@@ -78,7 +78,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testUniqueGeneratedRouteNames()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->match('/a-a', function () {});
$controllers->match('/a_a', function () {});
......@@ -91,7 +91,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testAssert()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->assert('id', '\d+');
$controller = $controllers->match('/{id}/{name}/{extra}', function () {})->assert('name', '\w+')->assert('extra', '.*');
$controllers->assert('extra', '\w+');
......@@ -103,7 +103,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testValue()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->value('id', '1');
$controller = $controllers->match('/{id}/{name}/{extra}', function () {})->value('name', 'Fabien')->value('extra', 'Symfony');
$controllers->value('extra', 'Twig');
......@@ -115,7 +115,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testConvert()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->convert('id', '1');
$controller = $controllers->match('/{id}/{name}/{extra}', function () {})->convert('name', 'Fabien')->convert('extra', 'Symfony');
$controllers->convert('extra', 'Twig');
......@@ -125,7 +125,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testRequireHttp()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->requireHttp();
$controller = $controllers->match('/{id}/{name}/{extra}', function () {})->requireHttps();
......@@ -138,7 +138,7 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testBefore()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->before('mid1');
$controller = $controllers->match('/{id}/{name}/{extra}', function () {})->before('mid2');
$controllers->before('mid3');
......@@ -148,11 +148,42 @@ class ControllerCollectionTest extends \PHPUnit_Framework_TestCase
public function testAfter()
{
$controllers = new ControllerCollection();
$controllers = new ControllerCollection(new Route());
$controllers->after('mid1');
$controller = $controllers->match('/{id}/{name}/{extra}', function () {})->after('mid2');
$controllers->after('mid3');
$this->assertEquals(array('mid1', 'mid2', 'mid3'), $controller->getRoute()->getOption('_after_middlewares'));
}
public function testRouteExtension()
{
$route = new MyRoute1();
$controller = new ControllerCollection($route);
$controller->foo('foo');
$this->assertEquals('foo', $route->foo);
}
/**
* @expectedException \BadMethodCallException
*/
public function testRouteMethodDoesNotExist()
{
$route = new MyRoute1();
$controller = new ControllerCollection($route);
$controller->bar();
}
}
class MyRoute1 extends Route
{
public $foo;
public function foo($value)
{
$this->foo = $value;
}
}
......@@ -88,4 +88,35 @@ class ControllerTest extends \PHPUnit_Framework_TestCase
array(new Route('/underscores_and.periods'), '_underscores_and.periods'),
);
}
public function testRouteExtension()
{
$route = new MyRoute();
$controller = new Controller($route);
$controller->foo('foo');
$this->assertEquals('foo', $route->foo);
}
/**
* @expectedException \BadMethodCallException
*/
public function testRouteMethodDoesNotExist()
{
$route = new MyRoute();
$controller = new Controller($route);
$controller->bar();
}
}
class MyRoute extends Route
{
public $foo;
public function foo($value)
{
$this->foo = $value;
}
}
......@@ -12,6 +12,7 @@
namespace Silex\Tests;
use Silex\Application;
use Silex\Route;
use Silex\ControllerCollection;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
......@@ -45,7 +46,7 @@ class FunctionalTest extends \PHPUnit_Framework_TestCase
public function testMount()
{
$mounted = new ControllerCollection();
$mounted = new ControllerCollection(new Route());
$mounted->get('/{name}', function ($name) { return new Response($name); });
$app = new Application();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment