Skip to content

Draft: zio-http server interpreter#1150

Closed
Fristi wants to merge 3 commits intosoftwaremill:masterfrom
vectos:zhttp
Closed

Draft: zio-http server interpreter#1150
Fristi wants to merge 3 commits intosoftwaremill:masterfrom
vectos:zhttp

Conversation

@Fristi
Copy link
Copy Markdown

@Fristi Fristi commented Apr 9, 2021

This is an initial attempt, but has a lot of todo's plus:

This is first sketch, I'm not an expert on websockets and ztapir. If someone can jump on the latter on a follow up PR, that would be great.

Comment thread server/zio-http/src/main/scala/sttp/tapir/server/zhttp/ZHttpInterpreter.scala Outdated
case RawBodyType.ByteArrayBody =>
request.data.content match {
case HttpContent.Complete(data) => Task.succeed(data.getBytes())
case HttpContent.Chunked(data) => data.fold("")(_ + _).map(_.getBytes)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't go through a String to get the bytes - as conversions (due to charsets etc) might malform the data. Maybe there's a way to concatenate byte chunks?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is wrong. Will correct once we have a Stream of bytes :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

case HttpContent.Complete(data) => Task.succeed(data.getBytes())
case HttpContent.Chunked(data) => data.fold("")(_ + _).map(_.getBytes)
}
case RawBodyType.ByteBufferBody => ???
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this & the next can be can typically implemented the same as getting a byte array

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need HttpContent to support a stream of bytes

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

case HttpContent.Chunked(data) => data.transduce(toBytes(StandardCharsets.UTF_8))
}

def toStream(): streams.BinaryStream = stream.asInstanceOf[streams.BinaryStream] //TODO: this is weird we need a aggressive type cast
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe if you type streams as : ZioStreams type inference will work? It's sometimes tricky with path-dependent types

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that, but didn't work

private def sttpToZHttpHeader(header: SttpHeader): ZHttpHeader =
ZHttpHeader(header.name, header.value)

def convert[I, O, R](route: Endpoint[I, Throwable, O, ZioStreams])(logic: I => RIO[R, O]): Http[R, Throwable] = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can call toHttp? that would follow the convention in other interpreters

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, also planning to support multiple endpoints in one go.

@adamw
Copy link
Copy Markdown
Member

adamw commented Apr 12, 2021

Thanks, looks promising! :)

@ubourdon
Copy link
Copy Markdown

This should work with ZServerEndpoint no ? from "com.softwaremill.sttp.tapir" %% "tapir-zio" becasue there re lots of useful things in this package.

@Fristi
Copy link
Copy Markdown
Author

Fristi commented Apr 12, 2021

Could, I'll leave that up for later. Focus is first to have a module.

I think the biggest thing to fix is that HttpContent from zio-http actual works with a ZStream[Any, Throwable, Byte] and also does the streaming. I've looked around a little bit, but not sure how the authors of zio-http want to implement this. I would suggest to draw inspiration from http4s-netty or reactive-streams-netty as I have a feeling it might be complex.

@Fristi Fristi mentioned this pull request Jun 25, 2021
@Fristi Fristi closed this Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants