Introduce AbstractEnvironment#validateProfile

Consolidates validation for profiles and provides a mechanism for
AbstractEnvironment subclasses to customize validation logic if
desired.

org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java

@@ -17,11 +17,10 @@
 package org.springframework.core.env;
 
 import static java.lang.String.format;
-import static org.springframework.util.StringUtils.commaDelimitedListToSet;
+import static org.springframework.util.StringUtils.commaDelimitedListToStringArray;
 import static org.springframework.util.StringUtils.trimAllWhitespace;
 
 import java.security.AccessControlException;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -182,7 +181,7 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 		if (this.activeProfiles.isEmpty()) {
 			String profiles = this.propertyResolver.getProperty(ACTIVE_PROFILES_PROPERTY_NAME);
 			if (StringUtils.hasText(profiles)) {
-				this.activeProfiles = commaDelimitedListToSet(trimAllWhitespace(profiles));
+				setActiveProfiles(commaDelimitedListToStringArray(trimAllWhitespace(profiles)));
 			}
 		}
 		return this.activeProfiles;
@@ -211,9 +210,9 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 	 */
 	protected Set<String> doGetDefaultProfiles() {
 		if (this.defaultProfiles.equals(this.getReservedDefaultProfiles())) {
-			String defaultProfiles = this.propertyResolver.getProperty(DEFAULT_PROFILES_PROPERTY_NAME);
-			if (defaultProfiles != null) {
-				this.defaultProfiles = commaDelimitedListToSet(trimAllWhitespace(defaultProfiles));
+			String profiles = this.propertyResolver.getProperty(DEFAULT_PROFILES_PROPERTY_NAME);
+			if (StringUtils.hasText(profiles)) {
+				this.setDefaultProfiles(commaDelimitedListToStringArray(trimAllWhitespace(profiles)));
 			}
 		}
 		return this.defaultProfiles;
@@ -228,7 +227,10 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 	 */
 	public void setDefaultProfiles(String... profiles) {
 		this.defaultProfiles.clear();
-		this.defaultProfiles.addAll(Arrays.asList(profiles));
+		for (String profile : profiles) {
+			this.validateProfile(profile);
+			this.defaultProfiles.add(profile);
+		}
 	}
 
 	public boolean acceptsProfiles(String... profiles) {
@@ -237,7 +239,7 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 		Set<String> activeProfiles = this.doGetActiveProfiles();
 		Set<String> defaultProfiles = this.doGetDefaultProfiles();
 		for (String profile : profiles) {
-			Assert.hasText(profile, "profile must not be empty");
+			this.validateProfile(profile);
 			if (activeProfiles.contains(profile)
 					|| (activeProfiles.isEmpty() && defaultProfiles.contains(profile))) {
 				activeProfileFound = true;
@@ -247,6 +249,18 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 		return activeProfileFound;
 	}
 
+	/**
+	 * Validate the given profile, called internally prior to adding to the set of
+	 * active or default profiles.
+	 * <p>Subclasses may override to impose further restrictions on profile syntax.
+	 * @throws IllegalArgumentException if the profile is null, empty or whitespace-only
+	 * @see #acceptsProfiles
+	 * @see #setDefaultProfiles
+	 */
+	protected void validateProfile(String profile) {
+		Assert.hasText(profile, "Invalid profile [" + profile + "]: must contain text");
+	}
+
 	public MutablePropertySources getPropertySources() {
 		return this.propertySources;
 	}

org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java

@@ -34,10 +34,13 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper
 	 * Specify the set of profiles active for this {@code Environment}. Profiles are
 	 * evaluated during container bootstrap to determine whether bean definitions
 	 * should be registered with the container.
+	 * <p>Any existing active profiles will be replaced with the given arguments; call
+	 * with zero arguments to clear the current set of active profiles.
 	 *
 	 * @see #setDefaultProfiles
 	 * @see org.springframework.context.annotation.Profile
 	 * @see AbstractEnvironment#ACTIVE_PROFILES_PROPERTY_NAME
+	 * @throws IllegalArgumentException if any profile is null, empty or whitespace-only
 	 */
 	void setActiveProfiles(String... profiles);
 
@@ -45,6 +48,7 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper
 	 * Specify the set of profiles to be made active by default if no other profiles
 	 * are explicitly made active through {@link #setActiveProfiles}.
 	 * @see AbstractEnvironment#DEFAULT_PROFILES_PROPERTY_NAME
+	 * @throws IllegalArgumentException if any profile is null, empty or whitespace-only
 	 */
 	void setDefaultProfiles(String... profiles);
 

org.springframework.core/src/main/java/org/springframework/core/env/Environment.java

@@ -97,12 +97,11 @@ public interface Environment extends PropertyResolver {
 	String[] getDefaultProfiles();
 
 	/**
-	 * @return whether one or more of the given profiles is active, or in the case of no
+	 * Return whether one or more of the given profiles is active or, in the case of no
 	 * explicit active profiles, whether one or more of the given profiles is included in
 	 * the set of default profiles
-	 * @throws IllegalArgumentException unless at least one profile has been specified
-	 * @throws IllegalArgumentException if any profile is the empty string or consists
-	 * only of whitespace
+	 * @throws IllegalArgumentException if called with zero arguments
+	 * @throws IllegalArgumentException if any profile is null, empty or whitespace-only
 	 * @see #getActiveProfiles
 	 * @see #getDefaultProfiles
 	 */

org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java

@@ -72,14 +72,54 @@ public class EnvironmentTests {
 	}
 
 	@Test
-	public void activeProfiles() {
+	public void activeProfilesIsEmptyByDefault() {
 		assertThat(environment.getActiveProfiles().length, is(0));
+	}
+
+	@Test
+	public void defaultProfilesContainsDefaultProfileByDefault() {
+		assertThat(environment.getDefaultProfiles().length, is(1));
+		assertThat(environment.getDefaultProfiles()[0], equalTo("default"));
+	}
+
+	@Test
+	public void setActiveProfiles() {
 		environment.setActiveProfiles("local", "embedded");
 		String[] activeProfiles = environment.getActiveProfiles();
 		assertThat(Arrays.asList(activeProfiles), hasItems("local", "embedded"));
 		assertThat(activeProfiles.length, is(2));
 	}
 
+	@Test(expected=IllegalArgumentException.class)
+	public void setActiveProfiles_withNullProfileArray() {
+		environment.setActiveProfiles((String[])null);
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void setActiveProfiles_withNullProfile() {
+		environment.setActiveProfiles((String)null);
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void setActiveProfiles_withEmptyProfile() {
+		environment.setActiveProfiles("");
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void setDefaultProfiles_withNullProfileArray() {
+		environment.setDefaultProfiles((String[])null);
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void setDefaultProfiles_withNullProfile() {
+		environment.setDefaultProfiles((String)null);
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void setDefaultProfiles_withEmptyProfile() {
+		environment.setDefaultProfiles("");
+	}
+
 	@Test
 	public void reservedDefaultProfile() {
 		assertThat(environment.getDefaultProfiles(), equalTo(new String[]{RESERVED_DEFAULT_PROFILE_NAME}));
@@ -100,7 +140,6 @@ public class EnvironmentTests {
 
 	@Test
 	public void getActiveProfiles_fromSystemProperties() {
-		assertThat(environment.getActiveProfiles().length, is(0));
 		System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, "foo");
 		assertThat(Arrays.asList(environment.getActiveProfiles()), hasItem("foo"));
 		System.getProperties().remove(ACTIVE_PROFILES_PROPERTY_NAME);
@@ -108,7 +147,6 @@ public class EnvironmentTests {
 
 	@Test
 	public void getActiveProfiles_fromSystemProperties_withMultipleProfiles() {
-		assertThat(environment.getActiveProfiles().length, is(0));
 		System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, "foo,bar");
 		assertThat(Arrays.asList(environment.getActiveProfiles()), hasItems("foo", "bar"));
 		System.getProperties().remove(ACTIVE_PROFILES_PROPERTY_NAME);
@@ -116,7 +154,6 @@ public class EnvironmentTests {
 
 	@Test
 	public void getActiveProfiles_fromSystemProperties_withMulitpleProfiles_withWhitespace() {
-		assertThat(environment.getActiveProfiles().length, is(0));
 		System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, " bar , baz "); // notice whitespace
 		assertThat(Arrays.asList(environment.getActiveProfiles()), hasItems("bar", "baz"));
 		System.getProperties().remove(ACTIVE_PROFILES_PROPERTY_NAME);
@@ -142,10 +179,26 @@ public class EnvironmentTests {
 	}
 
 	@Test(expected=IllegalArgumentException.class)
-	public void acceptsProfiles_mustSpecifyAtLeastOne() {
+	public void acceptsProfiles_withEmptyArgumentList() {
 		environment.acceptsProfiles();
 	}
 
+	@Test(expected=IllegalArgumentException.class)
+	public void acceptsProfiles_withNullArgumentList() {
+		environment.acceptsProfiles((String[])null);
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void acceptsProfiles_withNullArgument() {
+		environment.acceptsProfiles((String)null);
+	}
+
+	@Test(expected=IllegalArgumentException.class)
+	public void acceptsProfiles_withEmptyArgument() {
+		environment.acceptsProfiles("");
+	}
+
+
 	@Test
 	public void acceptsProfiles_activeProfileSetProgrammatically() {
 		assertThat(environment.acceptsProfiles("p1", "p2"), is(false));
@@ -174,6 +227,30 @@ public class EnvironmentTests {
 		assertThat(environment.acceptsProfiles("p1"), is(true));
 	}
 
+	@Test
+	public void environmentSubclass_withCustomProfileValidation() {
+		ConfigurableEnvironment env = new AbstractEnvironment() {
+			@Override
+			protected void validateProfile(String profile) {
+				super.validateProfile(profile);
+				if (profile.contains("-")) {
+					throw new IllegalArgumentException(
+							"Invalid profile [" + profile + "]: must not contain dash character");
+				}
+			}
+		};
+
+		env.addActiveProfile("validProfile"); // succeeds
+
+		try {
+			env.addActiveProfile("invalid-profile");
+			fail("expected validation exception");
+		} catch (IllegalArgumentException ex) {
+			assertThat(ex.getMessage(),
+					equalTo("Invalid profile [invalid-profile]: must not contain dash character"));
+		}
+	}
+
 	@Test
 	public void getSystemProperties_withAndWithoutSecurityManager() {
 		System.setProperty(ALLOWED_PROPERTY_NAME, ALLOWED_PROPERTY_VALUE);