Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: DefinedRouteCollector to use RouteCollectionInterface #8911

Merged
merged 6 commits into from
Jun 15, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 3 additions & 28 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -7197,34 +7197,9 @@
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getDefaultNamespace\\(\\)\\.$#',
'count' => 2,
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getFiltersForRoute\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getRegisteredControllers\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getRoutesOptions\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:isFiltered\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:setHTTPVerb\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Router/Router.php',
'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getRegisteredControllers\\(\\)\\.$#',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are fixing the interface, why are we not adding this missing method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method is only for AutoRouting Improved, and marked as @interal.
I don't think the RouteCollectionInterface should have it.

'count' => 1,
'path' => __DIR__ . '/system/Router/Router.php',
];
$ignoreErrors[] = [
'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
Expand Down
2 changes: 1 addition & 1 deletion system/Router/DefinedRouteCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
*/
final class DefinedRouteCollector
{
public function __construct(private readonly RouteCollection $routeCollection)
public function __construct(private readonly RouteCollectionInterface $routeCollection)
{
}

Expand Down
80 changes: 75 additions & 5 deletions system/Router/RouteCollectionInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@
* A Route Collection's sole job is to hold a series of routes. The required
* number of methods is kept very small on purpose, but implementors may
* add a number of additional methods to customize how the routes are defined.
*
* The RouteCollection provides the Router with the routes so that it can determine
* which controller should be run.
*/
interface RouteCollectionInterface
{
Expand Down Expand Up @@ -62,11 +59,19 @@ public function addPlaceholder($placeholder, ?string $pattern = null);
*/
public function setDefaultNamespace(string $value);

/**
* Returns the default namespace.
*/
public function getDefaultNamespace(): string;

/**
* Sets the default controller to use when no other controller has been
* specified.
*
* @return RouteCollectionInterface
*
* @TODO The default controller is only for auto-routing. So this should be
* removed in the future.
*/
public function setDefaultController(string $value);

Expand All @@ -86,6 +91,9 @@ public function setDefaultMethod(string $value);
* doesn't work well with PHP method names....
*
* @return RouteCollectionInterface
*
* @TODO This method is only for auto-routing. So this should be removed in
* the future.
*/
public function setTranslateURIDashes(bool $value);

Expand All @@ -96,6 +104,9 @@ public function setTranslateURIDashes(bool $value);
* defined routes.
*
* If FALSE, will stop searching and do NO automatic routing.
*
* @TODO This method is only for auto-routing. So this should be removed in
* the future.
*/
public function setAutoRoute(bool $value): self;

Expand All @@ -107,6 +118,9 @@ public function setAutoRoute(bool $value): self;
* This setting is passed to the Router class and handled there.
*
* @param callable|null $callable
*
* @TODO This method is not related to the route collection. So this should
* be removed in the future.
*/
public function set404Override($callable = null): self;

Expand All @@ -115,13 +129,19 @@ public function set404Override($callable = null): self;
* or the controller/string.
*
* @return (Closure(string): (ResponseInterface|string|void))|string|null
*
* @TODO This method is not related to the route collection. So this should
* be removed in the future.
*/
public function get404Override();

/**
* Returns the name of the default controller. With Namespace.
*
* @return string
*
* @TODO The default controller is only for auto-routing. So this should be
* removed in the future.
*/
public function getDefaultController();

Expand All @@ -136,22 +156,48 @@ public function getDefaultMethod();
* Returns the current value of the translateURIDashes setting.
*
* @return bool
*
* @TODO This method is only for auto-routing. So this should be removed in
* the future.
*/
public function shouldTranslateURIDashes();

/**
* Returns the flag that tells whether to autoRoute URI against Controllers.
*
* @return bool
*
* @TODO This method is only for auto-routing. So this should be removed in
* the future.
*/
public function shouldAutoRoute();

/**
* Returns the raw array of available routes.
*
* @return array
* @param non-empty-string|null $verb HTTP verb like `GET`,`POST` or `*` or `CLI`.
* @param bool $includeWildcard Whether to include '*' routes.
*/
public function getRoutes(?string $verb = null, bool $includeWildcard = true): array;

/**
* Returns one or all routes options
*
* @param string|null $from The route path (with placeholders or regex)
* @param string|null $verb HTTP verb like `GET`,`POST` or `*` or `CLI`.
*
* @return array<string, int|string> [key => value]
*/
public function getRoutes();
public function getRoutesOptions(?string $from = null, ?string $verb = null): array;

/**
* Sets the current HTTP verb.
*
* @param string $verb HTTP verb
*
* @return $this
*/
public function setHTTPVerb(string $verb);

/**
* Returns the current HTTP Verb being used.
Expand Down Expand Up @@ -194,4 +240,28 @@ public function getRedirectCode(string $routeKey): int;
* Get the flag that limit or not the routes with {locale} placeholder to App::$supportedLocales
*/
public function shouldUseSupportedLocalesOnly(): bool;

/**
* Checks a route (using the "from") to see if it's filtered or not.
*
* @param string|null $verb HTTP verb like `GET`,`POST` or `*` or `CLI`.
*/
public function isFiltered(string $search, ?string $verb = null): bool;

/**
* Returns the filters that should be applied for a single route, along
* with any parameters it might have. Parameters are found by splitting
* the parameter name on a colon to separate the filter name from the parameter list,
* and the splitting the result on commas. So:
*
* 'role:admin,manager'
*
* has a filter of "role", with parameters of ['admin', 'manager'].
*
* @param string $search routeKey
* @param string|null $verb HTTP verb like `GET`,`POST` or `*` or `CLI`.
*
* @return list<string> filter_name or filter_name:arguments like 'role:admin,manager'
*/
public function getFiltersForRoute(string $search, ?string $verb = null): array;
}
3 changes: 1 addition & 2 deletions system/Router/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public function __construct(RouteCollectionInterface $routes, ?Request $request
);
} else {
$this->autoRouter = new AutoRouter(
$this->collection->getRoutes('CLI', false), // @phpstan-ignore-line
$this->collection->getRoutes('CLI', false),
$this->collection->getDefaultNamespace(),
$this->collection->getDefaultController(),
$this->collection->getDefaultMethod(),
Expand Down Expand Up @@ -400,7 +400,6 @@ public function getLocale()
*/
protected function checkRoutes(string $uri): bool
{
// @phpstan-ignore-next-line
$routes = $this->collection->getRoutes($this->collection->getHTTPVerb());

// Don't waste any time
Expand Down
Loading