Merge pull request #2747 from softwaremill/zio-http-better-path-decode

Improve the efficiency of request handling in ZIO Http & Play

doc/server/play.md

@@ -48,6 +48,16 @@ val countCharactersRoutes: Routes =
   PlayServerInterpreter().toRoutes(countCharactersEndpoint.serverLogic(countCharacters _))
 ```
 
+```eval_rst
+.. note::
+
+  A single Play application can contain both tapir-managed andPlay-managed routes. However, because of the 
+  routing implementation in Play, the shape of the paths that tapir/Play-native handlers serve should not 
+  overlap. The shape of the path includes exact path segments, single- and multi-wildcards. Otherwise, request handling 
+  will throw an exception. We don't expect users to encounter this as a problem, however the implementation here 
+  diverges a bit comparing to other interpreters.
+```
+
 ## Bind the routes
 
 ### Creating the HTTP server manually

doc/server/ziohttp.md

@@ -24,7 +24,7 @@ Next, instead of the usual `import sttp.tapir._`, you should import (or extend t
 import sttp.tapir.ztapir._
 ```
 
-This brings into scope all of the [basic](../endpoint/basics.md) input/output descriptions, which can be used to define an endpoint.
+This brings into scope all the [basic](../endpoint/basics.md) input/output descriptions, which can be used to define an endpoint.
 
 ```eval_rst
 .. note::
@@ -59,6 +59,16 @@ val countCharactersHttp: HttpApp[Any, Throwable] =
   ZioHttpInterpreter().toHttp(countCharactersEndpoint.zServerLogic(countCharacters))
 ```
 
+```eval_rst
+.. note::
+
+  A single ZIO-Http application can contain both tapir-managed and ZIO-Http-managed routes. However, because of the 
+  routing implementation in ZIO Http, the shape of the paths that tapir/ZIO-Http-native handlers serve should not 
+  overlap. The shape of the path includes exact path segments, single- and multi-wildcards. Otherwise, request handling 
+  will throw an exception. We don't expect users to encounter this as a problem, however the implementation here 
+  diverges a bit comparing to other interpreters.
+```
+
 ## Server logic
 
 When defining the business logic for an endpoint, the following methods are available, which replace the

server/play-server/src/main/scala/sttp/tapir/server/play/PlayServerInterpreter.scala

@@ -10,18 +10,11 @@ import play.api.mvc._
 import play.api.routing.Router.Routes
 import sttp.capabilities.WebSockets
 import sttp.capabilities.akka.AkkaStreams
-import sttp.model.{Method, StatusCode}
+import sttp.model.Method
 import sttp.monad.FutureMonad
 import sttp.tapir.server.ServerEndpoint
-import sttp.tapir.server.interceptor.{DecodeFailureContext, RequestResult}
-import sttp.tapir.server.interpreter.{
-  BodyListener,
-  DecodeBasicInputs,
-  DecodeBasicInputsResult,
-  DecodeInputsContext,
-  FilterServerEndpoints,
-  ServerInterpreter
-}
+import sttp.tapir.server.interceptor.RequestResult
+import sttp.tapir.server.interpreter.{BodyListener, FilterServerEndpoints, ServerInterpreter}
 import sttp.tapir.server.model.ServerResponse
 
 import scala.concurrent.{ExecutionContext, Future}
