Multiple route filters

[iRedds]

Description
https://forum.codeigniter.com/thread-80117-post-390128.html

Several filters can be specified for a route.
Instead of filter aliases, you can use the name of the filter class.

$route->add('foo', 'Home::index', ['filter' => ['filterAlias', \App\Filters\SomeFilter::class]);

Also fixed typehint in dockblock in the RouteCollection methods. Missing Closure.

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

@iRedds Coding standards checking was failed.
https://github.com/codeigniter4/CodeIgniter4/pull/5108/checks?check_run_id=3648422172

Run composer cs-fix, please.

[kenjis]

This is a breaking API change. But it seems we must change the return type for multiple filters.

[iRedds]

What are your suggestions?

[kenjis]

@MGatner This PR contains public API changes.
Can we include this on the next release or we need to wait for the next major version (v5)?

If we can change the return type, array(string[]) is better.

[MGatner]

We have to stick to semantic versioning, which means we need to leave the signature or save this for version 5. Workarounds could be adding a new method and deprecating this one, or returning an encoded string (like a CSV of classes & aliases).

[kenjis]

If we want to include this in the next version (not v5),
it seems returning an encoded string (like a CSV of classes & aliases) is the only choice.

@iRedds What do you think?

[iRedds]

I think this is counterproductive.
The filter alias already contains ":" and "," to separate the alias and parameters. And adding one more separator will complicate read/write (by human).

An array of strings is the best solution in my opinion.

[kenjis]

@iRedds I didn't say about RouteCollection::add() usage.

$collection->add('foo', 'TestController::foo', ['filter' => ['filter1', 'filter2:param']]);

This is not a breaking change. Because the 3rd param type is ?array.

But if we change public method signatures, it is breaking change.
In this PR, getFilterForRoute() and enableFilter() are changed.
So this PR will not be able to be merged until the next major version 5.0.

[iRedds]

I think maintainers are too conservative about breaking changes.
If the backward compatibility of commonly used functions is broken, that's BC. It's my opinion.

The current policy negatively affects the improvement and popularization of the framework.
And version 5.0 looks like a meme "ten thousand years later" (c) SBSP

I have nothing to say. ggwp

[kenjis]

I also feel strongly constrained by the inability to change the method signatures.

I also understand that the policy of following semantic versioning has its value.

[MGatner]

getFilterForRoute() is actually not a part of the RouteCollectionInterface. While this has its own issues, it makes it easier for us to work with. The only places this method is used is a negligible Test support and in the Router:

CodeIgniter4/system/Router/Router.php

Line 147 in 9843737

$this->filterInfo = $this->collection->getFilterForRoute($this->matchedRoute[0]);

Here's my recommendation:

• Add a new method getFiltersForRoute() (notice the plural) with the desired signature and behavior
• Deprecated the current getFilterForRoute() with a doc block and in the User Guide files
• Change Router to use the new method when possible (e.g. if (method_exists('getFiltersForRoute', $this->collection))

[kenjis]

@MGatner I sent another PR #5128 with the direction you recommend.

• Change Router to use the new method when possible (e.g. if (method_exists('getFiltersForRoute', $this->collection))

method_exists() can't be used, because if user extends CI4's class, when upgrading CI4, the user class automatically inherits the new method.

I couldn't think of a way to automatically determine when to use a new method.
So I added a new Config class and users define it.

[MGatner]

If a user class extends RouteCollection then we should be fine. Basically we are finding a way to add a new "optional" feature, and need to make the rest of our code work with or without that feature. 99% of cases developers will be using our native routing classes, no problem; in the 1% (more like .001%) case we need to be sure the framework still works with any RouteCollectionInterface.

[MGatner]

@kenjis I understand better after looking at your PR: this is a larger issue than just this one method. I will wait and look these over when I'm on desktop and can compare files more easily.

[paulbalandan]

Closed in favor of #5128