8

I am trying to create a neat construction with for-comprehension for business logic built on futures. Here is a sample which contains a working example based on Exception handling:

(for {
  // find the user by id, findUser(id) returns Future[Option[User]]
  userOpt <- userDao.findUser(userId)        
  _ = if (!userOpt.isDefined) throw new EntityNotFoundException(classOf[User], userId)

  user = userOpt.get       

  // authenticate it, authenticate(user) returns Future[AuthResult]
  authResult <- userDao.authenticate(user)   
  _ = if (!authResult.ok) throw new AuthFailedException(userId)

  // find the good owned by the user, findGood(id) returns Future[Option[Good]]
  goodOpt <- goodDao.findGood(goodId)        
  _ = if (!good.isDefined) throw new EntityNotFoundException(classOf[Good], goodId)

  good = goodOpt.get        

  // check ownership for the user, checkOwnership(user, good) returns Future[Boolean]
  ownership <- goodDao.checkOwnership(user, good)
  if (!ownership) throw new OwnershipException(user, good)

  _ <- goodDao.remove(good) 
} yield {
  renderJson(Map(
    "success" -> true
  ))
})
.recover {
  case ex: EntityNotFoundException =>
    /// ... handle error cases ...
    renderJson(Map(
        "success" -> false,
        "error" -> "Your blahblahblah was not found in our database"
    ))
  case ex: AuthFailedException =>
    /// ... handle error cases ...
  case ex: OwnershipException =>
    /// ... handle error cases ...
}

However this might be seen as a non-functional or non-Scala way to handle the things. Is there a better way to do this?

Note that these errors come from different sources - some are at the business level ('checking ownership') and some are at controller level ('authorization') and some are at db level ('entity not found'). So approaches when you derive them from a single common error type might not work.

Andrey Chaschev
  • 15,055
  • 5
  • 43
  • 63

3 Answers3

8

Don't use exceptions for expected behaviour.

It's not nice in Java, and it's really not nice in Scala. Please see this question for more information about why you should avoid using exceptions for regular control flow. Scala is very well equipped to avoid using exceptions: you can use Eithers.

The trick is to define some failures you might encounter, and convert your Options into Eithers that wrap these failures.

// Failures.scala
object Failures {
   sealed trait Failure

   // Four types of possible failures here
   case object UserNotFound extends Failure
   case object NotAuthenticated extends Failure
   case object GoodNotFound extends Failure
   case object NoOwnership extends Failure
   // Put other errors here...

   // Converts options into Eithers for you
   implicit class opt2either[A](opt: Option[A]) {
      def withFailure(f: Failure) = opt.fold(Left(f))(a => Right(a))
   }
}

Using these helpers, you can make your for comprehension readable and exception free:

import Failures._    

// Helper function to make ownership checking more readable in the for comprehension
def checkGood(user: User, good: Good) = {
    if(checkOwnership(user, good))
        Right(good)
    else
        Left(NoOwnership)
}

// First create the JSON
val resultFuture: Future[Either[Failure, JsResult]] = for {
    userRes <- userDao.findUser(userId)
    user    <- userRes.withFailure(UserNotFound).right
    authRes <- userDao.authenticate(user)
    auth    <- authRes.withFailure(NotAuthenticated).right
    goodRes <- goodDao.findGood(goodId)
    good    <- goodRes.withFailure(GoodNotFound).right
    checkedGood <- checkGood(user, good).right
} yield renderJson(Map("success" -> true)))

// Check result and handle any failures 
resultFuture.map { result =>
    result match {
        case Right(json) => json // serve json
        case Left(failure) => failure match {
            case UserNotFound => // Handle errors
            case NotAuthenticated =>
            case GoodNotFound =>
            case NoOwnership =>
            case _ =>
        }
    }
}
Community
  • 1
  • 1
