From 1d9ea9f0d1887408b30d059252d4fca35e93d6d3 Mon Sep 17 00:00:00 2001 From: Julien Richard-Foy Date: Wed, 14 Oct 2020 21:49:21 +0200 Subject: [PATCH] Adopt a more idiomatic style - Use precise API error responses in endpoints descriptions, rather than the generic type `ApiError` - Make sure responses and documentation are consistent together - Quick fix to the authentication directive --- .../lab/api/endpoints4s/ApartmentsApi.scala | 22 ++------ .../ApartmentsEndpointsDefinition.scala | 30 +++++------ .../ApartmentsEndpointsServer.scala | 36 ++++++------- .../api/endpoints4s/CustomStatusCodes.scala | 38 ++++++++++---- .../api/endpoints4s/QueryStringParams.scala | 4 +- .../lab/api/endpoints4s/SecuritySupport.scala | 4 +- .../InMemoryNarrowerApartmentsStorage.scala | 52 +++++++++++++++++++ .../storage/NarrowerApartmentsStorage.scala | 16 ++++++ 8 files changed, 134 insertions(+), 68 deletions(-) create mode 100644 src/main/scala/io/scalac/lab/api/storage/InMemoryNarrowerApartmentsStorage.scala create mode 100644 src/main/scala/io/scalac/lab/api/storage/NarrowerApartmentsStorage.scala diff --git a/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsApi.scala b/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsApi.scala index 9b19e3b..8eb2866 100644 --- a/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsApi.scala +++ b/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsApi.scala @@ -11,8 +11,7 @@ import io.scalac.lab.api.model.ApiError import io.scalac.lab.api.model.ApiError.UnauthorizedError import io.scalac.lab.api.security.Security.ApiKey import io.scalac.lab.api.security.SecurityService -import io.scalac.lab.api.storage.InMemoryApartmentsStorage -import io.scalac.lab.api.tapir.ApartmentsEndpointsServer +import io.scalac.lab.api.storage.InMemoryNarrowerApartmentsStorage import scala.concurrent.{ExecutionContext, Future} import scala.io.StdIn @@ -24,7 +23,7 @@ object ApartmentsApi extends App { private object DocumentedEndpoints extends ApartmentsEndpointsDefinition - with openapi.Endpoints + with openapi.EndpointsWithCustomErrors with openapi.JsonEntitiesFromSchemas with openapi.JsonSchemas { @@ -45,22 +44,9 @@ object ApartmentsApi extends App { implicit tupler: Tupler.Aux[U, ApiKey, I]): DocumentedEndpoint = super - .authenticatedEndpoint(request, response, docs) + .authenticatedEndpoint(request, response ++ unauthorized, docs) .withSecurity(SecurityRequirement("apiKeyAuth", SecurityScheme("apiKey", None, Some("api-key"), Some("header"), None, None))) - override def withStatusCodes[A](responses: List[DocumentedResponse], codes: Int*): List[DocumentedResponse] = { - responses :++ codes.flatMap { - case 400 => badRequest - case 401 => unauthorized - case 404 => notFound - } - } - - // We are going to return empty responses here for errors, - // because we want to customize status codes for each endpoint independently - override lazy val clientErrorsResponse: List[DocumentedResponse] = List.empty - override lazy val serverErrorResponse: List[DocumentedResponse] = List.empty - } private object DocumentationServer extends server.Endpoints with server.JsonEntitiesFromEncodersAndDecoders { @@ -82,7 +68,7 @@ object ApartmentsApi extends App { ) } - private val apiStorage = new InMemoryApartmentsStorage() + private val apiStorage = new InMemoryNarrowerApartmentsStorage() private val apiSecurity = new SecurityService { override def authenticate(token: Option[String]): Future[Either[ApiError, ApiKey]] = token match { diff --git a/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsDefinition.scala b/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsDefinition.scala index d567fdd..7a77bea 100644 --- a/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsDefinition.scala +++ b/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsDefinition.scala @@ -1,8 +1,9 @@ package io.scalac.lab.api.endpoints4s -import endpoints4s.algebra.{Endpoints, JsonEntitiesFromSchemas} +import endpoints4s.algebra.{EndpointsWithCustomErrors, JsonEntitiesFromSchemas} import endpoints4s.generic.JsonSchemas -import io.scalac.lab.api.model.{Address, Apartment, ApiError, Paging} +import io.scalac.lab.api.model.ApiError.{BadRequestError, NotFoundError} +import io.scalac.lab.api.model.{Address, Apartment, Paging} import io.scalac.lab.api.security.Security.ApiKey /** @@ -16,7 +17,7 @@ import io.scalac.lab.api.security.Security.ApiKey * - fifth for deleting apartment by id. */ trait ApartmentsEndpointsDefinition - extends Endpoints + extends EndpointsWithCustomErrors with JsonEntitiesFromSchemas with JsonSchemas with SecuritySupport @@ -26,41 +27,38 @@ trait ApartmentsEndpointsDefinition private implicit val addressSchema: JsonSchema[Address] = genericJsonSchema private implicit val apartmentSchema: JsonSchema[Apartment] = genericJsonSchema - val listApartments: Endpoint[(Paging, ApiKey), Either[ApiError, List[Apartment]]] = + val listApartments: Endpoint[(Paging, ApiKey), Either[BadRequestError, List[Apartment]]] = authenticatedEndpoint( request = get(path / "v1" / "data" / "apartments" /? pagingQueryString), - response = withStatusCodes(ok(jsonResponse[List[Apartment]], Some("A list of apartments")), Unauthorized, BadRequest), + response = badRequest.orElse(ok(jsonResponse[List[Apartment]], Some("A list of apartments"))), docs = EndpointDocs().withDescription(Some("An endpoint responsible for listing all available apartments")) ) - val getApartment: Endpoint[(Int, ApiKey), Either[ApiError, Apartment]] = + val getApartment: Endpoint[(Int, ApiKey), Either[NotFoundError, Apartment]] = authenticatedEndpoint( request = get(path / "v1" / "data" / "apartments" / segment[Int]("id", Some("The identifier of the apartment to be found"))), - response = withStatusCodes(ok(jsonResponse[Apartment], Some("An apartment found for given id")), Unauthorized, NotFound), + response = notFound.orElse(ok(jsonResponse[Apartment], Some("An apartment found for given id"))), docs = EndpointDocs().withDescription(Some("An endpoint responsible for getting specific apartment by id")) ) - val findApartment: Endpoint[(Address, ApiKey), Either[ApiError, Apartment]] = + val findApartment: Endpoint[(Address, ApiKey), Either[Either[BadRequestError, NotFoundError], Apartment]] = authenticatedEndpoint( request = get(path / "v1" / "data" / "apartments" / "search" /? addressQueryString), - response = withStatusCodes(ok(jsonResponse[Apartment], Some("An apartment found for query string parameters")), - Unauthorized, - BadRequest, - NotFound), + response = badRequest.orElse(notFound).orElse(ok(jsonResponse[Apartment], Some("An apartment found for query string parameters"))), docs = EndpointDocs().withDescription(Some("An endpoint responsible for finding specific apartment by search predicates")) ) - val addApartment: Endpoint[(Apartment, ApiKey), Either[ApiError, Apartment]] = + val addApartment: Endpoint[(Apartment, ApiKey), Either[BadRequestError, Apartment]] = authenticatedEndpoint( request = post(path / "v1" / "data" / "apartments", jsonRequest[Apartment], Some("An apartment to be added in the storage")), - response = withStatusCodes(ok(jsonResponse[Apartment], Some("An apartment saved in the storage")), Unauthorized, BadRequest), + response = badRequest.orElse(ok(jsonResponse[Apartment], Some("An apartment saved in the storage"))), docs = EndpointDocs().withDescription(Some("An endpoint responsible for adding new apartment")) ) - val deleteApartment: Endpoint[(Int, ApiKey), Either[ApiError, Apartment]] = + val deleteApartment: Endpoint[(Int, ApiKey), Either[NotFoundError, Apartment]] = authenticatedEndpoint( request = delete(path / "v1" / "data" / "apartments" / segment[Int]("id", Some("The identifier of the apartment to be deleted"))), - response = withStatusCodes(ok(jsonResponse[Apartment], Some("An apartment deleted")), Unauthorized, NotFound), + response = notFound.orElse(ok(jsonResponse[Apartment], Some("An apartment deleted"))), docs = EndpointDocs().withDescription(Some("An endpoint responsible for deleting apartment by id")) ) diff --git a/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsServer.scala b/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsServer.scala index 1258ce6..b9035bb 100644 --- a/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsServer.scala +++ b/src/main/scala/io/scalac/lab/api/endpoints4s/ApartmentsEndpointsServer.scala @@ -1,24 +1,22 @@ package io.scalac.lab.api.endpoints4s import akka.http.scaladsl.server.Directives._ -import akka.http.scaladsl.server.{Directive1, Route} +import akka.http.scaladsl.server.{Directive1, Route, StandardRoute} import de.heikoseeberger.akkahttpcirce.FailFastCirceSupport import endpoints4s.Tupler -import endpoints4s.akkahttp.server.{BuiltInErrors, Endpoints, JsonEntitiesFromSchemas} -import io.circe.generic.auto._ -import io.scalac.lab.api.model.ApiError +import endpoints4s.akkahttp.server.JsonEntitiesFromSchemas +import endpoints4s.algebra.EndpointsWithCustomErrors +import io.scalac.lab.api.model.ApiError.UnauthorizedError import io.scalac.lab.api.security.Security.ApiKey import io.scalac.lab.api.security.SecurityService -import io.scalac.lab.api.storage.ApartmentsStorage +import io.scalac.lab.api.storage.NarrowerApartmentsStorage import scala.util.Success -class ApartmentsEndpointsServer(storage: ApartmentsStorage, security: SecurityService) +class ApartmentsEndpointsServer(storage: NarrowerApartmentsStorage, security: SecurityService) extends ApartmentsEndpointsDefinition - with Endpoints - with BuiltInErrors - with JsonEntitiesFromSchemas - with FailFastCirceSupport { + with EndpointsWithCustomErrors + with JsonEntitiesFromSchemas { val listApartmentsRoute: Route = listApartments.implementedByAsync { case (paging, _) => storage.list(paging.from, paging.limit) } @@ -33,18 +31,16 @@ class ApartmentsEndpointsServer(storage: ApartmentsStorage, security: SecuritySe val deleteApartmentRoute: Route = deleteApartment.implementedByAsync { case (id, _) => storage.delete(id) } override def authenticatedRequest[A, B](request: Directive1[A])(implicit tupler: Tupler.Aux[A, ApiKey, B]): Directive1[B] = { - optionalHeaderValueByName("api-key").flatMap { authToken: Option[String] => - onComplete(security.authenticate(authToken)).flatMap { - case Success(Right(apiKey)) => request.map(r => tupler(r, apiKey)) - case _ => complete((Unauthorized, authToken.fold("Missing API key")(_ => "Incorrect API key"))) + request.flatMap { entity => + optionalHeaderValueByName("api-key").flatMap { authToken: Option[String] => + onComplete(security.authenticate(authToken)).flatMap { + case Success(Right(apiKey)) => provide(tupler(entity, apiKey)) + case _ => + val error = UnauthorizedError(authToken.fold("Missing API key")(_ => "Incorrect API key")) + StandardRoute(unauthorized(error)) + } } } } - override def withStatusCodes[A](response: A => Route, codes: StatusCode*): Either[ApiError, A] => Route = { - case Left(x @ ApiError.UnauthorizedError(_)) => complete(Unauthorized, x) - case Left(x @ ApiError.NotFoundError(_)) => complete(NotFound, x) - case Left(x @ ApiError.BadRequestError(_)) => complete(BadRequest, x) - case Right(value) => response(value) - } } diff --git a/src/main/scala/io/scalac/lab/api/endpoints4s/CustomStatusCodes.scala b/src/main/scala/io/scalac/lab/api/endpoints4s/CustomStatusCodes.scala index cbe8fbe..f5039aa 100644 --- a/src/main/scala/io/scalac/lab/api/endpoints4s/CustomStatusCodes.scala +++ b/src/main/scala/io/scalac/lab/api/endpoints4s/CustomStatusCodes.scala @@ -1,8 +1,8 @@ package io.scalac.lab.api.endpoints4s -import endpoints4s.algebra.{Endpoints, JsonEntitiesFromSchemas} +import endpoints4s.Invalid +import endpoints4s.algebra.{EndpointsWithCustomErrors, JsonEntitiesFromSchemas} import endpoints4s.generic.JsonSchemas -import io.scalac.lab.api.model.ApiError import io.scalac.lab.api.model.ApiError._ /** @@ -13,14 +13,30 @@ import io.scalac.lab.api.model.ApiError._ * - 401 `Unauthorized` with `UnauthorizedError` as json entity * - 404 `NotFound` with `NotFoundError` as json entity * */ -trait CustomStatusCodes extends Endpoints with JsonEntitiesFromSchemas with JsonSchemas { +trait CustomStatusCodes extends EndpointsWithCustomErrors with JsonEntitiesFromSchemas with JsonSchemas { private implicit val badRequestSchema: JsonSchema[BadRequestError] = genericJsonSchema private implicit val unauthorizedSchema: JsonSchema[UnauthorizedError] = genericJsonSchema private implicit val notFoundSchema: JsonSchema[NotFoundError] = genericJsonSchema - val badRequest: Response[BadRequestError] = - response(BadRequest, jsonResponse[BadRequestError], Some("Api error returned when request could not be processed properly")) + // Tell endpoints4s that client errors are modeled with our custom type, BadRequestError + type ClientErrors = BadRequestError + + // Conversion from endpoints4s internal representation of client errors to our custom type + def invalidToClientErrors(invalid: Invalid): BadRequestError = + BadRequestError(invalid.errors.mkString(". ")) + // Conversion from our custom type to endpoints4s internal representation of client errors + def clientErrorsToInvalid(clientErrors: BadRequestError): Invalid = + Invalid(clientErrors.reason) + + def clientErrorsResponseEntity: ResponseEntity[BadRequestError] = jsonResponse[BadRequestError] + + // Override the documentation of the response for client errors + override lazy val clientErrorsResponse: Response[BadRequestError] = + badRequest(docs = Some("Api error returned when request could not be processed properly")) + + // We define three shorthands for our API responses, `badRequest`, `unauthorized`, and `notFound` + val badRequest: Response[BadRequestError] = clientErrorsResponse val unauthorized: Response[UnauthorizedError] = response(Unauthorized, jsonResponse[UnauthorizedError], Some("Api error returned when request could not be authenticated")) @@ -28,10 +44,12 @@ trait CustomStatusCodes extends Endpoints with JsonEntitiesFromSchemas with Json val notFound: Response[NotFoundError] = response(NotFound, jsonResponse[NotFoundError], Some("Api error returned when entity could not be found")) - /** - * Uses given status codes to handle or create documentation based on that. - * Note that single response may have more than one status code. - **/ - def withStatusCodes[A](response: Response[A], codes: StatusCode*): Response[Either[ApiError, A]] + // We use the same representation as endpoints4s for server errors + type ServerError = Throwable + def throwableToServerError(throwable: Throwable): Throwable = throwable + def serverErrorToThrowable(serverError: Throwable): Throwable = serverError + def serverErrorResponseEntity: ResponseEntity[Throwable] = + jsonResponse(field[String]("reason") + .xmap[Throwable](new Exception(_))(_.toString)) } diff --git a/src/main/scala/io/scalac/lab/api/endpoints4s/QueryStringParams.scala b/src/main/scala/io/scalac/lab/api/endpoints4s/QueryStringParams.scala index cd29bba..3011543 100644 --- a/src/main/scala/io/scalac/lab/api/endpoints4s/QueryStringParams.scala +++ b/src/main/scala/io/scalac/lab/api/endpoints4s/QueryStringParams.scala @@ -1,7 +1,7 @@ package io.scalac.lab.api.endpoints4s import endpoints4s.Validated -import endpoints4s.algebra.Endpoints +import endpoints4s.algebra.{Endpoints, EndpointsWithCustomErrors} import io.scalac.lab.api.model.{Address, Paging} /** @@ -19,7 +19,7 @@ import io.scalac.lab.api.model.{Address, Paging} * - address.street cannot be empty * - address.number cannot be empty and needs to contain at least one digit **/ -trait QueryStringParams extends Endpoints { +trait QueryStringParams extends EndpointsWithCustomErrors { private val pagingFrom = qs[Int]("from", Some("Indicates where we should start returning data from")) private val pagingLimit = qs[Option[Int]]("limit", Some("An optional number of rows to be returned")) diff --git a/src/main/scala/io/scalac/lab/api/endpoints4s/SecuritySupport.scala b/src/main/scala/io/scalac/lab/api/endpoints4s/SecuritySupport.scala index be0b7c7..1eb2992 100644 --- a/src/main/scala/io/scalac/lab/api/endpoints4s/SecuritySupport.scala +++ b/src/main/scala/io/scalac/lab/api/endpoints4s/SecuritySupport.scala @@ -1,10 +1,10 @@ package io.scalac.lab.api.endpoints4s import endpoints4s.Tupler -import endpoints4s.algebra.Endpoints +import endpoints4s.algebra.{Endpoints, EndpointsWithCustomErrors} import io.scalac.lab.api.security.Security.ApiKey -trait SecuritySupport extends Endpoints { +trait SecuritySupport extends EndpointsWithCustomErrors { /** A request which is rejected by the server if it does not contain a valid authentication token */ def authenticatedRequest[I, O](request: Request[I])(implicit tupler: Tupler.Aux[I, ApiKey, O]): Request[O] diff --git a/src/main/scala/io/scalac/lab/api/storage/InMemoryNarrowerApartmentsStorage.scala b/src/main/scala/io/scalac/lab/api/storage/InMemoryNarrowerApartmentsStorage.scala new file mode 100644 index 0000000..c295781 --- /dev/null +++ b/src/main/scala/io/scalac/lab/api/storage/InMemoryNarrowerApartmentsStorage.scala @@ -0,0 +1,52 @@ +package io.scalac.lab.api.storage +import java.util.concurrent.atomic.AtomicLong + +import io.scalac.lab.api.model.Apartment +import io.scalac.lab.api.model.ApiError.{BadRequestError, NotFoundError} + +import scala.concurrent.Future + +// Exactly the same implementation as `InMemoryApartmentsStorage`, except it +// extends `NarrowerApartmentsStorage`. +class InMemoryNarrowerApartmentsStorage(initState: () => List[Apartment] = () => List.empty) extends NarrowerApartmentsStorage { + private val storageId = new AtomicLong() + private var storage: List[Apartment] = initState() + + override def list(from: Int, limit: Option[Int]): Future[Either[BadRequestError, List[Apartment]]] = + Future.successful(Right(storage.slice(from, from + limit.getOrElse(storage.length)))) + + override def get(id: Int): Future[Either[NotFoundError, Apartment]] = + storage.find(_.id.contains(id)) match { + case Some(value) => Future.successful(Right(value)) + case None => Future.successful(Left(NotFoundError(s"Apartment with id: $id not found!"))) + } + + override def find(city: String, street: String, number: String): Future[Either[Either[BadRequestError, NotFoundError], Apartment]] = { + storage.find { s => + s.address.city.equalsIgnoreCase(city) && + s.address.street.equalsIgnoreCase(street) && + s.address.number.equalsIgnoreCase(number) + } match { + case Some(value) => Future.successful(Right(value)) + case None => + Future.successful(Left(Right(NotFoundError(s"Apartment for city: $city, street: $street, number: $number not found!")))) + } + } + + override def save(apartment: Apartment): Future[Either[BadRequestError, Apartment]] = { + val apartmentWithId = apartment.copy(id = Some(storageId.getAndIncrement())) + + storage = storage :+ apartmentWithId + Future.successful(Right(apartmentWithId)) + } + + override def delete(id: Int): Future[Either[NotFoundError, Apartment]] = { + storage.find(_.id.contains(id)) match { + case Some(value) => + storage = storage.filterNot(_.id.contains(id)) + Future.successful(Right(value)) + case None => + Future.successful(Left(NotFoundError(s"Apartment with id: $id not found!"))) + } + } +} diff --git a/src/main/scala/io/scalac/lab/api/storage/NarrowerApartmentsStorage.scala b/src/main/scala/io/scalac/lab/api/storage/NarrowerApartmentsStorage.scala new file mode 100644 index 0000000..664e778 --- /dev/null +++ b/src/main/scala/io/scalac/lab/api/storage/NarrowerApartmentsStorage.scala @@ -0,0 +1,16 @@ +package io.scalac.lab.api.storage + +import io.scalac.lab.api.model.Apartment +import io.scalac.lab.api.model.ApiError.{BadRequestError, NotFoundError} + +import scala.concurrent.Future + +// Similar to `ApartmentsStorage`, but uses precise return +// types instead of `ApiError` everywhere +trait NarrowerApartmentsStorage { + def list(from: Int, limit: Option[Int]): Future[Either[BadRequestError, List[Apartment]]] + def get(id: Int): Future[Either[NotFoundError, Apartment]] + def find(city: String, street: String, number: String): Future[Either[Either[BadRequestError, NotFoundError], Apartment]] + def save(a: Apartment): Future[Either[BadRequestError, Apartment]] + def delete(id: Int): Future[Either[NotFoundError, Apartment]] +}