Fix download snapshot warnings (#882)

* [WIP] Fix download snapshot warnings * Fix download snapshot warnings * Inline `sourceSet` function * Remove (now) unused gav parameter * Define a unit test that triggers the SNAPSHOT handling * Update src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java Co-authored-by: Tim te Beek <[email protected]> * Tidy up --------- Co-authored-by: Tim te Beek <[email protected]> Co-authored-by: Tim te Beek <[email protected]>
openrewrite · Oct 29, 2024 · 020ebcf · 020ebcf
1 parent 3a23f61
commit 020ebcf
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 32 deletions.
diff --git a/.gitignore b/.gitignore
@@ -9,3 +9,4 @@ pom.xml.versionsBackup
 .classpath
 .project
 .settings
+.vscode/
diff --git a/src/main/java/org/openrewrite/maven/ConfigurableRewriteMojo.java b/src/main/java/org/openrewrite/maven/ConfigurableRewriteMojo.java
@@ -111,7 +111,7 @@ protected Set<String> getExclusions() {
     private LinkedHashSet<String> plainTextMasks;
 
     /**
-     * Allows to add additional plain text masks without overriding
+     * Allows adding additional plain text masks without overriding
      * the defaults.
      */
     @Nullable
@@ -169,7 +169,7 @@ protected Set<String> getPlainTextMasks() {
      * Whether to throw an exception if an activeRecipe fails configuration validation.
      * This may happen if the activeRecipe is improperly configured, or any downstream recipes are improperly configured.
      * <p>
-     * For the time, this default is "false" to prevent one improperly recipe from failing the build.
+     * For the time, this default is "false" to prevent one improper recipe from failing the build.
      * In the future, this default may be changed to "true" to be more restrictive.
      */
     @Parameter(property = "rewrite.failOnInvalidActiveRecipes", alias = "failOnInvalidActiveRecipes", defaultValue = "false")

diff --git a/src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java b/src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java
@@ -47,14 +47,18 @@
 import org.openrewrite.marker.ci.BuildEnvironment;
 import org.openrewrite.maven.cache.InMemoryMavenPomCache;
 import org.openrewrite.maven.cache.MavenPomCache;
+import org.openrewrite.maven.internal.RawPom;
 import org.openrewrite.maven.internal.RawRepositories;
+import org.openrewrite.maven.tree.Pom;
 import org.openrewrite.maven.tree.ProfileActivation;
+import org.openrewrite.maven.tree.ResolvedGroupArtifactVersion;
 import org.openrewrite.maven.utilities.MavenWrapper;
 import org.openrewrite.style.NamedStyles;
 import org.openrewrite.xml.tree.Xml;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.charset.Charset;
 import java.nio.file.*;
 import java.nio.file.attribute.BasicFileAttributes;
@@ -88,6 +92,7 @@ public class MavenMojoProjectParser {
 
     private static final String MVN_JVM_CONFIG = ".mvn/jvm.config";
     private static final String MVN_MAVEN_CONFIG = ".mvn/maven.config";
+    private static final String MAVEN_COMPILER_PLUGIN = "org.apache.maven.plugins:maven-compiler-plugin";
 
     @Nullable
     public static MavenPomCache POM_CACHE;
@@ -137,7 +142,7 @@ public Stream<SourceFile> listSourceFiles(MavenProject mavenProject, List<NamedS
             Xml.Document maven = parseMaven(mavenProject, projectProvenance, ctx);
             return listSourceFiles(mavenProject, maven, projectProvenance, styles, ctx);
         } else {
-            //If running across all project, iterate and parse source files from each project
+            //If running across all projects, iterate and parse source files from each project
             Map<MavenProject, List<Marker>> projectProvenances = mavenSession.getProjects().stream()
                     .collect(Collectors.toMap(Function.identity(), this::generateProvenance));
             Map<MavenProject, Xml.Document> projectMap = parseMaven(mavenSession.getProjects(), projectProvenances, ctx);
@@ -216,7 +221,7 @@ public Stream<SourceFile> listSourceFiles(MavenProject mavenProject, Xml.@Nullab
     }
 
     private static Optional<Charset> getCharset(MavenProject mavenProject) {
-        String compilerPluginKey = "org.apache.maven.plugins:maven-compiler-plugin";
+        String compilerPluginKey = MAVEN_COMPILER_PLUGIN;
         Plugin plugin = Optional.ofNullable(mavenProject.getPlugin(compilerPluginKey))
                 .orElseGet(() -> mavenProject.getPluginManagement().getPluginsAsMap().get(compilerPluginKey));
         if (plugin != null && plugin.getConfiguration() instanceof Xpp3Dom) {
@@ -267,7 +272,7 @@ private static JavaVersion getSrcMainJavaVersion(MavenProject mavenProject) {
         String sourceCompatibility = null;
         String targetCompatibility = null;
 
-        Plugin compilerPlugin = mavenProject.getPlugin("org.apache.maven.plugins:maven-compiler-plugin");
+        Plugin compilerPlugin = mavenProject.getPlugin(MAVEN_COMPILER_PLUGIN);
         if (compilerPlugin != null && compilerPlugin.getConfiguration() instanceof Xpp3Dom) {
             Xpp3Dom dom = (Xpp3Dom) compilerPlugin.getConfiguration();
             Xpp3Dom release = dom.getChild("release");
@@ -310,7 +315,7 @@ static JavaVersion getSrcTestJavaVersion(MavenProject mavenProject) {
         String sourceCompatibility = null;
         String targetCompatibility = null;
 
-        Plugin compilerPlugin = mavenProject.getPlugin("org.apache.maven.plugins:maven-compiler-plugin");
+        Plugin compilerPlugin = mavenProject.getPlugin(MAVEN_COMPILER_PLUGIN);
         if (compilerPlugin != null && compilerPlugin.getConfiguration() instanceof Xpp3Dom) {
             Xpp3Dom dom = (Xpp3Dom) compilerPlugin.getConfiguration();
             Xpp3Dom release = dom.getChild("testRelease");
@@ -417,7 +422,7 @@ private Stream<SourceFile> processMainSources(
         }
 
         List<Marker> mainProjectProvenance = new ArrayList<>(projectProvenance);
-        mainProjectProvenance.add(sourceSet("main", dependencies, typeCache));
+        mainProjectProvenance.add(JavaSourceSet.build("main", dependencies));
         mainProjectProvenance.add(getSrcMainJavaVersion(mavenProject));
 
         //Filter out any generated source files from the returned list, as we do not want to apply the recipe to the
@@ -474,7 +479,7 @@ private Stream<SourceFile> processTestSources(
         }
 
         List<Marker> markers = new ArrayList<>(projectProvenance);
-        markers.add(sourceSet("test", testDependencies, typeCache));
+        markers.add(JavaSourceSet.build("test", testDependencies));
         markers.add(getSrcTestJavaVersion(mavenProject));
 
         // Any resources parsed from "test/resources" should also have the test source set added to them.
@@ -506,10 +511,6 @@ private Stream<SourceFile> processTestSources(
         return null;
     }
 
-    private static JavaSourceSet sourceSet(String name, List<Path> dependencies, JavaTypeCache typeCache) {
-        return JavaSourceSet.build(name, dependencies);
-    }
-
     public Xml.@Nullable Document parseMaven(MavenProject mavenProject, List<Marker> projectProvenance, ExecutionContext ctx) throws MojoFailureException {
         return parseMaven(singletonList(mavenProject), singletonMap(mavenProject, projectProvenance), ctx).get(mavenProject);
     }
@@ -520,30 +521,29 @@ public Map<MavenProject, Xml.Document> parseMaven(List<MavenProject> mavenProjec
             return emptyMap();
         }
 
-        MavenProject topLevelProject = mavenSession.getTopLevelProject();
-        logInfo(topLevelProject, "Resolving Poms...");
-
-        Set<Path> allPoms = new LinkedHashSet<>();
-        mavenProjects.forEach(p -> collectPoms(p, allPoms));
-        for (MavenProject mavenProject : mavenProjects) {
-            mavenSession.getProjectDependencyGraph().getUpstreamProjects(mavenProject, true).forEach(p -> collectPoms(p, allPoms));
-        }
-        MavenParser.Builder mavenParserBuilder = MavenParser.builder().mavenConfig(baseDir.resolve(MVN_MAVEN_CONFIG));
-
         MavenSettings settings = buildSettings();
         MavenExecutionContextView mavenExecutionContext = MavenExecutionContextView.view(ctx);
         mavenExecutionContext.setMavenSettings(settings);
         mavenExecutionContext.setResolutionListener(new MavenLoggingResolutionEventListener(logger));
 
-        if (pomCacheEnabled) {
-            //The default pom cache is enabled as a two-layer cache L1 == in-memory and L2 == RocksDb
-            //If the flag is set to false, only the default, in-memory cache is used.
-            mavenExecutionContext.setPomCache(getPomCache(pomCacheDirectory, logger));
+        // The default pom cache is enabled as a two-layer cache L1 == in-memory and L2 == RocksDb
+        // If the flag is set to false, only the default, in-memory cache is used.
+        MavenPomCache pomCache = pomCacheEnabled ? getPomCache(pomCacheDirectory, logger) : mavenExecutionContext.getPomCache();
+        mavenExecutionContext.setPomCache(pomCache);
+
+        MavenProject topLevelProject = mavenSession.getTopLevelProject();
+        logInfo(topLevelProject, "Resolving Poms...");
+
+        Set<Path> allPoms = new LinkedHashSet<>();
+        mavenProjects.forEach(p -> collectPoms(p, allPoms, mavenExecutionContext));
+        for (MavenProject mavenProject : mavenProjects) {
+            mavenSession.getProjectDependencyGraph().getUpstreamProjects(mavenProject, true).forEach(p -> collectPoms(p, allPoms, mavenExecutionContext));
         }
 
+        MavenParser.Builder mavenParserBuilder = MavenParser.builder().mavenConfig(baseDir.resolve(MVN_MAVEN_CONFIG));
         List<String> activeProfiles = topLevelProject.getActiveProfiles().stream().map(Profile::getId).collect(toList());
         if (!activeProfiles.isEmpty()) {
-            mavenParserBuilder.activeProfiles(activeProfiles.toArray(new String[]{}));
+            mavenParserBuilder.activeProfiles(activeProfiles.toArray(new String[0]));
         }
 
         List<SourceFile> mavens = mavenParserBuilder.build()
@@ -611,16 +611,22 @@ public Map<MavenProject, Xml.Document> parseMaven(List<MavenProject> mavenProjec
      *
      * @param project A maven project to examine for any children/parent poms.
      * @param paths   A list of paths to poms that have been collected so far.
+     * @param ctx     The execution context for the current project.
      */
-    private void collectPoms(MavenProject project, Set<Path> paths) {
-        paths.add(pomPath(project));
+    private void collectPoms(MavenProject project, Set<Path> paths, MavenExecutionContextView ctx) {
+        if (!paths.add(pomPath(project))) {
+            return;
+        }
+
+        ResolvedGroupArtifactVersion gav = createResolvedGAV(project, ctx);
+        ctx.getPomCache().putPom(gav, createPom(project));
 
         // children
         if (project.getCollectedProjects() != null) {
             for (MavenProject child : project.getCollectedProjects()) {
                 Path path = pomPath(child);
                 if (!paths.contains(path)) {
-                    collectPoms(child, paths);
+                    collectPoms(child, paths, ctx);
                 }
             }
         }
@@ -629,7 +635,7 @@ private void collectPoms(MavenProject project, Set<Path> paths) {
         while (parent != null && parent.getFile() != null) {
             Path path = pomPath(parent);
             if (!paths.contains(path)) {
-                collectPoms(parent, paths);
+                collectPoms(parent, paths, ctx);
             }
             parent = parent.getParent();
         }
@@ -644,7 +650,7 @@ private static Path pomPath(MavenProject mavenProject) {
         // org.eclipse.tycho:tycho-packaging-plugin:update-consumer-pom produces a synthetic pom
         if (pomPath.endsWith(".tycho-consumer-pom.xml")) {
             Path normalPom = mavenProject.getBasedir().toPath().resolve("pom.xml");
-            // check for existence of the POM, since Tycho can work pom-less
+            // check for the existence of the POM, since Tycho can work pom-less
             if (Files.isReadable(normalPom) && Files.isRegularFile(normalPom)) {
                 return normalPom;
             }
@@ -807,4 +813,24 @@ private void logDebug(MavenProject mavenProject, String message) {
     private static <E extends Throwable> E sneakyThrow(Throwable e) throws E {
         return (E) e;
     }
+
+    private static ResolvedGroupArtifactVersion createResolvedGAV(MavenProject project, MavenExecutionContextView ctx) {
+        return new ResolvedGroupArtifactVersion(
+                ctx.getLocalRepository().getUri(),
+                project.getGroupId(),
+                project.getArtifactId(),
+                project.getVersion(),
+                project.getVersion().endsWith("-SNAPSHOT") ? null : project.getVersion()
+        );
+    }
+
+    private static @Nullable Pom createPom(MavenProject project) {
+        Path pomPath = project.getFile().toPath();
+        try (InputStream is = Files.newInputStream(pomPath)) {
+            RawPom rawPom = RawPom.parse(is, null);
+            return rawPom.toPom(project.getBasedir().toPath().relativize(pomPath), null);
+        } catch (IOException e) {
+            return null;
+        }
+    }
 }
diff --git a/src/test/java/org/openrewrite/maven/BasicIT.java b/src/test/java/org/openrewrite/maven/BasicIT.java
@@ -80,4 +80,15 @@ void resolves_settings(MavenExecutionResult result) {
                 .allSatisfy(line -> assertThat(line).doesNotContain("Illegal character in path at index 1"));
     }
 
+    @MavenTest
+    @MavenGoal("${project.groupId}:${project.artifactId}:${project.version}:dryRun")
+    void snapshot_ok(MavenExecutionResult result) {
+        assertThat(result)
+                .isSuccessful()
+                .out()
+                .warn()
+                .isEmpty();
+        assertThat(result).out().info().contains("Running recipe(s)...");
+    }
+
 }
diff --git a/src/test/resources-its/org/openrewrite/maven/BasicIT/snapshot_ok/pom.xml b/src/test/resources-its/org/openrewrite/maven/BasicIT/snapshot_ok/pom.xml
@@ -0,0 +1,32 @@
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xmlns="http://maven.apache.org/POM/4.0.0"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>org.openrewrite.maven</groupId>
+    <artifactId>snapshot_ok</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>jar</packaging>
+    <name>BasicIT#snapshot_ok</name>
+
+    <properties>
+        <maven.compiler.source>1.8</maven.compiler.source>
+        <maven.compiler.target>1.8</maven.compiler.target>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+    </properties>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>@project.groupId@</groupId>
+                <artifactId>@project.artifactId@</artifactId>
+                <version>@project.version@</version>
+                <configuration>
+                    <activeRecipes>
+                        <recipe>org.openrewrite.java.format.AutoFormat</recipe>
+                    </activeRecipes>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+</project>