DCKing
  • 4,053
  • 2
  • 23
  • 41
  • This looks really nice and decent, though I need time to process it, because I'm relatively new to Scala :-) So I guess it has something to do with for-comprehension magic when handling right projections, like in this question: http://stackoverflow.com/questions/10866710/using-eithers-with-scala-for-syntax? – Andrey Chaschev Jun 06 '14 at 23:24
  • @AndreyChaschev Yeah something like that. You can reduce the overhead of helper functions by having your DAOs return `Either`s with corresponding failures, but that depends on taste I guess! – DCKing Jun 09 '14 at 11:12
  • 1
    @DCKing, if `findUser` returns a `Future[Option[User]]` as indicated by the OP then your for-comprehension won't work. You switch types to an `Either` on the second step of the for-comprehension and as far as I know, you can't do this. If you start with a `Future` then you need to continue using `Future` throughout the rest of the steps. – cmbaxter Apr 29 '15 at 17:05
  • @cmbaxter "You switch types to an Either on the second step of the for-comprehension and as far as I know, you can't do this." Have you checked? Because I thought I checked this with actual code. – DCKing Apr 30 '15 at 18:14
  • @DCKing, yes, I checked. Please refer to http://stackoverflow.com/questions/29949666/scala-futures-and-eithers. Also, since the first item is a `Future[Option[Int]]` then it's calling `flatMap` on that to get the next item in the sequence and that item is not producing a `Future`. – cmbaxter Apr 30 '15 at 19:02
  • In that case, my apologies. I will accept suggestions to improve this answer that doesn't make a mess of the types! – DCKing Oct 30 '15 at 11:21
7

You could clean up the for comprehension a little to look like this:

  for {
    user <- findUser(userId)
    authResult <- authUser(user)      
    good <- findGood(goodId)
    _ <- checkOwnership(user, good)    
    _ <- goodDao.remove(good) 
  } yield {
    renderJson(Map(
      "success" -> true
    ))
  }

Assuming these methods:

def findUser(id:Long) = find(id, userDao.findUser)
def findGood(id:Long) = find(id, goodDao.findGood)

def find[T:ClassTag](id:Long, f:Long => Future[Option[T]]) = {
  f(id).flatMap{
    case None => Future.failed(new EntityNotFoundException(implicitly[ClassTag[T]].runtimeClass, id))
    case Some(entity) => Future.successful(entity)
  }    
}

def authUser(user:User) = {
  userDao.authenticate(user).flatMap{
    case result if result.ok => Future.failed(new AuthFailedException(userId))
    case result => Future.successful(result)
  }    
}

def checkOwnership(user:User, good:Good):Future[Boolean] = {
  val someCondition = true //real logic for ownership check goes here
  if (someCondition) Future.successful(true)
  else Future.failed(new OwnershipException(user, good))
}

The idea here is to use flatMap to turn things like Options that are returned wrapped in Futures into failed Futures when they are None. There are going to be a lot of ways to do clean up that for comp and this is one possible way to do it.

cmbaxter
  • 34,269
  • 4
  • 81
  • 93
  • 1
    Beat me to it. This is exactly the same idea I mentioned in my answer and it's a good one. You really want your for-comprehension to be readable and clear. – wheaties Jun 06 '14 at 14:18
  • This is an interesting conversion idea, may be it would play even better with implicit conversions for `Option`s as @DCKing suggested. – Andrey Chaschev Jun 06 '14 at 23:45
5

The central challenge is that for-comprehensions can only work on one monad at a time, in this case it being the Future monad and the only way to short-circuit a sequence of future calls is for the future to fail. This works because the subsequent calls in the for-comprehension are just map and flatmap calls, and the behavior of a map/flatmap on a failed Future is to return that future and not execute the provided body (i.e. the function being called).

What you are trying to achieve is the short-cicuiting of a workflow based on some conditions and not do it by failing the future. This can be done by wrapping the result in another container, let's call it Result[A], which gives the comprehension a type of Future[Result[A]]. Result would either contain a result value, or be a terminating result. The challenge is how to:

  • provide subsequent function calls the value contained by a prior non-terminating Result
  • prevent the subsequent function call from being evaluated if the Result is terminating

map/flatmap seem like the candidates for doing these types of compositions, except we will have to call them manually, since the only map/flatmap that the for-comprehension can evaluate is one that results in a Future[Result[A]].

Result could be defined as:

trait Result[+A] {

  // the intermediate Result
  def value: A

  // convert this result into a final result based on another result
  def given[B](other: Result[B]): Result[A] = other match {
    case x: Terminator => x
    case v => this
  }

  // replace the value of this result with the provided one
  def apply[B](v: B): Result[B]

  // replace the current result with one based on function call
  def flatMap[A2 >: A, B](f: A2 => Future[Result[B]]): Future[Result[B]]

  // create a new result using the value of both
  def combine[B](other: Result[B]): Result[(A, B)] = other match {
    case x: Terminator => x
    case b => Successful((value, b.value))
  }
}

For each call, the action is really a potential action, as calling it on or with a terminating result, will simply maintain the terminating result. Note that Terminator is a Result[Nothing] since it will never contain a value and any Result[+A] can be a Result[Nothing].

