Fosrestbundle: RFC: Deprecate route creation based on class/method names

[Tobion]

IMO the automagic creation of routes based on the class and method names is the worst part of the rest bundle.

1. it's alot of magic and things easily break just by renaming a class or method
2. it's hard to understand, document and maintain
3. it does not work with the new best practice of symfony to not suffix your contoller actions with *Action because of https://github.com/FriendsOfSymfony/FOSRestBundle/blob/1665b98c6543cf52699f6a1558121ca54c7565b2/Routing/Loader/Reader/RestActionReader.php#L475-L477
   But we also cannot really remove this limitation because it would automagically create routes for methods you might not want, like setters if you use setter injection in some places
4. it does not work with controller.service_arguments, see #1807
5. there are so many issues and edge cases with this
   
   
   
   • #1759 is a huge problem. but it cannot really be changed without breaking bc. if we change the route names, it would be a bc break.
   
   
   • #1953
   
   
   • #1837 and #1733
   
   
   • #1808
   
   
   • #1789
   
   
   • #1310
   
   
   • #1657
   
   

So I'd like a way to use the RestBundle routing features like

• FOS\RestBundle\Controller\Annotations\Get and all the other method shortcuts
• FOS\RestBundle\Controller\Annotations\Version
• adding the format to the routes path

But without the route magic of creating routes based on class/method names. So routes need to be created explicitly using the symfony core routing annotations or the fos rest bundle shortcuts. This also means no more

• FOS\RestBundle\Controller\Annotations\RouteResource
• FOS\RestBundle\Routing\ClassResourceInterface

We could either deprecate this magic or create a config option to disable it.

[lsmith77]

I am not sure I understand .. you can use
https://symfony.com/doc/master/bundles/FOSRestBundle/7-manual-route-definition.html and the annotations listed there without having to disable the magic way .. so I am not sure what you are asking here.

do you want to deprecate it out of principle or because its actually making something impossible for you?

[Tobion]

I can only use manual route definitions as long as my actions method are suffixed. See 3)
As soon as I change it, all routes are gone. But we also cannot change that because of all the magic.
And even if you use manual route definitions, there is still magic that you cannot disable:

1. methods where you do not set a route on will create a route magically, so you have to add a NoRoute annotation explicitly to avoid that.
2. the route name is still magic, see https://symfony.com/doc/master/bundles/FOSRestBundle/7-manual-route-definition.html#method-name-prefix Even if I set a route name, the route name get's changed based on the method name. So refactoring a method name, breaks all routing url generation. And if you are unlucky and change it to an action name that already exists, it will override routes due to #1759

[lsmith77]

ok. so how about we add new class level annotations to configure this?

[lsmith77]

BTW as for deprecating .. I am open to this. I think anyone that really is focused on RAD is by now anyway using API Platform and the main use for this Bundle are people that are doing a lot of custom things that likely do not really benefit from the magic.

[Tobion]

ok. so how about we add new class level annotations to configure this?

I'd prefer a config option on fos_rest to be able to disable this globablly. So I don't have to do it for each controller. I mean, if the default behavior was disabled, then it would make sense to have a class-level annotation to enable this. This would make the code more self-explaning as we could just document this annotation what it means. But as the default is enabled, disabling it for each class does not make it better.

[lsmith77]

globally is not possible since it could have surprising issues with existing, especially 3rd party controllers.

[Tobion]

Well, the default would still be enabled for BC. So if people disable it, it's up to them. Maybe they don't use 3rd party rest controllers.
Also the argument that enabling/disabling things might break 3rd party controllers can be applied to any config like symfony routing itself or fos rest body converters (https://symfony.com/doc/master/bundles/FOSRestBundle/request_body_converter_listener.html)

[lsmith77]

no .. we specifically solved this issue with 2.x by allowing listener to be scoped by zones:
https://symfony.com/doc/master/bundles/FOSRestBundle/3-listener-support.html#zone-listener

[Tobion]

If I disable https://github.com/FriendsOfSymfony/FOSRestBundle/blob/39fc21db2820f419bea955da83e42c08dea693f2/DependencyInjection/FOSRestExtension.php#L147 then those listeners are not active, thus breaking 3rd party controllers that rely on it. The zones do not help at all in this regard.

[lsmith77]

yes .. I can break things but I can fix them again by properly configuring zones .. what you are suggesting would lead to Bundles that cannot ever be combined in the same kernel.

[danrot]

TBH, we at Sulu mainly use this bundle for its automatic route generation using the ClassResourceInterface. Would the feature also be removed in combination with that interface?

I think if you drop it, we would probably not use it anymore. I don't see any other huge advantages, which we couldn't solve by adding a few lines of code on our own, and then we would have less problems on every major update on Symfony.

[Tobion]

ClassResourceInterface would also be removed. The question is, do you like this route generation from method names? Or would you rather define routes explicitly?

[danrot]

What exactly do you mean by defining routes explicitly? The way I see it, using FOSRestBundle does not provide any huge benefit compared to just using Symfony routing itself anymore.

And to answer your question, yes, the route generation from method names was exactly what I liked about this bundle.

[Tobion]

What exactly do you mean by defining routes explicitly? The way I see it, using FOSRestBundle does not provide any huge benefit compared to just using Symfony routing itself anymore.

Yes exactly. That's a good thing as it makes things more standard and understandable.

Why do you like the rest-bundle route generation? What are the advantages?

[danrot]

Now I have a getAction, postAction, etc. in all controllers of my codebase, so that all controller look fairly familiar, which is currently enforced by the FOSRestBundle. Also, I like the fact that I don't have to configure so much. Because configuring that for every controller on its own involves the risk that different standards for each controller are established.

[xabbuh]

I think it's time to make a final decision here as I would like to finalise the 2.8 release which should contain all the deprecations for features that will be removed in 3.0 (I hope to tag 3.0 during April). I do not use this feature, I do not really well understand the code implementing this and I will probably not spend much time investing any issues related to it in the future. But that's only my personal POV. I am also going to ask for feedback on Twitter to gather more feedback.

[xabbuh]

FYI: https://twitter.com/xabbuh/status/1244687305572528128 (please share for reach)

[lsmith77]

I think we can deprecate them .. if there is interest in this feature in the future, then the code can be moved into a separate bundle. I personally like them for the reasons @danrot mentioned .. in 98% of the cases it just works and provides a consistency that isn’t easily achieved manually (I have seen plenty of API’s where accidentally in some cases collections are singular and in other plural).

[danrot]

Have you already came to a decision regarding that? We would like to support on Symfony 5 in our next release, and how we achieve that is heavily dependant on this decision.

[xabbuh]

@danrot I have started on deprecating this feature (see #2152 for the draft PR).

[danrot]

I have already seen that, but does that mean that the support will be dropped in 3.0 already?

[xabbuh]

Yes, we will deprecate it in 2.8 and remove it in 3.0.

[lsmith77]

@danrot but it should be quite easy to create a new bundle and move the functionality there.

[alexander-schranz]

We definitely need this behaviour from BC side and we will move it to an own Bundle. Thought the name will be something like FOSRestRoutingBundle. Do you want to have it in the FriendsOfSymfony organization on github else we will create the bundle in our https://github.com/handcraftedinthealps/ organization.

[lsmith77]

I am fine making it FOSRestRoutingBundle .. could you create such a repo on your side, call it FOSRestRoutingBundle and once it is ready we move it over to the friendsofsymfony org

[xabbuh]

see #2152