From 3ec8255ec25edf76adaa5a5f2a756763e2667d08 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Tue, 5 May 2026 12:06:13 +0200 Subject: [PATCH] Store exceptions for negative lookup results in Maven Pom Downloader --- .../maven/internal/MavenPomDownloader.java | 24 ++++++- .../openrewrite/maven/AddDependencyTest.java | 2 - .../maven/MavenDependencyFailuresTest.java | 3 - .../maven/UpgradeDependencyVersionTest.java | 2 - .../maven/UpgradePluginVersionTest.java | 2 - .../internal/MavenPomDownloaderTest.java | 68 +++++++++++++++++++ 6 files changed, 90 insertions(+), 11 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 f57902a3416..0d37ab574b5 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 @@ -72,6 +72,8 @@ public class MavenPomDownloader { private static final String RELEASE = "RELEASE"; private static final List NAMED_VERSIONS = Arrays.asList(LATEST, RELEASE); + private static final String NEGATIVE_RESULT_MESSAGES = "org.openrewrite.maven.internal.MavenPomDownloader.negativeResultMessages"; + private final MavenPomCache mavenCache; private final Map projectPoms; @@ -172,6 +174,15 @@ byte[] sendRequest(HttpSender.Request request) throws IOException, HttpSenderRes } } + @SuppressWarnings("unchecked") + private Map negativeResultMessages() { + return (Map) ctx.getMessages().computeIfAbsent(NEGATIVE_RESULT_MESSAGES, k -> new HashMap()); + } + + private static String negativeResultKey(MavenRepository repo, GroupArtifactVersion gav) { + return repo.getUri() + '|' + gav.getGroupId() + ':' + gav.getArtifactId() + ':' + gav.getVersion(); + } + private Map projectPomsByGav(Map projectPoms) { Map result = new HashMap<>(); for (Pom projectPom : projectPoms.values()) { @@ -326,9 +337,15 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv // If there was no fatal failure while attempting to find metadata and there was no metadata retrieved // from the current repo, cache an empty result. mavenCache.putMavenMetadata(URI.create(repo.getUri()), gav, null); + String originalResponse = repositoryResponses.get(repo); + if (originalResponse != null) { + negativeResultMessages().put(negativeResultKey(repo, gav), originalResponse); + } } } else if (!result.isPresent()) { - repositoryResponses.put(repo, "Did not attempt to download because of a previous failure to retrieve from this repository."); + String originalResponse = negativeResultMessages().get(negativeResultKey(repo, gav)); + repositoryResponses.put(repo, originalResponse != null ? originalResponse : + "Did not attempt to download because of a previous failure to retrieve from this repository."); } // Merge metadata from repository and cache metadata result. @@ -718,6 +735,7 @@ public Pom download(GroupArtifactVersion gav, if (e.isClientSideException()) { //If the exception is a common, client-side exception, cache an empty result. mavenCache.putPom(resolvedGav, null); + negativeResultMessages().put(negativeResultKey(repo, gav), e.getMessage()); } } catch (IOException | UncheckedIOException e) { repositoryResponses.put(repo, e.getMessage()); @@ -727,7 +745,9 @@ public Pom download(GroupArtifactVersion gav, sample.stop(timer.tags("outcome", "cached").register(Metrics.globalRegistry)); return result.get(); } else { - repositoryResponses.put(repo, "Did not attempt to download because of a previous failure to retrieve from this repository."); + String originalResponse = negativeResultMessages().get(negativeResultKey(repo, gav)); + repositoryResponses.put(repo, originalResponse != null ? originalResponse : + "Did not attempt to download because of a previous failure to retrieve from this repository."); } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java index 3ffadf52726..ef9278af337 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java @@ -17,7 +17,6 @@ import org.intellij.lang.annotations.Language; import org.jspecify.annotations.Nullable; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -80,7 +79,6 @@ void addDependenciesOnEmptyProject() { } @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void dontAddDuplicateIfUpdateModelOnPriorRecipeCycleFailed() { rewriteRun( spec -> spec diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/MavenDependencyFailuresTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/MavenDependencyFailuresTest.java index 2575ae92d51..2c4483083a0 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/MavenDependencyFailuresTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/MavenDependencyFailuresTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.maven; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.openrewrite.*; @@ -42,7 +41,6 @@ class MavenDependencyFailuresTest implements RewriteTest { @DocumentExample @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void unresolvableParent() { // Dad said he was heading to the corner store for cigarettes, and hasn't been resolvable for the past 20 years :'( rewriteRun( spec -> spec @@ -81,7 +79,6 @@ void unresolvableParent() { // Dad said he was heading to the corner store for c } @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void unresolvableMavenMetadata() { rewriteRun( spec -> spec diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java index a5be98f3f2f..4213764069c 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java @@ -1761,7 +1761,6 @@ void deriveFromNexus() { } @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void deriveFromNexusUpgrade() { rewriteRun( spec -> spec.recipe(new UpgradeDependencyVersion("*", "*", "latest.patch", null, null, null)), @@ -1827,7 +1826,6 @@ void noManagedVersion() { } @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void badManagedVersion() { rewriteRun( spec -> spec.recipe(new UpgradeDependencyVersion("*", "*", "latest.patch", null, null, null)), diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradePluginVersionTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradePluginVersionTest.java index c4d50c7e13f..0d036e58723 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradePluginVersionTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/UpgradePluginVersionTest.java @@ -208,7 +208,6 @@ void update() { @Issue("https://github.com/openrewrite/rewrite/issues/5065") @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void repoUnreachable() { rewriteRun( spec -> spec.recipe(new UpgradePluginVersion( @@ -248,7 +247,6 @@ void repoUnreachable() { @Issue("https://github.com/openrewrite/rewrite/issues/5065") @Test - @Disabled("2026-05-04 temporarily disabled after Artifactory introduction") void noNewerVersion() { rewriteRun( spec -> spec.recipe(new UpgradePluginVersion( 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..2d381c7628a 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 @@ -800,6 +800,74 @@ public MockResponse dispatch(RecordedRequest request) { } } + @Test + void replayOriginalMetadataFailureOnSubsequentCall() throws Exception { + try (var server = new MockWebServer()) { + server.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest request) { + return new MockResponse().setResponseCode(404).setBody("not found"); + } + }); + server.start(); + + // given a repository that always 404s for metadata, and a downloader that shares an ExecutionContext + MavenRepository repository = MavenRepository.builder() + .id("flaky") + .uri("http://%s:%d/".formatted(server.getHostName(), server.getPort())) + .knownToExist(true) + .build(); + var downloader = new MavenPomDownloader(emptyMap(), ctx); + var ga = new GroupArtifact("does.not", "exist"); + + // when downloadMetadata is invoked twice with the same context + String firstMessage = assertThrows(MavenDownloadingException.class, + () -> downloader.downloadMetadata(ga, null, List.of(repository))).getMessage(); + String secondMessage = assertThrows(MavenDownloadingException.class, + () -> downloader.downloadMetadata(ga, null, List.of(repository))).getMessage(); + + // then the second failure replays the original 404, not a generic "did not attempt" notice + assertThat(firstMessage).contains("404"); + assertThat(secondMessage) + .contains("404") + .doesNotContain("Did not attempt to download"); + } + } + + @Test + void replayOriginalPomFailureOnSubsequentCall() throws Exception { + try (var server = new MockWebServer()) { + server.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest request) { + return new MockResponse().setResponseCode(404).setBody(""); + } + }); + server.start(); + + // given a repository that 404s for both POM and JAR, and a downloader that shares an ExecutionContext + MavenRepository repository = MavenRepository.builder() + .id("flaky") + .uri("http://%s:%d/".formatted(server.getHostName(), server.getPort())) + .knownToExist(true) + .build(); + var downloader = new MavenPomDownloader(emptyMap(), ctx); + var gav = new GroupArtifactVersion("does.not", "exist", "1.0.0"); + + // when download is invoked twice with the same context + String firstMessage = assertThrows(MavenDownloadingException.class, + () -> downloader.download(gav, null, null, List.of(repository))).getMessage(); + String secondMessage = assertThrows(MavenDownloadingException.class, + () -> downloader.download(gav, null, null, List.of(repository))).getMessage(); + + // then the second failure replays the original 404, not a generic "did not attempt" notice + assertThat(firstMessage).contains("404"); + assertThat(secondMessage) + .contains("404") + .doesNotContain("Did not attempt to download"); + } + } + @SuppressWarnings("ConstantConditions") @Test void mergeMetadata() throws Exception {