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

Refactor callable resolver, router, route, and route group #2098

Merged
merged 13 commits into from Dec 11, 2016
@@ -9,6 +9,7 @@
namespace Slim;
use Exception;
use Slim\Interfaces\CallableResolverInterface;
use Throwable;
use Closure;
use InvalidArgumentException;
@@ -52,6 +53,16 @@ class App
*/
private $container;
/**
* @var \Slim\Interfaces\CallableResolverInterface
*/
protected $callableResolver;
/**
* @var \Slim\Interfaces\RouterInterface
*/
protected $router;
/********************************************************************************
* Constructor
*******************************************************************************/
@@ -94,7 +105,9 @@ public function getContainer()
*/
public function add($callable)
{
return $this->addMiddleware(new DeferredCallable($callable, $this->container));
return $this->addMiddleware(
new DeferredCallable($callable, $this->getCallableResolver())
);
}
/**
@@ -117,6 +130,76 @@ public function __call($method, $args)
throw new \BadMethodCallException("Method $method is not a valid method");
}
/********************************************************************************
* Setter and getter methods
*******************************************************************************/
/**
* Set callable resolver
*
* @param CallableResolverInterface $resolver
*/
public function setCallableResolver(CallableResolverInterface $resolver)
{
$this->callableResolver = $resolver;
}
/**
* Get callable resolver
*
* @return CallableResolver|null
*/
public function getCallableResolver()
{
if (! $this->callableResolver instanceof CallableResolverInterface) {
$this->callableResolver = new CallableResolver($this->container);
}
return $this->callableResolver;
}
/**
* Set router
*
* @param RouterInterface $router
*/
public function setRouter(RouterInterface $router)
{
$this->router = $router;
}
/**
* Get router
*
* @return RouterInterface
*/
public function getRouter()
{
if (! $this->router instanceof RouterInterface) {
$router = null;
// Fetch router from container if container is set
if ($this->container instanceof ContainerInterface) {

This comment has been minimized.

Copy link
@akrabat

akrabat Dec 10, 2016

Member

Same here, I think?

This comment has been minimized.

Copy link
@codeguy

codeguy Dec 10, 2016

Author Member

Just pushed a new commit. Luckily, this brought several affected unit tests to my attention.

$router = $this->container->get('router');
}
// If router is invalid, use default
if (! $router instanceof RouterInterface) {
$router = new Router();
if ($this->container instanceof ContainerInterface) {
$router->setContainer($this->container);
}
if ($this->callableResolver instanceof CallableResolverInterface) {
$router->setCallableResolver($this->callableResolver);
}
}
$this->router = $router;
}
return $this->router;
}
/********************************************************************************
* Router proxy methods
*******************************************************************************/
@@ -223,19 +306,22 @@ public function any($pattern, $callable)
*/
public function map(array $methods, $pattern, $callable)
{
if ($callable instanceof Closure) {
// TODO: Should we bind route callable to container elsewhere?
if ($this->container instanceof ContainerInterface && $callable instanceof \Closure) {
$callable = $callable->bindTo($this->container);
}
$route = $this->container->get('router')->map($methods, $pattern, $callable);
if (is_callable([$route, 'setContainer'])) {
$route->setContainer($this->container);
}
$router = $this->getRouter();
$route = $router->map($methods, $pattern, $callable);
// TODO: Set route output buffering without a container
// TODO: Refactor app settings out of container
if (is_callable([$route, 'setOutputBuffering'])) {
$route->setOutputBuffering($this->container->get('settings')['outputBuffering']);
}
// TODO: Route callable binding and output buffering should be handled within router
return $route;
}
@@ -254,10 +340,18 @@ public function map(array $methods, $pattern, $callable)
public function group($pattern, $callable)
{
/** @var RouteGroup $group */
$group = $this->container->get('router')->pushGroup($pattern, $callable);
$group->setContainer($this->container);
$router = $this->getRouter();
$group = $router->pushGroup($pattern, $callable);
// TODO: Set group container and resolver inside Router
if ($this->container instanceof ContainerInterface) {
$group->setContainer($this->container);
}
if ($this->callableResolver instanceof CallableResolverInterface) {
$group->setCallableResolver($this->callableResolver);
}
$group($this);
$this->container->get('router')->popGroup();
$router->popGroup();
return $group;
}
@@ -308,10 +402,10 @@ public function run($silent = false)
*/
public function process(ServerRequestInterface $request, ResponseInterface $response)
{
// Ensure basePath is set
$router = $this->container->get('router');
$router = $this->getRouter();
// Dispatch the Router first if the setting for this is on
// TODO: Refactor settings out of container
if ($this->container->get('settings')['determineRouteBeforeAppMiddleware'] === true) {
// Dispatch router (note: you won't be able to alter routes after this)
$request = $this->dispatchRouterAndPrepareRoute($request, $router);
@@ -362,6 +456,7 @@ public function respond(ResponseInterface $response)
if ($body->isSeekable()) {
$body->rewind();
}
// TODO: Refactor settings out of container
$settings = $this->container->get('settings');
$chunkSize = $settings['responseChunkSize'];
@@ -415,7 +510,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
$routeInfo = $request->getAttribute('routeInfo');
/** @var \Slim\Interfaces\RouterInterface $router */
$router = $this->container->get('router');
$router = $this->getRouter();
// If router hasn't been dispatched or the URI changed then dispatch
if (null === $routeInfo || ($routeInfo['request'] !== [$request->getMethod(), (string) $request->getUri()])) {
@@ -487,6 +582,7 @@ protected function finalize(ResponseInterface $response)
return $response->withoutHeader('Content-Type')->withoutHeader('Content-Length');
}
// TODO: Refactor settings out of container
// Add Content-Length header if `addContentLengthHeader` setting is set
if (isset($this->container->get('settings')['addContentLengthHeader']) &&
$this->container->get('settings')['addContentLengthHeader'] == true) {
@@ -87,6 +87,10 @@ public function resolve($toResolve)
));
}
if ($this->container instanceof ContainerInterface && $resolved instanceof \Closure) {
$resolved = $resolved->bindTo($this->container);
}
return $resolved;
}
}

