Skip to content

Commit

Permalink
Tweaked the router to allow 'default' route parameters to be correctl…
Browse files Browse the repository at this point in the history
…y passed through to the action.
  • Loading branch information
Daniel Bettles committed Mar 26, 2023
1 parent 33c5808 commit 6491e4d
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 36 deletions.
4 changes: 4 additions & 0 deletions .markdownlint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"MD024": false,
"MD013": false
}
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased]

No unreleased changes.

## [4.0.0] - 2023-03-26

### Added

- 'Default' route parameters will be processed, and passed through to, the action.

### Removed

- `AbstractAction::createNotFoundException()` because it was a bit pointless 🤦‍♂️
Expand Down Expand Up @@ -108,7 +116,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

First stable release.

[unreleased]: https://github.com/danbettles/marigold/compare/v3.0.0...HEAD
[unreleased]: https://github.com/danbettles/marigold/compare/v4.0.0...HEAD
[4.0.0]: https://github.com/danbettles/marigold/compare/v3.0.0...v4.0.0
[3.0.0]: https://github.com/danbettles/marigold/compare/v2.5.0...v3.0.0
[2.5.0]: https://github.com/danbettles/marigold/compare/v2.4.0...v2.5.0
[2.4.0]: https://github.com/danbettles/marigold/compare/v2.3.3...v2.4.0
Expand Down
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ parameters:

typeAliases:
HeadersArray: 'array<string,string>'
RouteArray: 'array{id:string,path:string,action:mixed,parameters?:array<string,mixed>}'

ignoreErrors:
-
Expand Down
91 changes: 60 additions & 31 deletions src/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,21 @@
use function array_combine;
use function array_filter;
use function array_intersect_key;
use function array_keys;
use function array_key_exists;
use function array_keys;
use function array_replace;
use function count;
use function explode;
use function implode;
use function preg_match;
use function preg_match_all;
use function strpos;
use function str_replace;
use function strpos;

use const ARRAY_FILTER_USE_KEY;
use const false;
use const null;
use const true;

/**
* Maps paths to actions.
Expand All @@ -35,37 +37,58 @@
* 'path' => '/posts/{postId}',
* 'action' => ['FooBar', 'baz'],
* ],
* [
* 'id' => 'showArticle',
* 'path' => '/articles/{articleId}',
* 'action' => ShowArticleAction::class,
* ],
* [
* 'id' => 'showAboutPage',
* 'path' => '/about',
* 'action' => ShowArticleAction::class,
* 'parameters' => [
* 'articleId' => 123,
* ],
* ],
* // ...
* ]
*
* `action` can be anything: the name of a method; a callable; whatever's appropriate for the app.
*
* A matched route, the return value of `match()`, will have an additional element, `parameters`, containing the values
* of any parameters found in the path.
* A matched route, the return value of `match()`, will always have an additional array element, `parameters`,
* containing the values of any parameters found in the path.
*
* Default values for parameters can be supplied in the `parameters` element.
*
* @phpstan-type MatchedRouteArray array{id:string,path:string,action:mixed,parameters:array<string,mixed>}
* @phpstan-type IndexedRouteArrayArray array<string,RouteArray>
* @phpstan-type PlaceholdersArray array<string,string>
*/
class Router
{
/**
* @var array{id:null,path:null,action:null}
* @var array<string,bool>
*/
private const EMPTY_ROUTE = [
'id' => null,
'path' => null,
'action' => null,
private const ROUTE_ELEMENTS = [
// Name => mandatory?
'id' => true,
'path' => true,
'action' => true,
'parameters' => false,
];

/**
* @var array<string,array{id:string,path:string,action:mixed}>
* @phpstan-var IndexedRouteArrayArray
*/
private array $routes;

/**
* @var array<string,array<string,string>>
* @phpstan-var array<string,PlaceholdersArray>
*/
private array $placeholdersByRouteId = [];

/**
* @param array<array{id:string,path:string,action:mixed}> $routes
* @phpstan-param RouteArray[] $routes
*/
public function __construct(array $routes)
{
Expand All @@ -78,8 +101,8 @@ private function countPathParts(string $path): int
}

/**
* @param array<string,array{id:string,path:string,action:mixed}> $routes
* @return array<string,array{id:string,path:string,action:mixed}>
* @phpstan-param IndexedRouteArrayArray $routes
* @phpstan-return IndexedRouteArrayArray
*/
private function eliminateUnmatchableRoutes(string $path, array $routes): array
{
Expand All @@ -93,21 +116,24 @@ private function eliminateUnmatchableRoutes(string $path, array $routes): array
}

/**
* @param array{id:string,path:string,action:mixed} $baseRoute
* @phpstan-param RouteArray $baseRoute
* @param array<string,string> $parameters
* @return array{id:string,path:string,action:mixed,parameters:array<string,string>}
* @phpstan-return MatchedRouteArray
*/
private function createMatchedRoute(
array $baseRoute,
array $parameters = []
): array {
$baseRoute['parameters'] = $parameters;
$baseRoute['parameters'] = array_replace(
($baseRoute['parameters'] ?? []),
$parameters
);

return $baseRoute;
}

/**
* @return array{id:string,path:string,action:mixed,parameters:array<string,string>}|null
* @phpstan-return MatchedRouteArray|null
* @throws OutOfBoundsException If there is no request URI in the server vars
* @throws InvalidArgumentException If the request URI is invalid
*/
Expand Down Expand Up @@ -216,7 +242,7 @@ public function generatePath(string $routeId, array $parameters = []): string
}

/**
* @param array<array{id:string,path:string,action:mixed}> $routes
* @phpstan-param RouteArray[] $routes
* @throws InvalidArgumentException If there are no routes
* @throws InvalidArgumentException If a route is missing elements
*/
Expand All @@ -226,39 +252,42 @@ private function setRoutes(array $routes): self
throw new InvalidArgumentException('There are no routes');
}

