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.

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

airframe-okhttp: Scala3 support prep. Remove Finagle from test dependency #2670

Merged
merged 3 commits into from
Dec 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class NetthRequestHandler(config: NettyServerConfig, dispatcher: NettyBackend.Fi

ctx.channel().remoteAddress() match {
case x: InetSocketAddress =>
req = req.withRemoteAddress(ServerAddress(s"${x.getAddress}:${x.getPort}"))
// TODO This address might be IPv6
req = req.withRemoteAddress(ServerAddress(s"${x.getHostString}:${x.getPort}"))
case _ =>
}

Expand All @@ -78,7 +79,6 @@ class NetthRequestHandler(config: NettyServerConfig, dispatcher: NettyBackend.Fi
val nettyResponse = toNettyResponse(v.asInstanceOf[Response])
writeResponse(msg, ctx, nettyResponse)
case OnError(ex) =>
warn(ex)
val resp = new DefaultHttpResponse(
HttpVersion.HTTP_1_1,
HttpResponseStatus.valueOf(HttpStatus.InternalServerError_500.code)
Expand All @@ -89,7 +89,7 @@ class NetthRequestHandler(config: NettyServerConfig, dispatcher: NettyBackend.Fi
}

private def writeResponse(req: HttpRequest, ctx: ChannelHandlerContext, resp: DefaultHttpResponse): Unit = {
val keepAlive = HttpUtil.isKeepAlive(req)
val keepAlive = HttpStatus.ofCode(resp.status().code()).isSuccessful && HttpUtil.isKeepAlive(req)
if (keepAlive) {
if (!req.protocolVersion().isKeepAliveDefault) {
resp.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
package wvlet.airframe.http.okhttp

import java.time.Duration

import wvlet.airframe.control.Control.withResource
import wvlet.airframe.http.HttpMessage.{Request, Response}
import wvlet.airframe.http.netty.{Netty, NettyServer}
import wvlet.airframe.http.{HttpClientException, HttpClientMaxRetryException, HttpStatus, Router}
import wvlet.airframe.http.finagle.{FinagleServer, FinagleServerConfig, newFinagleServerDesign}
import wvlet.airspec.AirSpec
import wvlet.log.LogSupport

import java.time.Duration

case class User(id: Int, name: String, requestId: String) {
def withRequestId(newRequestId: String): User = User(id, name, newRequestId)
}

case class UserRequest(id: Int, name: String)
case class DeleteRequestBody(force: Boolean)

trait FinagleClientTestApi extends LogSupport {
import com.twitter.finagle.http.{Request, Response, Status}
trait NettyTestApi extends LogSupport {
import wvlet.airframe.http.{Endpoint, HttpMethod}

@Endpoint(method = HttpMethod.GET, path = "/")
Expand All @@ -25,7 +25,7 @@ trait FinagleClientTestApi extends LogSupport {
}

private def getRequestId(request: Request): String = {
request.headerMap.getOrElse("X-Request-Id", "N/A")
request.header.getOrElse("X-Request-Id", "N/A")
}

@Endpoint(method = HttpMethod.GET, path = "/user/:id")
Expand Down Expand Up @@ -71,12 +71,12 @@ trait FinagleClientTestApi extends LogSupport {
@Endpoint(method = HttpMethod.GET, path = "/busy")
def busy: Response = {
trace("called busy method")
Response(Status.InternalServerError)
Response(HttpStatus.InternalServerError_500)
}

@Endpoint(method = HttpMethod.GET, path = "/forbidden")
def forbidden: Response = {
Response(Status.Forbidden)
Response(HttpStatus.Forbidden_403)
}

@Endpoint(method = HttpMethod.GET, path = "/readtimeout")
Expand All @@ -87,19 +87,19 @@ trait FinagleClientTestApi extends LogSupport {

@Endpoint(method = HttpMethod.GET, path = "/response")
def rawResponse: Response = {
val r = Response(Status.Ok)
r.setContentString("raw response")
r
val r = Response(HttpStatus.Ok_200)
r.withContent("raw response")
}
}

class OkHttpClientTest extends AirSpec {
val r = Router.add[FinagleClientTestApi]
private val r = Router.add[NettyTestApi]

override protected def design =
newFinagleServerDesign(FinagleServerConfig(name = "test-server", router = r))
override protected def design = {
Netty.server.withRouter(r).designWithSyncClient
}

test("create client") { (server: FinagleServer) =>
test("create client") { (server: NettyServer) =>
def addRequestId(request: okhttp3.Request.Builder): okhttp3.Request.Builder = {
request.addHeader("X-Request-Id", "10")
}
Expand Down Expand Up @@ -183,7 +183,7 @@ class OkHttpClientTest extends AirSpec {
}
}

test("fail request") { (server: FinagleServer) =>
test("fail request") { (server: NettyServer) =>
withResource(
OkHttpClient.newClient(
// Test for the full URI
Expand Down Expand Up @@ -216,7 +216,7 @@ class OkHttpClientTest extends AirSpec {
}
}

test("read timeout") { (server: FinagleServer) =>
test("read timeout") { (server: NettyServer) =>
withResource(
OkHttpClient.newClient(
s"http://${server.localAddress}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ object ServerAddress extends LogSupport {
}
ServerAddress(uri.getHost, port, scheme)
} else {
val pos = address.indexOf(":")
// Take the last section separated by colon because the address might be IPv6
val pos = address.lastIndexOf(":")
if (pos > 0) {
val port = address.substring(pos + 1, address.length).toInt
val scheme = port match {
Expand Down
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ lazy val okhttp =
"com.squareup.okhttp3" % "okhttp" % "4.10.0"
)
)
.dependsOn(http.jvm, finagle % Test)
.dependsOn(http.jvm, netty % Test)

lazy val httpRecorder =
project
Expand Down