Merge pull request #183 from jprinet/feature/strip-routes-comments

Strip build dependent comments in Play compilers generated files
gradle · Apr 24, 2023 · 5f6a3d0 · 5f6a3d0
2 parents 2fff17d + 2940cdd
commit 5f6a3d0
Show file tree

Hide file tree

Showing 14 changed files with 215 additions and 8 deletions.
diff --git a/src/docs/asciidoc/10-plugin-conventions.adoc b/src/docs/asciidoc/10-plugin-conventions.adoc
@@ -8,4 +8,6 @@ include::13-tasks.adoc[]
 
 include::14-source-sets.adoc[]
 
-include::15-dependency-configurations.adoc[]
+include::15-dependency-configurations.adoc[]
+
+include::16-generated-files-comments.adoc[]
diff --git a/src/docs/asciidoc/16-generated-files-comments.adoc b/src/docs/asciidoc/16-generated-files-comments.adoc
@@ -0,0 +1,4 @@
+=== Generated files comments
+
+Files generated by Twirl and Route compilers contain comments which are changing across builds (absolute path and date depending on the framework version), this prevents tasks using those files as inputs to benefit from build cache.
+The plugin is post-processing those files to remove timestamp and convert absolute paths to relative paths.
diff --git a/src/docs/asciidoc/50-changes.adoc b/src/docs/asciidoc/50-changes.adoc
@@ -1,5 +1,9 @@
 == Change Log
 
