Best practices to use Future inside a Controller (Play+Scala)

Question

In my Play web-app I am using val resultRack = Await.result(futureList, Duration.Inf) to get the result from a Future. Is there another better way (using best practices) to get the result from the DB? If I use onComplete or onSuccess my Controller finishes execution and the result is not in the val yet. Below is my method of the Controller. Everything is working, but I need to follow more best practices in Scala.

Edited: I am already using Action.async on other methods. But in this one I cannot use, basically because of the either.fold. I guess I need a map surrounding all the code of the method before to validate the json.

  def addRack = Action(parse.json) { request =>
    val either = request.body.validate[Rack]
    either.fold(
      errors => BadRequest("invalid json Rack.\n"),
      rack => {
        val f: Future[Option[RackRow]] = rackRepository.getById(rack.id)
        val result = Await.result(f, Duration.Inf)
        result match {
          case Some(r) =>
            // If the Rack already exists we update the produced and currentTime properties
            val fGpu: Future[Seq[GpuRow]] = gpuRepository.getByRack(r.id)
            // val total = fGpu.map(_.map(_.produced).sum)
            val resultGpu = Await.result(fGpu, Duration.Inf)
            val total = resultGpu.map(_.produced).sum
            rackRepository.update(r.id, Some(total), Some(System.currentTimeMillis))
            Ok("Rack already exists! Updated produced and currentTime.\n")
          case None =>
            // If the Rack does not exist we create it.
            val rackRow = RackRow(rack.id, rack.produced, System.currentTimeMillis)
            rackRepository.insert(rackRow)
            Ok
        }
      }
    )
  }

New method using flatMap and map. My problem is that I am creating and filling the seq rackSeq inside the Controller. The gpuSeq that I am using to create this object is not evaluated because it is from the Future. How should I do to evaluate this Future gpuSeq? On my results I only can see the rackSeq, but the list of gpuSeq is always empty.

Also, if the code Util.toTime(at) throws an error I cannot catch this with recover. As I understood by the answers I could do this....

  def getRacks(at: String) = Action.async { implicit request: Request[AnyContent] =>

    var rackSeq: Seq[Rack] = Seq.empty
    var gpuSeq: Seq[Gpu] = Seq.empty

    rackRepository.get(Util.toTime(at)).flatMap { resultRack: Seq[RackRow] =>
      resultRack.map { r: RackRow =>
        gpuRepository.getByRack(r.id).map { result: Seq[GpuRow] =>
          result.map { gpuRow: GpuRow =>
            gpuSeq = gpuSeq :+ Gpu(gpuRow.id, gpuRow.rackId, gpuRow.produced, Util.toDate(gpuRow.installedAt))
            println(gpuRow)
          }
        }
        val rack = Rack(r.id, r.produced, Util.toDate(r.currentHour), gpuSeq)
        rackSeq = rackSeq :+ rack
      }

      //      val result = Await.result(listGpu, Duration.Inf)
      //      result.foreach { gpuRow =>
      //        gpuSeq = gpuSeq :+ Gpu(gpuRow.id, gpuRow.rackId, gpuRow.produced, Util.toDate(gpuRow.installedAt))
      //      }
      Future.successful(Ok(Json.toJson(rackSeq)).as(JSON))
    }.recover {
      case pe: ParseException => BadRequest(Json.toJson("Error on parse String to time."))
      case e: Exception => BadRequest(Json.toJson("Error to get racks."))
      case _ => BadRequest(Json.toJson("Unknow error to get racks."))
    }
  }

Answer 1

Do not ever use Await.result inside a Play controller. This will block the thread and kill one of the main benefits of using a reactive framework like Play. Instead map or flatMap the Future to generate a Result. For example, suppose you have the following RackRepository:

class RackRepository {
  def racks: Future[Seq[Rack]] = ???
}

In your controller, instead of doing:

def wrong = Action {
  val racks: Future[Seq[Rack]] = rackRepository.racks
  // This is wrong, don't do that
  val racksSeq = Await.result(racks, Duration.Inf)
  Ok(Json.toJson(racksSeq))
}

