From 50b130e5b0f1ba6752df1f3d34df21cc28adf99d Mon Sep 17 00:00:00 2001 From: John Charman Date: Wed, 28 Jun 2023 13:33:48 +0100 Subject: [PATCH 1/6] Assert incorrect prioritisation by router Router prioritises dynamic by order they appear, not by how many dynamic elements they contain --- tests/Router/APIeceOfCakeTest.php | 77 ++++++++++++++++++++ tests/fixtures/APIeceOfCake.json | 114 ++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 tests/Router/APIeceOfCakeTest.php create mode 100644 tests/fixtures/APIeceOfCake.json diff --git a/tests/Router/APIeceOfCakeTest.php b/tests/Router/APIeceOfCakeTest.php new file mode 100644 index 0000000..280996f --- /dev/null +++ b/tests/Router/APIeceOfCakeTest.php @@ -0,0 +1,77 @@ +readFromAbsoluteFilePath($apiFilePath); + $routeCollection = (new RouteCollector())->collect($api); + + // I expect to have no hosted routes + self::assertEmpty($routeCollection->routes['hosted']['static']); + self::assertEmpty($routeCollection->routes['hosted']['dynamic']['servers']); + + // I expect to have one hostless static route + self::assertSame( + 'findSpongeCakes', + $routeCollection->routes['hostless']['static']['']['static']['/cakes/sponge']['get'] + ); + + // I expect to have the following hostless dynamic routes + $hostlessDynamicRoutes = [ + '/{cakeType}/{icing}' => ['get' => 'findDessertByIcing', 'post' => 'addDessertByIcing'], + '/cakes/{icing}' => ['get' => 'findCakesByIcing', 'post' => 'addCakesByIcing'], + '/{cakeType}/sponge' => ['get' => 'findSpongeByDesserts'] + ]; + self::assertSame( + $hostlessDynamicRoutes, + $routeCollection->routes['hostless']['static']['']['dynamic']['paths'] + ); + + return $routeCollection; + } + + #[Test, TestDox('Completely static paths should take priority over any other')] + #[Depends('itCollectsPathsFromAPIeceOfCake')] + public function itRoutesToCompletelyStaticPathFirst(RouteCollection $routeCollection): void + { + $expectedOperationId = 'findSpongeCakes'; + $sut = new Router($routeCollection); + + $actualOperationId = $sut->route('/cakes/sponge', 'get'); + + self::assertSame($expectedOperationId, $actualOperationId); + } + + #[Test, TestDox('Paths with less dynamic elements should take priority')] + #[Depends('itCollectsPathsFromAPIeceOfCake')] + public function itRoutesToPartiallyDynamicBeforeCompletelyDynamic(RouteCollection $routeCollection): void + { + $expectedOperationId = 'findCakesByIcing'; + $sut = new Router($routeCollection); + + $actualOperationId = $sut->route('/cakes/chocolate', 'get'); + + self::assertSame($expectedOperationId, $actualOperationId); + } +} diff --git a/tests/fixtures/APIeceOfCake.json b/tests/fixtures/APIeceOfCake.json new file mode 100644 index 0000000..41791b1 --- /dev/null +++ b/tests/fixtures/APIeceOfCake.json @@ -0,0 +1,114 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "APIece of Cake", + "description": "An API for paths to take to make a cake to bake", + "version": "1.0.0" + }, + "paths": { + "/{cakeType}/{icing}": { + "parameters": [ + { + "name": "cakeType", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "icing", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "get": { + "operationId": "findDessertByIcing", + "responses": { + "200": { + "description": "Successful Dessert Response" + } + } + }, + "post": { + "operationId": "addDessertByIcing", + "responses": { + "200": { + "description": "Successful Dessert Response" + } + } + } + }, + "/cakes/sponge": { + "get": { + "operationId": "findSpongeCakes", + "responses": { + "200": { + "description": "Successful Cake Response" + } + } + } + }, + "/cakes/{icing}": { + "parameters": [ + { + "name": "icing", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "get": { + "operationId": "findCakesByIcing", + "responses": { + "200": { + "description": "Successful Cake Response" + } + } + }, + "post": { + "operationId": "addCakesByIcing", + "parameters": [ + { + "name": "icing", + "in": "path", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "description": "Successful Cake Response" + } + } + } + }, + "/{cakeType}/sponge": { + "parameters": [ + { + "name": "cakeType", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "get": { + "operationId": "findSpongeByDesserts", + "responses": { + "200": { + "description": "Successful Sponge Response" + } + } + } + } + } +} From 95c01f64e989a87edd53024e02214e2b6d4fb29e Mon Sep 17 00:00:00 2001 From: John Charman Date: Wed, 28 Jun 2023 18:24:47 +0100 Subject: [PATCH 2/6] Refactor RouteCollector --- src/Router/Collector/RouteCollector.php | 195 ++++-------------- src/Router/ValueObject/Route/Path.php | 39 ++++ src/Router/ValueObject/{ => Route}/Route.php | 5 +- src/Router/ValueObject/Route/Server.php | 73 +++++++ src/Router/ValueObject/RouteCollection.php | 53 +++++ .../Commands/CacheOpenAPIRoutesTest.php | 2 +- .../Service/CacheOpenAPIRoutesTest.php | 2 +- tests/Router/Collector/RouteCollectorTest.php | 2 +- 8 files changed, 214 insertions(+), 157 deletions(-) create mode 100644 src/Router/ValueObject/Route/Path.php rename src/Router/ValueObject/{ => Route}/Route.php (62%) create mode 100644 src/Router/ValueObject/Route/Server.php diff --git a/src/Router/Collector/RouteCollector.php b/src/Router/Collector/RouteCollector.php index 1199ed0..0ac5f03 100644 --- a/src/Router/Collector/RouteCollector.php +++ b/src/Router/Collector/RouteCollector.php @@ -9,42 +9,73 @@ use cebe\openapi\spec\PathItem; use Membrane\OpenAPIRouter\Exception\CannotProcessOpenAPI; use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; -use Membrane\OpenAPIRouter\Router\ValueObject\Route; +use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; +use Membrane\OpenAPIRouter\Router\ValueObject\Route\Server as ServerRoute; use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; class RouteCollector { public function collect(OpenApi $openApi): RouteCollection { - $routes = $this->collectRoutes($openApi); + $collection = $this->collectRoutes($openApi); - if ($routes === []) { + if ($collection === []) { throw CannotRouteOpenAPI::noRoutes(); } - return $this->sortRoutes($this->mergeRoutes(...$routes)); + return RouteCollection::fromServers(...$collection); } - /** @return Route[] */ + /** @return array */ + private function getServers(OpenApi|PathItem|Operation $object): array + { + $uniqueServers = array_unique(array_map(fn($p) => rtrim($p->url, '/'), $object->servers)); + return array_combine($uniqueServers, array_map(fn($p) => $this->getRegex($p), $uniqueServers)); + } + + private function getRegex(string $path): string + { + $regex = preg_replace('#{[^/]+}#', '([^/]+)', $path); + assert($regex !== null); // The pattern is hardcoded, valid regex so should not cause an error in preg_replace + + return $regex; + } + + /** @return array */ private function collectRoutes(OpenApi $openApi): array { + $collection = []; + $rootServers = $this->getServers($openApi); + foreach ($rootServers as $url => $regex) { + $collection[$url] ??= new ServerRoute($url, $regex); + } + $operationIds = []; foreach ($openApi->paths as $path => $pathObject) { + $pathRegex = $this->getRegex($path); + $pathServers = $this->getServers($pathObject); - foreach ($pathObject->getOperations() as $operation => $operationObject) { + foreach ($pathServers as $url => $regex) { + $collection[$url] ??= new ServerRoute($url, $regex); + } + + foreach ($pathObject->getOperations() as $method => $operationObject) { $operationServers = $this->getServers($operationObject); + foreach ($operationServers as $url => $regex) { + $collection[$url] ??= new ServerRoute($url, $regex); + } // TODO remove this conditional once OpenAPIFileReader requires operationId if ($operationObject->operationId === null) { - throw CannotProcessOpenAPI::missingOperationId($path, $operation); + throw CannotProcessOpenAPI::missingOperationId($path, $method); } if (isset($operationIds[$operationObject->operationId])) { throw CannotProcessOpenAPI::duplicateOperationId( $operationObject->operationId, $operationIds[$operationObject->operationId], - ['path' => $path, 'operation' => $operation] + ['path' => $path, 'operation' => $method] ); } @@ -56,152 +87,14 @@ private function collectRoutes(OpenApi $openApi): array $servers = $rootServers; } - $collection[] = new Route( - $servers, - $path, - $operation, - $operationObject->operationId - ); - - $operationIds[$operationObject->operationId] = [ - 'path' => $path, - 'operation' => $operation - ]; - } - } - return $collection ?? []; - } - - /** @return string[] */ - private function getServers(OpenApi|PathItem|Operation $object): array - { - return array_unique(array_map(fn($p) => rtrim($p->url, '/'), $object->servers)); - } - - /** @return string[][][] */ - private function mergeRoutes(Route ...$routes): array - { - foreach ($routes as $route) { - foreach ($route->servers as $server) { - $routesArray[$server][$route->path][$route->method] = $route->operationId; - } - } - - return $routesArray ?? []; - } - - /** @param string[][][] $routes */ - private function sortRoutes(array $routes): RouteCollection - { - $routesWithSortedPaths = []; - - foreach ($routes as $server => $paths) { - $routesWithSortedPaths[$server] = $this->sortPaths($paths); - } - - $routesWithSortedServers = $this->sortServers($routesWithSortedPaths); - - return $routesWithSortedServers; - } - - /** - * @param string[][] $paths - * @return array{ - * 'static': string[][], - * 'dynamic': array{ - * 'regex': string, - * 'paths': string[][] - * } - * } - */ - private function sortPaths(array $paths): array - { - $staticPaths = $dynamicPaths = $groupRegex = []; - - foreach ($paths as $path => $operations) { - $pathRegex = $this->getRegex($path); - if ($path === $pathRegex) { - $staticPaths[$path] = $operations; - } else { - $dynamicPaths[$path] = $operations; - $groupRegex[] = sprintf('%s(*MARK:%s)', $pathRegex, $path); - } - } - - return [ - 'static' => $staticPaths, - 'dynamic' => [ - 'regex' => sprintf('#^(?|%s)$#', implode('|', $groupRegex)), - 'paths' => $dynamicPaths, - ], - ]; - } - - /** - * @param array $servers - */ - private function sortServers(array $servers): RouteCollection - { - $hostedServers = $hostlessServers = []; - foreach ($servers as $server => $paths) { - if (parse_url($server, PHP_URL_HOST) === null) { - $hostlessServers[$server] = $paths; - } else { - $hostedServers[$server] = $paths; - } - } - - $hostedStaticServers = $hostedDynamicServers = $hostedGroupRegex = []; - foreach ($hostedServers as $server => $paths) { - $serverRegex = $this->getRegex($server); - if ($server === $serverRegex) { - $hostedStaticServers[$server] = $paths; - } else { - $hostedDynamicServers[$server] = $paths; - $hostedGroupRegex[] = sprintf('%s(*MARK:%s)', $serverRegex, $server); - } - } + foreach ($servers as $url => $regex) { + $collection[$url]->addRoute(new Route($path, $pathRegex, $method, $operationObject->operationId)); + } - $hostlessStaticServers = $hostlessDynamicServers = $hostlessGroupRegex = []; - foreach ($hostlessServers as $server => $paths) { - $serverRegex = $this->getRegex($server); - if ($server === $serverRegex) { - $hostlessStaticServers[$server] = $paths; - } else { - $hostlessDynamicServers[$server] = $paths; - $hostlessGroupRegex[] = sprintf('%s(*MARK:%s)', $serverRegex, $server); + $operationIds[$operationObject->operationId] = ['path' => $path, 'operation' => $method]; } } - return new RouteCollection([ - 'hosted' => [ - 'static' => $hostedStaticServers, - 'dynamic' => [ - 'regex' => sprintf('#^(?|%s)#', implode('|', $hostedGroupRegex)), - 'servers' => $hostedDynamicServers, - ], - ], - 'hostless' => [ - 'static' => $hostlessStaticServers, - 'dynamic' => [ - 'regex' => sprintf('#^(?|%s)#', implode('|', $hostlessGroupRegex)), - 'servers' => $hostlessDynamicServers, - ], - ], - ]); - } - - private function getRegex(string $path): string - { - $regex = preg_replace('#{[^/]+}#', '([^/]+)', $path); - assert($regex !== null); // The pattern is hardcoded, valid regex so should not cause an error in preg_replace - - return $regex; + return array_filter($collection, fn($s) => !$s->isEmpty()); } } diff --git a/src/Router/ValueObject/Route/Path.php b/src/Router/ValueObject/Route/Path.php new file mode 100644 index 0000000..5c20a68 --- /dev/null +++ b/src/Router/ValueObject/Route/Path.php @@ -0,0 +1,39 @@ + */ + private array $operations = []; + + public function __construct( + public readonly string $url, + public readonly string $regex, + ) { + } + + public function addRoute(Route $route): void + { + $this->operations[$route->method] = $route->operationId; + } + + public function isDynamic(): bool + { + return $this->url !== $this->regex; + } + + public function isEmpty(): bool + { + return count($this->operations) === 0; + } + + public function jsonSerialize(): array + { + return [...$this->operations]; + } +} diff --git a/src/Router/ValueObject/Route.php b/src/Router/ValueObject/Route/Route.php similarity index 62% rename from src/Router/ValueObject/Route.php rename to src/Router/ValueObject/Route/Route.php index 999c46a..4e3a704 100644 --- a/src/Router/ValueObject/Route.php +++ b/src/Router/ValueObject/Route/Route.php @@ -2,14 +2,13 @@ declare(strict_types=1); -namespace Membrane\OpenAPIRouter\Router\ValueObject; +namespace Membrane\OpenAPIRouter\Router\ValueObject\Route; class Route { - /** @param string[] $servers */ public function __construct( - public readonly array $servers, public readonly string $path, + public readonly string $pathRegex, public readonly string $method, public readonly string $operationId ) { diff --git a/src/Router/ValueObject/Route/Server.php b/src/Router/ValueObject/Route/Server.php new file mode 100644 index 0000000..fec943f --- /dev/null +++ b/src/Router/ValueObject/Route/Server.php @@ -0,0 +1,73 @@ +*/ + private array $paths = []; + + public function __construct( + public readonly string $url, + public readonly string $regex, + ) { + } + + public function addRoute(Route $route): void + { + if (!isset($this->paths[$route->path])) { + $this->addPath(new Path($route->path, $route->pathRegex)); + } + + $this->paths[$route->path]->addRoute($route); + } + + public function isDynamic(): bool + { + return $this->url !== $this->regex; + } + + public function isEmpty(): bool + { + return count(array_filter($this->paths, fn($p) => !$p->isEmpty())) === 0; + } + + public function isHosted(): bool + { + return parse_url($this->url, PHP_URL_HOST) !== null; + } + + public function jsonSerialize(): mixed + { + $filteredPaths = array_filter($this->paths, fn($p) => !$p->isEmpty()); + + $staticPaths = $dynamicPaths = $regex = []; + foreach ($filteredPaths as $path) { + if ($path->isDynamic()) { + $dynamicPaths[$path->url] = $path->jsonSerialize(); + $regex[] = sprintf('%s(*MARK:%s)', $path->regex, $path->url); + } else { + $staticPaths[$path->url] = $path->jsonSerialize(); + } + } + + return [ + 'static' => $staticPaths, + 'dynamic' => [ + 'regex' => sprintf('#^(?|%s)$#', implode('|', $regex)), + 'paths' => $dynamicPaths + ] + ]; + } + + private function addPath(Path $path): void + { + if (!isset($this->paths[$path->url])) { + $this->paths[$path->url] = $path; + } + } +} diff --git a/src/Router/ValueObject/RouteCollection.php b/src/Router/ValueObject/RouteCollection.php index 1e926d2..22c84cb 100644 --- a/src/Router/ValueObject/RouteCollection.php +++ b/src/Router/ValueObject/RouteCollection.php @@ -4,6 +4,8 @@ namespace Membrane\OpenAPIRouter\Router\ValueObject; +use Membrane\OpenAPIRouter\Router\ValueObject\Route\Server; + class RouteCollection { /** @@ -40,4 +42,55 @@ public function __construct( public readonly array $routes ) { } + + public static function fromServers(Server ...$servers): self + { + $filteredServers = array_filter($servers, fn($s) => !$s->isEmpty()); + + $hostedServers = $hostlessServers = []; + foreach ($servers as $server) { + if ($server->isHosted()) { + $hostedServers[$server->url] = $server; + } else { + $hostlessServers[$server->url] = $server; + } + } + + $hostedStaticServers = $hostedDynamicServers = $hostedGroupRegex = []; + foreach ($hostedServers as $server) { + if ($server->isDynamic()) { + $hostedDynamicServers[$server->url] = $server->jsonSerialize(); + $hostedGroupRegex[] = sprintf('%s(*MARK:%s)', $server->regex, $server->url); + } else { + $hostedStaticServers[$server->url] = $server->jsonSerialize(); + } + } + + $hostlessStaticServers = $hostlessDynamicServers = $hostlessGroupRegex = []; + foreach ($hostlessServers as $server) { + if ($server->isDynamic()) { + $hostlessDynamicServers[$server->url] = $server->jsonSerialize(); + $hostlessGroupRegex[] = sprintf('%s(*MARK:%s)', $server->regex, $server->url); + } else { + $hostlessStaticServers[$server->url] = $server->jsonSerialize(); + } + } + + return new self([ + 'hosted' => [ + 'static' => $hostedStaticServers, + 'dynamic' => [ + 'regex' => sprintf('#^(?|%s)#', implode('|', $hostedGroupRegex)), + 'servers' => $hostedDynamicServers, + ], + ], + 'hostless' => [ + 'static' => $hostlessStaticServers, + 'dynamic' => [ + 'regex' => sprintf('#^(?|%s)#', implode('|', $hostlessGroupRegex)), + 'servers' => $hostlessDynamicServers, + ], + ], + ]); + } } diff --git a/tests/Console/Commands/CacheOpenAPIRoutesTest.php b/tests/Console/Commands/CacheOpenAPIRoutesTest.php index 25ced35..48336c7 100644 --- a/tests/Console/Commands/CacheOpenAPIRoutesTest.php +++ b/tests/Console/Commands/CacheOpenAPIRoutesTest.php @@ -8,7 +8,7 @@ use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; use Membrane\OpenAPIRouter\Reader\OpenAPIFileReader; use Membrane\OpenAPIRouter\Router\Collector\RouteCollector; -use Membrane\OpenAPIRouter\Router\ValueObject\Route; +use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamDirectory; diff --git a/tests/Console/Service/CacheOpenAPIRoutesTest.php b/tests/Console/Service/CacheOpenAPIRoutesTest.php index e3ed1ef..fd570a7 100644 --- a/tests/Console/Service/CacheOpenAPIRoutesTest.php +++ b/tests/Console/Service/CacheOpenAPIRoutesTest.php @@ -9,7 +9,7 @@ use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; use Membrane\OpenAPIRouter\Reader\OpenAPIFileReader; use Membrane\OpenAPIRouter\Router\Collector\RouteCollector; -use Membrane\OpenAPIRouter\Router\ValueObject\Route; +use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamDirectory; diff --git a/tests/Router/Collector/RouteCollectorTest.php b/tests/Router/Collector/RouteCollectorTest.php index 3be5c14..9760d3d 100644 --- a/tests/Router/Collector/RouteCollectorTest.php +++ b/tests/Router/Collector/RouteCollectorTest.php @@ -7,7 +7,7 @@ use cebe\openapi\Reader; use Membrane\OpenAPIRouter\Exception\CannotProcessOpenAPI; use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; -use Membrane\OpenAPIRouter\Router\ValueObject\Route; +use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; From 88ed2cbb8b98f74a020784a63c6c17fff63b24ac Mon Sep 17 00:00:00 2001 From: John Charman Date: Thu, 29 Jun 2023 09:42:55 +0100 Subject: [PATCH 3/6] Add prioritisation for partially dynamic routes --- src/Router/ValueObject/Route/Path.php | 5 +++++ src/Router/ValueObject/Route/Server.php | 6 ++++++ src/Router/ValueObject/RouteCollection.php | 5 +++-- tests/Router/APIeceOfCakeTest.php | 5 +++-- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/Router/ValueObject/Route/Path.php b/src/Router/ValueObject/Route/Path.php index 5c20a68..e2cd6c4 100644 --- a/src/Router/ValueObject/Route/Path.php +++ b/src/Router/ValueObject/Route/Path.php @@ -27,6 +27,11 @@ public function isDynamic(): bool return $this->url !== $this->regex; } + public function howManyDynamicComponents(): int + { + return substr_count($this->regex, '([^/]+)'); + } + public function isEmpty(): bool { return count($this->operations) === 0; diff --git a/src/Router/ValueObject/Route/Server.php b/src/Router/ValueObject/Route/Server.php index fec943f..1c184c5 100644 --- a/src/Router/ValueObject/Route/Server.php +++ b/src/Router/ValueObject/Route/Server.php @@ -31,6 +31,11 @@ public function isDynamic(): bool return $this->url !== $this->regex; } + public function howManyDynamicComponents(): int + { + return substr_count($this->regex, '([^/]+)'); + } + public function isEmpty(): bool { return count(array_filter($this->paths, fn($p) => !$p->isEmpty())) === 0; @@ -44,6 +49,7 @@ public function isHosted(): bool public function jsonSerialize(): mixed { $filteredPaths = array_filter($this->paths, fn($p) => !$p->isEmpty()); + usort($filteredPaths, fn($p) => $p->howManyDynamicComponents()); $staticPaths = $dynamicPaths = $regex = []; foreach ($filteredPaths as $path) { diff --git a/src/Router/ValueObject/RouteCollection.php b/src/Router/ValueObject/RouteCollection.php index 22c84cb..63903d0 100644 --- a/src/Router/ValueObject/RouteCollection.php +++ b/src/Router/ValueObject/RouteCollection.php @@ -46,9 +46,10 @@ public function __construct( public static function fromServers(Server ...$servers): self { $filteredServers = array_filter($servers, fn($s) => !$s->isEmpty()); - + usort($filteredServers, fn($s) => $s->howManyDynamicComponents()); + $hostedServers = $hostlessServers = []; - foreach ($servers as $server) { + foreach ($filteredServers as $server) { if ($server->isHosted()) { $hostedServers[$server->url] = $server; } else { diff --git a/tests/Router/APIeceOfCakeTest.php b/tests/Router/APIeceOfCakeTest.php index 280996f..5192626 100644 --- a/tests/Router/APIeceOfCakeTest.php +++ b/tests/Router/APIeceOfCakeTest.php @@ -39,9 +39,10 @@ public function itCollectsPathsFromAPIeceOfCake(): RouteCollection // I expect to have the following hostless dynamic routes $hostlessDynamicRoutes = [ - '/{cakeType}/{icing}' => ['get' => 'findDessertByIcing', 'post' => 'addDessertByIcing'], + '/{cakeType}/sponge' => ['get' => 'findSpongeByDesserts'], '/cakes/{icing}' => ['get' => 'findCakesByIcing', 'post' => 'addCakesByIcing'], - '/{cakeType}/sponge' => ['get' => 'findSpongeByDesserts'] + '/{cakeType}/{icing}' => ['get' => 'findDessertByIcing', 'post' => 'addDessertByIcing'], + ]; self::assertSame( $hostlessDynamicRoutes, From 0c29a4cd9bea3cbe78695c2f457550489fe034aa Mon Sep 17 00:00:00 2001 From: John Charman Date: Thu, 29 Jun 2023 09:50:20 +0100 Subject: [PATCH 4/6] Rename namespaces --- src/Console/Service/CacheOpenAPIRoutes.php | 2 +- src/Router/Collector/RouteCollector.php | 6 +++--- src/Router/{ValueObject => }/Route/Path.php | 2 +- src/Router/{ValueObject => }/Route/Route.php | 2 +- src/Router/{ValueObject => }/Route/Server.php | 2 +- src/Router/{ValueObject => }/RouteCollection.php | 4 ++-- src/Router/Router.php | 1 - tests/Console/Commands/CacheOpenAPIRoutesTest.php | 4 ++-- tests/Console/Service/CacheOpenAPIRoutesTest.php | 4 ++-- tests/Router/APIeceOfCakeTest.php | 2 +- tests/Router/Collector/RouteCollectorTest.php | 4 ++-- tests/Router/RouterTest.php | 1 - 12 files changed, 16 insertions(+), 18 deletions(-) rename src/Router/{ValueObject => }/Route/Path.php (93%) rename src/Router/{ValueObject => }/Route/Route.php (81%) rename src/Router/{ValueObject => }/Route/Server.php (97%) rename src/Router/{ValueObject => }/RouteCollection.php (96%) diff --git a/src/Console/Service/CacheOpenAPIRoutes.php b/src/Console/Service/CacheOpenAPIRoutes.php index 50d09b7..1e16759 100644 --- a/src/Console/Service/CacheOpenAPIRoutes.php +++ b/src/Console/Service/CacheOpenAPIRoutes.php @@ -9,7 +9,7 @@ use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; use Membrane\OpenAPIRouter\Reader\OpenAPIFileReader; use Membrane\OpenAPIRouter\Router\Collector\RouteCollector; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; +use Membrane\OpenAPIRouter\Router\RouteCollection; use Psr\Log\LoggerInterface; class CacheOpenAPIRoutes diff --git a/src/Router/Collector/RouteCollector.php b/src/Router/Collector/RouteCollector.php index 0ac5f03..45256a2 100644 --- a/src/Router/Collector/RouteCollector.php +++ b/src/Router/Collector/RouteCollector.php @@ -9,9 +9,9 @@ use cebe\openapi\spec\PathItem; use Membrane\OpenAPIRouter\Exception\CannotProcessOpenAPI; use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; -use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; -use Membrane\OpenAPIRouter\Router\ValueObject\Route\Server as ServerRoute; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; +use Membrane\OpenAPIRouter\Router\Route\Route; +use Membrane\OpenAPIRouter\Router\Route\Server as ServerRoute; +use Membrane\OpenAPIRouter\Router\RouteCollection; class RouteCollector { diff --git a/src/Router/ValueObject/Route/Path.php b/src/Router/Route/Path.php similarity index 93% rename from src/Router/ValueObject/Route/Path.php rename to src/Router/Route/Path.php index e2cd6c4..85307ac 100644 --- a/src/Router/ValueObject/Route/Path.php +++ b/src/Router/Route/Path.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Membrane\OpenAPIRouter\Router\ValueObject\Route; +namespace Membrane\OpenAPIRouter\Router\Route; use JsonSerializable; diff --git a/src/Router/ValueObject/Route/Route.php b/src/Router/Route/Route.php similarity index 81% rename from src/Router/ValueObject/Route/Route.php rename to src/Router/Route/Route.php index 4e3a704..5547f5c 100644 --- a/src/Router/ValueObject/Route/Route.php +++ b/src/Router/Route/Route.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Membrane\OpenAPIRouter\Router\ValueObject\Route; +namespace Membrane\OpenAPIRouter\Router\Route; class Route { diff --git a/src/Router/ValueObject/Route/Server.php b/src/Router/Route/Server.php similarity index 97% rename from src/Router/ValueObject/Route/Server.php rename to src/Router/Route/Server.php index 1c184c5..362d6ab 100644 --- a/src/Router/ValueObject/Route/Server.php +++ b/src/Router/Route/Server.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Membrane\OpenAPIRouter\Router\ValueObject\Route; +namespace Membrane\OpenAPIRouter\Router\Route; use JsonSerializable; diff --git a/src/Router/ValueObject/RouteCollection.php b/src/Router/RouteCollection.php similarity index 96% rename from src/Router/ValueObject/RouteCollection.php rename to src/Router/RouteCollection.php index 63903d0..d255ac4 100644 --- a/src/Router/ValueObject/RouteCollection.php +++ b/src/Router/RouteCollection.php @@ -2,9 +2,9 @@ declare(strict_types=1); -namespace Membrane\OpenAPIRouter\Router\ValueObject; +namespace Membrane\OpenAPIRouter\Router; -use Membrane\OpenAPIRouter\Router\ValueObject\Route\Server; +use Membrane\OpenAPIRouter\Router\Route\Server; class RouteCollection { diff --git a/src/Router/Router.php b/src/Router/Router.php index 66a5e38..eec3b0f 100644 --- a/src/Router/Router.php +++ b/src/Router/Router.php @@ -5,7 +5,6 @@ namespace Membrane\OpenAPIRouter\Router; use Membrane\OpenAPIRouter\Exception\CannotRouteRequest; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; class Router { diff --git a/tests/Console/Commands/CacheOpenAPIRoutesTest.php b/tests/Console/Commands/CacheOpenAPIRoutesTest.php index 48336c7..1222e12 100644 --- a/tests/Console/Commands/CacheOpenAPIRoutesTest.php +++ b/tests/Console/Commands/CacheOpenAPIRoutesTest.php @@ -8,8 +8,8 @@ use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; use Membrane\OpenAPIRouter\Reader\OpenAPIFileReader; use Membrane\OpenAPIRouter\Router\Collector\RouteCollector; -use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; +use Membrane\OpenAPIRouter\Router\Route\Route; +use Membrane\OpenAPIRouter\Router\RouteCollection; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamDirectory; use PHPUnit\Framework\Attributes\CoversClass; diff --git a/tests/Console/Service/CacheOpenAPIRoutesTest.php b/tests/Console/Service/CacheOpenAPIRoutesTest.php index fd570a7..2b36a49 100644 --- a/tests/Console/Service/CacheOpenAPIRoutesTest.php +++ b/tests/Console/Service/CacheOpenAPIRoutesTest.php @@ -9,8 +9,8 @@ use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; use Membrane\OpenAPIRouter\Reader\OpenAPIFileReader; use Membrane\OpenAPIRouter\Router\Collector\RouteCollector; -use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; +use Membrane\OpenAPIRouter\Router\Route\Route; +use Membrane\OpenAPIRouter\Router\RouteCollection; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamDirectory; use PHPUnit\Framework\Attributes\CoversClass; diff --git a/tests/Router/APIeceOfCakeTest.php b/tests/Router/APIeceOfCakeTest.php index 5192626..02b26af 100644 --- a/tests/Router/APIeceOfCakeTest.php +++ b/tests/Router/APIeceOfCakeTest.php @@ -4,8 +4,8 @@ use Membrane\OpenAPIRouter\Reader\OpenAPIFileReader; use Membrane\OpenAPIRouter\Router\Collector\RouteCollector; +use Membrane\OpenAPIRouter\Router\RouteCollection; use Membrane\OpenAPIRouter\Router\Router; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Depends; use PHPUnit\Framework\Attributes\Test; diff --git a/tests/Router/Collector/RouteCollectorTest.php b/tests/Router/Collector/RouteCollectorTest.php index 9760d3d..c4072e3 100644 --- a/tests/Router/Collector/RouteCollectorTest.php +++ b/tests/Router/Collector/RouteCollectorTest.php @@ -7,8 +7,8 @@ use cebe\openapi\Reader; use Membrane\OpenAPIRouter\Exception\CannotProcessOpenAPI; use Membrane\OpenAPIRouter\Exception\CannotRouteOpenAPI; -use Membrane\OpenAPIRouter\Router\ValueObject\Route\Route; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; +use Membrane\OpenAPIRouter\Router\Route\Route; +use Membrane\OpenAPIRouter\Router\RouteCollection; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; diff --git a/tests/Router/RouterTest.php b/tests/Router/RouterTest.php index 9100bd4..8edcaff 100644 --- a/tests/Router/RouterTest.php +++ b/tests/Router/RouterTest.php @@ -5,7 +5,6 @@ namespace Membrane\OpenAPIRouter\Router; use Membrane\OpenAPIRouter\Exception\CannotRouteRequest; -use Membrane\OpenAPIRouter\Router\ValueObject\RouteCollection; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; From 963f06044d1b49153d87a17d739a0b4b00e94ed5 Mon Sep 17 00:00:00 2001 From: John Charman Date: Thu, 29 Jun 2023 10:03:12 +0100 Subject: [PATCH 5/6] Appease PHPStan --- src/Router/Route/Path.php | 1 + src/Router/Route/Server.php | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Router/Route/Path.php b/src/Router/Route/Path.php index 85307ac..77b89fd 100644 --- a/src/Router/Route/Path.php +++ b/src/Router/Route/Path.php @@ -37,6 +37,7 @@ public function isEmpty(): bool return count($this->operations) === 0; } + /** @return array */ public function jsonSerialize(): array { return [...$this->operations]; diff --git a/src/Router/Route/Server.php b/src/Router/Route/Server.php index 362d6ab..c622ca4 100644 --- a/src/Router/Route/Server.php +++ b/src/Router/Route/Server.php @@ -46,7 +46,12 @@ public function isHosted(): bool return parse_url($this->url, PHP_URL_HOST) !== null; } - public function jsonSerialize(): mixed + /** @return array{ + * 'static': array>, + * 'dynamic': array{'regex': string, 'paths': array>} + * } + */ + public function jsonSerialize(): array { $filteredPaths = array_filter($this->paths, fn($p) => !$p->isEmpty()); usort($filteredPaths, fn($p) => $p->howManyDynamicComponents()); From 198eff0cba3998eaa1533299fb848c97fee06270 Mon Sep 17 00:00:00 2001 From: John Charman Date: Thu, 29 Jun 2023 10:31:15 +0100 Subject: [PATCH 6/6] Fix dynamic comparison --- src/Router/Route/Server.php | 2 +- src/Router/RouteCollection.php | 2 +- tests/Router/APIeceOfCakeTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Router/Route/Server.php b/src/Router/Route/Server.php index c622ca4..b04ae6d 100644 --- a/src/Router/Route/Server.php +++ b/src/Router/Route/Server.php @@ -54,7 +54,7 @@ public function isHosted(): bool public function jsonSerialize(): array { $filteredPaths = array_filter($this->paths, fn($p) => !$p->isEmpty()); - usort($filteredPaths, fn($p) => $p->howManyDynamicComponents()); + usort($filteredPaths, fn($a, $b) => $a->howManyDynamicComponents() <=> $b->howManyDynamicComponents()); $staticPaths = $dynamicPaths = $regex = []; foreach ($filteredPaths as $path) { diff --git a/src/Router/RouteCollection.php b/src/Router/RouteCollection.php index d255ac4..92d6c9d 100644 --- a/src/Router/RouteCollection.php +++ b/src/Router/RouteCollection.php @@ -46,7 +46,7 @@ public function __construct( public static function fromServers(Server ...$servers): self { $filteredServers = array_filter($servers, fn($s) => !$s->isEmpty()); - usort($filteredServers, fn($s) => $s->howManyDynamicComponents()); + usort($filteredServers, fn($a, $b) => $a->howManyDynamicComponents() <=> $b->howManyDynamicComponents()); $hostedServers = $hostlessServers = []; foreach ($filteredServers as $server) { diff --git a/tests/Router/APIeceOfCakeTest.php b/tests/Router/APIeceOfCakeTest.php index 02b26af..4c44762 100644 --- a/tests/Router/APIeceOfCakeTest.php +++ b/tests/Router/APIeceOfCakeTest.php @@ -39,8 +39,8 @@ public function itCollectsPathsFromAPIeceOfCake(): RouteCollection // I expect to have the following hostless dynamic routes $hostlessDynamicRoutes = [ - '/{cakeType}/sponge' => ['get' => 'findSpongeByDesserts'], '/cakes/{icing}' => ['get' => 'findCakesByIcing', 'post' => 'addCakesByIcing'], + '/{cakeType}/sponge' => ['get' => 'findSpongeByDesserts'], '/{cakeType}/{icing}' => ['get' => 'findDessertByIcing', 'post' => 'addDessertByIcing'], ];