Once upon a time, there was once a young gentleman who read some tutorials, some of the documentation, a fair few StackOverflow questions and maybe even asked one or two. After he had done so, he thought he had a fair handle on how things worked and went about building the functionality he wanted, and it worked! Until the wicked witch came along and tried from a different browser with no magic cookies and he realised that the UserIdentity
had been persisted between users... this was not good.
From the Play! docs (the Guice docs say similar)
New instances are created every time a component is needed. If a component is used more than once, then, by default, multiple instances of the component will be created. If you only want a single instance of a component then you need to mark it as a singleton.
The top of my IndexController
class (my assumption is that each request calls the index action once, and therefore requests a new UserIdentity
— is this wrong?)
@Singleton
class IndexController @Inject() (contentModel: ContentModel, site: Site, injector: Injector) extends Controller {
def index(url: String, page: Int = 1, sort: String, dir: Int) = Action.async { implicit request =>
implicit val queryString : QueryString = request.queryString
val userIdentityClass = injector.getInstance(classOf[UserIdentity])
implicit val userIdentity : Future[UserIdentity] = userIdentityClass.getUserIdentity
doContent(url, page, sort, dir) fallbackTo doAlias(url) fallbackTo do404(url)
}
userIdentity
is then passed implicitly into the bowels of the machine.
The getUserIdentity
method calls the getUser
method which does the authentication, and sets the user : UserModel
property. This is all passed in a future so that when we utilise userIdentity
later in the app we can map it so that we know auth is finished.
Here's the get user method and the println
that is being called for a non-authenticated user.
private def getUser(implicit request:Request[AnyContent]) : Future[UserModel] = {
if (user != null) {
println("User already set")
Future { user }
} else {
//Go and find the user from cookie/some other auth stuff
Note var user : UserModel = null
is on the UserIdentity
class so it should be null
on a new instance.
To get an instance on demand rather than when the class is instantiated, you'll use a provider like so:
class MyController @Inject() (userIdProv: Provider[UserIdentity])
More notes on your code:
If you can avoid it, don't inject the injector but the actual class (or a provider thereof) that you need. On the one hand you are creating a dependency to guice itself. Second, what would you inject in a unit test? A mock of the injector? Most importantly, however, injecting the component you depend on directly simply makes your code clearer. Think about it from a modelling perspective: You have this one thing that depends on this other thing to accomplish its task -- Rather than: I have this one thing that depends on an in injector (very technical, un-specific).
Why is your controller a singleton? First of all, play controllers are singletons anyway (not stated in the docs, so you probably can't rely on this being the case forever) and second, why would you want them to be singletons? Note that having a singleton always introduces some sort of a state - which is something you'll want to avoid unless there is a very specific need.
Future { user }
. This will create and execute a future. What you really want is a Future.successful(user)
which basically just wraps the user with the correct type.