From aa6c6ddfdb121d837a85953422916288b82591b0 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Mon, 9 Dec 2024 23:45:00 +0800
Subject: [PATCH] Fix path support after unlimited optional placeholders

When using a route which contains both an unlimited optional
placeholder, and another optional placeholder afterwards, the incorrect
values are collected.

For example, given the following route:

```
/go/to/[{location:.*}[/info/{subpage}]]
```

The following behaviour is currently observed:

- route: `/go/to/[{location:.*}[/info/{subpage}]]`
- url: `/go/to/australia/perth/info/about`
- location: `'australia/perth/info/about'`
- subpage: `''`

Note that the `location` contains `/info/about` and the `subpage` is
empty.

This is inconsistent with the behaviour where an unlimited value is
_not_ used:

- route: `/go/to/[{location}[/info/{subpage}]]`
- url: `/go/to/australia/info/about`
- location: `'australia'`
- subpage: `'about'`

In the case of the unlimited optional path, the expected behaviour is:
The correct value would be:

- route: `/go/to/[{location:.*}[/info/{subpage}]]`
- url: `/go/to/australia/perth/info/about`
- location: `'australia/perth'`
- subpage: `'about'`

This commit change updates the route dispatcher to reverse the order of
the routes when adding routes to the router, which brings the unlimited
path placeholder format inline with limited path placeholders.

Signed-off-by: Andrew Nicols <andrew@nicols.co.uk>
---
 src/RouteCollector.php             |  2 +-
 test/Dispatcher/DispatcherTest.php | 24 ++++++++++++++++++++++++
 test/RouteCollectorTest.php        | 19 +++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/RouteCollector.php b/src/RouteCollector.php
index c1c1762..0897924 100644
--- a/src/RouteCollector.php
+++ b/src/RouteCollector.php
@@ -38,7 +38,7 @@ public function __construct(RouteParser $routeParser, DataGenerator $dataGenerat
     public function addRoute($httpMethod, $route, $handler)
     {
         $route = $this->currentGroupPrefix . $route;
-        $routeDatas = $this->routeParser->parse($route);
+        $routeDatas = array_reverse($this->routeParser->parse($route));
         foreach ((array) $httpMethod as $method) {
             foreach ($routeDatas as $routeData) {
                 $this->dataGenerator->addRoute($method, $routeData, $handler);
diff --git a/test/Dispatcher/DispatcherTest.php b/test/Dispatcher/DispatcherTest.php
index c2edf8c..075196f 100644
--- a/test/Dispatcher/DispatcherTest.php
+++ b/test/Dispatcher/DispatcherTest.php
@@ -413,6 +413,30 @@ public function provideFoundDispatchCases()
 
         $cases[] = ['POST', '/bar', $callback, 'handler1', ['foo' => 'bar']];
 
+        // 27 -------------------------------------------------------------------------------------->
+
+        $callback = function (RouteCollector $r) {
+            $r->addRoute('GET', '/about[/{aboutwhat}[/location]]', 'handler0');
+        };
+
+        $cases[] = ['GET', '/about/some/location', $callback, 'handler0', [
+            'aboutwhat' => 'some',
+        ], [
+            '_route' => '/about[/{aboutwhat}[/location]]',
+        ]];
+
+        // 28 -------------------------------------------------------------------------------------->
+
+        $callback = function (RouteCollector $r) {
+            $r->addRoute('GET', '/about[/{aboutwhat:.*}[/location]]', 'handler0');
+        };
+
+        $cases[] = ['GET', '/about/the/nested/location', $callback, 'handler0', [
+            'aboutwhat' => 'the/nested',
+        ], [
+            '_route' => '/about[/{aboutwhat:.*}[/location]]',
+        ]];
+
         // x -------------------------------------------------------------------------------------->
 
         return $cases;
diff --git a/test/RouteCollectorTest.php b/test/RouteCollectorTest.php
index cc54407..7196e79 100644
--- a/test/RouteCollectorTest.php
+++ b/test/RouteCollectorTest.php
@@ -90,6 +90,25 @@ public function testGroups()
 
         $this->assertSame($expected, $r->routes);
     }
+
+    public function testOptionalRoutesCanBeUsed(): void
+    {
+        $r = new DummyRouteCollector();
+
+        $r->head('/head[/{optional}]', 'headHandler');
+        $r->get('/get[/{optional}/hello]', 'getHandler');
+        $r->post('/post[/{optional:.*}]', 'postHandler');
+        $r->delete('/delete[/{optional:.*}/hello]', 'deleteHandler');
+
+        $expected = [
+            ['HEAD', '/head[/{optional}]', 'headHandler'],
+            ['GET', '/get[/{optional}/hello]', 'getHandler'],
+            ['POST', '/post[/{optional:.*}]', 'postHandler'],
+            ['DELETE', '/delete[/{optional:.*}/hello]', 'deleteHandler'],
+        ];
+
+        self::assertSame($expected, $r->routes);
+    }
 }
 
 class DummyRouteCollector extends RouteCollector