finagle (fix): Properly set http method and path parameters in conver…

…tToFinagleRequest (#3179) ## Overview - Currently, `convertToFinagleRequest` doesn't respect `Method`, `request path` and etc --------- Co-authored-by: Taro L. Saito <[email protected]>
wvlet · Aug 31, 2023 · 7033637 · 7033637
1 parent 6eae024
commit 7033637
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 3 deletions.
diff --git a/airframe-http-finagle/src/main/scala/wvlet/airframe/http/finagle/package.scala b/airframe-http-finagle/src/main/scala/wvlet/airframe/http/finagle/package.scala
@@ -179,7 +179,12 @@ package object finagle {
   }
 
   def convertToFinagleRequest(request: HttpMessage.Request): Request = {
-    val req = http.Request()
+    val req = if (request.method == "GET") {
+      val params = request.query.toSeq.map(e => (e.key, e.value))
+      http.Request(s"${request.path}", params: _*)
+    } else {
+      http.Request(http.Method(request.method), s"${request.path}")
+    }
     for (h <- request.header.entries) {
       req.headerMap.add(h.key, h.value)
     }

diff --git a/airframe-http-finagle/src/test/scala/wvlet/airframe/http/finagle/FinagleTest.scala b/airframe-http-finagle/src/test/scala/wvlet/airframe/http/finagle/FinagleTest.scala
@@ -14,10 +14,9 @@
 package wvlet.airframe.http.finagle
 
 import java.nio.charset.StandardCharsets
-
 import com.twitter.finagle.http
 import com.twitter.finagle.http.Status
-import wvlet.airframe.http.{HttpMultiMap, HttpStatus}
+import wvlet.airframe.http.{HttpMessage, HttpMethod, HttpMultiMap, HttpStatus}
 import wvlet.airspec.AirSpec
 
 /**
@@ -64,4 +63,40 @@ class FinagleTest extends AirSpec {
     r.contentBytes shouldBe "hello world".getBytes(StandardCharsets.UTF_8)
     resp.toRaw shouldBeTheSameInstanceAs resp
   }
+
+  test("convertToFinagleRequest test") {
+    test("basic") {
+      val req = HttpMessage.Request(HttpMethod.GET, "/foo")
+
+      val response = convertToFinagleRequest(req)
+
+      response.method.name shouldBe "GET"
+      response.path shouldBe "/foo"
+    }
+    test("GET with query parameters") {
+      val req = HttpMessage.Request(HttpMethod.GET, "/foo?bar=true&baz=1234")
+
+      val response = convertToFinagleRequest(req)
+
+      response.method.name shouldBe "GET"
+      response.path shouldBe "/foo"
+      response.params.contains("bar") shouldBe true
+      response.params.contains("baz") shouldBe true
+      response.params.getBoolean("bar") shouldBe Some(true)
+      response.params.getInt("baz") shouldBe Some(1234)
+    }
+    test("POST with request body") {
+      val req = HttpMessage
+        .Request(HttpMethod.POST, "/json")
+        .withContentTypeJson
+        .withContent("""{"bar": "123456"}""")
+
+      val response = convertToFinagleRequest(req)
+
+      response.method.name shouldBe "POST"
+      response.path shouldBe "/json"
+      response.headerMap.get("Content-Type") shouldBe Option("application/json;charset=utf-8")
+      response.contentString shouldBe """{"bar": "123456"}"""
+    }
+  }
 }