Skip to content

Commit

Permalink
Fix JavaParser.dependenciesFromResources to always return resources e…
Browse files Browse the repository at this point in the history
…ven when there are already matching entries in ~/.rewrite/classpath.

This fixes a scenario where the function could return different values based on the pre-existing contents of that directory.

Before this change dependenciesFromResources() preferred a jar that is already extracted into ~/.rewrite/classpath over classpath resources. But since it's a prefix match the same prefix might match a different jar in ~/.rewrite/classpath than it does in the jar.
For example a jar contains "snakeyaml-2.2.jar" and ~/.rewrite/classpath contains "snakeyaml-1.3.jar" the dependenciesFromResources() when called with the query "snakeyaml" will use 1.3, even though the author of the recipe/module packed in 2.2 and expects that to be used
  • Loading branch information
sambsnyd committed Jun 11, 2024
1 parent ddc560f commit 2d22596
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
import org.openrewrite.test.RewriteTest;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -47,7 +49,7 @@ void incompleteAssignment() {
"""
@Deprecated(since=)
public class A {}
"""
"""
)
);
}
Expand Down Expand Up @@ -88,10 +90,18 @@ public class PersistenceManagerImpl {
}

@Test
void dependenciesFromResources(@TempDir Path temp) {
void dependenciesFromResources(@TempDir Path temp) throws Exception {
JavaParserExecutionContextView ctx = JavaParserExecutionContextView.view(new InMemoryExecutionContext());
ctx.setParserClasspathDownloadTarget(temp.toFile());
assertThat(JavaParser.dependenciesFromResources(ctx, "guava-31.0-jre")).isNotEmpty();
// Put a decoy file in the target directory to ensure that it is not used
Files.write(temp.resolve("guava-30.0-jre.jar"), "decoy for test purposes; not a real jar".getBytes());
List<Path> classpath = JavaParser.dependenciesFromResources(ctx, "guava");
assertThat(classpath)
.singleElement()
.matches(Files::exists, "File extracted from classpath resources exists on disk")
.matches(path -> path.endsWith("guava-31.0-jre.jar"),
"classpathFromResources should return guava-31.0-jre.jar from resources, even when the target " +
"directory contains guava-30.0-jre.jar which has the same prefix");
}

@Test
Expand Down
23 changes: 4 additions & 19 deletions rewrite-java/src/main/java/org/openrewrite/java/JavaParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,30 +109,15 @@ static List<Path> dependenciesFromClasspath(String... artifactNames) {
}

static List<Path> dependenciesFromResources(ExecutionContext ctx, String... artifactNamesWithVersions) {
if(artifactNamesWithVersions.length == 0) {
return Collections.emptyList();
}
List<Path> artifacts = new ArrayList<>(artifactNamesWithVersions.length);
Set<String> missingArtifactNames = new LinkedHashSet<>(artifactNamesWithVersions.length);
missingArtifactNames.addAll(Arrays.asList(artifactNamesWithVersions));
File resourceTarget = JavaParserExecutionContextView.view(ctx)
.getParserClasspathDownloadTarget();

nextArtifact:
for (String artifactName : artifactNamesWithVersions) {
Pattern jarPattern = Pattern.compile("[/\\\\]" + artifactName + "-?.*\\.jar$");
File[] extracted = resourceTarget.listFiles();
if (extracted != null) {
for (File file : extracted) {
if (jarPattern.matcher(file.getPath()).find()) {
artifacts.add(file.toPath());
continue nextArtifact;
}
}
}
missingArtifactNames.add(artifactName);
}

if (missingArtifactNames.isEmpty()) {
return artifacts;
}

Class<?> caller;
try {
// StackWalker is only available in Java 15+, but right now we only use classloader isolated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1130,17 +1130,17 @@ void indirectBomImportedFromParent() {
"""
<project>
<modelVersion>4.0.0</modelVersion>
<artifactId>b</artifactId>
<groupId>org.openrewrite.maven</groupId>
<version>0.1.0-SNAPSHOT</version>
<packaging>pom</packaging>
<properties>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
</properties>
<dependencyManagement>
<dependencies>
<dependency>
Expand All @@ -1161,12 +1161,12 @@ void indirectBomImportedFromParent() {
"""
<project>
<modelVersion>4.0.0</modelVersion>
<artifactId>c</artifactId>
<groupId>org.openrewrite.maven</groupId>
<version>0.1.0-SNAPSHOT</version>
<packaging>pom</packaging>
<dependencyManagement>
<dependencies>
<dependency>
Expand All @@ -1176,7 +1176,7 @@ void indirectBomImportedFromParent() {
</dependency>
</dependencies>
</dependencyManagement>
</project>
</pro ject>
"""
)
),
Expand All @@ -1185,11 +1185,11 @@ void indirectBomImportedFromParent() {
"""
<project>
<modelVersion>4.0.0</modelVersion>
<groupId>org.openrewrite.maven</groupId>
<artifactId>d</artifactId>
<version>0.1.0-SNAPSHOT</version>
<properties>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
Expand Down Expand Up @@ -1532,7 +1532,7 @@ void managedDependencyInTransitiveAndPom() {
<artifactId>a</artifactId>
<version>1.0.0</version>
<packaging>jar</packaging>
<dependencyManagement>
<dependencies>
<dependency>
Expand Down Expand Up @@ -1606,7 +1606,7 @@ void profileNoJdkActivation() {
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1</version>
<profiles>
<profile>
<id>old-jdk</id>
Expand Down Expand Up @@ -2474,7 +2474,7 @@ void pluginManagement() {
<groupId>org.openrewrite.maven</groupId>
<artifactId>a</artifactId>
<version>0.1.0-SNAPSHOT</version>
<build>
<pluginManagement>
<plugins>
Expand Down

0 comments on commit 2d22596

Please sign in to comment.