From abb80e16f579275811d4166bfd6387f504b25b16 Mon Sep 17 00:00:00 2001 From: adastraperangusta <39689932+adastraperangusta@users.noreply.github.com> Date: Thu, 12 Dec 2024 23:21:23 +0100 Subject: [PATCH] Allow pom download failures (#4738) * add configuration option * add configuration option to allow pom download failure * code formatting and typo * Apply formatter to minimize diff * Update rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java Co-authored-by: Tim te Beek * add test for the expected behaviour * file scheme handling * we don't nedd a property anymore * prepare test for remote repository case * tests updates * try for jar if remote pom doesn't exist * Don't download the jar bytes; only check if response is successful * Minimize diff as compared to main for now * Record the absence of the pom file when jar is found * Minor polish --------- Co-authored-by: Stef Co-authored-by: Tim te Beek Co-authored-by: Tim te Beek --- .../maven/internal/MavenPomDownloader.java | 114 +++++++++++++----- .../internal/MavenPomDownloaderTest.java | 93 ++++++++++++++ .../maven/tree/ResolvedPomTest.java | 104 ++++++++++++++++ 3 files changed, 283 insertions(+), 28 deletions(-) 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 364078042d7..26e59770b35 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 @@ -561,51 +561,75 @@ public Pom download(GroupArtifactVersion gav, Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion()); try { - File f = new File(uri); + File pomFile = new File(uri); + File jarFile = pomFile.toPath().resolveSibling(gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".jar").toFile(); - //NOTE: The pom may exist without a .jar artifact if the pom packaging is "pom" - if (!f.exists()) { + //NOTE: + // - The pom may exist without a .jar artifact if the pom packaging is "pom" + // - The jar may exist without a pom, if manually installed + if (!pomFile.exists() && !jarFile.exists()) { continue; } - try (FileInputStream fis = new FileInputStream(f)) { - 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()) { - File jar = f.toPath().resolveSibling(gav.getArtifactId() + '-' + versionMaybeDatedSnapshot + ".jar").toFile(); - if (!jar.exists() || jar.length() == 0) { - // The jar has not been downloaded, making this dependency unusable. - continue; - } + RawPom rawPom; + if (pomFile.exists()) { + try (FileInputStream fis = new FileInputStream(pomFile)) { + rawPom = RawPom.parse(fis, Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot); } + } else { + // Record the absense of the pom file + ctx.getResolutionListener().downloadError(gav, uris, (containingPom == null) ? null : containingPom.getRequested()); + // infer rawPom from jar + rawPom = rawPomFromGav(gav); + } - if (repo.getUri().equals(MavenRepository.MAVEN_LOCAL_DEFAULT.getUri())) { - // so that the repository path is the same regardless of username - pom = pom.withRepository(MavenRepository.MAVEN_LOCAL_USER_NEUTRAL); - } + Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav); - if (!Objects.equals(versionMaybeDatedSnapshot, pom.getVersion())) { - pom = pom.withGav(pom.getGav().withDatedSnapshotVersion(versionMaybeDatedSnapshot)); + if (pom.getPackaging() == null || pom.hasJarPackaging()) { + if (!jarFile.exists() || jarFile.length() == 0) { + // The jar has not been downloaded, making this dependency unusable. + continue; } - mavenCache.putPom(resolvedGav, pom); - ctx.getResolutionListener().downloadSuccess(resolvedGav, containingPom); - sample.stop(timer.tags("outcome", "from maven local").register(Metrics.globalRegistry)); - return pom; } + + if (repo.getUri().equals(MavenRepository.MAVEN_LOCAL_DEFAULT.getUri())) { + // so that the repository path is the same regardless of username + pom = pom.withRepository(MavenRepository.MAVEN_LOCAL_USER_NEUTRAL); + } + + if (!Objects.equals(versionMaybeDatedSnapshot, pom.getVersion())) { + pom = pom.withGav(pom.getGav().withDatedSnapshotVersion(versionMaybeDatedSnapshot)); + } + mavenCache.putPom(resolvedGav, pom); + ctx.getResolutionListener().downloadSuccess(resolvedGav, containingPom); + sample.stop(timer.tags("outcome", "from maven local").register(Metrics.globalRegistry)); + return pom; } catch (IOException e) { // unable to read the pom from a file-based repository. repositoryResponses.put(repo, e.getMessage()); } } else { try { - byte[] responseBody = requestAsAuthenticatedOrAnonymous(repo, uri.toString()); + RawPom rawPom; + try { + byte[] responseBody = requestAsAuthenticatedOrAnonymous(repo, uri.toString()); + rawPom = RawPom.parse( + new ByteArrayInputStream(responseBody), + Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot + ); + } catch (HttpSenderResponseException e) { + repositoryResponses.put(repo, e.getMessage()); + // When `pom` is not found, try to see if `jar` exists for the same GAV + if (!e.isClientSideException() || !jarExistsForPomUri(repo, uri.toString())) { + throw e; + } + // Record the absense of the pom file + ctx.getResolutionListener().downloadError(gav, uris, (containingPom == null) ? null : containingPom.getRequested()); + // Continue with a recreated pom + rawPom = rawPomFromGav(gav); + } Path inputPath = Paths.get(gav.getGroupId(), gav.getArtifactId(), gav.getVersion()); - RawPom rawPom = RawPom.parse( - new ByteArrayInputStream(responseBody), - Objects.equals(versionMaybeDatedSnapshot, gav.getVersion()) ? null : versionMaybeDatedSnapshot - ); Pom pom = rawPom.toPom(inputPath, repo).withGav(resolvedGav); if (!Objects.equals(versionMaybeDatedSnapshot, pom.getVersion())) { pom = pom.withGav(pom.getGav().withDatedSnapshotVersion(versionMaybeDatedSnapshot)); @@ -637,6 +661,12 @@ public Pom download(GroupArtifactVersion gav, .setRepositoryResponses(repositoryResponses); } + private RawPom rawPomFromGav(GroupArtifactVersion gav) { + return new RawPom(null, null, gav.getGroupId(), gav.getArtifactId(), gav.getVersion(), null, + null, null, null, "jar", null, null, null, + null, null, null, null, null, null); + } + /** * Gets the base version from snapshot timestamp version. */ @@ -850,6 +880,34 @@ private ReachabilityResult reachable(HttpSender.Request.Builder request) { } } + private boolean jarExistsForPomUri(MavenRepository repo, String pomUrl) { + String jarUrl = pomUrl.replaceAll("\\.pom$", ".jar"); + try { + try { + return Failsafe.with(retryPolicy).get(() -> { + HttpSender.Request authenticated = applyAuthenticationAndTimeoutToRequest(repo, httpSender.get(jarUrl)).build(); + try (HttpSender.Response response = httpSender.send(authenticated)) { + return response.isSuccessful(); + } + }); + } catch (FailsafeException failsafeException) { + Throwable cause = failsafeException.getCause(); + if (cause instanceof HttpSenderResponseException && hasCredentials(repo) && + ((HttpSenderResponseException) cause).isClientSideException()) { + return Failsafe.with(retryPolicy).get(() -> { + HttpSender.Request unauthenticated = httpSender.get(jarUrl).build(); + try (HttpSender.Response response = httpSender.send(unauthenticated)) { + return response.isSuccessful(); + } + }); + } + } + } catch (Throwable e) { + // Not interested in exceptions downloading the jar; we'll throw the original exception for the pom + } + return false; + } + /** * Replicates Apache Maven's behavior to attempt anonymous download if repository credentials prove invalid 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 f16810183a6..152265c02fd 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 @@ -573,6 +573,42 @@ void skipsLocalInvalidArtifactsEmptyJar(@TempDir Path localRepository) throws IO .download(new GroupArtifactVersion("com.bad", "bad-artifact", "1"), null, null, List.of(mavenLocal))); } + @Test + void dontAllowPomDowloadFailureWithoutJar(@TempDir Path localRepository) throws IOException, MavenDownloadingException { + MavenRepository mavenLocal = MavenRepository.builder() + .id("local") + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + + // Do not return invalid dependency + assertThrows(MavenDownloadingException.class, () -> new MavenPomDownloader(emptyMap(), ctx) + .download(new GroupArtifactVersion("com.bad", "bad-artifact", "1"), null, null, List.of(mavenLocal))); + } + + @Test + void allowPomDowloadFailureWithJar(@TempDir Path localRepository) throws IOException, MavenDownloadingException { + MavenRepository mavenLocal = MavenRepository.builder() + .id("local") + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + + // Create a valid jar + Path localJar = localRepository.resolve("com/some/some-artifact/1/some-artifact-1.jar"); + assertThat(localJar.getParent().toFile().mkdirs()).isTrue(); + Files.writeString(localJar, "some content not to be empty"); + + // Do not throw exception since we have a jar + var result = new MavenPomDownloader(emptyMap(), ctx) + .download(new GroupArtifactVersion("com.some", "some-artifact", "1"), null, null, List.of(mavenLocal)); + assertThat(result.getGav().getGroupId()).isEqualTo("com.some"); + assertThat(result.getGav().getArtifactId()).isEqualTo("some-artifact"); + assertThat(result.getGav().getVersion()).isEqualTo("1"); + } + @Test void doNotRenameRepoForCustomMavenLocal(@TempDir Path tempDir) throws MavenDownloadingException, IOException { GroupArtifactVersion gav = createArtifact(tempDir); @@ -1023,5 +1059,62 @@ public MockResponse dispatch(RecordedRequest recordedRequest) { throw new RuntimeException(e); } } + + @Test + @DisplayName("Throw exception if there is no pom and no jar for the artifact") + @Issue("https://github.com/openrewrite/rewrite/issues/4687") + void pomNotFoundWithNoJarShouldThrow() throws Exception { + try (MockWebServer mockRepo = getMockServer()) { + mockRepo.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest recordedRequest) { + assert recordedRequest.getPath() != null; + return new MockResponse().setResponseCode(404).setBody(""); + } + }); + mockRepo.start(); + var repositories = List.of(MavenRepository.builder() + .id("id") + .uri("http://%s:%d/maven".formatted(mockRepo.getHostName(), mockRepo.getPort())) + .username("user") + .password("pass") + .build()); + + var downloader = new MavenPomDownloader(emptyMap(), ctx); + var gav = new GroupArtifactVersion("fred", "fred", "1"); + assertThrows(MavenDownloadingException.class, () -> downloader.download(gav, null, null, repositories)); + } + } + + @Test + @DisplayName("Don't throw exception if there is no pom and but there is a jar for the artifact") + @Issue("https://github.com/openrewrite/rewrite/issues/4687") + void pomNotFoundWithJarFoundShouldntThrow() throws Exception { + try (MockWebServer mockRepo = getMockServer()) { + mockRepo.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest recordedRequest) { + assert recordedRequest.getPath() != null; + if (recordedRequest.getPath().endsWith("fred/fred/1/fred-1.pom")) + return new MockResponse().setResponseCode(404).setBody(""); + return new MockResponse().setResponseCode(200).setBody("some bytes so the jar isn't empty"); + } + }); + mockRepo.start(); + var repositories = List.of(MavenRepository.builder() + .id("id") + .uri("http://%s:%d/maven".formatted(mockRepo.getHostName(), mockRepo.getPort())) + .username("user") + .password("pass") + .build()); + + var gav = new GroupArtifactVersion("fred", "fred", "1"); + var downloader = new MavenPomDownloader(emptyMap(), ctx); + Pom downloaded = downloader.download(gav, null, null, repositories); + assertThat(downloaded.getGav().getGroupId()).isEqualTo("fred"); + assertThat(downloaded.getGav().getArtifactId()).isEqualTo("fred"); + assertThat(downloaded.getGav().getVersion()).isEqualTo("1"); + } + } } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java index 0d2714390ab..b1c0e12f1bd 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/ResolvedPomTest.java @@ -16,9 +16,20 @@ package org.openrewrite.maven.tree; import com.fasterxml.jackson.databind.ObjectMapper; +import org.intellij.lang.annotations.Language; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.Issue; +import org.openrewrite.maven.MavenExecutionContextView; import org.openrewrite.test.RewriteTest; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -278,4 +289,97 @@ void resolveExecutionsFromDifferentParents() { ) ); } + + @Nested + @Issue("https://github.com/openrewrite/rewrite/issues/4687") + class TolerateMissingPom { + + @Language("xml") + private static final String POM_WITH_DEPENDENCY = """ + + 4.0.0 + foo + bar + 1.0-SNAPSHOT + + + com.some + some-artifact + 1 + + + + """; + + @TempDir + Path localRepository; + @TempDir + Path localRepository2; + + @Test + void singleRepositoryContainingJar() throws IOException { + MavenRepository mavenLocal = createMavenRepository(localRepository, "local"); + createJarFile(localRepository); + + List> downloadErrorArgs = new ArrayList<>(); + MavenExecutionContextView ctx = MavenExecutionContextView.view(new InMemoryExecutionContext(Throwable::printStackTrace)); + ctx.setRepositories(List.of(mavenLocal)); + ctx.setResolutionListener(new ResolutionEventListener() { + @Override + public void downloadError(GroupArtifactVersion gav, List attemptedUris, @Nullable Pom containing) { + List list = new ArrayList<>(); + list.add(gav); + list.add(attemptedUris); + list.add(containing); + downloadErrorArgs.add(list); + } + }); + rewriteRun( + spec -> spec.executionContext(ctx), + pomXml(POM_WITH_DEPENDENCY) + ); + assertThat(downloadErrorArgs).hasSize(1); + } + + @Test + void twoRepositoriesSecondContainingJar() throws IOException { + MavenRepository mavenLocal = createMavenRepository(localRepository, "local"); + MavenRepository mavenLocal2 = createMavenRepository(localRepository2, "local2"); + createJarFile(localRepository2); + + List> downloadErrorArgs = new ArrayList<>(); + MavenExecutionContextView ctx = MavenExecutionContextView.view(new InMemoryExecutionContext(Throwable::printStackTrace)); + ctx.setRepositories(List.of(mavenLocal, mavenLocal2)); + ctx.setResolutionListener(new ResolutionEventListener() { + @Override + public void downloadError(GroupArtifactVersion gav, List attemptedUris, @Nullable Pom containing) { + List list = new ArrayList<>(); + list.add(gav); + list.add(attemptedUris); + list.add(containing); + downloadErrorArgs.add(list); + } + }); + rewriteRun( + spec -> spec.executionContext(ctx), + pomXml(POM_WITH_DEPENDENCY) + ); + assertThat(downloadErrorArgs).hasSize(1); + } + + private static void createJarFile(Path localRepository1) throws IOException { + Path localJar = localRepository1.resolve("com/some/some-artifact/1/some-artifact-1.jar"); + assertThat(localJar.getParent().toFile().mkdirs()).isTrue(); + Files.writeString(localJar, "some content not to be empty"); + } + + private static MavenRepository createMavenRepository(Path localRepository, String name) { + return MavenRepository.builder() + .id(name) + .uri(localRepository.toUri().toString()) + .snapshots(false) + .knownToExist(true) + .build(); + } + } }