Have Application compose a RouteCollector instead of PathBasedRoutingMiddleware

This is a change proposed while updating the documentation for version 3.
Having two routing middleware is confusing, particularly since the
`PathBasedRoutingMiddleware` simply provides methods for modifying the
state of the the composed router.

zend-expressive-router 3.0.0rc5 updates to rename
`PathBasedRoutingMiddleware` to `Zend\Expressive\Router\RouteCollector`,
and to no longer extend `RouteMiddleware`. This allows it to do its one
job: create and return `Route` instances that are also injected into the
composed router.

This patch updates to that version, and modifies `Application` to
compose a `RouteCollector` instead of `PathBasedRoutingMiddleware`.

Author: weierophinney

composer.json

@@ -27,7 +27,7 @@
         "psr/container": "^1.0",
         "psr/http-message": "^1.0.1",
         "psr/http-server-middleware": "^1.0",
-        "zendframework/zend-expressive-router": "^3.0.0rc4",
+        "zendframework/zend-expressive-router": "^3.0.0rc5",
         "zendframework/zend-expressive-template": "^2.0.0alpha1",
         "zendframework/zend-httphandlerrunner": "^1.0.1",
         "zendframework/zend-stratigility": "^3.0.0rc1"

composer.lock

@@ -13,7 +13,7 @@
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
-use Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware;
+use Zend\Expressive\Router\RouteCollector;
 use Zend\HttpHandlerRunner\RequestHandlerRunner;
 use Zend\Stratigility\MiddlewarePipeInterface;
 
@@ -32,7 +32,7 @@ class Application implements MiddlewareInterface, RequestHandlerInterface
     private $pipeline;
 
     /**
-     * @var PathBasedRoutingMiddleware
+     * @var RouteCollector
      */
     private $routes;
 
@@ -44,7 +44,7 @@ class Application implements MiddlewareInterface, RequestHandlerInterface
     public function __construct(
         MiddlewareFactory $factory,
         MiddlewarePipeInterface $pipeline,
-        PathBasedRoutingMiddleware $routes,
+        RouteCollector $routes,
         RequestHandlerRunner $runner
     ) {
         $this->factory = $factory;

src/Application.php

@@ -40,7 +40,7 @@ public function getDependencies() : array
                 IMPLICIT_HEAD_MIDDLEWARE    => Router\Middleware\ImplicitHeadMiddleware::class,
                 IMPLICIT_OPTIONS_MIDDLEWARE => Router\Middleware\ImplicitOptionsMiddleware::class,
                 NOT_FOUND_MIDDLEWARE        => Handler\NotFoundHandler::class,
-                ROUTE_MIDDLEWARE            => Router\Middleware\PathBasedRoutingMiddleware::class,
+                ROUTE_MIDDLEWARE            => Router\Middleware\RouteMiddleware::class,
             ],
             'factories' => [
                 Application::class                       => Container\ApplicationFactory::class,

src/ConfigProvider.php

@@ -13,7 +13,7 @@
 use Zend\Expressive\Application;
 use Zend\Expressive\ApplicationPipeline;
 use Zend\Expressive\MiddlewareFactory;
-use Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware;
+use Zend\Expressive\Router\RouteCollector;
 use Zend\HttpHandlerRunner\RequestHandlerRunner;
 
 /**
@@ -25,7 +25,7 @@
  * - Zend\Expressive\MiddlewareFactory.
  * - Zend\Expressive\ApplicationPipeline, which should resolve to a
  *   Zend\Stratigility\MiddlewarePipeInterface instance.
- * - Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware.
+ * - Zend\Expressive\Router\RouteCollector.
  * - Zend\HttpHandler\RequestHandlerRunner.
  */
 class ApplicationFactory
@@ -35,7 +35,7 @@ public function __invoke(ContainerInterface $container) : Application
         return new Application(
             $container->get(MiddlewareFactory::class),
             $container->get(ApplicationPipeline::class),
-            $container->get(PathBasedRoutingMiddleware::class),
+            $container->get(RouteCollector::class),
             $container->get(RequestHandlerRunner::class)
         );
     }

src/Container/ApplicationFactory.php

@@ -19,8 +19,8 @@
 use TypeError;
 use Zend\Expressive\Application;
 use Zend\Expressive\MiddlewareFactory;
-use Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware as RouteMiddleware;
 use Zend\Expressive\Router\Route;
+use Zend\Expressive\Router\RouteCollector;
 use Zend\HttpHandlerRunner\RequestHandlerRunner;
 use Zend\Stratigility\Middleware\PathMiddlewareDecorator;
 use Zend\Stratigility\MiddlewarePipe;
