Reverse router aggregation

Fixes playframework#1390.

* Implemented reverse router aggregation
* Replaced routesFiles task with sources in routes task
* Added scripted test for testing router aggregation
* Added support for compiling/including .sbt code snippets in the
  documentation

documentation/manual/detailedTopics/build/AggregatingReverseRouters.md

@@ -0,0 +1,13 @@
+# Aggregating reverse routers
+
+In some situations you want to share reverse routers between sub projects that are not dependent on each other.
+
+For example, you might have a `web` sub project, and an `api` sub project.  These sub projects may have no dependence on each other, except that the `web` project wants to render links to the `api` project (for making AJAX calls), while the `api` project wants to render links to the `web` (rendering the web link for a resource in JSON).  In this situation, it would be convenient to use the reverse router, but since these projects don't depend on each other, you can't.
+
+Play's routes compiler offers a feature that allows a common dependency to generate the reverse routers for projects that depend on it so that the reverse routers can be shared between those projects.  This is configured using the `aggregateReverseRoutes` sbt configuration item, like this:
+
+@[content](code/aggregate.sbt)
+
+In this setup, the reverse routers for `api` and `web` will be generated as part of the `common` project.  Meanwhile, the forwards routers for `api` and `web` will still generate forwards routers, but not reverse routers, because their reverse routers have already been generated in the `common` project which they depend on, so they don't need to generate them.
+
+> Note that the `common` project has a type of `Project` explicitly declared.  This is because there is a recursive reference between it and the `api` and `web` projects, through the `dependsOn` method and `aggregateReverseRoutes` setting, so the Scala type checker needs an explicit type somewhere in the chain of recursion.

documentation/manual/detailedTopics/build/Build.md