$numExpectedRouteEls = count(self::EMPTY_ROUTE);
$mandatoryRouteElements = array_filter(self::ROUTE_ELEMENTS);
$numMandatoryRouteElements = count($mandatoryRouteElements);

foreach ($routes as $i => $route) {
$filteredRoute = array_intersect_key($route, self::EMPTY_ROUTE);
// In reality, some routes won't be valid, hence why we need to adjust PHPStan's expectations.
/** @phpstan-var mixed[] */
$filteredRoute = array_intersect_key($route, self::ROUTE_ELEMENTS);

$numMandatoryInFiltered = count(array_intersect_key($filteredRoute, $mandatoryRouteElements));

// In reality, some routes may not be what we were hoping for, hence why we need to adjust PHPStan's
// expectations.
/** @phpstan-var mixed[] $filteredRoute */
if ($numExpectedRouteEls !== count($filteredRoute)) {
if ($numMandatoryRouteElements !== $numMandatoryInFiltered) {
throw new InvalidArgumentException(
"The route at index `{$i}` is missing elements. " .
'Required: ' . implode(', ', array_keys(self::EMPTY_ROUTE)) . '.'
'Required: ' . implode(', ', array_keys($mandatoryRouteElements)) . '.'
);
}

/** @phpstan-var RouteArray $filteredRoute */

$routeId = $route['id'];
/** @var array{id:string,path:string,action:mixed} $filteredRoute */
$this->routes[$routeId] = $filteredRoute;
}

return $this;
}

/**
* @return array<string,array{id:string,path:string,action:mixed}>
* @phpstan-return IndexedRouteArrayArray
*/
public function getRoutes(): array
{
return $this->routes;
}

/**
* @return array<string,string>
* @phpstan-return PlaceholdersArray
*/
private function getRoutePlaceholders(string $routeId): array
{
Expand All @@ -269,9 +298,9 @@ private function getRoutePlaceholders(string $routeId): array
$matches = null;
$matched = (bool) preg_match_all("~\{($placeholderNamePattern)\}~", $route['path'], $matches);

/** @var array<string,string> */
/** @phpstan-var PlaceholdersArray */
$placeholders = $matched
// name => placeholder
// Name => placeholder. E.g. "articleId" => "{articleId}".
? array_combine($matches[1], $matches[0])
: []
;
Expand Down
51 changes: 47 additions & 4 deletions tests/src/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public function providesRoutesContainingInvalid(): array

/**
* @dataProvider providesRoutesContainingInvalid
* @param array<array{id:string,path:string,action:mixed}> $routesContainingInvalid (Using the valid type to silence PHPStan.)
* @phpstan-param RouteArray[] $routesContainingInvalid (Using the valid type to silence PHPStan.)
*/
public function testThrowsAnExceptionIfARouteIsInvalid(
int $routeIndex,
Expand Down Expand Up @@ -273,13 +273,56 @@ public function providesMatchableRoutes(): array
],
$this->createHttpRequest(['REQUEST_URI' => '/articles/the-quick-brown-fox']),
],
[
[
'id' => 'showAboutPage',
'path' => '/about',
'action' => 'ShowAboutPageAction',
'parameters' => [
'articleId' => 'about-page',
],
],
[
[
'id' => 'showAboutPage',
'path' => '/about',
'action' => 'ShowAboutPageAction',
'parameters' => [
'articleId' => 'about-page',
],
],
],
$this->createHttpRequest(['REQUEST_URI' => '/about']),
],
// 'Default' parameters will be overridden.
[
[
'id' => 'showArticle',
'path' => '/articles/{articleId}',
'action' => 'anything',
'parameters' => [
'articleId' => '456',
],
],
[
[
'id' => 'showArticle',
'path' => '/articles/{articleId}',
'action' => 'anything',
'parameters' => [
'articleId' => '123',
],
],
],
$this->createHttpRequest(['REQUEST_URI' => '/articles/456']),
],
];
}

/**
* @dataProvider providesMatchableRoutes
* @param array{path:string,action:mixed,parameters:string[]} $expectedRoute
* @param array<array{id:string,path:string,action:mixed}> $routes
* @phpstan-param RouteArray[] $routes
*/
public function testMatchAttemptsToFindAMatchingRoute(
array $expectedRoute,
Expand Down Expand Up @@ -332,7 +375,7 @@ public function providesUnmatchableRoutes(): array

/**
* @dataProvider providesUnmatchableRoutes
* @param array<array{id:string,path:string,action:mixed}> $unmatchableRoutes
* @phpstan-param RouteArray[] $unmatchableRoutes
*/
public function testMatchReturnsNullIfThereIsNoMatchingRoute(
array $unmatchableRoutes,
Expand Down Expand Up @@ -478,7 +521,7 @@ public function providesIncompleteArgsForGeneratepath(): array

/**
* @dataProvider providesIncompleteArgsForGeneratepath
* @param array<array{id:string,path:string,action:mixed}> $routes
* @phpstan-param RouteArray[] $routes
* @param array<string,string|int> $parameters
*/
public function testGeneratepathThrowsAnExceptionIfInsufficientParametersWerePassed(
Expand Down

0 comments on commit 6491e4d

Please sign in to comment.