Route different HTTP verbs to different methods, with the same route name, with annotations

[Bilge]

Description

Symfony supports routing different HTTP verbs to the same method with the same route name, but not different methods with the same route name.

    /**
     * @Route("/foo", name="foo", methods={"GET", "POST"})
     */
     public function __invoke() {}

However, if we want to split this out into two methods, this is not possible.

    /**
     * @Route("/foo", name="foo", methods={"GET"})
     */
     public function get() {}

    /**
     * @Route("/foo", name="foo", methods={"POST"})
     */
     public function post() {}

In this case, the latter definition overwrites the former, since they have the same name. Yet the same name is permitted for different verbs, provided they are backed by the same controller method. In my view this is problematic because Symfony routing limitations should not hamstring the implementation.

Example

No response

[stof]

That's expected: having a single route with multiple values in the methods array is totally not the same than having 2 routes. and the controller reference is part of the route definition, so if you need 2 different controllers, they cannot be the same route.

[stof]

here is the same config in Yaml format, that makes it clear that you have 2 routes with different configs in the second case:

foo:
  path: /foo
  methods: [GET, POST]
  controller: 'App\Controller\FooController::__invoke'

foo:
  path: /foo
  methods: [GET]
  controller: 'App\Controller\FooController::get'

foo: # not actually valid Yaml, as duplicate keys are forbidden at the Yaml level too
  path: /foo
  methods: [POST]
  controller: 'App\Controller\FooController::post'

As you can see, the controller config is different between the 2 routes, and so we really need to register 2 routes.

[Bilge]

It's expected because that's what you expect. That in no way contradicts the point that this is a severely limiting factor in routing flexibility. It should be possible to express this kind of configuration, since even though routes and controller mapping must be bi-directional, when only the verb varies this should be valid because verbs are actually unidirectional since they play no part in URL generation.

[stof]

@Bilge routes are not only about url generation. they are also about url matching.

[stof]

thus, your expectation would mean "it is allowed to register routes with duplicate names, but only if all their configuration impacting url generation is strictly the same, so that we can pick a random one among them to generate the URL and still be correct". That's quite weird (not even counting that it is a BC break, because the current code allows to override a route definition by redeclaring the name)

[Bilge]

I suppose it might be pretty weird, but perhaps it would be limited to the existing route merging syntax only, to make it more clear and less weird. That is, where part of the route is declared on the class and part on the method.

/**
 * @Route("/foo", name="foo")
 */
class Foo {
    /**
     * @Route(methods={"GET"})
     */
     public function get() {}

    /**
     * @Route(methods={"POST"})
     */
     public function post() {}
}

[stof]

The class-level annotation is about defining class-scoped defaults. This does not change the fact that we have to register 2 routes, and that we must register 2 routes (otherwise, the matching for GET and POST would not use different controllers).

[Bilge]

At the moment that is the case, yes. But I am wondering if the system could not be adapted to better support this.

[stof]

@Bilge well, for the matching, those are definitely different routes.

And making the route name not being a unique identifier of the route is both a BC break (due to the existing overriding behavior when reusing the name) and a WTF moment (as we still need to enforce that all routes sharing the same name are configured the same for all features that impact URL generation, so that the name can keep being used as an identifier for the URL generation, which is very hard to explain).
And if we take into account the fact that the URL matcher also relies on some kind of URL generation when having to redirect (trailing / vs no trailing /), I'm not even sure that the matching-only features can actually be excluded from this list of features needing to stay the same.

Btw, _defaults does impact the URL generation in subtle ways, so there is not a lot of configuration options that are matching-only.

So I would vote -1 for going into that direction.

[codedmonkey]

Multiple verbs are allowed for a single route because it makes configuration easier when a route does the same thing with different verbs. What we have to keep in mind is that different verbs can do different things (post data from the request is only available when using the POST verb).

A common use case for this is the form builder. I could argue it's a bad idea even here, but it's useful because of some specific edge cases. 1) Most logic that's exclusive to the POST verb of submitting a form is abstracted by the form builder. 2) It saves the hassle of having to store form errors in a session or cache when redirecting back to the form.

In short, I think adding this would make it easier for developers to introduce unexpected behavior into their application and could hinder DX for their peers.

[GromNaN]

@Bilge why do you need the same route name for POST and GET ? That is an internal key that should never be exposed.

If the name is suffixed for the 2nd route, the UrlGenerator can still use the 1st route name.

    /**
     * @Route("/foo", name="foo", methods={"GET"})
     */
     public function get() {}

    /**
     * @Route("/foo", name="foo_post", methods={"POST"})
     */
     public function post() {}

[Bilge]

That's a valid point. I guess the only reason I can think of is to maintain a neater and tidier list in the `debug:router` table...

…

On Wed, 2 Feb 2022, 18:58 Jérôme Tamarelle, ***@***.***> wrote: @Bilge <https://github.com/Bilge> why do you *need* the same route name for POST and GET ? That is an internal key that should never be exposed. If the name is suffixed for the 2nd route, the UrlGenerator can still use the 1st route name. /** * @route("/foo", name="foo", methods={"GET"}) */ public function get() {} /** * @route("/foo", name="foo_post", methods={"POST"}) */ public function post() {} — Reply to this email directly, view it on GitHub <#45218 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADS4YWGC7IEO3INBY2DJRDUZF5FJANCNFSM5NAPJ7KA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[GromNaN]

I'm closing since the conception of the UrlMatcher make it impossible as @stof explained.