@@ -22,19 +22,7 @@ For now, we are going to concern ourselves with the `/build.sbt` file and the `/
 
 When you use the `activator new foo` command, the build description file, `/build.sbt`, will be generated like this:
 
-```scala
-name := "foo"
-
-version := "1.0-SNAPSHOT"
-
-libraryDependencies ++= Seq(
-  jdbc,
-  anorm,
-  cache
-)
-
-lazy val root = (project in file(".")).enablePlugins(PlayScala)
-```
+@[default](code/build.sbt)
 
 The `name` line defines the name of your application and it will be the same as the name of your application's root directory, `/`, which is derived from the argument that you gave to the `activator new` command. 
 

documentation/manual/detailedTopics/build/code/aggregate.sbt

@@ -0,0 +1,19 @@
+//#content
+lazy val common: Project = (project in file("common"))
+  .enablePlugins(PlayScala)
+  .settings(
+    aggregateReverseRoutes := Seq(api, web)
+  )
+
+lazy val api = (project in file("api"))
+  .enablePlugins(PlayScala)
+  .dependsOn(common)
+
+lazy val web = (project in file("web"))
+  .enablePlugins(PlayScala)
+  .dependsOn(common)
+//#content
+
+lazy val root = (project in file("."))
+  .enablePlugins(PlayScala)
+  .dependsOn(api, web)

documentation/manual/detailedTopics/build/code/build.sbt

@@ -0,0 +1,13 @@
+//#default
+name := "foo"
+
+version := "1.0-SNAPSHOT"
+
+libraryDependencies ++= Seq(
+  jdbc,
+  anorm,
+  cache
+)
+
+lazy val root = (project in file(".")).enablePlugins(PlayScala)
+//#default

documentation/manual/detailedTopics/build/index.toc

@@ -2,5 +2,6 @@ Build:The Build System
 SBTSettings:About sbt settings
 SBTDependencies:Manage application dependencies
 SBTSubProjects:Working with sub-projects
+AggregatingReverseRouters:Aggregating reverse routers
 CompilationSpeed:Improving Compilation Times
 SBTCookbook:Cookbook

documentation/project/Build.scala

@@ -70,7 +70,6 @@ object ApplicationBuild extends Build {
     scalaVersion := PlayVersion.scalaVersion,
 
     fork in Test := true
-
   ).settings(externalPlayModuleSettings:_*)
    .dependsOn(
       playDocs,

framework/runtests

@@ -48,7 +48,7 @@ echo "[info] ---- RUNNING DOCUMENTATION TESTS"
 echo "[info]"
 
 cd ../documentation
-$BUILD "$@" clean ${CROSSBUILD}test validate-docs
+$BUILD "$@" clean ${CROSSBUILD}test evaluateSbtFiles validate-docs
 cd $CURRENT
 
 echo "[info]"

framework/src/play-docs-sbt-plugin/src/main/scala/com/typesafe/play/docs/sbtplugin/PlayDocsPlugin.scala

@@ -9,16 +9,15 @@ import java.util.jar.JarFile
 import com.typesafe.play.docs.sbtplugin.PlayDocsValidation.{ CodeSamplesReport, MarkdownRefReport }
 import play.core.BuildDocHandler
 import play.core.server.ServerWithStop
-import play.sbt.{ PlayImport, Colors }
-import PlayImport._
+import play.routes.compiler.RoutesCompiler.RoutesCompilerTask
 import play.TemplateImports
-import play.routes.compiler.InjectedRoutesGenerator
 import play.sbt.Colors
 import play.sbt.routes.RoutesCompiler
+import play.sbt.routes.RoutesKeys._
 import sbt._
 import sbt.Keys._
-import sbt.plugins.JvmPlugin
 import scala.collection.JavaConverters._
+import scala.util.control.NonFatal
 
 object Imports {
   object PlayDocsKeys {
@@ -40,10 +39,10 @@ object Imports {
 
     val javaManualSourceDirectories = SettingKey[Seq[File]]("java-manual-source-directories")
     val scalaManualSourceDirectories = SettingKey[Seq[File]]("scala-manual-source-directories")
-    val javaRoutesSourceManaged = SettingKey[File]("java-routes-source-managed")
-    val scalaRoutesSourceManaged = SettingKey[File]("scala-routes-source-managed")
     val javaTwirlSourceManaged = SettingKey[File]("java-routes-source-managed")
     val scalaTwirlSourceManaged = SettingKey[File]("scala-routes-source-managed")
+
+    val evaluateSbtFiles = TaskKey[Unit]("evaluateSbtFiles", "Evaluate all the sbt files in the project")
   }
 }
 
@@ -55,7 +54,7 @@ object PlayDocsPlugin extends AutoPlugin {
 
   override def trigger = NoTrigger
 
-  override def requires = JvmPlugin
+  override def requires = RoutesCompiler
 
   override def projectSettings = docsRunSettings ++ docsReportSettings ++ docsTestSettings
 
@@ -90,13 +89,9 @@ object PlayDocsPlugin extends AutoPlugin {
     unmanagedSourceDirectories in Test ++= javaManualSourceDirectories.value ++ scalaManualSourceDirectories.value,
     unmanagedResourceDirectories in Test ++= javaManualSourceDirectories.value ++ scalaManualSourceDirectories.value,
 
-    javaRoutesSourceManaged := target.value / "routes" / "java",
-    scalaRoutesSourceManaged := target.value / "routes" / "scala",
     javaTwirlSourceManaged := target.value / "twirl" / "java",
     scalaTwirlSourceManaged := target.value / "twirl" / "scala",
     managedSourceDirectories in Test ++= Seq(
-      javaRoutesSourceManaged.value,
-      scalaRoutesSourceManaged.value,
       javaTwirlSourceManaged.value,
       scalaTwirlSourceManaged.value
     ),
@@ -110,12 +105,39 @@ object PlayDocsPlugin extends AutoPlugin {
       compileTemplates(from, to, TemplateImports.defaultScalaTemplateImports.asScala, s.log)
     },
 
-    sourceGenerators in Test <+= (javaManualSourceDirectories, javaRoutesSourceManaged, streams) map { (from, to, s) =>
-      RoutesCompiler.compileRoutes((from * "*.routes").get, InjectedRoutesGenerator, to, Seq("play.libs.F"), true, true, s.cacheDirectory / "javaroutes", s.log)
+    routesCompilerTasks in Test := {
+      val javaRoutes = (javaManualSourceDirectories.value * "*.routes").get
+      val scalaRoutes = (scalaManualSourceDirectories.value * "*.routes").get
+      (javaRoutes.map(_ -> Seq("play.libs.F")) ++ scalaRoutes.map(_ -> Nil)).map {
+        case (file, imports) => RoutesCompilerTask(file, imports, true, true, true)
+      }
     },
 
-    sourceGenerators in Test <+= (scalaManualSourceDirectories, scalaRoutesSourceManaged, streams) map { (from, to, s) =>
-      RoutesCompiler.compileRoutes((from * "*.routes").get, InjectedRoutesGenerator, to, Nil, true, true, s.cacheDirectory / "scalaroutes", s.log)
+    routesGenerator := InjectedRoutesGenerator,
+
+    evaluateSbtFiles := {
+      val unit = loadedBuild.value.units(thisProjectRef.value.build)
+      val (eval, structure) = Load.defaultLoad(state.value, unit.localBase, state.value.log)
+      val sbtFiles = ((unmanagedSourceDirectories in Test).value * "*.sbt").get
+      val log = state.value.log
+      if (sbtFiles.nonEmpty) {
+        log.info("Testing .sbt files...")
+      }
+      val result = sbtFiles.map { sbtFile =>
+        val relativeFile = relativeTo(baseDirectory.value)(sbtFile).getOrElse(sbtFile.getAbsolutePath)
+        try {
+          EvaluateConfigurations.evaluateConfiguration(eval(), sbtFile, unit.imports)(unit.loader)
+          log.info(s"  ${Colors.green("+")} $relativeFile")
+          true
+        } catch {
+          case NonFatal(_) =>
+            log.error(s" ${Colors.yellow("x")} $relativeFile")
+            false
+        }
+      }
+      if (result.contains(false)) {
+        throw new TestsFailedException
+      }
     },
 
     parallelExecution in Test := false,

framework/src/routes-compiler/src/main/scala/play/routes/compiler/RoutesCompiler.scala

@@ -58,27 +58,34 @@ object RoutesCompiler {
 
   }
 
+  /**
+   * A routes compiler task.
+   *
+   * @param file The routes file to compile.
+   * @param additionalImports The additional imports.
+   * @param forwardsRouter Whether a forwards router should be generated.
+   * @param reverseRouter Whether a reverse router should be generated.
+   * @param namespaceReverseRouter Whether the reverse router should be namespaced.
+   */
+  case class RoutesCompilerTask(file: File, additionalImports: Seq[String], forwardsRouter: Boolean, reverseRouter: Boolean, namespaceReverseRouter: Boolean)
+
   /**
    * Compile the given routes file
    *
-   * @param file The routes file to compile
+   * @param task The routes compilation task
+   * @param generator The routes generator
    * @param generatedDir The directory to place the generated source code in
-   * @param additionalImports Additional imports to add to the output files
-   * @param generateReverseRouter Whether the reverse router should be generated
-   * @param namespaceReverseRouter Whether the reverse router should be namespaced
    * @return Either the list of files that were generated (right) or the routes compilation errors (left)
    */
-  def compile(file: File, generator: RoutesGenerator, generatedDir: File, additionalImports: Seq[String], generateReverseRouter: Boolean = true,
-    namespaceReverseRouter: Boolean = false): Either[Seq[RoutesCompilationError], Seq[File]] = {
+  def compile(task: RoutesCompilerTask, generator: RoutesGenerator, generatedDir: File): Either[Seq[RoutesCompilationError], Seq[File]] = {
 
-    val namespace = Option(file.getName).filter(_.endsWith(".routes")).map(_.dropRight(".routes".size))
+    val namespace = Option(task.file.getName).filter(_.endsWith(".routes")).map(_.dropRight(".routes".size))
       .orElse(Some("router"))
 
-    val routeFile = file.getAbsoluteFile
+    val routeFile = task.file.getAbsoluteFile
 
     RoutesFileParser.parse(routeFile).right.map { rules =>
-      val generated = generator.generate(routeFile, namespace, rules, additionalImports, generateReverseRouter,
-        namespaceReverseRouter)
+      val generated = generator.generate(task, namespace, rules)
       generated.map {
         case (filename, content) =>
           val file = new File(generatedDir, filename)

framework/src/routes-compiler/src/main/scala/play/routes/compiler/RoutesGenerator.scala

@@ -5,20 +5,18 @@ package play.routes.compiler
 
 import java.io.File
 
+import play.routes.compiler.RoutesCompiler.RoutesCompilerTask
+
 trait RoutesGenerator {
   /**
    * Generate a router
    *
-   * @param file The routes file that it's being generated for
+   * @param task The routes compile task
    * @param namespace The namespace of the router
    * @param rules The routing rules
-   * @param additionalImports Any additional imports
-   * @param reverseRouter Whether a reverse router should be generated
-   * @param namespaceReverseRouter Whether the reverse router should be namespaced
    * @return A sequence of output filenames to file contents
    */
-  def generate(file: File, namespace: Option[String], rules: List[Rule], additionalImports: Seq[String],
-    reverseRouter: Boolean, namespaceReverseRouter: Boolean): Seq[(String, String)]
+  def generate(task: RoutesCompilerTask, namespace: Option[String], rules: List[Rule]): Seq[(String, String)]
 
   /**
    * An identifier for this routes generator.
@@ -38,21 +36,28 @@ object StaticRoutesGenerator extends RoutesGenerator {
 
   val id = "static"
 
-  def generate(file: File, namespace: Option[String], rules: List[Rule], additionalImports: Seq[String],
-    reverseRouter: Boolean, namespaceReverseRouter: Boolean): Seq[(String, String)] = {
+  def generate(task: RoutesCompilerTask, namespace: Option[String], rules: List[Rule]): Seq[(String, String)] = {
 
     val filePrefix = namespace.map(_.replace('.', '/') + "/").getOrElse("") + "/routes"
 
-    val sourceInfo = RoutesSourceInfo(file.getCanonicalPath.replace(File.separator, "/"), new java.util.Date().toString)
+    val sourceInfo = RoutesSourceInfo(task.file.getCanonicalPath.replace(File.separator, "/"), new java.util.Date().toString)
     val routes = rules.collect { case r: Route => r }
 
-    val files = Seq(filePrefix + "_routing.scala" -> generateRouter(sourceInfo, namespace, additionalImports, rules))
-    if (reverseRouter) {
-      (files :+ filePrefix + "_reverseRouting.scala" -> generateReverseRouter(sourceInfo, namespace, additionalImports, routes, namespaceReverseRouter)) ++
-        generateJavaWrappers(sourceInfo, namespace, rules, namespaceReverseRouter)
+    val forwardsRoutesFiles = if (task.forwardsRouter) {
+      Seq(filePrefix + "_routing.scala" -> generateRouter(sourceInfo, namespace, task.additionalImports, rules))
+    } else {
+      Nil
+    }
+
+    val reverseRoutesFiles = if (task.reverseRouter) {
+      Seq(filePrefix + "_reverseRouting.scala" -> generateReverseRouter(sourceInfo, namespace, task.additionalImports,
+        routes, task.namespaceReverseRouter)) ++
+        generateJavaWrappers(sourceInfo, namespace, rules, task.namespaceReverseRouter)
     } else {
-      files
+      Nil
     }
+
+    forwardsRoutesFiles ++ reverseRoutesFiles
   }
 
   private def generateRouter(sourceInfo: RoutesSourceInfo, namespace: Option[String], additionalImports: Seq[String], rules: List[Rule]) =
@@ -94,21 +99,26 @@ object InjectedRoutesGenerator extends RoutesGenerator {
 
   case class Dependency[+T <: Rule](ident: String, clazz: String, rule: T)
 
-  def generate(file: File, namespace: Option[String], rules: List[Rule], additionalImports: Seq[String],
-    reverseRouter: Boolean, namespaceReverseRouter: Boolean): Seq[(String, String)] = {
+  def generate(task: RoutesCompilerTask, namespace: Option[String], rules: List[Rule]): Seq[(String, String)] = {
 
     val filePrefix = namespace.map(_.replace('.', '/') + "/").getOrElse("") + "/routes"
 
-    val sourceInfo = RoutesSourceInfo(file.getCanonicalPath.replace(File.separator, "/"), new java.util.Date().toString)
+    val sourceInfo = RoutesSourceInfo(task.file.getCanonicalPath.replace(File.separator, "/"), new java.util.Date().toString)
     val routes = rules.collect { case r: Route => r }
 
-    val files = Seq(filePrefix + "_routing.scala" -> generateRouter(sourceInfo, namespace, additionalImports, rules))
-    if (reverseRouter) {
-      (files :+ filePrefix + "_reverseRouting.scala" -> generateReverseRouter(sourceInfo, namespace, additionalImports, routes, namespaceReverseRouter)) ++
-        generateJavaWrappers(sourceInfo, namespace, rules, namespaceReverseRouter)
+    val forwardsRoutesFiles = if (task.forwardsRouter) {
+      Seq(filePrefix + "_routing.scala" -> generateRouter(sourceInfo, namespace, task.additionalImports, rules))
+    } else Nil
+
+    val reverseRoutesFiles = if (task.reverseRouter) {
+      Seq(filePrefix + "_reverseRouting.scala" -> generateReverseRouter(sourceInfo, namespace, task.additionalImports,
+        routes, task.namespaceReverseRouter)) ++
+        generateJavaWrappers(sourceInfo, namespace, rules, task.namespaceReverseRouter)
     } else {
-      files
+      Nil
     }
+
+    forwardsRoutesFiles ++ reverseRoutesFiles
   }
 
   private def generateRouter(sourceInfo: RoutesSourceInfo, namespace: Option[String], additionalImports: Seq[String], rules: List[Rule]) = {

framework/src/routes-compiler/src/test/scala/play/routes/compiler/RoutesCompilerSpec.scala

@@ -6,8 +6,7 @@ package play.routes.compiler
 import java.io.File
 
 import org.specs2.mutable.Specification
-
-import scala.io.Source
+import play.routes.compiler.RoutesCompiler.RoutesCompilerTask
 
 object RoutesCompilerSpec extends Specification {
 
@@ -34,7 +33,7 @@ object RoutesCompilerSpec extends Specification {
 
     "generate routes classes for route definitions that pass the checks" in withTempDir { tmp =>
       val file = new File(this.getClass.getClassLoader.getResource("generating.routes").toURI)
-      RoutesCompiler.compile(file, StaticRoutesGenerator, tmp, Seq())
+      RoutesCompiler.compile(RoutesCompilerTask(file, Seq.empty, true, true, false), StaticRoutesGenerator, tmp)
 
       val generatedRoutes = new File(tmp, "generating/routes_routing.scala")
       generatedRoutes.exists() must beTrue
@@ -45,15 +44,12 @@ object RoutesCompilerSpec extends Specification {
 
     "check if there are no routes using overloaded handler methods" in withTempDir { tmp =>
       val file = new File(this.getClass.getClassLoader.getResource("duplicateHandlers.routes").toURI)
-      RoutesCompiler.compile(file, StaticRoutesGenerator, tmp, Seq.empty) must beLeft
+      RoutesCompiler.compile(RoutesCompilerTask(file, Seq.empty, true, true, false), StaticRoutesGenerator, tmp) must beLeft
     }
 
     "check if routes with type projection are compiled" in withTempDir { tmp =>
       val file = new File(this.getClass.getClassLoader.getResource("complexTypes.routes").toURI)
-      object A {
-        type B = Int
-      }
-      RoutesCompiler.compile(file, StaticRoutesGenerator, tmp, Seq.empty) must beRight
+      RoutesCompiler.compile(RoutesCompilerTask(file, Seq.empty, true, true, false), StaticRoutesGenerator, tmp) must beRight
     }
   }
 }