From 9bff2f8abac0904cbdb3bc54332b3511e54b0c4c Mon Sep 17 00:00:00 2001 From: John Charman Date: Mon, 11 Sep 2023 15:36:11 +0100 Subject: [PATCH 1/2] Add infection/infection --- composer.json | 8 +- infection.json5 | 11 + src/Route/Path.php | 11 +- src/Route/Route.php | 16 -- src/Route/Server.php | 23 +- src/RouteCollection.php | 8 +- src/RouteCollector.php | 60 ++--- src/Router.php | 2 - .../Command/CacheOpenAPIRoutesTest.php | 2 +- .../Service/CacheOpenAPIRoutesTest.php | 2 +- tests/Exception/CannotRouteRequestTest.php | 32 +++ tests/Route/PathTest.php | 79 +++++++ tests/Route/ServerTest.php | 30 +++ tests/RouteCollectorTest.php | 2 +- tests/RouterTest.php | 209 ++++++++++++++---- 15 files changed, 369 insertions(+), 126 deletions(-) create mode 100644 infection.json5 delete mode 100644 src/Route/Route.php create mode 100644 tests/Exception/CannotRouteRequestTest.php create mode 100644 tests/Route/PathTest.php create mode 100644 tests/Route/ServerTest.php diff --git a/composer.json b/composer.json index ff07687..61d850f 100644 --- a/composer.json +++ b/composer.json @@ -18,6 +18,7 @@ "symfony/console": "^6.2" }, "require-dev": { + "infection/infection": "^0.27.0", "phpunit/phpunit": "^10.2", "phpstan/phpstan": "^1.8", "squizlabs/php_codesniffer": "^3.7", @@ -26,5 +27,10 @@ }, "bin": [ "bin/membrane-router" - ] + ], + "config": { + "allow-plugins": { + "infection/extension-installer": true + } + } } diff --git a/infection.json5 b/infection.json5 new file mode 100644 index 0000000..ca97ca2 --- /dev/null +++ b/infection.json5 @@ -0,0 +1,11 @@ +{ + "$schema": "vendor/infection/infection/resources/schema.json", + "source": { + "directories": [ + "src" + ] + }, + "mutators": { + "@default": true + } +} \ No newline at end of file diff --git a/src/Route/Path.php b/src/Route/Path.php index 789a7ae..ea238f1 100644 --- a/src/Route/Path.php +++ b/src/Route/Path.php @@ -8,18 +8,21 @@ final class Path implements JsonSerializable { + public readonly string $regex; /** @var array */ private array $operations = []; public function __construct( - public readonly string $url, - public readonly string $regex, + public readonly string $url ) { + $regex = preg_replace('#{[^/]+}#', '([^/]+)', $this->url); + assert(is_string($regex)); + $this->regex = $regex; } - public function addRoute(Route $route): void + public function addRoute(string $method, string $operationId): void { - $this->operations[$route->method] = $route->operationId; + $this->operations[$method] = $operationId; } public function isDynamic(): bool diff --git a/src/Route/Route.php b/src/Route/Route.php deleted file mode 100644 index 66901e5..0000000 --- a/src/Route/Route.php +++ /dev/null @@ -1,16 +0,0 @@ -*/ private array $paths = []; public function __construct( - public readonly string $url, - public readonly string $regex, + public readonly string $url ) { + $regex = preg_replace('#{[^/]+}#', '([^/]+)', $this->url); + assert(is_string($regex)); + $this->regex = $regex; } - public function addRoute(Route $route): void + public function addRoute(string $pathUrl, string $method, string $operationId): void { - if (!isset($this->paths[$route->path])) { - $this->addPath(new Path($route->path, $route->pathRegex)); + if (!isset($this->paths[$pathUrl])) { + $this->paths[$pathUrl] = new Path($pathUrl); } - $this->paths[$route->path]->addRoute($route); + $this->paths[$pathUrl]->addRoute($method, $operationId); } public function isDynamic(): bool @@ -77,11 +81,4 @@ public function jsonSerialize(): array ] ]; } - - private function addPath(Path $path): void - { - if (!isset($this->paths[$path->url])) { - $this->paths[$path->url] = $path; - } - } } diff --git a/src/RouteCollection.php b/src/RouteCollection.php index 941fb5c..90d305c 100644 --- a/src/RouteCollection.php +++ b/src/RouteCollection.php @@ -6,7 +6,7 @@ use Membrane\OpenAPIRouter\Route\Server; -class RouteCollection +final class RouteCollection { /** * @param array{ @@ -45,14 +45,14 @@ public function __construct( public static function fromServers(Server ...$servers): self { - $filteredServers = array_filter($servers, fn($s) => !$s->isEmpty()); + usort($servers, fn(Server $a, Server $b) => strlen($b->regex) <=> strlen($a->regex)); usort( - $filteredServers, + $servers, fn(Server $a, Server $b) => $a->howManyDynamicComponents() <=> $b->howManyDynamicComponents() ); $hostedServers = $hostlessServers = []; - foreach ($filteredServers as $server) { + foreach ($servers as $server) { if ($server->isHosted()) { $hostedServers[$server->url] = $server; } else { diff --git a/src/RouteCollector.php b/src/RouteCollector.php index e8f7220..78c0a43 100644 --- a/src/RouteCollector.php +++ b/src/RouteCollector.php @@ -8,8 +8,7 @@ use cebe\openapi\spec\Operation; use cebe\openapi\spec\PathItem; use Membrane\OpenAPIRouter\Exception\CannotCollectRoutes; -use Membrane\OpenAPIRouter\Route\Route; -use Membrane\OpenAPIRouter\Route\Server as ServerRoute; +use Membrane\OpenAPIRouter\Route\Server; class RouteCollector { @@ -24,44 +23,20 @@ public function collect(OpenApi $openApi): RouteCollection return RouteCollection::fromServers(...$collection); } - /** @return array */ + /** @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 ($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); - } - - if ($operationServers !== []) { - $servers = $operationServers; - } elseif ($pathServers !== []) { - $servers = $pathServers; - } else { - $servers = $rootServers; + $servers = $this->getServers($openApi, $pathObject, $operationObject); + foreach ($servers as $serverUrl) { + if (!isset($collection[$serverUrl])) { + $collection[$serverUrl] = new Server($serverUrl); + } + $collection[$serverUrl]->addRoute($path, $method, $operationObject->operationId); } - - foreach ($servers as $url => $regex) { - $collection[$url]->addRoute(new Route($path, $pathRegex, $method, $operationObject->operationId)); - } - - $operationIds[$operationObject->operationId] = ['path' => $path, 'operation' => $method]; } } @@ -69,17 +44,16 @@ private function collectRoutes(OpenApi $openApi): array } /** @return array */ - private function getServers(OpenApi|PathItem|Operation $object): array + private function getServers(OpenApi $openAPI, PathItem $path, Operation $operation): 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 + if ($operation->servers !== []) { + $servers = $operation->servers; + } elseif ($path->servers !== []) { + $servers = $path->servers; + } else { + $servers = $openAPI->servers; + } - return $regex; + return array_unique(array_map(fn($p) => rtrim($p->url, '/'), $servers)); } } diff --git a/src/Router.php b/src/Router.php index e3c0289..1777003 100644 --- a/src/Router.php +++ b/src/Router.php @@ -52,8 +52,6 @@ private function routeServer(array $servers, string $url, string $method): ?stri { // Check static servers first $staticServers = $servers['static']; - // Prioritize server names of greater length - uksort($staticServers, fn($a, $b) => strlen($a) <=> strlen($b)); foreach ($staticServers as $staticServer => $paths) { if (str_starts_with($url, $staticServer)) { diff --git a/tests/Console/Command/CacheOpenAPIRoutesTest.php b/tests/Console/Command/CacheOpenAPIRoutesTest.php index 5b0a486..e21ded4 100644 --- a/tests/Console/Command/CacheOpenAPIRoutesTest.php +++ b/tests/Console/Command/CacheOpenAPIRoutesTest.php @@ -22,7 +22,7 @@ #[CoversClass(CacheOpenAPIRoutes::class)] #[CoversClass(Exception\CannotCollectRoutes::class)] #[UsesClass(Service\CacheOpenAPIRoutes::class)] -#[UsesClass(Route\Route::class), UsesClass(Route\Server::class), UsesClass(Route\Path::class)] +#[UsesClass(Route\Server::class), UsesClass(Route\Path::class)] #[UsesClass(RouteCollection::class)] #[UsesClass(RouteCollector::class)] class CacheOpenAPIRoutesTest extends TestCase diff --git a/tests/Console/Service/CacheOpenAPIRoutesTest.php b/tests/Console/Service/CacheOpenAPIRoutesTest.php index b32c0c9..8750078 100644 --- a/tests/Console/Service/CacheOpenAPIRoutesTest.php +++ b/tests/Console/Service/CacheOpenAPIRoutesTest.php @@ -20,7 +20,7 @@ #[CoversClass(CacheOpenAPIRoutes::class)] #[CoversClass(CannotCollectRoutes::class)] #[UsesClass(RouteCollector::class)] -#[UsesClass(Route\Route::class), UsesClass(Route\Server::class), UsesClass(Route\Path::class)] +#[UsesClass(Route\Server::class), UsesClass(Route\Path::class)] #[UsesClass(RouteCollection::class)] class CacheOpenAPIRoutesTest extends TestCase { diff --git a/tests/Exception/CannotRouteRequestTest.php b/tests/Exception/CannotRouteRequestTest.php new file mode 100644 index 0000000..a4fa5fb --- /dev/null +++ b/tests/Exception/CannotRouteRequestTest.php @@ -0,0 +1,32 @@ + [404, CannotRouteRequest::notFound()]; + yield '405' => [405, CannotRouteRequest::methodNotAllowed()]; + } + + #[Test] + #[DataProvider('provideErrorCodes')] + public function itConstructsFromErrorCodes(int $errorCode, CannotRouteRequest $expectedException): void + { + $actualException = CannotRouteRequest::fromErrorCode($errorCode); + + self::assertEquals($expectedException, CannotRouteRequest::fromErrorCode($errorCode)); + self::assertSame($errorCode, $actualException->getCode()); + } +} diff --git a/tests/Route/PathTest.php b/tests/Route/PathTest.php new file mode 100644 index 0000000..4d4be43 --- /dev/null +++ b/tests/Route/PathTest.php @@ -0,0 +1,79 @@ + ['/static/path', '/static/path'], + 'partially dynamic path' => ['/([^/]+)/path', '/{dynamic}/path'], + 'dynamic path' => ['/([^/]+)/([^/]+)', '/{dynamic}/{path}}'] + ]; + } + + #[Test] + #[DataProvider('provideUrlsToBaseRegexOn')] + public function itHasARegexBasedOnTheUrl(string $expected, string $url): void + { + self::assertSame($expected, (new Path($url))->regex); + } + + public static function provideUrlsToCheckIfDynamic(): array + { + return [ + 'static path' => [false, '/static/path'], + 'partially dynamic path' => [true, '/{dynamic}/path'], + 'dynamic path' => [true, '/{dynamic}/{path}}'] + ]; + } + + #[Test] + #[DataProvider('provideUrlsToCheckIfDynamic')] + public function itCanTellIfItIsDynamic(bool $expected, string $url): void + { + $sut = new Path($url); + + self::assertSame($expected, $sut->isDynamic()); + } + + public static function provideUrlsToCountDynamicComponents(): array + { + return [ + 'static path' => [0, '/static/path'], + 'partially dynamic path' => [1, '/{dynamic}/path'], + 'dynamic path' => [2, '/{dynamic}/{path}}'] + ]; + } + + #[Test] + #[DataProvider('provideUrlsToCountDynamicComponents')] + public function itCanCountDynamicComponents(int $expected, string $url): void + { + $sut = new Path($url); + + self::assertSame($expected, $sut->howManyDynamicComponents()); + } + + #[Test] + public function itCanAddRoutes(): void + { + $sut = new Path('/path'); + + self::assertTrue($sut->isEmpty()); + + $sut->addRoute('get', 'get-operation-id'); + + self::assertFalse($sut->isEmpty()); + } +} diff --git a/tests/Route/ServerTest.php b/tests/Route/ServerTest.php new file mode 100644 index 0000000..b324666 --- /dev/null +++ b/tests/Route/ServerTest.php @@ -0,0 +1,30 @@ + ['https://test.com/static', 'https://test.com/static'], + 'partially dynamic server' => ['https://test.com/([^/]+)', 'https://test.com/{dynamic}'], + ]; + } + + #[Test] + #[DataProvider('provideUrls')] + public function itCreatesARegexBasedOnTheUrl(string $expectedRegex, string $url): void + { + self::assertSame($expectedRegex, (new Server($url))->regex); + } +} diff --git a/tests/RouteCollectorTest.php b/tests/RouteCollectorTest.php index 3418059..2db182d 100644 --- a/tests/RouteCollectorTest.php +++ b/tests/RouteCollectorTest.php @@ -20,7 +20,7 @@ #[CoversClass(RouteCollector::class)] #[CoversClass(CannotCollectRoutes::class)] -#[UsesClass(Route\Route::class), UsesClass(Route\Server::class), UsesClass(Route\Path::class)] +#[UsesClass(Route\Server::class), UsesClass(Route\Path::class)] #[UsesClass(RouteCollection::class)] class RouteCollectorTest extends TestCase { diff --git a/tests/RouterTest.php b/tests/RouterTest.php index b8abd2c..8bbc1bf 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -5,9 +5,11 @@ namespace Membrane\OpenAPIRouter\Tests; use Generator; +use Membrane\OpenAPIReader\FileFormat; use Membrane\OpenAPIReader\OpenAPIVersion; use Membrane\OpenAPIReader\Reader; -use Membrane\OpenAPIRouter\Exception\CannotRouteRequest; +use Membrane\OpenAPIRouter\Exception; +use Membrane\OpenAPIRouter\Route; use Membrane\OpenAPIRouter\RouteCollection; use Membrane\OpenAPIRouter\RouteCollector; use Membrane\OpenAPIRouter\Router; @@ -15,10 +17,14 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\TestDox; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[CoversClass(Router::class)] -#[CoversClass(CannotRouteRequest::class)] +#[CoversClass(Exception\CannotRouteRequest::class)] +#[UsesClass(RouteCollection::class)] +#[UsesClass(RouteCollector::class)] +#[UsesClass(Route\Path::class), UsesClass(Route\Server::class)] class RouterTest extends TestCase { private const FIXTURES = __DIR__ . '/fixtures/'; @@ -51,25 +57,25 @@ public static function unsuccessfulRouteProvider(): array { return [ 'petstore-expanded: incorrect server url' => [ - CannotRouteRequest::notFound(), + Exception\CannotRouteRequest::notFound(), 'https://hatshop.dapper.net/api/pets', 'get', self::getPetStoreRouteCollection(), ], 'petstore-expanded: correct static server url but incorrect path' => [ - CannotRouteRequest::notFound(), + Exception\CannotRouteRequest::notFound(), 'http://petstore.swagger.io/api/hats', 'get', self::getPetStoreRouteCollection(), ], 'WeirdAndWonderful: correct dynamic erver url but incorrect path' => [ - CannotRouteRequest::notFound(), + Exception\CannotRouteRequest::notFound(), 'http://weird.io/however/but', 'get', self::getWeirdAndWonderfulRouteCollection(), ], 'petstore-expanded: correct url but incorrect method' => [ - CannotRouteRequest::methodNotAllowed(), + Exception\CannotRouteRequest::methodNotAllowed(), 'http://petstore.swagger.io/api/pets', 'delete', self::getPetStoreRouteCollection(), @@ -80,7 +86,7 @@ public static function unsuccessfulRouteProvider(): array #[Test] #[DataProvider('unsuccessfulRouteProvider')] public function unsuccessfulRouteTest( - CannotRouteRequest $expected, + Exception\CannotRouteRequest $expected, string $path, string $method, RouteCollection $operationCollection @@ -140,49 +146,172 @@ public static function successfulRouteProvider(): array ]; } - #[Test] - #[DataProvider('successfulRouteProvider')] - public function successfulRouteTest( - string $expected, - string $path, - string $method, - RouteCollection $operationCollection - ): void { - $sut = new Router($operationCollection); + public static function providePathsToPrioritise(): Generator + { + $operation = fn(string $operationId) => [ + 'operationId' => $operationId, + 'responses' => [200 => ['description' => '']] + ]; + $pathParameter = fn(string $name) => [ + 'name' => $name, + 'in' => 'path', + 'required' => true, + 'schema' => ['type' => 'string'] + ]; + + $testCases = [ + 'static > dynamic' => [ + '/static/path', + [ + '/static/path' => ['get' => $operation('first')], + '/{dynamic}/{path}' => [ + 'get' => $operation('second'), + 'parameters' => [$pathParameter('dynamic'), $pathParameter('path')] + ] + ] + ], + 'partially dynamic > completely dynamic' => [ + '/which/path', + [ + '/{partially-dynamic}/path' => [ + 'get' => $operation('first'), + 'parameters' => [$pathParameter('partially-dynamic')] + ], + '/{dynamic}/{path}' => [ + 'get' => $operation('second'), + 'parameters' => [$pathParameter('dynamic'), $pathParameter('path')] + ], + ] + ], + 'less dynamic components > more dynamic components' => [ + '/which/path/to/pick', + [ + '/{dynamic}/path/to/pick' => [ + 'get' => $operation('first'), + 'parameters' => [$pathParameter('partially-dynamic')] + ], + '/{dynamic}/{path}/to/pick' => [ + 'get' => $operation('second'), + 'parameters' => [$pathParameter('dynamic'), $pathParameter('path')] + ], + ] + ] + ]; - $actual = $sut->route($path, $method); + $openAPI = fn(array $paths) => json_encode([ + 'openapi' => '3.0.0', + 'info' => ['title' => '', 'version' => '1.0.0'], + 'paths' => $paths, + ]); - self::assertSame($expected, $actual); + foreach ($testCases as $description => $testCase) { + yield $description => [$testCase[0], $openAPI($testCase[1])]; + yield "$description (order of paths reversed)" => [$testCase[0], $openAPI(array_reverse($testCase[1]))]; + } } - public static function provideRoutingPriorities(): Generator + #[Test, TestDox('Relative urls are prioritised based on the number of dynamic components they have, least first')] + #[DataProvider('providePathsToPrioritise')] + public function itPrioritisesPathsCorrectly(string $url, string $openAPI): void { - yield 'completely static path prioritise over anything dynamic' => [ - 'findSpongeCakes', - '/cakes/sponge', - 'get', - self::getAPieceOfCakeRouteCollection() + $openAPI = (new Reader([OpenAPIVersion::Version_3_0])) + ->readFromString($openAPI, FileFormat::Json); + + $routeCollection = (new RouteCollector()) + ->collect($openAPI); + + $priority = (new Router($routeCollection)) + ->route($url, 'get'); + + self::assertSame('first', $priority); + } + + public static function provideServersToPrioritise(): Generator + { + $minimalPath = fn(string $operationId) => [ + 'get' => [ + 'operationId' => $operationId, + 'responses' => [200 => ['description' => '']] + ] ]; - yield 'partially dynamic path to prioritise over anything with more dynamic parts' => [ - 'findCakesByIcing', - '/cakes/chocolate', - 'get', - self::getAPieceOfCakeRouteCollection() + + $testCases = [ + 'longer > shorter' => [ + 'http://this/path/please', + [['url' => 'http://this'], ['url' => 'http://this/path']], + ['/please' => $minimalPath('first'), '/path/please' => $minimalPath('second')] + ], + 'static > dynamic' => [ + 'http://this/path/please', + [ + ['url' => 'http://this'], + [ + 'url' => 'http://{demonstrative-pronoun}/path', + 'variables' => ['demonstrative-pronoun' => ['default' => 'this']] + ] + ], + ['/path/please' => $minimalPath('first'), '/please' => $minimalPath('second')] + ], + 'less dynamic components > more dynamic components' => [ + 'http://this/path/pretty/please', + [ + [ + 'url' => 'http://{demonstrative-pronoun}/{route}/pretty', + 'variables' => [ + 'demonstrative-pronoun' => ['default' => 'this'], + 'route' => ['default' => 'path'] + ] + ], + [ + 'url' => 'http://{demonstrative-pronoun}/path', + 'variables' => ['demonstrative-pronoun' => ['default' => 'this']] + ], + ], + ['/pretty/please' => $minimalPath('first'), '/please' => $minimalPath('second')] + ] + ]; + + $openAPI = fn(array $servers, array $paths) => json_encode([ + 'openapi' => '3.0.0', + 'info' => ['title' => '', 'version' => '1.0.0'], + 'servers' => $servers, + 'paths' => $paths, + ]); + + foreach ($testCases as $description => $testCase) { + yield $description => [ + $testCase[0], + $openAPI($testCase[1], $testCase[2]) + ]; + yield "$description (order of servers reversed)" => [ + $testCase[0], + $openAPI(array_reverse($testCase[1]), $testCase[2]) + ]; + yield "$description (order of paths reversed)" => [ + $testCase[0], + $openAPI($testCase[1], array_reverse($testCase[2])) + ]; + yield "$description (order of paths and servers reversed)" => [ + $testCase[0], + $openAPI(array_reverse($testCase[1]), array_reverse($testCase[2])) + ]; + } } - #[Test, TestDox('When routing the priority will be paths with less dynamic components first')] - #[DataProvider('provideRoutingPriorities')] - public function itWillPrioritiseRoutesWithMoreStaticComponentsFirst( - string $expectedOperationId, - string $url, - string $method, - RouteCollection $routeCollection - ): void { - $sut = new Router($routeCollection); + #[Test, TestDox('Servers are prioritised by length and number of dynamic components')] + #[DataProvider('provideServersToPrioritise')] + public function itPrioritisesServersCorrectly(string $url, string $openAPI): void + { + $openAPI = (new Reader([OpenAPIVersion::Version_3_0])) + ->readFromString($openAPI, FileFormat::Json); + + $routeCollection = (new RouteCollector()) + ->collect($openAPI); - $actualOperationId = $sut->route($url, $method); + $priority = (new Router($routeCollection)) + ->route($url, 'get'); - self::assertSame($expectedOperationId, $actualOperationId); + self::assertSame('first', $priority); } } From bb843ea66b8e6f214969968ef3cfb89e64a373ee Mon Sep 17 00:00:00 2001 From: John Charman Date: Mon, 11 Sep 2023 19:50:45 +0100 Subject: [PATCH 2/2] Improve tests --- src/Exception/CannotRouteRequest.php | 17 ++-- src/Route/Path.php | 4 +- src/Route/Server.php | 8 +- src/RouteCollector.php | 20 ++--- tests/Route/PathTest.php | 62 ++++++++++++-- tests/Route/ServerTest.php | 116 ++++++++++++++++++++++++- tests/RouteCollectionTest.php | 121 +++++++++++++++++++++++++++ tests/RouteCollectorTest.php | 29 +++++++ 8 files changed, 339 insertions(+), 38 deletions(-) create mode 100644 tests/RouteCollectionTest.php diff --git a/src/Exception/CannotRouteRequest.php b/src/Exception/CannotRouteRequest.php index dfafaed..73d3e61 100644 --- a/src/Exception/CannotRouteRequest.php +++ b/src/Exception/CannotRouteRequest.php @@ -6,21 +6,20 @@ /* This exception occurs when the request does not match any routes in your route collection. */ -class CannotRouteRequest extends \RuntimeException +use RuntimeException; + +class CannotRouteRequest extends RuntimeException { public const NOT_FOUND = 404; public const METHOD_NOT_ALLOWED = 405; public static function fromErrorCode(int $errorCode): self { - switch ($errorCode) { - case self::NOT_FOUND: - return self::notFound(); - case self::METHOD_NOT_ALLOWED: - return self::methodNotAllowed(); - default: - return new self(); - } + assert($errorCode === 404 || $errorCode === 405); + return match ($errorCode) { + self::NOT_FOUND => self::notFound(), + self::METHOD_NOT_ALLOWED => self::methodNotAllowed(), + }; } public static function notFound(): self diff --git a/src/Route/Path.php b/src/Route/Path.php index ea238f1..fca88db 100644 --- a/src/Route/Path.php +++ b/src/Route/Path.php @@ -43,6 +43,8 @@ public function isEmpty(): bool /** @return array */ public function jsonSerialize(): array { - return [...$this->operations]; + $operations = $this->operations; + ksort($operations); + return $operations; } } diff --git a/src/Route/Server.php b/src/Route/Server.php index 70162ae..dd54f99 100644 --- a/src/Route/Server.php +++ b/src/Route/Server.php @@ -42,7 +42,7 @@ public function howManyDynamicComponents(): int public function isEmpty(): bool { - return count(array_filter($this->paths, fn($p) => !$p->isEmpty())) === 0; + return count($this->paths) === 0; } public function isHosted(): bool @@ -57,14 +57,14 @@ public function isHosted(): bool */ public function jsonSerialize(): array { - $filteredPaths = array_filter($this->paths, fn($p) => !$p->isEmpty()); + $paths = $this->paths; usort( - $filteredPaths, + $paths, fn(Path $a, Path $b) => $a->howManyDynamicComponents() <=> $b->howManyDynamicComponents() ); $staticPaths = $dynamicPaths = $regex = []; - foreach ($filteredPaths as $path) { + foreach ($paths as $path) { if ($path->isDynamic()) { $dynamicPaths[$path->url] = $path->jsonSerialize(); $regex[] = sprintf('%s(*MARK:%s)', $path->regex, $path->url); diff --git a/src/RouteCollector.php b/src/RouteCollector.php index 78c0a43..13080d9 100644 --- a/src/RouteCollector.php +++ b/src/RouteCollector.php @@ -13,18 +13,6 @@ class RouteCollector { public function collect(OpenApi $openApi): RouteCollection - { - $collection = $this->collectRoutes($openApi); - - if ($collection === []) { - throw CannotCollectRoutes::noRoutes(); - } - - return RouteCollection::fromServers(...$collection); - } - - /** @return array */ - private function collectRoutes(OpenApi $openApi): array { $collection = []; @@ -40,7 +28,11 @@ private function collectRoutes(OpenApi $openApi): array } } - return array_filter($collection, fn($s) => !$s->isEmpty()); + if ($collection === []) { + throw CannotCollectRoutes::noRoutes(); + } + + return RouteCollection::fromServers(...$collection); } /** @return array */ @@ -54,6 +46,6 @@ private function getServers(OpenApi $openAPI, PathItem $path, Operation $operati $servers = $openAPI->servers; } - return array_unique(array_map(fn($p) => rtrim($p->url, '/'), $servers)); + return array_map(fn($p) => rtrim($p->url, '/'), $servers); } } diff --git a/tests/Route/PathTest.php b/tests/Route/PathTest.php index 4d4be43..ed0f034 100644 --- a/tests/Route/PathTest.php +++ b/tests/Route/PathTest.php @@ -4,6 +4,7 @@ namespace Membrane\OpenAPIRouter\Tests\Route; +use Generator; use Membrane\OpenAPIRouter\Route\Path; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; @@ -18,7 +19,7 @@ public static function provideUrlsToBaseRegexOn(): array return [ 'static path' => ['/static/path', '/static/path'], 'partially dynamic path' => ['/([^/]+)/path', '/{dynamic}/path'], - 'dynamic path' => ['/([^/]+)/([^/]+)', '/{dynamic}/{path}}'] + 'dynamic path' => ['/([^/]+)/([^/]+)', '/{dynamic}/{path}'] ]; } @@ -34,7 +35,7 @@ public static function provideUrlsToCheckIfDynamic(): array return [ 'static path' => [false, '/static/path'], 'partially dynamic path' => [true, '/{dynamic}/path'], - 'dynamic path' => [true, '/{dynamic}/{path}}'] + 'dynamic path' => [true, '/{dynamic}/{path}'] ]; } @@ -42,9 +43,7 @@ public static function provideUrlsToCheckIfDynamic(): array #[DataProvider('provideUrlsToCheckIfDynamic')] public function itCanTellIfItIsDynamic(bool $expected, string $url): void { - $sut = new Path($url); - - self::assertSame($expected, $sut->isDynamic()); + self::assertSame($expected, (new Path($url))->isDynamic()); } public static function provideUrlsToCountDynamicComponents(): array @@ -52,7 +51,7 @@ public static function provideUrlsToCountDynamicComponents(): array return [ 'static path' => [0, '/static/path'], 'partially dynamic path' => [1, '/{dynamic}/path'], - 'dynamic path' => [2, '/{dynamic}/{path}}'] + 'dynamic path' => [2, '/{dynamic}/{path}'] ]; } @@ -60,9 +59,7 @@ public static function provideUrlsToCountDynamicComponents(): array #[DataProvider('provideUrlsToCountDynamicComponents')] public function itCanCountDynamicComponents(int $expected, string $url): void { - $sut = new Path($url); - - self::assertSame($expected, $sut->howManyDynamicComponents()); + self::assertSame($expected, (new Path($url))->howManyDynamicComponents()); } #[Test] @@ -76,4 +73,51 @@ public function itCanAddRoutes(): void self::assertFalse($sut->isEmpty()); } + + public static function providePathsToJsonSerialize(): Generator + { + $expected = [ + 'delete' => 'delete-operation', + 'get' => 'get-operation', + 'post' => 'post-operation' + ]; + + $operations = [ + ['get', 'get-operation'], + ['post', 'post-operation'], + ['delete', 'delete-operation'], + ]; + + yield [ + $expected, + (function() use ($operations) { + $sut = new Path('/path'); + foreach ($operations as $operation) { + $sut->addRoute(...$operation); + } + return $sut; + })(), + ]; + yield [ + $expected, + (function() use ($operations) { + $sut = new Path('/path'); + foreach (array_reverse($operations) as $operation) { + $sut->addRoute(...$operation); + } + return $sut; + })(), + ]; + } + + #[Test] + public function itIsJsonSerializable(): void + { + $sut = new Path('/path'); + + $sut->addRoute('get', 'get-operation-id'); + $sut->addRoute('post', 'post-operation-id'); + + self::assertSame(['get' => 'get-operation-id', 'post' => 'post-operation-id'], $sut->jsonSerialize()); + } } diff --git a/tests/Route/ServerTest.php b/tests/Route/ServerTest.php index b324666..e306d33 100644 --- a/tests/Route/ServerTest.php +++ b/tests/Route/ServerTest.php @@ -4,13 +4,17 @@ namespace Membrane\OpenAPIRouter\Tests\Route; +use Generator; +use Membrane\OpenAPIRouter\Route\Path; use Membrane\OpenAPIRouter\Route\Server; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[CoversClass(Server::class)] +#[UsesClass(Path::class)] class ServerTest extends TestCase { public static function provideUrls(): array @@ -23,8 +27,118 @@ public static function provideUrls(): array #[Test] #[DataProvider('provideUrls')] - public function itCreatesARegexBasedOnTheUrl(string $expectedRegex, string $url): void + public function itHasARegexBasedOnTheUrl(string $expectedRegex, string $url): void { self::assertSame($expectedRegex, (new Server($url))->regex); } + + public static function provideUrlsToCheckIfDynamic(): array + { + return [ + 'static path' => [false, 'http://static/server'], + 'partially dynamic path' => [true, 'http://{dynamic}/server'], + 'dynamic path' => [true, 'http://{dynamic}/{server}'] + ]; + } + + #[Test] + #[DataProvider('provideUrlsToCheckIfDynamic')] + public function itCanTellIfItIsDynamic(bool $expected, string $url): void + { + self::assertSame($expected, (new Server($url))->isDynamic()); + } + + public static function provideUrlsToCountDynamicComponents(): array + { + return [ + 'static server' => [0, 'http://static/server'], + 'partially dynamic path' => [1, 'http://{dynamic}/server'], + 'dynamic path' => [2, 'http://{dynamic}/{server}'] + ]; + } + + #[Test] + #[DataProvider('provideUrlsToCountDynamicComponents')] + public function itCanCountDynamicComponents(int $expected, string $url): void + { + self::assertSame($expected, (new Server($url))->howManyDynamicComponents()); + } + + #[Test] + public function itCanAddRoutes(): void + { + $sut = new Server('http://server.com'); + + self::assertTrue($sut->isEmpty()); + + $sut->addRoute('/path', 'get', 'get-operation-id'); + + self::assertFalse($sut->isEmpty()); + } + + public static function provideUrlsToCheckIfHosted(): array + { + return [ + 'hostless' => [false, '/hostless/v1'], + 'hosted' => [true, 'http://www.hosted.io'] + ]; + } + + #[Test] + #[DataProvider('provideUrlsToCheckIfHosted')] + public function itCanTellIfItIsHosted(bool $expected, string $url): void + { + self::assertSame($expected, (new Server($url))->isHosted()); + } + + public static function provideServersToJsonSerialize(): Generator + { + $expected = [ + 'static' => ['/path' => ['get' => 'get-path', 'post' => 'post-path']], + 'dynamic' => [ + 'regex' => '#^(?|/([^/]+)/path(*MARK:/{another}/path)|/([^/]+)/([^/]+)/path(*MARK:/{yet}/{another}/path))$#', + 'paths' => [ + '/{another}/path' => ['get' => 'get-another-path'], + '/{yet}/{another}/path' => ['delete' => 'delete-yet-another-path'], + ], + ], + ]; + + $paths = [ + ['/path', 'get', 'get-path'], + ['/path', 'post', 'post-path'], + ['/{another}/path', 'get', 'get-another-path'], + ['/{yet}/{another}/path', 'delete', 'delete-yet-another-path'], + ]; + + + + yield [ + $expected, + (function () use ($paths) { + $server = new Server('www.server.net'); + foreach ($paths as $path) { + $server->addRoute(...$path); + } + return $server; + })() + ]; + yield [ + $expected, + (function () use ($paths) { + $server = new Server('www.servver.net'); + foreach (array_reverse($paths) as $path) { + $server->addRoute(...$path); + } + return $server; + })() + ]; + } + + #[Test] + #[DataProvider('provideServersToJsonSerialize')] + public function itIsJsonSerializable(array $expected, Server $sut): void + { + self::assertSame($expected, $sut->jsonSerialize()); + } } diff --git a/tests/RouteCollectionTest.php b/tests/RouteCollectionTest.php new file mode 100644 index 0000000..6999f32 --- /dev/null +++ b/tests/RouteCollectionTest.php @@ -0,0 +1,121 @@ + [ + 'static' => [ + 'https://www.server.io' => [ + 'static' => [ + '/static/path' => ['post' => 'post-static-path'], + ], + 'dynamic' => [ + 'regex' => '#^(?|/([^/]+)/path(*MARK:/{dynamic}/path)|/([^/]+)/([^/]+)/path(*MARK:/{very}/{dynamic}/path))$#', + 'paths' => [ + '/{dynamic}/path' => ['get' => 'get-dynamic-path'], + '/{very}/{dynamic}/path' => ['patch' => 'patch-very-dynamic-path'] + ] + ] + ] + + ], + 'dynamic' => [ + 'regex' => '#^(?|https://www.server.net/([^/]+)(*MARK:https://www.server.net/{version})|https://([^/]+).server.net/([^/]+)(*MARK:https://{environment}.server.net/{version}))#', + 'servers' => [ + 'https://www.server.net/{version}' => [ + 'static' => [ + '/static/path' => [ + 'delete' => 'delete-static-path', + 'get' => 'get-static-path', + ] + ], + 'dynamic' => [ + 'regex' => '#^(?|/([^/]+)/path(*MARK:/{dynamic}/path))$#', + 'paths' => [ + '/{dynamic}/path' => ['post' => 'post-dynamic-path'] + ], + ] + ], + 'https://{environment}.server.net/{version}' => [ + 'static' => [], + 'dynamic' => [ + 'regex' => '#^(?|/([^/]+)/([^/]+)/([^/]+)(*MARK:/{very}/{dynamic}/{path}}))$#', + 'paths' => [ + '/{very}/{dynamic}/{path}}' => ['post' => 'post-very-dynamic-path'] + ], + ] + ], + ] + ] + ], + 'hostless' => [ + 'static' => [ + '' => [ + 'static' => [], + 'dynamic' => [ + 'regex' => '#^(?|/([^/]+)/([^/]+)/([^/]+)(*MARK:/{very}/{dynamic}/{path}}))$#', + 'paths' => [ + '/{very}/{dynamic}/{path}}' => ['get' => 'get-very-dynamic-path'] + ] + ] + ], + ], + 'dynamic' => ['regex' => '#^(?|)#', 'servers' => []] + ], + ]); + + $servers = [ + (function() { + $server = new Route\Server('https://www.server.net/{version}'); + $server->addRoute('/static/path', 'get', 'get-static-path'); + $server->addRoute('/{dynamic}/path', 'post', 'post-dynamic-path'); + $server->addRoute('/static/path', 'delete', 'delete-static-path'); + return $server; + })(), + (function() { + $server = new Route\Server('https://www.server.io'); + $server->addRoute('/{very}/{dynamic}/path', 'patch', 'patch-very-dynamic-path'); + $server->addRoute('/{dynamic}/path', 'get', 'get-dynamic-path'); + $server->addRoute('/static/path', 'post', 'post-static-path'); + return $server; + })(), + (function() { + $server = new Route\Server('https://{environment}.server.net/{version}'); + $server->addRoute('/{very}/{dynamic}/{path}}', 'post', 'post-very-dynamic-path'); + return $server; + })(), + (function() { + $server = new Route\Server(''); + $server->addRoute('/{very}/{dynamic}/{path}}', 'get', 'get-very-dynamic-path'); + return $server; + })(), + ]; + + yield [$expected, ...$servers]; + yield [$expected, ...array_reverse($servers)]; + } + + #[Test] + #[DataProvider('provideServers')] + public function itCanBeConstructedFromServers(RouteCollection $expected, Route\Server ...$servers): void + { + self::assertEquals($expected, RouteCollection::fromServers(...$servers)); + } +} diff --git a/tests/RouteCollectorTest.php b/tests/RouteCollectorTest.php index 2db182d..9249270 100644 --- a/tests/RouteCollectorTest.php +++ b/tests/RouteCollectorTest.php @@ -44,6 +44,35 @@ public function throwExceptionIfThereAreNoRoutes(): void (new RouteCollector())->collect($openAPI); } + #[Test] + public function removesDuplicateServers(): void + { + $openAPI = (new Reader([OpenAPIVersion::Version_3_0, OpenAPIVersion::Version_3_1])) + ->readFromString( + json_encode([ + 'openapi' => '3.0.0', + 'info' => ['title' => '', 'version' => '1.0.0'], + 'servers' => [ + ['url' => 'https://www.server.net'], + ['url' => 'https://www.server.net/'], + ], + 'paths' => [ + '/path' => [ + 'get' => [ + 'operationId' => 'get-path', + 'responses' => [200 => ['description' => 'Successful Response']] + ] + ] + ] + ]), + FileFormat::Json + ); + + $routeCollection = (new RouteCollector())->collect($openAPI); + + self::assertCount(1, $routeCollection->routes['hosted']['static']); + } + public static function collectTestProvider(): Generator { yield 'petstore-expanded.json' => [