+[discrete]
+=== v0.14 (TBD)
+* Add configuration to post-process routes comments ({uri-github-issues}/109[issue #109]).
+
 [discrete]
 === v0.13 (2023-01-24)
 * Remove usage of internal Gradle API org.gradle.api.internal.artifacts.dsl.LazyPublishArtifact

diff --git a/...tegTest/groovy/org/gradle/playframework/tasks/AbstractRoutesCompileIntegrationTest.groovy b/...tegTest/groovy/org/gradle/playframework/tasks/AbstractRoutesCompileIntegrationTest.groovy
@@ -6,8 +6,11 @@ import org.gradle.testkit.runner.BuildResult
 import org.gradle.testkit.runner.TaskOutcome
 import org.junit.Assume
 
+import java.nio.charset.StandardCharsets
+
 import static org.gradle.playframework.fixtures.Repositories.playRepositories
 import static org.gradle.playframework.fixtures.file.FileFixtures.assertContentsHaveChangedSince
+import static org.gradle.playframework.fixtures.file.FileFixtures.assertModificationTimeHasChangedSince
 import static org.gradle.playframework.fixtures.file.FileFixtures.snapshot
 import static org.gradle.playframework.plugins.PlayRoutesPlugin.ROUTES_COMPILE_TASK_NAME
 
@@ -121,7 +124,7 @@ GET     /newroute                          ${controllers()}.Application.index()
 
         and:
         assertContentsHaveChangedSince(scalaRoutesFileSnapshot, getScalaRoutesFile())
-        assertContentsHaveChangedSince(javaRoutesFileSnapshot, getJavaRoutesFile())
+        assertModificationTimeHasChangedSince(javaRoutesFileSnapshot, getJavaRoutesFile())
         assertContentsHaveChangedSince(reverseRoutesFileSnapshot, getReverseRoutesFile())
 
         when:
@@ -259,4 +262,18 @@ $ROUTES_COMPILE_TASK_NAME {
         new File(destinationDir, getReverseRoutesFileName('', '')).text.contains("extra.package")
         new File(destinationDir, getScalaRoutesFileName('', '')).text.contains("extra.package")
     }
+
+    def "post-processed generated comments contain path and timestamp replacements"() {
+        given:
+        withRoutesTemplate()
+        when:
+        build(ROUTES_COMPILE_TASK_NAME)
+        then:
+        createRouteFileList().each {
+            def generatedFile = new File(destinationDir, it)
+            assert generatedFile.isFile()
+            assert generatedFile.getText(StandardCharsets.UTF_8.toString()).contains("// @(SOURCE):conf/routes")
+            assert !generatedFile.getText(StandardCharsets.UTF_8.toString()).contains("// @(DATE)")
+        }
+    }
 }
diff --git a/src/integTest/groovy/org/gradle/playframework/tasks/TwirlCompileIntegrationTest.groovy b/src/integTest/groovy/org/gradle/playframework/tasks/TwirlCompileIntegrationTest.groovy
@@ -83,7 +83,7 @@ class TwirlCompileIntegrationTest extends PlayMultiVersionIntegrationTest {
         BuildResult result = build(SCALA_COMPILE_TASK_NAME)
         then:
         result.task(TWIRL_COMPILE_TASK_PATH).outcome == TaskOutcome.SUCCESS
-        result.task(SCALA_COMPILE_TASK_PATH).outcome == TaskOutcome.SUCCESS
+        result.task(SCALA_COMPILE_TASK_PATH).outcome == TaskOutcome.UP_TO_DATE
     }
 
     def "can specify additional imports for a Twirl template"() {
@@ -290,6 +290,26 @@ class TwirlCompileIntegrationTest extends PlayMultiVersionIntegrationTest {
         result.output.contains("Twirl compiler could not find a matching template for 'test.scala.custom'.")
     }
 
+    @Unroll
+    def "post-process generated comments"() {
+        given:
+        twirlTemplate("test.scala.${format}") << template
+        when:
+        build(SCALA_COMPILE_TASK_NAME)
+        then:
+        def generatedFile = new File(destinationDir, "${format}/test.template.scala")
+        generatedFile.isFile()
+        generatedFile.text.contains("SOURCE: views/test.scala.${format}")
+        !generatedFile.text.contains("DATE:")
+
+        where:
+        format | templateFormat     | template
+        "js"   | 'JavaScriptFormat' | '@(username: String) alert(@helper.json(username));'
+        "xml"  | 'XmlFormat'        | '@(username: String) <xml> <username> @username </username>'
+        "txt"  | 'TxtFormat'        | '@(username: String) @username'
+        "html" | 'HtmlFormat'       | '@(username: String) <html> <body> <h1>Hello @username</h1> </body> </html>'
+    }
+
     def withTemplateSource(File templateFile) {
         templateFile << """@(message: String)
 

diff --git a/src/integTestFixtures/groovy/org/gradle/playframework/fixtures/file/FileFixtures.groovy b/src/integTestFixtures/groovy/org/gradle/playframework/fixtures/file/FileFixtures.groovy
@@ -32,6 +32,11 @@ final class FileFixtures {
         assertNotEquals(oldSnapshot.hash, now.hash)
     }
 
+    static void assertModificationTimeHasChangedSince(Snapshot oldSnapshot, File file) {
+        Snapshot now = snapshot(file)
+        assertNotEquals(oldSnapshot.modTime, now.modTime)
+    }
+
     private static File assertIsFile(File file) {
         assertTrue(file.isFile())
     }

diff --git a/src/main/java/org/gradle/playframework/tasks/RoutesCompile.java b/src/main/java/org/gradle/playframework/tasks/RoutesCompile.java
@@ -2,7 +2,6 @@
 
 import org.gradle.api.Action;
 import org.gradle.api.file.ConfigurableFileCollection;
-import org.gradle.api.file.Directory;
 import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.file.FileTree;
 import org.gradle.api.provider.ListProperty;
@@ -52,7 +51,6 @@ public class RoutesCompile extends SourceTask {
     private final Property<PlayPlatform> platform;
     private final Property<Boolean> injectedRoutesGenerator;
     private final ConfigurableFileCollection routesCompilerClasspath;
-
     @Inject
     public RoutesCompile(WorkerExecutor workerExecutor) {
         this.workerExecutor = workerExecutor;
@@ -105,7 +103,7 @@ public ConfigurableFileCollection getRoutesCompilerClasspath() {
     @TaskAction
     @SuppressWarnings("Convert2Lambda")
     void compile() {
-        RoutesCompileSpec spec = new DefaultRoutesCompileSpec(getSource().getFiles(), getOutputDirectory().get().getAsFile(), isJavaProject(), getNamespaceReverseRouter().get(), getGenerateReverseRoutes().get(), getInjectedRoutesGenerator().get(), getAdditionalImports().get());
+        RoutesCompileSpec spec = new DefaultRoutesCompileSpec(getSource().getFiles(), getOutputDirectory().get().getAsFile(), isJavaProject(), getNamespaceReverseRouter().get(), getGenerateReverseRoutes().get(), getInjectedRoutesGenerator().get(), getAdditionalImports().get(), getProject().getProjectDir());
 
         if (GradleVersion.current().compareTo(GradleVersion.version("5.6")) < 0) {
             workerExecutor.submit(RoutesCompileRunnable.class, workerConfiguration -> {
@@ -177,4 +175,5 @@ public Property<Boolean> getGenerateReverseRoutes() {
     public Property<Boolean> getInjectedRoutesGenerator() {
         return injectedRoutesGenerator;
     }
+
 }
diff --git a/src/main/java/org/gradle/playframework/tools/internal/routes/DefaultRoutesCompileSpec.java b/src/main/java/org/gradle/playframework/tools/internal/routes/DefaultRoutesCompileSpec.java
@@ -11,15 +11,17 @@ public class DefaultRoutesCompileSpec implements RoutesCompileSpec {
     private final boolean generateReverseRoutes;
     private final boolean injectedRoutesGenerator;
     private final Collection<String> additionalImports;
+    private final File projectDir;
 
-    public DefaultRoutesCompileSpec(Iterable<File> sourceFiles, File outputDirectory, boolean javaProject, boolean namespaceReverseRouter, boolean generateReverseRoutes, boolean injectedRoutesGenerator, Collection<String> additionalImports) {
+    public DefaultRoutesCompileSpec(Iterable<File> sourceFiles, File outputDirectory, boolean javaProject, boolean namespaceReverseRouter, boolean generateReverseRoutes, boolean injectedRoutesGenerator, Collection<String> additionalImports, File projectDir) {
         this.sourceFiles = sourceFiles;
         this.outputDirectory = outputDirectory;
         this.javaProject = javaProject;
         this.namespaceReverseRouter = namespaceReverseRouter;
         this.generateReverseRoutes = generateReverseRoutes;
         this.injectedRoutesGenerator = injectedRoutesGenerator;
         this.additionalImports = additionalImports;
+        this.projectDir = projectDir;
     }
 
     @Override
@@ -56,4 +58,9 @@ public boolean isInjectedRoutesGenerator() {
     public Collection<String> getAdditionalImports() {
         return additionalImports;
     }
+
+    @Override
+    public File getProjectDir() {
+        return projectDir;
+    }
 }
diff --git a/src/main/java/org/gradle/playframework/tools/internal/routes/DefaultRoutesPostProcessor.java b/src/main/java/org/gradle/playframework/tools/internal/routes/DefaultRoutesPostProcessor.java
@@ -0,0 +1,56 @@
+package org.gradle.playframework.tools.internal.routes;
+
+import org.gradle.util.RelativePathUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.stream.Stream;
+
+/** This post processor fixes build / project dependent comments (DATE and SOURCE) from the RoutesCompiler generated files.
+ * Here is an example:
+ * @GENERATOR:play-routes-compiler
+ * @(DATE): Mon Apr 03 10:27:51 CEST 2023
+ * @(SOURCE):/private/var/folders/79/xmc9yr493y75ptry2_nrx3r00000gn/T/junit4995996226044083355/conf/routes
+**/
+class DefaultRoutesPostProcessor implements Serializable {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(DefaultRoutesPostProcessor.class);
+
+    void execute(RoutesCompileSpec spec) {
+        String sourceReplacementString = getSourceReplacementString(spec.getSources(), spec.getProjectDir());
+
+        try (Stream<Path> stream = Files.find(spec.getDestinationDir().toPath(), Integer.MAX_VALUE, (filePath, fileAttr) -> fileAttr.isRegularFile())) {
+            stream.forEach(routeFile -> process(routeFile, sourceReplacementString));
+        } catch (IOException e) {
+            LOGGER.warn("Unable to post-process files", e);
+        }
+    }
+
+    private String getSourceReplacementString(Iterable<File> sources, File projectDir) {
+        String sourceReplacementString = "";
+
+        if(sources.iterator().hasNext()) {
+            File sourceFile = sources.iterator().next();
+            sourceReplacementString = "// @(SOURCE):" + RelativePathUtil.relativePath(projectDir, sourceFile);
+        }
+
+        return sourceReplacementString;
+    }
+
+    private void process(Path routeFile, String sourceReplacementString) {
+        try {
+            String content = new String(Files.readAllBytes(routeFile), StandardCharsets.UTF_8);
+            content = content.replaceAll("(?m)^// @(SOURCE):.*", sourceReplacementString);
+            content = content.replaceAll("(?m)^// @(DATE):.*", "");
+            Files.write(routeFile, content.getBytes(StandardCharsets.UTF_8));
+        } catch (IOException e) {
+            LOGGER.warn(String.format("Unable to post-process file %s", routeFile.getFileName()), e);
+        }
+    }
+}
diff --git a/src/main/java/org/gradle/playframework/tools/internal/routes/RoutesCompileSpec.java b/src/main/java/org/gradle/playframework/tools/internal/routes/RoutesCompileSpec.java
@@ -18,4 +18,6 @@ public interface RoutesCompileSpec extends PlayCompileSpec, Serializable {
     boolean isInjectedRoutesGenerator();
 
     Collection<String> getAdditionalImports();
+
+    File getProjectDir();
 }
diff --git a/src/main/java/org/gradle/playframework/tools/internal/routes/RoutesCompiler.java b/src/main/java/org/gradle/playframework/tools/internal/routes/RoutesCompiler.java
@@ -12,8 +12,11 @@
 public class RoutesCompiler implements Compiler<RoutesCompileSpec>, Serializable {
     private final VersionedRoutesCompilerAdapter adapter;
 
+    private final DefaultRoutesPostProcessor postProcessor;
+
     public RoutesCompiler(VersionedRoutesCompilerAdapter adapter) {
         this.adapter = adapter;
+        this.postProcessor = new DefaultRoutesPostProcessor();
     }
 
     @Override
@@ -42,6 +45,9 @@ public WorkResult execute(RoutesCompileSpec spec) {
             didWork = ret || didWork;
         }
 
+        // Post-process routes files
+        postProcessor.execute(spec);
+
         return WorkResults.didWork(didWork);
     }
 

diff --git a/src/main/java/org/gradle/playframework/tools/internal/twirl/DefaultTwirlPostProcessor.java b/src/main/java/org/gradle/playframework/tools/internal/twirl/DefaultTwirlPostProcessor.java
@@ -0,0 +1,79 @@
+package org.gradle.playframework.tools.internal.twirl;
+
+import org.gradle.api.internal.file.RelativeFile;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.Serializable;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+/** This post processor fixes build / project dependent comments (DATE and SOURCE) from the TwirlCompiler generated files.
+ * Here is an example:
+ * -- GENERATED --
+ * DATE: Mon Apr 03 10:27:51 CEST 2023
+ * SOURCE: /private/var/folders/79/xmc9yr493y75ptry2_nrx3r00000gn/T/junit4995996226044083355/app/views/test.scala.html
+ * HASH: 4bbbe5fde39afa0d46da8df7714a136a78170d6a
+ * MATRIX: 728->1|841->19|869->20|920->45|948->53
+ * LINES: 21->1|26->1|26->1|26->1|26->1
+ * -- GENERATED --
+*/
+ class DefaultTwirlPostProcessor implements Serializable {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(DefaultTwirlPostProcessor.class);
+
+    private static final String GENERATED_TAG = "-- GENERATED --";
+    private static final String GENERATED_LINE_PREFIX_DATE = "DATE: ";
+    private static final String GENERATED_LINE_PREFIX_SOURCE = "SOURCE: ";
+
+    void execute(TwirlCompileSpec spec) {
+        String sourceReplacementString = getSourceReplacementString(spec.getSources());
+
+        try (Stream<Path> stream = Files.find(spec.getDestinationDir().toPath(), Integer.MAX_VALUE, (filePath, fileAttr) -> fileAttr.isRegularFile())) {
+            stream.forEach(routeFile -> process(routeFile, sourceReplacementString));
+        } catch (IOException e) {
+            LOGGER.warn("Unable to post-process files", e);
+        }
+    }
+
+    private String getSourceReplacementString(Iterable<RelativeFile> sources) {
+        String sourceReplacementString = "";
+
+        if(sources.iterator().hasNext()) {
+            RelativeFile sourceFile = sources.iterator().next();
+            sourceReplacementString = "SOURCE: " + sourceFile.getRelativePath();
+        }
+
+        return sourceReplacementString;
+    }
+
+    private void process(Path generatedFile, String sourceReplacementString) {
+        try {
+            List<String> generatedSourceLines = Files.readAllLines(generatedFile, StandardCharsets.UTF_8);
+            List<String> updatedSourceLines = new ArrayList<>();
+
+            boolean isInGeneratedSection = false;
+            for (String currentLine : generatedSourceLines) {
+                if(currentLine.contains(GENERATED_TAG)) {
+                    isInGeneratedSection = !isInGeneratedSection;
+                }
+                if(isInGeneratedSection && currentLine.contains(GENERATED_LINE_PREFIX_SOURCE)) {
+                    // update path to relative and keep trailing spaces
+                    String updatedLine = currentLine.substring(0, currentLine.indexOf(GENERATED_LINE_PREFIX_SOURCE)) + sourceReplacementString;
+                    updatedSourceLines.add(updatedLine);
+                } else if(!(isInGeneratedSection && currentLine.contains(GENERATED_LINE_PREFIX_DATE))) {
+                    updatedSourceLines.add(currentLine);
+                }
+            }
+
+            Files.write(generatedFile, updatedSourceLines, StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            LOGGER.warn(String.format("Unable to post-process file %s", generatedFile.getFileName()), e);
+        }
+    }
+}
diff --git a/src/main/java/org/gradle/playframework/tools/internal/twirl/TwirlCompiler.java b/src/main/java/org/gradle/playframework/tools/internal/twirl/TwirlCompiler.java
@@ -23,8 +23,11 @@ public class TwirlCompiler implements Compiler<TwirlCompileSpec>, Serializable {
 
     private final VersionedTwirlCompilerAdapter adapter;
 
+    private final DefaultTwirlPostProcessor postProcessor;
+
     public TwirlCompiler(VersionedTwirlCompilerAdapter adapter) {
         this.adapter = adapter;
+        this.postProcessor = new DefaultTwirlPostProcessor();
     }
 
     @Override
@@ -48,6 +51,9 @@ public WorkResult execute(TwirlCompileSpec spec) {
             }
         }
 
+        // Post-process files
+        postProcessor.execute(spec);
+
         return WorkResults.didWork(!outputFiles.isEmpty());
     }
 

diff --git a/src/main/java/org/gradle/playframework/tools/internal/twirl/TwirlCompilerFactory.java b/src/main/java/org/gradle/playframework/tools/internal/twirl/TwirlCompilerFactory.java
@@ -22,7 +22,7 @@ public static VersionedTwirlCompilerAdapter createAdapter(PlayPlatform playPlatf
                 return new TwirlCompilerAdapterV13X("1.3.13", scalaCompatibilityVersion, playTwirlAdapter);
             case PLAY_2_7_X:
             default:
-                return new TwirlCompilerAdapterV13X("1.4.2", scalaCompatibilityVersion, playTwirlAdapter);
+                return new TwirlCompilerAdapterV13X("1.5.1", scalaCompatibilityVersion, playTwirlAdapter);
         }
     }