Merge pull request #24327 from dreis2211

* pr/24327:
  Polish 'Fail on recursive references in profile groups'
  Fail on recursive references in profile groups

Closes gh-24327

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/Profiles.java

@@ -27,6 +27,7 @@ import java.util.LinkedHashSet;
 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;
@@ -34,6 +35,7 @@ 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;
@@ -111,24 +113,46 @@ public class Profiles implements Iterable<String> {
 	}
 
 	private List<String> expandProfiles(List<String> profiles) {
-		Deque<String> stack = new ArrayDeque<>();
+		if (CollectionUtils.isEmpty(profiles)) {
-		asReversedList(profiles).forEach(stack::push);
+			return Collections.emptyList();
-		Set<String> expandedProfiles = new LinkedHashSet<>();
+		}
+		Deque<String> stack = new ArrayDeque<>(profiles);
+		Set<String> expanded = new LinkedHashSet<>();
 		while (!stack.isEmpty()) {
 			String current = stack.pop();
-			expandedProfiles.add(current);
+			expanded.add(current);
-			asReversedList(this.groups.get(current)).forEach(stack::push);
+			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);
 		}
-		return asUniqueItemList(StringUtils.toStringArray(expandedProfiles));
+		return asUniqueItemList(StringUtils.toStringArray(expanded));
 	}
 
 	private List<String> asReversedList(List<String> list) {
-		if (list == null || list.isEmpty()) {
+		if (CollectionUtils.isEmpty(list)) {
 			return Collections.emptyList();
 		}
 		List<String> reversed = new ArrayList<>(list);
 		Collections.reverse(reversed);
-		return Collections.unmodifiableList(reversed);
+		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) {

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ProfilesTests.java

@@ -27,6 +27,7 @@ import org.springframework.core.env.Environment;
 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}.
@@ -359,4 +360,35 @@ class ProfilesTests {
 		assertThat(profiles.isAccepted("x")).isTrue();
 	}
 
+	@Test
+	void simpleRecursiveReferenceInProfileGroupThrowsException() {
+		MockEnvironment environment = new MockEnvironment();
+		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'");
+	}
+
+	@Test
+	void multipleRecursiveReferenceInProfileGroupThrowsException() {
+		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'");
+	}
+
+	@Test
+	void complexRecursiveReferenceInProfileGroupThrowsException() {
+		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");
+		Binder binder = Binder.get(environment);
+		assertThatIllegalStateException().isThrownBy(() -> new Profiles(environment, binder, null))
+				.withMessageContaining("Profiles could not be resolved. Remove profile 'a' from group: 'e'");
+	}
+
 }