Skip to content

Commit

Permalink
Shift approach to use middleware names instead of scope paths.
Browse files Browse the repository at this point in the history 
Convert the implementation to attach middleware names to routes. This
allows scopes to be isolated and the builder to function as a stateful
builder and not a proxy for state in the route collection.

This also allows scopes to be isolated and still inherit middleware from
their containing scope.
  • Loading branch information
@markstory
markstory committed Jul 16, 2017
1 parent 05fd40e commit bd2973a
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 171 deletions.
13 changes: 8 additions & 5 deletions src/Routing/Middleware/RoutingMiddleware.php
View file
Expand Up @@ -82,15 +82,18 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
try {
Router::setRequestContext($request);
$params = (array)$request->getAttribute('params', []);
$middleware = [];
if (empty($params['controller'])) {
$parsedBody = $request->getParsedBody();
if (is_array($parsedBody) && isset($parsedBody['_method'])) {
$request = $request->withMethod($parsedBody['_method']);
}
$request = $request->withAttribute(
'params',
Router::parseRequest($request)
);
$params = Router::parseRequest($request);
if (isset($params['_middleware'])) {
$middleware = $params['_middleware'];
unset($params['_middleware']);
}
$request = $request->withAttribute('params', $params);
}
} catch (RedirectException $e) {
return new RedirectResponse(
Expand All @@ -99,7 +102,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
$response->getHeaders()
);
}
$matching = Router::getRouteCollection()->getMatchingMiddleware($request->getUri()->getPath());
$matching = Router::getRouteCollection()->getMiddleware($middleware);
if (!$matching) {
return $next($request, $response);
}
Expand Down
53 changes: 25 additions & 28 deletions src/Routing/RouteCollection.php
View file
Expand Up @@ -483,6 +483,17 @@ public function hasMiddleware($name)
return isset($this->_middleware[$name]);
}

/**
* Check if the named middleware or middleware group has been registered.
*
* @param string $name The name of the middleware to check.
* @return bool
*/
public function middlewareExists($name)
{
return $this->hasMiddleware($name) || $this->hasMiddlewareGroup($name);
}

/**
* Apply a registered middleware(s) for the provided path
*
Expand Down Expand Up @@ -511,41 +522,27 @@ public function applyMiddleware($path, array $middleware)
}

/**
* Get an array of middleware that matches the provided URL.
*
* Only the middleware applied to the longest matching scope will be used.
* Get an array of middleware given a list of names
*
* @param string $needle The URL path to find middleware for.
* @param array $names The names of the middleware to fetch
* @return array
* @throws \RuntimeException when a requested middleware does not exist.
*/
public function getMatchingMiddleware($needle)
public function getMiddleware(array $names)
{
$paths = $this->_middlewarePaths;
$keys = array_map('strlen', array_keys($paths));
array_multisort($keys, SORT_DESC, $paths);

foreach ($paths as $pattern => &$middleware) {
$matching = [];

foreach ($middleware as $key => $name) {
if ($this->hasMiddlewareGroup($name)) {
unset($middleware[$key]);
$middleware = array_merge($middleware, $this->_middlewareGroups[$name]);
}
$out = [];
foreach ($names as $name) {
if ($this->hasMiddlewareGroup($name)) {
$out = array_merge($out, $this->getMiddleware($this->_middlewareGroups[$name]));
continue;
}

if (preg_match($pattern, $needle)) {
$matching = array_merge($matching, $middleware);
$resolved = [];

foreach ($matching as $name) {
$resolved[] = $this->_middleware[$name];
}

return array_values($resolved);
if (!$this->hasMiddleware($name)) {
$message = "The middleware named '$name' has not been registered. Use registerMiddleware() to define it.";
throw new RuntimeException($message);
}
$out[] = $this->_middleware[$name];
}

return [];
return $out;
}
}
69 changes: 65 additions & 4 deletions tests/TestCase/Routing/Middleware/RoutingMiddlewareTest.php
View file
Expand Up @@ -210,7 +210,6 @@ public function testFakedRequestMethodParsed()
*/
public function testInvokeScopedMiddleware()
{
$this->counter = 0;
Router::scope('/api', function ($routes) {
$routes->registerMiddleware('first', function ($req, $res, $next) {
$this->log[] = 'first';
Expand All @@ -222,9 +221,10 @@ public function testInvokeScopedMiddleware()

return $next($req, $res);
});
$routes->connect('/ping', ['controller' => 'Pings']);
// Connect middleware in reverse to test ordering.
$routes->applyMiddleware('second', 'first');

$routes->connect('/ping', ['controller' => 'Pings']);
});

$request = ServerRequestFactory::fromGlobals([
Expand All @@ -250,7 +250,6 @@ public function testInvokeScopedMiddleware()
*/
public function testInvokeScopedMiddlewareReturnResponse()
{
$this->counter = 0;
Router::scope('/', function ($routes) {
$routes->registerMiddleware('first', function ($req, $res, $next) {
$this->log[] = 'first';
Expand Down Expand Up @@ -294,7 +293,6 @@ public function testInvokeScopedMiddlewareReturnResponse()
*/
public function testInvokeScopedMiddlewareReturnResponseMainScope()
{
$this->counter = 0;
Router::scope('/', function ($routes) {
$routes->registerMiddleware('first', function ($req, $res, $next) {
$this->log[] = 'first';
Expand Down Expand Up @@ -330,4 +328,67 @@ public function testInvokeScopedMiddlewareReturnResponseMainScope()
$this->assertSame($response, $result, 'Should return result');
$this->assertSame(['first'], $this->log);
}

/**
* Test invoking middleware & scope separation
*
* Re-opening a scope should not inherit middleware declared
* in the first context.
*
* @dataProvider scopedMiddlewareUrlProvider
* @return void
*/
public function testInvokeScopedMiddlewareIsolatedScopes($url, $expected)
{
Router::scope('/', function ($routes) {
$routes->registerMiddleware('first', function ($req, $res, $next) {
$this->log[] = 'first';

return $next($req, $res);
});
$routes->registerMiddleware('second', function ($req, $res, $next) {
$this->log[] = 'second';

return $next($req, $res);
});

$routes->scope('/api', function ($routes) {
$routes->applyMiddleware('first');
$routes->connect('/ping', ['controller' => 'Pings']);
});

$routes->scope('/api', function ($routes) {
$routes->applyMiddleware('second');
$routes->connect('/version', ['controller' => 'Version']);
});
});

$request = ServerRequestFactory::fromGlobals([
'REQUEST_METHOD' => 'GET',
'REQUEST_URI' => $url
]);
$response = new Response();
$next = function ($req, $res) {
$this->log[] = 'last';

return $res;
};
$middleware = new RoutingMiddleware();
$result = $middleware($request, $response, $next);
$this->assertSame($response, $result, 'Should return result');
$this->assertSame($expected, $this->log);
}

/**
* Provider for scope isolation test.
*
* @return array
*/
public function scopedMiddlewareUrlProvider()
{
return [
['/api/ping', ['first', 'last']],
['/api/version', ['second', 'last']],
];
}
}
134 changes: 0 additions & 134 deletions tests/TestCase/Routing/RouteCollectionTest.php
View file
Expand Up @@ -718,138 +718,4 @@ public function testMiddlewareGroupUnregisteredMiddleware()
{
$this->collection->middlewareGroup('group', ['bad']);
}

/**
* Test adding middleware with a placeholder in the path.
*
* @return void
*/
public function testApplyMiddlewareBasic()
{
$mock = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$this->collection->registerMiddleware('callable', $mock);
$this->collection->registerMiddleware('callback_two', $mock);

$result = $this->collection->applyMiddleware('/api', ['callable', 'callback_two']);
$this->assertSame($result, $this->collection);
}

/**
* Test adding middleware with a placeholder in the path.
*
* @return void
*/
public function testGetMatchingMiddlewareBasic()
{
$mock = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$this->collection->registerMiddleware('callable', $mock);
$this->collection->registerMiddleware('callback_two', $mock);

$result = $this->collection->applyMiddleware('/api', ['callable']);
$middleware = $this->collection->getMatchingMiddleware('/api/v1/articles');
$this->assertCount(1, $middleware);
$this->assertSame($middleware[0], $mock);
}

/**
* Test enabling and matching
*
* @return void
*/
public function testGetMatchingMiddlewareMultiplePaths()
{
$mock = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$mockTwo = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$this->collection->registerMiddleware('callable', $mock);
$this->collection->registerMiddleware('callback_two', $mockTwo);

$this->collection->applyMiddleware('/api', ['callable']);
$this->collection->applyMiddleware('/api/v1/articles', ['callback_two']);

$middleware = $this->collection->getMatchingMiddleware('/articles');
$this->assertCount(0, $middleware);

$middleware = $this->collection->getMatchingMiddleware('/api/v1/articles/1');
$this->assertCount(1, $middleware);
$this->assertEquals([$mock], $middleware, 'Should only match the strongest scope');

$middleware = $this->collection->getMatchingMiddleware('/api/v1/comments');
$this->assertCount(1, $middleware);
$this->assertEquals([$mock], $middleware, 'Should not match /articles middleware');
}

/**
* Test enabling and matching
*
* @return void
*/
public function testGetMatchingMiddlewareDeduplicate()
{
$mock = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$mockTwo = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$this->collection->registerMiddleware('callable', $mock);
$this->collection->registerMiddleware('callback_two', $mockTwo);

$this->collection->applyMiddleware('/api', ['callable']);
$this->collection->applyMiddleware('/api/v1/articles', ['callback_two', 'callable']);

$middleware = $this->collection->getMatchingMiddleware('/api/v1/articles/1');
$this->assertCount(2, $middleware);
$this->assertEquals([$mock, $mockTwo], $middleware, 'Both middleware match');
}

/**
* Test adding middleware with a placeholder in the path.
*
* @return void
*/
public function testApplyMiddlewareWithPlaceholder()
{
$mock = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$this->collection->registerMiddleware('callable', $mock);

$this->collection->applyMiddleware('/api-:version/articles/:article_id/comments', ['callable']);

$middleware = $this->collection->getMatchingMiddleware('/api-1/articles/comments');
$this->assertEmpty($middleware);

$middleware = $this->collection->getMatchingMiddleware('/api-1/articles/yay/comments');
$this->assertEquals([$mock], $middleware);

$middleware = $this->collection->getMatchingMiddleware('/api-1/articles/123/comments');
$this->assertEquals([$mock], $middleware);

$middleware = $this->collection->getMatchingMiddleware('/api-abc123/articles/abc-123/comments/99');
$this->assertEquals([$mock], $middleware);
}

/**
* Test applying middleware to a scope when it doesn't exist
*
* @expectedException \RuntimeException
* @expectedExceptionMessage Cannot apply 'bad' middleware or middleware group to path '/api'. It has not been registered.
* @return void
*/
public function testApplyMiddlewareUnregistered()
{
$mock = $this->getMockBuilder('\stdClass')
->setMethods(['__invoke'])
->getMock();
$this->collection->registerMiddleware('callable', $mock);
$this->collection->applyMiddleware('/api', ['callable', 'bad']);
}
}

0 comments on commit bd2973a

Please sign in to comment.