From 49e206fe59cf08fa808c3fbb3ae8103008dd0503 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 24 Sep 2021 17:22:00 +0900 Subject: [PATCH 1/5] docs: add missing @param type Clouser --- system/Router/RouteCollection.php | 22 +++++++++++----------- system/Router/RouteCollectionInterface.php | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index e975a1ac86b9..87aeaa0486a0 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -512,7 +512,7 @@ public function map(array $routes = [], ?array $options = null): RouteCollection * Example: * $routes->add('news', 'Posts::index'); * - * @param array|string $to + * @param array|Closure|string $to */ public function add(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -821,7 +821,7 @@ public function presenter(string $name, ?array $options = null): RouteCollection * Example: * $route->match( ['get', 'post'], 'users/(:num)', 'users/$1); * - * @param array|string $to + * @param array|Closure|string $to */ public function match(array $verbs = [], string $from = '', $to = '', ?array $options = null): RouteCollectionInterface { @@ -841,7 +841,7 @@ public function match(array $verbs = [], string $from = '', $to = '', ?array $op /** * Specifies a route that is only available to GET requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function get(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -853,7 +853,7 @@ public function get(string $from, $to, ?array $options = null): RouteCollectionI /** * Specifies a route that is only available to POST requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function post(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -865,7 +865,7 @@ public function post(string $from, $to, ?array $options = null): RouteCollection /** * Specifies a route that is only available to PUT requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function put(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -877,7 +877,7 @@ public function put(string $from, $to, ?array $options = null): RouteCollectionI /** * Specifies a route that is only available to DELETE requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function delete(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -889,7 +889,7 @@ public function delete(string $from, $to, ?array $options = null): RouteCollecti /** * Specifies a route that is only available to HEAD requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function head(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -901,7 +901,7 @@ public function head(string $from, $to, ?array $options = null): RouteCollection /** * Specifies a route that is only available to PATCH requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function patch(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -913,7 +913,7 @@ public function patch(string $from, $to, ?array $options = null): RouteCollectio /** * Specifies a route that is only available to OPTIONS requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function options(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -925,7 +925,7 @@ public function options(string $from, $to, ?array $options = null): RouteCollect /** * Specifies a route that is only available to command-line requests. * - * @param array|string $to + * @param array|Closure|string $to */ public function cli(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1083,7 +1083,7 @@ protected function fillRouteParams(string $from, ?array $params = null): string * the request method(s) that this route will work for. They can be separated * by a pipe character "|" if there is more than one. * - * @param array|string $to + * @param array|Closure|string $to */ protected function create(string $verb, string $from, $to, ?array $options = null) { diff --git a/system/Router/RouteCollectionInterface.php b/system/Router/RouteCollectionInterface.php index da25077da0fb..a55b5d80a0b9 100644 --- a/system/Router/RouteCollectionInterface.php +++ b/system/Router/RouteCollectionInterface.php @@ -28,8 +28,8 @@ interface RouteCollectionInterface /** * Adds a single route to the collection. * - * @param array|string $to - * @param array $options + * @param array|Closure|string $to + * @param array $options * * @return mixed */ From bc94d3ada7741a75ceb76dc362d091c05c099c41 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 24 Sep 2021 17:28:17 +0900 Subject: [PATCH 2/5] feat: add classname filter --- system/Filters/Filters.php | 4 +++- tests/system/Router/RouterTest.php | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 9e3a0aa7913d..04fd0e9d0a75 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -334,7 +334,9 @@ public function enableFilter(string $name, string $when = 'before') $this->arguments[$name] = $params; } - if (! array_key_exists($name, $this->config->aliases)) { + if (class_exists($name)) { + $this->config->aliases[$name] = $name; + } elseif (! array_key_exists($name, $this->config->aliases)) { throw FilterException::forNoAlias($name); } diff --git a/tests/system/Router/RouterTest.php b/tests/system/Router/RouterTest.php index 3ef8a9e257d5..dd6ca83ad6fd 100644 --- a/tests/system/Router/RouterTest.php +++ b/tests/system/Router/RouterTest.php @@ -16,6 +16,7 @@ use CodeIgniter\HTTP\IncomingRequest; use CodeIgniter\Test\CIUnitTestCase; use Config\Modules; +use Tests\Support\Filters\Customfilter; /** * @internal @@ -529,6 +530,20 @@ static function (RouteCollection $routes) { $this->assertSame('api-auth', $router->getFilter()); } + public function testRouteWorksWithClassnameFilter() + { + $collection = $this->collection; + + $collection->add('foo', 'TestController::foo', ['filter' => Customfilter::class]); + $router = new Router($collection, $this->request); + + $router->handle('foo'); + + $this->assertSame('\TestController', $router->controllerName()); + $this->assertSame('foo', $router->methodName()); + $this->assertSame('Tests\Support\Filters\Customfilter', $router->getFilter()); + } + /** * @see https://github.com/codeigniter4/CodeIgniter4/issues/1240 */ From 98bad0a885a47744cb262c445db550d9d6d90db2 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 24 Sep 2021 17:31:19 +0900 Subject: [PATCH 3/5] feat: add multiple filters for a route --- app/Config/Feature.php | 27 +++++++++++++++++++++++++++ system/CodeIgniter.php | 21 +++++++++++++++++---- system/Filters/Filters.php | 20 ++++++++++++++++++++ system/Router/RouteCollection.php | 23 +++++++++++++++++++++++ system/Router/Router.php | 30 +++++++++++++++++++++++++++++- tests/system/CodeIgniterTest.php | 24 ++++++++++++++++++++++++ tests/system/Router/RouterTest.php | 19 +++++++++++++++++++ 7 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 app/Config/Feature.php diff --git a/app/Config/Feature.php b/app/Config/Feature.php new file mode 100644 index 000000000000..af42534ac423 --- /dev/null +++ b/app/Config/Feature.php @@ -0,0 +1,27 @@ +enableFilter($routeFilter, 'before'); - $filters->enableFilter($routeFilter, 'after'); + $multipleFiltersEnabled = config('Feature')->multipleFilters ?? false; + if ($multipleFiltersEnabled) { + $filters->enableFilters($routeFilter, 'before'); + $filters->enableFilters($routeFilter, 'after'); + } else { + // for backward compatibility + $filters->enableFilter($routeFilter, 'before'); + $filters->enableFilter($routeFilter, 'after'); + } } $uri = $this->determinePath(); @@ -690,7 +697,7 @@ public function displayPerformanceMetrics(string $output): string * * @throws RedirectException * - * @return string|null + * @return string|string[]|null */ protected function tryToRouteIt(?RouteCollectionInterface $routes = null) { @@ -719,7 +726,13 @@ protected function tryToRouteIt(?RouteCollectionInterface $routes = null) $this->benchmark->stop('routing'); - return $this->router->getFilter(); + // for backward compatibility + $multipleFiltersEnabled = config('Feature')->multipleFilters ?? false; + if (! $multipleFiltersEnabled) { + return $this->router->getFilter(); + } + + return $this->router->getFilters(); } /** diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php index 04fd0e9d0a75..c81dadf3330f 100644 --- a/system/Filters/Filters.php +++ b/system/Filters/Filters.php @@ -319,6 +319,8 @@ public function addFilter(string $class, ?string $alias = null, string $when = ' * are passed to the filter when executed. * * @return Filters + * + * @deprecated Use enableFilters(). This method will be private. */ public function enableFilter(string $name, string $when = 'before') { @@ -354,6 +356,24 @@ public function enableFilter(string $name, string $when = 'before') return $this; } + /** + * Ensures that specific filters is on and enabled for the current request. + * + * Filters can have "arguments". This is done by placing a colon immediately + * after the filter name, followed by a comma-separated list of arguments that + * are passed to the filter when executed. + * + * @return Filters + */ + public function enableFilters(array $names, string $when = 'before') + { + foreach ($names as $filter) { + $this->enableFilter($filter, $when); + } + + return $this; + } + /** * Returns the arguments for a specified key, or all. * diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 87aeaa0486a0..1dcbfe9f590d 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -1040,6 +1040,8 @@ public function isFiltered(string $search, ?string $verb = null): bool * 'role:admin,manager' * * has a filter of "role", with parameters of ['admin', 'manager']. + * + * @deprecated Use getFiltersForRoute() */ public function getFilterForRoute(string $search, ?string $verb = null): string { @@ -1048,6 +1050,27 @@ public function getFilterForRoute(string $search, ?string $verb = null): string return $options[$search]['filter'] ?? ''; } + /** + * 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']. + */ + public function getFiltersForRoute(string $search, ?string $verb = null): array + { + $options = $this->loadRoutesOptions($verb); + + if (is_string($options[$search]['filter'])) { + return [$options[$search]['filter']]; + } + + return $options[$search]['filter'] ?? []; + } + /** * Given a * diff --git a/system/Router/Router.php b/system/Router/Router.php index ea59b2217b23..621c54a5e9dd 100644 --- a/system/Router/Router.php +++ b/system/Router/Router.php @@ -99,9 +99,19 @@ class Router implements RouterInterface * if the matched route should be filtered. * * @var string|null + * + * @deprecated Use $filtersInfo */ protected $filterInfo; + /** + * The filter info from Route Collection + * if the matched route should be filtered. + * + * @var string[] + */ + protected $filtersInfo = []; + /** * Stores a reference to the RouteCollection object. * @@ -144,7 +154,13 @@ public function handle(?string $uri = null) if ($this->checkRoutes($uri)) { if ($this->collection->isFiltered($this->matchedRoute[0])) { - $this->filterInfo = $this->collection->getFilterForRoute($this->matchedRoute[0]); + $multipleFiltersEnabled = config('Feature')->multipleFilters ?? false; + if ($multipleFiltersEnabled) { + $this->filtersInfo = $this->collection->getFiltersForRoute($this->matchedRoute[0]); + } else { + // for backward compatibility + $this->filterInfo = $this->collection->getFilterForRoute($this->matchedRoute[0]); + } } return $this->controller; @@ -166,12 +182,24 @@ public function handle(?string $uri = null) * Returns the filter info for the matched route, if any. * * @return string + * + * @deprecated Use getFilters() */ public function getFilter() { return $this->filterInfo; } + /** + * Returns the filter info for the matched route, if any. + * + * @return string[] + */ + public function getFilters(): array + { + return $this->filtersInfo; + } + /** * Returns the name of the matched controller. * diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 0a3ad80921d7..0e6de8eadda0 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -17,6 +17,7 @@ use CodeIgniter\Test\Mock\MockCodeIgniter; use Config\App; use Config\Modules; +use Tests\Support\Filters\Customfilter; /** * @backupGlobals enabled @@ -172,6 +173,29 @@ public function testControllersCanReturnResponseObject() $this->assertStringContainsString("You want to see 'about' page.", $output); } + public function testControllersRunFilterByClassName() + { + $_SERVER['argv'] = ['index.php', 'pages/about']; + $_SERVER['argc'] = 2; + + $_SERVER['REQUEST_URI'] = '/pages/about'; + + // Inject mock router. + $routes = Services::routes(); + $routes->add('pages/about', static function () { + return Services::request()->url; + }, ['filter' => Customfilter::class]); + + $router = Services::router($routes, Services::request()); + Services::injectMock('router', $router); + + ob_start(); + $this->codeigniter->useSafeOutput(true)->run(); + $output = ob_get_clean(); + + $this->assertStringContainsString('http://hellowworld.com', $output); + } + public function testResponseConfigEmpty() { $_SERVER['argv'] = ['index.php', '/']; diff --git a/tests/system/Router/RouterTest.php b/tests/system/Router/RouterTest.php index dd6ca83ad6fd..0c264fbdc43b 100644 --- a/tests/system/Router/RouterTest.php +++ b/tests/system/Router/RouterTest.php @@ -544,6 +544,25 @@ public function testRouteWorksWithClassnameFilter() $this->assertSame('Tests\Support\Filters\Customfilter', $router->getFilter()); } + public function testRouteWorksWithMultipleFilters() + { + $feature = config('Feature'); + $feature->multipleFilters = true; + + $collection = $this->collection; + + $collection->add('foo', 'TestController::foo', ['filter' => ['filter1', 'filter2:param']]); + $router = new Router($collection, $this->request); + + $router->handle('foo'); + + $this->assertSame('\TestController', $router->controllerName()); + $this->assertSame('foo', $router->methodName()); + $this->assertSame(['filter1', 'filter2:param'], $router->getFilters()); + + $feature->multipleFilters = false; + } + /** * @see https://github.com/codeigniter4/CodeIgniter4/issues/1240 */ From 69d33822e94680f6e9a1b31c75f4a59f8534af97 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 24 Sep 2021 17:35:48 +0900 Subject: [PATCH 4/5] docs: add multiple filters for a route and classname filter --- user_guide_src/source/incoming/routing.rst | 28 +++++++++++++++++-- .../source/installation/upgrade_415.rst | 28 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/user_guide_src/source/incoming/routing.rst b/user_guide_src/source/incoming/routing.rst index d03d59ddf263..8c959d87cdbf 100644 --- a/user_guide_src/source/incoming/routing.rst +++ b/user_guide_src/source/incoming/routing.rst @@ -352,15 +352,37 @@ can modify the generated routes, or further restrict them. The ``$options`` arra Applying Filters ---------------- -You can alter the behavior of specific routes by supplying a filter to run before or after the controller. This is especially handy during authentication or api logging:: +You can alter the behavior of specific routes by supplying filters to run before or after the controller. This is especially handy during authentication or api logging. +The value for the filter can be a string or an array of strings: + +* matching the aliases defined in ``app/Config/Filters.php``. +* filter classnames + +See `Controller filters `_ for more information on setting up filters. + +**Alias filter** + +You specify an alias defined in ``app/Config/Filters.php`` for the filter value:: $routes->add('admin',' AdminController::index', ['filter' => 'admin-auth']); -The value for the filter must match one of the aliases defined within ``app/Config/Filters.php``. You may also supply arguments to be passed to the filter's ``before()`` and ``after()`` methods:: +You may also supply arguments to be passed to the alias filter's ``before()`` and ``after()`` methods:: $routes->add('users/delete/(:segment)', 'AdminController::index', ['filter' => 'admin-auth:dual,noreturn']); -See `Controller filters `_ for more information on setting up filters. +**Classname filter** + +You specify a filter classname for the filter value:: + + $routes->add('admin',' AdminController::index', ['filter' => \App\Filters\SomeFilter::class]); + +**Multiple filters** + +.. important:: *Multiple filters* is disabled by default. Because it breaks backward compatibility. If you want to use it, you need to configure. See *Multiple filters for a route* in :doc:`/installation/upgrade_415` for the details. + +You specify an array for the filter value:: + + $routes->add('admin',' AdminController::index', ['filter' => ['admin-auth', \App\Filters\SomeFilter::class]]); Assigning Namespace ------------------- diff --git a/user_guide_src/source/installation/upgrade_415.rst b/user_guide_src/source/installation/upgrade_415.rst index bb075023e7bc..b05d0d0030a6 100644 --- a/user_guide_src/source/installation/upgrade_415.rst +++ b/user_guide_src/source/installation/upgrade_415.rst @@ -25,3 +25,31 @@ Update the definition of the session table. See the :doc:`/libraries/sessions` f The change was introduced in v4.1.2. But due to `a bug `_, the DatabaseHandler Driver did not work properly. + +**Multiple filters for a route** + +A new feature to set multiple filters for a route. + +.. important:: This feature is disabled by default. Because it breaks backward compatibility. + +If you want to use this, you need to set the property ``$multipleFilters`` ``true`` in ``app/Config/Feature.php``. +If you enable it: + +- ``CodeIgniter\CodeIgniter::handleRequest()`` uses + - ``CodeIgniter\Filters\Filters::enableFilters()``, instead of ``enableFilter()`` +- ``CodeIgniter\CodeIgniter::tryToRouteIt()`` uses + - ``CodeIgniter\Router\Router::getFilters()``, instead of ``getFilter()`` +- ``CodeIgniter\Router\Router::handle()`` uses + - the property ``$filtersInfo``, instead of ``$filterInfo`` + - ``CodeIgniter\Router\RouteCollection::getFiltersForRoute()``, instead of ``getFilterForRoute()`` + +If you extended the above classes, then you need to change them. + +The following methods and a property have been deprecated: + +- ``CodeIgniter\Filters\Filters::enableFilter()`` +- ``CodeIgniter\Router\Router::getFilter()`` +- ``CodeIgniter\Router\RouteCollection::getFilterForRoute()`` +- ``CodeIgniter\Router\RouteCollection``'s property ``$filterInfo`` + +See *Applying Filters* in :doc:`Routing ` for the functionality. From 3ee6632e8f7b2787f72a356f80da5b5abf154e3f Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 25 Sep 2021 11:20:39 +0900 Subject: [PATCH 5/5] chore: suppress phpstan error for CodeIgniter\\Router\\RouteCollectionInterface --- phpstan.neon.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 3847a2f02a69..8861b753fe26 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -33,7 +33,7 @@ parameters: - '#Access to an undefined property CodeIgniter\\Database\\BaseConnection::\$mysqli|\$schema#' - '#Access to an undefined property CodeIgniter\\Database\\ConnectionInterface::(\$DBDriver|\$connID|\$likeEscapeStr|\$likeEscapeChar|\$escapeChar|\$protectIdentifiers|\$schema)#' - '#Call to an undefined method CodeIgniter\\Database\\BaseConnection::_(disable|enable)ForeignKeyChecks\(\)#' - - '#Call to an undefined method CodeIgniter\\Router\\RouteCollectionInterface::(getDefaultNamespace|isFiltered|getFilterForRoute|getRoutesOptions)\(\)#' + - '#Call to an undefined method CodeIgniter\\Router\\RouteCollectionInterface::(getDefaultNamespace|isFiltered|getFilterForRoute|getFiltersForRoute|getRoutesOptions)\(\)#' - '#Cannot access property [\$a-z_]+ on ((bool\|)?object\|resource)#' - '#Cannot call method [a-zA-Z_]+\(\) on ((bool\|)?object\|resource)#' - '#Method CodeIgniter\\Router\\RouteCollectionInterface::getRoutes\(\) invoked with 1 parameter, 0 required#'