Merge pull request codeigniter4#7903 from kenjis/multipleFilters-default

refactor: always use multiple filters
michalsn · Sep 14, 2023 · 28fe6ff · 28fe6ff
2 parents 28d541b + 581a5db
commit 28fe6ff
Show file tree

Hide file tree

Showing 13 changed files with 55 additions and 125 deletions.
diff --git a/app/Config/Feature.php b/app/Config/Feature.php
@@ -9,20 +9,6 @@
  */
 class Feature extends BaseConfig
 {
-    /**
-     * Enable multiple filters for a route or not.
-     *
-     * If you enable this:
-     *   - 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:
-     *     - property $filtersInfo, instead of $filterInfo
-     *     - CodeIgniter\Router\RouteCollection::getFiltersForRoute(), instead of getFilterForRoute()
-     */
-    public bool $multipleFilters = false;
-
     /**
      * Use improved new auto routing instead of the default legacy version.
      */

diff --git a/phpstan-baseline.php b/phpstan-baseline.php
@@ -2346,11 +2346,6 @@
 	'count' => 2,
 	'path' => __DIR__ . '/system/Router/Router.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getFilterForRoute\\(\\)\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/system/Router/Router.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getFiltersForRoute\\(\\)\\.$#',
 	'count' => 1,

diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php
@@ -31,7 +31,6 @@
 use CodeIgniter\Router\Router;
 use Config\App;
 use Config\Cache;
-use Config\Feature;
 use Config\Kint as KintConfig;
 use Config\Services;
 use Exception;
@@ -452,15 +451,8 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
             // If any filters were specified within the routes file,
             // we need to ensure it's active for the current request
             if ($routeFilter !== null) {
-                $multipleFiltersEnabled = config(Feature::class)->multipleFilters ?? false;
-                if ($multipleFiltersEnabled) {
-                    $filters->enableFilters($routeFilter, 'before');
-                    $filters->enableFilters($routeFilter, 'after');
-                } else {
-                    // for backward compatibility
-                    $filters->enableFilter($routeFilter, 'before');
-                    $filters->enableFilter($routeFilter, 'after');
-                }
+                $filters->enableFilters($routeFilter, 'before');
+                $filters->enableFilters($routeFilter, 'after');
             }
 
             // Run "before" filters
@@ -810,12 +802,6 @@ protected function tryToRouteIt(?RouteCollectionInterface $routes = null)
 
         $this->benchmark->stop('routing');
 
-        // for backward compatibility
-        $multipleFiltersEnabled = config(Feature::class)->multipleFilters ?? false;
-        if (! $multipleFiltersEnabled) {
-            return $this->router->getFilter();
-        }
-
         return $this->router->getFilters();
     }
 

diff --git a/system/Commands/Utilities/Routes/FilterFinder.php b/system/Commands/Utilities/Routes/FilterFinder.php
@@ -15,7 +15,6 @@
 use CodeIgniter\Filters\Filters;
 use CodeIgniter\HTTP\Exceptions\RedirectException;
 use CodeIgniter\Router\Router;
-use Config\Feature;
 use Config\Services;
 
 /**
@@ -38,13 +37,6 @@ private function getRouteFilters(string $uri): array
     {
         $this->router->handle($uri);
 
-        $multipleFiltersEnabled = config(Feature::class)->multipleFilters ?? false;
-        if (! $multipleFiltersEnabled) {
-            $filter = $this->router->getFilter();
-
-            return $filter === null ? [] : [$filter];
-        }
-
         return $this->router->getFilters();
     }
 

diff --git a/system/Filters/Filters.php b/system/Filters/Filters.php
@@ -335,12 +335,8 @@ public function addFilter(string $class, ?string $alias = null, string $when = '
      * are passed to the filter when executed.
      *
      * @param string $name filter_name or filter_name:arguments like 'role:admin,manager'
-     *
-     * @return $this
-     *
-     * @deprecated Use enableFilters(). This method will be private.
      */
-    public function enableFilter(string $name, string $when = 'before')
+    private function enableFilter(string $name, string $when = 'before'): void
     {
         // Get arguments and clean name
         [$name, $arguments]     = $this->getCleanName($name);
@@ -362,8 +358,6 @@ public function enableFilter(string $name, string $when = 'before')
             $this->filters[$when][]    = $name;
             $this->filtersClass[$when] = array_merge($this->filtersClass[$when], $classNames);
         }
