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..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 @@ -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()) { @@ -258,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) { @@ -363,9 +390,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; } @@ -879,7 +907,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() { @@ -889,7 +917,11 @@ 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; + } return normalized; } } @@ -1259,6 +1291,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); } @@ -1301,6 +1351,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/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 78e1c41115f..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 @@ -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; @@ -152,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()) @@ -1336,6 +1352,94 @@ 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); + }); + } + + @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);