This file was deleted.

@@ -97,6 +97,9 @@ public function register($container)
if (method_exists($router, 'setContainer')) {
$router->setContainer($container);
}
if (method_exists($router, 'setCallableResolver')) {
$router->setCallableResolver($container['callableResolver']);
}
return $router;
};
@@ -9,35 +9,38 @@
namespace Slim;
use Closure;
use Interop\Container\ContainerInterface;
use Slim\Interfaces\CallableResolverInterface;
class DeferredCallable
{
use CallableResolverAwareTrait;
/**
* @var callable|string
*/
protected $callable;
private $callable;
/** @var ContainerInterface */
private $container;
/**
* @var CallableResolverInterface|null
*/
protected $callableResolver;
/**
* DeferredMiddleware constructor.
*
* @param callable|string $callable
* @param ContainerInterface $container
* @param CallableResolverInterface|null $resolver
*/
public function __construct($callable, ContainerInterface $container = null)
public function __construct($callable, CallableResolverInterface $resolver = null)
{
$this->callable = $callable;
$this->container = $container;
$this->callableResolver = $resolver;
}
public function __invoke()
{
$callable = $this->resolveCallable($this->callable);
if ($callable instanceof Closure) {
$callable = $callable->bindTo($this->container);
$callable = $this->callable;
if ($this->callableResolver) {
$callable = $this->callableResolver->resolve($callable);
}
$args = func_get_args();
return call_user_func_array($callable, $args);
@@ -9,6 +9,7 @@
namespace Slim;
use Interop\Container\ContainerInterface;
use Slim\Interfaces\CallableResolverInterface;
/**
* A routable, middleware-aware object
@@ -18,15 +19,18 @@
*/
abstract class Routable
{
use CallableResolverAwareTrait;
/**
* Route callable
*
* @var callable
*/
protected $callable;
/**
* @var \Slim\Interfaces\CallableResolverInterface
*/
protected $callableResolver;
/**
* Container
*
@@ -68,6 +72,26 @@ public function getPattern()
return $this->pattern;
}
/**
* Set callable resolver
*
* @param CallableResolverInterface $resolver
*/
public function setCallableResolver(CallableResolverInterface $resolver)
{
$this->callableResolver = $resolver;
}
/**
* Get callable resolver
*
* @return CallableResolverInterface|null
*/
public function getCallableResolver()
{
return $this->callableResolver;
}
/**
* Set container for use with resolveCallable
*
@@ -90,7 +114,7 @@ public function setContainer(ContainerInterface $container)
*/
public function add($callable)
{
$this->middleware[] = new DeferredCallable($callable, $this->container);
$this->middleware[] = new DeferredCallable($callable, $this->callableResolver);
return $this;
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.