3

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."))
    }
  }
wwkudu
  • 2,778
  • 3
  • 28
  • 41
Felipe
  • 7,013
  • 8
  • 44
  • 102

3 Answers3

12

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.

marcospereira
  • 12,045
  • 3
  • 46
  • 52
  • thanks for your answer. I am using map and flatMap now. My only doubt now is how to return a value from a flatMap. I edited the question. Could you help me please? – Felipe Jan 23 '18 at 15:18
  • You don't return only "a value" inside your `flatMap`, but instead a `Future[T]` (where `T` is some type). Let's first see `map`: the result of `future.map(Future.successful(Ok))` is a `Future[Future[Result]]`. But `Action.async` expects only a `Future[Result]` and then you need to use `flatMap` since `future.flatMap(Future.successful(Ok))` returns a `Future[Result]`. See more [here](https://stackoverflow.com/questions/31641190/futures-map-vs-flatmap). – marcospereira Jan 23 '18 at 18:54
  • Hi @marcospereira, I edited the question to contemplate the use of `flatMap` and `map`. I guess with this code I am doing what you said in your answer. But I still cannot see the `gpuList` inside the `rackList`. – Felipe Jan 23 '18 at 19:14
  • See my edit. There is now enough information to figure it out how to correct your code. – marcospereira Jan 23 '18 at 20:00
  • My problem is on the second example. The first one I got to work like you said. Maybe it is better to open a new question..... – Felipe Jan 23 '18 at 20:27
5

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].

  1. 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.

o-0
  • 1,713
  • 14
  • 29
  • thank you for your answer Dave. How do I return Ok using flatMap nested with map? I edited the question to show you my example. – Felipe Jan 23 '18 at 15:08
  • @FelipeOliveiraGutierrez no problem. Within the body of `flatMap` and `map` you write your result as usual (without wrapping it in a `Future`). Do you get any specific error/warning? – o-0 Jan 23 '18 at 17:19
  • Yes, I write without wrapping in a Future. I have this compilation error: `type mismatch; [error] found : play.api.mvc.Result. [error] required: scala.concurrent.Future[play.api.mvc.Result]`. If I write like this `Future.successful(Ok(Json.toJson(rackSeq)).as(JSON))` , I got nothing inside the `rackSeq`. I guess it is because the `rackSeq` is been processed inside the map. – Felipe Jan 23 '18 at 17:55
  • I edited the second method on the question to explain better what I am seeing. the method getRacks returns only the Rack, but not the gpuList. It only returns the gpuList if I use Await.result to block the thread. I hope that I could be clear of what is my issue. – Felipe Jan 23 '18 at 18:35
2

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

Luca T.
  • 1,641
  • 1
  • 14
  • 18