Skip to content

Commit

Permalink
Fix #3908 - Maven repositories in settings.xml override repositories …
Browse files Browse the repository at this point in the history 
…of the same id in pom.xml, including the default maven central repository
  • Loading branch information
@sambsnyd
sambsnyd committed Jan 13, 2024
1 parent b5e239c commit 9cf0f03
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 46 deletions.
17 changes: 15 additions & 2 deletions rewrite-maven/src/main/java/org/openrewrite/maven/MavenExecutionContextView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.time.Duration;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Collections.emptyList;
import static org.openrewrite.maven.tree.MavenRepository.MAVEN_LOCAL_DEFAULT;
Expand Down Expand Up @@ -195,11 +196,12 @@ public MavenExecutionContextView setMavenSettings(@Nullable MavenSettings settin
}

putMessage(MAVEN_SETTINGS, settings);
setActiveProfiles(Arrays.asList(activeProfiles));
List<String> effectiveActiveProfiles = mapActiveProfiles(settings, activeProfiles);
setActiveProfiles(effectiveActiveProfiles);
setCredentials(mapCredentials(settings));
setMirrors(mapMirrors(settings));
setLocalRepository(settings.getMavenLocal());
setRepositories(mapRepositories(settings, Arrays.asList(activeProfiles)));
setRepositories(mapRepositories(settings, effectiveActiveProfiles));

return this;
}
Expand All @@ -209,6 +211,17 @@ public MavenSettings getSettings() {
return getMessage(MAVEN_SETTINGS, null);
}

private static List<String> mapActiveProfiles(MavenSettings settings, String... activeProfiles) {
if(settings.getActiveProfiles() == null) {
return Arrays.asList(activeProfiles);
}
return Stream.concat(
settings.getActiveProfiles().getActiveProfiles().stream(),
Arrays.stream(activeProfiles))
.distinct()
.collect(Collectors.toList());
}

