Skip to content

Commit e20a5dc

Browse files
Jammy-LouieJammy-Louie
committed
Maven: infer pom packaging when cached BOM lacks explicit <packaging>
1 parent 979d63b commit e20a5dc

2 files changed

Lines changed: 83 additions & 26 deletions

File tree

‎rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ private List<Pom> getAncestryWithinProject(Pom projectPom, Map<Path, Pom> projec
235235
.normalize();
236236
Pom parentPom = projectPoms.get(parentPath);
237237
return parentPom != null && parentPom.getGav().getGroupId().equals(parent.getGav().getGroupId()) &&
238-
parentPom.getGav().getArtifactId().equals(parent.getGav().getArtifactId()) ? parentPom : null;
238+
parentPom.getGav().getArtifactId().equals(parent.getGav().getArtifactId()) ? parentPom : null;
239239
}
240240

241241
public MavenMetadata downloadMetadata(GroupArtifact groupArtifact, @Nullable ResolvedPom containingPom, List<MavenRepository> repositories) throws MavenDownloadingException {
@@ -275,9 +275,9 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv
275275
try {
276276
String scheme = URI.create(repo.getUri()).getScheme();
277277
String baseUri = repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") +
278-
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
279-
gav.getArtifactId() + '/' +
280-
(gav.getVersion() == null || NAMED_VERSIONS.contains(gav.getVersion().toUpperCase()) ? "" : gav.getVersion() + '/');
278+
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
279+
gav.getArtifactId() + '/' +
280+
(gav.getVersion() == null || NAMED_VERSIONS.contains(gav.getVersion().toUpperCase()) ? "" : gav.getVersion() + '/');
281281

282282
if ("file".equals(scheme)) {
283283
// A maven repository can be expressed as a URI with a file scheme
@@ -372,8 +372,8 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv
372372

373373
String scheme = URI.create(repo.getUri()).getScheme();
374374
String uri = repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") +
375-
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
376-
gav.getArtifactId() + '/';
375+
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
376+
gav.getArtifactId() + '/';
377377

378378
try {
379379
MavenMetadata.Versioning versioning;
@@ -513,7 +513,7 @@ public Pom download(GroupArtifactVersion gav,
513513
// The requested gav might itself have an unresolved placeholder in the version, so also check raw values
514514
for (Pom projectPom : projectPoms.values()) {
515515
if (!projectPom.getGroupId().equals(gav.getGroupId()) ||
516-
!projectPom.getArtifactId().equals(gav.getArtifactId())) {
516+
!projectPom.getArtifactId().equals(gav.getArtifactId())) {
517517
continue;
518518
}
519519

@@ -589,10 +589,10 @@ public Pom download(GroupArtifactVersion gav,
589589

590590
if (result == null) {
591591
URI uri = URI.create(repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") +
592-
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
593-
gav.getArtifactId() + '/' +
594-
gav.getVersion() + '/' +
595-
gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".pom");
592+
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
593+
gav.getArtifactId() + '/' +
594+
gav.getVersion() + '/' +
595+
gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".pom");
596596
uris.add(uri.toString());
597597
if ("file".equals(uri.getScheme())) {
598598
Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion());
@@ -620,9 +620,19 @@ public Pom download(GroupArtifactVersion gav,
620620
RawPom rawPom = RawPom.parse(fis, Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot);
621621
Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav);
622622

623-
if (pom.getPackaging() == null || pom.hasJarPackaging()) {
623+
if (pom.hasJarPackaging()) {
624624
if (!jarFileExists || jarFile.length() == 0) {
625-
// The jar has not been downloaded, making this dependency unusable.
625+
// Explicit jar packaging but the JAR is unusable: skip so the next repo can be tried.
626+
continue;
627+
}
628+
} else if (pom.getPackaging() == null) {
629+
if (!jarFileExists && !pom.getDependencyManagement().isEmpty()) {
630+
// BOM whose <packaging>pom</packaging> was lost in transit (e.g. proxy stripped it).
631+
// Gating on dependencyManagement avoids misidentifying jar artifacts that rely on
632+
// Maven's jar default and happen to have a missing local JAR.
633+
pom = pom.withPackaging("pom");
634+
} else if (!jarFileExists || jarFile.length() == 0) {
635+
// No BOM signal and the JAR is missing or empty: fall through to the next repo
626636
continue;
627637
}
628638
}
@@ -778,9 +788,9 @@ private GroupArtifactVersion handleSnapshotTimestampVersion(GroupArtifactVersion
778788
if (gav.getVersion() != null && gav.getVersion().endsWith("-SNAPSHOT")) {
779789
for (ResolvedGroupArtifactVersion pinnedSnapshotVersion : new MavenExecutionContextView(ctx).getPinnedSnapshotVersions()) {
780790
if (pinnedSnapshotVersion.getDatedSnapshotVersion() != null &&
781-
pinnedSnapshotVersion.getGroupId().equals(gav.getGroupId()) &&
782-
pinnedSnapshotVersion.getArtifactId().equals(gav.getArtifactId()) &&
783-
pinnedSnapshotVersion.getVersion().equals(gav.getVersion())) {
791+
pinnedSnapshotVersion.getGroupId().equals(gav.getGroupId()) &&
792+
pinnedSnapshotVersion.getArtifactId().equals(gav.getArtifactId()) &&
793+
pinnedSnapshotVersion.getVersion().equals(gav.getVersion())) {
784794
return pinnedSnapshotVersion.getDatedSnapshotVersion();
785795
}
786796
}
@@ -887,8 +897,8 @@ Iterable<MavenRepository> distinctNormalizedRepositories(
887897
MavenRepository normalized = normalizeRepository(repo, ctx, containingPom);
888898

889899
if (normalized != null &&
890-
(acceptsVersion == null || repositoryAcceptsVersion(normalized, acceptsVersion, containingPom)) &&
891-
seen.put(normalized.getId(), normalized) == null) {
900+
(acceptsVersion == null || repositoryAcceptsVersion(normalized, acceptsVersion, containingPom)) &&
901+
seen.put(normalized.getId(), normalized) == null) {
892902
return normalized;
893903
}
894904
}
@@ -1074,7 +1084,7 @@ private boolean jarExistsForPomUri(MavenRepository repo, String pomUrl) {
10741084
} catch (FailsafeException failsafeException) {
10751085
Throwable cause = failsafeException.getCause();
10761086
if (cause instanceof HttpSenderResponseException && hasCredentials(repo) &&
1077-
((HttpSenderResponseException) cause).isClientSideException()) {
1087+
((HttpSenderResponseException) cause).isClientSideException()) {
10781088
return Failsafe.with(retryPolicy).get(() -> {
10791089
HttpSender.Request unauthenticated = httpSender.head(jarUrl).build();
10801090
try (HttpSender.Response response = httpSender.send(unauthenticated)) {

‎rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java‎

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -867,19 +867,31 @@ void mergeMetadata() throws Exception {
867867
}
868868

869869
@Test
870-
void skipsLocalInvalidArtifactsMissingJar(@TempDir Path localRepository) throws Exception {
871-
Path localArtifact = localRepository.resolve("com/bad/bad-artifact");
870+
void resolvesLocalPomWithoutPackagingAsBom(@TempDir Path localRepository) throws Exception {
871+
// A cached BOM POM with no <packaging> element and no JAR (BOMs never have JARs) must resolve
872+
// from cache rather than being silently skipped and falling through to a remote fetch.
873+
// The non-empty <dependencyManagement> is the BOM signal that gates the inference.
874+
Path localArtifact = localRepository.resolve("com/example/example-bom");
872875
assertThat(localArtifact.toFile().mkdirs()).isTrue();
873876
Files.createDirectories(localArtifact.resolve("1"));
874877

875-
Path localPom = localRepository.resolve("com/bad/bad-artifact/1/bad-artifact-1.pom");
878+
Path localPom = localRepository.resolve("com/example/example-bom/1/example-bom-1.pom");
876879
Files.writeString(localPom,
877880
//language=xml
878881
"""
879882
<project>
880-
<groupId>com.bad</groupId>
881-
<artifactId>bad-artifact</artifactId>
883+
<groupId>com.example</groupId>
884+
<artifactId>example-bom</artifactId>
882885
<version>1</version>
886+
<dependencyManagement>
887+
<dependencies>
888+
<dependency>
889+
<groupId>com.example</groupId>
890+
<artifactId>some-lib</artifactId>
891+
<version>1.0</version>
892+
</dependency>
893+
</dependencies>
894+
</dependencyManagement>
883895
</project>
884896
"""
885897
);
@@ -891,10 +903,45 @@ void skipsLocalInvalidArtifactsMissingJar(@TempDir Path localRepository) throws
891903
.knownToExist(true)
892904
.build();
893905

894-
// Does not return invalid dependency.
906+
Pom pom = new MavenPomDownloader(emptyMap(), ctx)
907+
.download(new GroupArtifactVersion("com.example", "example-bom", "1"), null, null, List.of(mavenLocal));
908+
assertThat(pom).isNotNull();
909+
assertThat(pom.getPackaging())
910+
.as("Unspecified packaging with no JAR but with dependencyManagement should be inferred as 'pom'")
911+
.isEqualTo("pom");
912+
}
913+
914+
@Test
915+
void doesNotInferPomPackagingForBareArtifactWithoutDependencyManagement(@TempDir Path localRepository) {
916+
// Safety boundary: when the POM has no <packaging>, no JAR, AND no <dependencyManagement>, it could
917+
// be a sloppy jar artifact rather than a BOM. Refuse to infer pom packaging and fall through to the
918+
// next repo so a remote (where available) can supply the missing JAR.
919+
Path localArtifact = localRepository.resolve("com/bare/bare-artifact");
920+
assertThat(localArtifact.toFile().mkdirs()).isTrue();
921+
assertDoesNotThrow(() -> Files.createDirectories(localArtifact.resolve("1")));
922+
923+
Path localPom = localRepository.resolve("com/bare/bare-artifact/1/bare-artifact-1.pom");
924+
assertDoesNotThrow(() -> Files.writeString(localPom,
925+
//language=xml
926+
"""
927+
<project>
928+
<groupId>com.bare</groupId>
929+
<artifactId>bare-artifact</artifactId>
930+
<version>1</version>
931+
</project>
932+
"""
933+
));
934+
935+
MavenRepository mavenLocal = MavenRepository.builder()
936+
.id("local")
937+
.uri(localRepository.toUri().toString())
938+
.snapshots(false)
939+
.knownToExist(true)
940+
.build();
941+
895942
assertThrows(MavenDownloadingException.class, () ->
896943
new MavenPomDownloader(emptyMap(), ctx)
897-
.download(new GroupArtifactVersion("com.bad", "bad-artifact", "1"), null, null, List.of(mavenLocal)));
944+
.download(new GroupArtifactVersion("com.bare", "bare-artifact", "1"), null, null, List.of(mavenLocal)));
898945
}
899946

900947
@Test

0 commit comments

Comments
 (0)
⚡