feature #66004 [Routing] Add addCondition() to combine a condition with the existing ones (nicolas-grekas)
This PR was merged into the 8.2 branch.
Discussion
----------
[Routing] Add addCondition() to combine a condition with the existing ones
| Q | A
| ------------- | ---
| Branch? | 8.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Issues | Fix #53171
| License | MIT
Today a route import can only *replace* the conditions of the routes it imports:
```yaml
app:
resource: ../src/Controller/
type: attribute
condition: 'request.isSecure()'
```
This wipes any `#[Route(condition: ...)]` set on the imported controllers. That is documented, and it is consistent with how an import applies `defaults` and `requirements`, but it means a group-wide guard and a per-route condition cannot coexist: whichever side you write, the other is silently discarded. That is what #53171 is about.
The underlying reason is a missing primitive. `Route` and `RouteCollection` both have `setCondition()` and neither has `addCondition()`, so "also require this" is not expressible anywhere in the component. This PR adds it:
* `Route::addCondition()` and `RouteCollection::addCondition()`, combining as `(existing) and (new)`
* `addCondition()` on the route, collection and import configurators
* an `add_condition` key on imports and route definitions
```yaml
app:
resource: ../src/Controller/
type: attribute
add_condition: 'request.isSecure()'
```
`condition` keeps its current meaning, so nothing changes for anyone who does not opt in.
### The alternative I did not take
The other way to resolve #53171 is to flip the default: deprecate an import-level `condition` that collides with a route-level one, and make the two combine in 9.0. That needs an escape hatch, because the colliding route is usually in a third-party bundle you cannot edit, so the deprecation would otherwise be impossible to silence. And the escape hatch has to be tri-state (an explicit "replace", an explicit "combine", and the unset case that still deprecates), which is a larger config surface than this whole PR, for a change that only moves a default.
### Status
This is prospective on my side. If we do not want the added API, then the honest outcome for #53171 is to close it as won't fix: the current behaviour is documented, it is consistent across the properties an import can set, and reversing it is a BC break nobody has found a cheap migration for. #53511 and #60343 both died trying.
Reported by `@mbuliard`. `@MatTheCat` built the deprecation route in #60343 and withdrew it, and `@stof` set out the constraints that make the flip expensive.
Commits
-------
0b8db47612c [Routing] Add addCondition() to combine a condition with the existing ones N
Nicolas Grekas committed
cb423dc2330d297af1fc2a2b9e0620a9484842e9