From 4264ebea6deb56c672575cc82e0508f1340e8e7e Mon Sep 17 00:00:00 2001 From: Sam Bryan Date: Tue, 20 Jun 2023 12:33:37 +0100 Subject: [PATCH 1/9] Bump http4s, http4s-servlet versions --- build.sbt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 52c7404..ea24986 100644 --- a/build.sbt +++ b/build.sbt @@ -24,8 +24,8 @@ lazy val root = project .aggregate(jettyServer, jettyClient) val jettyVersion = "9.4.50.v20221201" -val http4sVersion = "0.23.17" -val http4sServletVersion = "0.23.13" +val http4sVersion = "0.23.21" +val http4sServletVersion = "0.23.14" val munitCatsEffectVersion = "1.0.7" val slf4jVersion = "1.7.25" From 68b9672f1ec86c46ff4cc6146a6092e1597bd07d Mon Sep 17 00:00:00 2001 From: Sam Bryan Date: Tue, 20 Jun 2023 12:34:21 +0100 Subject: [PATCH 2/9] Remove use of async_ Fixes cancellation to match CE3.5 semantics --- .../http4s/jetty/server/JettyLifeCycle.scala | 134 +++++++++--------- .../jetty/server/JettyServerSuite.scala | 31 ++-- 2 files changed, 83 insertions(+), 82 deletions(-) diff --git a/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala b/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala index 8ed65fc..4f83d1d 100644 --- a/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala +++ b/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala @@ -18,8 +18,7 @@ package org.http4s.jetty.server import cats.effect._ import cats.syntax.all._ -import org.eclipse.jetty.util.component.Destroyable -import org.eclipse.jetty.util.component.LifeCycle +import org.eclipse.jetty.util.component.{Destroyable, LifeCycle} private[jetty] object JettyLifeCycle { @@ -55,36 +54,39 @@ private[jetty] object JettyLifeCycle { * internally, e.g. due to some internal error occurring. */ private[this] def stopLifeCycle[F[_]](lifeCycle: LifeCycle)(implicit F: Async[F]): F[Unit] = - F.async_[Unit] { cb => - lifeCycle.addLifeCycleListener( - new LifeCycle.Listener { - override def lifeCycleStopped(a: LifeCycle): Unit = - cb(Right(())) - override def lifeCycleFailure(a: LifeCycle, error: Throwable): Unit = - cb(Left(error)) - } - ) + F.async[Unit] { cb => + F.delay { + val listener = + new LifeCycle.Listener { + override def lifeCycleStopped(a: LifeCycle): Unit = + cb(Right(())) + override def lifeCycleFailure(a: LifeCycle, error: Throwable): Unit = + cb(Left(error)) + } + lifeCycle.addLifeCycleListener(listener) - // In the general case, it is not sufficient to merely call stop(). For - // example, the concrete implementation of stop() for the canonical - // Jetty Server instance will shortcut to a `return` call taking no - // action if the server is "stopping". This method _can't_ return until - // we are _actually stopped_, so we have to check three different states - // here. + // In the general case, it is not sufficient to merely call stop(). For + // example, the concrete implementation of stop() for the canonical + // Jetty Server instance will shortcut to a `return` call taking no + // action if the server is "stopping". This method _can't_ return until + // we are _actually stopped_, so we have to check three different states + // here. - if (lifeCycle.isStopped) { - // If the first case, we are already stopped, so our listener won't be - // called and we just return. - cb(Right(())) - } else if (lifeCycle.isStopping()) { - // If it is stopping, we need to wait for our listener to get invoked. - () - } else { - // If it is neither stopped nor stopping, we need to request a stop - // and then wait for the event. It is imperative that we add the - // listener beforehand here. Otherwise we have some very annoying race - // conditions. - lifeCycle.stop() + if (lifeCycle.isStopped) { + // If the first case, we are already stopped, so our listener won't be + // called and we just return. + cb(Right(())) + } else if (lifeCycle.isStopping()) { + // If it is stopping, we need to wait for our listener to get invoked. + () + } else { + // If it is neither stopped nor stopping, we need to request a stop + // and then wait for the event. It is imperative that we add the + // listener beforehand here. Otherwise we have some very annoying race + // conditions. + lifeCycle.stop() + } + Some(F.delay(lifeCycle.removeLifeCycleListener(listener))) } } @@ -95,51 +97,53 @@ private[jetty] object JettyLifeCycle { * (or starting) this will fail. */ private[this] def startLifeCycle[F[_]](lifeCycle: LifeCycle)(implicit F: Async[F]): F[Unit] = - F.async_[Unit] { cb => - lifeCycle.addLifeCycleListener( - new LifeCycle.Listener { + F.async[Unit] { cb => + F.delay { + val listener = new LifeCycle.Listener { override def lifeCycleStarted(a: LifeCycle): Unit = cb(Right(())) override def lifeCycleFailure(a: LifeCycle, error: Throwable): Unit = cb(Left(error)) } - ) + lifeCycle.addLifeCycleListener(listener) - // Sanity check to ensure the LifeCycle component is not already - // started. A couple of notes here. - // - // - There is _always_ going to be a small chance of a race condition - // here in the final branch where we invoke `lifeCycle.start()` _if_ - // something else has a reference to the `LifeCycle` - // value. Thankfully, unlike the stopLifeCycle function, this is - // completely in the control of the caller. As long as the caller - // doesn't leak the reference (or call .start() themselves) nothing - // internally to Jetty should ever invoke .start(). - // - Jetty components allow for reuse in many cases, unless the - // .destroy() method is invoked (and the particular type implements - // `Destroyable`, it's not part of `LifeCycle`). Jetty uses this for - // "soft" resets of the `LifeCycle` component. Thus it is possible - // that this `LifeCycle` component has been started before, though I - // don't recommend this and we don't (at this time) do that in the - // http4s codebase. - if (lifeCycle.isStarted) { - cb( - Left( - new IllegalStateException( - "Attempting to start Jetty LifeCycle component, but it is already started." + // Sanity check to ensure the LifeCycle component is not already + // started. A couple of notes here. + // + // - There is _always_ going to be a small chance of a race condition + // here in the final branch where we invoke `lifeCycle.start()` _if_ + // something else has a reference to the `LifeCycle` + // value. Thankfully, unlike the stopLifeCycle function, this is + // completely in the control of the caller. As long as the caller + // doesn't leak the reference (or call .start() themselves) nothing + // internally to Jetty should ever invoke .start(). + // - Jetty components allow for reuse in many cases, unless the + // .destroy() method is invoked (and the particular type implements + // `Destroyable`, it's not part of `LifeCycle`). Jetty uses this for + // "soft" resets of the `LifeCycle` component. Thus it is possible + // that this `LifeCycle` component has been started before, though I + // don't recommend this and we don't (at this time) do that in the + // http4s codebase. + if (lifeCycle.isStarted) { + cb( + Left( + new IllegalStateException( + "Attempting to start Jetty LifeCycle component, but it is already started." + ) ) ) - ) - } else if (lifeCycle.isStarting) { - cb( - Left( - new IllegalStateException( - "Attempting to start Jetty LifeCycle component, but it is already starting." + } else if (lifeCycle.isStarting) { + cb( + Left( + new IllegalStateException( + "Attempting to start Jetty LifeCycle component, but it is already starting." + ) ) ) - ) - } else { - lifeCycle.start() + } else { + lifeCycle.start() + } + Some(F.delay(lifeCycle.removeLifeCycleListener(listener))) } } } diff --git a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala index fccda0f..25eba38 100644 --- a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala +++ b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala @@ -18,16 +18,11 @@ package org.http4s package jetty package server -import cats.effect.IO -import cats.effect.Resource -import cats.effect.Temporal +import cats.effect.{IO, Resource, Temporal} import munit.CatsEffectSuite import org.eclipse.jetty.client.HttpClient -import org.eclipse.jetty.client.api.Request -import org.eclipse.jetty.client.api.Response -import org.eclipse.jetty.client.api.Result -import org.eclipse.jetty.client.util.BufferingResponseListener -import org.eclipse.jetty.client.util.StringContentProvider +import org.eclipse.jetty.client.api.{Request, Response, Result} +import org.eclipse.jetty.client.util.{BufferingResponseListener, StringContentProvider} import org.http4s.dsl.io._ import org.http4s.server.Server @@ -74,15 +69,17 @@ class JettyServerSuite extends CatsEffectSuite { private val jettyServer = ResourceFixture[Server](serverR) private def fetchBody(req: Request): IO[String] = - IO.async_ { cb => - val listener = new BufferingResponseListener() { - override def onFailure(resp: Response, t: Throwable) = - cb(Left(t)) - - override def onComplete(result: Result) = - cb(Right(getContentAsString)) - } - req.send(listener) + IO.async { cb => + IO { + val listener = new BufferingResponseListener() { + override def onFailure(resp: Response, t: Throwable) = + cb(Left(t)) + + override def onComplete(result: Result) = + cb(Right(getContentAsString)) + } + req.send(listener) + }.as(Some(IO.unit)) } private def get(server: Server, path: String): IO[String] = { From 3c4a2821f70766613ba4ea8037f7307429bd7642 Mon Sep 17 00:00:00 2001 From: sgjbryan Date: Tue, 20 Jun 2023 20:33:17 +0100 Subject: [PATCH 3/9] Run scalafixAll --- .../org/http4s/jetty/server/JettyLifeCycle.scala | 3 ++- .../org/http4s/jetty/server/JettyServerSuite.scala | 11 ++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala b/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala index 4f83d1d..ef7479b 100644 --- a/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala +++ b/jetty-server/src/main/scala/org/http4s/jetty/server/JettyLifeCycle.scala @@ -18,7 +18,8 @@ package org.http4s.jetty.server import cats.effect._ import cats.syntax.all._ -import org.eclipse.jetty.util.component.{Destroyable, LifeCycle} +import org.eclipse.jetty.util.component.Destroyable +import org.eclipse.jetty.util.component.LifeCycle private[jetty] object JettyLifeCycle { diff --git a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala index 25eba38..a94a755 100644 --- a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala +++ b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala @@ -18,11 +18,16 @@ package org.http4s package jetty package server -import cats.effect.{IO, Resource, Temporal} +import cats.effect.IO +import cats.effect.Resource +import cats.effect.Temporal import munit.CatsEffectSuite import org.eclipse.jetty.client.HttpClient -import org.eclipse.jetty.client.api.{Request, Response, Result} -import org.eclipse.jetty.client.util.{BufferingResponseListener, StringContentProvider} +import org.eclipse.jetty.client.api.Request +import org.eclipse.jetty.client.api.Response +import org.eclipse.jetty.client.api.Result +import org.eclipse.jetty.client.util.BufferingResponseListener +import org.eclipse.jetty.client.util.StringContentProvider import org.http4s.dsl.io._ import org.http4s.server.Server From 65251dc4371c541e69d1afe60ae76e8801ad06cc Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Mon, 26 Jun 2023 20:15:34 -0400 Subject: [PATCH 4/9] Update scala3-library to 3.3.0 --- .github/workflows/ci.yml | 12 ++++++------ build.sbt | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 426649b..153cc25 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,16 +29,16 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest] - scala: [2.13.8, 2.12.17, 3.2.0] + scala: [2.13.8, 2.12.17, 3.3.0] java: [temurin@8, temurin@11, temurin@17] exclude: - scala: 2.12.17 java: temurin@11 - scala: 2.12.17 java: temurin@17 - - scala: 3.2.0 + - scala: 3.3.0 java: temurin@11 - - scala: 3.2.0 + - scala: 3.3.0 java: temurin@17 runs-on: ${{ matrix.os }} steps: @@ -244,12 +244,12 @@ jobs: tar xf targets.tar rm targets.tar - - name: Download target directories (3.2.0) + - name: Download target directories (3.3.0) uses: actions/download-artifact@v2 with: - name: target-${{ matrix.os }}-${{ matrix.java }}-3.2.0 + name: target-${{ matrix.os }}-${{ matrix.java }}-3.3.0 - - name: Inflate target directories (3.2.0) + - name: Inflate target directories (3.3.0) run: | tar xf targets.tar rm targets.tar diff --git a/build.sbt b/build.sbt index ea24986..d244ab3 100644 --- a/build.sbt +++ b/build.sbt @@ -12,7 +12,7 @@ ThisBuild / developers := List( ThisBuild / tlSitePublishBranch := Some("main") val Scala213 = "2.13.8" -ThisBuild / crossScalaVersions := Seq(Scala213, "2.12.17", "3.2.0") +ThisBuild / crossScalaVersions := Seq(Scala213, "2.12.17", "3.3.0") ThisBuild / scalaVersion := Scala213 // the default Scala ThisBuild / resolvers += From 8432dad2bdbb9161248162c0508cdbe967c1d8d2 Mon Sep 17 00:00:00 2001 From: Sam Bryan Date: Tue, 27 Jun 2023 17:33:09 +0100 Subject: [PATCH 5/9] Try switching IO.never in test --- .../test/scala/org/http4s/jetty/server/JettyServerSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala index a94a755..42402a7 100644 --- a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala +++ b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala @@ -62,7 +62,7 @@ class JettyServerSuite extends CatsEffectSuite { Ok(req.body) case GET -> Root / "never" => - IO.never + IO.async(_ => IO.pure(Some(IO.unit))) case GET -> Root / "slow" => Temporal[IO].sleep(50.millis) *> Ok("slow") From 36acff689fae4f35ce8337d727b4649d18f9f17c Mon Sep 17 00:00:00 2001 From: Sam Bryan Date: Tue, 27 Jun 2023 17:48:28 +0100 Subject: [PATCH 6/9] Try switching test client to blocking --- .../jetty/server/JettyServerSuite.scala | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala index 42402a7..db0e056 100644 --- a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala +++ b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala @@ -18,15 +18,10 @@ package org.http4s package jetty package server -import cats.effect.IO -import cats.effect.Resource -import cats.effect.Temporal +import cats.effect.{IO, Resource, Temporal} import munit.CatsEffectSuite import org.eclipse.jetty.client.HttpClient import org.eclipse.jetty.client.api.Request -import org.eclipse.jetty.client.api.Response -import org.eclipse.jetty.client.api.Result -import org.eclipse.jetty.client.util.BufferingResponseListener import org.eclipse.jetty.client.util.StringContentProvider import org.http4s.dsl.io._ import org.http4s.server.Server @@ -74,18 +69,7 @@ class JettyServerSuite extends CatsEffectSuite { private val jettyServer = ResourceFixture[Server](serverR) private def fetchBody(req: Request): IO[String] = - IO.async { cb => - IO { - val listener = new BufferingResponseListener() { - override def onFailure(resp: Response, t: Throwable) = - cb(Left(t)) - - override def onComplete(result: Result) = - cb(Right(getContentAsString)) - } - req.send(listener) - }.as(Some(IO.unit)) - } + IO.interruptible(req.send().getContentAsString()) private def get(server: Server, path: String): IO[String] = { val req = client().newRequest(s"http://127.0.0.1:${server.address.getPort}$path") From f06f45556f9e935b42ca6586d1315a9255ee4e8d Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Fri, 30 Jun 2023 18:34:11 -0400 Subject: [PATCH 7/9] Upgrade to http4s-servlet-0.23.15-RC1 --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index d244ab3..92e36c7 100644 --- a/build.sbt +++ b/build.sbt @@ -25,7 +25,7 @@ lazy val root = project val jettyVersion = "9.4.50.v20221201" val http4sVersion = "0.23.21" -val http4sServletVersion = "0.23.14" +val http4sServletVersion = "0.23.15-RC1" val munitCatsEffectVersion = "1.0.7" val slf4jVersion = "1.7.25" From e37f12ff665295191cb42fe40bd70d6290c1ad60 Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Fri, 30 Jun 2023 19:07:59 -0400 Subject: [PATCH 8/9] Touch up the imports --- .../test/scala/org/http4s/jetty/server/JettyServerSuite.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala index db0e056..7584f3d 100644 --- a/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala +++ b/jetty-server/src/test/scala/org/http4s/jetty/server/JettyServerSuite.scala @@ -18,7 +18,9 @@ package org.http4s package jetty package server -import cats.effect.{IO, Resource, Temporal} +import cats.effect.IO +import cats.effect.Resource +import cats.effect.Temporal import munit.CatsEffectSuite import org.eclipse.jetty.client.HttpClient import org.eclipse.jetty.client.api.Request From 56872e5ba3a0a621ffc1d48f370c15f141eb3514 Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Mon, 3 Jul 2023 16:43:09 -0400 Subject: [PATCH 9/9] http4s-0.23.22, http4s-servlet-0.23.15 --- build.sbt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 92e36c7..a5972cb 100644 --- a/build.sbt +++ b/build.sbt @@ -24,8 +24,8 @@ lazy val root = project .aggregate(jettyServer, jettyClient) val jettyVersion = "9.4.50.v20221201" -val http4sVersion = "0.23.21" -val http4sServletVersion = "0.23.15-RC1" +val http4sVersion = "0.23.22" +val http4sServletVersion = "0.23.15" val munitCatsEffectVersion = "1.0.7" val slf4jVersion = "1.7.25"