Search code examples
scaladictionaryplayframework-2.0for-comprehensionflatmap

Scala async/callback code rewriting


Simple code that should check user by pass, user is active and after that update last login datetime.

  def authenticate() = Action.async { implicit request => 
    loginForm.bindFromRequest.fold(
        errors => Future.successful(BadRequest(views.html.logon(errors))),
        usersData =>{
           val cursor =  this.collection.find(BSONDocument("name" -> usersData._1)).one[Account].map(_.filter(p=>p.password == hashedPass(usersData._2, usersData._1)))
           cursor.flatMap(p => p match {
               case None => Future.successful(BadRequest(views.html.logon(loginForm.withGlobalError("user/pass incorect!!!"))))
               case Some(user) => {
                 if(!user.active) 
                   Future.successful(BadRequest(views.html.logon(loginForm.withGlobalError("inactive!!!"))))
                 else collection.update(BSONDocument("_id" -> user.id), 
                          BSONDocument("$set" -> 
                          BSONDocument("lastLogin" -> BSONDateTime(new org.joda.time.DateTime().getMillis()))))
                          .flatMap(x => gotoLoginSucceeded(user.id.stringify))

               }
               })
            })
  }  

How to rewrite it to less flatMap/map spaghetti?

Another solution

def authenticate() = AsyncStack { implicit request => 
loginForm.bindFromRequest.fold(
    errors => Future.successful(BadRequest(views.html.logon(errors))),
    usersData =>{
      for{
        user <- this.collection.find(BSONDocument("name" -> usersData._1)).one[Account].map(_.filter(p=>p.password == hashedPass(usersData._2, usersData._1)))
        update <- {
         lazy val update = collection.update(BSONDocument("_id" -> user.get.id), 
         BSONDocument("$set" -> 
         BSONDocument("lastLogin" -> BSONDateTime(new org.joda.time.DateTime().getMillis()))))
         update
        }
        result <- {
         lazy val result = gotoLoginSucceeded(user.get.id.stringify)
         result
        } 
      } yield
        if(user.isEmpty) BadRequest(views.html.logon(loginForm.withGlobalError("login\pass mismatch")))
        else if(!user.get.active) BadRequest(views.html.logon(loginForm.withGlobalError("inactive")))
        else if(update.err.isEmpty) result
        else  InternalServerError(views.html.logon(loginForm.withGlobalError("server error")))
        })

}


Solution

  • I would probably refactor the code into something like this:

    def authenticate() = Action.async { implicit request => 
      loginForm.bindFromRequest.fold(
         hasErrors = displayFormWithErrors,
         success = loginUser)
    }  
    
    private def displayFormWithErrors[T](errors:Form[T]) = 
      Future.successful(BadRequest(views.html.logon(errors)))
    
    private def loginUser(userData:(String, String)) = {
      val (username, password) = userData
    
      findUser(username, password)
        .flatMap {
          case None => 
            showLoginFormWithError("user/pass incorect!!!")
          case Some(user) if (!user.active) =>
            showLoginFormWithError("inactive!!!")
          case Some(user) =>
            updateUserAndRedirect(user)
      }
    }
    
    private def findUser(username:String, password:String) =
      this.collection
        .find(BSONDocument("name" -> username))
        .one[Account]
        .map(_.filter(_.password == hashedPass(password, username)))
    
    private def showLoginFormWithError(error:String) = 
      Future.successful(BadRequest(
        views.html.logon(loginForm.withGlobalError(error))))
    
    private def updateUserAndRedirect(user:Account) = 
      updateLastLogin(user)
        .flatMap(_ => gotoLoginSucceeded(user.id.stringify))
    
    private def updateLastLogin(user:Account) = 
      collection
        .update(BSONDocument("_id" -> user.id), 
                  BSONDocument("$set" -> 
                  BSONDocument("lastLogin" -> 
                  BSONDateTime(new JodaDateTime().getMillis()))))