Merge pull request #40 from lichess-org/revert-handler

Revert handler

Author: lenguyenthanh

core/play/src/main/scala/play/api/http/HttpRequestHandler.scala

@@ -21,16 +21,16 @@ trait HttpRequestHandler:
    * will be tagged with routing information. It is also acceptable to simply return the request as is. Play
    * will switch to using the returned request from this point in in its request handling.
    *
-   * The reason why the API allows returning a modified request, rather than just wrapping the action in a new
-   * action that modifies the request, is so that Play can pass this request to other handlers, such as error
-   * handlers, or filters, and they will get the tagged/modified request.
+   * The reason why the API allows returning a modified request, rather than just wrapping the Handler in a
+   * new Handler that modifies the request, is so that Play can pass this request to other handlers, such as
+   * error handlers, or filters, and they will get the tagged/modified request.
    *
    * @param request
    *   The request to handle
    * @return
    *   The possibly modified/tagged request, and a handler to handle it
    */
-  def handlerForRequest(request: RequestHeader): (RequestHeader, EssentialAction)
+  def handlerForRequest(request: RequestHeader): (RequestHeader, Handler)
 
 /**
  * Implementation of a [HttpRequestHandler] that always returns NotImplemented results
@@ -96,7 +96,7 @@ class DefaultHttpRequestHandler(
     context.isEmpty ||
       (path.startsWith(context) && (path.length == context.length || path.charAt(context.length) == '/'))
 
-  override def handlerForRequest(request: RequestHeader): (RequestHeader, EssentialAction) =
+  override def handlerForRequest(request: RequestHeader): (RequestHeader, Handler) =
     def handleWithStatus(status: Int) =
       ActionBuilder.ignoringBody.async(BodyParsers.utils.empty)(req =>
         errorHandler.onClientError(req, status)
@@ -107,17 +107,18 @@ class DefaultHttpRequestHandler(
      * isn't explicitly routed try routing it as a GET request. Second, if no routing information is present,
      * fall back to a 404 error.
      */