@@ -47,17 +40,10 @@ trait PlayServerInterpreter {
   ): Routes = {
     implicit val monad: FutureMonad = new FutureMonad()
 
+    val filterServerEndpoints = FilterServerEndpoints(serverEndpoints)
+
     new PartialFunction[RequestHeader, Handler] {
-      override def isDefinedAt(request: RequestHeader): Boolean = {
-        val serverRequest = PlayServerRequest(request, request)
-        serverEndpoints.exists { se =>
-          DecodeBasicInputs(se.securityInput.and(se.input), DecodeInputsContext(serverRequest)) match {
-            case (DecodeBasicInputsResult.Values(_, _), _) => true
-            case (DecodeBasicInputsResult.Failure(input, failure), _) =>
-              playServerOptions.decodeFailureHandler(DecodeFailureContext(se.endpoint, input, failure, serverRequest)).isDefined
-          }
-        }
-      }
+      override def isDefinedAt(request: RequestHeader): Boolean = filterServerEndpoints(PlayServerRequest(request, request)).nonEmpty
 
       override def apply(header: RequestHeader): Handler =
         if (isWebSocket(header))
@@ -79,7 +65,7 @@ trait PlayServerInterpreter {
         implicit val bodyListener: BodyListener[Future, PlayResponseBody] = new PlayBodyListener
         val serverRequest = PlayServerRequest(header, request)
         val interpreter = new ServerInterpreter(
-          FilterServerEndpoints(serverEndpoints),
+          filterServerEndpoints,
           new PlayRequestBody(playServerOptions),
           new PlayToResponseBody,
           playServerOptions.interceptors,
@@ -89,7 +75,12 @@ trait PlayServerInterpreter {
         interpreter(serverRequest)
           .map {
             case RequestResult.Failure(_) =>
-              Left(Result(header = ResponseHeader(StatusCode.NotFound.code), body = HttpEntity.NoEntity))
+              throw new RuntimeException(
+                s"The path: ${request.path} matches the shape of some endpoint, but none of the " +
+                  s"endpoints decoded the request successfully, and the decode failure handler didn't provide a " +
+                  s"response. Play requires that if the path shape matches some endpoints, the request " +
+                  s"should be handled by tapir."
+              )
             case RequestResult.Response(response: ServerResponse[PlayResponseBody]) =>
               val headers: Map[String, String] = response.headers
                 .foldLeft(Map.empty[String, List[String]]) { (a, b) =>

server/tests/src/main/scala/sttp/tapir/server/tests/ServerBasicTests.scala

@@ -12,6 +12,7 @@ import sttp.monad.MonadError
 import sttp.tapir._
 import sttp.tapir.generic.auto._
 import sttp.tapir.json.circe._
+import sttp.tapir.server.ServerEndpoint
 import sttp.tapir.server.interceptor.decodefailure.{DecodeFailureHandler, DefaultDecodeFailureHandler}
 import sttp.tapir.tests.Basic._
 import sttp.tapir.tests.TestUtil._
@@ -583,8 +584,12 @@ class ServerBasicTests[F[_], OPTIONS, ROUTE](
     testServer(
       "two endpoints, same path prefix, one without trailing slashes, second accepting trailing slashes",
       NonEmptyList.of(
-        route(endpoint.get.in("p1" / "p2").in(noTrailingSlash).out(stringBody).serverLogic((_: Unit) => pureResult("e1".asRight[Unit]))),
-        route(endpoint.get.in("p1" / "p2").in(paths).out(stringBody).serverLogic((_: List[String]) => pureResult("e2".asRight[Unit])))
+        route(
+          List[ServerEndpoint[Any, F]](
+            endpoint.get.in("p1" / "p2").in(noTrailingSlash).out(stringBody).serverLogic((_: Unit) => pureResult("e1".asRight[Unit])),
+            endpoint.get.in("p1" / "p2").in(paths).out(stringBody).serverLogic((_: List[String]) => pureResult("e2".asRight[Unit]))
+          )
+        )
       )
     ) { (backend, baseUri) =>
       basicStringRequest.get(uri"$baseUri/p1/p2").send(backend).map(_.body shouldBe "e1") >>

server/zio-http-server/src/main/scala/sttp/tapir/server/ziohttp/ZioHttpInterpreter.scala

@@ -4,19 +4,13 @@ import io.netty.handler.codec.http.HttpResponseStatus
 import sttp.capabilities.zio.ZioStreams
 import sttp.model.{HeaderNames, Header => SttpHeader}
 import sttp.monad.MonadError
-import sttp.tapir.server.interceptor.{DecodeFailureContext, RequestResult}
+import sttp.tapir.server.interceptor.RequestResult
 import sttp.tapir.server.interceptor.reject.RejectInterceptor
-import sttp.tapir.server.interpreter.{
-  DecodeBasicInputs,
-  DecodeBasicInputsResult,
-  DecodeInputsContext,
-  FilterServerEndpoints,
-  ServerInterpreter
-}
+import sttp.tapir.server.interpreter.{FilterServerEndpoints, ServerInterpreter}
 import sttp.tapir.ztapir._
-import zio.http.{Body, Handler, HttpApp, Http, Request, Response}
-import zio.http.model.{Status, Header => ZioHttpHeader, Headers => ZioHttpHeaders, HttpError}
 import zio._
+import zio.http._
+import zio.http.model.{Status, Header => ZioHttpHeader, Headers => ZioHttpHeaders}
 
 trait ZioHttpInterpreter[R] {
   def zioHttpServerOptions: ZioHttpServerOptions[R] = ZioHttpServerOptions.default
@@ -29,17 +23,20 @@ trait ZioHttpInterpreter[R] {
     implicit val monadError: MonadError[RIO[R & R2, *]] = new RIOMonadError[R & R2]
     val widenedSes = ses.map(_.widen[R & R2])
     val widenedServerOptions = zioHttpServerOptions.widen[R & R2]
+    val zioHttpRequestBody = new ZioHttpRequestBody(widenedServerOptions)
+    val zioHttpResponseBody = new ZioHttpToResponseBody
+    val interceptors = RejectInterceptor.disableWhenSingleEndpoint(widenedServerOptions.interceptors, widenedSes)
 
-    val interpreter = new ServerInterpreter[ZioStreams, RIO[R & R2, *], ZioHttpResponseBody, ZioStreams](
-      FilterServerEndpoints[ZioStreams, RIO[R & R2, *]](widenedSes),
-      new ZioHttpRequestBody(widenedServerOptions),
-      new ZioHttpToResponseBody,
-      RejectInterceptor.disableWhenSingleEndpoint(widenedServerOptions.interceptors, widenedSes),
-      zioHttpServerOptions.deleteFile
-    )
-
-    def handleRequest(req: Request) = {
+    def handleRequest(req: Request, filteredEndpoints: List[ZServerEndpoint[R & R2, ZioStreams]]) = {
       Handler.fromZIO {
+        val interpreter = new ServerInterpreter[ZioStreams, RIO[R & R2, *], ZioHttpResponseBody, ZioStreams](
+          _ => filteredEndpoints,
+          zioHttpRequestBody,
+          zioHttpResponseBody,
+          interceptors,
+          zioHttpServerOptions.deleteFile
+        )
+
         interpreter
           .apply(ZioHttpServerRequest(req))
           .foldZIO(
@@ -60,28 +57,29 @@ trait ZioHttpInterpreter[R] {
                     body = resp.body.map { case (stream, _) => Body.fromStream(stream) }.getOrElse(Body.empty)
                   )
                 )
-              case RequestResult.Failure(_) => ZIO.succeed(HttpError.NotFound("Not Found").toResponse)
+              case RequestResult.Failure(_) =>
+                ZIO.fail(
+                  new RuntimeException(
+                    s"The path: ${req.path} matches the shape of some endpoint, but none of the " +
+                      s"endpoints decoded the request successfully, and the decode failure handler didn't provide a " +
+                      s"response. ZIO Http requires that if the path shape matches some endpoints, the request " +
+                      s"should be handled by tapir."
+                  )
+                )
             }
           )
       }
     }
 
-    val routes = new PartialFunction[Request, Handler[R & R2, Throwable, Request, Response]] {
-      override def isDefinedAt(request: Request): Boolean = {
-        val serverRequest = ZioHttpServerRequest(request)
-        ses.exists { se =>
-          DecodeBasicInputs(se.securityInput.and(se.input), DecodeInputsContext(serverRequest)) match {
-            case (DecodeBasicInputsResult.Values(_, _), _) => true
-            case (DecodeBasicInputsResult.Failure(input, failure), _) =>
-              zioHttpServerOptions.decodeFailureHandler(DecodeFailureContext(se.endpoint, input, failure, serverRequest)).isDefined
-          }
-        }
+    val serverEndpointsFilter = FilterServerEndpoints[ZioStreams, RIO[R & R2, *]](widenedSes)
+    Http.fromOptionalHandlerZIO { request =>
+      // pre-filtering the endpoints by shape to determine, if this request should be handled by tapir
+      val filteredEndpoints = serverEndpointsFilter.apply(ZioHttpServerRequest(request))
+      filteredEndpoints match {
+        case Nil => ZIO.fail(None)
+        case _   => ZIO.succeed(handleRequest(request, filteredEndpoints))
       }
-
-      override def apply(req: Request) = handleRequest(req)
     }
-
-    Http.collectHandler[Request](routes)
   }
 
   private def sttpToZioHttpHeader(hl: (String, Seq[SttpHeader])): List[ZioHttpHeader] = {