From 7e76f37bc274824e23dfd97a091cdf60907017ca Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Fri, 25 Oct 2024 21:55:30 +1000 Subject: [PATCH 01/25] Refactor RoutesFilter to use a RouteFilterOptionsInterface object Previously, RoutesFilter was taking an array with the filter options (i.e., method, path, name, etc) but, as @gsteel noted, there was a perfectly good RouteFilterOptions class available. I don't know why I developed it, but overlooked it. This change refactors RoutesFilter to use it instead, then goes one step further and introduces a RouteFilterOptionsInterface so that, when no filter options are available, an EmptyRouteFilterOptions can be supplied, hopefully making the code easier to intuit and maintain. Signed-off-by: Matthew Setter --- composer.lock | 30 ++-- src/Routes/Filter/EmptyRouteFilterOptions.php | 38 +++++ src/Routes/Filter/RouteFilterOptions.php | 45 +++--- .../Filter/RouteFilterOptionsInterface.php | 23 +++ src/Routes/Filter/RoutesFilter.php | 92 ++++------- src/Routes/ListRoutesCommand.php | 14 +- src/Routes/ListRoutesCommandFactory.php | 3 +- test/Routes/Filter/RouteFilterOptionsTest.php | 2 +- test/Routes/ListRoutesCommandTest.php | 15 +- test/Routes/RoutesFilterTest.php | 153 ++++++++---------- 10 files changed, 217 insertions(+), 198 deletions(-) create mode 100644 src/Routes/Filter/EmptyRouteFilterOptions.php create mode 100644 src/Routes/Filter/RouteFilterOptionsInterface.php diff --git a/composer.lock b/composer.lock index d7c20ab6..6aedda86 100644 --- a/composer.lock +++ b/composer.lock @@ -285,16 +285,16 @@ }, { "name": "laminas/laminas-escaper", - "version": "2.14.0", + "version": "2.15.0", "source": { "type": "git", "url": "https://github.com/laminas/laminas-escaper.git", - "reference": "0f7cb975f4443cf22f33408925c231225cfba8cb" + "reference": "c612b0488ae486284c39885efca494c180f16351" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laminas/laminas-escaper/zipball/0f7cb975f4443cf22f33408925c231225cfba8cb", - "reference": "0f7cb975f4443cf22f33408925c231225cfba8cb", + "url": "https://api.github.com/repos/laminas/laminas-escaper/zipball/c612b0488ae486284c39885efca494c180f16351", + "reference": "c612b0488ae486284c39885efca494c180f16351", "shasum": "" }, "require": { @@ -306,12 +306,12 @@ "zendframework/zend-escaper": "*" }, "require-dev": { - "infection/infection": "^0.27.9", - "laminas/laminas-coding-standard": "~3.0.0", + "infection/infection": "^0.27.11", + "laminas/laminas-coding-standard": "~3.0.1", "maglnet/composer-require-checker": "^3.8.0", - "phpunit/phpunit": "^9.6.16", + "phpunit/phpunit": "^9.6.22", "psalm/plugin-phpunit": "^0.19.0", - "vimeo/psalm": "^5.21.1" + "vimeo/psalm": "^5.26.1" }, "type": "library", "autoload": { @@ -343,7 +343,7 @@ "type": "community_bridge" } ], - "time": "2024-10-24T10:12:53+00:00" + "time": "2024-12-17T19:39:54+00:00" }, { "name": "laminas/laminas-httphandlerrunner", @@ -5292,16 +5292,16 @@ }, { "name": "spatie/array-to-xml", - "version": "3.3.0", + "version": "3.4.0", "source": { "type": "git", "url": "https://github.com/spatie/array-to-xml.git", - "reference": "f56b220fe2db1ade4c88098d83413ebdfc3bf876" + "reference": "7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/spatie/array-to-xml/zipball/f56b220fe2db1ade4c88098d83413ebdfc3bf876", - "reference": "f56b220fe2db1ade4c88098d83413ebdfc3bf876", + "url": "https://api.github.com/repos/spatie/array-to-xml/zipball/7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67", + "reference": "7dcfc67d60b0272926dabad1ec01f6b8a5fb5e67", "shasum": "" }, "require": { @@ -5344,7 +5344,7 @@ "xml" ], "support": { - "source": "https://github.com/spatie/array-to-xml/tree/3.3.0" + "source": "https://github.com/spatie/array-to-xml/tree/3.4.0" }, "funding": [ { @@ -5356,7 +5356,7 @@ "type": "github" } ], - "time": "2024-05-01T10:20:27+00:00" + "time": "2024-12-16T12:45:15+00:00" }, { "name": "squizlabs/php_codesniffer", diff --git a/src/Routes/Filter/EmptyRouteFilterOptions.php b/src/Routes/Filter/EmptyRouteFilterOptions.php new file mode 100644 index 00000000..35a04cd9 --- /dev/null +++ b/src/Routes/Filter/EmptyRouteFilterOptions.php @@ -0,0 +1,38 @@ + */ private array $methods = []; - /** - * @param string|array $methods - */ + /** @param string|null|array $methods */ public function __construct( - string $middleware = '', - string $name = '', - string $path = '', - $methods = [] + private string $middleware = "", + private string $name = "", + private string $path = "", + $methods = "" ) { if (is_string($methods)) { - $this->methods = [$methods]; + $this->methods = [strtoupper($methods)]; } + if (is_array($methods)) { + array_walk($methods, fn(string &$value) => $value = strtoupper($value)); $this->methods = $methods; } - $this->middleware = $middleware; - $this->name = $name; - $this->path = $path; } public function has(string $filterOption): bool { - if (in_array($filterOption, ['middleware', 'name', 'path'])) { - return $this->$filterOption !== null; + if (! in_array($filterOption, ['methods', 'middleware', 'name', 'path'])) { + return false; } - if ($filterOption === 'methods') { - return [] !== $this->methods; + if (in_array($filterOption, ['middleware', 'name', 'path'])) { + return $this->$filterOption !== ""; } - return false; + return $this->methods !== []; } public function getMiddleware(): string @@ -66,9 +61,6 @@ public function getPath(): string return $this->path; } - /** - * @return array - */ public function getMethods(): array { return $this->methods; @@ -78,7 +70,10 @@ public function toArray(): array { $values = []; foreach (get_object_vars($this) as $key => $value) { - if (! empty($value)) { + if (is_string($value) && $value !== "") { + $values[$key] = $value; + } + if (is_array($value) && ! empty($value)) { $values[$key] = $value; } } diff --git a/src/Routes/Filter/RouteFilterOptionsInterface.php b/src/Routes/Filter/RouteFilterOptionsInterface.php new file mode 100644 index 00000000..34bb400b --- /dev/null +++ b/src/Routes/Filter/RouteFilterOptionsInterface.php @@ -0,0 +1,23 @@ + + */ + public function getMethods(): array; + + public function toArray(): array; +} diff --git a/src/Routes/Filter/RoutesFilter.php b/src/Routes/Filter/RoutesFilter.php index cf210b16..13793633 100644 --- a/src/Routes/Filter/RoutesFilter.php +++ b/src/Routes/Filter/RoutesFilter.php @@ -10,17 +10,13 @@ use Iterator; use Mezzio\Router\Route; -use function array_filter; use function array_intersect; use function array_walk; use function in_array; -use function is_array; -use function is_string; use function preg_match; use function sprintf; use function str_replace; use function stripos; -use function strtoupper; /** * RoutesFilter filters a traversable list of Route objects based on any of the four Route criteria, @@ -35,6 +31,14 @@ final class RoutesFilter extends FilterIterator { /** * @param ArrayIterator $routes + * @param RouteFilterOptionsInterface $filterOptions An array storing the list of route options to + * filter a route on along with their respective values. + * The four allowed route options are: name, path, method, + * and middleware. Name and path can be a fixed string, + * such as user.profile, or a regular expression, such as + * user.*. Middleware can only contain a class name. + * Method can be either a string which contains one of + * the allowed HTTP methods, or an array of HTTP methods. */ public function __construct( ArrayIterator $routes, @@ -57,7 +61,7 @@ public function __construct( ); } - public function getFilterOptions(): array + public function getFilterOptions(): RouteFilterOptionsInterface { return $this->filterOptions; } @@ -67,91 +71,65 @@ public function accept(): bool /** @var Route $route */ $route = $this->getInnerIterator()->current(); - if (empty($this->filterOptions)) { - return true; - } - - if (! empty($this->filterOptions['name'])) { - return $route->getName() === $this->filterOptions['name'] + if ($this->filterOptions->has("name")) { + return $route->getName() === $this->filterOptions->getName() || $this->matchesByRegex($route, 'name'); } - if (! empty($this->filterOptions['path'])) { - return $route->getPath() === $this->filterOptions['path'] + if ($this->filterOptions->has("path")) { + return $route->getPath() === $this->filterOptions->getPath() || $this->matchesByRegex($route, 'path'); } - if (! empty($this->filterOptions['method'])) { - return $this->matchesByMethod($route); + if ($this->filterOptions->has("middleware")) { + return $this->matchesByMiddleware($route); } - if (! empty($this->filterOptions['middleware'])) { - return $this->matchesByMiddleware($route); + if ($this->filterOptions->has("methods")) { + return $this->matchesByMethod($route); } - return false; + return true; } /** * Match the route against a regular expression based on the field in $matchType. * - * $matchType can be either "path" or "name". + * @param 'name'|'path' $routeAttribute */ - public function matchesByRegex(Route $route, string $routeAttribute): bool + private function matchesByRegex(Route $route, string $routeAttribute): bool { + if (! in_array($routeAttribute, ["name", "path"])) { + return false; + } + if ($routeAttribute === 'path') { - $path = (string) $this->filterOptions['path']; + $path = $this->filterOptions->getPath(); return (bool) preg_match( sprintf("/^%s/", str_replace('/', '\/', $path)), $route->getPath() ); } - if ($routeAttribute === 'name') { - return (bool) preg_match( - sprintf( - "/%s/", - (string) $this->filterOptions['name'] - ), - $route->getName() - ); - } - - return false; + return (bool) preg_match( + sprintf("/%s/", $this->filterOptions->getName()), + $route->getName() + ); } /** * Match if the current route supports the method(s) supplied. */ - public function matchesByMethod(Route $route): bool + private function matchesByMethod(Route $route): bool { if ($route->allowsAnyMethod()) { return true; } - if ($this->filterOptions['method'] === Route::HTTP_METHOD_ANY) { - return true; - } - - if (is_string($this->filterOptions['method'])) { - return in_array( - strtoupper($this->filterOptions['method']), - $route->getAllowedMethods() ?? [] - ); - } - - if (is_array($this->filterOptions['method'])) { - array_walk( - $this->filterOptions['method'], - fn(string &$value) => $value = strtoupper($value) - ); - return ! empty(array_intersect( - $this->filterOptions['method'], - $route->getAllowedMethods() ?? [] - )); - } - - return false; + return array_intersect( + $this->filterOptions->getMethods(), + $route->getAllowedMethods() ?? [] + ) !== []; } /** @@ -163,7 +141,7 @@ public function matchesByMethod(Route $route): bool * the class' name. The intent is to perform checks from the least to the * most computationally expensive, to avoid excessive overhead. */ - public function matchesByMiddleware(Route $route): bool + private function matchesByMiddleware(Route $route): bool { $middlewareClass = $route->getMiddleware()::class; $matchesMiddleware = (string) $this->filterOptions['middleware']; diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index fecd2c57..632ce0a0 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -7,6 +7,8 @@ use ArrayIterator; use Mezzio\Router\Route; use Mezzio\Router\RouteCollector; +use Mezzio\Tooling\Routes\Filter\RouteFilterOptions; +use Mezzio\Tooling\Routes\Filter\RouteFilterOptionsInterface; use Mezzio\Tooling\Routes\Filter\RoutesFilter; use Mezzio\Tooling\Routes\Sorter\RouteSorterByName; use Mezzio\Tooling\Routes\Sorter\RouteSorterByPath; @@ -160,12 +162,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int : new RouteSorterByPath(); usort($this->routes, $sorter); - $this->filterOptions = [ - 'method' => strtolower((string) $input->getOption('supports-method')), - 'middleware' => strtolower((string) $input->getOption('has-middleware')), - 'name' => strtolower((string) $input->getOption('has-name')), - 'path' => strtolower((string) $input->getOption('has-path')), - ]; + $this->filterOptions = new RouteFilterOptions( + strtolower((string) $input->getOption('has-middleware')), + strtolower((string) $input->getOption('has-name')), + strtolower((string) $input->getOption('has-path')), + [strtolower((string) $input->getOption('supports-method'))], + ); switch ($format) { case 'json': diff --git a/src/Routes/ListRoutesCommandFactory.php b/src/Routes/ListRoutesCommandFactory.php index d38d88b6..6c7dcad4 100644 --- a/src/Routes/ListRoutesCommandFactory.php +++ b/src/Routes/ListRoutesCommandFactory.php @@ -6,6 +6,7 @@ use Mezzio\Application; use Mezzio\MiddlewareFactory; +use Mezzio\Tooling\Routes\Filter\EmptyRouteFilterOptions; use Mezzio\Tooling\Routes\RoutesFileConfigLoader; use Psr\Container\ContainerInterface; @@ -21,6 +22,6 @@ public function __invoke(ContainerInterface $container): ListRoutesCommand $configLoader = new RoutesFileConfigLoader('config/routes.php', $application, $factory, $container); - return new ListRoutesCommand($container, $configLoader); + return new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); } } diff --git a/test/Routes/Filter/RouteFilterOptionsTest.php b/test/Routes/Filter/RouteFilterOptionsTest.php index 4d5dd851..9bd8c1f0 100644 --- a/test/Routes/Filter/RouteFilterOptionsTest.php +++ b/test/Routes/Filter/RouteFilterOptionsTest.php @@ -58,7 +58,7 @@ public function testCanGetArrayRepresentation(array $options, array $expectedRes $routeFilterOptions = new RouteFilterOptions($middleware, $name, $path, $methods); - $this->assertSame($expectedResult, $routeFilterOptions->toArray()); + $this->assertEquals($expectedResult, $routeFilterOptions->toArray()); } /** diff --git a/test/Routes/ListRoutesCommandTest.php b/test/Routes/ListRoutesCommandTest.php index ae5a772e..254dcd58 100644 --- a/test/Routes/ListRoutesCommandTest.php +++ b/test/Routes/ListRoutesCommandTest.php @@ -7,6 +7,7 @@ use Mezzio\Router\Route; use Mezzio\Router\RouteCollector; use Mezzio\Tooling\Routes\ConfigLoaderInterface; +use Mezzio\Tooling\Routes\Filter\EmptyRouteFilterOptions; use Mezzio\Tooling\Routes\ListRoutesCommand; use MezzioTest\Tooling\Routes\Middleware\ExpressMiddleware; use MezzioTest\Tooling\Routes\Middleware\SimpleMiddleware; @@ -50,7 +51,7 @@ protected function setUp(): void $this->input = $this->createMock(InputInterface::class); $this->output = $this->createMock(ConsoleOutputInterface::class); $this->routeCollection = $this->createMock(RouteCollector::class); - $this->command = new ListRoutesCommand($container, $configLoader); + $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); } /** @@ -139,7 +140,7 @@ public function testSuccessfulExecutionEmitsExpectedOutput(): void /** @var ConfigLoaderInterface&MockObject $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($container, $configLoader); + $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); $outputFormatter = new OutputFormatter(false); @@ -193,7 +194,7 @@ public function testRendersAnEmptyResultWhenNoRoutesArePresent(): void ->expects($this->once()) ->method('load'); - $this->command = new ListRoutesCommand($container, $configLoader); + $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -244,7 +245,7 @@ public function testRendersRoutesAsJsonWhenFormatSetToJson(): void ->expects($this->once()) ->method('load'); - $this->command = new ListRoutesCommand($container, $configLoader); + $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -298,7 +299,7 @@ public function testThatOnlyAllowedFormatsCanBeSupplied(string $format): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($container, $configLoader); + $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -376,7 +377,7 @@ public function testCanSortResults(string $sortOrder): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($this->container, $configLoader); + $this->command = new ListRoutesCommand($this->container, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -449,7 +450,7 @@ public function testCanFilterRoutingTable(array $filterOptions): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($container, $configLoader); + $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') diff --git a/test/Routes/RoutesFilterTest.php b/test/Routes/RoutesFilterTest.php index 715c450c..4690c1f3 100644 --- a/test/Routes/RoutesFilterTest.php +++ b/test/Routes/RoutesFilterTest.php @@ -6,6 +6,8 @@ use ArrayIterator; use Mezzio\Router\Route; +use Mezzio\Tooling\Routes\Filter\RouteFilterOptions; +use Mezzio\Tooling\Routes\Filter\RouteFilterOptionsInterface; use Mezzio\Tooling\Routes\Filter\RoutesFilter; use MezzioTest\Tooling\Routes\Middleware\ExpressMiddleware; use MezzioTest\Tooling\Routes\Middleware\SimpleMiddleware; @@ -61,30 +63,12 @@ public function setUp(): void ]; } - public function testFiltersOutEmptyOptions(): void - { - $routeFilter = new RoutesFilter( - new ArrayIterator($this->routes), - [ - 'middleware' => null, - 'name' => '', - 'path' => '/user', - ] - ); - - $this->assertSame( - ['path' => '/user'], - $routeFilter->getFilterOptions() - ); - } - /** - * @param array $filterOptions * @dataProvider validFilterDataProvider */ public function testCanFilterRoutesWithStringSearchExpression( int $expectedNumberOfRoutes, - array $filterOptions = [] + RouteFilterOptionsInterface $filterOptions ): void { $this->setUp(); @@ -93,139 +77,136 @@ public function testCanFilterRoutesWithStringSearchExpression( $this->assertCount( $expectedNumberOfRoutes, $routeFilter, - sprintf( - 'Filtered with %s', - var_export($filterOptions, true) - ) + sprintf('Filtered with %s', var_export($filterOptions, true)) ); } /** - * @psalm-return array}> + * @psalm-return array */ public static function validFilterDataProvider(): array { return [ 'middleware-simple-compound-name' => [ 5, - [ - 'middleware' => 'ExpressMiddleware', - ], + new RouteFilterOptions( + middleware: 'ExpressMiddleware', + ), ], 'middleware-simple-class-name' => [ 6, - [ - 'middleware' => 'Tooling', - ], + new RouteFilterOptions( + middleware: 'Tooling', + ), ], 'middleware-regex' => [ 6, - [ - 'middleware' => 'Tooling.*Middleware', - ], + new RouteFilterOptions( + middleware: 'Tooling.*Middleware', + ), ], 'middleware-fqcn' => [ 5, - [ - 'middleware' => ExpressMiddleware::class, - ], + new RouteFilterOptions( + middleware: ExpressMiddleware::class, + ), ], 'name-bare' => [ 1, - [ - 'name' => 'home', - ], + new RouteFilterOptions( + name: 'home', + ), ], 'name-regex' => [ 5, - [ - 'name' => 'user.*', - ], + new RouteFilterOptions( + name: 'user.*', + ), ], 'path-fq' => [ 1, - [ - 'path' => '/user', - ], + new RouteFilterOptions( + path: '/user', + ), ], 'path-fq-regex' => [ 4, - [ - 'path' => '/log.*', - ], + new RouteFilterOptions( + path: '/log.*', + ), ], 'path-root' => [ 6, - [ - 'path' => '/', - ], + new RouteFilterOptions( + path: '/', + ), ], 'method-get' => [ 6, - [ - 'method' => 'GET', - ], + new RouteFilterOptions( + methods: ['get'], + ), ], 'method-any' => [ 6, - [ - 'method' => Route::HTTP_METHOD_ANY, - ], + new RouteFilterOptions( + methods: Route::HTTP_METHOD_ANY, + ), ], 'method-get-lc' => [ 6, - [ - 'method' => 'get', - ], + new RouteFilterOptions( + methods: 'get', + ), ], 'method-post-lc' => [ 2, - [ - 'method' => 'post', - ], + new RouteFilterOptions( + methods: 'post', + ), ], - /*[ + [ 6, - [ - 'method' => ['POST', 'GET'], - ], + new RouteFilterOptions( + methods: ['POST', 'GET'], + ), ], [ 2, - [ - 'method' => ['POST'], - ], + new RouteFilterOptions( + methods: ['POST'], + ), ], [ 6, - [ - 'method' => ['GET'], - ], + new RouteFilterOptions( + methods: ['GET'], + ), ], [ 1, - [ - 'method' => ['PATCH'], - ], + new RouteFilterOptions( + methods: ['PATCH'], + ), ], [ 2, - [ - 'method' => ['PATCH', 'POST'], - ], + new RouteFilterOptions( + methods: ['PATCH', 'POST'], + ), ], [ 2, - [ - 'method' => ['patch', 'post'], - ], + new RouteFilterOptions( + methods: ['patch', 'post'], + ), ], [ 1, - [ - 'method' => ['patch'], - ], - ],*/ + new RouteFilterOptions( + methods: ['patch'], + ), + ], ]; } } From 0e75926e9a21a0b79c13dece2c063420b696c8fb Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Fri, 25 Oct 2024 22:11:28 +1000 Subject: [PATCH 02/25] Add docblock comments for RouteFilterOptionsInterface methods Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptionsInterface.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Routes/Filter/RouteFilterOptionsInterface.php b/src/Routes/Filter/RouteFilterOptionsInterface.php index 34bb400b..812afcf1 100644 --- a/src/Routes/Filter/RouteFilterOptionsInterface.php +++ b/src/Routes/Filter/RouteFilterOptionsInterface.php @@ -6,15 +6,29 @@ interface RouteFilterOptionsInterface { + /** + * Determines if the specified filter ($filterOption) has been set + */ public function has(string $filterOption): bool; + /** + * Retrieves a route middleware filter the available routing data by + */ public function getMiddleware(): string; + /** + * Retrieves a route name filter the available routing data by + */ public function getName(): string; + /** + * Retrieves a route path to filter the available routing data by + */ public function getPath(): string; /** + * Returns any route methods to filter the available routing data by + * * @return array */ public function getMethods(): array; From b42ad5e2c5ffa5801d0a43bfa266242c9cbd8b23 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Fri, 25 Oct 2024 22:21:53 +1000 Subject: [PATCH 03/25] Refactor filter options as class member variables This change stores the filter options as member variables so that they're more readily reusable and so that it should be less likely to make a mistake with using them at some point in the future. Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index 2a9b2d90..fa361e3b 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -15,6 +15,7 @@ final class RouteFilterOptions implements RouteFilterOptionsInterface { /** @var array */ private array $methods = []; + private array $allowedFilterOptions = ['methods', 'middleware', 'name', 'path']; /** @param string|null|array $methods */ public function __construct( @@ -33,13 +34,18 @@ public function __construct( } } + private function getFilterOptionsMinusMethods(): array + { + return array_filter($this->allowedFilterOptions, fn($value) => $value !== "methods"); + } + public function has(string $filterOption): bool { - if (! in_array($filterOption, ['methods', 'middleware', 'name', 'path'])) { + if (! in_array($filterOption, $this->allowedFilterOptions)) { return false; } - if (in_array($filterOption, ['middleware', 'name', 'path'])) { + if (in_array($filterOption, $this->getFilterOptionsMinusMethods())) { return $this->$filterOption !== ""; } @@ -70,10 +76,11 @@ public function toArray(): array { $values = []; foreach (get_object_vars($this) as $key => $value) { - if (is_string($value) && $value !== "") { + if (in_array($key, $this->getFilterOptionsMinusMethods()) && $value !== "") { $values[$key] = $value; } - if (is_array($value) && ! empty($value)) { + + if ($key === "methods" && ! empty($value)) { $values[$key] = $value; } } From bb8a3d18f1f59aba6df2bbca183fdfe86a4761e1 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 15:47:18 +1000 Subject: [PATCH 04/25] Refactor RouteFilterOptions to require specific types without defaults This change removes the class' default options, replacing them with unions of non-empty-string|null. In doing so it's explicit that null means the option is unset and should be ignored. This change was motivated by @gsteel's comment https://github.com/mezzio/mezzio-tooling/pull/52#discussion_r1808661057. I believe that I've implemented it correctly. Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 23 ++++--- .../Filter/RouteFilterOptionsInterface.php | 10 ++-- src/Routes/Filter/RoutesFilter.php | 6 +- test/Routes/RoutesFilterTest.php | 60 +++++++++++++++++++ 4 files changed, 78 insertions(+), 21 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index fa361e3b..d385b36f 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -4,6 +4,7 @@ namespace Mezzio\Tooling\Routes\Filter; +use function array_filter; use function array_walk; use function get_object_vars; use function in_array; @@ -13,16 +14,14 @@ final class RouteFilterOptions implements RouteFilterOptionsInterface { - /** @var array */ - private array $methods = []; private array $allowedFilterOptions = ['methods', 'middleware', 'name', 'path']; /** @param string|null|array $methods */ public function __construct( - private string $middleware = "", - private string $name = "", - private string $path = "", - $methods = "" + private string|null $middleware, + private string|null $name, + private string|null $path, + private array|string|null $methods ) { if (is_string($methods)) { $this->methods = [strtoupper($methods)]; @@ -46,28 +45,28 @@ public function has(string $filterOption): bool } if (in_array($filterOption, $this->getFilterOptionsMinusMethods())) { - return $this->$filterOption !== ""; + return $this->$filterOption !== null; } - return $this->methods !== []; + return $this->methods !== [] && $this->methods !== null; } - public function getMiddleware(): string + public function getMiddleware(): string|null { return $this->middleware; } - public function getName(): string + public function getName(): string|null { return $this->name; } - public function getPath(): string + public function getPath(): string|null { return $this->path; } - public function getMethods(): array + public function getMethods(): array|string|null { return $this->methods; } diff --git a/src/Routes/Filter/RouteFilterOptionsInterface.php b/src/Routes/Filter/RouteFilterOptionsInterface.php index 812afcf1..2e874bd5 100644 --- a/src/Routes/Filter/RouteFilterOptionsInterface.php +++ b/src/Routes/Filter/RouteFilterOptionsInterface.php @@ -14,24 +14,22 @@ public function has(string $filterOption): bool; /** * Retrieves a route middleware filter the available routing data by */ - public function getMiddleware(): string; + public function getMiddleware(): string|null; /** * Retrieves a route name filter the available routing data by */ - public function getName(): string; + public function getName(): string|null; /** * Retrieves a route path to filter the available routing data by */ - public function getPath(): string; + public function getPath(): string|null; /** * Returns any route methods to filter the available routing data by - * - * @return array */ - public function getMethods(): array; + public function getMethods(): array|string|null; public function toArray(): array; } diff --git a/src/Routes/Filter/RoutesFilter.php b/src/Routes/Filter/RoutesFilter.php index 13793633..2607bee5 100644 --- a/src/Routes/Filter/RoutesFilter.php +++ b/src/Routes/Filter/RoutesFilter.php @@ -7,12 +7,11 @@ use ArrayIterator; use Exception; use FilterIterator; -use Iterator; use Mezzio\Router\Route; use function array_intersect; -use function array_walk; use function in_array; +use function is_string; use function preg_match; use function sprintf; use function str_replace; @@ -126,8 +125,9 @@ private function matchesByMethod(Route $route): bool return true; } + $methods = $this->filterOptions->getMethods() ?? []; return array_intersect( - $this->filterOptions->getMethods(), + is_string($methods) ? [$methods] : $methods, $route->getAllowedMethods() ?? [] ) !== []; } diff --git a/test/Routes/RoutesFilterTest.php b/test/Routes/RoutesFilterTest.php index 4690c1f3..50e2dc2c 100644 --- a/test/Routes/RoutesFilterTest.php +++ b/test/Routes/RoutesFilterTest.php @@ -91,120 +91,180 @@ public static function validFilterDataProvider(): array 5, new RouteFilterOptions( middleware: 'ExpressMiddleware', + name: null, + path: null, + methods: null, ), ], 'middleware-simple-class-name' => [ 6, new RouteFilterOptions( middleware: 'Tooling', + name: null, + path: null, + methods: null, ), ], 'middleware-regex' => [ 6, new RouteFilterOptions( middleware: 'Tooling.*Middleware', + name: null, + path: null, + methods: null, ), ], 'middleware-fqcn' => [ 5, new RouteFilterOptions( middleware: ExpressMiddleware::class, + name: null, + path: null, + methods: null, ), ], 'name-bare' => [ 1, new RouteFilterOptions( name: 'home', + middleware: null, + path: null, + methods: null, ), ], 'name-regex' => [ 5, new RouteFilterOptions( name: 'user.*', + middleware: null, + path: null, + methods: null, ), ], 'path-fq' => [ 1, new RouteFilterOptions( path: '/user', + middleware: null, + name: null, + methods: null, ), ], 'path-fq-regex' => [ 4, new RouteFilterOptions( path: '/log.*', + middleware: null, + name: null, + methods: null, ), ], 'path-root' => [ 6, new RouteFilterOptions( path: '/', + middleware: null, + name: null, + methods: null, ), ], 'method-get' => [ 6, new RouteFilterOptions( methods: ['get'], + middleware: null, + name: null, + path: null, ), ], 'method-any' => [ 6, new RouteFilterOptions( methods: Route::HTTP_METHOD_ANY, + middleware: null, + name: null, + path: null, ), ], 'method-get-lc' => [ 6, new RouteFilterOptions( methods: 'get', + middleware: null, + name: null, + path: null, ), ], 'method-post-lc' => [ 2, new RouteFilterOptions( methods: 'post', + middleware: null, + name: null, + path: null, ), ], [ 6, new RouteFilterOptions( methods: ['POST', 'GET'], + middleware: null, + name: null, + path: null, ), ], [ 2, new RouteFilterOptions( methods: ['POST'], + middleware: null, + name: null, + path: null, ), ], [ 6, new RouteFilterOptions( methods: ['GET'], + middleware: null, + name: null, + path: null, ), ], [ 1, new RouteFilterOptions( methods: ['PATCH'], + middleware: null, + name: null, + path: null, ), ], [ 2, new RouteFilterOptions( methods: ['PATCH', 'POST'], + middleware: null, + name: null, + path: null, ), ], [ 2, new RouteFilterOptions( methods: ['patch', 'post'], + middleware: null, + name: null, + path: null, ), ], [ 1, new RouteFilterOptions( methods: ['patch'], + middleware: null, + name: null, + path: null, ), ], ]; From d75d4bacd2df8f0eb00bd8b456cab49426de082b Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 15:49:46 +1000 Subject: [PATCH 05/25] Replace the Exception with a Throwable in RoutesFilter This is to ensure that any possible type of error is caught and handled. Signed-off-by: Matthew Setter --- src/Routes/Filter/RoutesFilter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Routes/Filter/RoutesFilter.php b/src/Routes/Filter/RoutesFilter.php index 2607bee5..be208077 100644 --- a/src/Routes/Filter/RoutesFilter.php +++ b/src/Routes/Filter/RoutesFilter.php @@ -5,9 +5,9 @@ namespace Mezzio\Tooling\Routes\Filter; use ArrayIterator; -use Exception; use FilterIterator; use Mezzio\Router\Route; +use Throwable; use function array_intersect; use function in_array; @@ -153,7 +153,7 @@ private function matchesByMiddleware(Route $route): bool sprintf('/%s/', $this->escapeNamespaceSeparatorForRegex($matchesMiddleware)), $middlewareClass ); - } catch (Exception) { + } catch (Throwable) { return false; } } From 59d84bb32ab9618251e26698c762327ffdc9da24 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 15:51:11 +1000 Subject: [PATCH 06/25] Refactor ListRoutesCommand to take a RouteCollector directly I'm not sure why I didn't do this originally. Perhaps I was experimenting to learn how routing works in Mezzio more deeply. Regardless, this change refactors ListRoutesCommand to take a RouteCollector object instead of a ContainerInterface and from that ContainerInterface fetching a RouteCollector. Signed-off-by: Matthew Setter --- src/Routes/ListRoutesCommand.php | 6 +- src/Routes/ListRoutesCommandFactory.php | 13 ++- test/Routes/ListRoutesCommandFactoryTest.php | 6 +- test/Routes/ListRoutesCommandTest.php | 93 ++++++-------------- 4 files changed, 45 insertions(+), 73 deletions(-) diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index 632ce0a0..06b2a05f 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -12,7 +12,6 @@ use Mezzio\Tooling\Routes\Filter\RoutesFilter; use Mezzio\Tooling\Routes\Sorter\RouteSorterByName; use Mezzio\Tooling\Routes\Sorter\RouteSorterByPath; -use Psr\Container\ContainerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputInterface; @@ -146,10 +145,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->configLoader->load(); - /** @var RouteCollector $routeCollector */ - $routeCollector = $this->container->get(RouteCollector::class); - $this->routes = $routeCollector->getRoutes(); - + $this->routes = $this->routeCollector->getRoutes(); if ([] === $this->routes) { $output->writeln(self::MSG_EMPTY_ROUTING_TABLE); return $result; diff --git a/src/Routes/ListRoutesCommandFactory.php b/src/Routes/ListRoutesCommandFactory.php index 6c7dcad4..c4d5b81e 100644 --- a/src/Routes/ListRoutesCommandFactory.php +++ b/src/Routes/ListRoutesCommandFactory.php @@ -6,6 +6,7 @@ use Mezzio\Application; use Mezzio\MiddlewareFactory; +use Mezzio\Router\RouteCollector; use Mezzio\Tooling\Routes\Filter\EmptyRouteFilterOptions; use Mezzio\Tooling\Routes\RoutesFileConfigLoader; use Psr\Container\ContainerInterface; @@ -20,8 +21,16 @@ public function __invoke(ContainerInterface $container): ListRoutesCommand /** @var MiddlewareFactory $factory */ $factory = $container->get(MiddlewareFactory::class); - $configLoader = new RoutesFileConfigLoader('config/routes.php', $application, $factory, $container); + $configLoader = new RoutesFileConfigLoader( + 'config/routes.php', + $application, + $factory, + $container + ); - return new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + /** @var RouteCollector $routeCollector */ + $routeCollector = $container->get(RouteCollector::class); + + return new ListRoutesCommand($routeCollector, $configLoader, new EmptyRouteFilterOptions()); } } diff --git a/test/Routes/ListRoutesCommandFactoryTest.php b/test/Routes/ListRoutesCommandFactoryTest.php index bf709004..ed19bc76 100644 --- a/test/Routes/ListRoutesCommandFactoryTest.php +++ b/test/Routes/ListRoutesCommandFactoryTest.php @@ -6,8 +6,10 @@ use Mezzio\Application; use Mezzio\MiddlewareFactory; +use Mezzio\Router\RouteCollector; use Mezzio\Tooling\Routes\ListRoutesCommand; use Mezzio\Tooling\Routes\ListRoutesCommandFactory; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; @@ -15,13 +17,15 @@ class ListRoutesCommandFactoryTest extends TestCase { public function testCanInstantiateListRoutesCommandObject(): void { + /** @var ContainerInterface&MockObject $container */ $container = $this->createMock(ContainerInterface::class); $container - ->expects($this->atMost(2)) + ->expects($this->atMost(3)) ->method('get') ->willReturnOnConsecutiveCalls( $this->createMock(Application::class), $this->createMock(MiddlewareFactory::class), + $this->createMock(RouteCollector::class), ); $factory = new ListRoutesCommandFactory(); diff --git a/test/Routes/ListRoutesCommandTest.php b/test/Routes/ListRoutesCommandTest.php index 254dcd58..69ce405f 100644 --- a/test/Routes/ListRoutesCommandTest.php +++ b/test/Routes/ListRoutesCommandTest.php @@ -13,7 +13,6 @@ use MezzioTest\Tooling\Routes\Middleware\SimpleMiddleware; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; use ReflectionClass; use ReflectionException; use ReflectionMethod; @@ -35,9 +34,6 @@ class ListRoutesCommandTest extends TestCase /** @var (RouteCollector&MockObject) */ private $routeCollection; - /** @var (ContainerInterface&MockObject) */ - private $container; - private ListRoutesCommand $command; protected function setUp(): void @@ -45,13 +41,14 @@ protected function setUp(): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - /** @var ContainerInterface $container */ - $container = $this->createMock(ContainerInterface::class); - $this->input = $this->createMock(InputInterface::class); $this->output = $this->createMock(ConsoleOutputInterface::class); $this->routeCollection = $this->createMock(RouteCollector::class); - $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand( + $this->routeCollection, + $configLoader, + new EmptyRouteFilterOptions() + ); } /** @@ -120,27 +117,20 @@ public function testConfigureSetsExpectedOptions(): void */ public function testSuccessfulExecutionEmitsExpectedOutput(): void { - $routes = [ + $routes = [ new Route("/", new SimpleMiddleware(), ['GET'], 'home'), new Route("/", new ExpressMiddleware(), ['GET'], 'home'), ]; + /** @var RouteCollector&MockObject $collector */ $collector = $this->createMock(RouteCollector::class); $collector ->expects($this->once()) ->method('getRoutes') ->willReturn($routes); - /** @var ContainerInterface&MockObject $container */ - $container = $this->createMock(ContainerInterface::class); - $container - ->expects($this->once()) - ->method('get') - ->with(RouteCollector::class) - ->willReturn($collector); - /** @var ConfigLoaderInterface&MockObject $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); $outputFormatter = new OutputFormatter(false); @@ -178,23 +168,16 @@ public function testSuccessfulExecutionEmitsExpectedOutput(): void public function testRendersAnEmptyResultWhenNoRoutesArePresent(): void { + /** @var RouteCollector&MockObject $collector */ $collector = $this->createMock(RouteCollector::class); - /** @var ContainerInterface&MockObject $container */ - $container = $this->createMock(ContainerInterface::class); - $container - ->expects($this->once()) - ->method('get') - ->with(RouteCollector::class) - ->willReturn($collector); - /** @var ConfigLoaderInterface&MockObject $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); $configLoader ->expects($this->once()) ->method('load'); - $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -221,31 +204,24 @@ public function testRendersAnEmptyResultWhenNoRoutesArePresent(): void public function testRendersRoutesAsJsonWhenFormatSetToJson(): void { - $routes = [ + $routes = [ new Route("/", new SimpleMiddleware(), ['GET'], 'home'), new Route("/", new ExpressMiddleware(), ['GET'], 'home'), ]; + /** @var RouteCollector&MockObject $collector */ $collector = $this->createMock(RouteCollector::class); $collector ->expects($this->once()) ->method('getRoutes') ->willReturn($routes); - /** @var ContainerInterface&MockObject $container */ - $container = $this->createMock(ContainerInterface::class); - $container - ->expects($this->once()) - ->method('get') - ->with(RouteCollector::class) - ->willReturn($collector); - /** @var ConfigLoaderInterface&MockObject $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); $configLoader ->expects($this->once()) ->method('load'); - $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -279,27 +255,21 @@ public function testRendersRoutesAsJsonWhenFormatSetToJson(): void */ public function testThatOnlyAllowedFormatsCanBeSupplied(string $format): void { - $routes = [ + $routes = [ new Route("/", new SimpleMiddleware(), ['GET'], 'home'), new Route("/", new ExpressMiddleware(), ['GET'], 'home'), ]; + + /** @var RouteCollector&MockObject $collector */ $collector = $this->createMock(RouteCollector::class); $collector ->expects($this->once()) ->method('getRoutes') ->willReturn($routes); - /** @var ContainerInterface&MockObject $container */ - $container = $this->createMock(ContainerInterface::class); - $container - ->expects($this->once()) - ->method('get') - ->with(RouteCollector::class) - ->willReturn($collector); - /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); $this->input ->method('getOption') @@ -367,17 +337,13 @@ public function testCanSortResults(string $sortOrder): void ->method('getRoutes') ->willReturn($routes); - $this->container = $this->createMock(ContainerInterface::class); - $this->container - ->expects($this->once()) - ->method('get') - ->willReturn( - $this->routeCollection, - ); - /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($this->container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand( + $this->routeCollection, + $configLoader, + new EmptyRouteFilterOptions() + ); $this->input ->method('getOption') @@ -434,23 +400,20 @@ public function testCanFilterRoutingTable(array $filterOptions): void new Route("/", new ExpressMiddleware(), ['GET'], 'home'), ]; + /** @var RouteCollector&MockObject $routeCollection */ $routeCollection = $this->createMock(RouteCollector::class); $routeCollection ->expects($this->once()) ->method('getRoutes') ->willReturn($routes); - /** @var ContainerInterface&MockObject $container */ - $container = $this->createMock(ContainerInterface::class); - $container - ->expects($this->once()) - ->method('get') - ->with(RouteCollector::class) - ->willReturn($routeCollection); - /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($container, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand( + $routeCollection, + $configLoader, + new EmptyRouteFilterOptions() + ); $this->input ->method('getOption') From 86d15316c22fced95dd582ee2f5fcb5b7ba13cd4 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 15:52:21 +1000 Subject: [PATCH 07/25] Minor code clean up Signed-off-by: Matthew Setter --- src/Routes/ListRoutesCommand.php | 4 ++-- test/Routes/ListRoutesCommandTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index 06b2a05f..23e0519e 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -188,7 +188,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $result; } - public function getRows(bool $requireNames = false): array + private function getRows(bool $requireNames = false): array { $rows = []; @@ -220,7 +220,7 @@ public function getRows(bool $requireNames = false): array return $rows; } - public function getSortOrder(InputInterface $input): string + private function getSortOrder(InputInterface $input): string { $sortOrder = strtolower((string) $input->getOption('sort')); return ! in_array($sortOrder, ['name', 'path']) diff --git a/test/Routes/ListRoutesCommandTest.php b/test/Routes/ListRoutesCommandTest.php index 69ce405f..dc983920 100644 --- a/test/Routes/ListRoutesCommandTest.php +++ b/test/Routes/ListRoutesCommandTest.php @@ -279,7 +279,7 @@ public function testThatOnlyAllowedFormatsCanBeSupplied(string $format): void false, // supports-method false, // has-name false, // has-path - false // sort + false // sort ); $this->output ->expects($this->once()) From 503747518536eba0e27b87f746d023a95f4e1b37 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 21:45:41 +1000 Subject: [PATCH 08/25] Refactor how ListRoutesCommandFactory retrieves a ConfigLoaderInterface This change refactors ListRoutesCommandFactory to retrieve a ConfigLoaderInterface object from the application's DiC instead of instantiating a RoutesFileConfigLoader object directly. The main reason for this is to provide flexibility to the user, should they have the need to retrieve routes in a different way than the default implementation, which searches in config/routes.php and in any routes registered through ConfigProvider classes. Signed-off-by: Matthew Setter --- src/Routes/ListRoutesCommandFactory.php | 19 +++---------------- test/Routes/ListRoutesCommandFactoryTest.php | 8 +++----- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/src/Routes/ListRoutesCommandFactory.php b/src/Routes/ListRoutesCommandFactory.php index c4d5b81e..9fd2c2c8 100644 --- a/src/Routes/ListRoutesCommandFactory.php +++ b/src/Routes/ListRoutesCommandFactory.php @@ -4,33 +4,20 @@ namespace Mezzio\Tooling\Routes; -use Mezzio\Application; -use Mezzio\MiddlewareFactory; use Mezzio\Router\RouteCollector; use Mezzio\Tooling\Routes\Filter\EmptyRouteFilterOptions; -use Mezzio\Tooling\Routes\RoutesFileConfigLoader; use Psr\Container\ContainerInterface; final class ListRoutesCommandFactory { public function __invoke(ContainerInterface $container): ListRoutesCommand { - /** @var Application $application */ - $application = $container->get(Application::class); - - /** @var MiddlewareFactory $factory */ - $factory = $container->get(MiddlewareFactory::class); - - $configLoader = new RoutesFileConfigLoader( - 'config/routes.php', - $application, - $factory, - $container - ); - /** @var RouteCollector $routeCollector */ $routeCollector = $container->get(RouteCollector::class); + /** @var ConfigLoaderInterface $configLoader */ + $configLoader = $container->get(ConfigLoaderInterface::class); + return new ListRoutesCommand($routeCollector, $configLoader, new EmptyRouteFilterOptions()); } } diff --git a/test/Routes/ListRoutesCommandFactoryTest.php b/test/Routes/ListRoutesCommandFactoryTest.php index ed19bc76..138fb985 100644 --- a/test/Routes/ListRoutesCommandFactoryTest.php +++ b/test/Routes/ListRoutesCommandFactoryTest.php @@ -4,9 +4,8 @@ namespace MezzioTest\Tooling\Routes; -use Mezzio\Application; -use Mezzio\MiddlewareFactory; use Mezzio\Router\RouteCollector; +use Mezzio\Tooling\Routes\ConfigLoaderInterface; use Mezzio\Tooling\Routes\ListRoutesCommand; use Mezzio\Tooling\Routes\ListRoutesCommandFactory; use PHPUnit\Framework\MockObject\MockObject; @@ -20,12 +19,11 @@ public function testCanInstantiateListRoutesCommandObject(): void /** @var ContainerInterface&MockObject $container */ $container = $this->createMock(ContainerInterface::class); $container - ->expects($this->atMost(3)) + ->expects($this->atMost(2)) ->method('get') ->willReturnOnConsecutiveCalls( - $this->createMock(Application::class), - $this->createMock(MiddlewareFactory::class), $this->createMock(RouteCollector::class), + $this->createMock(ConfigLoaderInterface::class), ); $factory = new ListRoutesCommandFactory(); From 26bf4e151907b52c28ddccab3f785f8161828425 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 21:48:54 +1000 Subject: [PATCH 09/25] Add a default way of retrieving routes from the application This change provides a default implementation for retrieving an application's routes configuration. It uses the stock approach of searching for routes in config/routes.php and any registered with the Application object in any of the loaded ConfigProvider classes. It also provides a simplistic example for building a custom implementation, should it be required. Signed-off-by: Matthew Setter --- .../DefaultRoutesConfigLoaderFactory.php | 36 +++++++++++++++++++ .../DefaultRoutesConfigLoaderFactoryTest.php | 35 ++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 src/Routes/DefaultRoutesConfigLoaderFactory.php create mode 100644 test/Routes/DefaultRoutesConfigLoaderFactoryTest.php diff --git a/src/Routes/DefaultRoutesConfigLoaderFactory.php b/src/Routes/DefaultRoutesConfigLoaderFactory.php new file mode 100644 index 00000000..1722e6c6 --- /dev/null +++ b/src/Routes/DefaultRoutesConfigLoaderFactory.php @@ -0,0 +1,36 @@ +get(Application::class); + + /** @var MiddlewareFactory $factory */ + $factory = $container->get(MiddlewareFactory::class); + + return new RoutesFileConfigLoader( + 'config/routes.php', + $application, + $factory, + $container + ); + } +} diff --git a/test/Routes/DefaultRoutesConfigLoaderFactoryTest.php b/test/Routes/DefaultRoutesConfigLoaderFactoryTest.php new file mode 100644 index 00000000..f6c348c6 --- /dev/null +++ b/test/Routes/DefaultRoutesConfigLoaderFactoryTest.php @@ -0,0 +1,35 @@ +createMock(ContainerInterface::class); + $container + ->expects($this->atMost(2)) + ->method("get") + ->willReturnOnConsecutiveCalls( + $this->createMock(Application::class), + $this->createMock(MiddlewareFactory::class), + ); + + $result = $factory($container); + + $this->assertInstanceOf(RoutesFileConfigLoader::class, $result); + } +} From 0f86b2af0f7e5e575c5260cdba1832b805082e51 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 14 Nov 2024 21:50:34 +1000 Subject: [PATCH 10/25] Load the DefaultRoutesConfigLoaderFactory with Mezzio Tooling This change updates the packages ConfigProvider class to register a ConfigLoaderInterface service with the DI container provided by DefaultRoutesConfigLoaderFactory. If a user needs to override this implementation, they can do so as per the additional documentation in the package's README. Signed-off-by: Matthew Setter --- README.md | 13 +++++++++++++ src/ConfigProvider.php | 3 +++ 2 files changed, 16 insertions(+) diff --git a/README.md b/README.md index 58121e89..48d3a635 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,19 @@ The command supports several options, listed in the table below. | `--has-middleware` | `-w` | Accepts a comma-separated list of one or more middleware classes, and filters out routes that do not require those classes. The classes can be fully-qualified, unqualified, or a regular expression, supported by the preg_* functions. For example, "\Mezzio\Middleware\LazyLoadingMiddleware,LazyLoadingMiddleware,\Mezzio*". | +##### Configuration + +By default, `Mezzio\Tooling\Routes\DefaultRoutesConfigLoaderFactory` registers a `ConfigLoaderInterface` service with the application's DI container, which retrieves the application's routes from two sources: + +- _config/routes.php_ +- Routes registered by any loaded `ConfigProvider` class + +However, this is a default/fallback implementation. +If you don't store any routes in _config/routes.php_ or need a custom implementation, then you need to do two things: + +1. Write a custom loader implementation that implements `Mezzio\Tooling\Routes\ConfigLoaderInterface` +2. Register it with the application's DI container as an alias for `Mezzio\Tooling\Routes\ConfigLoaderInterface` + ##### Usage Example Here is an example of what you can expect from running the command. diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index d802cd8b..0b424357 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -25,6 +25,8 @@ use Mezzio\Tooling\Module\DeregisterCommandFactory; use Mezzio\Tooling\Module\RegisterCommand; use Mezzio\Tooling\Module\RegisterCommandFactory; +use Mezzio\Tooling\Routes\ConfigLoaderInterface; +use Mezzio\Tooling\Routes\DefaultRoutesConfigLoaderFactory; use Mezzio\Tooling\Routes\ListRoutesCommand; use Mezzio\Tooling\Routes\ListRoutesCommandFactory; @@ -66,6 +68,7 @@ public function getDependencies(): array { return [ 'factories' => [ + ConfigLoaderInterface::class => DefaultRoutesConfigLoaderFactory::class, Create::class => CreateFactory::class, CreateActionCommand::class => CreateActionCommandFactory::class, CreateCommand::class => CreateCommandFactory::class, From 903ed62a4613349970158b874a9c672cba72d9b5 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:01:28 +1000 Subject: [PATCH 11/25] Update src/Routes/Filter/RouteFilterOptionsInterface.php Co-authored-by: George Steel Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptionsInterface.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Routes/Filter/RouteFilterOptionsInterface.php b/src/Routes/Filter/RouteFilterOptionsInterface.php index 2e874bd5..426f748c 100644 --- a/src/Routes/Filter/RouteFilterOptionsInterface.php +++ b/src/Routes/Filter/RouteFilterOptionsInterface.php @@ -28,8 +28,10 @@ public function getPath(): string|null; /** * Returns any route methods to filter the available routing data by + * + * @return list */ - public function getMethods(): array|string|null; + public function getMethods(): array; public function toArray(): array; } From 3083c20b64199b30010d597152a12e0f70b18cf4 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:02:30 +1000 Subject: [PATCH 12/25] Update src/Routes/Filter/RouteFilterOptions.php Co-authored-by: George Steel Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index d385b36f..c717acca 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -28,7 +28,7 @@ public function __construct( } if (is_array($methods)) { - array_walk($methods, fn(string &$value) => $value = strtoupper($value)); + $this->methods = array_map(static fn (string $value): string => strtoupper($value), $methods); $this->methods = $methods; } } From 73e9dc252152c45b25a0d0c07c77402aa2efff1b Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:12:16 +1000 Subject: [PATCH 13/25] Clean up file to match changes in recent commits Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index c717acca..6337d95c 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -5,7 +5,7 @@ namespace Mezzio\Tooling\Routes\Filter; use function array_filter; -use function array_walk; +use function array_map; use function get_object_vars; use function in_array; use function is_array; @@ -28,7 +28,7 @@ public function __construct( } if (is_array($methods)) { - $this->methods = array_map(static fn (string $value): string => strtoupper($value), $methods); + $this->methods = array_map(static fn(string $value): string => strtoupper($value), $methods); $this->methods = $methods; } } @@ -66,7 +66,7 @@ public function getPath(): string|null return $this->path; } - public function getMethods(): array|string|null + public function getMethods(): array { return $this->methods; } From cd023e6d5f41f0f57e61ce93761747adcf3dfd78 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:12:38 +1000 Subject: [PATCH 14/25] Update README to match style guide Thanks to @froschdesign for pointing this out in https://github.com/mezzio/mezzio-tooling/pull/58#discussion_r1850128201. Signed-off-by: Matthew Setter --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 48d3a635..995b4038 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ The command supports several options, listed in the table below. By default, `Mezzio\Tooling\Routes\DefaultRoutesConfigLoaderFactory` registers a `ConfigLoaderInterface` service with the application's DI container, which retrieves the application's routes from two sources: -- _config/routes.php_ +- `config/routes.php` - Routes registered by any loaded `ConfigProvider` class However, this is a default/fallback implementation. From 73626810ebd4b2a24d9e09406302cdb322911ce7 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:14:15 +1000 Subject: [PATCH 15/25] Remove redundant EmptyRouteFilterOptions As pointed out by @gsteel, this implementation is not required. Signed-off-by: Matthew Setter --- src/Routes/Filter/EmptyRouteFilterOptions.php | 38 ------------------- src/Routes/ListRoutesCommandFactory.php | 3 +- test/Routes/ListRoutesCommandTest.php | 27 ++++--------- 3 files changed, 8 insertions(+), 60 deletions(-) delete mode 100644 src/Routes/Filter/EmptyRouteFilterOptions.php diff --git a/src/Routes/Filter/EmptyRouteFilterOptions.php b/src/Routes/Filter/EmptyRouteFilterOptions.php deleted file mode 100644 index 35a04cd9..00000000 --- a/src/Routes/Filter/EmptyRouteFilterOptions.php +++ /dev/null @@ -1,38 +0,0 @@ -get(ConfigLoaderInterface::class); - return new ListRoutesCommand($routeCollector, $configLoader, new EmptyRouteFilterOptions()); + return new ListRoutesCommand($routeCollector, $configLoader); } } diff --git a/test/Routes/ListRoutesCommandTest.php b/test/Routes/ListRoutesCommandTest.php index dc983920..0bf9d1b9 100644 --- a/test/Routes/ListRoutesCommandTest.php +++ b/test/Routes/ListRoutesCommandTest.php @@ -7,7 +7,6 @@ use Mezzio\Router\Route; use Mezzio\Router\RouteCollector; use Mezzio\Tooling\Routes\ConfigLoaderInterface; -use Mezzio\Tooling\Routes\Filter\EmptyRouteFilterOptions; use Mezzio\Tooling\Routes\ListRoutesCommand; use MezzioTest\Tooling\Routes\Middleware\ExpressMiddleware; use MezzioTest\Tooling\Routes\Middleware\SimpleMiddleware; @@ -44,11 +43,7 @@ protected function setUp(): void $this->input = $this->createMock(InputInterface::class); $this->output = $this->createMock(ConsoleOutputInterface::class); $this->routeCollection = $this->createMock(RouteCollector::class); - $this->command = new ListRoutesCommand( - $this->routeCollection, - $configLoader, - new EmptyRouteFilterOptions() - ); + $this->command = new ListRoutesCommand($this->routeCollection, $configLoader); } /** @@ -130,7 +125,7 @@ public function testSuccessfulExecutionEmitsExpectedOutput(): void /** @var ConfigLoaderInterface&MockObject $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader); $outputFormatter = new OutputFormatter(false); @@ -177,7 +172,7 @@ public function testRendersAnEmptyResultWhenNoRoutesArePresent(): void ->expects($this->once()) ->method('load'); - $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader); $this->input ->method('getOption') @@ -221,7 +216,7 @@ public function testRendersRoutesAsJsonWhenFormatSetToJson(): void ->expects($this->once()) ->method('load'); - $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader); $this->input ->method('getOption') @@ -269,7 +264,7 @@ public function testThatOnlyAllowedFormatsCanBeSupplied(string $format): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($collector, $configLoader, new EmptyRouteFilterOptions()); + $this->command = new ListRoutesCommand($collector, $configLoader); $this->input ->method('getOption') @@ -339,11 +334,7 @@ public function testCanSortResults(string $sortOrder): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand( - $this->routeCollection, - $configLoader, - new EmptyRouteFilterOptions() - ); + $this->command = new ListRoutesCommand($this->routeCollection, $configLoader); $this->input ->method('getOption') @@ -409,11 +400,7 @@ public function testCanFilterRoutingTable(array $filterOptions): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand( - $routeCollection, - $configLoader, - new EmptyRouteFilterOptions() - ); + $this->command = new ListRoutesCommand($routeCollection, $configLoader); $this->input ->method('getOption') From 9969934de441c00c562596873e74442af09fa524 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:15:25 +1000 Subject: [PATCH 16/25] Update RouteFilterOptions to match RouteFilterOptionsInterface Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index 6337d95c..7017f9e5 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -16,12 +16,12 @@ final class RouteFilterOptions implements RouteFilterOptionsInterface { private array $allowedFilterOptions = ['methods', 'middleware', 'name', 'path']; - /** @param string|null|array $methods */ + /** @param list $methods */ public function __construct( private string|null $middleware, private string|null $name, private string|null $path, - private array|string|null $methods + private array $methods ) { if (is_string($methods)) { $this->methods = [strtoupper($methods)]; From 3b83456085119abc65fdca90c5f3f56856782074 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:16:15 +1000 Subject: [PATCH 17/25] Remove unrequired RouteFilterOptionsInterface parameter As @gsteel pointed out, this was provided in the constructor but overwritten later, making it redundant. Signed-off-by: Matthew Setter --- src/Routes/ListRoutesCommand.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index 23e0519e..632ce0a0 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -12,6 +12,7 @@ use Mezzio\Tooling\Routes\Filter\RoutesFilter; use Mezzio\Tooling\Routes\Sorter\RouteSorterByName; use Mezzio\Tooling\Routes\Sorter\RouteSorterByPath; +use Psr\Container\ContainerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputInterface; @@ -145,7 +146,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->configLoader->load(); - $this->routes = $this->routeCollector->getRoutes(); + /** @var RouteCollector $routeCollector */ + $routeCollector = $this->container->get(RouteCollector::class); + $this->routes = $routeCollector->getRoutes(); + if ([] === $this->routes) { $output->writeln(self::MSG_EMPTY_ROUTING_TABLE); return $result; @@ -188,7 +192,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $result; } - private function getRows(bool $requireNames = false): array + public function getRows(bool $requireNames = false): array { $rows = []; @@ -220,7 +224,7 @@ private function getRows(bool $requireNames = false): array return $rows; } - private function getSortOrder(InputInterface $input): string + public function getSortOrder(InputInterface $input): string { $sortOrder = strtolower((string) $input->getOption('sort')); return ! in_array($sortOrder, ['name', 'path']) From 6c60b573121b9414bdac5518040ed42dace21f1c Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:23:05 +1000 Subject: [PATCH 18/25] Update ConfigProvider configuration for RoutesFileConfigLoader As per @gsteel's recommendation in https://github.com/mezzio/mezzio-tooling/pull/58#discussion_r1842207832, this change changes how the class is registered with the DI container. Signed-off-by: Matthew Setter --- src/ConfigProvider.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index 0b424357..3ed30a87 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -29,6 +29,7 @@ use Mezzio\Tooling\Routes\DefaultRoutesConfigLoaderFactory; use Mezzio\Tooling\Routes\ListRoutesCommand; use Mezzio\Tooling\Routes\ListRoutesCommandFactory; +use Mezzio\Tooling\Routes\RoutesFileConfigLoader; final class ConfigProvider { @@ -67,8 +68,10 @@ public function getConsoleConfig(): array public function getDependencies(): array { return [ + 'aliases' => [ + ConfigLoaderInterface::class => RoutesFileConfigLoader::class, + ], 'factories' => [ - ConfigLoaderInterface::class => DefaultRoutesConfigLoaderFactory::class, Create::class => CreateFactory::class, CreateActionCommand::class => CreateActionCommandFactory::class, CreateCommand::class => CreateCommandFactory::class, @@ -80,6 +83,7 @@ public function getDependencies(): array MigrateInteropMiddlewareCommand::class => MigrateInteropMiddlewareCommandFactory::class, MigrateMiddlewareToRequestHandlerCommand::class => MigrateMiddlewareToRequestHandlerCommandFactory::class, RegisterCommand::class => RegisterCommandFactory::class, + RoutesFileConfigLoader::class => DefaultRoutesConfigLoaderFactory::class, ], ]; } From 925c90ffcc7b3d34bdede1402f62e6ca30bce1b0 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:36:35 +1000 Subject: [PATCH 19/25] Remove redundant toArray method from RouteFilterOptions As pointed out by @gsteel, the method is not used outside of tests, so can be removed. Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 16 --------- .../Filter/RouteFilterOptionsInterface.php | 2 -- test/Routes/Filter/RouteFilterOptionsTest.php | 24 -------------- test/Routes/RoutesFilterTest.php | 33 ++++++++++++------- 4 files changed, 21 insertions(+), 54 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index 7017f9e5..a97e1c95 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -70,20 +70,4 @@ public function getMethods(): array { return $this->methods; } - - public function toArray(): array - { - $values = []; - foreach (get_object_vars($this) as $key => $value) { - if (in_array($key, $this->getFilterOptionsMinusMethods()) && $value !== "") { - $values[$key] = $value; - } - - if ($key === "methods" && ! empty($value)) { - $values[$key] = $value; - } - } - - return $values; - } } diff --git a/src/Routes/Filter/RouteFilterOptionsInterface.php b/src/Routes/Filter/RouteFilterOptionsInterface.php index 426f748c..b7a29781 100644 --- a/src/Routes/Filter/RouteFilterOptionsInterface.php +++ b/src/Routes/Filter/RouteFilterOptionsInterface.php @@ -32,6 +32,4 @@ public function getPath(): string|null; * @return list */ public function getMethods(): array; - - public function toArray(): array; } diff --git a/test/Routes/Filter/RouteFilterOptionsTest.php b/test/Routes/Filter/RouteFilterOptionsTest.php index 9bd8c1f0..efdfebdc 100644 --- a/test/Routes/Filter/RouteFilterOptionsTest.php +++ b/test/Routes/Filter/RouteFilterOptionsTest.php @@ -37,30 +37,6 @@ public function testCanInitialiseOptionsCorrectly(): void $this->assertSame($options['methods'], $routeFilterOptions->getMethods()); } - /** - * @param array $options - * @param array $expectedResult - * @dataProvider initDataProvider - */ - public function testCanGetArrayRepresentation(array $options, array $expectedResult): void - { - /** @var string $middleware */ - $middleware = $options['middleware'] ?? ''; - - /** @var string $name */ - $name = $options['name'] ?? ''; - - /** @var string $path */ - $path = $options['path'] ?? ''; - - /** @var array $methods */ - $methods = $options['methods'] ?? []; - - $routeFilterOptions = new RouteFilterOptions($middleware, $name, $path, $methods); - - $this->assertEquals($expectedResult, $routeFilterOptions->toArray()); - } - /** * @return array>>> */ diff --git a/test/Routes/RoutesFilterTest.php b/test/Routes/RoutesFilterTest.php index 50e2dc2c..067185f3 100644 --- a/test/Routes/RoutesFilterTest.php +++ b/test/Routes/RoutesFilterTest.php @@ -93,7 +93,7 @@ public static function validFilterDataProvider(): array middleware: 'ExpressMiddleware', name: null, path: null, - methods: null, + methods: [], ), ], 'middleware-simple-class-name' => [ @@ -102,7 +102,7 @@ public static function validFilterDataProvider(): array middleware: 'Tooling', name: null, path: null, - methods: null, + methods: [], ), ], 'middleware-regex' => [ @@ -111,7 +111,7 @@ public static function validFilterDataProvider(): array middleware: 'Tooling.*Middleware', name: null, path: null, - methods: null, + methods: [], ), ], 'middleware-fqcn' => [ @@ -120,7 +120,7 @@ public static function validFilterDataProvider(): array middleware: ExpressMiddleware::class, name: null, path: null, - methods: null, + methods: [], ), ], 'name-bare' => [ @@ -129,7 +129,7 @@ public static function validFilterDataProvider(): array name: 'home', middleware: null, path: null, - methods: null, + methods: [], ), ], 'name-regex' => [ @@ -138,7 +138,7 @@ public static function validFilterDataProvider(): array name: 'user.*', middleware: null, path: null, - methods: null, + methods: [], ), ], 'path-fq' => [ @@ -147,7 +147,7 @@ public static function validFilterDataProvider(): array path: '/user', middleware: null, name: null, - methods: null, + methods: [], ), ], 'path-fq-regex' => [ @@ -156,7 +156,7 @@ public static function validFilterDataProvider(): array path: '/log.*', middleware: null, name: null, - methods: null, + methods: [], ), ], 'path-root' => [ @@ -165,7 +165,7 @@ public static function validFilterDataProvider(): array path: '/', middleware: null, name: null, - methods: null, + methods: [], ), ], 'method-get' => [ @@ -180,7 +180,7 @@ public static function validFilterDataProvider(): array 'method-any' => [ 6, new RouteFilterOptions( - methods: Route::HTTP_METHOD_ANY, + methods: [Route::HTTP_METHOD_ANY], middleware: null, name: null, path: null, @@ -189,7 +189,7 @@ public static function validFilterDataProvider(): array 'method-get-lc' => [ 6, new RouteFilterOptions( - methods: 'get', + methods: ['get'], middleware: null, name: null, path: null, @@ -198,7 +198,7 @@ public static function validFilterDataProvider(): array 'method-post-lc' => [ 2, new RouteFilterOptions( - methods: 'post', + methods: ['post'], middleware: null, name: null, path: null, @@ -222,6 +222,15 @@ public static function validFilterDataProvider(): array path: null, ), ], + [ + 6, + new RouteFilterOptions( + methods: ['get'], + middleware: null, + name: null, + path: null, + ), + ], [ 6, new RouteFilterOptions( From dac21ebd2f2c03838924578a47a3c638e0779655 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 10 Dec 2024 20:38:39 +1000 Subject: [PATCH 20/25] Fix up a small bug in RouteFilterOptions This change fixes a small bug in the class constructor that overrides the work sanitising the method data provided in the call to array_map. Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index a97e1c95..614b9f5b 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -15,22 +15,16 @@ final class RouteFilterOptions implements RouteFilterOptionsInterface { private array $allowedFilterOptions = ['methods', 'middleware', 'name', 'path']; + private array $methods; /** @param list $methods */ public function __construct( private string|null $middleware, private string|null $name, private string|null $path, - private array $methods + array $methods ) { - if (is_string($methods)) { - $this->methods = [strtoupper($methods)]; - } - - if (is_array($methods)) { - $this->methods = array_map(static fn(string $value): string => strtoupper($value), $methods); - $this->methods = $methods; - } + $this->methods = array_map(static fn(string $value): string => strtoupper($value), $methods); } private function getFilterOptionsMinusMethods(): array From fbc82c8b35be33a126294954384e552a5cdeb2f1 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 17 Dec 2024 09:02:23 +1000 Subject: [PATCH 21/25] Clean up code to reflect that getMethods only returns an array Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 8 ++++---- src/Routes/Filter/RoutesFilter.php | 6 ++---- src/Routes/ListRoutesCommand.php | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index 614b9f5b..60f5972f 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -6,10 +6,7 @@ use function array_filter; use function array_map; -use function get_object_vars; use function in_array; -use function is_array; -use function is_string; use function strtoupper; final class RouteFilterOptions implements RouteFilterOptionsInterface @@ -42,7 +39,7 @@ public function has(string $filterOption): bool return $this->$filterOption !== null; } - return $this->methods !== [] && $this->methods !== null; + return $this->methods !== []; } public function getMiddleware(): string|null @@ -60,6 +57,9 @@ public function getPath(): string|null return $this->path; } + /** + * @return array + */ public function getMethods(): array { return $this->methods; diff --git a/src/Routes/Filter/RoutesFilter.php b/src/Routes/Filter/RoutesFilter.php index be208077..a9f99e14 100644 --- a/src/Routes/Filter/RoutesFilter.php +++ b/src/Routes/Filter/RoutesFilter.php @@ -11,7 +11,6 @@ use function array_intersect; use function in_array; -use function is_string; use function preg_match; use function sprintf; use function str_replace; @@ -125,10 +124,9 @@ private function matchesByMethod(Route $route): bool return true; } - $methods = $this->filterOptions->getMethods() ?? []; return array_intersect( - is_string($methods) ? [$methods] : $methods, - $route->getAllowedMethods() ?? [] + $this->filterOptions->getMethods(), + $route->getAllowedMethods() ) !== []; } diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index 632ce0a0..86b7429c 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -203,7 +203,7 @@ public function getRows(bool $requireNames = false): array /** @var Route $route */ foreach ($routesIterator as $route) { - $routeMethods = implode(',', $route->getAllowedMethods() ?? []); + $routeMethods = implode(',', $route->getAllowedMethods()); if ($requireNames) { $rows[] = [ 'name' => $route->getName(), From 17ba63957fd01173fab0bf55f9b82e3bf6486211 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Tue, 17 Dec 2024 09:03:20 +1000 Subject: [PATCH 22/25] Use Symfony's CommandTester in ListRoutesCommandTest Thanks for feedback from @gsteel, this change introduces Symfony's CommandTester in ListRoutesCommandTest to simplify the tests and to make them (far) more maintainable. Signed-off-by: Matthew Setter --- src/Routes/Filter/RoutesFilter.php | 4 +- src/Routes/ListRoutesCommand.php | 2 +- test/Routes/ListRoutesCommandTest.php | 274 +++++++++----------------- 3 files changed, 93 insertions(+), 187 deletions(-) diff --git a/src/Routes/Filter/RoutesFilter.php b/src/Routes/Filter/RoutesFilter.php index a9f99e14..921e7800 100644 --- a/src/Routes/Filter/RoutesFilter.php +++ b/src/Routes/Filter/RoutesFilter.php @@ -102,7 +102,7 @@ private function matchesByRegex(Route $route, string $routeAttribute): bool } if ($routeAttribute === 'path') { - $path = $this->filterOptions->getPath(); + $path = (string) $this->filterOptions->getPath(); return (bool) preg_match( sprintf("/^%s/", str_replace('/', '\/', $path)), $route->getPath() @@ -110,7 +110,7 @@ private function matchesByRegex(Route $route, string $routeAttribute): bool } return (bool) preg_match( - sprintf("/%s/", $this->filterOptions->getName()), + sprintf("/%s/", (string) $this->filterOptions->getName()), $route->getName() ); } diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index 86b7429c..632ce0a0 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -203,7 +203,7 @@ public function getRows(bool $requireNames = false): array /** @var Route $route */ foreach ($routesIterator as $route) { - $routeMethods = implode(',', $route->getAllowedMethods()); + $routeMethods = implode(',', $route->getAllowedMethods() ?? []); if ($requireNames) { $rows[] = [ 'name' => $route->getName(), diff --git a/test/Routes/ListRoutesCommandTest.php b/test/Routes/ListRoutesCommandTest.php index 0bf9d1b9..c1410bfa 100644 --- a/test/Routes/ListRoutesCommandTest.php +++ b/test/Routes/ListRoutesCommandTest.php @@ -14,24 +14,17 @@ use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionException; -use ReflectionMethod; -use Symfony\Component\Console\Formatter\OutputFormatter; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\ConsoleOutputInterface; +use Symfony\Component\Console\Tester\CommandTester; +use function array_key_exists; +use function implode; use function str_replace; use function strtoupper; class ListRoutesCommandTest extends TestCase { - /** @var (InputInterface&MockObject) */ - private $input; - - /** @var (ConsoleOutputInterface&MockObject) */ - private $output; - /** @var (RouteCollector&MockObject) */ - private $routeCollection; + private $routeCollector; private ListRoutesCommand $command; @@ -40,18 +33,8 @@ protected function setUp(): void /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->input = $this->createMock(InputInterface::class); - $this->output = $this->createMock(ConsoleOutputInterface::class); - $this->routeCollection = $this->createMock(RouteCollector::class); - $this->command = new ListRoutesCommand($this->routeCollection, $configLoader); - } - - /** - * @throws ReflectionException - */ - private function reflectExecuteMethod(): ReflectionMethod - { - return new ReflectionMethod($this->command, 'execute'); + $this->routeCollector = $this->createMock(RouteCollector::class); + $this->command = new ListRoutesCommand($this->routeCollector, $configLoader); } public function testConfigureSetsExpectedDescription(): void @@ -127,38 +110,22 @@ public function testSuccessfulExecutionEmitsExpectedOutput(): void $configLoader = $this->createMock(ConfigLoaderInterface::class); $this->command = new ListRoutesCommand($collector, $configLoader); - $outputFormatter = new OutputFormatter(false); + $command = new CommandTester($this->command); + $command->execute([]); - $this->output - ->method('getFormatter') - ->willReturn($outputFormatter); + $command->assertCommandIsSuccessful(); + $output = $command->getDisplay(); + $commandOutput = <<output - ->expects($this->atMost(7)) - ->method('writeln'); - // phpcs:enable - $this->input - ->method('getOption') - ->willReturnOnConsecutiveCalls( - 'table', - 'name', - false, - false, - false, - false - ); - - $method = $this->reflectExecuteMethod(); +EOF; - self::assertSame( - 0, - $method->invoke( - $this->command, - $this->input, - $this->output - ) - ); + $this->assertSame($commandOutput, $output); } public function testRendersAnEmptyResultWhenNoRoutesArePresent(): void @@ -174,27 +141,13 @@ public function testRendersAnEmptyResultWhenNoRoutesArePresent(): void $this->command = new ListRoutesCommand($collector, $configLoader); - $this->input - ->method('getOption') - ->with('format') - ->willReturnOnConsecutiveCalls('table', false); - $this->output - ->expects($this->once()) - ->method('writeln') - ->with( - "There are no routes in the application's routing table." - ); - - $method = $this->reflectExecuteMethod(); + $command = new CommandTester($this->command); + $command->execute([]); - self::assertSame( - 0, - $method->invoke( - $this->command, - $this->input, - $this->output - ) - ); + $command->assertCommandIsSuccessful(); + $output = $command->getDisplay(); + $commandOutput = "There are no routes in the application's routing table.\n"; + $this->assertSame($commandOutput, $output); } public function testRendersRoutesAsJsonWhenFormatSetToJson(): void @@ -217,31 +170,21 @@ public function testRendersRoutesAsJsonWhenFormatSetToJson(): void ->method('load'); $this->command = new ListRoutesCommand($collector, $configLoader); + $command = new CommandTester($this->command); + $command->execute([ + '--format' => 'json', + ]); - $this->input - ->method('getOption') - ->willReturnOnConsecutiveCalls( - 'json', // format - false, // supports-method - false, // has-middleware - false, // has-name - false, // has-path - false - ); - $this->output - ->expects($this->atMost(2)) - ->method('writeln'); - - $method = $this->reflectExecuteMethod(); - - self::assertSame( - 0, - $method->invoke( - $this->command, - $this->input, - $this->output - ) - ); + $command->assertCommandIsSuccessful(); + $output = $command->getDisplay(); + // phpcs:disable Generic.Files.LineLength + $commandOutput = <<assertSame($output, $commandOutput); } /** @@ -266,33 +209,14 @@ public function testThatOnlyAllowedFormatsCanBeSupplied(string $format): void $configLoader = $this->createMock(ConfigLoaderInterface::class); $this->command = new ListRoutesCommand($collector, $configLoader); - $this->input - ->method('getOption') - ->willReturnOnConsecutiveCalls( - $format, // format - false, // has-middleware - false, // supports-method - false, // has-name - false, // has-path - false // sort - ); - $this->output - ->expects($this->once()) - ->method('writeln') - ->with( - "Invalid output format supplied. Valid options are 'table' and 'json'" - ); - - $method = $this->reflectExecuteMethod(); + $command = new CommandTester($this->command); + $command->execute([ + '--format' => $format, + ]); - self::assertSame( - -1, - $method->invoke( - $this->command, - $this->input, - $this->output - ) - ); + $output = $command->getDisplay(); + $commandOutput = "Invalid output format supplied. Valid options are 'table' and 'json'\n"; + $this->assertSame($commandOutput, $output); } /** @@ -320,46 +244,31 @@ public static function invalidFormatDataProvider(): array * @dataProvider sortRoutingTableDataProvider * @throws ReflectionException */ - public function testCanSortResults(string $sortOrder): void + public function testCanSortResults(string $sortOrder, string $commandOutput): void { - $routes = [ + $routes = [ new Route("/contact", new SimpleMiddleware(), ['GET'], 'contact'), new Route("/", new ExpressMiddleware(), ['GET'], 'home'), ]; - $this->routeCollection = $this->createMock(RouteCollector::class); - $this->routeCollection + $this->routeCollector = $this->createMock(RouteCollector::class); + $this->routeCollector ->expects($this->once()) ->method('getRoutes') ->willReturn($routes); /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($this->routeCollection, $configLoader); - - $this->input - ->method('getOption') - ->willReturnOnConsecutiveCalls( - 'json', // format - $sortOrder, // sort - false, // supports-method - false, // has-middleware - false, // has-name - false // has-path - ); - $this->output - ->expects($this->atMost(2)) - ->method('writeln'); - - $method = $this->reflectExecuteMethod(); - - self::assertSame( - 0, - $method->invoke( - $this->command, - $this->input, - $this->output - ) - ); + $this->command = new ListRoutesCommand($this->routeCollector, $configLoader); + + $command = new CommandTester($this->command); + $command->execute([ + '--format' => "json", + '--sort' => $sortOrder, + ]); + + $command->assertCommandIsSuccessful(); + $output = $command->getDisplay(); + $this->assertSame($commandOutput, $output); } /** @@ -371,11 +280,17 @@ public static function sortRoutingTableDataProvider(): array return [ [ 'name', - '[{"name":"contact","path":"\/contact","methods":"GET","middleware":"MezzioTest\\\\Tooling\\\\Routes\\\\Middleware\\\\SimpleMiddleware"},{"name":"home","path":"\/","methods":"GET","middleware":"MezzioTest\\\\Tooling\\\\Routes\\\\Middleware\\\\ExpressMiddleware"}]', + <<createMock(RouteCollector::class); - $routeCollection + /** @var RouteCollector&MockObject $routeCollector */ + $routeCollector = $this->createMock(RouteCollector::class); + $routeCollector ->expects($this->once()) ->method('getRoutes') ->willReturn($routes); /** @var ConfigLoaderInterface $configLoader */ $configLoader = $this->createMock(ConfigLoaderInterface::class); - $this->command = new ListRoutesCommand($routeCollection, $configLoader); - - $this->input - ->method('getOption') - ->willReturnOnConsecutiveCalls( - 'json', // format - false, // sort - false, // supports-method - $filterOptions['middleware'], // has-middleware - false, // has-name - false // has-path - ); + $this->command = new ListRoutesCommand($routeCollector, $configLoader); - $this->output - ->expects($this->atMost(2)) - ->method('writeln'); - - $method = $this->reflectExecuteMethod(); + $command = new CommandTester($this->command); + $commandOptions = [ + '--format' => "json", + ]; + if (array_key_exists("middleware", $filterOptions)) { + $commandOptions['--has-middleware'] = implode(",", $filterOptions["middleware"]); + } + $command->execute($commandOptions); - self::assertSame( - 0, - $method->invoke( - $this->command, - $this->input, - $this->output - ) - ); + $output = $command->getDisplay(); + $this->assertSame($commandOutput, $output); + $command->assertCommandIsSuccessful(); } /** @@ -437,7 +339,11 @@ public static function filterRoutingTableDataProvider(): array // phpcs:disable Generic.Files.LineLength return [ [ - ['middleware' => 'ExpressMiddleware'], + ['middleware' => ['ExpressMiddleware']], + << Date: Thu, 19 Dec 2024 14:15:34 +1000 Subject: [PATCH 23/25] Fix a bug when filtering out null methods This change handles if null methods or Route::HTTP_METHOD_ANY is listed as a method. Signed-off-by: Matthew Setter --- src/Routes/Filter/RouteFilterOptions.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Routes/Filter/RouteFilterOptions.php b/src/Routes/Filter/RouteFilterOptions.php index 60f5972f..ad77051b 100644 --- a/src/Routes/Filter/RouteFilterOptions.php +++ b/src/Routes/Filter/RouteFilterOptions.php @@ -21,6 +21,7 @@ public function __construct( private string|null $path, array $methods ) { + $methods = array_filter($methods, 'strlen'); $this->methods = array_map(static fn(string $value): string => strtoupper($value), $methods); } From 94f1ce1ed474ca6a96171e99a440b24767255b99 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 19 Dec 2024 14:19:26 +1000 Subject: [PATCH 24/25] Initialise RoutesFilter with a RouteFilterOptions object This change uses a RouteFilterOptions object instead of an array, as the class is much more specific than an array and was already available in the source tree. Signed-off-by: Matthew Setter --- src/Routes/Filter/RoutesFilter.php | 10 ++-------- src/Routes/ListRoutesCommand.php | 5 +---- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/Routes/Filter/RoutesFilter.php b/src/Routes/Filter/RoutesFilter.php index 921e7800..699da7f5 100644 --- a/src/Routes/Filter/RoutesFilter.php +++ b/src/Routes/Filter/RoutesFilter.php @@ -48,15 +48,9 @@ public function __construct( * Middleware can only contain a class name. Method can be either a string which contains one of the * allowed HTTP methods, or an array of HTTP methods. */ - private array $filterOptions = [] + private RouteFilterOptions $filterOptions ) { parent::__construct($routes); - - // Filter out any options that are, effectively, "empty". - $this->filterOptions = array_filter( - $this->filterOptions, - fn($value) => ! empty($value) - ); } public function getFilterOptions(): RouteFilterOptionsInterface @@ -142,7 +136,7 @@ private function matchesByMethod(Route $route): bool private function matchesByMiddleware(Route $route): bool { $middlewareClass = $route->getMiddleware()::class; - $matchesMiddleware = (string) $this->filterOptions['middleware']; + $matchesMiddleware = $this->filterOptions->getMiddleware(); try { return $middlewareClass === $matchesMiddleware diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index 632ce0a0..fa728810 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -8,11 +8,9 @@ use Mezzio\Router\Route; use Mezzio\Router\RouteCollector; use Mezzio\Tooling\Routes\Filter\RouteFilterOptions; -use Mezzio\Tooling\Routes\Filter\RouteFilterOptionsInterface; use Mezzio\Tooling\Routes\Filter\RoutesFilter; use Mezzio\Tooling\Routes\Sorter\RouteSorterByName; use Mezzio\Tooling\Routes\Sorter\RouteSorterByPath; -use Psr\Container\ContainerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputInterface; @@ -30,8 +28,7 @@ class ListRoutesCommand extends Command /** @var array */ private array $routes = []; - /** @var array */ - private array $filterOptions = []; + private RouteFilterOptions $filterOptions; private const HELP = <<<'EOT' Prints the application's routing table. From d7380b5e0d1741c38cec8318755f9ac369806645 Mon Sep 17 00:00:00 2001 From: Matthew Setter Date: Thu, 19 Dec 2024 14:21:32 +1000 Subject: [PATCH 25/25] Correct code regression from recent rebase Signed-off-by: Matthew Setter --- src/Routes/ListRoutesCommand.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Routes/ListRoutesCommand.php b/src/Routes/ListRoutesCommand.php index fa728810..e2ebf423 100644 --- a/src/Routes/ListRoutesCommand.php +++ b/src/Routes/ListRoutesCommand.php @@ -79,7 +79,7 @@ class ListRoutesCommand extends Command public static $defaultName = 'mezzio:routes:list'; public function __construct( - private readonly ContainerInterface $container, + private readonly RouteCollector $routeCollector, private readonly ConfigLoaderInterface $configLoader ) { parent::__construct(); @@ -143,9 +143,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->configLoader->load(); - /** @var RouteCollector $routeCollector */ - $routeCollector = $this->container->get(RouteCollector::class); - $this->routes = $routeCollector->getRoutes(); + $this->routes = $this->routeCollector->getRoutes(); if ([] === $this->routes) { $output->writeln(self::MSG_EMPTY_ROUTING_TABLE);