2

I am new to Scala and currently trying to work with the play framework.

This is working code I wrote:

def authenticate = Action (BodyParsers.parse.json) { req =>
    req.body.validate[AuthenticationForm].map {form =>
        UserRepository.findByCredentials(form).map { user =>
            user.apiKeys.find(_.deviceId == form.deviceId).map { apiKey =>
                Ok(Json.toJson(apiKey))
            }.getOrElse({
                // HOW DO I TRANSFORM THIS INTO MORE BEAUTIFUL CODE
                val createdApiKey = ApiKeyRepository.create(new ApiKey(form.deviceId, form.deviceId))
                val userToWithNewApiKey = user.copy(apiKeys = user.apiKeys.:+(createdApiKey))
                UserRepository.update(userToWithNewApiKey)

                Ok(Json.toJson(createdApiKey))
            })
        }.getOrElse {
            Unauthorized
        }
    }.getOrElse {
        BadRequest
    }
}

Well, that does not look tooo nice. Can we do better? I can not, yet. But I saw this stackoverflow post: https://stackoverflow.com/a/24085333/3038183 That looks pretty nice :)

Now I am wondering, how to transform my code, so it looks like in the given example. Of course I already tried, but I could not make it compile and also do not know how to handle the code after the comment ("HOW DO I TRANSFORM THIS INTO MORE BEAUTIFUL CODE"). In my case I am using play.api.mvc.Result instead of "Failure" as given in the link above. So of what type should my Either[play.api.mvc.Result, ?WhatHere?] be?

Best regards

EDIT: I accepted Travis' answer. Thank you very much.

For anyone interested here is better looking code I could write thanks to Travis:

def getApiKey(user: User, deviceId: String) : ApiKey = {
  user.apiKeys.find(_.deviceId == deviceId).getOrElse {
    val createdApiKey =
      ApiKeyRepository.create(new ApiKey(deviceId, deviceId))

    val userToWithNewApiKey =
      user.copy(apiKeys = user.apiKeys.:+(createdApiKey))

    UserRepository.update(userToWithNewApiKey)
    createdApiKey
  }
}

def authenticate = Action (BodyParsers.parse.json) { req =>
  (for {
    form <- req.body.validate[AuthenticationForm].asOpt.toRight(BadRequest).right
    user <- UserRepository.findByCredentials(form).toRight(Unauthorized).right
  } yield {
      Ok(Json.toJson(getApiKey(user, form.deviceId)))
  }).merge
}
Community
  • 1
  • 1
Tim Joseph
  • 847
  • 2
  • 14
  • 28

1 Answers1

3

This is quick and untested but should be a decent start. First you can collapse some of the nesting by using toRight to get Either[Status, ?]. Either isn't monadic, but its right projection (which we can get using .right is). Once failure is no longer a possibility, we use yield to work with the results. I've rewritten your apiKey stuff a little to avoid duplicating the Ok(Json.toJson(key)) part.

def authenticate = Action (BodyParsers.parse.json) { req =>
  for {
    form <- req.body.asOpt.toRight[Status](BadRequest).right
    user <- UserRepository.findByCredentials(form).toRight[Status](
      Unauthorized
    ).right
  } yield {
    val apiKey = user.apiKeys.find(_.deviceId == form.deviceId).getOrElse {
      val createdApiKey =
        ApiKeyRepository.create(new ApiKey(form.deviceId, form.deviceId))

      val userToWithNewApiKey =
        user.copy(apiKeys = user.apiKeys.:+(createdApiKey))

      UserRepository.update(userToWithNewApiKey)
      createdApiKey
    }
    Ok(Json.toJson(apiKey)): Status
  }.e.merge
}

The final result of the for-comprehension (i.e. everything except the .e.merge) is a RightProjection[Status, Status]. We convert that back to a plain old Either[Status, Status] with .e. At this point we no longer need to track the distinction between failures and success, so we convert the whole thing to a Status with .merge.

Travis Brown
  • 138,631
  • 12
  • 375
  • 680