Skip to content

Commit 3ec8255

Browse files
greg-at-modernegreg-at-moderne
committed
Store exceptions for negative lookup results in Maven Pom Downloader
1 parent ec4fb3d commit 3ec8255

6 files changed

Lines changed: 90 additions & 11 deletions

File tree

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ public class MavenPomDownloader {
7272
private static final String RELEASE = "RELEASE";
7373
private static final List<String> NAMED_VERSIONS = Arrays.asList(LATEST, RELEASE);
7474

75+
private static final String NEGATIVE_RESULT_MESSAGES = "org.openrewrite.maven.internal.MavenPomDownloader.negativeResultMessages";
76+
7577

7678
private final MavenPomCache mavenCache;
7779
private final Map<Path, Pom> projectPoms;
@@ -172,6 +174,15 @@ byte[] sendRequest(HttpSender.Request request) throws IOException, HttpSenderRes
172174
}
173175
}
174176

177+
@SuppressWarnings("unchecked")
178+
private Map<String, String> negativeResultMessages() {
179+
return (Map<String, String>) ctx.getMessages().computeIfAbsent(NEGATIVE_RESULT_MESSAGES, k -> new HashMap<String, String>());
180+
}
181+
182+
private static String negativeResultKey(MavenRepository repo, GroupArtifactVersion gav) {
183+
return repo.getUri() + '|' + gav.getGroupId() + ':' + gav.getArtifactId() + ':' + gav.getVersion();
184+
}
185+
175186
private Map<GroupArtifactVersion, Pom> projectPomsByGav(Map<Path, Pom> projectPoms) {
176187
Map<GroupArtifactVersion, Pom> result = new HashMap<>();
177188
for (Pom projectPom : projectPoms.values()) {
@@ -326,9 +337,15 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv
326337
// If there was no fatal failure while attempting to find metadata and there was no metadata retrieved
327338
// from the current repo, cache an empty result.
328339
mavenCache.putMavenMetadata(URI.create(repo.getUri()), gav, null);
340+
String originalResponse = repositoryResponses.get(repo);
341+
if (originalResponse != null) {
342+
negativeResultMessages().put(negativeResultKey(repo, gav), originalResponse);
343+
}
329344
}
330345
} else if (!result.isPresent()) {
331-
repositoryResponses.put(repo, "Did not attempt to download because of a previous failure to retrieve from this repository.");
346+
String originalResponse = negativeResultMessages().get(negativeResultKey(repo, gav));
347+
repositoryResponses.put(repo, originalResponse != null ? originalResponse :
348+
"Did not attempt to download because of a previous failure to retrieve from this repository.");
332349
}
333350

334351
// Merge metadata from repository and cache metadata result.
@@ -718,6 +735,7 @@ public Pom download(GroupArtifactVersion gav,
718735
if (e.isClientSideException()) {
719736
//If the exception is a common, client-side exception, cache an empty result.
720737
mavenCache.putPom(resolvedGav, null);
738+
negativeResultMessages().put(negativeResultKey(repo, gav), e.getMessage());
721739
}
722740
} catch (IOException | UncheckedIOException e) {
723741
repositoryResponses.put(repo, e.getMessage());
@@ -727,7 +745,9 @@ public Pom download(GroupArtifactVersion gav,
727745
sample.stop(timer.tags("outcome", "cached").register(Metrics.globalRegistry));
728746
return result.get();
729747
} else {
730-
repositoryResponses.put(repo, "Did not attempt to download because of a previous failure to retrieve from this repository.");
748+
String originalResponse = negativeResultMessages().get(negativeResultKey(repo, gav));
749+
repositoryResponses.put(repo, originalResponse != null ? originalResponse :
750+
"Did not attempt to download because of a previous failure to retrieve from this repository.");
731751
}
732752
}
733753

‎rewrite-maven/src/test/java/org/openrewrite/maven/AddDependencyTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import org.intellij.lang.annotations.Language;
1919
import org.jspecify.annotations.Nullable;
20-
import org.junit.jupiter.api.Disabled;
2120
import org.junit.jupiter.api.Test;
2221
import org.junit.jupiter.params.ParameterizedTest;
2322
import org.junit.jupiter.params.provider.ValueSource;
@@ -80,7 +79,6 @@ void addDependenciesOnEmptyProject() {
8079
}
8180

