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 container out of router, route, route group #2102

Merged
merged 4 commits into from Dec 20, 2016
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -177,10 +177,6 @@ public function getRouter()
{
if (! $this->router instanceof RouterInterface) {
$router = new Router();
$container = $this->getContainer();
if ($container instanceof ContainerInterface) {
$router->setContainer($container);
}
$resolver = $this->getCallableResolver();
if ($resolver instanceof CallableResolverInterface) {
$router->setCallableResolver($resolver);
@@ -304,22 +300,20 @@ public function any($pattern, $callable)
*/
public function map(array $methods, $pattern, $callable)
{
// TODO: Should we bind route callable to container elsewhere?
// Bind route callable to container, if present
if ($this->container instanceof ContainerInterface && $callable instanceof \Closure) {
$callable = $callable->bindTo($this->container);
}
$router = $this->getRouter();
$route = $router->map($methods, $pattern, $callable);
// Create route
$route = $this->getRouter()->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;
}
@@ -340,10 +334,6 @@ public function group($pattern, $callable)
/** @var RouteGroup $group */
$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);
}
@@ -104,4 +104,11 @@ public function relativePathFor($name, array $data = [], array $queryParams = []
* @throws InvalidArgumentException If required data not provided
*/
public function pathFor($name, array $data = [], array $queryParams = []);
/**
* Set default route invocation strategy
*
* @param InvocationStrategyInterface $strategy
*/
public function setDefaultInvocationStrategy(InvocationStrategyInterface $strategy);
}
@@ -8,7 +8,6 @@
*/
namespace Slim;
use Interop\Container\ContainerInterface;
use Slim\Interfaces\CallableResolverInterface;
/**
@@ -31,13 +30,6 @@ abstract class Routable
*/
protected $callableResolver;
/**
* Container
*
* @var ContainerInterface
*/
protected $container;
/**
* Route middleware
*
@@ -92,19 +84,6 @@ public function getCallableResolver()
return $this->callableResolver;
}
/**
* Set container for use with resolveCallable
*
* @param ContainerInterface $container
*
* @return self
*/
public function setContainer(ContainerInterface $container)
{
$this->container = $container;
return $this;
}
/**
* Prepend middleware to the middleware collection
*
@@ -9,12 +9,10 @@
namespace Slim;
use Exception;
use Slim\Interfaces\CallableResolverInterface;
use Throwable;
use InvalidArgumentException;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;
use Slim\Exception\SlimException;
use Slim\Handlers\Strategies\RequestResponse;
use Slim\Interfaces\InvocationStrategyInterface;
use Slim\Interfaces\RouteInterface;
@@ -65,6 +63,11 @@ class Route extends Routable implements RouteInterface
*/
protected $outputBuffering = 'append';
/**
* @var \Slim\Interfaces\InvocationStrategyInterface
*/
protected $routeInvocationStrategy;
/**
* Route parameters
*
@@ -88,6 +91,27 @@ public function __construct($methods, $pattern, $callable, $groups = [], $identi
$this->callable = $callable;
$this->groups = $groups;
$this->identifier = 'route' . $identifier;
$this->routeInvocationStrategy = new RequestResponse();
}
/**
* Set route invocation strategy
*
* @param InvocationStrategyInterface $strategy
*/
public function setInvocationStrategy(InvocationStrategyInterface $strategy)
{
$this->routeInvocationStrategy = $strategy;
}
/**
* Get route invocation strategy
*
* @return InvocationStrategyInterface
*/
public function getInvocationStrategy()
{
return $this->routeInvocationStrategy;
}
/**
@@ -331,7 +355,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
}
/** @var InvocationStrategyInterface $handler */
$handler = isset($this->container) ? $this->container->get('foundHandler') : new RequestResponse();
$handler = $this->routeInvocationStrategy;
// invoke route callable
if ($this->outputBuffering === false) {
@@ -9,14 +9,14 @@
namespace Slim;
use FastRoute\Dispatcher;
use Interop\Container\ContainerInterface;
use InvalidArgumentException;
use RuntimeException;
use Psr\Http\Message\ServerRequestInterface;
use FastRoute\RouteCollector;
use FastRoute\RouteParser;
use FastRoute\RouteParser\Std as StdParser;
use Slim\Interfaces\CallableResolverInterface;
use Slim\Interfaces\InvocationStrategyInterface;
use Slim\Interfaces\RouteGroupInterface;
use Slim\Interfaces\RouterInterface;
use Slim\Interfaces\RouteInterface;
@@ -31,13 +31,6 @@
*/
class Router implements RouterInterface
{
/**
* Container Interface
*
* @var ContainerInterface
*/
protected $container;
/**
* Parser
*
@@ -52,6 +45,11 @@ class Router implements RouterInterface
*/
protected $callableResolver;
/**
* @var \Slim\Interfaces\InvocationStrategyInterface
*/
protected $routeInvocationStrategy;
/**
* Base path used in pathFor()
*
@@ -101,6 +99,26 @@ public function __construct(RouteParser $parser = null)
$this->routeParser = $parser ?: new StdParser;
}
/**
* Set default route invocation strategy
*
* @param InvocationStrategyInterface $strategy
*/
public function setDefaultInvocationStrategy(InvocationStrategyInterface $strategy)
{
$this->routeInvocationStrategy = $strategy;
}
/**
* Get default route invocation strategy
*
* @return InvocationStrategyInterface|null
*/
public function getDefaultInvocationStrategy()
{
return $this->routeInvocationStrategy;
}
/**
* Set the base path used in pathFor()
*
@@ -152,14 +170,6 @@ public function setCallableResolver(CallableResolverInterface $resolver)
$this->callableResolver = $resolver;
}
/**
* @param ContainerInterface $container
*/
public function setContainer(ContainerInterface $container)
{
$this->container = $container;
}
/**
* Add route
*
@@ -227,8 +237,8 @@ protected function createRoute($methods, $pattern, $callable)
if ($this->callableResolver) {
$route->setCallableResolver($this->callableResolver);
}
if ($this->container) {
$route->setContainer($this->container);
if ($this->routeInvocationStrategy) {
$route->setInvocationStrategy($this->routeInvocationStrategy);
}
return $route;
@@ -1,5 +1,7 @@
# How to upgrade

* [2098] - You need to add the App's router to the container for a straight upgrade. If you've created your own router factory in the container though, then you need to set it into the $app.
* [2102] - You must inject custom route invocation strategy with `$app->getRouter()->setDefaultInvocationStrategy($myStrategy)`

This comment has been minimized.

Copy link
@crocodile2u

crocodile2u Dec 21, 2016

Because introduction of a new method to the interface (setDefaultInvocationStrategy) is a BC break for user implementations, it has to be mentioned in the docs as well.

This comment has been minimized.

Copy link
@danopz

danopz Dec 21, 2016

Member

This PR is for Slim4 so BC is implied 😉

This comment has been minimized.

Copy link
@akrabat

akrabat Dec 21, 2016

Member

Yes. UPGRADING.md will be used to create the docs for how to upgrade from 3 to 4.

This comment has been minimized.

Copy link
@crocodile2u

crocodile2u Dec 21, 2016

So it's a proper time to add this info to UPGRADING.md. Smth like "If you're using a custom router implementing Slim\Interfaces\RouterInterface, please note that you will have to implement a new method - setDefaultInvocationStrategy()"


[2098]: 92612999931799571752400592
[2098]: https://github.com/slimphp/Slim/pull/2098
[2102]: https://github.com/slimphp/Slim/pull/2102
@@ -1119,6 +1119,7 @@ public function testInvokeWithMatchingRouteWithNamedParameterRequestResponseArgS
};
$app = new App($c);
$app->getRouter()->setDefaultInvocationStrategy($c['foundHandler']);
$app->get('/foo/{name}', function ($req, $res, $name) {
return $res->write("Hello {$name}");
});
@@ -1380,6 +1381,7 @@ public function testCurrentRequestAttributesAreNotLostWhenAddingRouteArgumentsRe
};
$app = new App($c);
$app->getRouter()->setDefaultInvocationStrategy($c['foundHandler']);
$app->get('/foo/{name}', function ($req, $res, $name) {
return $res->write($req->getAttribute('one') . $name);
});
@@ -192,8 +192,8 @@ public function testAddMiddlewareAsString()
$container = new Container();
$container['MiddlewareStub'] = new MiddlewareStub();
$route->setContainer($container);
$resolver = new CallableResolver($container);
$route->setCallableResolver($resolver);
$route->add('MiddlewareStub:run');
$env = Environment::mock();
@@ -223,7 +223,6 @@ public function testControllerInContainer()
$deferred = new DeferredCallable('CallableTest:toCall', $resolver);
$route = new Route(['GET'], '/', $deferred);
$route->setContainer($container);
$route->setCallableResolver($resolver);
$uri = Uri::createFromString('https://example.com:80');
@@ -410,8 +409,8 @@ public function testInvokeDeferredCallable()
};
$route = new Route(['GET'], '/', 'CallableTest:toCall');
$route->setContainer($container);
$route->setCallableResolver($resolver);
$route->setInvocationStrategy($container['foundHandler']);
$uri = Uri::createFromString('https://example.com:80');
$body = new Body(fopen('php://temp', 'r+'));
@@ -446,8 +445,8 @@ public function testChangingCallable()
};
$route = new Route(['GET'], '/', 'CallableTest:toCall'); //Note that this doesn't actually exist
$route->setContainer($container);
$route->setCallableResolver($resolver);
$route->setInvocationStrategy($container['foundHandler']);
$route->setCallable('CallableTest2:toCall'); //Then we fix it here.
@@ -8,7 +8,6 @@
*/
namespace Slim\Tests;
use Slim\Container;
use Slim\Router;
class RouterTest extends \PHPUnit_Framework_TestCase
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.