Skip to content
Merged
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
16 changes: 3 additions & 13 deletions Slim/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
Expand Down
7 changes: 7 additions & 0 deletions Slim/Interfaces/RouterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
21 changes: 0 additions & 21 deletions Slim/Routable.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/
namespace Slim;

use Interop\Container\ContainerInterface;
use Slim\Interfaces\CallableResolverInterface;

/**
Expand All @@ -31,13 +30,6 @@ abstract class Routable
*/
protected $callableResolver;

/**
* Container
*
* @var ContainerInterface
*/
protected $container;

/**
* Route middleware
*
Expand Down Expand Up @@ -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
*
Expand Down
30 changes: 27 additions & 3 deletions Slim/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -65,6 +63,11 @@ class Route extends Routable implements RouteInterface
*/
protected $outputBuffering = 'append';

/**
* @var \Slim\Interfaces\InvocationStrategyInterface
*/
protected $routeInvocationStrategy;

/**
* Route parameters
*
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 28 additions & 18 deletions Slim/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,13 +31,6 @@
*/
class Router implements RouterInterface
{
/**
* Container Interface
*
* @var ContainerInterface
*/
protected $container;

/**
* Parser
*
Expand All @@ -52,6 +45,11 @@ class Router implements RouterInterface
*/
protected $callableResolver;

/**
* @var \Slim\Interfaces\InvocationStrategyInterface
*/
protected $routeInvocationStrategy;

/**
* Base path used in pathFor()
*
Expand Down Expand Up @@ -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()
*
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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)`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR is for Slim4 so BC is implied 😉

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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
2 changes: 2 additions & 0 deletions tests/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
});
Expand Down Expand Up @@ -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);
});
Expand Down
9 changes: 4 additions & 5 deletions tests/RouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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+'));
Expand Down Expand Up @@ -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.

Expand Down
1 change: 0 additions & 1 deletion tests/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/
namespace Slim\Tests;

use Slim\Container;
use Slim\Router;

class RouterTest extends \PHPUnit_Framework_TestCase
Expand Down