Skip to content

Commit

Permalink
alerts-server: Remove error related dependencies from AbstractJsonCon…
Browse files Browse the repository at this point in the history
…troller

This is a part of a big refactor with the goal to allow the use of the
AbstractJsonController in different applications.

The changes include:
- Rename PrivateError to ServerError
- Split the JsonErrorRenderer into PublicErrorRenderer and ApplicationErrorMapper
- The JsonControllerComponents are now required to be implemented to set the
  application dependant classes.
- PublicError is moved to the commons package.
- Base application errors are moved to the commons package.
- JsonFieldValidationError is now a subclass of JsonControllerErrors.
- The ApplicationError file is split into several files, one per error type.
- Existing controllers now use MyJsonController.
- AbstractJsonControllerSpec now uses specific errors defined locally.
  • Loading branch information
AlexITC committed Feb 19, 2018
1 parent ed9700f commit 43aab04
Show file tree
Hide file tree
Showing 32 changed files with 297 additions and 180 deletions.
2 changes: 1 addition & 1 deletion alerts-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ These tests are useful for verifying that the API works as expected on the clien
Here are some details about the architecure design, they could be useful if you want to understand the code.

### Application results
Most results computed by our code base depend on the [ApplicationError](app/com/alexitc/coinalerts/errors/ApplicationError.scala) trait, it simply represents and error produced by our code that we should be able to handle.
Most results computed by our code base depend on the [ApplicationError](app/com/alexitc/coinalerts/commons/applicationErrors.scala) trait, it simply represents and error produced by our code that we should be able to handle.

There is a extensive use of [Scalatic Or and Every](http://www.scalactic.org/user_guide/OrAndEvery) for returning alternative error results, while it is similar to scala `Either[L, R]` type, it has a built-in mechanism for returning several errors instead of just 1 and it has a built-in non empty list make our code safer.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,11 @@ abstract class AbstractJsonController @Inject() (components: JsonControllerCompo
case _: ConflictError => Results.Conflict
case _: NotFoundError => Results.NotFound
case _: AuthenticationError => Results.Unauthorized
case _: PrivateError => Results.InternalServerError
case _: ServerError => Results.InternalServerError
}