@@ -32,7 +32,7 @@ public function setUp()
     {
         $this->factory = $this->prophesize(MiddlewareFactory::class);
         $this->pipeline = $this->prophesize(MiddlewarePipeInterface::class);
-        $this->routes = $this->prophesize(RouteMiddleware::class);
+        $this->routes = $this->prophesize(RouteCollector::class);
         $this->runner = $this->prophesize(RequestHandlerRunner::class);
 
         $this->app = new Application(
@@ -333,7 +333,7 @@ public function testAnyMethodPassesNullForMethodWhenAllArgumentsPresent($middlew
         $this->assertSame($route, $this->app->any('/foo', $middleware, 'foo'));
     }
 
-    public function testGetRoutesProxiesToRouteMiddleware()
+    public function testGetRoutesProxiesToRouteCollector()
     {
         $route = $this->prophesize(Route::class)->reveal();
         $this->routes->getRoutes()->willReturn([$route]);

test/ApplicationTest.php

@@ -25,8 +25,9 @@
 use Zend\Expressive\MiddlewareFactory;
 use Zend\Expressive\Router\Middleware\DispatchMiddleware;
 use Zend\Expressive\Router\Middleware\MethodNotAllowedMiddleware;
-use Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware;
+use Zend\Expressive\Router\Middleware\RouteMiddleware;
 use Zend\Expressive\Router\Route;
+use Zend\Expressive\Router\RouteCollector;
 use Zend\Expressive\Router\RouterInterface;
 use Zend\HttpHandlerRunner\RequestHandlerRunner;
 use Zend\Stratigility\MiddlewarePipe;
@@ -46,7 +47,10 @@ class ApplicationConfigInjectionDelegatorTest extends TestCase
     /** @var MethodNotAllowedMiddleware|ObjectProphecy */
     private $methodNotAllowedMiddleware;
 
-    /** @var PathBasedRoutingMiddleware */
+    /** @var RouteCollector */
+    private $routeCollector;
+
+    /** @var RouteMiddleware */
     private $routeMiddleware;
 
     /** @var RouterInterface|ObjectProphecy */
@@ -56,10 +60,8 @@ public function setUp()
     {
         $this->container = $this->mockContainerInterface();
         $this->router = $this->prophesize(RouterInterface::class);
-        $this->routeMiddleware = new PathBasedRoutingMiddleware(
-            $this->router->reveal(),
-            new Response()
-        );
+        $this->routeCollector = new RouteCollector($this->router->reveal());
+        $this->routeMiddleware = new RouteMiddleware($this->router->reveal());
         $this->dispatchMiddleware = $this->prophesize(DispatchMiddleware::class)->reveal();
         $this->methodNotAllowedMiddleware = $this->prophesize(MethodNotAllowedMiddleware::class)->reveal();
     }
@@ -73,7 +75,7 @@ public function createApplication()
         return new Application(
             $factory,
             $pipeline,
-            $this->routeMiddleware,
+            $this->routeCollector,
             $runner
         );
     }
@@ -139,63 +141,6 @@ public static function assertPipelineContainsInstanceOf($class, $pipeline, $mess
         Assert::assertThat($found, Assert::isTrue(), $message);
     }
 
-    public static function assertRouteMiddleware(MiddlewareInterface $middleware)
-    {
-        if ($middleware instanceof PathBasedRoutingMiddleware) {
-            Assert::assertInstanceOf(PathBasedRoutingMiddleware::class, $middleware);
-            return;
-        }
-
-        if (! $middleware instanceof Middleware\LazyLoadingMiddleware) {
-            Assert::fail('Middleware is not an instance of PathBasedRoutingMiddleware');
-        }
-
-        Assert::assertAttributeSame(
-            PathBasedRoutingMiddleware::class,
-            'middlewareName',
-            $middleware,
-            'Middleware is not an instance of PathBasedRoutingMiddleware'
-        );
-    }
-
-    public static function assertDispatchMiddleware(MiddlewareInterface $middleware)
-    {
-        if ($middleware instanceof DispatchMiddleware) {
-            Assert::assertInstanceOf(DispatchMiddleware::class, $middleware);
-            return;
-        }
-
-        if (! $middleware instanceof Middleware\LazyLoadingMiddleware) {
-            Assert::fail('Middleware is not an instance of DispatchMiddleware');
-        }
-
-        Assert::assertAttributeSame(
-            DispatchMiddleware::class,
-            'middlewareName',
-            $middleware,
-            'Middleware is not an instance of DispatchMiddleware'
-        );
-    }
-
-    public static function assertMethodNotAllowedMiddleware(MiddlewareInterface $middleware)
-    {
-        if ($middleware instanceof MethodNotAllowedMiddleware) {
-            Assert::assertInstanceOf(MethodNotAllowedMiddleware::class, $middleware);
-            return;
-        }
-
-        if (! $middleware instanceof Middleware\LazyLoadingMiddleware) {
-            Assert::fail('Middleware is not an instance of MethodNotAllowedMiddleware');
-        }
-
-        Assert::assertAttributeSame(
-            MethodNotAllowedMiddleware::class,
-            'middlewareName',
-            $middleware,
-            'Middleware is not an instance of MethodNotAllowedMiddleware'
-        );
-    }
-
     public function callableMiddlewares()
     {
         return [
@@ -416,16 +361,6 @@ public function testInjectRoutesFromConfigWillSkipSpecsThatOmitPath()
             ],
         ];
         $this->container->has('config')->willReturn(false);
-        $this->injectServiceInContainer(
-            $this->container,
-            PathBasedRoutingMiddleware::class,
-            $this->routeMiddleware
-        );
-        $this->injectServiceInContainer(
-            $this->container,
-            DispatchMiddleware::class,
-            $this->dispatchMiddleware
-        );
 
         $app = $this->createApplication();
 
@@ -447,16 +382,6 @@ public function testInjectRoutesFromConfigWillSkipSpecsThatOmitMiddleware()
             ],
         ];
         $this->container->has('config')->willReturn(false);