What you do is, you use Action.async and map over your future to generate a result:

def list = Action.async {
  rackRepository.racks.map { racks =>
    Ok(Json.toJson(racks))
  }
}

If you need to nest multiple future results, then use flatMap instead.

Edit:

From your first example, what you need to do is to understand the difference between map and flatMap. This one looks like a good start:

Futures - map vs flatmap

Let's look at some examples:

val firstFuture: Future[String] = ??? // it does not mater where it comes from
val secondFuture: Future[String] = ??? // it does not mater where it comes from

val f1: Future[Int] = firstFuture.map(_.toInt)
val f2: Future[Future[String]] = firstFuture.map(secondFuture)
val f3: Future[String] = firstFuture.flatMap(secondFuture)

// Let's start to combine the future values
val f4: Future[Future[String]] = firstFuture.map { first =>
  secondFuture.map { second =>
    first + second // concatenate
  }
}

// But what if we want a Future[String] instead of a Future[Future[String]]?
// flatMap to the rescue!
val f5: Future[String] = firstFuture.flatMap { first =>
  secondFuture.map { second =>
    first + second // concatenate
  }
}

See? No Await. Then we have your code:

val fGpu: Future[Seq[GpuRow]] = gpuRepository.getByRack(r.id)
// val total = fGpu.map(_.map(_.produced).sum)
val resultGpu = Await.result(fGpu, Duration.Inf)

Why not combine flatMap and map as I did for the f5? In other words, why to Await on fGpu instead of map it to return a Future[Result]?

gpuRepository.getByRack(r.id).map { gpuRows =>
  val total = gpuRows.map(_.produced).sum
  rackRepository.update(r.id, Some(total), Some(System.currentTimeMillis))
  Ok("Rack already exists! Updated produced and currentTime.\n")
}

Of course, you need to use Action.async and flatMap for f.

Answer 2

Couple of things here regarding your code and then ofcourse about your question about the future:

1. Dont mix a controller with a model: generally speaking controller are a set of methods (within a controller class) that gets a request and returns a result(OK, REDIRECT, etc.). Models are a set of methods within classes/objects/interfaces that gets a set of parameters, deals with external resources and returns its result to the controller.

2. Methods are your friend: you can divide your code more into different methods. For example, your method name is addRack but its body also covers some processing, you could put them in different methods either in the controller, or models; depends where they belong.

3. Never Await: There is a reason for it, which is you are occupying threads and won't leave them alone during the await time. This will lead to inefficiency of your app in terms of memory and cpu usage.

4. Map is your friend: use map when you call a method that is returning the future. For example, you want to call this method:

def hiFromFuture : Future[String] = Future{...}

hiFromFuture.map{
  futureResult: String => //when Future is successful 
}

You should also use flatmap if you have multiple consecutive future calls. For example, assume that hiFromFuture2 has the same signature/body as hiFromFuture:

hiFromFuture.map{
  futureResult: String => hiFromFuture2.map{ futureResult => }

}

should be written as:

hiFromFuture.flatMap{
  futureResult: String => //when first future is successful 
    hiFromFuture2.map{ 
      futureResult => //when second future is successful 
    }
}

to avoid the Future[Future[String]]; and get Future[String].

5. Recover as well, for failed futures: What if you don't get the result? You use recover. For example:

   hiFromFuture.map{gotData => Ok("success")}.recover{case e: Exception => BadRequest}

Please note that you can use any exception that you are expecting within the recover block.

Answer 3

If your problem is how to manage the error case of json validation, in a context where the successful path will return a Future, you can simply wrap the Result in an already successfully completed Future, i.e. Future.successful(BadRequest(...)). Like below

def addRack = Action(parse.json).async { request =>
  val either = request.body.validate[Rack]
  either.fold(
    errors => Future.successful(BadRequest("invalid json Rack.\n")),
    rack => {
      rackRepository.getById(rack.id).map {
        case Some(r) =>
          //...

          Ok("Rack already exists! Updated produced and currentTime.\n")
        case None =>
          //...

          Ok
      }
    }
  )
}

Then when you have nested futures you should flatmap as well stated by marcospereira and Dave Rose