Why do futures get called in a for comprehension if they aren't used?

Question

I'm trying to implement a system that caches requests to an external API. If the response is in cache, then no request should be made to the external site.

I have two methods:

// Check to see if the response is in the database
def checkCache(searchParameters: JsValue): Future[Option[JsValue]]

// Call the external API and get the JSON response
def getResponse(path: String): Future[JsValue]

And then I try to do the following:

val json: Future[JsValue] = for {
  databaseJson <- checkCache(searchParameters)
  externalJson <- getResponse(path)
} yield databaseJson match {
  case None => externalJson
  case Some(x) => x
}

This works, but a request to the external API is made all the time, even when the cached result is returned. This is obviously not what I want because it's slow.

How do I fix this?

Answer 1

The for comprehension maps over the futures, not the Option within it. Your code would translate to this

checkCache(searchParameters) flatMap { databaseJson =>
  getResponse(path) map { externalJson =>
    databaseJson match {
      case None => externalJson
      case Some(x) => x
    }
  }
}

So you are always calling getResponse() obviously.

You need something along the lines of this (not tested):

checkCache(searchParameters) flatMap { databaseJson =>
  databaseJson match {
    case None => getResponse(path)
    case Some(x) => Future.successful(x)
  }
}

Answer 2

You could also give this a shot:

val json: Future[JsValue] = for {
  databaseJson <- checkCache(searchParameters)
  json <- databaseJson.fold(getResponse(path))(js =>Future.successful(js))
} yield json

Similar in spirit to Marius Soutier's answer, but doing the Option checking via a fold right in the for comprehension.

Answer 3

This is a little cheesy, but still:

checkCache(params) map (_.get) fallbackTo getResponse(path)

Also not tested.

Update:

I didn't like the conversion to failure by None.get, but actually this isn't cheesy at all but is very natural. More naturally:

checkCache(params) filter (_.nonEmpty) map (_.get) fallbackTo getResponse(path)

The Some.get is now just a wart due to the asymmetry in the two futures (that they are not both Option).