-        $this->injectServiceInContainer(
-            $this->container,
-            PathBasedRoutingMiddleware::class,
-            $this->routeMiddleware
-        );
-        $this->injectServiceInContainer(
-            $this->container,
-            DispatchMiddleware::class,
-            $this->dispatchMiddleware
-        );
 
         $app = $this->createApplication();
 

test/Container/ApplicationConfigInjectionDelegatorTest.php

@@ -15,7 +15,7 @@
 use Zend\Expressive\ApplicationPipeline;
 use Zend\Expressive\Container\ApplicationFactory;
 use Zend\Expressive\MiddlewareFactory;
-use Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware;
+use Zend\Expressive\Router\RouteCollector;
 use Zend\HttpHandlerRunner\RequestHandlerRunner;
 use Zend\Stratigility\MiddlewarePipeInterface;
 
@@ -25,13 +25,13 @@ public function testFactoryProducesAnApplication()
     {
         $middlewareFactory = $this->prophesize(MiddlewareFactory::class)->reveal();
         $pipeline = $this->prophesize(MiddlewarePipeInterface::class)->reveal();
-        $routeMiddleware = $this->prophesize(PathBasedRoutingMiddleware::class)->reveal();
+        $routeCollector = $this->prophesize(RouteCollector::class)->reveal();
         $runner = $this->prophesize(RequestHandlerRunner::class)->reveal();
 
         $container = $this->prophesize(ContainerInterface::class);
         $container->get(MiddlewareFactory::class)->willReturn($middlewareFactory);
         $container->get(ApplicationPipeline::class)->willReturn($pipeline);
-        $container->get(PathBasedRoutingMiddleware::class)->willReturn($routeMiddleware);
+        $container->get(RouteCollector::class)->willReturn($routeCollector);
         $container->get(RequestHandlerRunner::class)->willReturn($runner);
 
         $factory = new ApplicationFactory();
@@ -41,7 +41,7 @@ public function testFactoryProducesAnApplication()
         $this->assertInstanceOf(Application::class, $application);
         $this->assertAttributeSame($middlewareFactory, 'factory', $application);
         $this->assertAttributeSame($pipeline, 'pipeline', $application);
-        $this->assertAttributeSame($routeMiddleware, 'routes', $application);
+        $this->assertAttributeSame($routeCollector, 'routes', $application);
         $this->assertAttributeSame($runner, 'runner', $application);
     }
 }

test/Container/ApplicationFactoryTest.php

@@ -28,7 +28,8 @@
 use Zend\Expressive\Router\Middleware\ImplicitHeadMiddleware;
 use Zend\Expressive\Router\Middleware\ImplicitOptionsMiddleware;
 use Zend\Expressive\Router\Middleware\MethodNotAllowedMiddleware;
-use Zend\Expressive\Router\Middleware\PathBasedRoutingMiddleware as RouteMiddleware;
+use Zend\Expressive\Router\Middleware\RouteMiddleware;
+use Zend\Expressive\Router\RouteCollector;
 use Zend\Expressive\Router\RouterInterface;
 use Zend\Expressive\Router\ZendRouter;
 use Zend\HttpHandlerRunner\RequestHandlerRunner;
@@ -71,12 +72,12 @@ public function createApplicationFromRouter(RouterInterface $router)
         $container = new MiddlewareContainer($this->container->reveal());
         $factory = new MiddlewareFactory($container);
         $pipeline = new MiddlewarePipe();
-        $routeMiddleware = new RouteMiddleware($router);
+        $collector = new RouteCollector($router);
         $runner = $this->prophesize(RequestHandlerRunner::class)->reveal();
         return new Application(
             $factory,
             $pipeline,
-            $routeMiddleware,
+            $collector,
             $runner
         );
     }