add missing @Singleton annotation to templates controllers

[naferx]

No description provided.

[gmethvin]

@naferx I don't see any reason why these need to be singleton. They don't hold any internal state.

[naferx]

@gmethvin You're right. They don't hold any internal state.
But I added it here by consistency because https://github.com/playframework/playframework/blob/master/templates/play-scala/app/controllers/HomeController.scala contains the annotation.

So, singleton in HomeController.scala must be removed ? or added to HomeController.java ?

WDYT. Thanks

[gmethvin]

Well, my general rule is to leave it off unless there is a good reason to do so. In HomeController's case I'd expect creating a new object to be faster than going through the Singleton scope logic to get the instance.

Assuming you keep your controllers stateless, there usually would be little or no benefit to using the Singleton scope, so I think leaving it off by default is a good idea.

[marcospereira]

At this comment, @jroper states that because the router is a singleton, "by association, the controllers it invokes are singleton". So, the singleton annotation is unnecessary here unless you are using the @ syntax at your routes file. From the docs:

The injected routes generator also supports the @ operator on routes, but it has a slightly different meaning (since everything is injected), if you prefix a controller with @, instead of that controller being directly injected, a JSR 330 Provider for that controller will be injected. This can be used, for example, to eliminate circular dependency issues, or if you want a new action instantiated per request.

Maybe a better version of this PR is remove these annotations and better document how controllers are created/injected?

[mkurz]

Yeah, this should be documented. I wasn't really aware that controllers even without the singleton annotation are effective still singletons.

[gmethvin]

@marcospereira You're right, in a typical Play app controllers would effectively be singleton. But you could possibly define your own router that isn't a singleton, or have multiple routers that referenced the same controller, in which case it would make a difference.

[marcospereira]

@gmethvin yep. :-)

My point is that instead of just add/remove annotations from the templates, we should instead document this behavior (when controllers are singletons and when they aren't).

[gmethvin]

Well, we can mention that the Router is Singleton and has the controllers injected, so they are effectively singletons. It doesn't necessarily have to work that way, though; that's just a consequence of the current implementation. If a user wants to ensure Singleton-ness, I'd still recommend they add the annotation or explicitly bind it in the Singleton scope.

[naferx]

@gmethvin, @marcospereira & @mkurz Thanks for the feedback.
I'll work on it over the weekend

[wsargent]

Closing due to inactivity -- @naferx if you pick this up, the templates have moved to https://github.com/playframework/play-java-intro for example