AbsaOSS / atum-service

Apache License 2.0
5 stars 1 forks source link

Envelope for return types #197

Closed salamonpavel closed 2 months ago

salamonpavel commented 3 months ago

Controllers, Repositories and Services intact, though the below changes have to be introduced.

object ApiResponse {

  sealed trait ApiResponse[T]

  case class SingleApiResponse[T](data: T) extends ApiResponse[T]
  case class MultiApiResponse[T](data: Seq[T]) extends ApiResponse[T]

  implicit class SingleApiResponseEnhancement[T](val data: T) extends AnyVal {
    def toSingleApiResponse: SingleApiResponse[T] = {
      SingleApiResponse(data)
    }
  }

  implicit class MultiApiResponseEnhancement[T](val data: Seq[T]) extends AnyVal {
    def toMultiApiResponse: MultiApiResponse[T] = {
      MultiApiResponse(data)
    }
  }

}

// in the Endpoints trait
protected val createOrUpdateAdditionalDataEndpoint
    : PublicEndpoint[AdditionalDataSubmitDTO, ErrorResponse, SingleApiResponse[AdditionalDataSubmitDTO], Any] = {
    apiV1.post
      .in(CreateOrUpdateAdditionalData)
      .in(jsonBody[AdditionalDataSubmitDTO])
      .out(statusCode(StatusCode.Ok))
//      .out(jsonBody[AdditionalDataSubmitDTO])
      .out(jsonBody[SingleApiResponse[AdditionalDataSubmitDTO]])
  }

// in the Routes trait
private def createAllServerRoutes(httpMonitoringConfig: HttpMonitoringConfig): HttpRoutes[HttpEnv.F] = {
    val metricsInterceptorOption: Option[MetricsRequestInterceptor[HttpEnv.F]] = {
      if (httpMonitoringConfig.enabled) Some(HttpMetrics.prometheusMetrics.metricsInterceptor()) else None
    }
    val endpoints = List(
      createServerEndpoint(createCheckpointEndpoint, CheckpointController.createCheckpoint),
      createServerEndpoint(createPartitioningEndpoint, PartitioningController.createPartitioningIfNotExists),
      // has to be mapped here
      createServerEndpoint(createOrUpdateAdditionalDataEndpoint, PartitioningController.createOrUpdateAdditionalData _ andThen(_.map(_.toSingleApiResponse))),
      createServerEndpoint(healthEndpoint, (_: Unit) => ZIO.unit),
    )
    ZHttp4sServerInterpreter[HttpEnv.Env](http4sServerOptions(metricsInterceptorOption)).from(endpoints).toRoutes
  }

// and additional json serde implicits
implicit def readsSingleApiResponse[T: Reads]: Reads[SingleApiResponse[T]] = Json.reads[SingleApiResponse[T]]
implicit def writesSingleApiResponse[T: Writes]: Writes[SingleApiResponse[T]] = Json.writes[SingleApiResponse[T]]
benedeki commented 3 months ago

Controllers, Repositories and Services intact, though the below changes have to be introduced.

// and additional json serde implicits
implicit def readsSingleApiResponse[T: Reads]: Reads[SingleApiResponse[T]] = Json.reads[SingleApiResponse[T]]
implicit def writesSingleApiResponse[T: Writes]: Writes[SingleApiResponse[T]] = Json.writes[SingleApiResponse[T]]

This is done only once, right? (Well, twice, also for MultiApiResponse)? Not for every T,,,

salamonpavel commented 3 months ago

Controllers, Repositories and Services intact, though the below changes have to be introduced.

// and additional json serde implicits
implicit def readsSingleApiResponse[T: Reads]: Reads[SingleApiResponse[T]] = Json.reads[SingleApiResponse[T]]
implicit def writesSingleApiResponse[T: Writes]: Writes[SingleApiResponse[T]] = Json.writes[SingleApiResponse[T]]

This is done only once, right? (Well, twice, also for MultiApiResponse)? Not for every T,,,

Yes, it's a method which generates Reads/Writes for SingleApiResponse[T] given a presence of Reads/Writes for type T. At some point these Reads/Writes will become obsolete, once we introduce Circe.

benedeki commented 3 months ago

I think it looks good, and simple enough. We can definitely go with this one.

But my original idea was even more ambitious (not sure if - easily - doable). What if having something like

class EnvelopedSingleApiEndpoint[] extends PublicEndpoint[] // that would would the conversion automatically

Perhaps with some more classes extended to be used that covers the envelope creation. In other words, so that the envelope doesn't have to be added, just the proper classes used/inherited.

salamonpavel commented 3 months ago

@benedeki I don't think it can be achieved by extending PublicEndpoint. The Tapir endpoints are merely descriptions of endpoints. We need to map the server logic. Instead of mapping individual ZIOs we could enhance the server logic.

object RoutesEnhancements {
  implicit class SingleZServerLogicEnhanced[I, E, O](val logic: I => ZIO[HttpEnv.Env, E, O]) extends AnyVal {
    def singleApiEnvelope: I => ZIO[HttpEnv.Env, E, SingleApiResponse[O]] = {
      logic.andThen(_.map(SingleApiResponse(_)))
    }
  }

  implicit class MultiZServerLogicEnhanced[I, E, O](val logic: I => ZIO[HttpEnv.Env, E, Seq[O]]) extends AnyVal {
    def multiApiEnvelope: I => ZIO[HttpEnv.Env, E, MultiApiResponse[O]] = {
      logic.andThen(_.map(MultiApiResponse(_)))
    }
  }
}

private def createAllServerRoutes(httpMonitoringConfig: HttpMonitoringConfig): HttpRoutes[HttpEnv.F] = {
    val metricsInterceptorOption: Option[MetricsRequestInterceptor[HttpEnv.F]] = {
      if (httpMonitoringConfig.enabled) Some(HttpMetrics.prometheusMetrics.metricsInterceptor()) else None
    }
    val endpoints = List(
      createServerEndpoint(createCheckpointEndpoint, CheckpointController.createCheckpoint),
      createServerEndpoint(createPartitioningEndpoint, PartitioningController.createPartitioningIfNotExists),
      // used here
      createServerEndpoint(createOrUpdateAdditionalDataEndpoint, (in => PartitioningController.createOrUpdateAdditionalData(in)).singleApiEnvelope),
      createServerEndpoint(healthEndpoint, (_: Unit) => ZIO.unit),
    )
    ZHttp4sServerInterpreter[HttpEnv.Env](http4sServerOptions(metricsInterceptorOption)).from(endpoints).toRoutes
  }

Or we could have simple methods and stay away of implicits altogether.

private def mapToSingleApiEnvelope[E, O](effect: ZIO[HttpEnv.Env, E, O]): ZIO[HttpEnv.Env, E, SingleApiResponse[O]] = {
    effect.map(SingleApiResponse(_))
}

    createServerEndpoint(createOrUpdateAdditionalDataEndpoint, (in: AdditionalDataSubmitDTO) => mapToSingleApiEnvelope(PartitioningController.createOrUpdateAdditionalData(in)))

We could also have two methods on BaseController. Probably the cleanest solution in my opinion.