Clean up routing middleware

Author: odan

Slim/Interfaces/RouteInterface.php

@@ -70,7 +70,7 @@ public function getArguments(): array;
     /**
      * Set route arguments.
      *
-     * @param array<string,mixed> $arguments The arguments.
+     * @param array<string,string> $arguments The arguments.
      *
      * @return RouteInterface
      */

Slim/Middleware/BasePathMiddleware.php

@@ -45,8 +45,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
             $basePath = $this->getBasePathByRequestUri($request);
         }
 
-        // $request = $request->withAttribute(RouteMatch::BASE_PATH_ATTRIBUTE, $basePath);
-
         $this->router->setBasePath($basePath);
 
         return $handler->handle($request);

Slim/Middleware/RoutingMiddleware.php

@@ -42,14 +42,14 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
     {
         $requestPath = $request->getUri()->getPath();
         $basePath = $this->router->getBasePath();
-        $dispatchPath = $this->stripBasePath($requestPath, $this->router->getBasePath());
+        $dispatchPath = $this->stripBasePath($requestPath, $basePath);
 
         $routingResult = $this->dispatcher->dispatch(
             $request->getMethod(),
             rawurldecode($dispatchPath)
         );
 
-        $routeMatch = $this->createRouteMatch($routingResult, $basePath);
+        $routeMatch = $this->createRouteMatch($routingResult);
         $request = $request->withAttribute(RouteMatch::class, $routeMatch);
 
         return $handler->handle($request);
@@ -58,7 +58,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
     /**
      * @param array<int, mixed> $routingResult
      */
-    private function createRouteMatch(array $routingResult, string $basePath): RouteMatch
+    private function createRouteMatch(array $routingResult): RouteMatch
     {
         $status = $routingResult[0] ?? null;
 

Slim/Routing/Route.php

@@ -11,6 +11,8 @@
 use Slim\Interfaces\MiddlewareCollectionInterface;
 use Slim\Interfaces\RouteInterface;
 
+use function array_key_exists;
+
 final class Route implements RouteInterface, MiddlewareCollectionInterface
 {
     use MiddlewareCollectionTrait;
@@ -45,7 +47,7 @@ final class Route implements RouteInterface, MiddlewareCollectionInterface
      *
      * @var array<string, string>
      */
-    private array $arguments;
+    private array $arguments = [];
 
     /**
      * @param array<string> $methods

tests/Middleware/RoutingMiddlewareTest.php

@@ -13,17 +13,19 @@
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Message\UriInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use RuntimeException;
 use Slim\Exception\HttpMethodNotAllowedException;
 use Slim\Exception\HttpNotFoundException;
 use Slim\Factory\AppFactory;
 use Slim\Interfaces\DispatcherInterface;
+use Slim\Interfaces\RouterInterface;
 use Slim\Interfaces\UrlGeneratorInterface;
 use Slim\Middleware\EndpointMiddleware;
 use Slim\Middleware\JsonBodyParserMiddleware;
 use Slim\Middleware\RoutingMiddleware;
 use Slim\Routing\RouteMatch;
-use Slim\Routing\RoutingResults;
 use Slim\Tests\Traits\AppTestTrait;
 
 final class RoutingMiddlewareTest extends TestCase
@@ -97,7 +99,7 @@ public function testRouteIsNotStoredOnMethodNotAllowed()
             } catch (HttpMethodNotAllowedException $exception) {
                 $request = $exception->getRequest();
 
-                // routingResults is available
+                // RouteMatch is available
                 /** @var RouteMatch $routeMatch */
                 $routeMatch = $request->getAttribute(RouteMatch::class);
                 $test->assertSame(DispatcherInterface::METHOD_NOT_ALLOWED, $routeMatch->getStatus());
@@ -140,7 +142,7 @@ public function testRouteIsNotStoredOnNotFound()
             } catch (HttpNotFoundException $exception) {
                 $request = $exception->getRequest();
 
-                // routingResults is available
+                // RouteMatch is available
                 $routeMatch = $request->getAttribute(RouteMatch::class);
                 $test->assertSame(DispatcherInterface::NOT_FOUND, $routeMatch->getStatus());
 
@@ -195,4 +197,41 @@ public function testRoutingWithBasePath(): void
         $this->assertSame('/api/users/123?page=2', $response->getHeaderLine('X-relativeUrlFor'));
         $this->assertSame('/api/users/123?page=2', $response->getHeaderLine('X-fullUrlFor'));
     }
+
+    public function testMethodNotAllowedThrowsRuntimeExceptionWhenAllowedMethodsPayloadIsInvalid(): void
+    {
+        $dispatcher = $this->createMock(DispatcherInterface::class);
+        $dispatcher
+            ->method('dispatch')
+            ->willReturn([
+                DispatcherInterface::METHOD_NOT_ALLOWED,
+                'GET',
+            ]);
+
+        $router = $this->createMock(RouterInterface::class);
+        $router
+            ->method('getBasePath')
+            ->willReturn('');
+
+        $middleware = new RoutingMiddleware($dispatcher, $router);
+
+        $request = $this->createMock(ServerRequestInterface::class);
+        $uri = $this->createMock(UriInterface::class);
+        $uri
+            ->method('getPath')
+            ->willReturn('/hello/foo');
+        $request
+            ->method('getUri')
+            ->willReturn($uri);
+        $request
+            ->method('getMethod')
+            ->willReturn('GET');
+
+        $handler = $this->createMock(RequestHandlerInterface::class);
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('Dispatcher returned invalid allowed methods.');
+
+        $middleware->process($request, $handler);
+    }
 }

tests/Routing/RouteMatchTest.php

@@ -41,9 +41,7 @@ public function testFoundRouteMatch(): void
 
     public function testNotFoundRouteMatch(): void
     {
-        $basePath = '/api';
-
-        $routeMatch = RouteMatch::notFound($basePath);
+        $routeMatch = RouteMatch::notFound();
 
         $this->assertFalse($routeMatch->isFound());
         $this->assertTrue($routeMatch->isNotFound());
@@ -57,9 +55,7 @@ public function testNotFoundRouteMatch(): void
     public function testMethodNotAllowedRouteMatch(): void
     {
         $allowedMethods = ['GET', 'POST'];
-        $basePath = '/api';
-
-        $routeMatch = RouteMatch::methodNotAllowed($allowedMethods, $basePath);
+        $routeMatch = RouteMatch::methodNotAllowed($allowedMethods);
 
         $this->assertFalse($routeMatch->isFound());
         $this->assertFalse($routeMatch->isNotFound());

tests/Routing/RouteTest.php

@@ -137,6 +137,23 @@ public function testGetMethodsReturnsCorrectMethods(): void
         $this->assertSame($methods, $route->getMethods());
     }
 
+    public function testSetArgumentsStoresStringArguments(): void
+    {
+        $methods = ['GET'];
+        $pattern = '/users/{id}';
+        $handler = function () {
+            return 'handler';
+        };
+
+        $route = new Route($methods, $pattern, $handler);
+
+        $arguments = ['id' => '123', 'slug' => 'john-doe'];
+        $route->setArguments($arguments);
+
+        $this->assertSame($arguments, $route->getArguments());
+        $this->assertSame('123', $route->getArgument('id'));
+    }
+
     private function createMiddleware(): MiddlewareInterface
     {
         return new class implements MiddlewareInterface {