Polish 'Fail on recursive references in profile groups'

See gh-24327

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

@@ -35,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;
@@ -112,31 +113,26 @@ public class Profiles implements Iterable<String> {
 	}
 
 	private List<String> expandProfiles(List<String> profiles) {
-		Deque<String> stack = new ArrayDeque<>();
-		asReversedList(profiles).forEach(stack::push);
-		Set<String> expandedProfiles = new LinkedHashSet<>();
+		if (CollectionUtils.isEmpty(profiles)) {
+			return Collections.emptyList();
+		}
+		Deque<String> stack = new ArrayDeque<>(profiles);
+		Set<String> expanded = new LinkedHashSet<>();
 		while (!stack.isEmpty()) {
 			String current = stack.pop();
-			expandedProfiles.add(current);
-			List<String> groupProfiles = asReversedList(this.groups.get(current));
-			Set<String> profileConflicts = getProfileConflicts(groupProfiles, expandedProfiles);
-			if (!profileConflicts.isEmpty()) {
-				String message = String.format("Profiles could not be resolved. Remove profiles %s from group: %s",
-						profileConflicts, current);
-				throw new IllegalStateException(message);
-
-			}
-			groupProfiles.forEach(stack::push);
+			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);
 		}
-		return asUniqueItemList(StringUtils.toStringArray(expandedProfiles));
-	}
-
-	private Set<String> getProfileConflicts(List<String> groupProfiles, Set<String> expandedProfiles) {
-		return groupProfiles.stream().filter(expandedProfiles::contains).collect(Collectors.toSet());
+		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);
@@ -144,6 +140,21 @@ public class Profiles implements Iterable<String> {
 		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);
 	}

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

@@ -367,7 +367,17 @@ class ProfilesTests {
 		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 profiles [a] from group: a");
+				.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
@@ -378,7 +388,7 @@ class ProfilesTests {
 		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 profiles [a] from group: e");
+				.withMessageContaining("Profiles could not be resolved. Remove profile 'a' from group: 'e'");
 	}
 
 }