Conversation
ghost
requested a review
from 
ghost
self-assigned this
| "dev.zio" %% "zio-interop-cats" % Versions.zioInteropCats, | ||
| "io.d11" %% "zhttp" % "1.0.0.0-RC17") | ||
| ) | ||
| .jvmPlatform(scalaVersions = scala2Versions) |
There was a problem hiding this comment.
maybe scala 3 would work as well?
| lazy val zioHttp: ProjectMatrix = (projectMatrix in file("server/zio-http")) | ||
| .settings(commonJvmSettings) | ||
| .settings( | ||
| name := "tapir-zio-http4", |
There was a problem hiding this comment.
I think the name should be without 4 :)
| @@ -131,6 +131,7 @@ lazy val allAggregates = core.projectRefs ++ | |||
| playServer.projectRefs ++ | |||
| vertxServer.projectRefs ++ | |||
| zioServer.projectRefs ++ | |||
There was a problem hiding this comment.
the zioServer name is misleading now. Maybe we could rename it to zioHttp4sServer?
| route: Endpoint[I, Throwable, O, ZioStreams] | ||
| )(logic: I => RIO[R, O]): Http[R, Throwable, Request, Response[R, Throwable]] = { | ||
| Http.fromEffectFunction[Request] { req => | ||
| implicit val interpret: ZHttpBodyListener[R] = new ZHttpBodyListener[R] |
| } | ||
| } | ||
|
|
||
| private[zhttp] def zioMonadError[R]: MonadError[RIO[R, *]] = new MonadError[RIO[R, *]] { |
There was a problem hiding this comment.
there's already one in sttp.tapir.server.http4s.ztapir. Maybe we could move it to the integrations/zio module and use the same implementation in both places?
|
|
||
| def asByteArray: Task[Array[Byte]] = request.content match { | ||
| case HttpData.Empty => Task.succeed(Array.emptyByteArray) | ||
| case HttpData.CompleteData(data) => Task(data.toArray) |
There was a problem hiding this comment.
This should probably be Task.succeed as well, as we have a strict value?
|
Looks good so far :) Now tests ;) One question - is it |
|
From what I see project name is |
|
I see my commits are included, should I close #1150 if you like to follow up on my work ? |
|
@bartekzylinski let's use zio-http consistently then |
| case RawBodyType.ByteArrayBody => asByteArray.map(RawValue(_)) | ||
| case RawBodyType.ByteBufferBody => asByteArray.map(bytes => ByteBuffer.wrap(bytes)).map(RawValue(_)) | ||
| case RawBodyType.InputStreamBody => asByteArray.map(new ByteArrayInputStream(_)).map(RawValue(_)) | ||
| case RawBodyType.FileBody => Task.effect(RawValue(Defaults.createTempFile())) |
There was a problem hiding this comment.
Here we should return the created file as part of RawValue so that in case of an error, the file can be cleaned up
|
|
||
| implicit val m: MonadError[RIO[Blocking, *]] = zioMonadError | ||
|
|
||
| new ServerBasicTests(createServerTest, interpreter, false, true, false, false).tests() ++ |
There was a problem hiding this comment.
[minor] it's good practice to name boolean parameters. I think IntelliJ has an inspection for that as well
|
Thanks - looks good :) One thing that's missing is documentation - similar as for the other interpreters. |
| package sttp.tapir.server.ziohttp | ||
|
|
||
| import cats.data.NonEmptyList | ||
| import cats.effect.{Async, IO, Resource} |
There was a problem hiding this comment.
Didn't know a ZIO module needs Cats :)
| } | ||
|
|
||
| override def server(routes: NonEmptyList[Http[Blocking, Throwable, Request, Response[Blocking, Throwable]]]): Resource[IO, Port] = { | ||
| val as: Async[IO] = Async[IO] |
There was a problem hiding this comment.
Any particular reason we need Cats' Async here?
There was a problem hiding this comment.
I guess it's just a test module but I'm just curious.
There was a problem hiding this comment.
The "test framework" for testing server interpreters is written using cats-effect, hence all of the interpreters use it (be it akka, cats, serverless, vertx, play or zio). This doesn't influence the implementation in any way, though.
# Conflicts: # server/zio-http/src/test/scala/sttp/tapir/server/ziohttp/ZioHttpServerTest.scala # server/zio-http/src/test/scala/sttp/tapir/server/ziohttp/ZioHttpTestServerInterpreter.scala
4 participants
No description provided.