From 33ca4c9cdcca325de5c4dad52243d3cff6379172 Mon Sep 17 00:00:00 2001 From: Yannick Heiber Date: Sat, 3 Feb 2024 13:11:52 +0100 Subject: [PATCH 01/10] Add Resource-based simple constructors As Java 21+ allows explicitly closing clients, this adds new constructors that return the clients wrapped in a Resource. On older Java versions, the release is a no-op, on newer it enforces the cleanup of resources. Also ensures the websocket client closes the input channel on release so the connection gets properly closed. Closes #1011 --- .github/workflows/ci.yml | 58 ++++++++++++++++++- .mergify.yml | 1 + build.sbt | 6 +- .../http4s/jdkhttpclient/JdkHttpClient.scala | 18 ++++++ .../http4s/jdkhttpclient/JdkWSClient.scala | 15 +++++ .../jdkhttpclient/JdkHttpClientSpec.scala | 2 +- .../jdkhttpclient/JdkWSClientSpec.scala | 2 +- 7 files changed, 96 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73499232..ead70587 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,12 +30,16 @@ jobs: matrix: os: [ubuntu-latest] scala: [2.12, 2.13, 3] - java: [temurin@11, temurin@17] + java: [temurin@11, temurin@17, temurin@21] exclude: - scala: 2.12 java: temurin@17 + - scala: 2.12 + java: temurin@21 - scala: 3 java: temurin@17 + - scala: 3 + java: temurin@21 runs-on: ${{ matrix.os }} timeout-minutes: 60 steps: @@ -70,6 +74,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Check that workflows are up to date run: sbt githubWorkflowCheck @@ -152,6 +169,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Download target directories (2.12) uses: actions/download-artifact@v4 with: @@ -246,6 +276,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Submit Dependencies uses: scalacenter/sbt-dependency-submission@v2 with: @@ -291,6 +334,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Generate site run: sbt docs/tlSite diff --git a/.mergify.yml b/.mergify.yml index d97e0ec2..a2fb1382 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -13,6 +13,7 @@ pull_request_rules: - status-success=Build and Test (ubuntu-latest, 2.12, temurin@11) - status-success=Build and Test (ubuntu-latest, 2.13, temurin@11) - status-success=Build and Test (ubuntu-latest, 2.13, temurin@17) + - status-success=Build and Test (ubuntu-latest, 2.13, temurin@21) - status-success=Build and Test (ubuntu-latest, 3, temurin@11) - status-success=Generate Site (ubuntu-latest, temurin@11) actions: diff --git a/build.sbt b/build.sbt index 9ebcb30a..ec40a096 100644 --- a/build.sbt +++ b/build.sbt @@ -80,7 +80,7 @@ ThisBuild / developers := List( ) ThisBuild / tlJdkRelease := Some(11) -ThisBuild / githubWorkflowJavaVersions := Seq("11", "17").map(JavaSpec.temurin(_)) +ThisBuild / githubWorkflowJavaVersions := Seq("11", "17", "21").map(JavaSpec.temurin(_)) ThisBuild / tlCiReleaseBranches := Seq("series/0.9") ThisBuild / tlSitePublishBranch := Some("series/0.9") @@ -109,8 +109,8 @@ lazy val docsSettings = "HTTP4S_VERSION_SHORT" -> http4sV.split("\\.").take(2).mkString("."), "SCALA_VERSION" -> CrossVersion.binaryScalaVersion(scalaVersion.value), "SCALA_VERSIONS" -> formatCrossScalaVersions((core / crossScalaVersions).value.toList) - ), - unusedCompileDependenciesFilter -= moduleFilter() + ) + // unusedCompileDependenciesFilter -= moduleFilter() ) def formatCrossScalaVersions(crossScalaVersions: List[String]): String = { diff --git a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala index 1c0472d7..07e19298 100644 --- a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala +++ b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala @@ -243,10 +243,28 @@ object JdkHttpClient { * [[cats.effect.kernel.Async.executor executor]], sets the * [[org.http4s.client.defaults.ConnectTimeout default http4s connect timeout]], and disables * [[https://github.com/http4s/http4s-jdk-http-client/issues/200 TLS 1.3 on JDK 11]]. + * + * On Java 21 and higher, prefer [[simpleResource]] as it actively closes the underlying client, + * releasing its resources early. */ def simple[F[_]](implicit F: Async[F]): F[Client[F]] = defaultHttpClient[F].map(apply(_)) + /** Like [[simple]], but wraps the client in a [[cats.effect.Resource Resource]] to ensure it's + * properly shut down on JVM 21 and higher. On lower Java versions, closing the resource does + * nothing (garbage collection will eventually clean up the client). + */ + def simpleResource[F[_]](implicit F: Async[F]): Resource[F, Client[F]] = + defaultHttpClientResource[F].map(apply(_)) + + private[jdkhttpclient] def defaultHttpClientResource[F[_]](implicit + F: Async[F] + ): Resource[F, HttpClient] = + Resource.make[F, HttpClient](defaultHttpClient[F]) { + case c: AutoCloseable => Sync[F].blocking(c.close()) + case _ => Applicative[F].unit + } + private[jdkhttpclient] def defaultHttpClient[F[_]](implicit F: Async[F]): F[HttpClient] = F.executor.flatMap { exec => F.delay { diff --git a/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala b/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala index 55851abb..b33f4daa 100644 --- a/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala +++ b/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala @@ -131,6 +131,11 @@ object JdkWSClient { }) } yield () } + // If the input side is still open (no close received from server), the JDK will not clean up the connection. + // This also implies the client can't be shutdown on Java 21+ as it waits for all open connections + // to be be closed. As we don't expect/handle anything coming on the input anymore + // at this point, we can safely abort. + _ <- F.delay(webSocket.abort()) } yield () } .map { case (webSocket, queue, closedDef, sendSem) => @@ -164,7 +169,17 @@ object JdkWSClient { * [[cats.effect.kernel.Async.executor executor]], sets the * [[org.http4s.client.defaults.ConnectTimeout default http4s connect timeout]], and disables * [[https://github.com/http4s/http4s-jdk-http-client/issues/200 TLS 1.3 on JDK 11]]. + * + * * On Java 21 and higher, prefer [[simpleResource]] as it actively closes the underlying + * client, releasing its resources early. */ def simple[F[_]](implicit F: Async[F]): F[WSClient[F]] = JdkHttpClient.defaultHttpClient[F].map(apply(_)) + + /** Like [[simple]], but wraps the client in a [[cats.effect.Resource Resource]] to ensure it's + * properly shut down on JVM 21 and higher. On lower Java versions, closing the resource does + * nothing (garbage collection will eventually clean up the client). + */ + def simpleResource[F[_]](implicit F: Async[F]): Resource[F, WSClient[F]] = + JdkHttpClient.defaultHttpClientResource[F].map(apply(_)) } diff --git a/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala b/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala index c7308651..4b5dc64d 100644 --- a/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala +++ b/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala @@ -28,7 +28,7 @@ import org.typelevel.ci._ import scala.concurrent.duration._ class JdkHttpClientSpec extends ClientRouteTestBattery("JdkHttpClient") { - def clientResource: Resource[IO, Client[IO]] = Resource.eval(JdkHttpClient.simple[IO]) + def clientResource: Resource[IO, Client[IO]] = JdkHttpClient.simpleResource[IO] // regression test for https://github.com/http4s/http4s-jdk-http-client/issues/395 test("Don't error with empty body and explicit Content-Length: 0") { diff --git a/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala b/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala index d81e63de..4e674a9f 100644 --- a/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala +++ b/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala @@ -36,7 +36,7 @@ import scala.concurrent.duration._ class JdkWSClientSpec extends CatsEffectSuite { val webSocket: IOFixture[WSClient[IO]] = - ResourceSuiteLocalFixture("webSocket", Resource.eval(JdkWSClient.simple[IO])) + ResourceSuiteLocalFixture("webSocket", JdkWSClient.simpleResource[IO]) val echoServerUri: IOFixture[Uri] = ResourceSuiteLocalFixture( "echoServerUri", From 0384cb94025f27857490fa25f2c750fdc4f2cb0e Mon Sep 17 00:00:00 2001 From: Yannick Heiber Date: Sat, 3 Feb 2024 13:20:50 +0100 Subject: [PATCH 02/10] Fix problem with unusedCompileDependenciesFilter --- build.sbt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index ec40a096..f01e13f7 100644 --- a/build.sbt +++ b/build.sbt @@ -1,4 +1,5 @@ import com.typesafe.tools.mima.core._ +import explicitdeps.ExplicitDepsPlugin.autoImport.moduleFilterRemoveValue lazy val root = project .in(file(".")) @@ -109,8 +110,8 @@ lazy val docsSettings = "HTTP4S_VERSION_SHORT" -> http4sV.split("\\.").take(2).mkString("."), "SCALA_VERSION" -> CrossVersion.binaryScalaVersion(scalaVersion.value), "SCALA_VERSIONS" -> formatCrossScalaVersions((core / crossScalaVersions).value.toList) - ) - // unusedCompileDependenciesFilter -= moduleFilter() + ), + unusedCompileDependenciesFilter -= moduleFilter() ) def formatCrossScalaVersions(crossScalaVersions: List[String]): String = { From 4cd2f1458d26e427f2bec9132e335783fbaafbe1 Mon Sep 17 00:00:00 2001 From: Yannick Heiber Date: Tue, 12 Nov 2024 14:16:34 +0100 Subject: [PATCH 03/10] Only offer Resource-based constructors --- .../org/http4s/jdkhttpclient/JdkHttpClient.scala | 15 +++++---------- .../org/http4s/jdkhttpclient/JdkWSClient.scala | 15 +++++---------- .../http4s/jdkhttpclient/BodyLeakExample.scala | 2 +- .../http4s/jdkhttpclient/DeadlockWorkaround.scala | 2 +- .../http4s/jdkhttpclient/JdkHttpClientSpec.scala | 2 +- .../http4s/jdkhttpclient/JdkWSClientSpec.scala | 2 +- 6 files changed, 14 insertions(+), 24 deletions(-) diff --git a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala index 07e19298..b32eb82b 100644 --- a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala +++ b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala @@ -244,17 +244,12 @@ object JdkHttpClient { * [[org.http4s.client.defaults.ConnectTimeout default http4s connect timeout]], and disables * [[https://github.com/http4s/http4s-jdk-http-client/issues/200 TLS 1.3 on JDK 11]]. * - * On Java 21 and higher, prefer [[simpleResource]] as it actively closes the underlying client, - * releasing its resources early. + * On Java 21 and higher, it actively closes the underlying client, releasing its resources + * early. On earlier Java versions, closing the underlying client is not possible, so the release + * is a no-op. On these Java versions (and there only), you can safely use + * [[cats.effect.Resource allocated]] to avoid dealing with resource management. */ - def simple[F[_]](implicit F: Async[F]): F[Client[F]] = - defaultHttpClient[F].map(apply(_)) - - /** Like [[simple]], but wraps the client in a [[cats.effect.Resource Resource]] to ensure it's - * properly shut down on JVM 21 and higher. On lower Java versions, closing the resource does - * nothing (garbage collection will eventually clean up the client). - */ - def simpleResource[F[_]](implicit F: Async[F]): Resource[F, Client[F]] = + def simple[F[_]](implicit F: Async[F]): Resource[F, Client[F]] = defaultHttpClientResource[F].map(apply(_)) private[jdkhttpclient] def defaultHttpClientResource[F[_]](implicit diff --git a/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala b/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala index 9738436b..49488e2f 100644 --- a/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala +++ b/core/src/main/scala/org/http4s/jdkhttpclient/JdkWSClient.scala @@ -170,16 +170,11 @@ object JdkWSClient { * [[org.http4s.client.defaults.ConnectTimeout default http4s connect timeout]], and disables * [[https://github.com/http4s/http4s-jdk-http-client/issues/200 TLS 1.3 on JDK 11]]. * - * * On Java 21 and higher, prefer [[simpleResource]] as it actively closes the underlying - * client, releasing its resources early. + * * On Java 21 and higher, it actively closes the underlying client, releasing its resources + * early. On earlier Java versions, closing the underlying client is not possible, so the release + * is a no-op. On these Java versions (and there only), you can safely use + * [[cats.effect.Resource allocated]] to avoid dealing with resource management. */ - def simple[F[_]](implicit F: Async[F]): F[WSClient[F]] = - JdkHttpClient.defaultHttpClient[F].map(apply(_)) - - /** Like [[simple]], but wraps the client in a [[cats.effect.Resource Resource]] to ensure it's - * properly shut down on JVM 21 and higher. On lower Java versions, closing the resource does - * nothing (garbage collection will eventually clean up the client). - */ - def simpleResource[F[_]](implicit F: Async[F]): Resource[F, WSClient[F]] = + def simple[F[_]](implicit F: Async[F]): Resource[F, WSClient[F]] = JdkHttpClient.defaultHttpClientResource[F].map(apply(_)) } diff --git a/core/src/test/scala/org/http4s/jdkhttpclient/BodyLeakExample.scala b/core/src/test/scala/org/http4s/jdkhttpclient/BodyLeakExample.scala index 7da879a8..b7bdad87 100644 --- a/core/src/test/scala/org/http4s/jdkhttpclient/BodyLeakExample.scala +++ b/core/src/test/scala/org/http4s/jdkhttpclient/BodyLeakExample.scala @@ -51,7 +51,7 @@ object BodyLeakExample extends IOApp { .withPort(port"8080") .withHttpApp(app) .build - .product(Resource.eval(JdkHttpClient.simple[IO])) + .product(JdkHttpClient.simple[IO]) .use { case (_, client) => for { counter <- Ref.of[IO, Long](0L) diff --git a/core/src/test/scala/org/http4s/jdkhttpclient/DeadlockWorkaround.scala b/core/src/test/scala/org/http4s/jdkhttpclient/DeadlockWorkaround.scala index e6d903bb..d471ca44 100644 --- a/core/src/test/scala/org/http4s/jdkhttpclient/DeadlockWorkaround.scala +++ b/core/src/test/scala/org/http4s/jdkhttpclient/DeadlockWorkaround.scala @@ -29,7 +29,7 @@ class DeadlockWorkaround extends CatsEffectSuite { test("fail to connect via TLSv1.3 on Java 11") { if (Runtime.version().feature() > 11) IO.pure(true) else - (JdkHttpClient.simple[IO], JdkWSClient.simple[IO]).flatMapN { (http, ws) => + (JdkHttpClient.simple[IO], JdkWSClient.simple[IO]).tupled.use { case (http, ws) => def testSSLFailure(r: IO[Unit]) = r.intercept[SSLHandshakeException] testSSLFailure(http.expect[Unit](uri"https://tls13.1d.pw")) *> testSSLFailure(ws.connectHighLevel(WSRequest(uri"wss://tls13.1d.pw")).use(_ => IO.unit)) diff --git a/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala b/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala index 4b5dc64d..77ef29b7 100644 --- a/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala +++ b/core/src/test/scala/org/http4s/jdkhttpclient/JdkHttpClientSpec.scala @@ -28,7 +28,7 @@ import org.typelevel.ci._ import scala.concurrent.duration._ class JdkHttpClientSpec extends ClientRouteTestBattery("JdkHttpClient") { - def clientResource: Resource[IO, Client[IO]] = JdkHttpClient.simpleResource[IO] + def clientResource: Resource[IO, Client[IO]] = JdkHttpClient.simple[IO] // regression test for https://github.com/http4s/http4s-jdk-http-client/issues/395 test("Don't error with empty body and explicit Content-Length: 0") { diff --git a/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala b/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala index 4e674a9f..d6604b7f 100644 --- a/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala +++ b/core/src/test/scala/org/http4s/jdkhttpclient/JdkWSClientSpec.scala @@ -36,7 +36,7 @@ import scala.concurrent.duration._ class JdkWSClientSpec extends CatsEffectSuite { val webSocket: IOFixture[WSClient[IO]] = - ResourceSuiteLocalFixture("webSocket", JdkWSClient.simpleResource[IO]) + ResourceSuiteLocalFixture("webSocket", JdkWSClient.simple[IO]) val echoServerUri: IOFixture[Uri] = ResourceSuiteLocalFixture( "echoServerUri", From 3d2262d585532647d970fea8a64f63a1f5b9f080 Mon Sep 17 00:00:00 2001 From: Yannick Heiber Date: Tue, 12 Nov 2024 14:20:51 +0100 Subject: [PATCH 04/10] Bump tlBaseVersion --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 24ca16f3..a425b0fa 100644 --- a/build.sbt +++ b/build.sbt @@ -72,7 +72,7 @@ val coreDeps = Seq( val scala213 = "2.13.15" ThisBuild / crossScalaVersions := Seq("2.12.20", scala213, "3.3.4") ThisBuild / scalaVersion := scala213 -ThisBuild / tlBaseVersion := "0.9" +ThisBuild / tlBaseVersion := "0.10" ThisBuild / startYear := Some(2019) ThisBuild / developers := List( tlGitHubDev("ChristopherDavenport", "Christopher Davenport"), From 3b83623947bd4bf87f7313ac8406c8fc23e0f75c Mon Sep 17 00:00:00 2001 From: Erlend Hamnaberg Date: Tue, 12 Nov 2024 17:50:53 +0100 Subject: [PATCH 05/10] start of series/0.10 --- build.sbt | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/build.sbt b/build.sbt index 01e59985..10e522e9 100644 --- a/build.sbt +++ b/build.sbt @@ -11,10 +11,6 @@ lazy val core = project name := "http4s-jdk-http-client", libraryDependencies ++= coreDeps, mimaBinaryIssueFilters ++= Seq( - // package private, due to #641 - ProblemFilters.exclude[IncompatibleMethTypeProblem]( - "org.http4s.jdkhttpclient.JdkHttpClient.defaultHttpClient" - ) ) ) @@ -71,7 +67,7 @@ val coreDeps = Seq( val scala213 = "2.13.15" ThisBuild / crossScalaVersions := Seq("2.12.20", scala213, "3.3.4") ThisBuild / scalaVersion := scala213 -ThisBuild / tlBaseVersion := "0.9" +ThisBuild / tlBaseVersion := "0.10" ThisBuild / startYear := Some(2019) ThisBuild / developers := List( tlGitHubDev("ChristopherDavenport", "Christopher Davenport"), @@ -80,9 +76,9 @@ ThisBuild / developers := List( ) ThisBuild / tlJdkRelease := Some(11) -ThisBuild / githubWorkflowJavaVersions := Seq("11", "17").map(JavaSpec.temurin(_)) -ThisBuild / tlCiReleaseBranches := Seq("series/0.9") -ThisBuild / tlSitePublishBranch := Some("series/0.9") +ThisBuild / githubWorkflowJavaVersions := Seq("11", "17", "21").map(JavaSpec.temurin(_)) +ThisBuild / tlCiReleaseBranches := Seq("series/0.10") +ThisBuild / tlSitePublishBranch := Some("series/0.10") lazy val docsSettings = Seq( @@ -93,9 +89,10 @@ lazy val docsSettings = import laika.config._ tlSiteHelium.value.site.versions( Versions - .forCurrentVersion(Version("0.9.x", "0.9")) + .forCurrentVersion(Version("0.10.x", "0.10")) .withOlderVersions( - Version("0.8.x", "0.8"), + Version("0.9.x", "0.9"), + Version("0.8.x", "0.8"), Version("0.7.x", "0.7"), Version("0.6.x", "0.6.0-M7"), Version("0.5.x", "0.5.0"), From f99e11acab4185403b6f3cc61ad2449e54e78812 Mon Sep 17 00:00:00 2001 From: Erlend Hamnaberg Date: Tue, 12 Nov 2024 17:54:19 +0100 Subject: [PATCH 06/10] regenerate workflow --- .github/workflows/ci.yml | 68 ++++++++++++++++++++++++++++++++++++---- .mergify.yml | 1 + 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f377d9b..148e6ca2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,12 +30,16 @@ jobs: matrix: os: [ubuntu-latest] scala: [2.12, 2.13, 3] - java: [temurin@11, temurin@17] + java: [temurin@11, temurin@17, temurin@21] exclude: - scala: 2.12 java: temurin@17 + - scala: 2.12 + java: temurin@21 - scala: 3 java: temurin@17 + - scala: 3 + java: temurin@21 runs-on: ${{ matrix.os }} timeout-minutes: 60 steps: @@ -73,6 +77,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Check that workflows are up to date run: sbt githubWorkflowCheck @@ -100,15 +117,15 @@ jobs: run: sbt '++ ${{ matrix.scala }}' unusedCompileDependenciesTest - name: Make target directories - if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.9') + if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.10') run: mkdir -p core/target project/target - name: Compress target directories - if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.9') + if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.10') run: tar cf targets.tar core/target project/target - name: Upload target directories - if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.9') + if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.10') uses: actions/upload-artifact@v4 with: name: target-${{ matrix.os }}-${{ matrix.java }}-${{ matrix.scala }} @@ -117,7 +134,7 @@ jobs: publish: name: Publish Artifacts needs: [build] - if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.9') + if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/series/0.10') strategy: matrix: os: [ubuntu-latest] @@ -158,6 +175,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Download target directories (2.12) uses: actions/download-artifact@v4 with: @@ -255,6 +285,19 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Submit Dependencies uses: scalacenter/sbt-dependency-submission@v2 with: @@ -303,11 +346,24 @@ jobs: if: matrix.java == 'temurin@17' && steps.setup-java-temurin-17.outputs.cache-hit == 'false' run: sbt +update + - name: Setup Java (temurin@21) + id: setup-java-temurin-21 + if: matrix.java == 'temurin@21' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: sbt + + - name: sbt update + if: matrix.java == 'temurin@21' && steps.setup-java-temurin-21.outputs.cache-hit == 'false' + run: sbt +update + - name: Generate site run: sbt docs/tlSite - name: Publish site - if: github.event_name != 'pull_request' && github.ref == 'refs/heads/series/0.9' + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/series/0.10' uses: peaceiris/actions-gh-pages@v4.0.0 with: github_token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.mergify.yml b/.mergify.yml index d97e0ec2..a2fb1382 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -13,6 +13,7 @@ pull_request_rules: - status-success=Build and Test (ubuntu-latest, 2.12, temurin@11) - status-success=Build and Test (ubuntu-latest, 2.13, temurin@11) - status-success=Build and Test (ubuntu-latest, 2.13, temurin@17) + - status-success=Build and Test (ubuntu-latest, 2.13, temurin@21) - status-success=Build and Test (ubuntu-latest, 3, temurin@11) - status-success=Generate Site (ubuntu-latest, temurin@11) actions: From 17d1d45db88e55b8b3b17b73a2c105642689a5b7 Mon Sep 17 00:00:00 2001 From: Erlend Hamnaberg Date: Tue, 19 Nov 2024 17:46:49 +0100 Subject: [PATCH 07/10] scalafmt --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 10e522e9..662be40d 100644 --- a/build.sbt +++ b/build.sbt @@ -92,7 +92,7 @@ lazy val docsSettings = .forCurrentVersion(Version("0.10.x", "0.10")) .withOlderVersions( Version("0.9.x", "0.9"), - Version("0.8.x", "0.8"), + Version("0.8.x", "0.8"), Version("0.7.x", "0.7"), Version("0.6.x", "0.6.0-M7"), Version("0.5.x", "0.5.0"), From 94f91f64b1b57c5bd82beae198cde17ca4ded8ca Mon Sep 17 00:00:00 2001 From: Erlend Hamnaberg Date: Tue, 19 Nov 2024 17:54:53 +0100 Subject: [PATCH 08/10] update docs --- docs/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/README.md b/docs/README.md index 987fccb7..ee80e913 100644 --- a/docs/README.md +++ b/docs/README.md @@ -50,7 +50,7 @@ import org.http4s.jdkhttpclient.JdkHttpClient // It comes for free with `cats.effect.IOApp`: import cats.effect.unsafe.implicits.global -val client: IO[Client[IO]] = JdkHttpClient.simple[IO] +val client: Resource[IO, Client[IO]] = JdkHttpClient.simple[IO] ``` #### Custom clients @@ -91,7 +91,7 @@ def fetchStatus[F[_]](c: Client[F], uri: Uri): F[Status] = c.status(Request[F](Method.GET, uri = uri)) client - .flatMap(c => fetchStatus(c, uri"https://http4s.org/")) + .use(c => fetchStatus(c, uri"https://http4s.org/")) .unsafeRunSync() ``` @@ -103,7 +103,7 @@ create a new `HttpClient` instance on every invocation: ```scala mdoc def fetchStatusInefficiently[F[_]: Async](uri: Uri): F[Status] = - JdkHttpClient.simple[F].flatMap(_.status(Request[F](Method.GET, uri = uri))) + JdkHttpClient.simple[F].use(_.status(Request[F](Method.GET, uri = uri))) ``` @:@ From 8637741e1943ca7f017762f9ab17f4c16e80f5bf Mon Sep 17 00:00:00 2001 From: Erlend Hamnaberg Date: Sat, 7 Dec 2024 10:40:29 +0100 Subject: [PATCH 09/10] Handle http/2 request body without content-length Http/2 is always streaming, so setting content-length is optional. --- .../scala/org/http4s/jdkhttpclient/JdkHttpClient.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala index b32eb82b..6a53f46e 100644 --- a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala +++ b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala @@ -63,10 +63,15 @@ object JdkHttpClient { def convertRequest(req: Request[F]): Resource[F, HttpRequest] = flow.toPublisher(req.body.chunks.map(_.toByteBuffer)).evalMap { publisher => convertHttpVersionFromHttp4s[F](req.httpVersion).map { version => + def isStreaming = version match { + case HttpClient.Version.HTTP_1_1 => req.isChunked + case HttpClient.Version.HTTP_2 => req.contentLength.isEmpty + } + val rb = HttpRequest.newBuilder .method( req.method.name, - if (req.isChunked) + if (isStreaming) BodyPublishers.fromPublisher(publisher) else req.contentLength match { From 393910c36da20b23332a9c928c0b268fdfcaddea Mon Sep 17 00:00:00 2001 From: Erlend Hamnaberg Date: Sat, 7 Dec 2024 12:37:23 +0100 Subject: [PATCH 10/10] ensure we drain the publisher if sending empty body --- .../http4s/jdkhttpclient/JdkHttpClient.scala | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala index 6a53f46e..61aa71d1 100644 --- a/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala +++ b/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala @@ -24,6 +24,7 @@ import fs2.Chunk import fs2.Stream import fs2.concurrent.SignallingRef import fs2.interop.flow +import org.http4s.EmptyBody import org.http4s.Header import org.http4s.Headers import org.http4s.HttpVersion @@ -63,21 +64,24 @@ object JdkHttpClient { def convertRequest(req: Request[F]): Resource[F, HttpRequest] = flow.toPublisher(req.body.chunks.map(_.toByteBuffer)).evalMap { publisher => convertHttpVersionFromHttp4s[F](req.httpVersion).map { version => - def isStreaming = version match { + def consumeFully = (req.body ne EmptyBody) && (version match { case HttpClient.Version.HTTP_1_1 => req.isChunked case HttpClient.Version.HTTP_2 => req.contentLength.isEmpty - } + }) val rb = HttpRequest.newBuilder .method( req.method.name, - if (isStreaming) + if (consumeFully) BodyPublishers.fromPublisher(publisher) else req.contentLength match { case Some(length) if length > 0L => BodyPublishers.fromPublisher(publisher, length) - case _ => BodyPublishers.noBody + case _ => + // If we dont do this, we might block finalization + publisher.subscribe(DrainingSubscriber) + BodyPublishers.noBody } ) .uri(URI.create(req.uri.renderString)) @@ -305,4 +309,12 @@ object JdkHttpClient { "via", "warning" ).map(CIString(_)) + + private object DrainingSubscriber extends Flow.Subscriber[ByteBuffer] { + override def onSubscribe(subscription: Flow.Subscription): Unit = + subscription.request(Long.MaxValue) + override def onNext(item: ByteBuffer): Unit = () + override def onError(throwable: Throwable): Unit = () + override def onComplete(): Unit = () + } }