diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java b/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java index 08eba615e2..4c6e5e3bfd 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java +++ b/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 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 activeProfiles = this.doGetActiveProfiles(); Set 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. + *

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; } diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java b/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java index 7e41e73b2f..264645b36a 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java +++ b/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. + *

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); diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/Environment.java b/org.springframework.core/src/main/java/org/springframework/core/env/Environment.java index e412384458..0a8ba0727b 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/Environment.java +++ b/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 */ diff --git a/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java b/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java index cf386c641f..f076735da1 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java +++ b/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);