Skip to content

Commit 6ef8aa9

Browse files
Strum355claude
andcommitted
fix(maven): address PR #491 review feedback
- Extract DEP_TREE_PLUGIN constant to avoid duplicated GAV string - Wrap Files.deleteIfExists in finally block with try/catch to prevent IOException from violating the "return original list" fallback contract - Log full exception (with stack trace) instead of just e.getMessage() - Replace absolute filesystem paths in effectivePom.xml fixture with relative paths - Add failure-path test verifying graceful fallback when dependency:tree invocation fails Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5625902 commit 6ef8aa9

3 files changed

Lines changed: 68 additions & 17 deletions

File tree

src/main/java/io/github/guacsec/trustifyda/providers/JavaMavenProvider.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.Map;
4242
import java.util.Objects;
4343
import java.util.Optional;
44+
import java.util.logging.Level;
4445
import java.util.logging.Logger;
4546
import java.util.stream.Collectors;
4647
import javax.xml.stream.XMLInputFactory;
@@ -60,6 +61,8 @@ public final class JavaMavenProvider extends BaseJavaProvider {
6061
private final String mvnExecutable;
6162
private static final String MVN = Operations.isWindows() ? "mvn.cmd" : "mvn";
6263
private static final String ARG_VERSION = "-v";
64+
private static final String DEP_TREE_PLUGIN =
65+
"org.apache.maven.plugins:maven-dependency-plugin:3.6.0:tree";
6366

6467
public JavaMavenProvider(Path manifest) {
6568
super(Type.MAVEN, manifest);
@@ -136,7 +139,7 @@ public Content provideStack() throws IOException {
136139
var mvnTreeCmdArgs =
137140
new ArrayList<>(
138141
List.of(
139-
"org.apache.maven.plugins:maven-dependency-plugin:3.6.0:tree",
142+
DEP_TREE_PLUGIN,
140143
"-Dscope=compile",
141144
"-Dverbose",
142145
"-DoutputType=text",
@@ -268,7 +271,7 @@ private List<DependencyAggregator> resolveVersionRanges(List<DependencyAggregato
268271
try {
269272
var cmd =
270273
buildMvnCommandArgs(
271-
"org.apache.maven.plugins:maven-dependency-plugin:3.6.0:tree",
274+
DEP_TREE_PLUGIN,
272275
"-Dscope=compile",
273276
"-DoutputType=text",
274277
String.format("-DoutputFile=%s", tmpFile.toString()),
@@ -304,14 +307,19 @@ private List<DependencyAggregator> resolveVersionRanges(List<DependencyAggregato
304307
}
305308
}
306309
} catch (Exception e) {
307-
log.warning(
310+
log.log(
311+
Level.WARNING,
308312
String.format(
309313
"Failed to resolve version ranges via dependency tree, "
310314
+ "using original versions: %s",
311-
e.getMessage()));
315+
e.getMessage()),
316+
e);
312317
return deps;
313318
} finally {
314-
Files.deleteIfExists(tmpFile);
319+
try {
320+
Files.deleteIfExists(tmpFile);
321+
} catch (IOException ignored) {
322+
}
315323
}
316324

317325
return deps;

src/test/java/io/github/guacsec/trustifyda/providers/Java_Maven_Provider_Test.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.Arrays;
3131
import java.util.Optional;
3232
import java.util.stream.Stream;
33+
import org.junit.jupiter.api.Test;
3334
import org.junit.jupiter.api.extension.ExtendWith;
3435
import org.junit.jupiter.params.ParameterizedTest;
3536
import org.junit.jupiter.params.provider.MethodSource;
@@ -244,6 +245,48 @@ void test_the_provideComponent_With_Path(String testFolder) throws IOException {
244245
}
245246
}
246247

248+
@Test
249+
void test_provideComponent_fallsBack_when_depTree_fails() throws IOException {
250+
String testFolder = "deps_with_version_range";
251+
252+
var targetPom = resolveFile(String.format("tst_manifests/maven/%s/pom.xml", testFolder));
253+
254+
String effectivePom;
255+
try (var is =
256+
getResourceAsStreamDecision(
257+
getClass(), String.format("tst_manifests/maven/%s/effectivePom.xml", testFolder))) {
258+
effectivePom = new String(is.readAllBytes());
259+
}
260+
261+
try (MockedStatic<Operations> mockedOperations = mockStatic(Operations.class)) {
262+
mockedOperations
263+
.when(() -> Operations.runProcess(any(), any(), any()))
264+
.thenAnswer(
265+
invocationOnMock -> {
266+
String result =
267+
getOutputFileAndOverwriteItWithMock(
268+
effectivePom, invocationOnMock, "-Doutput=");
269+
if (result == null) {
270+
throw new IOException("Simulated dependency:tree failure");
271+
}
272+
return result;
273+
});
274+
mockedOperations.when(() -> Operations.getCustomPathOrElse(anyString())).thenReturn("mvn");
275+
mockedOperations
276+
.when(() -> Operations.getExecutable(anyString(), anyString()))
277+
.thenReturn("mvn");
278+
279+
var content = new JavaMavenProvider(targetPom).provideComponent();
280+
281+
assertThat(content.type).isEqualTo(Api.CYCLONEDX_MEDIA_TYPE);
282+
String sbom = new String(content.buffer);
283+
assertThat(sbom).contains("log4j");
284+
assertThat(sbom).contains("snappy-java");
285+
// Version range should remain unresolved since dep tree failed
286+
assertThat(sbom).contains("[1.2.17,1.3.0)");
287+
}
288+
}
289+
247290
private String dropIgnored(String s) {
248291
return s.replaceAll("\\s+", "").replaceAll("\"timestamp\":\"[a-zA-Z0-9\\-\\:]+\",", "");
249292
}

src/test/resources/tst_manifests/maven/deps_with_version_range/effectivePom.xml

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,22 +54,22 @@
5454
</pluginRepository>
5555
</pluginRepositories>
5656
<build>
57-
<sourceDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/src/main/java</sourceDirectory>
58-
<scriptSourceDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/src/main/scripts</scriptSourceDirectory>
59-
<testSourceDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/src/test/java</testSourceDirectory>
60-
<outputDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target/classes</outputDirectory>
61-
<testOutputDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target/test-classes</testOutputDirectory>
57+
<sourceDirectory>src/main/java</sourceDirectory>
58+
<scriptSourceDirectory>src/main/scripts</scriptSourceDirectory>
59+
<testSourceDirectory>src/test/java</testSourceDirectory>
60+
<outputDirectory>target/classes</outputDirectory>
61+
<testOutputDirectory>target/test-classes</testOutputDirectory>
6262
<resources>
6363
<resource>
64-
<directory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/src/main/resources</directory>
64+
<directory>src/main/resources</directory>
6565
</resource>
6666
</resources>
6767
<testResources>
6868
<testResource>
69-
<directory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/src/test/resources</directory>
69+
<directory>src/test/resources</directory>
7070
</testResource>
7171
</testResources>
72-
<directory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target</directory>
72+
<directory>target</directory>
7373
<finalName>pom-with-version-range-for-tests-0.0.1</finalName>
7474
<pluginManagement>
7575
<plugins>
@@ -208,7 +208,7 @@
208208
<goal>site</goal>
209209
</goals>
210210
<configuration>
211-
<outputDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target/site</outputDirectory>
211+
<outputDirectory>target/site</outputDirectory>
212212
<reportPlugins>
213213
<reportPlugin>
214214
<groupId>org.apache.maven.plugins</groupId>
@@ -224,7 +224,7 @@
224224
<goal>deploy</goal>
225225
</goals>
226226
<configuration>
227-
<outputDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target/site</outputDirectory>
227+
<outputDirectory>target/site</outputDirectory>
228228
<reportPlugins>
229229
<reportPlugin>
230230
<groupId>org.apache.maven.plugins</groupId>
@@ -235,7 +235,7 @@
235235
</execution>
236236
</executions>
237237
<configuration>
238-
<outputDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target/site</outputDirectory>
238+
<outputDirectory>target/site</outputDirectory>
239239
<reportPlugins>
240240
<reportPlugin>
241241
<groupId>org.apache.maven.plugins</groupId>
@@ -247,6 +247,6 @@
247247
</plugins>
248248
</build>
249249
<reporting>
250-
<outputDirectory>/home/zgrinber/git/trustify-da-java-client/src/test/resources/tst_manifests/maven/deps_with_version_range/target/site</outputDirectory>
250+
<outputDirectory>target/site</outputDirectory>
251251
</reporting>
252252
</project>

0 commit comments

Comments
 (0)