Integrate DunglasActionBundle

[dunglas]

Alternative to #959.
As "actions" and __invoke seems to be confusing and a too big step fo now, let's focus on the essential for now: automatic dependency building.

[GuilhemN]

I think it would be great to use symfony/finder in your bundle to be able to use something like ../src/*/Controller instead.

[dunglas]

Why not.

[dunglas]

It should default to something like that, to avoid registering controllers and services from third-party bundles. I'll do it. That way, we can easily map Controller and Command without any risk to interfere with commands and controllers defined by public bundles.

[weaverryan]

I don't think it's worth any extra overhead, and there's no precedent for it in other service imports.

[weaverryan]

I didn't expect the configuration to work like this. It had never occurred to me to do any sort of autowiring across all bundles - I initially don't like that idea, and it complicates the configuration (I had to research what the difference was between the autodiscover and directories keys).

Also:

• what causes console commands to autowire? Are the action and console keys significant? I think they are. If so, it's weird to me to also allow other keys - like foo (used in the bundle README). I understand that this is to allow other directories to be autowired, but I don't like that sometimes the keys are special, and sometimes not. It also means we can't catch typos.
• You mentioned Still an autowiring_type definition to add in Symfony to avoid this - what
  does this refer to?
• Would it be possible to avoid the special action-annotation route type altogether? In other words, extend @Route to use a service definition for the controller if one exists? I ask because it feels like there are two ways to auto-register a controller as a service: either by using some config in config.yml or by using the action-annotation. The line between what each does is blurry. I think extending @Route may be a non-trivial task, but it makes a lot more sense to me: it's in the spirit of autowiring where we (in this case the @Route system) effectively "asks" the container for the service for an instance and it returns it.

[weaverryan]

btw, I am cautiously interested about all of this - my criticisms are because if it we were to go to the trouble of supporting this, let's do it really well.

[weaverryan]

Yea, I see now that action-annotation does nothing more than auto-set the _controller to the action.* pattern. That feels weak.

A more generic solution where @Route just works with service controllers looks even more appealing. In fact, if we could pull it off, then it could work in YAML too using the existing syntax - e.g.

homepage:
    path: /
    defaults:
        # or using the A:B:C syntax
       _controllers: AppBundle\Controller\MainController::homepage

If that class were represented as a service, it could/would use that service automatically. Otherwise, it would create a new instance like now.

I'm assuming you've thought about this - it would require a service that contained a run-time class => service id map for resolution.

[dunglas]

I agree with most of your arguments and I've made changes accordingly in the bundle:

• The "autodiscover" config option have been removed, only user bundles are now scanned by default, it's safer in better on all front (Support traditional Symfony conventions dunglas/DunglasActionBundle#26)
• Standard Controller and Command are now the default config (Support traditional Symfony conventions dunglas/DunglasActionBundle#26) - it allows to be as close as possible to current Symfony conventions
• The action-annotation routing type is being removed, the usual annotation will work (Support standard annotation type in prefix dunglas/DunglasActionBundle#29)

I'm not confortable with adding some magic in the router to guess the service corresponding to the type. The router supports services for a while using the 'service_name:method' syntax for while. It's what use the bundle. I prefer to keep it as is it will be easier to debug.

For the missing autowiring_type, it's for KernelInterface but it's not related anymore to this PR.

[wouterj]

empty lines should be removed

[wouterj]

I'm still not very sure if this is a great move for the standard edition. It can of course be great to use this bundle while developing applications, but the boilerplate code in the standard edition plays a big role in the learning process of a developer. Automatically injecting the container and having access to a get() method in the controller is already magic for beginners, imagine what happends when seeing this automatic injection stuff (as we discussed on the hackday, magic is great to advocate the framework and when you understand what's going on, but it only causes confusion in the months/years between these two phases).

Also, I'm against recommending controllers as service as the best practice. Most of the time, it only makes ones life harder (there are lots of questions about controller as services on IRC and not that much about normal controllers). Anyway, that's just my personal opinion, so feel free to just ignore it :)

[fabpot]

Just for the record, I'm also very against controllers as services since they were introduced. I still think this is an anti-pattern.

[dunglas]

@wouterj, as one of the main documenter of Symfony, it's important to convince you of benefits of this approach.

the boilerplate code in the standard edition plays a big role in the learning process of a developer. Automatically injecting the container and having access to a get() method in the controller is already magic for beginners, imagine what happends when seeing this automatic injection stuff

IMO, the "new" controller system is less magical and less Symfony-specific than the current one. And so, it is less difficult to explain it to complete newcomers:

Currently:

• The Controller directory is a magic directory where controller classes must be put
• Those classes usually (even if it's not mandatory) extend a Symfony specific class (the abstract controller)
• To reference this class, you must learn an uncommon Symfony specific syntax (MyBundle:MyController:myAction with a lot of possible misunderstanding (for instance, I often forgot to remove the mandatory Action suffix of controller methods when I configure the routing)
• To register a service, you must create a Symfony specific config file (YAML or XML service definition)
• To access this service, you need to learn the API of the Symfony specific container

It's a lot to learn and it's mandatory even to create a small application. Many parts are not obvious when you are a newcomer. It's obvious for us because we use Symfony for years, but when I teach Symfony to students having only a small Java background, I can say that it's hard to dive into the framework.

IMO, all those stuffs to learn initially in Symfony partially explain that many developers decide to start with Laravel instead of Symfony (you dive very easily into Laravel - and their marketing is awesome but it's not related).

On the other hand, here is how the new system works:

• The Controller directory is still a magic directory where controller classes must be put (nothing different here)
• Those classes are just POPO, you can use a helper trait to inject common dependencies if you want to, but there is nothing magic here (at least, less than in the current solution)
• To reference this class, you just use its FQN in the routing system, just use it's FQN (no Symfony specific syntax here). Even if we all know that is a trick (the service name is the same than class name), it's elegant and easy to learn for newcomers
• To register and access a service, just create a class anywhere in your project, inject it in the constructor of your controller, and you're done (no XML or YAML syntax to learn, no config file to write, intuitive approach already used in many other libraries including Laravel and PHP DI, and common in other programming languages too, no dependencies to Symfony specific things like the container or the abstract controller class)

IMO (but I can be wrong), it's far less concept to learn to dive into Symfony, and it's an improved developper experience:

1. create your logic business in POPO as usual
2. create a class in the Controller directory
3. type hint dependencies you need in your controller, Symfony automatically give you access to an instance of it
4. create any method returning a Response instance and map it to a route

You have a working Symfony application.

How can it be easier?

Also, I'm against recommending controllers as service as the best practice. Most of the time, it only makes ones life harder (there are lots of questions about controller as services on IRC and not that much about normal controllers). Anyway, that's just my personal opinion, so feel free to just ignore it :)

Controller as a service is an implementation detail of my bundle, we don't have to (and we shouldn't) introduce this notion to newcomers (I haven't mentioned it in the first part of my comment).

[dunglas]

@fabpot I usually never use controller as a service too in private projects. As explained in my previous comment the goal here isn't to promote controller as a service as best practice (it can be a "hidden" detail). It's just a way to enable automatic object construction trough autowiring.

[gnugat]

@dunglas: I know it might not sound that good, but would you consider creating a "Symfony Action Edition"? It could be promoted in the list of distributions.

[Pierstoval]

@dunglas: I know it might not sound that good, but would you consider creating a "Symfony Action Edition"? It could be promoted in the list of distributions.

There's a dedicated discussion about editions in symfony/symfony-installer#39

[gquemener]

Just adding my personnal thought, this is the same debat as the one that occurs about the KnpLabs RAD (Rapid Application Development) Bundle a few years back.

This bundle intends to add many auto registration features (by following simple convention) among other things and question was raised about integrating it in the Symfony Standard Edition.

The point was then that Symfony Standard was not opinonated and thus would never rely on such project.

Solution was to create our own edition, which is a pretty acceptable solution IMO.

[Pierstoval]

@gquemener thanks for pointing the fact that you created a special Symfony edition, because it is another argument in the debate about Symfony editions created in symfony/symfony-installer#39 😉

[GuilhemN]

What do you think about instead adding a new config option to the FrameworkBundle reproducing DunglasActionBundle's config ?
It would be disabled by default, wouldn't force people to install a new bundle for such small feature and wouldn't change any convention.
It would not be focused on controllers (it looks like it was the main concern?) and people who would like to use it would have to explicitly choose the directories autowired.

I think this is the best option as it doesn't force us to modify the standard edition temporarily (as DunglasActionBundle would probably have been imported in the core after some time), neither to change conventions, change bests standards and whatever. This would only be a new feature that may be used or not.

[wouterj]

Why would FrameworkBundle need Dunglas's bundle config, but not that of FOSRestBundle for instance?

[GuilhemN]

@wouterj I don't think we can make comparisons here as FOSRestBundle is very specific to API management whereas DunglasActionBundle can be used in any project.

[wouterj]

Alright, why would Frameworkbundle need Dunglas's bundle config, but not that of TacticianBundle?

In order words, Dunglas's bundle is nice, but I have no idea why it is more "core", more "nice", more "important to show to Symfonyians" than other bundles around there. Why would Dunglas's bundle need such special treatment?

[GuilhemN]

I don't think we need two interfaces for the same thing (the standard CommandInterface and CommandBus). It could be in the core but the current implementation would probably have to be deprecated to avoid having to support several interfaces doing the same thing.

BTW we're not talking about the same amount of lines. Tactician is composed of many files, here we're talking about maybe two hundred lines without the tests.

But I insist on the fact that it wouldn't be a new standard but only a new feature.

Edit: ok, i got the principle of tactician.
I still don't think this is the same: Command bus is a pattern and every developer uses its own patterns.
Imo the core should only contain features that a significant part of the users may use and that won't have to change frequently (in case another pattern becomes the standard for example).

[bocharsky-bw]

What's problem simply to add this bundle manually with $ composer require and configure it? It takes a few minutes. I don't think it's a big deal. If it is - look at KnpLabs/KnpRadBundle. It provides rapid development with Symfony and has many cool features, but I don't think to have it out-of-the-box is a good idea.

[GuilhemN]

@bocharsky-bw I don't think an entire bundle should be created and loaded for such small feature.
I also think that features that can be used by a significant part of the community should be in the core (as long as they don't add too much complexity).
BTW when something is in the core we know it is well supported/debugged and that we can receive feedbacks when changing something.

[dunglas]

The problem can be summarized by this tweet: https://twitter.com/lsmith/status/763074867919028224

Having a feature in the core (or in an officially supported bundle) instead of in a third-party, community maintained, repository makes a huge difference in term of marketing. Laravel has a feature like this one builtin, Laravel has a CQRS-like system builtin, Laravel has autowiring (and we now have it in core too - that makes a difference, some discussions appeared on Reddit PHP recently mentioning that Symfony now has autowiring builtin too).

Symfony is the undisputed leader in the field of PHP middlewares. It competes with J2EE and is the foundation of high-quality projects including Laravel itself, Drupal and many more. That's great.

But Symfony also has everything to be at the same time an excellent RAD framework. It only lacks some marketing, branding, official support and advertising.

Importing such feature in core will help.

[nilportugues]

This is no minor discussion but I agree on @dunglas' points.

If for instance, is a matter of rebranding this bundle, do so. Having an edition for this is a no-go as it is just causing pointless fragmentation.

[dunglas]

See #1034