Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Allow routes to be registered fluently #14354

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Illuminate/Foundation/Http/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function __construct(Application $app, Router $router)
}

foreach ($this->routeMiddleware as $key => $middleware) {
$router->middleware($key, $middleware);
$router->middlewareAlias($key, $middleware);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Routing/ControllerDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function dispatch(Route $route, $controller, $method)
return $controller->callAction($method, $parameters);
}

return call_user_func_array([$controller, $method], $parameters);
return $controller->{$method}(...array_values($parameters));
}

/**
Expand Down
8 changes: 7 additions & 1 deletion src/Illuminate/Routing/ResourceRegistrar.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,13 @@ protected function getResourceAction($resource, $controller, $method, $options)
{
$name = $this->getResourceName($resource, $method, $options);

return ['as' => $name, 'uses' => $controller.'@'.$method];
$action = ['as' => $name, 'uses' => $controller.'@'.$method];

if (isset($options['middleware'])) {
$action['middleware'] = $options['middleware'];
}

return $action;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/Illuminate/Routing/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,9 @@ public function __construct($methods, $uri, $action)
/**
* Run the route action and return the response.
*
* @param \Illuminate\Http\Request $request
* @return mixed
*/
public function run(Request $request)
public function run()
{
$this->container = $this->container ?: new Container;

Expand Down
154 changes: 154 additions & 0 deletions src/Illuminate/Routing/RouteRegistrar.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
<?php

namespace Illuminate\Routing;

use Closure;

class RouteRegistrar
{
/**
* The router instance.
*
* @var \Illuminate\Routing\Router
*/
protected $router;

/**
* The attributes to pass on to the router.
*
* @var array
*/
protected $attributes = [];

/**
* The methods to dynamically pass through to the router.
*
* @var array
*/
protected $passthru = [
'get', 'post', 'put', 'patch', 'delete', 'options', 'any',
];

/**
* The attributes that are aliased.
*
* @var array
*/
protected $aliases = [
'name' => 'as',
];

/**
* Create a new route registrar instance.
*
* @param \Illuminate\Routing\Router $router
* @return void
*/
public function __construct(Router $router)
{
$this->router = $router;
}

/**
* Set the value for a given attribute.
*
* @param string $key
* @param mixed $value
* @return $this
*/
public function attribute($key, $value)
{
$this->attributes[array_get($this->aliases, $key, $key)] = $value;

return $this;
}

/**
* Route a resource to a controller.
*
* @param string $name
* @param string $controller
* @param array $options
* @return void
*/
public function resource($name, $controller, array $options = [])
{
$this->router->resource($name, $controller, $this->attributes + $options);
}

/**
* Create a route group with shared attributes.
*
* @param \Closure $callback
* @return void
*/
public function group($callback)
{
$this->router->group($this->attributes, $callback);
}

/**
* Register a new route with the given verbs.
*
* @param array|string $methods
* @param string $uri
* @param \Closure|array|string|null $action
* @return \Illuminate\Routing\Route
*/
public function match($methods, $uri, $action = null)
{
return $this->router->match($methods, $uri, $this->compileAction($action));
}

/**
* Register a new route with the router.
*
* @param string $method
* @param string $uri
* @param \Closure|array|string|null $action
* @return \Illuminate\Routing\Route
*/
protected function registerRoute($method, $uri, $action = null)
{
if (! is_array($action)) {
$action = array_merge($this->attributes, $action ? ['uses' => $action] : []);
}

return $this->router->{$method}($uri, $this->compileAction($action));
}

/**
* Compile the action into an array including the attributes.
*
* @param \Closure|array|string|null $action
* @return array
*/
protected function compileAction($action)
{
if (is_null($action)) {
return $this->attributes;
}

if (is_string($action) || $action instanceof Closure) {
$action = ['uses' => $action];
}

return array_merge($this->attributes, $action);
}

/**
* Dynamically handle calls into the route registrar.
*
* @param string $method
* @param array $parameters
* @return \Illuminate\Routing\Route|$this
*/
public function __call($method, $parameters)
{
if (in_array($method, $this->passthru)) {
return $this->registerRoute($method, ...$parameters);
}

return $this->attribute($method, $parameters[0]);
}
}
24 changes: 21 additions & 3 deletions src/Illuminate/Routing/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@

class Router implements RegistrarContract
{
use Macroable;
use Macroable {
__call as macroCall;
}

/**
* The event dispatcher instance.
Expand Down Expand Up @@ -639,7 +641,7 @@ protected function runRouteWithinStack(Route $route, Request $request)
->through($middleware)
->then(function ($request) use ($route) {
return $this->prepareResponse(
$request, $route->run($request)
$request, $route->run()
);
});
}
Expand Down Expand Up @@ -862,7 +864,7 @@ public function getMiddleware()
* @param string $class
* @return $this
*/
public function middleware($name, $class)
public function middlewareAlias($name, $class)
{
$this->middleware[$name] = $class;

Expand Down Expand Up @@ -1228,4 +1230,20 @@ public function getPatterns()
{
return $this->patterns;
}

/**
* Dynamically handle calls into the router instance.
*
* @param string $method
* @param array $parameters
* @return mixed
*/
public function __call($method, $parameters)
{
if (static::hasMacro($method)) {
return $this->macroCall($method, $parameters);
}

return (new RouteRegistrar($this))->attribute($method, $parameters[0]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be done like here

Copy link
Contributor Author

@JosephSilber JosephSilber Jul 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rkgrep why???

The resource registrar has a lot of logic as to how it registers everything, so in some cases it makes sense to want to override it.

This route registrar serves the singular purpose of collecting route configuration before the route is actually created. Doesn't make much sense to override it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, perhaps I made a hasty decision on that )

}
}
2 changes: 1 addition & 1 deletion tests/Auth/AuthorizesResourcesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected function assertHasMiddleware($controller, $method, $middleware)
{
$router = new Router(new Illuminate\Events\Dispatcher);

$router->middleware('can', 'AuthorizesResourcesMiddleware');
$router->middlewareAlias('can', 'AuthorizesResourcesMiddleware');
$router->get($method)->uses('AuthorizesResourcesController@'.$method);

$this->assertEquals(
Expand Down
Loading