Skip to content

dependency injection docs need to describe interaction with routes generator#2045

Merged
richdougherty merged 1 commit intoplayframework:masterfrom
jroper:2045-di-routes-generator
Apr 1, 2015
Merged

dependency injection docs need to describe interaction with routes generator#2045
richdougherty merged 1 commit intoplayframework:masterfrom
jroper:2045-di-routes-generator

Conversation

@jroper
Copy link
Member

@jroper jroper commented Mar 31, 2015

saw someone asking about this on the mailing list and they were confused when we pointed them to the scala docs since they are writing in java
http://www.playframework.com/documentation/2.2.x/ScalaDependencyInjection

@huntc
Copy link
Contributor

huntc commented Nov 17, 2013

Thanks Ben. The docs could reference the java based Activator template (which includes a tutorial):

http://typesafe.com/activator/template/play-spring-data-jpa

@richdougherty
Copy link
Member

What do you think about including references to relevant Activator templates from the Play docs? I can imagine they might be helpful.

@benmccann
Copy link
Contributor Author

If we include an Activator template for this we should probably include the Guice one along with the Spring one http://typesafe.com/activator/template/play-guice

i think it'd be good to have an example directly in the docs as well. it's usually easiest to just copy paste an example from a web page. downloading a project, extracting it, finding the relevant file, and opening it up is really only something i'd do for something rather large and involved

@benmccann benmccann added this to the 2.4.0 milestone Feb 26, 2015
@pvlugter
Copy link
Contributor

pvlugter commented Mar 5, 2015

@benmccann
Copy link
Contributor Author

I just did a quick glance at the docs. Do you still need to start a route with @ to dependency inject a Controller? That's not mentioned

@jroper
Copy link
Member

jroper commented Mar 5, 2015

@benmccann No. Something that should probably be mentioned is somewhere other than the migration guide is the difference between the routes generators, they are covered reasonably well in the migration guide though:

https://www.playframework.com/documentation/2.4.x/Migration24#Routing

So, to answer your question, if using the old static routes generator, yes, you need to use @ to dependency inject a controller. In the injected routes controller, everything is injected, regardless, the @ still has an effect, it means a Provider is injected, which actually replicates the current behaviour - the controller is looked up each time it's used, which may be useful for people who depend on that behaviour, and also may be useful for breaking cycles.

@benmccann
Copy link
Contributor Author

Thanks James. Yeah, the migration guide has good info there. And we should copy it to the dependency injection docs as well

@benmccann benmccann changed the title dependency injection docs are scala only dependency injection docs need to describe interaction with routes generator Mar 5, 2015
@jroper
Copy link
Member

jroper commented Mar 31, 2015

Attached PR.

@jroper jroper added the has-pr label Mar 31, 2015
@jroper jroper force-pushed the 2045-di-routes-generator branch from 0b6bfd6 to 736bdb5 Compare March 31, 2015 06:22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prototype controller? like a controller that's in development that isn't fully launched yet? why would injecting a provider help in that case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, prototype is spring terminology, for a component that is instantiated every time its requested. It's useful when the component has state that should not be shared between different times its used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh. I'd never heard that term as a Guice user.

Isn't the default behavior in Guice to create a new instance every time it's requested unless you mark it as a Singleton? Given that Play uses Guice by default, is there any difference? Or does Play hold onto a controller instance after it's injected?

We should be very clear about this since it could cause threadsafety issues if it doesn't act as the user expects.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's the default behaviour for Guice (and Guice doesn't have a term for it's default behaviour, which is why I used the spring terminology, where the default is singleton but you can explicitly make it prototype). Play holds a reference to the router, and the router holds a reference to the controllers, so while the controllers will be "prototype", ie if you were to have a another component that depended on the controller, it would get a difference instance to the one the router has, the controller the router has will never change unless you use the @ annotation.

As far as clarity about it - do controllers usually hold state that shouldn't be shared? I don't see a reason why they should.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used Webwork / Struts 2 when I was at Google. Each time a route was hit, it would create a new controller instance. That's also what Play's been doing to-date. I'm pretty used to that behavior now. I'm not sure what other frameworks do.

Of course the downside with that approach is that it creates a new controller instance each time you hit the route. I think it's reasonable to provide both options. I'm not entirely sure which one should be the default - the safer one or the more performant one. But we should definitely let users know they can't use non-threadsafe classes as instance variables unless they specify that in the routes file. It'd be easy not to expect that coming from Play 2.3.x

I might reword that paragraph as:

Play holds a reference to the router, and the router holds a reference to the controllers. When using the injected routes generator, prefixing the action with an @ symbol takes on a special meaning, it means instead of the controller being injected directly, a Provider of the controller will be injected. This allows, for example, use of non-threadsafe classes or an option for breaking cyclic dependencies. If @ is not specified, then that route will have slightly better performance, but you will need to recognize that any state is shared and ensure the instance members are threadsafe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoah, hold on there, talking about internal google tech? Aren't you gonna by fried by Google's do no evil laser for talking about that?

I'll be sure to stay indoors and away from lasers :-)

From a performance perspective, the new instance instantiation is really quite negligible. Looking up out of an injector can have some performance implications, but I think injecting a Provider should be no problem.

Why not just always do that then? Is there a reason to have both options?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benmccann
thanks for the conversation made here,I hate I haven't track the Play2.4 progress.
I just decide to migrate our spray based rest service to play based one,and come back to play 2.4.
finally I found that the DI in play 2.4 is really great but still a little mysterious。especially the@ here.
I think the document should enhanced about the DI,the @,and the prototype ,hen,I have to dig it here.
thanks for the detail here.:)
AFAIS,the @ have special means in the context of InjectedRouter,I think that when with the @,we should be careful for creating actors in the controller.

So In short it seems that in the context of InjectedRouter without the @ will instantiate the controller only once,and the user should make sure the controller is thread-safe if there have some state.and when there is the @,the controller will be created everytime the router route the request to the controller's action method right?but the user should make sure that some resource allocation too,like creating actors but with a specified name.

anyway the DI is really a big move.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jroper as I found some conversation in #3428
you mentioned thateffectively be a singleton which may means that If I have something like @Inject() (x:ControllerX),then the controller will not be singleton ?
and how about I add @Singleton at the class ControllerX
thanks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, as long as a component is only ever injected once into a singleton (eg, a controller into the singleton router), that component will be effectively singleton, as soon as it gets injected into another component, it's no longer a singleton. However, if you declare the component to be singleton, for example using the @Singleton annotation, then it will be a singleton.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jroper yeah,thanks.I test it that @Singleton and @ in the context of InjectedRouter ,it still a singleton.
after I develop with the DI way today,I found with the injected DAO,Serviceand Controller,it's a pleasure,thanks ,2.4 is really great.

@benmccann
Copy link
Contributor Author

Thanks James! Looks good to me. Just one small thing I had a question about

richdougherty added a commit that referenced this pull request Apr 1, 2015
dependency injection docs need to describe interaction with routes generator
@richdougherty richdougherty merged commit c13a7ce into playframework:master Apr 1, 2015
@jroper jroper deleted the 2045-di-routes-generator branch April 24, 2015 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants