Skip to content

Maven: infer pom packaging when cached BOM lacks explicit <packaging> #7536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jammy-Louie wants to merge 1 commit into
base: main
Choose a base branch
from
+83 −26
Open

Maven: infer pom packaging when cached BOM lacks explicit <packaging> #7536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 29 additions & 19 deletions rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private List<Pom> getAncestryWithinProject(Pom projectPom, Map<Path, Pom> projec
.normalize();
Pom parentPom = projectPoms.get(parentPath);
return parentPom != null && parentPom.getGav().getGroupId().equals(parent.getGav().getGroupId()) &&
parentPom.getGav().getArtifactId().equals(parent.getGav().getArtifactId()) ? parentPom : null;
parentPom.getGav().getArtifactId().equals(parent.getGav().getArtifactId()) ? parentPom : null;
}

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

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

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

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

Expand Down Expand Up @@ -589,10 +589,10 @@ public Pom download(GroupArtifactVersion gav,

if (result == null) {
URI uri = URI.create(repo.getUri() + (repo.getUri().endsWith("/") ? "" : "/") +
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
gav.getArtifactId() + '/' +
gav.getVersion() + '/' +
gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".pom");
requireNonNull(gav.getGroupId()).replace('.', '/') + '/' +
gav.getArtifactId() + '/' +
gav.getVersion() + '/' +
gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".pom");
uris.add(uri.toString());
if ("file".equals(uri.getScheme())) {
Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion());
Expand Down Expand Up @@ -620,9 +620,19 @@ public Pom download(GroupArtifactVersion gav,
RawPom rawPom = RawPom.parse(fis, Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot);
Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav);

if (pom.getPackaging() == null || pom.hasJarPackaging()) {
if (pom.hasJarPackaging()) {
if (!jarFileExists || jarFile.length() == 0) {
// The jar has not been downloaded, making this dependency unusable.
// Explicit jar packaging but the JAR is unusable: skip so the next repo can be tried.
continue;
}
} else if (pom.getPackaging() == null) {
if (!jarFileExists && !pom.getDependencyManagement().isEmpty()) {
// BOM whose <packaging>pom</packaging> was lost in transit (e.g. proxy stripped it).
// Gating on dependencyManagement avoids misidentifying jar artifacts that rely on
// Maven's jar default and happen to have a missing local JAR.
pom = pom.withPackaging("pom");
} else if (!jarFileExists || jarFile.length() == 0) {
// No BOM signal and the JAR is missing or empty: fall through to the next repo
continue;
}
}
Expand Down Expand Up @@ -778,9 +788,9 @@ private GroupArtifactVersion handleSnapshotTimestampVersion(GroupArtifactVersion
if (gav.getVersion() != null && gav.getVersion().endsWith("-SNAPSHOT")) {
for (ResolvedGroupArtifactVersion pinnedSnapshotVersion : new MavenExecutionContextView(ctx).getPinnedSnapshotVersions()) {
if (pinnedSnapshotVersion.getDatedSnapshotVersion() != null &&
pinnedSnapshotVersion.getGroupId().equals(gav.getGroupId()) &&
pinnedSnapshotVersion.getArtifactId().equals(gav.getArtifactId()) &&
pinnedSnapshotVersion.getVersion().equals(gav.getVersion())) {
pinnedSnapshotVersion.getGroupId().equals(gav.getGroupId()) &&
pinnedSnapshotVersion.getArtifactId().equals(gav.getArtifactId()) &&
pinnedSnapshotVersion.getVersion().equals(gav.getVersion())) {
return pinnedSnapshotVersion.getDatedSnapshotVersion();
}
}
Expand Down Expand Up @@ -887,8 +897,8 @@ Iterable<MavenRepository> distinctNormalizedRepositories(
MavenRepository normalized = normalizeRepository(repo, ctx, containingPom);

if (normalized != null &&
(acceptsVersion == null || repositoryAcceptsVersion(normalized, acceptsVersion, containingPom)) &&
seen.put(normalized.getId(), normalized) == null) {
(acceptsVersion == null || repositoryAcceptsVersion(normalized, acceptsVersion, containingPom)) &&
seen.put(normalized.getId(), normalized) == null) {
return normalized;
}
}
Expand Down Expand Up @@ -1074,7 +1084,7 @@ private boolean jarExistsForPomUri(MavenRepository repo, String pomUrl) {
} catch (FailsafeException failsafeException) {
Throwable cause = failsafeException.getCause();
if (cause instanceof HttpSenderResponseException && hasCredentials(repo) &&
((HttpSenderResponseException) cause).isClientSideException()) {
((HttpSenderResponseException) cause).isClientSideException()) {
return Failsafe.with(retryPolicy).get(() -> {
HttpSender.Request unauthenticated = httpSender.head(jarUrl).build();
try (HttpSender.Response response = httpSender.send(unauthenticated)) {
Expand Down
61 changes: 54 additions & 7 deletions rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,19 +867,31 @@ void mergeMetadata() throws Exception {
}

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

Path localPom = localRepository.resolve("com/bad/bad-artifact/1/bad-artifact-1.pom");
Path localPom = localRepository.resolve("com/example/example-bom/1/example-bom-1.pom");
Files.writeString(localPom,
//language=xml
"""
<project>
<groupId>com.bad</groupId>
<artifactId>bad-artifact</artifactId>
<groupId>com.example</groupId>
<artifactId>example-bom</artifactId>
<version>1</version>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>com.example</groupId>
<artifactId>some-lib</artifactId>
<version>1.0</version>
</dependency>
</dependencies>
</dependencyManagement>
</project>
"""
);
Expand All @@ -891,10 +903,45 @@ void skipsLocalInvalidArtifactsMissingJar(@TempDir Path localRepository) throws
.knownToExist(true)
.build();

// Does not return invalid dependency.
Pom pom = new MavenPomDownloader(emptyMap(), ctx)
.download(new GroupArtifactVersion("com.example", "example-bom", "1"), null, null, List.of(mavenLocal));
assertThat(pom).isNotNull();
assertThat(pom.getPackaging())
.as("Unspecified packaging with no JAR but with dependencyManagement should be inferred as 'pom'")
.isEqualTo("pom");
}

@Test
void doesNotInferPomPackagingForBareArtifactWithoutDependencyManagement(@TempDir Path localRepository) {
// Safety boundary: when the POM has no <packaging>, no JAR, AND no <dependencyManagement>, it could
// be a sloppy jar artifact rather than a BOM. Refuse to infer pom packaging and fall through to the
// next repo so a remote (where available) can supply the missing JAR.
Path localArtifact = localRepository.resolve("com/bare/bare-artifact");
assertThat(localArtifact.toFile().mkdirs()).isTrue();
assertDoesNotThrow(() -> Files.createDirectories(localArtifact.resolve("1")));

Path localPom = localRepository.resolve("com/bare/bare-artifact/1/bare-artifact-1.pom");
assertDoesNotThrow(() -> Files.writeString(localPom,
//language=xml
"""
<project>
<groupId>com.bare</groupId>
<artifactId>bare-artifact</artifactId>
<version>1</version>
</project>
"""
));

MavenRepository mavenLocal = MavenRepository.builder()
.id("local")
.uri(localRepository.toUri().toString())
.snapshots(false)
.knownToExist(true)
.build();

assertThrows(MavenDownloadingException.class, () ->
new MavenPomDownloader(emptyMap(), ctx)
.download(new GroupArtifactVersion("com.bad", "bad-artifact", "1"), null, null, List.of(mavenLocal)));
.download(new GroupArtifactVersion("com.bare", "bare-artifact", "1"), null, null, List.of(mavenLocal)));
}

@Test
Expand Down
⚡