Repository: scala-steward-org/scala-steward
Pull request #2910 (2023-01-10 18:17:48)

HttpJsonClient: unify client calls
This unifies the client calls in `HttpJsonClient`. There is now only one method that calls the underlying client that works for all other methods. The nice thing about this is that we only need to handle decoding failures and unexpected responses in one place.

codecov[bot]: # [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/2910?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org) Report
Base: **89.69**% // Head: **89.73**% // Increases project coverage by **`+0.03%`** :tada:
> Coverage data is based on head [(`3aa9f12`)](https://codecov.io/gh/scala-steward-org/scala-steward/pull/2910?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org) compared to base [(`0c67b21`)](https://codecov.io/gh/scala-steward-org/scala-steward/commit/0c67b217a2be800d1dd77f39c9eabb226144353a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org).
> Patch coverage: 93.75% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2910      +/-   ##
==========================================
+ Coverage   89.69%   89.73%   +0.03%     
==========================================
  Files         158      158              
  Lines        3038     3038              
  Branches      200      204       +4     
==========================================
+ Hits         2725     2726       +1     
+ Misses        313      312       -1     
```


| [Impacted Files](https://codecov.io/gh/scala-steward-org/scala-steward/pull/2910?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org) | Coverage Δ | |
|---|---|---|
| [...la/org/scalasteward/core/util/HttpJsonClient.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/2910?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS91dGlsL0h0dHBKc29uQ2xpZW50LnNjYWxh) | `90.32% <93.75%> (+3.22%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/2910?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scala-steward-org).


Patch:
diff --git a/README.md b/README.md
index 42b351b5..12dbe6f1 100644
--- a/README.md
+++ b/README.md
@@ -70,6 +70,7 @@ Thanks goes to these wonderful people for contributing to Scala Steward:
 * [Anton Sviridov](https://github.com/keynmol)
 * [Antonio Gelameris](https://github.com/TonioGela)
 * [Arjun Dhawan](https://github.com/arjun-1)
+* [Arman Bilge](https://github.com/armanbilge)
 * [Arnold Farkas](https://github.com/sapka12)
 * [Arulselvan Madhavan](https://github.com/ArulselvanMadhavan)
 * [Barry O'Neill](https://github.com/barryoneill)
diff --git a/build.sbt b/build.sbt
index 7391a636..1f3b1b84 100644
--- a/build.sbt
+++ b/build.sbt
@@ -137,7 +137,7 @@ lazy val core = myCrossProject("core")
       Dependencies.circeLiteral % Test,
       Dependencies.disciplineMunit % Test,
       Dependencies.http4sDsl % Test,
-      Dependencies.http4sBlazeServer % Test,
+      Dependencies.http4sEmberServer % Test,
       Dependencies.munit % Test,
       Dependencies.munitCatsEffect % Test,
       Dependencies.munitScalacheck % Test,
diff --git a/modules/core/src/main/scala/org/scalasteward/core/util/HttpJsonClient.scala b/modules/core/src/main/scala/org/scalasteward/core/util/HttpJsonClient.scala
index fe12844a..cc708237 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/util/HttpJsonClient.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/util/HttpJsonClient.scala
@@ -24,7 +24,7 @@ import org.http4s.Status.Successful
 import org.http4s._
 import org.http4s.circe.{jsonEncoderOf, jsonOf}
 import org.http4s.client.Client
-import org.http4s.headers.Link
+import org.http4s.headers.{Accept, Link, MediaRangeAndQValue}
 import scala.util.control.NoStackTrace
 
 final class HttpJsonClient[F[_]](implicit
@@ -39,22 +39,17 @@ final class HttpJsonClient[F[_]](implicit
   def getAll[A: Decoder](uri: Uri, modify: ModReq): F[List[A]] =
     all[A](GET, uri, modify, Nil)
 
-  private[this] def all[A: Decoder](
+  private def all[A: Decoder](
       method: Method,
       uri: Uri,
       modify: ModReq,
       xs: List[A]
   ): F[List[A]] =
-    requestWithHeader[A, Link](method, uri, modify).flatMap {
-      case (res, Some(linkHeader)) =>
-        linkHeader.values.find(_.rel === Option("next")) match {
-          case Some(link) =>
-            all(method, link.uri, modify, res :: xs)
-          case None =>
-            (res :: xs).pure[F]
-        }
-      case (res, None) =>
-        (res :: xs).pure[F]
+    requestWithHeaders[A](method, uri, modify)(jsonOf).flatMap { case (a, headers) =>
+      headers.get[Link].flatMap(_.values.find(_.rel.contains("next"))) match {
+        case Some(linkValue) => all(method, linkValue.uri, modify, a :: xs)
+        case None            => F.pure(a :: xs)
+      }
     }
 
   def post[A: Decoder](uri: Uri, modify: ModReq): F[A] =
@@ -78,40 +73,39 @@ final class HttpJsonClient[F[_]](implicit
   def patchWithBody[A: Decoder, B: Encoder](uri: Uri, body: B, modify: ModReq): F[A] =
     patch[A](uri, modify.compose(_.withEntity(body)(jsonEncoderOf[B])))
 
-  private[this] def requestWithHeader[A, H](
-      method: Method,
-      uri: Uri,
-      modify: ModReq
-  )(implicit d: Decoder[A], hs: Header.Select[H]): F[(A, Option[hs.F[H]])] =
-    modify(Request[F](method, uri))
-      .flatMap(client.run(_).use {
-        case Successful(resp) =>
-          val decoder = ourJsonOf[A](uri, method)
-          decoder.decode(resp, strict = false).rethrowT.map(_ -> resp.headers.get[H])
-        case resp =>
-          toUnexpectedResponse(uri, method, resp).flatMap(_.raiseError)
-      })
-
   private def request[A: Decoder](method: Method, uri: Uri, modify: ModReq): F[A] =
-    client.expectOr[A](modify(Request[F](method, uri)))(resp =>
-      toUnexpectedResponse(uri, method, resp)
-    )(ourJsonOf(uri, method))
+    requestWithHeaders[A](method, uri, modify)(jsonOf).map { case (a, _) => a }
 
   private def request_(method: Method, uri: Uri, modify: ModReq): F[Unit] =
-    client.expectOr[Unit](modify(Request[F](method, uri)))(resp =>
-      toUnexpectedResponse(uri, method, resp)
-    )
+    requestWithHeaders[Unit](method, uri, modify).void
+
+  // adapted from https://github.com/http4s/http4s/blob/c89ebc2d844c5c93dcc1307e5b9361a2c38bfd00/client/shared/src/main/scala/org/http4s/client/DefaultClient.scala#L91-L105
+  private def requestWithHeaders[A](method: Method, uri: Uri, modify: ModReq)(implicit
+      d: EntityDecoder[F, A]
+  ): F[(A, Headers)] =
+    modify(Request[F](method, uri)).flatMap { req =>
+      val r = Nel
+        .fromList(d.consumes.toList.map(MediaRangeAndQValue(_)))
+        .fold(req)(m => req.addHeader(Accept(m)))
+
+      client.run(r).use {
+        case Successful(resp) =>
+          d.decode(resp, strict = false).value.flatMap {
+            case Right(a) => F.pure((a, resp.headers))
+            case Left(failure) =>
+              handleFailure(resp)(DecodeFailureWithContext(uri, method, _, Some(failure)))
+          }
+        case resp =>
+          handleFailure(resp)(UnexpectedResponse(uri, method, resp.headers, resp.status, _))
+      }
+    }
 
-  private def ourJsonOf[A](uri: Uri, method: Method)(implicit d: Decoder[A]): EntityDecoder[F, A] =
-    jsonOf[F, A].transform(_.leftMap(failure => DecodeFailureWithContext(uri, method, failure)))
+  private def handleFailure[A](resp: Response[F])(fromBody: String => Throwable): F[A] =
+    bodyToString(resp.body).map(fromBody).flatMap(F.raiseError[A])
 
-  private def toUnexpectedResponse(
-      uri: Uri,
-      method: Method,
-      response: Response[F]
-  ): F[Throwable] = {
-    val body = response.body.through(fs2.text.utf8.decode).compile.string
-    body.map(UnexpectedResponse(uri, method, response.headers, response.status, _))
+  private def bodyToString(body: EntityBody[F]): F[String] = {
+    val s = body.through(fs2.text.utf8.decode).compile.string
+    s.handleError(t => s"<body not available (${t.getMessage})>")
   }
 }
 
@@ -119,18 +113,18 @@ final class HttpJsonClient[F[_]](implicit
 final case class DecodeFailureWithContext(
     uri: Uri,
     method: Method,
-    underlying: DecodeFailure
-) extends DecodeFailure {
+    body: String,
+    cause: Some[DecodeFailure]
+) extends DecodeFailure
+    with NoStackTrace {
   override val message: String =
     s"""|uri: $uri
         |method: $method
-        |message: ${underlying.message}""".stripMargin
-
-  override def cause: Option[Throwable] =
-    underlying.some
+        |message: ${cause.value.message}
+        |body: $body""".stripMargin
 
   override def toHttpResponse[F[_]](httpVersion: HttpVersion): Response[F] =
-    underlying.toHttpResponse(httpVersion)
+    cause.value.toHttpResponse(httpVersion)
 }
 
 /** Like `org.http4s.client.UnexpectedStatus` but contains additional context to ease debugging. */
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index 6c319ad8..5fb0bb48 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -24,7 +24,7 @@ object Dependencies {
   val http4sCirce = "org.http4s" %% "http4s-circe" % http4sCore.revision
   val http4sClient = "org.http4s" %% "http4s-client" % http4sCore.revision
   val http4sDsl = "org.http4s" %% "http4s-dsl" % http4sCore.revision
-  val http4sBlazeServer = "org.http4s" %% "http4s-blaze-server" % "1.0.0-M38"
+  val http4sEmberServer = "org.http4s" %% "http4s-ember-server" % http4sCore.revision
   val http4sJdkhttpClient = "org.http4s" %% "http4s-jdk-http-client" % "1.0.0-M8"
   val log4catsSlf4j = "org.typelevel" %% "log4cats-slf4j" % "2.5.0"
   val logbackClassic = "ch.qos.logback" % "logback-classic" % "1.4.5"


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala b/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala
index 93183ff6..94abc080 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala
@@ -11,6 +11,8 @@ import org.typelevel.ci._
 import eu.timepit.refined.auto._
 import eu.timepit.refined.types.numeric.PosInt
 
+import scala.concurrent.duration._
+
 class ClientConfigurationTest extends CatsEffectSuite {
 
   private val userAgentValue = "my-user-agent"
@@ -71,7 +73,7 @@ class ClientConfigurationTest extends CatsEffectSuite {
 
   test("disableFollowRedirect does not follow redirect") {
     import org.http4s.Method._
-    import org.http4s.blaze.server._
+    import org.http4s.ember.server._
     import org.http4s.client.dsl.io._
 
     val regularClient = ClientConfiguration.build[IO](
@@ -83,7 +85,11 @@ class ClientConfigurationTest extends CatsEffectSuite {
       ClientConfiguration.setUserAgent(dummyUserAgent)
     )
     val getServer =
-      BlazeServerBuilder[IO].bindAny("localhost").withHttpApp(routes.orNotFound).resource
+      EmberServerBuilder
+        .default[IO]
+        .withShutdownTimeout(1.second)
+        .withHttpApp(routes.orNotFound)
+        .build
 
     val test = (regularClient, disabledClient, getServer).tupled.use {
       case (regClient, disClient, s) =>
diff --git a/modules/core/src/test/scala/org/scalasteward/core/util/HttpJsonClientTest.scala b/modules/core/src/test/scala/org/scalasteward/core/util/HttpJsonClientTest.scala
index 74cd4169..3b475f58 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/util/HttpJsonClientTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/util/HttpJsonClientTest.scala
@@ -2,10 +2,10 @@ package org.scalasteward.core.util
 
 import cats.syntax.all._
 import munit.CatsEffectSuite
-import org.http4s.HttpApp
 import org.http4s.dsl.Http4sDsl
 import org.http4s.headers.{Link, LinkValue}
 import org.http4s.syntax.all._
+import org.http4s.{HttpApp, HttpVersion, Status}
 import org.scalasteward.core.mock.MockContext.context._
 import org.scalasteward.core.mock.{MockEff, MockState}
 
@@ -34,7 +34,8 @@ class HttpJsonClientTest extends CatsEffectSuite with Http4sDsl[MockEff] {
       .map(_.leftMap(_.getMessage))
     val expected = Left("""uri: https://example.org
                           |method: GET
-                          |message: Malformed message body: Invalid JSON""".stripMargin)
+                          |message: Malformed message body: Invalid JSON
+                          |body:  "1 """.stripMargin)
     assertIO(obtained, expected)
   }
 
@@ -46,11 +47,9 @@ class HttpJsonClientTest extends CatsEffectSuite with Http4sDsl[MockEff] {
     val obtained = httpJsonClient
       .get[String](uri"https://example.org", _.pure[MockEff])
       .runA(state)
-      .attempt
-      .map(_.leftMap(_.getMessage))
-    val expected = Left("""uri: https://example.org
-                          |method: GET
-                          |message: Invalid message body: Could not decode JSON: 1""".stripMargin)
+      .attemptNarrow[DecodeFailureWithContext]
+      .map(_.leftMap(_.toHttpResponse(HttpVersion.`HTTP/1.1`).status))
+    val expected = Left(Status.UnprocessableEntity)
     assertIO(obtained, expected)
   }
 }