Merge pull request #10171 from playframework/mergify/bp/2.8.x/pr-10153

Correctly handle "_root_." prefix in routes file (needed for namespaceReverseRouter) (bp #10153)

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

@@ -68,15 +68,17 @@ object HandlerInvokerFactory {
 
   private def loadJavaControllerClass(handlerDef: HandlerDef): Class[_] = {
     try {
-      handlerDef.classLoader.loadClass(handlerDef.controller)
+      handlerDef.classLoader.loadClass(handlerDef.controller.stripPrefix("_root_."))
     } catch {
       case e: ClassNotFoundException =>
         // Try looking up relative to the routers package name.
         // This was primarily implemented for the documentation project so that routers could be namespaced and so
         // they could reference controllers relative to their own package.
         if (handlerDef.routerPackage.length > 0) {
           try {
-            handlerDef.classLoader.loadClass(handlerDef.routerPackage + "." + handlerDef.controller)
+            handlerDef.classLoader.loadClass(
+              handlerDef.routerPackage + "." + handlerDef.controller.stripPrefix("_root_.")
+            )
           } catch {
             case NonFatal(_) => throw e
           }

dev-mode/routes-compiler/src/main/scala/play/routes/compiler/RoutesGenerator.scala

@@ -181,7 +181,7 @@ object InjectedRoutesGenerator extends RoutesGenerator {
       routes: List[Route],
       namespaceReverseRouter: Boolean
   ) = {
-    routes.groupBy(_.call.packageName).map {
+    routes.groupBy(_.call.packageName.map(_.stripPrefix("_root_."))).map {
       case (pn, routes) =>
         val packageName = namespace
           .filter(_ => namespaceReverseRouter)
@@ -209,7 +209,7 @@ object InjectedRoutesGenerator extends RoutesGenerator {
       routes: List[Route],
       namespaceReverseRouter: Boolean
   ) = {
-    routes.groupBy(_.call.packageName).map {
+    routes.groupBy(_.call.packageName.map(_.stripPrefix("_root_."))).map {
       case (pn, routes) =>
         val packageName = namespace
           .filter(_ => namespaceReverseRouter)
@@ -236,7 +236,7 @@ object InjectedRoutesGenerator extends RoutesGenerator {
       rules: List[Rule],
       namespaceReverseRouter: Boolean
   ) = {
-    rules.collect { case r: Route => r }.groupBy(_.call.packageName).map {
+    rules.collect { case r: Route => r }.groupBy(_.call.packageName.map(_.stripPrefix("_root_."))).map {
       case (pn, routes) =>
         val packageName = namespace
           .filter(_ => namespaceReverseRouter)

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-injected-routes-compilation/app/utils/JavaScriptRouterGenerator.scala

@@ -17,6 +17,7 @@ object JavaScriptRouterGenerator extends App {
       "jsRoutes",
       None,
       host,
+      Assets.versioned,
       Application.index,
       Application.post,
       Application.withParam,

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-injected-routes-compilation/conf/routes

@@ -72,7 +72,8 @@ GET         /routestest             controllers.Application.routetest(yield)
 # Test for default values for scala keywords
 GET         /routesdefault          controllers.Application.routedefault(type ?= "x")
 
-GET         /public/*file           controllers.Assets.versioned(path="/public", file: controllers.Assets.Asset)
+# Let's prefix with "_root_." (should not make any difference however because namespaceReverseRouter is not set in build.sbt)
+GET         /public/*file           _root_.controllers.Assets.versioned(path="/public", file: controllers.Assets.Asset)
 
 # This triggers a string interpolation warning, since there is an identifier called "routes" in scope, and it
 # generates a non interpolated string containing $routes. As does this comment.

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-injected-routes-compilation/tests/assets/JavaScriptRouterSpec.js

@@ -43,4 +43,13 @@ describe("The JavaScript router", function() {
         var data = jsRoutesBadHost.controllers.Application.index();
         assert(data.absoluteURL().indexOf("'}}};alert(1);a={a:{a:{a:'") >= 0)
     });
+    it("should generate a url for assets", function() {
+        var data = jsRoutes.controllers.Assets.versioned('hello.png');
+        assert.equal("/public/hello.png", data.url);
+    });
+    it("should provide the GET method for assets", function() {
+        var data = jsRoutes.controllers.Assets.versioned();
+        assert.equal("GET", data.method);
+    });
+
 });

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/app/utils/JavaScriptRouterGenerator.scala

@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package utils
+
+import java.nio.file.Files
+import java.nio.file.Paths
+
+object JavaScriptRouterGenerator extends App {
+
+  val jsFile = play.api.routing
+    .JavaScriptReverseRouter(
+      "jsRoutes",
+      None,
+      "localhost",
+      router.controllers.routes.javascript.Assets.versioned,
+    )
+    .body
+
+  // Add module exports for node
+  val jsModule = jsFile +
+    """
+      |module.exports = jsRoutes
+    """.stripMargin
+
+  val path = Paths.get(args(0))
+  Files.createDirectories(path.getParent)
+  Files.write(path, jsModule.getBytes("UTF-8"))
+
+}

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/build.sbt

@@ -0,0 +1,55 @@
+//
+// Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+//
+lazy val root = (project in file("."))
+  .enablePlugins(PlayScala)
+
+namespaceReverseRouter := true
+
+libraryDependencies ++= Seq(guice, specs2 % Test)
+
+scalaVersion := sys.props("scala.version")
+updateOptions := updateOptions.value.withLatestSnapshots(false)
+evictionWarningOptions in update ~= (_.withWarnTransitiveEvictions(false).withWarnDirectEvictions(false))
+
+// can't use test directory since scripted calls its script "test"
+sourceDirectory in Test := baseDirectory.value / "tests"
+
+scalaSource in Test := baseDirectory.value / "tests"
+
+// Generate a js router so we can test it with mocha
+val generateJsRouter = TaskKey[Seq[File]]("generate-js-router")
+
+generateJsRouter := {
+  (runMain in Compile).toTask(" utils.JavaScriptRouterGenerator target/web/jsrouter/jsRoutes.js").value
+  Seq(target.value / "web" / "jsrouter" / "jsRoutes.js")
+}
+
+resourceGenerators in TestAssets         += Def.task(generateJsRouter.value).taskValue
+managedResourceDirectories in TestAssets += target.value / "web" / "jsrouter"
+
+// We don't want source position mappers is this will make it very hard to debug
+sourcePositionMappers := Nil
+
+play.sbt.routes.RoutesKeys.routesImport := Nil
+ScriptedTools.dumpRoutesSourceOnCompilationFailure
+
+scalacOptions ++= {
+  Seq(
+    "-deprecation",
+    "-encoding",
+    "UTF-8",
+    "-feature",
+    "-language:existentials",
+    "-language:higherKinds",
+    "-language:implicitConversions",
+    "-unchecked",
+    "-Xfatal-warnings",
+    "-Xlint",
+    "-Yno-adapted-args",
+    "-Ywarn-dead-code",
+    "-Ywarn-numeric-widen",
+    "-Ywarn-value-discard",
+    "-Xfuture"
+  )
+}

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/conf/routes

@@ -0,0 +1,5 @@
+#
+# Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+#
+
+GET         /public/*file           _root_.controllers.Assets.versioned(path="/public", file: _root_.controllers.Assets.Asset)

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/project/plugins.sbt

@@ -0,0 +1,8 @@
+//
+// Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+//
+
+updateOptions := updateOptions.value.withLatestSnapshots(false)
+addSbtPlugin("com.typesafe.play" % "sbt-plugin"         % sys.props("project.version"))
+addSbtPlugin("com.typesafe.play" % "sbt-scripted-tools" % sys.props("project.version"))
+addSbtPlugin("com.typesafe.sbt"  % "sbt-mocha"          % "1.1.2")

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/public/css/main.css.md5

@@ -0,0 +1 @@
+abcd1234

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/public/css/minmain-min.css.md5

@@ -0,0 +1 @@
+abcd1234

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/test

@@ -0,0 +1,5 @@
+# Stage it, so it's easier to debug
+> playRoutes
+> compile
+> test:compile
+> test

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/tests/RouterSpec.scala

@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package test
+
+import play.api.test._
+
+object RouterSpec extends PlaySpecification {
+
+  "document the router" in new WithApplication() {
+    val someRoute = implicitApp.injector
+      .instanceOf[play.api.routing.Router]
+      .documentation
+      .find(r => r._1 == "GET" && r._2.startsWith("/public/"))
+    someRoute must beSome[(String, String, String)]
+    val route = someRoute.get
+    route._2 must_== "/public/$file<.+>"
+    route._3 must startWith("""_root_.controllers.Assets.versioned(path:String = "/public", file:_root_.controllers.Assets.Asset)""")
+  }
+
+  "The assets reverse route support" should {
+    "fingerprint assets" in new WithApplication() {
+      router.controllers.routes.Assets.versioned("css/main.css").url must_== "/public/css/abcd1234-main.css"
+    }
+    "selected the minified version" in new WithApplication() {
+      router.controllers.routes.Assets.versioned("css/minmain.css").url must_== "/public/css/abcd1234-minmain-min.css"
+    }
+    "work for non fingerprinted assets" in new WithApplication() {
+      router.controllers.routes.Assets.versioned("css/nonfingerprinted.css").url must_== "/public/css/nonfingerprinted.css"
+    }
+    "selected the minified non fingerprinted version" in new WithApplication() {
+      router.controllers.routes.Assets
+        .versioned("css/nonfingerprinted-minmain.css")
+        .url must_== "/public/css/nonfingerprinted-minmain-min.css"
+    }
+  }
+}

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-namespace-reverse-router/tests/assets/JavaScriptRouterSpec.js

@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) Lightbend Inc. <https://www.lightbend.com>
+ */
+var assert = require("assert");
+var jsRoutes = require("./jsRoutes");
+
+describe("The JavaScript router", function() {
+    it("should generate a url for assets", function() {
+        var data = jsRoutes.router.controllers.Assets.versioned('hello.png');
+        assert.equal("/public/hello.png", data.url);
+    });
+    it("should provide the GET method for assets", function() {
+        var data = jsRoutes.router.controllers.Assets.versioned();
+        assert.equal("GET", data.method);
+    });
+});

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-routes-compilation/app/utils/JavaScriptRouterGenerator.scala

@@ -15,6 +15,7 @@ object JavaScriptRouterGenerator extends App {
       "jsRoutes",
       None,
       "localhost",
+      Assets.versioned,
       Application.index,
       Application.post,
       Application.withParam,

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/routes-compiler-routes-compilation/tests/assets/JavaScriptRouterSpec.js

@@ -25,4 +25,12 @@ describe("The JavaScript router", function() {
         var data = jsRoutes.controllers.Application.takeBool(true);
         assert.equal("/take-bool?b=true", data.url);
     });
+    it("should generate a url for assets", function() {
+        var data = jsRoutes.controllers.Assets.versioned('hello.png');
+        assert.equal("/public/hello.png", data.url);
+    });
+    it("should provide the GET method for assets", function() {
+        var data = jsRoutes.controllers.Assets.versioned();
+        assert.equal("GET", data.method);
+    });
 });