The terminating result is defined as:

sealed trait Terminator extends Result[Nothing] {
  val value = throw new IllegalStateException()

  // The terminator will always short-circuit and return itself as
  // the success rather than execute the provided block, thus
  // propagating the terminating result
  def flatMap[A2 >: Nothing, B](f: A2 => Future[Result[B]]): Future[Result[B]] =
    Future.successful(this)

  // if we apply just a value to a Terminator the result is always the Terminator
  def apply[B](v: B): Result[B] = this

  // this apply is a convenience function for returning this terminator
  // or a successful value if the input has some value
  def apply[A](opt: Option[A]) = opt match {
    case None => this
    case Some(v) => Successful[A](v)
  }

  // this apply is a convenience function for returning this terminator or
  // a UnitResult
  def apply(bool: Boolean): Result[Unit] = if (bool) UnitResult else this
}

The terminating result makes it possible to to short-circuit calls to functions that require a value [A] when we've already met our terminating condition.

The non-terminating result is defined as:

trait SuccessfulResult[+A] extends Result[A] {

  def apply[B](v: B): Result[B] = Successful(v)

  def flatMap[A2 >: A, B](f: A2 => Future[Result[B]]): Future[Result[B]] = f(value)
}

case class Successful[+A](value: A) extends SuccessfulResult[A]

case object UnitResult extends SuccessfulResult[Unit] {
  val value = {}
}

The non-teminating result makes it possible to provide the contained value [A] to functions. For good measure, I've also predefined a UnitResult for functions that are purely side-effecting, like goodDao.removeGood.

Now let's define your good, but terminating conditions:

case object UserNotFound extends Terminator

case object NotAuthenticated extends Terminator

case object GoodNotFound extends Terminator

case object NoOwnership extends Terminator

Now we have the tools to create the the workflow you were looking for. Each for comprehention wants a function that returns a Future[Result[A]] on the right-hand side, producing a Result[A] on the left-hand side. The flatMap on Result[A] makes it possible to call (or short-circuit) a function that requires an [A] as input and we can then map its result to a new Result:

def renderJson(data: Map[Any, Any]): JsResult = ???
def renderError(message: String): JsResult = ???

val resultFuture = for {

  // apply UserNotFound to the Option to conver it into Result[User] or UserNotFound
  userResult <- userDao.findUser(userId).map(UserNotFound(_))

  // apply NotAuthenticated to AuthResult.ok to create a UnitResult or NotAuthenticated
  authResult <- userResult.flatMap(user => userDao.authenticate(user).map(x => NotAuthenticated(x.ok)))

  goodResult <- authResult.flatMap(_ => goodDao.findGood(goodId).map(GoodNotFound(_)))

  // combine user and good, so we can feed it into checkOwnership
  comboResult = userResult.combine(goodResult)

  ownershipResult <- goodResult.flatMap { case (user, good) => goodDao.checkOwnership(user, good).map(NoOwnership(_))}

  // in order to call removeGood with a good value, we take the original
  // good result and potentially convert it to a Terminator based on
  // ownershipResult via .given
  _ <- goodResult.given(ownershipResult).flatMap(good => goodDao.removeGood(good).map(x => UnitResult))
} yield {

  // ownership was the last result we cared about, so we apply the output
  // to it to create a Future[Result[JsResult]] or some Terminator
  ownershipResult(renderJson(Map(
    "success" -> true
  )))
}

// now we can map Result into its value or some other value based on the Terminator
val jsFuture = resultFuture.map {
  case UserNotFound => renderError("User not found")
  case NotAuthenticated => renderError("User not authenticated")
  case GoodNotFound => renderError("Good not found")
  case NoOwnership => renderError("No ownership")
  case x => x.value
}

I know that's a whole lot of setup, but at least the Result type can be used for any Future for-comprehension that has terminating conditions.

Arne Claassen
  • 13,120
  • 3
  • 58
  • 98
  • I've added a bit more explanation for how and why `Result` does what it does. HTH. – Arne Claassen May 02 '15 at 14:20
  • This is a nice solution which addresses the exact problem and is easier for the eye than two others. I am marking it as accepted. Since you are top 2% of the week and there are two other wall-of-text-type answers, I could have done it straight away. But of course you have to read it first. :) My guess is that readability could somehow be improved to have more resemblance with if-then-else branching. – Andrey Chaschev May 03 '15 at 08:51