From bf5a01b1241ea52bc2869ddcf85f681907aa6f5d Mon Sep 17 00:00:00 2001 From: Maxim Schuwalow Date: Fri, 27 Sep 2024 22:13:12 +0200 Subject: [PATCH 1/3] Sandbox routes later & don't pass handled responses into error handlers (#3146) --- .../src/test/scala/zio/http/RouteSpec.scala | 37 +++++++++++++++ .../src/main/scala/zio/http/Route.scala | 45 ++++++++++++++----- 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/zio-http/jvm/src/test/scala/zio/http/RouteSpec.scala b/zio-http/jvm/src/test/scala/zio/http/RouteSpec.scala index a0fc0f8b0a..4d195e2f14 100644 --- a/zio-http/jvm/src/test/scala/zio/http/RouteSpec.scala +++ b/zio-http/jvm/src/test/scala/zio/http/RouteSpec.scala @@ -135,6 +135,12 @@ object RouteSpec extends ZIOHttpSpec { bodyString <- response.body.asString } yield assertTrue(extractStatus(response) == Status.InternalServerError, bodyString == "error") }, + test("handleErrorCause should pass through responses in error channel of handled routes") { + val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.fail(Response.ok) } + val errorHandled = route.handleErrorCause(_ => Response.text("error").status(Status.InternalServerError)) + val request = Request.get(URL.decode("/endpoint").toOption.get) + errorHandled.toRoutes.runZIO(request).map(response => assertTrue(extractStatus(response) == Status.Ok)) + }, test("handleErrorCauseZIO should handle defects") { val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.dieMessage("hmm...") } val errorHandled = @@ -145,6 +151,18 @@ object RouteSpec extends ZIOHttpSpec { bodyString <- response.body.asString } yield assertTrue(extractStatus(response) == Status.InternalServerError, bodyString == "error") }, + test("handleErrorCauseZIO should pass through responses in error channel of handled routes") { + val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.fail(Response.ok) } + val request = Request.get(URL.decode("/endpoint").toOption.get) + for { + ref <- Ref.make(false) + errorHandled = route.handleErrorCauseZIO(_ => + ref.set(true) *> ZIO.succeed(Response.text("error").status(Status.InternalServerError)), + ) + response <- errorHandled.toRoutes.runZIO(request) + refValue <- ref.get + } yield assertTrue(extractStatus(response) == Status.Ok, !refValue) + }, test("handleErrorRequestCause should handle defects") { val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.dieMessage("hmm...") } val errorHandled = @@ -155,6 +173,13 @@ object RouteSpec extends ZIOHttpSpec { bodyString <- response.body.asString } yield assertTrue(extractStatus(response) == Status.InternalServerError, bodyString == "error") }, + test("handleErrorRequestCause should pass through responses in error channel of handled routes") { + val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.fail(Response.ok) } + val errorHandled = + route.handleErrorRequestCause((_, _) => Response.text("error").status(Status.InternalServerError)) + val request = Request.get(URL.decode("/endpoint").toOption.get) + errorHandled.toRoutes.runZIO(request).map(response => assertTrue(extractStatus(response) == Status.Ok)) + }, test("handleErrorRequestCauseZIO should handle defects") { val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.dieMessage("hmm...") } val errorHandled = route.handleErrorRequestCauseZIO((_, _) => @@ -166,6 +191,18 @@ object RouteSpec extends ZIOHttpSpec { bodyString <- response.body.asString } yield assertTrue(extractStatus(response) == Status.InternalServerError, bodyString == "error") }, + test("handleErrorRequestCauseZIO should pass through responses in error channel of handled routes") { + val route = Method.GET / "endpoint" -> handler { (_: Request) => ZIO.fail(Response.ok) } + val request = Request.get(URL.decode("/endpoint").toOption.get) + for { + ref <- Ref.make(false) + errorHandled = route.handleErrorRequestCauseZIO((_, _) => + ref.set(true) *> ZIO.succeed(Response.text("error").status(Status.InternalServerError)), + ) + response <- errorHandled.toRoutes.runZIO(request) + refValue <- ref.get + } yield assertTrue(extractStatus(response) == Status.Ok, !refValue) + }, test( "Routes with context can eliminate environment type partially when elimination produces intersection type environment", ) { diff --git a/zio-http/shared/src/main/scala/zio/http/Route.scala b/zio-http/shared/src/main/scala/zio/http/Route.scala index 9db2d28d13..617a0f3586 100644 --- a/zio-http/shared/src/main/scala/zio/http/Route.scala +++ b/zio-http/shared/src/main/scala/zio/http/Route.scala @@ -49,10 +49,21 @@ sealed trait Route[-Env, +Err] { self => * Handles all typed errors in the route by converting them into responses. * This method can be used to convert a route that does not handle its errors * into one that does handle its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. */ final def handleError(f: Err => Response)(implicit trace: Trace): Route[Env, Nothing] = self.handleErrorCauseZIO(c => ErrorResponseConfig.configRef.get.map(Response.fromCauseWith(c, _)(f))) + /** + * Handles all typed errors in the route by converting them into a zio effect + * that produces a response. This method can be used to convert a route that + * does not handle its errors into one that does handle its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. + */ final def handleErrorZIO[Env1 <: Env]( f: Err => ZIO[Env1, Nothing, Response], )(implicit trace: Trace): Route[Env1, Nothing] = @@ -67,13 +78,16 @@ sealed trait Route[-Env, +Err] { self => * Handles all typed errors, as well as all non-recoverable errors, by * converting them into responses. This method can be used to convert a route * that does not handle its errors into one that does handle its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. */ final def handleErrorCause(f: Cause[Err] => Response)(implicit trace: Trace): Route[Env, Nothing] = self match { case Provided(route, env) => Provided(route.handleErrorCause(f), env) case Augmented(route, aspect) => Augmented(route.handleErrorCause(f), aspect) case Handled(routePattern, handler, location) => - Handled(routePattern, handler.map(_.mapErrorCause(c => f(c.asInstanceOf[Cause[Nothing]]))), location) + Handled(routePattern, handler.map(_.mapErrorCause(_.failureOrCause.fold(identity, f))), location) case Unhandled(pattern, handler0, zippable, location) => val handler2: Handler[Any, Nothing, RoutePattern[_], Handler[Env, Response, Request, Response]] = @@ -89,7 +103,6 @@ sealed trait Route[-Env, +Err] { self => } } - // Sandbox before applying aspect: paramHandler.mapErrorCause(f) } @@ -101,6 +114,9 @@ sealed trait Route[-Env, +Err] { self => * converting them into a ZIO effect that produces the response. This method * can be used to convert a route that does not handle its errors into one * that does handle its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. */ final def handleErrorCauseZIO[Env1 <: Env]( f: Cause[Err] => ZIO[Env1, Nothing, Response], @@ -120,7 +136,7 @@ sealed trait Route[-Env, +Err] { self => case Augmented(route, aspect) => Augmented(route.handleErrorCauseZIO(f).asInstanceOf[Route[Any, Nothing]], aspect) case Handled(routePattern, handler, location) => - Handled(routePattern, handler.map(_.mapErrorCauseZIO(c => f(c.asInstanceOf[Cause[Nothing]]))), location) + Handled(routePattern, handler.map(_.mapErrorCauseZIO(_.failureOrCause.fold(ZIO.succeed(_), f))), location) case Unhandled(pattern, handler0, zippable, location) => val handler2: Handler[Any, Nothing, RoutePattern[_], Handler[Env1, Response, Request, Response]] = { @@ -174,6 +190,9 @@ sealed trait Route[-Env, +Err] { self => * taking into account the request that caused the error. This method can be * used to convert a route that does not handle its errors into one that does * handle its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. */ final def handleErrorRequest(f: (Err, Request) => Response)(implicit trace: Trace): Route[Env, Nothing] = self.handleErrorRequestCauseZIO((request, cause) => @@ -185,6 +204,9 @@ sealed trait Route[-Env, +Err] { self => * converting them into responses, taking into account the request that caused * the error. This method can be used to convert a route that does not handle * its errors into one that does handle its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. */ final def handleErrorRequestCause(f: (Request, Cause[Err]) => Response)(implicit trace: Trace): Route[Env, Nothing] = self match { @@ -195,7 +217,7 @@ sealed trait Route[-Env, +Err] { self => routePattern, handler0.map { h => Handler.fromFunctionHandler { (req: Request) => - h.mapErrorCause(c => f(req, c.asInstanceOf[Cause[Nothing]])) + h.mapErrorCause(_.failureOrCause.fold(identity, f(req, _))) } }, location, @@ -229,6 +251,9 @@ sealed trait Route[-Env, +Err] { self => * account the request that caused the error. This method can be used to * convert a route that does not handle its errors into one that does handle * its errors. + * + * If the underlying handler uses the error channel to send responses, this + * method will not pass the responses to the provided function. */ final def handleErrorRequestCauseZIO[Env1 <: Env]( f: (Request, Cause[Err]) => ZIO[Env1, Nothing, Response], @@ -252,7 +277,7 @@ sealed trait Route[-Env, +Err] { self => routePattern, handler.map { handler => Handler.fromFunctionHandler { (req: Request) => - handler.mapErrorCauseZIO(c => f(req, c.asInstanceOf[Cause[Nothing]])) + handler.mapErrorCauseZIO(_.failureOrCause.fold(ZIO.succeed(_), f(req, _))) } }, location, @@ -336,10 +361,8 @@ object Route { def handledIgnoreParams[Env]( routePattern: RoutePattern[_], - )(handler0: Handler[Env, Response, Request, Response])(implicit trace: Trace): Route[Env, Nothing] = { - // Sandbox before constructing: - Route.Handled(routePattern, handler((_: RoutePattern[_]) => handler0.sandbox), Trace.empty) - } + )(handler0: Handler[Env, Response, Request, Response])(implicit trace: Trace): Route[Env, Nothing] = + Route.Handled(routePattern, handler((_: RoutePattern[_]) => handler0), Trace.empty) def handled[Params, Env](rpm: RoutePattern[Params]): HandledConstructor[Env, Params] = new HandledConstructor[Env, Params](rpm) @@ -365,7 +388,7 @@ object Route { } // Sandbox before applying aspect: - paramHandler.sandbox + paramHandler } } @@ -414,7 +437,7 @@ object Route { location: Trace, ) extends Route[Env, Nothing] { override def toHandler(implicit ev: Nothing <:< Response, trace: Trace): Handler[Env, Response, Request, Response] = - Handler.fromZIO(handler(routePattern)).flatten + Handler.fromZIO(handler(routePattern).map(_.sandbox)).flatten override def toString = s"Route.Handled(${routePattern}, ${location})" } From 83a24a56abe5cb8b97c96a92bab189c5d91edba3 Mon Sep 17 00:00:00 2001 From: "zio-assistant[bot]" <130037499+zio-assistant[bot]@users.noreply.github.com> Date: Sat, 28 Sep 2024 05:41:54 +0200 Subject: [PATCH 2/3] Update README.md (#3145) Co-authored-by: ZIO Assistant --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5c8b92ec0a..73dfc6c7ea 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Some of the key features of ZIO HTTP are: Setup via `build.sbt`: ```scala -libraryDependencies += "dev.zio" %% "zio-http" % "3.0.0" +libraryDependencies += "dev.zio" %% "zio-http" % "3.0.1" ``` **NOTES ON VERSIONING:** From 92af675902d4af87ef88a53a08b5855e7a33674d Mon Sep 17 00:00:00 2001 From: Nabil Abdel-Hafeez <7283535+987Nabil@users.noreply.github.com> Date: Sun, 29 Sep 2024 10:49:58 +0200 Subject: [PATCH 3/3] Fix OpenAPIGen for opt payloads (#3181) --- .../scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala | 3 +-- .../main/scala/zio/http/endpoint/openapi/JsonSchema.scala | 6 +++--- .../main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala | 4 ++++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala b/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala index f24807a138..7ca59548c8 100644 --- a/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala +++ b/zio-http/jvm/src/test/scala/zio/http/endpoint/openapi/OpenAPIGenSpec.scala @@ -1029,8 +1029,7 @@ object OpenAPIGenSpec extends ZIOSpecDefault { | { | "$ref" : "#/components/schemas/Payload" | } - | ], - | "description" : "" + | ] | } | } | }, diff --git a/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala b/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala index cdc38d22d2..d2063c2bc8 100644 --- a/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala +++ b/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/JsonSchema.scala @@ -55,11 +55,11 @@ private[openapi] case class SerializableJsonSchema( if (nullable && schemaType.isDefined) copy(schemaType = Some(schemaType.get.add("null"))) else if (nullable && oneOf.isDefined) - copy(oneOf = Some(oneOf.get :+ typeNull)) + copy(oneOf = Some((oneOf.get :+ typeNull).distinct)) else if (nullable && allOf.isDefined) - SerializableJsonSchema(allOf = Some(Chunk(this, typeNull))) + SerializableJsonSchema(allOf = Some((allOf.get :+ typeNull).distinct)) else if (nullable && anyOf.isDefined) - copy(anyOf = Some(anyOf.get :+ typeNull)) + copy(anyOf = Some((anyOf.get :+ typeNull).distinct)) else if (nullable && ref.isDefined) SerializableJsonSchema(anyOf = Some(Chunk(typeNull, this))) else diff --git a/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala b/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala index 201bddb047..19727820ae 100644 --- a/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala +++ b/zio-http/shared/src/main/scala/zio/http/endpoint/openapi/OpenAPIGen.scala @@ -984,6 +984,10 @@ object OpenAPIGen { } case t: Transform[_, _, _] => nominal(t.schema, referenceType) + case Schema.Optional(inner, _) => + nominal(inner, referenceType) + case Schema.Lazy(schema0) => + nominal(schema0(), referenceType) case _ => None }