feat(routing): processed feedback from brendt

- Renamed Regexes to Regex as it is already plural - Make Regex limit a power of 2.. because it should be - Make RouteMatch isFound a computed method based on routeMark
tempestphp · Nov 12, 2024 · f9ead7f · f9ead7f
1 parent d27e883
commit f9ead7f
Show file tree

Hide file tree

Showing 11 changed files with 40 additions and 36 deletions.
diff --git a/src/Tempest/Http/src/RouteConfig.php b/src/Tempest/Http/src/RouteConfig.php
@@ -4,7 +4,7 @@
 
 namespace Tempest\Http;
 
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
 final class RouteConfig
 {
@@ -13,7 +13,7 @@ public function __construct(
         public array $staticRoutes = [],
         /** @var array<string, array<string, Route>> */
         public array $dynamicRoutes = [],
-        /** @var array<string, MatchingRegexes> */
+        /** @var array<string, MatchingRegex> */
         public array $matchingRegexes = [],
     ) {
     }

diff --git a/...struction/RouteMatchingRegexesBuilder.php → ...onstruction/RouteMatchingRegexBuilder.php b/...struction/RouteMatchingRegexesBuilder.php → ...onstruction/RouteMatchingRegexBuilder.php
@@ -4,22 +4,22 @@
 
 namespace Tempest\Http\Routing\Construction;
 
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
-final readonly class RouteMatchingRegexesBuilder
+final readonly class RouteMatchingRegexBuilder
 {
     // This limit is guesstimated using a small script with an ever in pattern feed into preg_match
-    private const int PREG_REGEX_SIZE_LIMIT = 32764;
+    private const int PREG_REGEX_SIZE_LIMIT = 32768;
 
-    private const int REGEX_SIZE_MARGIN = 264;
+    private const int REGEX_SIZE_MARGIN = 256;
 
     private const REGEX_SIZE_LIMIT = self::PREG_REGEX_SIZE_LIMIT - self::REGEX_SIZE_MARGIN;
 
     public function __construct(private RouteTreeNode $rootNode)
     {
     }
 
-    public function toRegex(): MatchingRegexes
+    public function toRegex(): MatchingRegex
     {
         // Holds all regex "chunks"
         $regexes = [];
@@ -101,7 +101,7 @@ public function toRegex(): MatchingRegexes
         }
 
         // Return all regex chunks including the current one
-        return new MatchingRegexes([
+        return new MatchingRegex([
             ...$regexes,
             '#' . substr($regex, 1) . '#',
         ]);

diff --git a/src/Tempest/Http/src/Routing/Construction/RoutingTree.php b/src/Tempest/Http/src/Routing/Construction/RoutingTree.php
@@ -4,7 +4,7 @@
 
 namespace Tempest\Http\Routing\Construction;
 
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
 /**
  * @internal
@@ -34,9 +34,9 @@ public function add(MarkedRoute $markedRoute): void
         $node->setTargetRoute($markedRoute);
     }
 
-    /** @return array<string, MatchingRegexes> */
+    /** @return array<string, MatchingRegex> */
     public function toMatchingRegexes(): array
     {
-        return array_map(static fn (RouteTreeNode $node) => (new RouteMatchingRegexesBuilder($node))->toRegex(), $this->roots);
+        return array_map(static fn (RouteTreeNode $node) => (new RouteMatchingRegexBuilder($node))->toRegex(), $this->roots);
     }
 }
diff --git a/src/Tempest/Http/src/Routing/Matching/GenericRouteMatcher.php b/src/Tempest/Http/src/Routing/Matching/GenericRouteMatcher.php
@@ -51,7 +51,7 @@ private function matchDynamicRoute(PsrRequest $request): ?MatchedRoute
         // Then we'll use this regex to see whether we have a match or not
         $matchResult = $matchingRegexForMethod->match($request->getUri()->getPath());
 
-        if (! $matchResult->isFound) {
+        if (! $matchResult->isFound()) {
             return null;
         }
 

diff --git a/.../src/Routing/Matching/MatchingRegexes.php → ...tp/src/Routing/Matching/MatchingRegex.php b/.../src/Routing/Matching/MatchingRegexes.php → ...tp/src/Routing/Matching/MatchingRegex.php
@@ -7,7 +7,7 @@
 use RuntimeException;
 use Tempest\Http\Routing\Construction\MarkedRoute;
 
-final readonly class MatchingRegexes
+final readonly class MatchingRegex
 {
     /**
      * @param string[] $patterns

diff --git a/src/Tempest/Http/src/Routing/Matching/RouteMatch.php b/src/Tempest/Http/src/Routing/Matching/RouteMatch.php
@@ -9,19 +9,23 @@
 final readonly class RouteMatch
 {
     private function __construct(
-        public bool $isFound,
         public ?string $mark,
         public array $matches,
     ) {
     }
 
     public static function match(array $params): self
     {
-        return new self(true, $params[MarkedRoute::REGEX_MARK_TOKEN], $params);
+        return new self($params[MarkedRoute::REGEX_MARK_TOKEN], $params);
     }
 
     public static function notFound(): self
     {
-        return new self(false, null, []);
+        return new self(null, []);
+    }
+
+    public function isFound(): bool
+    {
+        return $this->mark !== null;
     }
 }
diff --git a/src/Tempest/Http/tests/RouteConfigTest.php b/src/Tempest/Http/tests/RouteConfigTest.php
@@ -8,7 +8,7 @@
 use Tempest\Http\Method;
 use Tempest\Http\Route;
 use Tempest\Http\RouteConfig;
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
 /**
  * @internal
@@ -25,7 +25,7 @@ public function test_serialization(): void
                 'POST' => ['b' => new Route('/', Method::POST)],
             ],
             [
-                'POST' => new MatchingRegexes(['#^(?|/([^/]++)(?|/1\/?$(*MARK:b)|/3\/?$(*MARK:d)))#']),
+                'POST' => new MatchingRegex(['#^(?|/([^/]++)(?|/1\/?$(*MARK:b)|/3\/?$(*MARK:d)))#']),
             ]
         );
 

diff --git a/src/Tempest/Http/tests/Routing/Construction/RouteConfiguratorTest.php b/src/Tempest/Http/tests/Routing/Construction/RouteConfiguratorTest.php
@@ -9,7 +9,7 @@
 use Tempest\Http\Route;
 use Tempest\Http\RouteConfig;
 use Tempest\Http\Routing\Construction\RouteConfigurator;
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
 /**
  * @internal
@@ -89,10 +89,10 @@ public function test_adding_dynamic_routes(): void
         ], $config->dynamicRoutes);
 
         $this->assertEquals([
-            'GET' => new MatchingRegexes([
+            'GET' => new MatchingRegex([
                 '#^(?|/dynamic(?|/([^/]++)(?|\/?$(*MARK:b)|/view\/?$(*MARK:d)|/([^/]++)(?|/([^/]++)(?|/([^/]++)\/?$(*MARK:e))))))#',
             ]),
-            'PATCH' => new MatchingRegexes([
+            'PATCH' => new MatchingRegex([
                 '#^(?|/dynamic(?|/([^/]++)\/?$(*MARK:c)))#',
             ]),
         ], $config->matchingRegexes);

diff --git a/src/Tempest/Http/tests/Routing/Construction/RoutingTreeTest.php b/src/Tempest/Http/tests/Routing/Construction/RoutingTreeTest.php
@@ -10,7 +10,7 @@
 use Tempest\Http\Routing\Construction\DuplicateRouteException;
 use Tempest\Http\Routing\Construction\MarkedRoute;
 use Tempest\Http\Routing\Construction\RoutingTree;
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
 /**
  * @internal
@@ -42,7 +42,7 @@ public function test_multiple_routes(): void
         $subject->add(new MarkedRoute('e', new Route('/{greeting}/brent', Method::GET)));
 
         $this->assertEquals([
-            'GET' => new MatchingRegexes([
+            'GET' => new MatchingRegex([
                 '#^(?|\/?$(*MARK:a)|/([^/]++)(?|/brent\/?$(*MARK:e)|/hello(?|/brent\/?$(*MARK:c)|/([^/]++)\/?$(*MARK:b))|/([^/]++)\/?$(*MARK:d)))#',
             ]),
         ], $subject->toMatchingRegexes());
@@ -61,8 +61,8 @@ public function test_chunked_routes(): void
         $matchingRegexes = $subject->toMatchingRegexes()['GET'];
         $this->assertGreaterThan(1, count($matchingRegexes->patterns));
 
-        $this->assertTrue($matchingRegexes->match('/test/0/route_0')->isFound);
-        $this->assertTrue($matchingRegexes->match('/test/1000/route_1000')->isFound);
+        $this->assertTrue($matchingRegexes->match('/test/0/route_0')->isFound());
+        $this->assertTrue($matchingRegexes->match('/test/1000/route_1000')->isFound());
     }
 
     public function test_multiple_http_methods(): void
@@ -72,8 +72,8 @@ public function test_multiple_http_methods(): void
         $subject->add(new MarkedRoute('b', new Route('/', Method::POST)));
 
         $this->assertEquals([
-            'GET' => new MatchingRegexes(['#^\/?$(*MARK:a)#']),
-            'POST' => new MatchingRegexes(['#^\/?$(*MARK:b)#']),
+            'GET' => new MatchingRegex(['#^\/?$(*MARK:a)#']),
+            'POST' => new MatchingRegex(['#^\/?$(*MARK:b)#']),
         ], $subject->toMatchingRegexes());
     }
 }
diff --git a/src/Tempest/Http/tests/Routing/Matching/GenericRouteMatcherTest.php b/src/Tempest/Http/tests/Routing/Matching/GenericRouteMatcherTest.php
@@ -10,7 +10,7 @@
 use Tempest\Http\Route;
 use Tempest\Http\RouteConfig;
 use Tempest\Http\Routing\Matching\GenericRouteMatcher;
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 
 /**
  * @internal
@@ -42,8 +42,8 @@ protected function setUp(): void
                 ],
             ],
             [
-                'GET' => new MatchingRegexes(['#^(?|/dynamic(?|/([^/]++)(?|/view\/?$(*MARK:d)|/([^/]++)(?|/([^/]++)(?|/([^/]++)\/?$(*MARK:e)))|\/?$(*MARK:b))))#']),
-                'PATCH' => new MatchingRegexes(['#^(?|/dynamic(?|/([^/]++)\/?$(*MARK:c)))#']),
+                'GET' => new MatchingRegex(['#^(?|/dynamic(?|/([^/]++)(?|/view\/?$(*MARK:d)|/([^/]++)(?|/([^/]++)(?|/([^/]++)\/?$(*MARK:e)))|\/?$(*MARK:b))))#']),
+                'PATCH' => new MatchingRegex(['#^(?|/dynamic(?|/([^/]++)\/?$(*MARK:c)))#']),
             ]
         );
 

diff --git a/.../Routing/Matching/MatchingRegexesTest.php → ...ts/Routing/Matching/MatchingRegexTest.php b/.../Routing/Matching/MatchingRegexesTest.php → ...ts/Routing/Matching/MatchingRegexTest.php
@@ -6,21 +6,21 @@
 
 use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\TestCase;
-use Tempest\Http\Routing\Matching\MatchingRegexes;
+use Tempest\Http\Routing\Matching\MatchingRegex;
 use Tempest\Http\Routing\Matching\RouteMatch;
 
 /**
  * @internal
  */
-final class MatchingRegexesTest extends TestCase
+final class MatchingRegexTest extends TestCase
 {
-    private MatchingRegexes $subject;
+    private MatchingRegex $subject;
 
     protected function setUp(): void
     {
         parent::setUp();
 
-        $this->subject = new MatchingRegexes([
+        $this->subject = new MatchingRegex([
             '#^(a)(*MARK:a)$#',
             '#^(b)(*MARK:b)$#',
             '#^(c)(*MARK:c)$#',
@@ -29,7 +29,7 @@ protected function setUp(): void
 
     public function test_empty(): void
     {
-        $subject = new MatchingRegexes([]);
+        $subject = new MatchingRegex([]);
 
         $this->assertEquals(RouteMatch::notFound(), $subject->match(''));
     }
@@ -41,7 +41,7 @@ public function test_match(string $expectedMatch): void
     {
         $match = $this->subject->match($expectedMatch);
 
-        $this->assertTrue($match->isFound);
+        $this->assertTrue($match->isFound());
         $this->assertEquals($expectedMatch, $match->mark);
         $this->assertEquals($expectedMatch, $match->matches[1]);
     }