private static List<MavenRepositoryCredentials> mapCredentials(MavenSettings settings) {
if (settings.getServers() != null) {
return settings.getServers().getServers().stream()
Expand Down
8 changes: 5 additions & 3 deletions rewrite-maven/src/main/java/org/openrewrite/maven/MavenSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,22 @@ public MavenSettings merge(@Nullable MavenSettings installSettings) {
}

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

if (profiles != null) {
for (Profile profile : profiles.getProfiles()) {
if (profile.isActive(activeProfiles) || (this.activeProfiles != null &&
profile.isActive(this.activeProfiles.getActiveProfiles()))) {
if (profile.repositories != null) {
activeRepositories.addAll(profile.repositories.getRepositories());
for (RawRepositories.Repository repository : profile.repositories.getRepositories()) {
activeRepositories.put(repository.getId(), repository);
}
}
}
}
}

return activeRepositories;
return new ArrayList<>(activeRepositories.values());
}

public MavenRepository getMavenLocal() {
Expand Down
21 changes: 11 additions & 10 deletions rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,35 +639,36 @@ private String datedSnapshotVersion(GroupArtifactVersion gav, @Nullable Resolved
return gav.getVersion();
}

private Collection<MavenRepository> distinctNormalizedRepositories(List<MavenRepository> repositories,
@Nullable ResolvedPom containingPom,
@Nullable String acceptsVersion) {
Set<MavenRepository> normalizedRepositories = new LinkedHashSet<>();
Collection<MavenRepository> distinctNormalizedRepositories(
List<MavenRepository> repositories,
@Nullable ResolvedPom containingPom,
@Nullable String acceptsVersion) {
LinkedHashMap<String, MavenRepository> normalizedRepositories = new LinkedHashMap<>();
if (addDefaultRepositories) {
normalizedRepositories.add(ctx.getLocalRepository());
normalizedRepositories.put(ctx.getLocalRepository().getId(), ctx.getLocalRepository());
}

for (MavenRepository repo : repositories) {
MavenRepository normalizedRepo = normalizeRepository(repo, containingPom);
if (normalizedRepo != null && (acceptsVersion == null || repositoryAcceptsVersion(normalizedRepo, acceptsVersion, containingPom))) {
normalizedRepositories.add(normalizedRepo);
normalizedRepositories.put(normalizedRepo.getId(), normalizedRepo);
}
}

// repositories from maven settings
for (MavenRepository repo : ctx.getRepositories(mavenSettings, activeProfiles)) {
MavenRepository normalizedRepo = normalizeRepository(repo, containingPom);
if (normalizedRepo != null && (acceptsVersion == null || repositoryAcceptsVersion(normalizedRepo, acceptsVersion, containingPom))) {
normalizedRepositories.add(normalizedRepo);
normalizedRepositories.put(normalizedRepo.getId(), normalizedRepo);
}
}
if (addDefaultRepositories) {
if (addDefaultRepositories && !normalizedRepositories.containsKey(MavenRepository.MAVEN_CENTRAL.getId())) {
MavenRepository normalizedRepo = normalizeRepository(MavenRepository.MAVEN_CENTRAL, containingPom);
if (normalizedRepo != null) {
normalizedRepositories.add(normalizedRepo);
normalizedRepositories.put(normalizedRepo.getId(), normalizedRepo);
}
}
return normalizedRepositories;
return normalizedRepositories.values();
}

@Nullable
Expand Down
24 changes: 6 additions & 18 deletions rewrite-maven/src/main/java/org/openrewrite/maven/tree/ProfileActivation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package org.openrewrite.maven.tree;

import com.fasterxml.jackson.annotation.JsonCreator;
import lombok.AccessLevel;
import lombok.Data;
import lombok.experimental.FieldDefaults;
Expand All @@ -37,19 +36,6 @@ public class ProfileActivation {
@Nullable
Property property;

public ProfileActivation() {
this.activeByDefault = null;
this.jdk = null;
this.property = null;
}

@JsonCreator
public ProfileActivation(@Nullable Boolean activeByDefault, @Nullable String jdk, @Nullable Property property) {
this.activeByDefault = activeByDefault;
this.jdk = jdk;
this.property = property;
}

public static boolean isActive(@Nullable String id, Iterable<String> activeProfiles,
@Nullable ProfileActivation activation) {
if (id != null) {
Expand All @@ -59,13 +45,15 @@ public static boolean isActive(@Nullable String id, Iterable<String> activeProfi
}
}
}
return activation != null && activation.isActive();
return activation != null &&
(activation.isActive() ||
// Active by default is *only* enabled when no other profile is marked active by any other mechanism
// So even this check for any other explicit activation is overly broad
(Boolean.TRUE.equals(activation.getActiveByDefault()) && !activeProfiles.iterator().hasNext()));
}

public boolean isActive() {
return (activeByDefault != null && activeByDefault) ||
isActiveByJdk() ||
isActiveByProperty();
return isActiveByJdk() || isActiveByProperty();
}

private boolean isActiveByJdk() {
Expand Down
47 changes: 46 additions & 1 deletion rewrite-maven/src/test/java/org/openrewrite/maven/MavenSettingsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,48 @@ void defaultActiveWhenNoOthersAreActive() {
assertThat(ctx.getRepositories().stream().map(MavenRepository::getUri)).containsExactly("https://activebydefault.com");
}

@Disabled
@Test
void idCollisionLastRepositoryWins() {
var ctx = MavenExecutionContextView.view(new InMemoryExecutionContext());
ctx.setMavenSettings(MavenSettings.parse(new Parser.Input(Paths.get("settings.xml"), () -> new ByteArrayInputStream(
//language=xml
"""
<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 http://maven.apache.org/xsd/settings-1.0.0.xsd">
<activeProfiles>
<activeProfile>
repo
</activeProfile>
</activeProfiles>
<profiles>
<profile>
<id>repo</id>
<repositories>
<repository>
<id>repo</id>
<url>https://firstloses.com</url>
</repository>
<repository>
<id>repo</id>
<url>https://secondloses.com</url>
</repository>
<repository>
<id>repo</id>
<url>https://lastwins.com</url>
</repository>
</repositories>
</profile>
</profiles>
</settings>
""".getBytes()
)), ctx));

assertThat(ctx.getRepositories())
.as("When multiple repositories have the same id in a maven settings file the last one wins. In a pom.xml an error would be thrown.")
.containsExactly(new MavenRepository("repo", "https://lastwins.com", null, null, null, null));
}

@Issue("https://github.com/openrewrite/rewrite/issues/131")
@Test
void defaultOnlyActiveIfNoOthersAreActive() {
Expand Down Expand Up @@ -157,6 +198,10 @@ void defaultOnlyActiveIfNoOthersAreActive() {
""".getBytes()
)), ctx));


assertThat(ctx.getActiveProfiles())
.containsExactly("repo");

assertThat(ctx.getRepositories().stream().map(MavenRepository::getUri))
.containsExactly("https://activebyactivationlist.com");
}
Expand Down
62 changes: 50 additions & 12 deletions rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,38 @@
*/
package org.openrewrite.maven.internal;

import dev.failsafe.Failsafe;
import dev.failsafe.RetryPolicy;
import okhttp3.mockwebserver.*;
import org.assertj.core.api.Condition;
import org.intellij.lang.annotations.Language;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.openrewrite.ExecutionContext;
import org.openrewrite.HttpSenderExecutionContextView;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.Issue;
import org.openrewrite.*;
import org.openrewrite.ipc.http.HttpSender;
import org.openrewrite.ipc.http.HttpUrlConnectionSender;
import org.openrewrite.maven.MavenDownloadingException;
import org.openrewrite.maven.MavenExecutionContextView;
import org.openrewrite.maven.MavenParser;
import org.openrewrite.maven.MavenSettings;
import org.openrewrite.maven.tree.GroupArtifact;
import org.openrewrite.maven.tree.GroupArtifactVersion;
import org.openrewrite.maven.tree.MavenMetadata;
import org.openrewrite.maven.tree.MavenRepository;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.net.SocketTimeoutException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeoutException;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static org.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
Expand Down Expand Up @@ -82,6 +76,48 @@ public MockResponse dispatch(RecordedRequest recordedRequest) {
}
}

@Issue("https://github.com/openrewrite/rewrite/issues/3908")
@Test
void centralIdOverridesDefaultRepository() {
var ctx = MavenExecutionContextView.view(new InMemoryExecutionContext());
ctx.setMavenSettings(MavenSettings.parse(new Parser.Input(Paths.get("settings.xml"), () -> new ByteArrayInputStream(
//language=xml
"""
<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0
https://maven.apache.org/xsd/settings-1.0.0.xsd">
<profiles>
<profile>
<id>central</id>
<repositories>
<repository>
<id>central</id>
<url>https://internalartifactrepository.yourorg.com</url>
</repository>
</repositories>
</profile>
</profiles>
<activeProfiles>
<activeProfile>central</activeProfile>
</activeProfiles>
</settings>
""".getBytes()
)), ctx));

// Avoid actually trying to reach the made-up https://internalartifactrepository.yourorg.com
for (MavenRepository repository : ctx.getRepositories()) {
repository.setKnownToExist(true);
}

var downloader = new MavenPomDownloader(emptyMap(), ctx);
Collection<MavenRepository> repos = downloader.distinctNormalizedRepositories(emptyList(), null, null);
assertThat(repos).areExactly(1, new Condition<>(repo -> "central".equals(repo.getId()),
"id \"central\""));
assertThat(repos).areExactly(1, new Condition<>(repo -> "https://internalartifactrepository.yourorg.com".equals(repo.getUri()),
"URI https://internalartifactrepository.yourorg.com"));
}

@Disabled("Flaky on CI")
@Test
void normalizeOssSnapshots() {
Expand Down Expand Up @@ -177,6 +213,7 @@ void useSnapshotTimestampVersion() {
mockRepo.setDispatcher(new Dispatcher() {
@Override
public MockResponse dispatch(RecordedRequest recordedRequest) {
assert recordedRequest.getPath() != null;
return !recordedRequest.getPath().endsWith("fred/fred/2020.0.2-SNAPSHOT/fred-2020.0.2-20210127.131051-2.pom") ?
new MockResponse().setResponseCode(404).setBody("") :
new MockResponse().setResponseCode(200).setBody(
Expand Down Expand Up @@ -579,6 +616,7 @@ void doNotRenameRepoForCustomMavenLocal(@TempDir Path tempDir) throws MavenDownl
var downloader = new MavenPomDownloader(emptyMap(), ctx);

var result = downloader.download(gav, null, null, List.of());
assertThat(result.getRepository()).isNotNull();
assertThat(result.getRepository().getUri()).startsWith(tempDir.toUri().toString());
}

Expand Down

0 comments on commit 9cf0f03

Please sign in to comment.