val json = errors.head match {
case error: PrivateError =>
case error: ServerError =>
val errorId = ErrorId.create
logPrivateError(error, errorId)
renderPrivateError(errorId)
Expand All @@ -274,18 +274,18 @@ abstract class AbstractJsonController @Inject() (components: JsonControllerCompo
private def renderPublicErrors(errors: ApplicationErrors)(implicit lang: Lang) = {
val jsonErrorList = errors
.toList
.flatMap(components.errorRenderer.toPublicErrorList)
.map(components.errorRenderer.renderPublicError)
.flatMap(components.applicationErrorMapper.toPublicErrorList)
.map(components.publicErrorRenderer.renderPublicError)

Json.obj("errors" -> jsonErrorList)
}

private def logPrivateError(error: PrivateError, errorId: ErrorId) = {
private def logPrivateError(error: ServerError, errorId: ErrorId) = {
logger.error(s"Unexpected internal error = ${errorId.string}", error.cause)
}

private def renderPrivateError(errorId: ErrorId) = {
val jsonError = components.errorRenderer.renderPrivateError(errorId)
val jsonError = components.publicErrorRenderer.renderPrivateError(errorId)

Json.obj("errors" -> List(jsonError))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.alexitc.coinalerts.commons

import play.api.i18n.Lang

trait ApplicationErrorMapper {

def toPublicErrorList(error: ApplicationError)(implicit lang: Lang): Seq[PublicError]
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.alexitc.coinalerts.commons

import com.alexitc.coinalerts.errors.ApplicationError
import org.scalactic.{Bad, Good, One, Or}

import scala.concurrent.{ExecutionContext, Future}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
package com.alexitc.coinalerts.commons

import javax.inject.Inject

import com.alexitc.coinalerts.errors.JsonErrorRenderer
import com.alexitc.coinalerts.services.JWTService
import play.api.mvc.MessagesControllerComponents

import scala.concurrent.ExecutionContext

class JsonControllerComponents @Inject() (
val messagesControllerComponents: MessagesControllerComponents,
val jwtService: JWTService,
val errorRenderer: JsonErrorRenderer,
val executionContext: ExecutionContext)
trait JsonControllerComponents {

def messagesControllerComponents: MessagesControllerComponents

// TODO: allow to override it
def jwtService: JWTService

def executionContext: ExecutionContext

def publicErrorRenderer: PublicErrorRenderer

def applicationErrorMapper: ApplicationErrorMapper

}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.alexitc.coinalerts.errors
package com.alexitc.coinalerts.commons

/**
* A PublicError could be displayed to anyone.
Expand All @@ -7,3 +7,9 @@ sealed trait PublicError
case class GenericPublicError(message: String) extends PublicError
case class FieldValidationError(field: String, message: String) extends PublicError
case class HeaderValidationError(header: String, message: String) extends PublicError

object PublicError {
def genericError(message: String): PublicError = {
GenericPublicError(message)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package com.alexitc.coinalerts.commons

import com.alexitc.coinalerts.core.ErrorId
import play.api.libs.json.{JsValue, Json}

class PublicErrorRenderer {
def renderPublicError(publicError: PublicError): JsValue = publicError match {
case e: GenericPublicError =>
val obj = Json.obj(
"type" -> "generic-error",
"message" -> e.message
)
Json.toJson(obj)

case e: FieldValidationError =>
val obj = Json.obj(
"type" -> "field-validation-error",
"field" -> e.field,
"message" -> e.message
)
Json.toJson(obj)

case e: HeaderValidationError =>
val obj = Json.obj(
"type" -> "header-validation-error",
"header" -> e.header,
"message" -> e.message
)
Json.toJson(obj)
}

def renderPrivateError(errorId: ErrorId) = {
Json.obj(
"type" -> "server-error",
"errorId" -> errorId.string
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.alexitc.coinalerts.commons

// Top-level errors
trait ApplicationError
trait InputValidationError extends ApplicationError
trait ConflictError extends ApplicationError
trait NotFoundError extends ApplicationError
trait AuthenticationError extends ApplicationError
trait ServerError extends ApplicationError {
// contains data private to the server
def cause: Throwable
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.alexitc.coinalerts.commons

import com.alexitc.coinalerts.core.MessageKey
import play.api.libs.json.JsPath

sealed trait JsonControllerErrors

// play json validation errors
case class JsonFieldValidationError(path: JsPath, errors: Seq[MessageKey]) extends JsonControllerErrors with InputValidationError
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.alexitc.coinalerts

import com.alexitc.coinalerts.errors.ApplicationError
import org.scalactic.{Every, Or}

import scala.concurrent.Future
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,13 @@ package com.alexitc.coinalerts.errors

import javax.inject.Inject

import com.alexitc.coinalerts.core.ErrorId
import com.alexitc.coinalerts.commons._
import play.api.i18n.{Lang, MessagesApi}
import play.api.libs.json.{JsValue, Json}

class JsonErrorRenderer @Inject() (messagesApi: MessagesApi) {

def renderPublicError(publicError: PublicError): JsValue = publicError match {
case e: GenericPublicError =>
val obj = Json.obj(
"type" -> "generic-error",
"message" -> e.message
)
Json.toJson(obj)

case e: FieldValidationError =>
val obj = Json.obj(
"type" -> "field-validation-error",
"field" -> e.field,
"message" -> e.message
)
Json.toJson(obj)

case e: HeaderValidationError =>
val obj = Json.obj(
"type" -> "header-validation-error",
"header" -> e.header,
"message" -> e.message
)
Json.toJson(obj)
}

def renderPrivateError(errorId: ErrorId) = {
Json.obj(
"type" -> "server-error",
"errorId" -> errorId.string
)
}

def toPublicError(message: String): PublicError = {
GenericPublicError(message)
}
class MyApplicationErrorMapper @Inject() (messagesApi: MessagesApi) extends ApplicationErrorMapper {

def toPublicErrorList(error: ApplicationError)(implicit lang: Lang): Seq[PublicError] = error match {
case _: PrivateError => List.empty
override def toPublicErrorList(error: ApplicationError)(implicit lang: Lang): Seq[PublicError] = error match {
case _: ServerError => List.empty

case error: JWTError =>
List(renderJWTError(error))
Expand All @@ -56,6 +19,7 @@ class JsonErrorRenderer @Inject() (messagesApi: MessagesApi) {
case error: ReCaptchaError =>
List(renderReCaptchaError(error))

// TODO: avoid requiring this core mapper here
case JsonFieldValidationError(path, errors) =>
val field = path.path.map(_.toJsonString.replace(".", "")).mkString(".")
errors.map { messageKey =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.alexitc.coinalerts.errors

import javax.inject.{Inject, Singleton}

import com.alexitc.coinalerts.commons.{PublicError, PublicErrorRenderer}
import com.alexitc.coinalerts.core.ErrorId
import org.slf4j.LoggerFactory
import play.api.http.HttpErrorHandler
Expand All @@ -12,12 +13,12 @@ import play.api.mvc._
import scala.concurrent._

@Singleton
class PlayErrorHandler @Inject() (errorRenderer: JsonErrorRenderer) extends HttpErrorHandler {
class PlayErrorHandler @Inject() (errorRenderer: PublicErrorRenderer) extends HttpErrorHandler {

private val logger = LoggerFactory.getLogger(this.getClass)

def onClientError(request: RequestHeader, statusCode: Int, message: String) = {
val publicError = errorRenderer.toPublicError(message)
val publicError = PublicError.genericError(message)
val error = errorRenderer.renderPublicError(publicError)
val json = Json.obj(
"errors" -> Json.arr(error)
Expand All @@ -38,4 +39,4 @@ class PlayErrorHandler @Inject() (errorRenderer: JsonErrorRenderer) extends Http
val result = InternalServerError(json)
Future.successful(result)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.alexitc.coinalerts.errors

import com.alexitc.coinalerts.commons.ConflictError

// Daily price alert
sealed trait DailyPriceAlertError
case object RepeatedDailyPriceAlertError extends DailyPriceAlertError with ConflictError
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.alexitc.coinalerts.errors

import com.alexitc.coinalerts.commons.{InputValidationError, NotFoundError}

sealed trait ExchangeCurrencyError
case object UnknownExchangeCurrencyIdError extends ExchangeCurrencyError with InputValidationError
case object RepeatedExchangeCurrencyError extends ExchangeCurrencyError with InputValidationError
case object ExchangeCurrencyNotFoundError extends ExchangeCurrencyError with NotFoundError
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.alexitc.coinalerts.errors

import com.alexitc.coinalerts.commons.{ConflictError, InputValidationError, NotFoundError}
import com.alexitc.coinalerts.core.Count

// Fixed price alert
sealed trait FixedPriceAlertError
case object InvalidPriceError extends FixedPriceAlertError with InputValidationError
case object InvalidBasePriceError extends FixedPriceAlertError with InputValidationError
case object FixedPriceAlertNotFoundError extends FixedPriceAlertError with NotFoundError
case class TooManyFixedPriceAlertsError(reachedLimit: Count) extends FixedPriceAlertError with ConflictError
case object InvalidFilterError extends FixedPriceAlertError with InputValidationError
case object InvalidOrderError extends FixedPriceAlertError with InputValidationError
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.alexitc.coinalerts.errors

import com.alexitc.coinalerts.commons.AuthenticationError

sealed trait JWTError
case object AuthorizationHeaderRequiredError extends JWTError with AuthenticationError
case object InvalidJWTError extends JWTError with AuthenticationError
Loading

0 comments on commit 43aab04

Please sign in to comment.