-
-        return $this;
     }
 
     /**

diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php
@@ -1228,25 +1228,6 @@ public function isFiltered(string $search, ?string $verb = null): bool
         return isset($options[$search]['filter']);
     }
 
-    /**
-     * Returns the filter 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'].
-     *
-     * @deprecated Use getFiltersForRoute()
-     */
-    public function getFilterForRoute(string $search, ?string $verb = null): string
-    {
-        $options = $this->loadRoutesOptions($verb);
-
-        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

diff --git a/system/Router/Router.php b/system/Router/Router.php
@@ -99,16 +99,6 @@ class Router implements RouterInterface
      */
     protected $detectedLocale;
 
-    /**
-     * The filter info from Route Collection
-     * 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.
@@ -179,19 +169,12 @@ public function handle(?string $uri = null)
         $uri = urldecode($uri);
 
         // Restart filterInfo
-        $this->filterInfo  = null;
         $this->filtersInfo = [];
 
         // Checks defined routes
         if ($this->checkRoutes($uri)) {
             if ($this->collection->isFiltered($this->matchedRoute[0])) {
-                $multipleFiltersEnabled = config(Feature::class)->multipleFilters ?? false;
-                if ($multipleFiltersEnabled) {
-                    $this->filtersInfo = $this->collection->getFiltersForRoute($this->matchedRoute[0]);
-                } else {
-                    // for backward compatibility
-                    $this->filterInfo = $this->collection->getFilterForRoute($this->matchedRoute[0]);
-                }
+                $this->filtersInfo = $this->collection->getFiltersForRoute($this->matchedRoute[0]);
             }
 
             return $this->controller;
@@ -212,18 +195,6 @@ public function handle(?string $uri = null)
         return $this->controllerName();
     }
 
-    /**
-     * Returns the filter info for the matched route, if any.
-     *
-     * @return string|null
-     *
-     * @deprecated Use getFilters()
-     */
-    public function getFilter()
-    {
-        return $this->filterInfo;
-    }
-
     /**
      * Returns the filter info for the matched route, if any.
      *

diff --git a/tests/system/Commands/Utilities/Routes/FilterFinderTest.php b/tests/system/Commands/Utilities/Routes/FilterFinderTest.php
@@ -171,8 +171,6 @@ public function testFindGlobalsAndRouteClassnameFilters(): void
 
     public function testFindGlobalsAndRouteMultipleFilters(): void
     {
-        config('Feature')->multipleFilters = true;
-
         $collection = $this->createRouteCollection();
         $collection->get('admin', ' AdminController::index', ['filter' => ['honeypot', InvalidChars::class]]);
         $router  = $this->createRouter($collection);
@@ -187,7 +185,5 @@ public function testFindGlobalsAndRouteMultipleFilters(): void
             'after'  => ['honeypot', InvalidChars::class, 'toolbar'],
         ];
         $this->assertSame($expected, $filters);
-
-        config('Feature')->multipleFilters = false;
     }
 }
diff --git a/tests/system/Filters/FiltersTest.php b/tests/system/Filters/FiltersTest.php
@@ -834,7 +834,7 @@ public function testEnableFilter(): void
         $filters       = $this->createFilters($filtersConfig);
 
         $filters = $filters->initialize('admin/foo/bar');
-        $filters->enableFilter('google', 'before');
+        $filters->enableFilters(['google'], 'before');
         $filters = $filters->getFilters();
 
         $this->assertContains('google', $filters['before']);
@@ -941,8 +941,8 @@ public function testEnableFilterWithArguments(): void
         $filters       = $this->createFilters($filtersConfig);
 
         $filters = $filters->initialize('admin/foo/bar');
-        $filters->enableFilter('role:admin , super', 'before');
-        $filters->enableFilter('role:admin , super', 'after');
+        $filters->enableFilters(['role:admin , super'], 'before');
+        $filters->enableFilters(['role:admin , super'], 'after');
         $found = $filters->getFilters();
 
         $this->assertContains('role', $found['before']);
@@ -973,8 +973,8 @@ public function testEnableFilterWithNoArguments(): void
         $filters       = $this->createFilters($filtersConfig);
 
         $filters = $filters->initialize('admin/foo/bar');
-        $filters->enableFilter('role', 'before');
-        $filters->enableFilter('role', 'after');
+        $filters->enableFilters(['role'], 'before');
+        $filters->enableFilters(['role'], 'after');
         $found = $filters->getFilters();
 
         $this->assertContains('role', $found['before']);
@@ -1005,7 +1005,7 @@ public function testEnableNonFilter(): void
         $filters       = $this->createFilters($filtersConfig);
 
         $filters = $filters->initialize('admin/foo/bar');
-        $filters->enableFilter('goggle', 'before');
+        $filters->enableFilters(['goggle'], 'before');
     }
 
     /**

diff --git a/tests/system/Router/RouterTest.php b/tests/system/Router/RouterTest.php
@@ -496,7 +496,7 @@ public function testRouteWorksWithFilters(): void
 
         $this->assertSame('\TestController', $router->controllerName());
         $this->assertSame('foobar', $router->methodName());
-        $this->assertSame('test', $router->getFilter());
+        $this->assertSame(['test'], $router->getFilters());
     }
 
     /**
@@ -526,25 +526,25 @@ static function (RouteCollection $routes): void {
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('index', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         $router->handle('api/posts/new');
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('new', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         $router->handle('api/posts/50');
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('show', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         $router->handle('api/posts/50/edit');
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('edit', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         // POST
         $this->collection->group(...$group);
@@ -556,7 +556,7 @@ static function (RouteCollection $routes): void {
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('create', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         // PUT
         $this->collection->group(...$group);
@@ -568,7 +568,7 @@ static function (RouteCollection $routes): void {
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('update', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         // PATCH
         $this->collection->group(...$group);
@@ -580,7 +580,7 @@ static function (RouteCollection $routes): void {
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('update', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
 
         // DELETE
         $this->collection->group(...$group);
@@ -592,7 +592,7 @@ static function (RouteCollection $routes): void {
 
         $this->assertSame('\App\Controllers\Api\PostController', $router->controllerName());
         $this->assertSame('delete', $router->methodName());
-        $this->assertSame('api-auth', $router->getFilter());
+        $this->assertSame(['api-auth'], $router->getFilters());
     }
 
     public function testRouteWorksWithClassnameFilter(): void
@@ -606,16 +606,13 @@ public function testRouteWorksWithClassnameFilter(): void
 
         $this->assertSame('\TestController', $router->controllerName());
         $this->assertSame('foo', $router->methodName());
-        $this->assertSame(Customfilter::class, $router->getFilter());
+        $this->assertSame([Customfilter::class], $router->getFilters());
 
         $this->resetServices();
     }
 
     public function testRouteWorksWithMultipleFilters(): void
     {
-        $feature                  = config('Feature');
-        $feature->multipleFilters = true;
-
         $collection = $this->collection;
 
         $collection->add('foo', 'TestController::foo', ['filter' => ['filter1', 'filter2:param']]);
@@ -626,8 +623,6 @@ public function testRouteWorksWithMultipleFilters(): void
         $this->assertSame('\TestController', $router->controllerName());
         $this->assertSame('foo', $router->methodName());
         $this->assertSame(['filter1', 'filter2:param'], $router->getFilters());
-
-        $feature->multipleFilters = false;
     }
 
     /**

diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst
@@ -26,6 +26,21 @@ Interface Changes
 Method Signature Changes
 ========================
 
+.. _v450-removed-deprecated-items:
+
+Removed Deprecated Items
+========================
+
+Filters
+-------
+
+- The following deprecated items have been removed, because now :ref:`multiple-filters` are always enabled.
+
+    - ``Filters::enableFilter()``
+    - ``RouteCollection::getFilterForRoute()``
+    - ``Router::$filterInfo``
+    - ``Router::getFilter()``
+
 Enhancements
 ************
 
@@ -65,6 +80,10 @@ Message Changes
 Changes
 *******
 
+- **Config:**
+    - ``Config\Feature::$multipleFilters`` has been removed, because now
+      :ref:`multiple-filters` are always enabled.
+
 Deprecations
 ************