diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java index f57902a3416..5bfb6b0fd75 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java @@ -235,7 +235,7 @@ private List getAncestryWithinProject(Pom projectPom, Map 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 repositories) throws MavenDownloadingException { @@ -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 @@ -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; @@ -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; } @@ -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()); @@ -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 pom 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; } } @@ -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(); } } @@ -887,8 +897,8 @@ Iterable 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; } } @@ -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)) { diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java index bc30a7d3c4b..95ffa6aafa0 100755 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java @@ -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 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 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 """ - com.bad - bad-artifact + com.example + example-bom 1 + + + + com.example + some-lib + 1.0 + + + """ ); @@ -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 , no JAR, AND no , 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 + """ + + com.bare + bare-artifact + 1 + + """ + )); + + 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