From 4963722988b437c0debf5d92ddf4faf7cc6b4ac5 Mon Sep 17 00:00:00 2001 From: Jonathan Schneider Date: Thu, 27 Aug 2026 13:11:46 -0400 Subject: [PATCH 1/3] Maven resolution: cool down an endpoint after it answers HTTP 429 A 429 is transient, so it is deliberately never negative-cached; but nothing remembered it either, so every subsequent metadata or POM lookup re-asked the throttled host, and a failed metadata request was followed by a second request for the directory listing. MavenExecutionContextView now keeps a host:port -> skip-until map next to the unreachable-endpoints set. sendRequest populates it on a 429 with a 60s cooldown; distinctNormalizedRepositories skips a cooling-down repository (reported through repositoryAccessFailedPreviously) and deriveMetadata does not follow a 429 with a listing request. Part 1 of #8682. --- .../maven/MavenExecutionContextView.java | 15 +++++++ .../maven/internal/MavenPomDownloader.java | 43 ++++++++++++++++++- .../internal/MavenPomDownloaderTest.java | 37 ++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenExecutionContextView.java b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenExecutionContextView.java index b77adf7f8b7..4ffbe12cf86 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/MavenExecutionContextView.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/MavenExecutionContextView.java @@ -31,6 +31,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; +import java.time.Instant; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; @@ -57,6 +58,7 @@ public class MavenExecutionContextView extends DelegatingExecutionContext { private static final String MAVEN_RESOLUTION_TIME = "org.openrewrite.maven.resolutionTime"; private static final String MAVEN_UNREACHABLE_ENDPOINTS = "org.openrewrite.maven.unreachableEndpoints"; private static final String MAVEN_AUTHENTICATION_REQUIRED_ENDPOINTS = "org.openrewrite.maven.authenticationRequiredEndpoints"; + private static final String MAVEN_THROTTLED_ENDPOINTS = "org.openrewrite.maven.throttledEndpoints"; public MavenExecutionContextView(ExecutionContext delegate) { super(delegate); @@ -107,6 +109,19 @@ public Set getAuthenticationRequiredEndpoints() { return computeMessageIfAbsent(MAVEN_AUTHENTICATION_REQUIRED_ENDPOINTS, k -> ConcurrentHashMap.newKeySet()); } + /** + * The rate-limiting counterpart to {@link #getUnreachableEndpoints()}: connection endpoints, each a + * {@code host:port}, that answered HTTP 429 during this execution, mapped to the instant until which + * requests to them are skipped rather than sent. A 429 is transient, so it is never negative-cached; this + * map is what keeps every subsequent lookup from re-asking a host that has already said it is throttling. + * As with unreachable endpoints, the key is {@code host:port} rather than the full URI because rate limits + * are imposed per host, not per path. The map is concurrent because resolution runs across multiple + * threads sharing one execution context. + */ + public Map getThrottledEndpoints() { + return computeMessageIfAbsent(MAVEN_THROTTLED_ENDPOINTS, k -> new ConcurrentHashMap<>()); + } + public MavenExecutionContextView setResolutionListener(ResolutionEventListener listener) { putMessage(MAVEN_RESOLUTION_LISTENER, listener); return this; 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 9d0e1877616..712af35993b 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 @@ -44,6 +44,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; +import java.time.Instant; import java.time.ZonedDateTime; import java.util.*; import java.util.concurrent.TimeoutException; @@ -64,6 +65,8 @@ public class MavenPomDownloader { .withMaxRetries(5) .build(); + private static final Duration THROTTLE_COOLDOWN = Duration.ofSeconds(60); + private static final Pattern SNAPSHOT_TIMESTAMP = Pattern.compile("^(.*-)?([0-9]{8}\\.[0-9]{6}-[0-9]+)$"); private static final String SNAPSHOT = "SNAPSHOT"; @@ -161,7 +164,14 @@ byte[] sendRequest(HttpSender.Request request) throws IOException, HttpSenderRes }); } catch (FailsafeException failsafeException) { if (failsafeException.getCause() instanceof HttpSenderResponseException) { - throw (HttpSenderResponseException) failsafeException.getCause(); + HttpSenderResponseException e = (HttpSenderResponseException) failsafeException.getCause(); + if (e.isThrottled()) { + String endpoint = endpointOrNull(URI.create(request.getUrl().toString())); + if (endpoint != null) { + ctx.getThrottledEndpoints().put(endpoint, Instant.now().plus(THROTTLE_COOLDOWN)); + } + } + throw e; } throw failsafeException; } catch (UncheckedIOException e) { @@ -171,6 +181,26 @@ byte[] sendRequest(HttpSender.Request request) throws IOException, HttpSenderRes } } + /** + * Whether the repository's endpoint answered HTTP 429 recently enough that it is still cooling down. + */ + private boolean throttled(MavenRepository repo) { + Map throttled = ctx.getThrottledEndpoints(); + if (throttled.isEmpty()) { + return false; + } + String endpoint = endpointOrNull(URI.create(repo.getUri())); + Instant until = endpoint == null ? null : throttled.get(endpoint); + if (until == null) { + return false; + } + if (Instant.now().isBefore(until)) { + return true; + } + throttled.remove(endpoint, until); + return false; + } + private Map projectPomsByGav(Map projectPoms) { Map result = new HashMap<>(); for (Pom projectPom : projectPoms.values()) { @@ -363,9 +393,10 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv * @return Metadata or null if the metadata cannot be derived. */ private @Nullable MavenMetadata deriveMetadata(GroupArtifactVersion gav, MavenRepository repo) throws HttpSenderResponseException, IOException, MavenDownloadingException { - if ((repo.getDeriveMetadataIfMissing() != null && !repo.getDeriveMetadataIfMissing()) || gav.getVersion() != null) { + if ((repo.getDeriveMetadataIfMissing() != null && !repo.getDeriveMetadataIfMissing()) || gav.getVersion() != null || throttled(repo)) { // Do not derive metadata if we cannot navigate/browse the artifacts. // Do not derive metadata if a specific version has been defined. + // Do not derive metadata from an endpoint that has just answered 429 to the metadata request. return null; } @@ -890,6 +921,10 @@ Iterable distinctNormalizedRepositories( if (normalized != null && (acceptsVersion == null || repositoryAcceptsVersion(normalized, acceptsVersion, containingPom)) && seen.put(normalized.getId(), normalized) == null) { + if (throttled(normalized)) { + ctx.getResolutionListener().repositoryAccessFailedPreviously(normalized.getUri()); + continue; + } return normalized; } } @@ -1301,6 +1336,10 @@ public boolean isAccessDenied() { return responseCode != null && 400 < responseCode && responseCode <= 403; } + public boolean isThrottled() { + return responseCode != null && responseCode == 429; + } + // Any response code below 100 implies that no connection was made. Sometimes 0 or -1 is used for connection failures. // 408 is sometimes used for connection timeouts as well. So we cannot assume that a 408 means the server is reached public boolean isServerReached() { 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 78e1c41115f..e0c36156a46 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 @@ -51,6 +51,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; +import java.time.Instant; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -1336,6 +1337,42 @@ void normalizeAcceptErrorStatuses(Integer status) { }); } + @Issue("https://github.com/openrewrite/rewrite/issues/8682") + @Test + void throttledEndpointIsNotAskedAgainUntilItsCooldownElapses() { + var ctx = MavenExecutionContextView.view(this.ctx); + List skipped = new ArrayList<>(); + ctx.setResolutionListener(new ResolutionEventListener() { + @Override + public void repositoryAccessFailedPreviously(String uri) { + skipped.add(uri); + } + }); + var downloader = new MavenPomDownloader(ctx); + mockServer(429, mockRepo -> { + var repositories = List.of(MavenRepository.builder() + .id("id") + .uri("https://%s:%d/maven/".formatted(mockRepo.getHostName(), mockRepo.getPort())) + .knownToExist(true) + .build()); + + assertThatThrownBy(() -> downloader.downloadMetadata(new GroupArtifact("org.example", "first"), null, repositories)) + .isInstanceOf(MavenDownloadingException.class); + // The maven-metadata.xml request only: a host that just answered 429 is not asked for a directory listing too + assertThat(mockRepo.getRequestCount()).isEqualTo(1); + + assertThatThrownBy(() -> downloader.downloadMetadata(new GroupArtifact("org.example", "second"), null, repositories)) + .isInstanceOf(MavenDownloadingException.class); + assertThat(mockRepo.getRequestCount()).isEqualTo(1); + assertThat(skipped).containsExactly(repositories.get(0).getUri()); + + ctx.getThrottledEndpoints().replaceAll((endpoint, until) -> Instant.EPOCH); + assertThatThrownBy(() -> downloader.downloadMetadata(new GroupArtifact("org.example", "second"), null, repositories)) + .isInstanceOf(MavenDownloadingException.class); + assertThat(mockRepo.getRequestCount()).isEqualTo(2); + }); + } + @Test void invalidArtifact() { var downloader = new MavenPomDownloader(emptyMap(), ctx); From 733db86e861c9ff79a91136ca6d298011a3ea698 Mon Sep 17 00:00:00 2001 From: Jonathan Schneider Date: Thu, 27 Aug 2026 13:13:21 -0400 Subject: [PATCH 2/3] Maven resolution: deduplicate repositories by URI, not by id distinctNormalizedRepositories deduped on the post-mirror id, so two entries with different ids and the same URL (generated settings, a POM re-declaring a configured repository) cost a request each per lookup. Key on the URI that would be asked instead, with the trailing slash trimmed and the host compared case-insensitively; the first occurrence wins so order and credentials are preserved. The "central" id check stays on id, since that is Maven's override rule for the implicit Central. Part 2 of #8682. --- .../maven/internal/MavenPomDownloader.java | 22 +++++++++++++++++-- .../internal/MavenPomDownloaderTest.java | 15 +++++++++++++ 2 files changed, 35 insertions(+), 2 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 712af35993b..b0fdc7f00d6 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 @@ -910,7 +910,7 @@ Iterable distinctNormalizedRepositories( // Return lazy iterable return () -> new Iterator() { private final Iterator repoIterator = repositoriesById.values().iterator(); - private final Map<@Nullable String, MavenRepository> seen = new LinkedHashMap<>(); + private final Set seen = new HashSet<>(); private @Nullable MavenRepository next; private @Nullable MavenRepository findNext() { @@ -920,7 +920,7 @@ Iterable distinctNormalizedRepositories( if (normalized != null && (acceptsVersion == null || repositoryAcceptsVersion(normalized, acceptsVersion, containingPom)) && - seen.put(normalized.getId(), normalized) == null) { + seen.add(uriKey(normalized))) { if (throttled(normalized)) { ctx.getResolutionListener().repositoryAccessFailedPreviously(normalized.getUri()); continue; @@ -1294,6 +1294,24 @@ private static boolean hasCredentials(MavenRepository repository) { return host == null ? null : host + ':' + uri.getPort(); } + /** + * Repositories are deduplicated by the URI that would be asked rather than by id: the id is Maven's + * override key, but two declarations of one URL under different ids would cost a request each. + */ + private static String uriKey(MavenRepository repository) { + String uri = repository.getUri(); + if (uri.endsWith("/")) { + uri = uri.substring(0, uri.length() - 1); + } + // Scheme and host are case-insensitive; the path is not + int authorityStart = uri.indexOf("://"); + if (authorityStart < 0) { + return uri; + } + int pathStart = uri.indexOf('/', authorityStart + 3); + return pathStart < 0 ? uri.toLowerCase() : uri.substring(0, pathStart).toLowerCase() + uri.substring(pathStart); + } + private MavenRepository applyMirrors(MavenRepository repository) { return MavenRepositoryMirror.apply(mirrors, repository); } 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 e0c36156a46..9093bdd21b4 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 @@ -153,6 +153,21 @@ void repositoryOrder() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/8682") + @Test + void repositoriesDeclaredUnderDifferentIdsForTheSameUriAreAskedOnce() { + var ctx = MavenExecutionContextView.view(new InMemoryExecutionContext()); + var repositories = List.of( + MavenRepository.builder().id("first").uri("https://repo.example.com/maven").knownToExist(true).build(), + MavenRepository.builder().id("second").uri("https://REPO.example.com/maven/").knownToExist(true).build(), + MavenRepository.builder().id("third").uri("https://repo.example.com/other/").knownToExist(true).build() + ); + + assertThat(new MavenPomDownloader(ctx).distinctNormalizedRepositories(repositories, null, null)) + .extracting(MavenRepository::getId) + .containsExactly("first", "third"); + } + @Nested class WithNativeHttpURLConnectionAndTLS { private final ExecutionContext ctx = HttpSenderExecutionContextView.view(new InMemoryExecutionContext()) From aa0e69d4f550809ef516f4524cf59cb3a9197332 Mon Sep 17 00:00:00 2001 From: Jonathan Schneider Date: Thu, 27 Aug 2026 13:15:34 -0400 Subject: [PATCH 3/3] Maven resolution: a mirror without a policy of its own keeps the mirrored repository's MavenRepositoryMirror.apply set the mirrored repository's releases and snapshots to "true" unless the mirror explicitly said "false". Maven's DefaultMirrorSelector copies the mirrored repository's policies onto the mirror instead, so mirroring Central (releases-only) through a settings should not make it a snapshot-accepting repository that joins every -SNAPSHOT lookup. When several repositories collapse onto one mirror, Maven unions their policies (DefaultRemoteRepositoryManager.mergeMirrors). The lazy repository iterator already yields the first occurrence that accepts the version being looked up, which is the same thing for a single lookup, but downloadMetadata was filtering by version only after deduplication and so would have dropped the snapshot-accepting mirrored repository in favour of the releases-only one. It now passes the version through to distinctNormalizedRepositories like download does. Part 3 of #8682. --- .../maven/internal/MavenPomDownloader.java | 5 +- .../maven/tree/MavenRepositoryMirror.java | 6 ++- .../internal/MavenPomDownloaderTest.java | 52 +++++++++++++++++++ .../maven/tree/MavenRepositoryMirrorTest.java | 22 ++++++++ 4 files changed, 79 insertions(+), 6 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 b0fdc7f00d6..eb3a4b2bf07 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 @@ -288,14 +288,11 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv Timer.Builder timer = Timer.builder("rewrite.maven.download").tag("type", "metadata"); MavenMetadata mavenMetadata = null; - Iterable normalizedRepos = distinctNormalizedRepositories(repositories, containingPom, null); + Iterable normalizedRepos = distinctNormalizedRepositories(repositories, containingPom, gav.getVersion()); Map repositoryResponses = new LinkedHashMap<>(); List attemptedUris = new ArrayList<>(); for (MavenRepository repo : normalizedRepos) { ctx.getResolutionListener().repository(repo, containingPom); - if (gav.getVersion() != null && !repositoryAcceptsVersion(repo, gav.getVersion(), containingPom)) { - continue; - } attemptedUris.add(repo.getUri()); Optional result = mavenCache.getMavenMetadata(URI.create(repo.getUri()), gav); if (result == null) { diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenRepositoryMirror.java b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenRepositoryMirror.java index e10eed2190d..b6d18b135b7 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenRepositoryMirror.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/tree/MavenRepositoryMirror.java @@ -127,8 +127,10 @@ public MavenRepository apply(MavenRepository repo) { if (matches(repo)) { return repo.withUri(url) .withId(id) - .withReleases(!Boolean.FALSE.equals(releases) ? "true" : "false") - .withSnapshots(!Boolean.FALSE.equals(snapshots) ? "true" : "false") + // As in Maven, a mirror has no policy of its own: the mirrored repository keeps its own + // unless the mirror explicitly overrides it + .withReleases(releases == null ? repo.getReleases() : releases.toString()) + .withSnapshots(snapshots == null ? repo.getSnapshots() : snapshots.toString()) // Since the URL has likely changed we cannot assume that the new repository is known to exist .withKnownToExist(false) .withTimeout(repo.getTimeout()); 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 9093bdd21b4..4f5c96edeb0 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 @@ -1388,6 +1388,58 @@ public void repositoryAccessFailedPreviously(String uri) { }); } + @Issue("https://github.com/openrewrite/rewrite/issues/8682") + @Test + void mirrorKeepsThePolicyOfEachRepositoryItMirrors() throws Exception { + var ctx = MavenExecutionContextView.view(this.ctx); + try (MockWebServer mirror = getMockServer()) { + List metadataRequests = synchronizedList(new ArrayList<>()); + mirror.setDispatcher(new Dispatcher() { + @Override + public MockResponse dispatch(RecordedRequest recordedRequest) { + if (recordedRequest.getPath() != null && recordedRequest.getPath().endsWith("maven-metadata.xml")) { + metadataRequests.add(recordedRequest.getPath()); + return new MockResponse().setResponseCode(200).setBody( + //language=xml + """ + + org.example + lib + + + 1.0-SNAPSHOT + + + + """); + } + return new MockResponse().setResponseCode(200).setBody(""); + } + }); + mirror.start(); + ctx.setMirrors(List.of(new MavenRepositoryMirror("mirror", + "https://%s:%d/maven/".formatted(mirror.getHostName(), mirror.getPort()), "*", null, null, null))); + var downloader = new MavenPomDownloader(ctx); + var gav = new GroupArtifactVersion("org.example", "lib", "1.0-SNAPSHOT"); + + // Central does not serve snapshots, and mirroring it does not change that + assertThatThrownBy(() -> downloader.downloadMetadata(gav, null, List.of(MAVEN_CENTRAL))) + .isInstanceOf(MavenDownloadingException.class); + assertThat(metadataRequests).isEmpty(); + + // A mirrored repository that does accept snapshots brings the mirror into the lookup + var snapshots = MavenRepository.builder() + .id("snapshots") + .uri("https://snapshots.example.com/maven") + .releases(false) + .snapshots(true) + .build(); + MavenMetadata metadata = downloader.downloadMetadata(gav, null, List.of(MAVEN_CENTRAL, snapshots)); + assertThat(metadata.getVersioning().getVersions()).containsExactly("1.0-SNAPSHOT"); + assertThat(metadataRequests).hasSize(1); + } + } + @Test void invalidArtifact() { var downloader = new MavenPomDownloader(emptyMap(), ctx); diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/MavenRepositoryMirrorTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/MavenRepositoryMirrorTest.java index 74966c78237..99877ed3edf 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/tree/MavenRepositoryMirrorTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/tree/MavenRepositoryMirrorTest.java @@ -16,6 +16,7 @@ package org.openrewrite.maven.tree; import org.junit.jupiter.api.Test; +import org.openrewrite.Issue; import java.util.List; @@ -78,6 +79,27 @@ void excludeFromWildcard() { assertThat(oneMirrored).extracting(MavenRepository::getUri).isEqualTo("https://mirror"); } + @Issue("https://github.com/openrewrite/rewrite/issues/8682") + @Test + void mirrorWithoutPolicyKeepsTheMirroredRepositoryPolicy() { + MavenRepositoryMirror mirror = new MavenRepositoryMirror("mirror", "https://mirror", "*", null, null, null); + + MavenRepository mirrored = mirror.apply(MavenRepository.MAVEN_CENTRAL); + + assertThat(mirrored.getReleases()).isEqualTo("true"); + assertThat(mirrored.getSnapshots()).isEqualTo("false"); + } + + @Test + void mirrorPolicyOverridesTheMirroredRepositoryPolicy() { + MavenRepositoryMirror mirror = new MavenRepositoryMirror("mirror", "https://mirror", "*", false, true, null); + + MavenRepository mirrored = mirror.apply(MavenRepository.MAVEN_CENTRAL); + + assertThat(mirrored.getReleases()).isEqualTo("false"); + assertThat(mirrored.getSnapshots()).isEqualTo("true"); + } + @Test void localM2RepositoryIsNeverMirrored() { MavenRepositoryMirror mirror = new MavenRepositoryMirror("mirror", "https://mirror", "*", true, true, null);