[Routing] Add {foo:bar} syntax to define a mapping between a route parameter and its corresponding request attribute
#54720
+128
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolas-grekas
force-pushed
the
route-mapping
branch
from
6a0f6b0
to
7a12e25
Compare
nicolas-grekas
changed the title
{foo:bar} syntax to define a mapping between a route parameter and its corresponding attribute{foo:bar} syntax to define a mapping between a route parameter and its corresponding request attribute
nicolas-grekas
force-pushed
the
route-mapping
branch
from
7a12e25
to
b2b5505
Compare
…parameter and its corresponding request attribute
nicolas-grekas
force-pushed
the
route-mapping
branch
from
b2b5505
to
1e091b9
Compare
fabpot
approved these changes
May 2, 2024
|
Thank you @nicolas-grekas. |
fabpot
added a commit
that referenced
this pull request
May 2, 2024
… favor of mapped route parameters (nicolas-grekas) This PR was merged into the 7.1 branch. Discussion ---------- [DoctrineBridge] Deprecate auto-mapping of entities in favor of mapped route parameters | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | | Issues | Fix #50739 | License | MIT Auto-mapping of entities on controllers is a foot-gun when multiple entities are listed on the signature of the controller. This is described extensively by e.g. `@stof` in the linked issue and in a few others. The issue is that we try to use all request attributes to call a `findOneBy`, but when there are many entities, there can be an overlap in the naming of the field/associations of both entities. In this PR, I propose to deprecate auto-mapping and to replace it with mapped route parameters, as introduced in #54720. If we go this way, people that use auto-mapping to e.g. load a `$conference` based on its `{slug}` will have to declare the mapping by using `{slug:conference}` instead. That makes everything explicit and keeps a nice DX IMHO, by not forcing a `#[MapEntity]` attribute for simple cases. Commits ------- a49f9ea [DoctrineBridge] Deprecate auto-mapping of entities in favor of mapped route parameters
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While trying to improve the DX of auto-mapping Doctrine entities and from the discussion on the related PR #54455, I realized that it would help a lot if we were able to express a mapping between route parameter and request attributes directly into route definitions.
This PR adds the ability to define a route with such a mapping:
On the router side, the side-effect of this is just that a new
_route_mappingarray is returned by the matcher, and nothing else.Then, in HttpKernel's RouterListener, we use that parameter to map route parameters to request attributes. On their turn, argument resolvers will just see a request attribute named
conference. But they can also now read the_route_mappingattribute and decide to do more tailored things depending on the mapping.For example, one could define this route:
This would be turned into a request attribute named
conferencewith['id' => 'the-id', 'slug' => 'the-slug']as content.This mapping concern already leaks into many value resolver attributes (see their "name" property).
For the entity value resolver, this feature will allow deprecating auto-mapping altogether, and will make things more explicit.