Shift approach to use middleware names instead of scope paths.

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.

src/Routing/Middleware/RoutingMiddleware.php

@@ -82,15 +82,18 @@ class RoutingMiddleware
         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(
@@ -99,7 +102,7 @@ class RoutingMiddleware
                 $response->getHeaders()
             );
         }
-        $matching = Router::getRouteCollection()->getMatchingMiddleware($request->getUri()->getPath());
+        $matching = Router::getRouteCollection()->getMiddleware($middleware);
         if (!$matching) {
             return $next($request, $response);
         }

src/Routing/RouteCollection.php

@@ -484,6 +484,17 @@ class RouteCollection
     }
 
     /**
+     * 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
      *
      * @param string $path The URL path to register middleware for.
@@ -511,41 +522,27 @@ class RouteCollection
     }
 
     /**
-     * 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;
     }
 }

tests/TestCase/Routing/Middleware/RoutingMiddlewareTest.php

@@ -210,7 +210,6 @@ class RoutingMiddlewareTest extends TestCase
      */
     public function testInvokeScopedMiddleware()
     {
-        $this->counter = 0;
         Router::scope('/api', function ($routes) {
             $routes->registerMiddleware('first', function ($req, $res, $next) {
                 $this->log[] = 'first';
@@ -222,9 +221,10 @@ class RoutingMiddlewareTest extends TestCase
 
                 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([
@@ -250,7 +250,6 @@ class RoutingMiddlewareTest extends TestCase
      */
     public function testInvokeScopedMiddlewareReturnResponse()
     {
-        $this->counter = 0;
         Router::scope('/', function ($routes) {
             $routes->registerMiddleware('first', function ($req, $res, $next) {
                 $this->log[] = 'first';
@@ -294,7 +293,6 @@ class RoutingMiddlewareTest extends TestCase
      */
     public function testInvokeScopedMiddlewareReturnResponseMainScope()
     {
-        $this->counter = 0;
         Router::scope('/', function ($routes) {
             $routes->registerMiddleware('first', function ($req, $res, $next) {
                 $this->log[] = 'first';
@@ -330,4 +328,67 @@ class RoutingMiddlewareTest extends TestCase
         $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']],
+        ];
+    }
 }

tests/TestCase/Routing/RouteCollectionTest.php

@@ -718,138 +718,4 @@ class RouteCollectionTest extends TestCase
     {
         $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']);
-    }
 }