Skip to content

[Routing] Fix removing aliases pointing to removed route in RouteCollection::remove() #52806

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

Merged

Conversation

fancyweb
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations?
Issues #52802
License MIT

When a route is removed from the collection, we want to remove all aliases pointing to it. The implementation is flawed because the $aliases array is indexed by the alias name, and not by the route since several aliases can point to the same route.

It'll fix the related issue because added FQCN aliases will be correctly removed when PrefixTrait::addPrefix() removes the route before reading it for each locale.

@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

@nicolas-grekas nicolas-grekas merged commit ce95b87 into symfony:5.4 Nov 29, 2023
@fancyweb fancyweb deleted the routing/fix-route-collection-remove branch November 29, 2023 17:14
@stof
Copy link
Member

stof commented Nov 29, 2023

AFAICT, the goal was not to remove the aliases pointing to a route but really to remove the alias when passing the alias name.

@stof
Copy link
Member

stof commented Nov 29, 2023

This PR makes it impossible to remove an alias.

nicolas-grekas added a commit that referenced this pull request Dec 1, 2023
…ve() (fancyweb)

This PR was merged into the 5.4 branch.

Discussion
----------

[Routing] Restore aliases removal in RouteCollection::remove()

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | #52806 (comment)
| License       | MIT

As suggested in #52831 (comment)

It's still a small behavior change on 5.4, let's see what people think 😃

Commits
-------

3c6dc17 [Routing] Restore aliases removal in RouteCollection::remove()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants