Allow recursive profile group references

Update the original fix for issue #24327 so that recursive elements are tolerated rather than fail. See gh-24327
spring-projects · Dec 9, 2020 · bef5fe2 · bef5fe2
1 parent a2a7bd5
commit bef5fe2
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 41 deletions.
diff --git a/...t-project/spring-boot/src/main/java/org/springframework/boot/context/config/Profiles.java b/...t-project/spring-boot/src/main/java/org/springframework/boot/context/config/Profiles.java
@@ -27,15 +27,13 @@
 import java.util.List;
 import java.util.Set;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 
 import org.springframework.boot.context.properties.bind.Bindable;
 import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.env.AbstractEnvironment;
 import org.springframework.core.env.Environment;
 import org.springframework.core.style.ToStringCreator;
-import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
@@ -113,22 +111,16 @@ private boolean hasExplicit(Supplier<String[]> supplier, String propertyValue, S
 	}
 
 	private List<String> expandProfiles(List<String> profiles) {
-		if (CollectionUtils.isEmpty(profiles)) {
-			return Collections.emptyList();
-		}
-		Deque<String> stack = new ArrayDeque<>(profiles);
-		Set<String> expanded = new LinkedHashSet<>();
+		Deque<String> stack = new ArrayDeque<>();
+		asReversedList(profiles).forEach(stack::push);
+		Set<String> expandedProfiles = new LinkedHashSet<>();
 		while (!stack.isEmpty()) {
 			String current = stack.pop();
-			expanded.add(current);
-			List<String> group = asReversedList(this.groups.get(current));
-			Set<String> conflicts = getProfileConflicts(group, expanded, stack);
-			Assert.state(conflicts.isEmpty(),
-					() -> String.format("Profiles could not be resolved. Remove %s from group: '%s'",
-							getProfilesDescription(conflicts), current));
-			group.forEach(stack::push);
+			if (expandedProfiles.add(current)) {
+				asReversedList(this.groups.get(current)).forEach(stack::push);
+			}
 		}
-		return asUniqueItemList(StringUtils.toStringArray(expanded));
+		return asUniqueItemList(StringUtils.toStringArray(expandedProfiles));
 	}
 
 	private List<String> asReversedList(List<String> list) {
@@ -140,21 +132,6 @@ private List<String> asReversedList(List<String> list) {
 		return reversed;
 	}
 
-	private Set<String> getProfileConflicts(List<String> group, Set<String> expanded, Deque<String> stack) {
-		if (group.isEmpty()) {
-			return Collections.emptySet();
-		}
-		return group.stream().filter((profile) -> expanded.contains(profile) || stack.contains(profile))
-				.collect(Collectors.toSet());
-	}
-
-	private String getProfilesDescription(Set<String> conflicts) {
-		if (conflicts.size() == 1) {
-			return "profile '" + conflicts.iterator().next() + "'";
-		}
-		return "profiles " + conflicts.stream().map((profile) -> "'" + profile + "'").collect(Collectors.joining(","));
-	}
-
 	private List<String> asUniqueItemList(String[] array) {
 		return asUniqueItemList(array, null);
 	}

diff --git a/...ject/spring-boot/src/test/java/org/springframework/boot/context/config/ProfilesTests.java b/...ject/spring-boot/src/test/java/org/springframework/boot/context/config/ProfilesTests.java
@@ -27,7 +27,6 @@
 import org.springframework.mock.env.MockEnvironment;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 
 /**
  * Tests for {@link Profiles}.
@@ -366,29 +365,29 @@ void simpleRecursiveReferenceInProfileGroupThrowsException() {
 		environment.setProperty("spring.profiles.active", "a,b,c");
 		environment.setProperty("spring.profiles.group.a", "a,e,f");
 		Binder binder = Binder.get(environment);
-		assertThatIllegalStateException().isThrownBy(() -> new Profiles(environment, binder, null))
-				.withMessageContaining("Profiles could not be resolved. Remove profile 'a' from group: 'a'");
+		Profiles profiles = new Profiles(environment, binder, null);
+		assertThat(profiles.getAccepted()).containsExactly("a", "e", "f", "b", "c");
 	}
 
 	@Test
-	void multipleRecursiveReferenceInProfileGroupThrowsException() {
+	void multipleRecursiveReferenceInProfileGroupIgnoresDuplicates() {
 		MockEnvironment environment = new MockEnvironment();
 		environment.setProperty("spring.profiles.active", "a,b,c");
 		environment.setProperty("spring.profiles.group.a", "a,b,f");
 		Binder binder = Binder.get(environment);
-		assertThatIllegalStateException().isThrownBy(() -> new Profiles(environment, binder, null))
-				.withMessageContaining("Profiles could not be resolved. Remove profiles 'a','b' from group: 'a'");
+		Profiles profiles = new Profiles(environment, binder, null);
+		assertThat(profiles.getAccepted()).containsExactly("a", "b", "f", "c");
 	}
 
 	@Test
-	void complexRecursiveReferenceInProfileGroupThrowsException() {
+	void complexRecursiveReferenceInProfileGroupIgnoresDuplicates() {
 		MockEnvironment environment = new MockEnvironment();
 		environment.setProperty("spring.profiles.active", "a,b,c");
-		environment.setProperty("spring.profiles.group.a", "e,f");
-		environment.setProperty("spring.profiles.group.e", "a,x,y");
+		environment.setProperty("spring.profiles.group.a", "e,f,g");
+		environment.setProperty("spring.profiles.group.e", "a,x,y,g");
 		Binder binder = Binder.get(environment);
-		assertThatIllegalStateException().isThrownBy(() -> new Profiles(environment, binder, null))
-				.withMessageContaining("Profiles could not be resolved. Remove profile 'a' from group: 'e'");
+		Profiles profiles = new Profiles(environment, binder, null);
+		assertThat(profiles.getAccepted()).containsExactly("a", "e", "x", "y", "g", "f", "b", "c");
 	}
 
 }