Move recursive Decorator-processing logic into `Decorator.invoke`, document what it does

Avoid reversing annotations list in Router.scala only to reverse the final results again when binding the arguments to `EntryPoint.invoke`

4 files changed, 63 insertions, 41 deletions

diff --git a/cask/src/cask/main/Decorators.scala b/cask/src/cask/main/Decorators.scala
index 573c139..0a68cba 100644
--- a/cask/src/cask/main/Decorators.scala
+++ b/cask/src/cask/main/Decorators.scala
@@ -1,7 +1,7 @@
 package cask.main
 
 import cask.internal.{Conversion, Router}
-import cask.internal.Router.ArgReader
+import cask.internal.Router.{ArgReader, EntryPoint}
 import cask.model.{Request, Response}
 
 /**
@@ -23,6 +23,48 @@ trait Decorator[InnerReturned, Input]{
   def wrapFunction(ctx: Request, delegate: Delegate): OuterReturned
   def getParamParser[T](implicit p: InputParser[T]) = p
 }
+object Decorator{
+  /**
+   * A stack of [[Decorator]]s is invoked recursively: each decorator's `wrapFunction`
+   * is invoked around the invocation of all inner decorators, with the inner-most
+   * decorator finally invoking the route's [[EntryPoint.invoke]] function.
+   *
+   * Each decorator (and the final `Endpoint`) contributes a dictionary of name-value
+   * bindings, which are eventually all passed to [[EntryPoint.invoke]]. Each decorator's
+   * dictionary corresponds to a different argument list on [[EntryPoint.invoke]]. The
+   * bindings passed from the router are aggregated with those from the `EndPoint` and
+   * used as the first argument list.
+   */
+  def invoke(ctx: Request,
+             metadata: Routes.EndpointMetadata[_],
+             routes: Routes,
+             routeBindings: Map[String, String],
+             remainingDecorators: List[RawDecorator],
+             bindings: List[Map[String, Any]]): Router.Result[Any] = try {
+    remainingDecorators match {
+      case head :: rest =>
+        head.wrapFunction(
+          ctx,
+          args => invoke(ctx, metadata, routes, routeBindings, rest, args :: bindings)
+            .asInstanceOf[Router.Result[cask.model.Response.Raw]]
+        )
+
+      case Nil =>
+        metadata.endpoint.wrapFunction(ctx, { (endpointBindings: Map[String, Any]) =>
+          val mergedEndpointBindings = endpointBindings ++ routeBindings.mapValues(metadata.endpoint.wrapPathSegment)
+          val finalBindings = mergedEndpointBindings :: bindings
+
+          metadata.entryPoint
+            .asInstanceOf[EntryPoint[cask.main.Routes, cask.model.Request]]
+            .invoke(routes, ctx, finalBindings)
+            .asInstanceOf[Router.Result[Nothing]]
+        })
+    }
+    // Make sure we wrap any exceptions that bubble up from decorator
+    // bodies, so outer decorators do not need to worry about their
+    // delegate throwing on them
+  }catch{case e: Throwable => Router.Result.Error.Exception(e) }
+}
 
 /**
   * A [[RawDecorator]] is a decorator that operates on the raw request and
diff --git a/cask/src/cask/main/Main.scala b/cask/src/cask/main/Main.scala
index a138e8a..fbcbff9 100644
--- a/cask/src/cask/main/Main.scala
+++ b/cask/src/cask/main/Main.scala
@@ -89,35 +89,14 @@ abstract class BaseMain{
         routeTries(effectiveMethod).lookup(Util.splitPath(exchange.getRequestPath).toList, Map()) match {
           case None => writeResponse(exchange, handleNotFound())
           case Some(((routes, metadata), routeBindings, remaining)) =>
-            val ctx = Request(exchange, remaining)
-            def rec(remaining: List[RawDecorator],
-                    bindings: List[Map[String, Any]]): Router.Result[Any] = try {
-              remaining match {
-                case head :: rest =>
-                  head.wrapFunction(
-                    ctx,
-                    args => rec(rest, args :: bindings)
-                      .asInstanceOf[cask.internal.Router.Result[cask.model.Response.Raw]]
-                  )
-
-                case Nil =>
-                  metadata.endpoint.wrapFunction(ctx, (endpointBindings: Map[String, Any]) =>
-                    metadata.entryPoint
-                      .asInstanceOf[EntryPoint[cask.main.Routes, cask.model.Request]]
-                      .invoke(
-                        routes, ctx,
-                        (endpointBindings ++ routeBindings.mapValues(metadata.endpoint.wrapPathSegment))
-                        :: bindings.reverse
-                      )
-                      .asInstanceOf[Router.Result[Nothing]]
-                  )
-              }
-              // Make sure we wrap any exceptions that bubble up from decorator
-              // bodies, so outer decorators do not need to worry about their
-              // delegate throwing on them
-            }catch{case e: Throwable => Router.Result.Error.Exception(e) }
-
-            rec((metadata.decorators ++ routes.decorators ++ mainDecorators).toList, Nil)match{
+            Decorator.invoke(
+              Request(exchange, remaining),
+              metadata,
+              routes,
+              routeBindings,
+              (mainDecorators ++ routes.decorators ++ metadata.decorators).toList,
+              Nil
+            ) match{
               case Router.Result.Success(res) => runner(res)
               case e: Router.Result.Error =>
                 writeResponse(
diff --git a/cask/src/cask/main/Routes.scala b/cask/src/cask/main/Routes.scala
index f133fdb..a6bdd65 100644
--- a/cask/src/cask/main/Routes.scala
+++ b/cask/src/cask/main/Routes.scala
@@ -1,7 +1,6 @@
 package cask.main
 
 import cask.internal.Router.EntryPoint
-import cask.model.Request
 
 import scala.reflect.macros.blackbox.Context
 import language.experimental.macros
@@ -19,33 +18,36 @@ object Routes{
 
       val routeParts = for{
         m <- c.weakTypeOf[T].members
-        val annotations = m.annotations.filter(_.tree.tpe <:< c.weakTypeOf[Decorator[_, _]]).reverse
+        annotations = m.annotations.filter(_.tree.tpe <:< c.weakTypeOf[Decorator[_, _]])
         if annotations.nonEmpty
       } yield {
-        if(!(annotations.head.tree.tpe <:< weakTypeOf[Endpoint[_, _]])) c.abort(
+        if(!(annotations.last.tree.tpe <:< weakTypeOf[Endpoint[_, _]])) c.abort(
           annotations.head.tree.pos,
           s"Last annotation applied to a function must be an instance of Endpoint, " +
-          s"not ${annotations.head.tree.tpe}"
+          s"not ${annotations.last.tree.tpe}"
         )
         val allEndpoints = annotations.filter(_.tree.tpe <:< weakTypeOf[Endpoint[_, _]])
         if(allEndpoints.length > 1) c.abort(
-          annotations.head.tree.pos,
+          annotations.last.tree.pos,
           s"You can only apply one Endpoint annotation to a function, not " +
           s"${allEndpoints.length} in ${allEndpoints.map(_.tree.tpe).mkString(", ")}"
         )
+
         val annotObjects =
           for(annot <- annotations)
           yield q"new ${annot.tree.tpe}(..${annot.tree.children.tail})"
+
         val annotObjectSyms =
           for(_ <- annotations.indices)
           yield c.universe.TermName(c.freshName("annotObject"))
+
         val route = router.extractMethod(
           m.asInstanceOf[MethodSymbol],
           weakTypeOf[T],
-          q"${annotObjectSyms.head}.convertToResultType",
+          q"${annotObjectSyms.last}.convertToResultType",
           tq"cask.Request",
-          annotObjectSyms.map(annotObjectSym => q"$annotObjectSym.getParamParser"),
-          annotObjectSyms.map(annotObjectSym => tq"$annotObjectSym.InputTypeAlias")
+          annotObjectSyms.reverse.map(annotObjectSym => q"$annotObjectSym.getParamParser"),
+          annotObjectSyms.reverse.map(annotObjectSym => tq"$annotObjectSym.InputTypeAlias")
         )
 
         val declarations =
@@ -55,8 +57,8 @@ object Routes{
         val res = q"""{
           ..$declarations
           cask.main.Routes.EndpointMetadata(
-            Seq(..${annotObjectSyms.drop(1)}),
-            ${annotObjectSyms.head},
+            Seq(..${annotObjectSyms.dropRight(1)}),
+            ${annotObjectSyms.last},
             $route
           )
         }"""
diff --git a/cask/test/src/test/cask/FailureTests.scala b/cask/test/src/test/cask/FailureTests.scala
index 6b01ec1..25530fa 100644
--- a/cask/test/src/test/cask/FailureTests.scala
+++ b/cask/test/src/test/cask/FailureTests.scala
@@ -33,4 +33,3 @@ object FailureTests extends TestSuite {
     }
   }
 }
-