8281
@Test
83-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
8482
void dontAddDuplicateIfUpdateModelOnPriorRecipeCycleFailed() {
8583
rewriteRun(
8684
spec -> spec

‎rewrite-maven/src/test/java/org/openrewrite/maven/MavenDependencyFailuresTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package org.openrewrite.maven;
1717

18-
import org.junit.jupiter.api.Disabled;
1918
import org.junit.jupiter.api.Test;
2019
import org.junit.jupiter.api.io.TempDir;
2120
import org.openrewrite.*;
@@ -42,7 +41,6 @@ class MavenDependencyFailuresTest implements RewriteTest {
4241

4342
@DocumentExample
4443
@Test
45-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
4644
void unresolvableParent() { // Dad said he was heading to the corner store for cigarettes, and hasn't been resolvable for the past 20 years :'(
4745
rewriteRun(
4846
spec -> spec
@@ -81,7 +79,6 @@ void unresolvableParent() { // Dad said he was heading to the corner store for c
8179
}
8280

8381
@Test
84-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
8582
void unresolvableMavenMetadata() {
8683
rewriteRun(
8784
spec -> spec

‎rewrite-maven/src/test/java/org/openrewrite/maven/UpgradeDependencyVersionTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1761,7 +1761,6 @@ void deriveFromNexus() {
17611761
}
17621762

17631763
@Test
1764-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
17651764
void deriveFromNexusUpgrade() {
17661765
rewriteRun(
17671766
spec -> spec.recipe(new UpgradeDependencyVersion("*", "*", "latest.patch", null, null, null)),
@@ -1827,7 +1826,6 @@ void noManagedVersion() {
18271826
}
18281827

18291828
@Test
1830-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
18311829
void badManagedVersion() {
18321830
rewriteRun(
18331831
spec -> spec.recipe(new UpgradeDependencyVersion("*", "*", "latest.patch", null, null, null)),

‎rewrite-maven/src/test/java/org/openrewrite/maven/UpgradePluginVersionTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,6 @@ void update() {
208208

209209
@Issue("https://github.com/openrewrite/rewrite/issues/5065")
210210
@Test
211-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
212211
void repoUnreachable() {
213212
rewriteRun(
214213
spec -> spec.recipe(new UpgradePluginVersion(
@@ -248,7 +247,6 @@ void repoUnreachable() {
248247

249248
@Issue("https://github.com/openrewrite/rewrite/issues/5065")
250249
@Test
251-
@Disabled("2026-05-04 temporarily disabled after Artifactory introduction")
252250
void noNewerVersion() {
253251
rewriteRun(
254252
spec -> spec.recipe(new UpgradePluginVersion(

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,74 @@ public MockResponse dispatch(RecordedRequest request) {
800800
}
801801
}
802802

803+
@Test
804+
void replayOriginalMetadataFailureOnSubsequentCall() throws Exception {
805+
try (var server = new MockWebServer()) {
806+
server.setDispatcher(new Dispatcher() {
807+
@Override
808+
public MockResponse dispatch(RecordedRequest request) {
809+
return new MockResponse().setResponseCode(404).setBody("not found");
810+
}
811+
});
812+
server.start();
813+
814+
// given a repository that always 404s for metadata, and a downloader that shares an ExecutionContext
815+
MavenRepository repository = MavenRepository.builder()
816+
.id("flaky")
817+
.uri("http://%s:%d/".formatted(server.getHostName(), server.getPort()))
818+
.knownToExist(true)
819+
.build();
820+
var downloader = new MavenPomDownloader(emptyMap(), ctx);
821+
var ga = new GroupArtifact("does.not", "exist");
822+
823+
// when downloadMetadata is invoked twice with the same context
824+
String firstMessage = assertThrows(MavenDownloadingException.class,
825+
() -> downloader.downloadMetadata(ga, null, List.of(repository))).getMessage();
826+
String secondMessage = assertThrows(MavenDownloadingException.class,
827+
() -> downloader.downloadMetadata(ga, null, List.of(repository))).getMessage();
828+
829+
// then the second failure replays the original 404, not a generic "did not attempt" notice
830+
assertThat(firstMessage).contains("404");
831+
assertThat(secondMessage)
832+
.contains("404")
833+
.doesNotContain("Did not attempt to download");
834+
}
835+
}
836+
837+
@Test
838+
void replayOriginalPomFailureOnSubsequentCall() throws Exception {
839+
try (var server = new MockWebServer()) {
840+
server.setDispatcher(new Dispatcher() {
841+
@Override
842+
public MockResponse dispatch(RecordedRequest request) {
843+
return new MockResponse().setResponseCode(404).setBody("");
844+
}
845+
});
846+
server.start();
847+
848+
// given a repository that 404s for both POM and JAR, and a downloader that shares an ExecutionContext
849+
MavenRepository repository = MavenRepository.builder()
850+
.id("flaky")
851+
.uri("http://%s:%d/".formatted(server.getHostName(), server.getPort()))
852+
.knownToExist(true)
853+
.build();
854+
var downloader = new MavenPomDownloader(emptyMap(), ctx);
855+
var gav = new GroupArtifactVersion("does.not", "exist", "1.0.0");
856+
857+
// when download is invoked twice with the same context
858+
String firstMessage = assertThrows(MavenDownloadingException.class,
859+
() -> downloader.download(gav, null, null, List.of(repository))).getMessage();
860+
String secondMessage = assertThrows(MavenDownloadingException.class,
861+
() -> downloader.download(gav, null, null, List.of(repository))).getMessage();
862+
863+
// then the second failure replays the original 404, not a generic "did not attempt" notice
864+
assertThat(firstMessage).contains("404");
865+
assertThat(secondMessage)
866+
.contains("404")
867+
.doesNotContain("Did not attempt to download");
868+
}
869+
}
870+
803871
@SuppressWarnings("ConstantConditions")
804872
@Test
805873
void mergeMetadata() throws Exception {

0 commit comments

Comments
 (0)
⚡