Skip to content

Fix MavenSettings value equality dropping LST-carried mirrors #8189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
pdelagrave wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Fix MavenSettings value equality dropping LST-carried mirrors #8189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3d8511b
Fix `MavenSettings` equals and hashCode comparing no fields
pdelagrave Jul 6, 2026
30b842c
Preserve id-less entries when merging Maven settings
pdelagrave Jul 6, 2026
e515591
Memoize effective Maven settings and mirrors per execution context
pdelagrave Jul 7, 2026
57edecb
Add tests pinning that a mirror of Maven Central redirects all resolu…
pdelagrave Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 34 additions & 9 deletions rewrite-maven/src/main/java/org/openrewrite/maven/MavenExecutionContextView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.openrewrite.maven;

import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import org.jspecify.annotations.Nullable;
import org.openrewrite.DelegatingExecutionContext;
import org.openrewrite.ExecutionContext;
Expand Down Expand Up @@ -52,6 +54,8 @@ public class MavenExecutionContextView extends DelegatingExecutionContext {
private static final String MAVEN_REPOSITORIES = "org.openrewrite.maven.repos";
private static final String MAVEN_PINNED_SNAPSHOT_VERSIONS = "org.openrewrite.maven.pinnedSnapshotVersions";
private static final String MAVEN_POM_CACHE = "org.openrewrite.maven.pomCache";
private static final String MAVEN_EFFECTIVE_SETTINGS_CACHE = "org.openrewrite.maven.effectiveSettingsCache";
private static final String MAVEN_MIRRORS_CACHE = "org.openrewrite.maven.mirrorsCache";
private static final String MAVEN_ARTIFACT_CACHE = "org.openrewrite.maven.artifactCache";
private static final String MAVEN_RESOLUTION_LISTENER = "org.openrewrite.maven.resolutionListener";
private static final String MAVEN_RESOLUTION_TIME = "org.openrewrite.maven.resolutionTime";
Expand Down Expand Up @@ -103,6 +107,7 @@ public ResolutionEventListener getResolutionListener() {

public MavenExecutionContextView setMirrors(@Nullable Collection<MavenRepositoryMirror> mirrors) {
putMessage(MAVEN_MIRRORS, mirrors);
putMessage(MAVEN_MIRRORS_CACHE, null);
return this;
}

Expand All @@ -112,15 +117,20 @@ public Collection<MavenRepositoryMirror> getMirrors() {

/**
* Get mirrors set on this execution context, unless overridden by a supplied maven settings file.
* Supplied settings take precedence: when they differ in value from the settings on this context,
* their mirrors replace (not merge with) the context's mirror list, including a list installed
* explicitly via {@link #setMirrors(Collection)}.
*
* @param mavenSettings The maven settings defining mirrors to use, if any.
* @return The mirrors to use for dependency resolution.
*/
public Collection<MavenRepositoryMirror> getMirrors(@Nullable MavenSettings mavenSettings) {
if (mavenSettings != null && !Objects.equals(mavenSettings, getSettings())) {
return mapMirrors(mavenSettings);
if (mavenSettings == null) {
return getMirrors();
}
return getMirrors();
return this.<Collection<MavenRepositoryMirror>>getIdentityCache(MAVEN_MIRRORS_CACHE)
.get(mavenSettings, settings ->
!Objects.equals(settings, getSettings()) ? mapMirrors(settings) : getMirrors());
}

public MavenExecutionContextView setCredentials(Collection<MavenRepositoryCredentials> credentials) {
Expand Down Expand Up @@ -312,6 +322,8 @@ public MavenExecutionContextView setMavenSettings(@Nullable MavenSettings settin
}

putMessage(MAVEN_SETTINGS, settings);
putMessage(MAVEN_EFFECTIVE_SETTINGS_CACHE, null);
putMessage(MAVEN_MIRRORS_CACHE, null);
List<String> effectiveActiveProfiles = mapActiveProfiles(settings, activeProfiles);
setActiveProfiles(effectiveActiveProfiles);
setCredentials(mapCredentials(settings));
Expand All @@ -332,13 +344,26 @@ public MavenExecutionContextView setMavenSettings(@Nullable MavenSettings settin
*
*/
public @Nullable MavenSettings effectiveSettings(MavenResolutionResult mrr) {
MavenSettings effectiveSettings = getMessage(MAVEN_SETTINGS);
if (effectiveSettings == null) {
effectiveSettings = mrr.getMavenSettings();
} else {
effectiveSettings = effectiveSettings.merge(mrr.getMavenSettings());
MavenSettings contextSettings = getMessage(MAVEN_SETTINGS);
MavenSettings parsedSettings = mrr.getMavenSettings();
if (contextSettings == null) {
return parsedSettings;
}
return effectiveSettings;
if (parsedSettings == null) {
return contextSettings;
}
// Recipes construct a downloader per visited tag, so memoize the merge per parsed-settings
// instance; returning a stable instance also lets getMirrors(MavenSettings) memoize.
return this.<MavenSettings>getIdentityCache(MAVEN_EFFECTIVE_SETTINGS_CACHE)
.get(parsedSettings, contextSettings::merge);
}

private <T> Cache<MavenSettings, T> getIdentityCache(String key) {
// Weak keys give identity comparison and let entries die with the settings instances they
// memoize for, so a long run over many repositories does not pin every parsed settings graph.
//noinspection unchecked
return (Cache<MavenSettings, T>) getMessages().computeIfAbsent(key,
k -> Caffeine.newBuilder().weakKeys().build());
}

private static List<String> mapActiveProfiles(MavenSettings settings, String... activeProfiles) {
Expand Down
97 changes: 55 additions & 42 deletions rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,33 +40,38 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.function.Function;
import java.util.function.UnaryOperator;

import static java.util.Collections.emptyList;
import static org.openrewrite.maven.tree.MavenRepository.MAVEN_LOCAL_DEFAULT;

@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE)
@ToString(onlyExplicitlyIncluded = true)
@EqualsAndHashCode(onlyExplicitlyIncluded = true)
@Data
@AllArgsConstructor
@JacksonXmlRootElement(localName = "settings")
public class MavenSettings {
@Nullable
@ToString.Include
String localRepository;

@Nullable
@NonFinal
@JsonIgnore
@EqualsAndHashCode.Exclude
MavenRepository mavenLocal;

@Nullable
@ToString.Include
Profiles profiles;

@Nullable
@ToString.Include
ActiveProfiles activeProfiles;

@Nullable
@ToString.Include
Mirrors mirrors;

@Nullable
Expand Down Expand Up @@ -203,7 +208,7 @@ private static boolean exists(Path path) {
}

public MavenSettings merge(@Nullable MavenSettings installSettings) {
return installSettings == null ? this : new MavenSettings(
return installSettings == null || equals(installSettings) ? this : new MavenSettings(
localRepository == null ? installSettings.localRepository : localRepository,
profiles == null ? installSettings.profiles : profiles.merge(installSettings.profiles),
activeProfiles == null ? installSettings.activeProfiles : activeProfiles.merge(installSettings.activeProfiles),
Expand All @@ -213,6 +218,41 @@ public MavenSettings merge(@Nullable MavenSettings installSettings) {
);
}

/**
* Entries from {@code preferred} win over same-id entries from {@code other}, and duplicate ids
* within one side collapse to the last value at the first position, as Maven does. Entries without
* an id cannot be correlated, so both sides' are kept; merging two equal settings stays idempotent
* only through the equality short-circuit in {@link #merge(MavenSettings)}.
*/
private static <T> List<T> mergeById(List<T> preferred, @Nullable List<T> other, Function<T, @Nullable String> id) {
List<T> merged = new ArrayList<>(preferred.size());
Map<String, Integer> indexOfId = new HashMap<>();
for (T t : preferred) {
String tId = id.apply(t);
Integer existing = tId == null ? null : indexOfId.get(tId);
if (existing != null) {
merged.set(existing, t);
} else {
if (tId != null) {
indexOfId.put(tId, merged.size());
}
merged.add(t);
}
}
if (other != null) {
for (T t : other) {
String tId = id.apply(t);
if (tId == null || !indexOfId.containsKey(tId)) {
if (tId != null) {
indexOfId.put(tId, merged.size());
}
merged.add(t);
}
}
}
return merged;
}

public List<RawRepositories.Repository> getActiveRepositories(Iterable<String> activeProfiles) {
LinkedHashMap<String, RawRepositories.Repository> activeRepositories = new LinkedHashMap<>();

Expand Down Expand Up @@ -332,6 +372,8 @@ private Proxy interpolate(Proxy proxy) {
@FieldDefaults(level = AccessLevel.PRIVATE)
@Getter
@Setter
@EqualsAndHashCode
@ToString
@AllArgsConstructor
@NoArgsConstructor
public static class Profiles {
Expand All @@ -340,20 +382,16 @@ public static class Profiles {
List<Profile> profiles = emptyList();

public Profiles merge(@Nullable Profiles profiles) {
final Map<String, Profile> merged = new LinkedHashMap<>();
for (Profile profile : this.profiles) {
merged.put(profile.id, profile);
}
if (profiles != null) {
profiles.getProfiles().forEach(profile -> merged.putIfAbsent(profile.getId(), profile));
}
return new Profiles(new ArrayList<>(merged.values()));
return new Profiles(
mergeById(this.profiles, profiles == null ? null : profiles.getProfiles(), Profile::getId));
}
}

@FieldDefaults(level = AccessLevel.PRIVATE)
@Getter
@Setter
@EqualsAndHashCode
@ToString
@AllArgsConstructor
@NoArgsConstructor
public static class ActiveProfiles {
Expand Down Expand Up @@ -401,6 +439,8 @@ public boolean isActive(String... activeProfiles) {
@FieldDefaults(level = AccessLevel.PRIVATE)
@Getter
@Setter
@EqualsAndHashCode
@ToString
@AllArgsConstructor
@NoArgsConstructor
public static class Mirrors {
Expand All @@ -409,14 +449,7 @@ public static class Mirrors {
List<Mirror> mirrors = emptyList();

public Mirrors merge(@Nullable Mirrors mirrors) {
final Map<String, Mirror> merged = new LinkedHashMap<>();
for (Mirror mirror : this.mirrors) {
merged.put(mirror.id, mirror);
}
if (mirrors != null) {
mirrors.getMirrors().forEach(mirror -> merged.putIfAbsent(mirror.getId(), mirror));
}
return new Mirrors(new ArrayList<>(merged.values()));
return new Mirrors(mergeById(this.mirrors, mirrors == null ? null : mirrors.getMirrors(), Mirror::getId));
}
}

Expand All @@ -442,6 +475,7 @@ public static class Mirror {
@FieldDefaults(level = AccessLevel.PRIVATE)
@Getter
@Setter
@EqualsAndHashCode
@AllArgsConstructor
@NoArgsConstructor
public static class Servers {
Expand All @@ -451,14 +485,7 @@ public static class Servers {
List<Server> servers = emptyList();

public Servers merge(@Nullable Servers servers) {
final Map<String, Server> merged = new LinkedHashMap<>();
for (Server server : this.servers) {
merged.put(server.id, server);
}
if (servers != null) {
servers.getServers().forEach(server -> merged.putIfAbsent(server.getId(), server));
}
return new Servers(new ArrayList<>(merged.values()));
return new Servers(mergeById(this.servers, servers == null ? null : servers.getServers(), Server::getId));
}
}

Expand All @@ -478,6 +505,7 @@ public static class Server {
@FieldDefaults(level = AccessLevel.PRIVATE)
@Getter
@Setter
@EqualsAndHashCode
@AllArgsConstructor
@NoArgsConstructor
public static class Proxies {
Expand All @@ -486,22 +514,7 @@ public static class Proxies {
List<Proxy> proxies = emptyList();

public Proxies merge(@Nullable Proxies proxies) {
final Map<String, Proxy> merged = new LinkedHashMap<>();
int nullIndex = 0;
for (Proxy proxy : this.proxies) {
String key = proxy.id != null ? proxy.id : "__null_" + nullIndex++;
merged.put(key, proxy);
}
if (proxies != null) {
for (Proxy proxy : proxies.getProxies()) {
if (proxy.getId() != null) {
merged.putIfAbsent(proxy.getId(), proxy);
} else {
merged.put("__null_" + nullIndex++, proxy);
}
}
}
return new Proxies(new ArrayList<>(merged.values()));
return new Proxies(mergeById(this.proxies, proxies == null ? null : proxies.getProxies(), Proxy::getId));
}
}

Expand Down
3 changes: 3 additions & 0 deletions rewrite-maven/src/main/java/org/openrewrite/maven/internal/RawRepositories.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
@FieldDefaults(level = AccessLevel.PRIVATE)
@Getter
@Setter
@EqualsAndHashCode
@ToString
public class RawRepositories {
@JacksonXmlProperty(localName = "repository")
@JacksonXmlElementWrapper(useWrapping = false)
Expand All @@ -51,6 +53,7 @@ public static class Repository {

@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE)
@EqualsAndHashCode
@ToString
@Getter
public static class ArtifactPolicy {

Expand Down
Loading
Loading