Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Envelope for return types #197

Closed
salamonpavel opened this issue May 17, 2024 · 4 comments · Fixed by #199
Closed

Envelope for return types #197

salamonpavel opened this issue May 17, 2024 · 4 comments · Fixed by #199
Assignees
@salamonpavel
Labels
enhancement New feature or request

Comments

@salamonpavel
Copy link
Collaborator

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]]
@salamonpavel salamonpavel added the enhancement New feature or request label May 17, 2024
@benedeki
Copy link
Contributor

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
Copy link
Collaborator Author

salamonpavel commented May 20, 2024 •
edited

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
Copy link
Contributor

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
Copy link
Collaborator Author

salamonpavel commented May 21, 2024 •
edited

@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.

@salamonpavel salamonpavel mentioned this issue May 22, 2024
Merged
@salamonpavel salamonpavel self-assigned this May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@salamonpavel salamonpavel

Labels
enhancement New feature or request
Projects
CPS small repos project
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

response envelope AbsaOSS/atum-service
2 participants
@benedeki @salamonpavel