-    def routeWithFallback(request: RequestHeader): EssentialAction =
+    def routeWithFallback(request: RequestHeader): Handler =
       routeRequest(request).getOrElse {
         request.method match
           // We automatically permit HEAD requests against any GETs without the need to
           // add an explicit mapping in Routes. Since we couldn't route the HEAD request,
-          // try to get an action for the equivalent GET request instead. Notes:
+          // try to get a Handler for the equivalent GET request instead. Notes:
           // 1. The handler returned will still be passed a HEAD request when it is
           //    actually evaluated.
           case HttpVerbs.HEAD =>
-            routeRequest(request.withMethod(HttpVerbs.GET)).getOrElse:
-              handleWithStatus(NOT_FOUND)
+            routeRequest(request.withMethod(HttpVerbs.GET)) match
+              case Some(handler: Handler) => handler
+              case None => handleWithStatus(NOT_FOUND)
           case _ =>
             // An Action for a 404 error
             handleWithStatus(NOT_FOUND)
@@ -128,16 +129,21 @@ class DefaultHttpRequestHandler(
     // 3. Modify the handler to do filtering, if necessary
     // 4. Again resolve any handlers that do preprocessing
     val routedHandler = routeWithFallback(request)
-    (request, filterHandler(request, routedHandler))
+    val (preprocessedRequest, preprocessedHandler) = Handler.applyStages(request, routedHandler)
+    val filteredHandler = filterHandler(preprocessedRequest, preprocessedHandler)
+    val (preprocessedPreprocessedRequest, preprocessedFilteredHandler) =
+      Handler.applyStages(preprocessedRequest, filteredHandler)
+    (preprocessedPreprocessedRequest, preprocessedFilteredHandler)
 
   /**
    * Update the given handler so that when the handler is run any filters will also be run. The default
    * behavior is to wrap all [[play.api.mvc.EssentialAction]]s by calling `filterAction`, but to leave other
    * kinds of handlers unchanged.
    */
-  protected def filterHandler(request: RequestHeader, action: EssentialAction): EssentialAction =
-    if inContext(request.path) then filterAction(action)
-    else action
+  protected def filterHandler(request: RequestHeader, handler: Handler): Handler =
+    handler match
+      case action: EssentialAction if inContext(request.path) => filterAction(action)
+      case handler => handler
 
   /**
    * Apply filters to the given action.
@@ -158,5 +164,5 @@ class DefaultHttpRequestHandler(
    * @return
    *   A handler to handle the request, if one can be found
    */
-  def routeRequest(request: RequestHeader): Option[EssentialAction] =
+  def routeRequest(request: RequestHeader): Option[Handler] =
     router().handlerFor(request)

core/play/src/main/scala/play/api/mvc/Action.scala

@@ -16,9 +16,10 @@ import scala.concurrent.*
  * An `EssentialAction` underlies every `Action`. Given a `RequestHeader`, an `EssentialAction` consumes the
  * request body (an `ByteString`) and returns a `Result`.
  *
- * An `EssentialAction` is the one that Play uses to handle requests.
+ * An `EssentialAction` is a `Handler`, which means it is one of the objects that Play uses to handle
+ * requests.
  */
-trait EssentialAction extends (RequestHeader => Accumulator[ByteString, Result]):
+trait EssentialAction extends (RequestHeader => Accumulator[ByteString, Result]) with Handler:
   self =>
 
   /** @return itself, for better support in the routes file. */

core/play/src/main/scala/play/api/mvc/Handler.scala

@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) from 2022 The Play Framework Contributors <https://github.com/playframework>, 2011-2021 Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package play.api.mvc
+
+import scala.annotation.tailrec
+
+/**
+ * An Handler handles a request. Play understands several types of handlers, for example `EssentialAction`s
+ * and `WebSocket`s.
+ *
+ * The `Handler` used to handle the request is controlled by `GlobalSetting`s's `onRequestReceived` method.
+ * The default implementation of `onRequestReceived` delegates to `onRouteRequest` which calls the default
+ * `Router`.
+ */
+trait Handler
+
+object Handler:
+
+  /**
+   * Some handlers are built as a series of stages, with each stage returning a new [[RequestHeader]] and
+   * another stage, until eventually a terminal handler is returned. This method processes all stages in a
+   * handler, if any, returning a terminal handler such as `EssentialAction` or `WebSocket`.
+   *
+   * @param requestHeader
+   *   The current RequestHeader.
+   * @param handler
+   *   The input Handler.
+   * @return
+   *   The new RequestHeader and Handler.
+   */
+  @tailrec
+  def applyStages(requestHeader: RequestHeader, handler: Handler): (RequestHeader, Handler) = handler match
+    case m: Stage =>
+      // Call the ModifyRequest logic to get the new header and handler. The
+      // new handler could have its own modifications to apply to the header
+      // so we call `applyPreprocessingHandlers` recursively on the result.
+      val (newRequestHeader, newHandler) = m.apply(requestHeader)
+      applyStages(newRequestHeader, newHandler)
+    case _ =>
+      // This is a normal handler that doesn't do any preprocessing.
+      (requestHeader, handler)
+
+  /**
+   * A special type of [[play.api.mvc.Handler]] which allows custom logic to be inserted during handling. A
+   * `Stage` accepts a `RequestHeader` then returns a new `RequestHeader` along with the next `Handler` to use
+   * during request handling. The next handler could be a terminal `Handler` like an [[EssentialAction]], but
+   * it could also be another `Stage`. This means it's possible to chains of `Stage`s that should each be
+   * executed in turn. To automatically execute all `Stage`s you can call
+   * [[play.api.mvc.Handler.applyStages]].
+   */
+  trait Stage extends Handler:
+    def apply(requestHeader: RequestHeader): (RequestHeader, Handler)
+
+  object Stage:
+
+    /**
+     * Create a `Stage` that modifies the request before calling the next handler.
+     */
+    def modifyRequest(
+        modifyRequestFunc: RequestHeader => RequestHeader,
+        wrappedHandler: Handler
+    ): Handler.Stage =
+      (requestHeader: RequestHeader) => (modifyRequestFunc(requestHeader), wrappedHandler)

core/play/src/main/scala/play/api/routing/Router.scala

@@ -5,8 +5,8 @@
 package play.api.routing
 
 import play.api.libs.typedmap.TypedKey
+import play.api.mvc.Handler
 import play.api.mvc.RequestHeader
-import play.api.mvc.EssentialAction
 import play.api.routing.Router.Routes
 
 /**
@@ -23,7 +23,7 @@ trait Router:
   /**
    * A lifted version of the routes partial function.
    */
-  final def handlerFor(request: RequestHeader): Option[EssentialAction] = routes.lift(request)
+  final def handlerFor(request: RequestHeader): Option[Handler] = routes.lift(request)
 
   /**
    * Compose two routers into one. The resulting router will contain both the routes in `this` as well as
@@ -40,7 +40,7 @@ object Router:
   /**
    * The type of the routes partial function
    */
-  type Routes = PartialFunction[RequestHeader, EssentialAction]
+  type Routes = PartialFunction[RequestHeader, Handler]
 
   /**
    * Request attributes used by the router.

core/play/src/main/scala/play/core/routing/GeneratedRouter.scala

@@ -32,7 +32,7 @@ object Route:
  * An included router
  */
 class Include(val router: Router):
-  def unapply(request: RequestHeader): Option[EssentialAction] =
+  def unapply(request: RequestHeader): Option[Handler] =
     router.routes.lift(request)
 
 /**
@@ -76,28 +76,27 @@ abstract class GeneratedRouter extends Router {
     errorHandler.onClientError(request, play.api.http.Status.BAD_REQUEST, error)
   }
 
-  def named(name: String)(generator: => EssentialAction): EssentialAction =
-    EssentialAction: (request: RequestHeader) =>
-      generator(request.addAttr(play.api.routing.Router.Attrs.ActionName, name))
+  def named(name: String)(generator: => Handler) =
+    Handler.Stage.modifyRequest(_.addAttr(play.api.routing.Router.Attrs.ActionName, name), generator)
 
-  def call(generator: => EssentialAction): EssentialAction =
+  def call(generator: => Handler): Handler =
     generator
 
-  def call[P](pa: Param[P])(generator: (P) => EssentialAction): EssentialAction =
+  def call[P](pa: Param[P])(generator: (P) => Handler): Handler =
     pa.value.fold(badRequest, generator)
 
   // Keep the old versions for avoiding compiler failures while building for Scala 2.10,
   // and for avoiding warnings when building for newer Scala versions
   // format: off
-  def call[A1, A2](pa1: Param[A1], pa2: Param[A2])(generator: Function2[A1, A2, EssentialAction]): EssentialAction = {
+  def call[A1, A2](pa1: Param[A1], pa2: Param[A2])(generator: Function2[A1, A2, Handler]): Handler = {
     (for
 a1 <- pa1.value
  a2 <- pa2.value
       yield (a1, a2))
       .fold(badRequest, { case (a1, a2) => generator(a1, a2) })
   }
 
-  def call[A1, A2, A3](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3])(generator: Function3[A1, A2, A3, EssentialAction]): EssentialAction = {
+  def call[A1, A2, A3](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3])(generator: Function3[A1, A2, A3, Handler]): Handler = {
     (for
 a1 <- pa1.value
  a2 <- pa2.value
@@ -106,7 +105,7 @@ a1 <- pa1.value
       .fold(badRequest, { case (a1, a2, a3) => generator(a1, a2, a3) })
   }
 
-  def call[A1, A2, A3, A4](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4])(generator: Function4[A1, A2, A3, A4, EssentialAction]): EssentialAction = {
+  def call[A1, A2, A3, A4](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4])(generator: Function4[A1, A2, A3, A4, Handler]): Handler = {
     (for
 a1 <- pa1.value
  a2 <- pa2.value
@@ -116,7 +115,7 @@ a1 <- pa1.value
       .fold(badRequest, { case (a1, a2, a3, a4) => generator(a1, a2, a3, a4) })
   }
 
-  def call[A1, A2, A3, A4, A5](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4], pa5: Param[A5])(generator: Function5[A1, A2, A3, A4, A5, EssentialAction]): EssentialAction = {
+  def call[A1, A2, A3, A4, A5](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4], pa5: Param[A5])(generator: Function5[A1, A2, A3, A4, A5, Handler]): Handler = {
     (for
 a1 <- pa1.value
  a2 <- pa2.value
@@ -127,7 +126,7 @@ a1 <- pa1.value
       .fold(badRequest, { case (a1, a2, a3, a4, a5) => generator(a1, a2, a3, a4, a5) })
   }
 
-  def call[A1, A2, A3, A4, A5, A6](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4], pa5: Param[A5], pa6: Param[A6])(generator: Function6[A1, A2, A3, A4, A5, A6, EssentialAction]): EssentialAction = {
+  def call[A1, A2, A3, A4, A5, A6](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4], pa5: Param[A5], pa6: Param[A6])(generator: Function6[A1, A2, A3, A4, A5, A6, Handler]): Handler = {
     (for
 a1 <- pa1.value
  a2 <- pa2.value
@@ -139,9 +138,35 @@ a1 <- pa1.value
       .fold(badRequest, { case (a1, a2, a3, a4, a5, a6) => generator(a1, a2, a3, a4, a5, a6) })
   }
 
+  def call[A1, A2, A3, A4, A5, A6, A7](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4], pa5: Param[A5], pa6: Param[A6], pa7: Param[A7])(generator: Function7[A1, A2, A3, A4, A5, A6, A7, Handler]): Handler = {
+    (for
+a1 <- pa1.value
+ a2 <- pa2.value
+ a3 <- pa3.value
+ a4 <- pa4.value
+ a5 <- pa5.value
+ a6 <- pa6.value
+ a7 <- pa7.value
+      yield (a1, a2, a3, a4, a5, a6, a7))
+      .fold(badRequest, { case (a1, a2, a3, a4, a5, a6, a7) => generator(a1, a2, a3, a4, a5, a6, a7) })
+  }
+
+  def call[A1, A2, A3, A4, A5, A6, A7, A8](pa1: Param[A1], pa2: Param[A2], pa3: Param[A3], pa4: Param[A4], pa5: Param[A5], pa6: Param[A6], pa7: Param[A7], pa8: Param[A8])(generator: Function8[A1, A2, A3, A4, A5, A6, A7, A8, Handler]): Handler = {
+    (for
+a1 <- pa1.value
+ a2 <- pa2.value
+ a3 <- pa3.value
+ a4 <- pa4.value
+ a5 <- pa5.value
+ a6 <- pa6.value
+ a7 <- pa7.value
+ a8 <- pa8.value
+      yield (a1, a2, a3, a4, a5, a6, a7, a8))
+      .fold(badRequest, { case (a1, a2, a3, a4, a5, a6, a7, a8) => generator(a1, a2, a3, a4, a5, a6, a7, a8) })
+  }
   // format: on
 
-  def call[T](params: List[Param[?]])(generator: (Seq[?]) => EssentialAction): EssentialAction =
+  def call[T](params: List[Param[?]])(generator: (Seq[?]) => Handler): Handler =
     (params
       .foldLeft[Either[String, Seq[?]]](Right(Seq[T]())) { (seq, param) =>
         seq.flatMap(s => param.value.map(s :+ _))

dev-mode/routes-compiler/src/main/twirl/play/routes/compiler/inject/forwardsRouter.scala.twirl

@@ -35,7 +35,7 @@ case include @ Include(path, router) => {
   private val prefixed_@(dep.ident)_@(pathIndex * 1000 + index) = Include(@(dep.ident))
 }}}}
 
-  val pathRouters: Map[String, PartialFunction[RequestHeader, EssentialAction]] = Map(
+  val pathRouters: Map[String, PartialFunction[RequestHeader, Handler]] = Map(
     @for(((path, rules), pathIndex) <- pathRules.zipWithIndex) {
       "@path" -> @ob
         @for((dep, index) <- rules.zipWithIndex){@dep.rule match {
@@ -57,7 +57,7 @@ case include @ Include(path, router) => {
     }
   )
 
-  val routes: PartialFunction[RequestHeader, EssentialAction] =
+  val routes: PartialFunction[RequestHeader, Handler] =
     @if(pathRules.isEmpty) { Map.empty } else {@ob
       val emptyPathRouter = pathRouters get ""
       Function unlift @ob (req: RequestHeader) =>

dev-mode/routes-compiler/src/test/resources/snapshot/routes/lila-generated.txt

@@ -1078,7 +1078,7 @@ final class Routes(
   )
 
 
-  val pathRouters: Map[String, PartialFunction[RequestHeader, EssentialAction]] = Map(
+  val pathRouters: Map[String, PartialFunction[RequestHeader, Handler]] = Map(
 
       "" -> {
 
@@ -2774,7 +2774,7 @@ final class Routes(
 
   )
 
-  val routes: PartialFunction[RequestHeader, EssentialAction] =
+  val routes: PartialFunction[RequestHeader, Handler] =
     {
       val emptyPathRouter = pathRouters get ""
       Function unlift { (req: RequestHeader) =>

transport/server/play-netty-server/src/main/scala/play/core/server/netty/PlayRequestHandler.scala

@@ -57,7 +57,7 @@ private[play] class PlayRequestHandler(
 
     val tryRequest: Try[RequestHeader] = modelConversion.convertRequest(channel, request)
 
-    def clientError(statusCode: Int, message: String): (RequestHeader, EssentialAction) =
+    def clientError(statusCode: Int, message: String): (RequestHeader, Handler) =
       val unparsedTarget = modelConversion.createUnparsedRequestTarget(request)
       val requestHeader = modelConversion.createRequestHeader(channel, request, unparsedTarget)
       val result = app.errorHandler.onClientError(
@@ -68,7 +68,7 @@ private[play] class PlayRequestHandler(
       // If there's a problem in parsing the request, then we should close the connection, once done with it
       requestHeader -> Server.actionForResult(result.map(_.withHeaders(HeaderNames.CONNECTION -> "close")))
 
-    val (requestHeader, handler): (RequestHeader, EssentialAction) = tryRequest match
+    val (requestHeader, handler): (RequestHeader, Handler) = tryRequest match
       case Failure(exception: TooLongFrameException) =>
         clientError(Status.REQUEST_URI_TOO_LONG, exception.getMessage)
       case Failure(exception) => clientError(Status.BAD_REQUEST, exception.getMessage)
@@ -80,7 +80,15 @@ private[play] class PlayRequestHandler(
         then clientError(Status.REQUEST_ENTITY_TOO_LARGE, "Request Entity Too Large")
         else Server.getHandlerFor(req, app)
 
-    handleAction(handler, requestHeader, request)
+    handler match
+      // execute normal action
+      case action: EssentialAction => handleAction(action, requestHeader, request)
+
+      // This case usually indicates an error in Play's internal routing or handling logic
+      case h =>
+        val ex = new IllegalStateException(s"Netty server doesn't handle Handlers of this type: $h")
+        logger.error(ex.getMessage, ex)
+        throw ex
 
   // ----------------------------------------------------------------
   // Netty overrides
@@ -169,7 +177,7 @@ private[play] class PlayRequestHandler(
       requestHeader: RequestHeader,
       request: HttpRequest
   ): Future[HttpResponse] =
-    given mat: Materializer = app.materializer
+    implicit val mat: Materializer = app.materializer
     import play.core.Execution.Implicits.trampoline
 
     // Execute the action on the Play default execution context