From 47ba819f964bf1f9dc915644b096204b57a10ef6 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 24 Apr 2023 17:22:41 +0200 Subject: [PATCH 1/2] Polish Environment and StandardEnvironmentTests See gh-30206 --- .../springframework/core/env/Environment.java | 30 +- .../springframework/core/env/Profiles.java | 28 +- .../core/env/ProfilesParser.java | 6 +- .../core/env/ProfilesTests.java | 24 +- .../core/env/StandardEnvironmentTests.java | 348 ++++++++++-------- 5 files changed, 244 insertions(+), 192 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/env/Environment.java b/spring-core/src/main/java/org/springframework/core/env/Environment.java index 8993ac52fc..c996040a44 100644 --- a/spring-core/src/main/java/org/springframework/core/env/Environment.java +++ b/spring-core/src/main/java/org/springframework/core/env/Environment.java @@ -56,6 +56,7 @@ package org.springframework.core.env; * of property sources prior to application context {@code refresh()}. * * @author Chris Beams + * @author Phillip Webb * @since 3.1 * @see PropertyResolver * @see EnvironmentCapable @@ -94,14 +95,17 @@ public interface Environment extends PropertyResolver { String[] getDefaultProfiles(); /** - * 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. If a profile begins with '!' the logic is inverted, - * i.e. the method will return {@code true} if the given profile is not active. - * For example, {@code env.acceptsProfiles("p1", "!p2")} will return {@code true} if - * profile 'p1' is active or 'p2' is not active. - * @throws IllegalArgumentException if called with zero arguments - * or if any profile is {@code null}, empty, or whitespace only + * Determine whether one or more of the given profiles is active — or + * in the case of no explicit {@linkplain #getActiveProfiles() active profiles}, + * whether one or more of the given profiles is included in the set of + * {@linkplain #getDefaultProfiles() default profiles}. + *

If a profile begins with '!' the logic is inverted, meaning this method + * will return {@code true} if the given profile is not active. For + * example, {@code env.acceptsProfiles("p1", "!p2")} will return {@code true} + * if profile 'p1' is active or 'p2' is not active. + * @throws IllegalArgumentException if called with a {@code null} array, an + * empty array, zero arguments or if any profile is {@code null}, empty, or + * whitespace only * @see #getActiveProfiles * @see #getDefaultProfiles * @see #acceptsProfiles(Profiles) @@ -111,8 +115,14 @@ public interface Environment extends PropertyResolver { boolean acceptsProfiles(String... profiles); /** - * Return whether the {@linkplain #getActiveProfiles() active profiles} - * match the given {@link Profiles} predicate. + * Determine whether the given {@link Profiles} predicate matches the + * {@linkplain #getActiveProfiles() active profiles} — or in the case + * of no explicit active profiles, whether the given {@code Profiles} predicate + * matches the {@linkplain #getDefaultProfiles() default profiles}. + *

If you wish to check a single profile expression, consider using + * {@link #acceptsProfiles(String)} instead. + * @since 5.1 + * @see Profiles#of(String...) */ boolean acceptsProfiles(Profiles profiles); diff --git a/spring-core/src/main/java/org/springframework/core/env/Profiles.java b/spring-core/src/main/java/org/springframework/core/env/Profiles.java index 7f4fba7073..340bb2b181 100644 --- a/spring-core/src/main/java/org/springframework/core/env/Profiles.java +++ b/spring-core/src/main/java/org/springframework/core/env/Profiles.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,32 +43,32 @@ public interface Profiles { /** * Create a new {@link Profiles} instance that checks for matches against - * the given profile strings. + * the given profile expressions. *

The returned instance will {@linkplain Profiles#matches(Predicate) match} - * if any one of the given profile strings matches. - *

A profile string may contain a simple profile name (for example - * {@code "production"}) or a profile expression. A profile expression allows + * if any one of the given profile expressions matches. + *

A profile expression may contain a simple profile name (for example + * {@code "production"}) or a compound expression. A compound expression allows * for more complicated profile logic to be expressed, for example * {@code "production & cloud"}. *

The following operators are supported in profile expressions. *

*

Please note that the {@code &} and {@code |} operators may not be mixed - * without using parentheses. For example {@code "a & b | c"} is not a valid - * expression; it must be expressed as {@code "(a & b) | c"} or + * without using parentheses. For example, {@code "a & b | c"} is not a valid + * expression: it must be expressed as {@code "(a & b) | c"} or * {@code "a & (b | c)"}. *

As of Spring Framework 5.1.17, two {@code Profiles} instances returned * by this method are considered equivalent to each other (in terms of * {@code equals()} and {@code hashCode()} semantics) if they are created - * with identical profile strings. - * @param profiles the profile strings to include + * with identical profile expressions. + * @param profileExpressions the profile expressions to include * @return a new {@link Profiles} instance */ - static Profiles of(String... profiles) { - return ProfilesParser.parse(profiles); + static Profiles of(String... profileExpressions) { + return ProfilesParser.parse(profileExpressions); } } diff --git a/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java b/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java index 4fac607d8f..8a17a01123 100644 --- a/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java +++ b/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java @@ -43,7 +43,7 @@ final class ProfilesParser { static Profiles parse(String... expressions) { - Assert.notEmpty(expressions, "Must specify at least one profile"); + Assert.notEmpty(expressions, "Must specify at least one profile expression"); Profiles[] parsed = new Profiles[expressions.length]; for (int i = 0; i < expressions.length; i++) { parsed[i] = parseExpression(expressions[i]); @@ -136,8 +136,8 @@ final class ProfilesParser { return activeProfile -> activeProfile.test(profile); } - private static Predicate isMatch(Predicate activeProfile) { - return profiles -> profiles.matches(activeProfile); + private static Predicate isMatch(Predicate activeProfiles) { + return profiles -> profiles.matches(activeProfiles); } diff --git a/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java b/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java index 01454ad10e..421d26edb8 100644 --- a/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java +++ b/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,30 +40,30 @@ class ProfilesTests { @Test void ofWhenNullThrowsException() { - assertThatIllegalArgumentException().isThrownBy(() -> - Profiles.of((String[]) null)) - .withMessageContaining("Must specify at least one profile"); + assertThatIllegalArgumentException() + .isThrownBy(() -> Profiles.of((String[]) null)) + .withMessage("Must specify at least one profile expression"); } @Test void ofWhenEmptyThrowsException() { assertThatIllegalArgumentException() .isThrownBy(Profiles::of) - .withMessageContaining("Must specify at least one profile"); + .withMessage("Must specify at least one profile expression"); } @Test void ofNullElement() { - assertThatIllegalArgumentException().isThrownBy(() -> - Profiles.of((String) null)) - .withMessageContaining("must contain text"); + assertThatIllegalArgumentException() + .isThrownBy(() -> Profiles.of((String) null)) + .withMessage("Invalid profile expression [null]: must contain text"); } @Test void ofEmptyElement() { - assertThatIllegalArgumentException().isThrownBy(() -> - Profiles.of(" ")) - .withMessageContaining("must contain text"); + assertThatIllegalArgumentException() + .isThrownBy(() -> Profiles.of(" ")) + .withMessage("Invalid profile expression [ ]: must contain text"); } @Test @@ -356,7 +356,7 @@ class ProfilesTests { private static void assertMalformed(Supplier supplier) { assertThatIllegalArgumentException() .isThrownBy(supplier::get) - .withMessageContaining("Malformed"); + .withMessageStartingWith("Malformed profile expression"); } private static Predicate activeProfiles(String... profiles) { diff --git a/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java b/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java index 0998e75be4..466606efcd 100644 --- a/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java +++ b/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,9 @@ package org.springframework.core.env; -import java.util.Arrays; import java.util.Map; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.core.SpringProperties; @@ -37,8 +37,7 @@ import static org.springframework.core.env.AbstractEnvironment.RESERVED_DEFAULT_ * @author Juergen Hoeller * @author Sam Brannen */ -@SuppressWarnings("deprecation") -public class StandardEnvironmentTests { +class StandardEnvironmentTests { private static final String ALLOWED_PROPERTY_NAME = "theanswer"; private static final String ALLOWED_PROPERTY_VALUE = "42"; @@ -78,8 +77,8 @@ public class StandardEnvironmentTests { assertThat(parent.getProperty("parentKey")).isEqualTo("parentVal"); assertThat(parent.getProperty("bothKey")).isEqualTo("parentBothVal"); - assertThat(child.getActiveProfiles()).isEqualTo(new String[]{"c1","c2"}); - assertThat(parent.getActiveProfiles()).isEqualTo(new String[]{"p1","p2"}); + assertThat(child.getActiveProfiles()).containsExactly("c1", "c2"); + assertThat(parent.getActiveProfiles()).containsExactly("p1", "p2"); child.merge(parent); @@ -91,8 +90,8 @@ public class StandardEnvironmentTests { assertThat(parent.getProperty("parentKey")).isEqualTo("parentVal"); assertThat(parent.getProperty("bothKey")).isEqualTo("parentBothVal"); - assertThat(child.getActiveProfiles()).isEqualTo(new String[]{"c1","c2","p1","p2"}); - assertThat(parent.getActiveProfiles()).isEqualTo(new String[]{"p1","p2"}); + assertThat(child.getActiveProfiles()).containsExactly("c1", "c2", "p1", "p2"); + assertThat(parent.getActiveProfiles()).containsExactly("p1", "p2"); } @Test @@ -118,16 +117,14 @@ public class StandardEnvironmentTests { @Test void defaultProfilesContainsDefaultProfileByDefault() { - assertThat(environment.getDefaultProfiles()).hasSize(1); - assertThat(environment.getDefaultProfiles()[0]).isEqualTo("default"); + assertThat(environment.getDefaultProfiles()).containsExactly("default"); } @Test void setActiveProfiles() { environment.setActiveProfiles("local", "embedded"); String[] activeProfiles = environment.getActiveProfiles(); - assertThat(activeProfiles).contains("local", "embedded"); - assertThat(activeProfiles).hasSize(2); + assertThat(activeProfiles).containsExactly("local", "embedded"); } @Test @@ -174,15 +171,12 @@ public class StandardEnvironmentTests { void addActiveProfile() { assertThat(environment.getActiveProfiles()).isEmpty(); environment.setActiveProfiles("local", "embedded"); - assertThat(environment.getActiveProfiles()).contains("local", "embedded"); - assertThat(environment.getActiveProfiles()).hasSize(2); + assertThat(environment.getActiveProfiles()).containsExactly("local", "embedded"); environment.addActiveProfile("p1"); - assertThat(environment.getActiveProfiles()).contains("p1"); - assertThat(environment.getActiveProfiles()).hasSize(3); + assertThat(environment.getActiveProfiles()).containsExactly("local", "embedded", "p1"); environment.addActiveProfile("p2"); environment.addActiveProfile("p3"); - assertThat(environment.getActiveProfiles()).contains("p2", "p3"); - assertThat(environment.getActiveProfiles()).hasSize(5); + assertThat(environment.getActiveProfiles()).containsExactly("local", "embedded", "p1", "p2", "p3"); } @Test @@ -192,24 +186,17 @@ public class StandardEnvironmentTests { env.getPropertySources().addFirst(new MockPropertySource().withProperty(ACTIVE_PROFILES_PROPERTY_NAME, "p1")); assertThat(env.getProperty(ACTIVE_PROFILES_PROPERTY_NAME)).isEqualTo("p1"); env.addActiveProfile("p2"); - assertThat(env.getActiveProfiles()).contains("p1", "p2"); + assertThat(env.getActiveProfiles()).containsExactly("p1", "p2"); } @Test void reservedDefaultProfile() { - assertThat(environment.getDefaultProfiles()).isEqualTo(new String[]{RESERVED_DEFAULT_PROFILE_NAME}); - System.setProperty(DEFAULT_PROFILES_PROPERTY_NAME, "d0"); - assertThat(environment.getDefaultProfiles()).isEqualTo(new String[]{"d0"}); - environment.setDefaultProfiles("d1", "d2"); - assertThat(environment.getDefaultProfiles()).isEqualTo(new String[]{"d1","d2"}); - System.clearProperty(DEFAULT_PROFILES_PROPERTY_NAME); - } - - @Test - void defaultProfileWithCircularPlaceholder() { + assertThat(environment.getDefaultProfiles()).containsExactly(RESERVED_DEFAULT_PROFILE_NAME); try { - System.setProperty(DEFAULT_PROFILES_PROPERTY_NAME, "${spring.profiles.default}"); - assertThatIllegalArgumentException().isThrownBy(environment::getDefaultProfiles); + System.setProperty(DEFAULT_PROFILES_PROPERTY_NAME, "d0"); + assertThat(environment.getDefaultProfiles()).containsExactly("d0"); + environment.setDefaultProfiles("d1", "d2"); + assertThat(environment.getDefaultProfiles()).containsExactly("d1","d2"); } finally { System.clearProperty(DEFAULT_PROFILES_PROPERTY_NAME); @@ -217,40 +204,23 @@ public class StandardEnvironmentTests { } @Test - void getActiveProfiles_systemPropertiesEmpty() { - assertThat(environment.getActiveProfiles()).isEmpty(); - System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, ""); - assertThat(environment.getActiveProfiles()).isEmpty(); - System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); - } - - @Test - void getActiveProfiles_fromSystemProperties() { - System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, "foo"); - assertThat(Arrays.asList(environment.getActiveProfiles())).contains("foo"); - System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); - } - - @Test - void getActiveProfiles_fromSystemProperties_withMultipleProfiles() { - System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, "foo,bar"); - assertThat(environment.getActiveProfiles()).contains("foo", "bar"); - System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); - } - - @Test - void getActiveProfiles_fromSystemProperties_withMultipleProfiles_withWhitespace() { - System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, " bar , baz "); // notice whitespace - assertThat(environment.getActiveProfiles()).contains("bar", "baz"); - System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); + void defaultProfileWithCircularPlaceholder() { + try { + System.setProperty(DEFAULT_PROFILES_PROPERTY_NAME, "${spring.profiles.default}"); + assertThatIllegalArgumentException() + .isThrownBy(environment::getDefaultProfiles) + .withMessage("Circular placeholder reference 'spring.profiles.default' in property definitions"); + } + finally { + System.clearProperty(DEFAULT_PROFILES_PROPERTY_NAME); + } } @Test void getDefaultProfiles() { - assertThat(environment.getDefaultProfiles()).isEqualTo(new String[] {RESERVED_DEFAULT_PROFILE_NAME}); + assertThat(environment.getDefaultProfiles()).containsExactly(RESERVED_DEFAULT_PROFILE_NAME); environment.getPropertySources().addFirst(new MockPropertySource().withProperty(DEFAULT_PROFILES_PROPERTY_NAME, "pd1")); - assertThat(environment.getDefaultProfiles()).hasSize(1); - assertThat(Arrays.asList(environment.getDefaultProfiles())).contains("pd1"); + assertThat(environment.getDefaultProfiles()).containsExactly("pd1"); } @Test @@ -258,82 +228,9 @@ public class StandardEnvironmentTests { environment.setDefaultProfiles(); assertThat(environment.getDefaultProfiles()).isEmpty(); environment.setDefaultProfiles("pd1"); - assertThat(Arrays.asList(environment.getDefaultProfiles())).contains("pd1"); + assertThat(environment.getDefaultProfiles()).containsExactly("pd1"); environment.setDefaultProfiles("pd2", "pd3"); - assertThat(environment.getDefaultProfiles()).doesNotContain("pd1"); - assertThat(environment.getDefaultProfiles()).contains("pd2", "pd3"); - } - - @Test - void acceptsProfiles_withEmptyArgumentList() { - assertThatIllegalArgumentException().isThrownBy( - environment::acceptsProfiles); - } - - @Test - void acceptsProfiles_withNullArgumentList() { - assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles((String[]) null)); - } - - @Test - void acceptsProfiles_withNullArgument() { - assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles((String) null)); - } - - @Test - void acceptsProfiles_withEmptyArgument() { - assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles("")); - } - - @Test - void acceptsProfiles_activeProfileSetProgrammatically() { - assertThat(environment.acceptsProfiles("p1", "p2")).isFalse(); - environment.setActiveProfiles("p1"); - assertThat(environment.acceptsProfiles("p1", "p2")).isTrue(); - environment.setActiveProfiles("p2"); - assertThat(environment.acceptsProfiles("p1", "p2")).isTrue(); - environment.setActiveProfiles("p1", "p2"); - assertThat(environment.acceptsProfiles("p1", "p2")).isTrue(); - } - - @Test - void acceptsProfiles_activeProfileSetViaProperty() { - assertThat(environment.acceptsProfiles("p1")).isFalse(); - environment.getPropertySources().addFirst(new MockPropertySource().withProperty(ACTIVE_PROFILES_PROPERTY_NAME, "p1")); - assertThat(environment.acceptsProfiles("p1")).isTrue(); - } - - @Test - void acceptsProfiles_defaultProfile() { - assertThat(environment.acceptsProfiles("pd")).isFalse(); - environment.setDefaultProfiles("pd"); - assertThat(environment.acceptsProfiles("pd")).isTrue(); - environment.setActiveProfiles("p1"); - assertThat(environment.acceptsProfiles("pd")).isFalse(); - assertThat(environment.acceptsProfiles("p1")).isTrue(); - } - - @Test - void acceptsProfiles_withNotOperator() { - assertThat(environment.acceptsProfiles("p1")).isFalse(); - assertThat(environment.acceptsProfiles("!p1")).isTrue(); - environment.addActiveProfile("p1"); - assertThat(environment.acceptsProfiles("p1")).isTrue(); - assertThat(environment.acceptsProfiles("!p1")).isFalse(); - } - - @Test - void acceptsProfiles_withInvalidNotOperator() { - assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles("p1", "!")); - } - - @Test - void acceptsProfiles_withProfileExpression() { - assertThat(environment.acceptsProfiles(Profiles.of("p1 & p2"))).isFalse(); - environment.addActiveProfile("p1"); - assertThat(environment.acceptsProfiles(Profiles.of("p1 & p2"))).isFalse(); - environment.addActiveProfile("p2"); - assertThat(environment.acceptsProfiles(Profiles.of("p1 & p2"))).isTrue(); + assertThat(environment.getDefaultProfiles()).containsExactly("pd2", "pd3"); } @Test @@ -343,48 +240,59 @@ public class StandardEnvironmentTests { protected void validateProfile(String profile) { super.validateProfile(profile); if (profile.contains("-")) { - throw new IllegalArgumentException( - "Invalid profile [" + profile + "]: must not contain dash character"); + throw new IllegalArgumentException("Invalid profile [" + profile + "]: must not contain dash character"); } } }; env.addActiveProfile("validProfile"); // succeeds - assertThatIllegalArgumentException().isThrownBy(() -> - env.addActiveProfile("invalid-profile")) + assertThatIllegalArgumentException() + .isThrownBy(() -> env.addActiveProfile("invalid-profile")) .withMessage("Invalid profile [invalid-profile]: must not contain dash character"); } @Test void suppressGetenvAccessThroughSystemProperty() { - System.setProperty("spring.getenv.ignore", "true"); - assertThat(environment.getSystemEnvironment().isEmpty()).isTrue(); - System.clearProperty("spring.getenv.ignore"); + try { + System.setProperty("spring.getenv.ignore", "true"); + assertThat(environment.getSystemEnvironment()).isEmpty(); + } + finally { + System.clearProperty("spring.getenv.ignore"); + } } @Test void suppressGetenvAccessThroughSpringProperty() { - SpringProperties.setProperty("spring.getenv.ignore", "true"); - assertThat(environment.getSystemEnvironment().isEmpty()).isTrue(); - SpringProperties.setProperty("spring.getenv.ignore", null); + try { + SpringProperties.setProperty("spring.getenv.ignore", "true"); + assertThat(environment.getSystemEnvironment()).isEmpty(); + } + finally { + SpringProperties.setProperty("spring.getenv.ignore", null); + } } @Test void suppressGetenvAccessThroughSpringFlag() { - SpringProperties.setFlag("spring.getenv.ignore"); - assertThat(environment.getSystemEnvironment().isEmpty()).isTrue(); - SpringProperties.setProperty("spring.getenv.ignore", null); + try { + SpringProperties.setFlag("spring.getenv.ignore"); + assertThat(environment.getSystemEnvironment()).isEmpty(); + } + finally { + SpringProperties.setProperty("spring.getenv.ignore", null); + } } @Test void getSystemProperties() { - System.setProperty(ALLOWED_PROPERTY_NAME, ALLOWED_PROPERTY_VALUE); - System.setProperty(DISALLOWED_PROPERTY_NAME, DISALLOWED_PROPERTY_VALUE); - System.getProperties().put(STRING_PROPERTY_NAME, NON_STRING_PROPERTY_VALUE); - System.getProperties().put(NON_STRING_PROPERTY_NAME, STRING_PROPERTY_VALUE); - try { + System.setProperty(ALLOWED_PROPERTY_NAME, ALLOWED_PROPERTY_VALUE); + System.setProperty(DISALLOWED_PROPERTY_NAME, DISALLOWED_PROPERTY_VALUE); + System.getProperties().put(STRING_PROPERTY_NAME, NON_STRING_PROPERTY_VALUE); + System.getProperties().put(NON_STRING_PROPERTY_NAME, STRING_PROPERTY_VALUE); + Map systemProperties = environment.getSystemProperties(); assertThat(systemProperties).isNotNull(); assertThat(System.getProperties()).isSameAs(systemProperties); @@ -408,4 +316,138 @@ public class StandardEnvironmentTests { assertThat(System.getenv()).isSameAs(systemEnvironment); } + @Nested + class GetActiveProfiles { + + @Test + void systemPropertiesEmpty() { + assertThat(environment.getActiveProfiles()).isEmpty(); + try { + System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, ""); + assertThat(environment.getActiveProfiles()).isEmpty(); + } + finally { + System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); + } + } + + @Test + void fromSystemProperties() { + try { + System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, "foo"); + assertThat(environment.getActiveProfiles()).containsExactly("foo"); + } + finally { + System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); + } + } + + @Test + void fromSystemProperties_withMultipleProfiles() { + try { + System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, "foo,bar"); + assertThat(environment.getActiveProfiles()).containsExactly("foo", "bar"); + } + finally { + System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); + } + } + + @Test + void fromSystemProperties_withMultipleProfiles_withWhitespace() { + try { + System.setProperty(ACTIVE_PROFILES_PROPERTY_NAME, " bar , baz "); // notice whitespace + assertThat(environment.getActiveProfiles()).containsExactly("bar", "baz"); + } + finally { + System.clearProperty(ACTIVE_PROFILES_PROPERTY_NAME); + } + } + } + + @Nested + class AcceptsProfilesTests { + + @Test + @SuppressWarnings("deprecation") + void withEmptyArgumentList() { + assertThatIllegalArgumentException().isThrownBy(environment::acceptsProfiles); + } + + @Test + @SuppressWarnings("deprecation") + void withNullArgumentList() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles((String[]) null)); + } + + @Test + @SuppressWarnings("deprecation") + void withNullArgument() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles((String) null)); + } + + @Test + @SuppressWarnings("deprecation") + void withEmptyArgument() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles("")); + } + + @Test + @SuppressWarnings("deprecation") + void activeProfileSetProgrammatically() { + assertThat(environment.acceptsProfiles("p1", "p2")).isFalse(); + environment.setActiveProfiles("p1"); + assertThat(environment.acceptsProfiles("p1", "p2")).isTrue(); + environment.setActiveProfiles("p2"); + assertThat(environment.acceptsProfiles("p1", "p2")).isTrue(); + environment.setActiveProfiles("p1", "p2"); + assertThat(environment.acceptsProfiles("p1", "p2")).isTrue(); + } + + @Test + @SuppressWarnings("deprecation") + void activeProfileSetViaProperty() { + assertThat(environment.acceptsProfiles("p1")).isFalse(); + environment.getPropertySources().addFirst(new MockPropertySource().withProperty(ACTIVE_PROFILES_PROPERTY_NAME, "p1")); + assertThat(environment.acceptsProfiles("p1")).isTrue(); + } + + @Test + @SuppressWarnings("deprecation") + void defaultProfile() { + assertThat(environment.acceptsProfiles("pd")).isFalse(); + environment.setDefaultProfiles("pd"); + assertThat(environment.acceptsProfiles("pd")).isTrue(); + environment.setActiveProfiles("p1"); + assertThat(environment.acceptsProfiles("pd")).isFalse(); + assertThat(environment.acceptsProfiles("p1")).isTrue(); + } + + @Test + @SuppressWarnings("deprecation") + void withNotOperator() { + assertThat(environment.acceptsProfiles("p1")).isFalse(); + assertThat(environment.acceptsProfiles("!p1")).isTrue(); + environment.addActiveProfile("p1"); + assertThat(environment.acceptsProfiles("p1")).isTrue(); + assertThat(environment.acceptsProfiles("!p1")).isFalse(); + } + + @Test + @SuppressWarnings("deprecation") + void withInvalidNotOperator() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.acceptsProfiles("p1", "!")); + } + + @Test + void withProfileExpression() { + assertThat(environment.acceptsProfiles(Profiles.of("p1 & p2"))).isFalse(); + environment.addActiveProfile("p1"); + assertThat(environment.acceptsProfiles(Profiles.of("p1 & p2"))).isFalse(); + environment.addActiveProfile("p2"); + assertThat(environment.acceptsProfiles(Profiles.of("p1 & p2"))).isTrue(); + } + + } + } From d023b94a42b2f87ca08ca169964558ad1d01e8c1 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 25 Apr 2023 10:24:15 +0200 Subject: [PATCH 2/2] Introduce Environment.matchesProfiles() for profile expressions Environment.acceptsProfiles(String...) was deprecated in 5.1 in conjunction with gh-17063 which introduced a new acceptsProfiles(Profiles) method to replace it. The deprecated method only supports OR semantics; whereas, the new method supports profile expressions. Thus, the goal was to encourage people to use the more powerful profile expressions instead of the limited OR support with profile names. However, there are use cases where it is difficult (if not impossible) to provide a Profiles instance, and there are use cases where it is simply preferable to provide profile expressions directly as strings. To address these issues, this commit introduces a new matchesProfiles() method in Environment that accepts a var-args list of profile expressions. Closes gh-30206 --- .../springframework/core/env/Environment.java | 29 ++++- .../core/env/StandardEnvironmentTests.java | 112 ++++++++++++++++++ 2 files changed, 138 insertions(+), 3 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/env/Environment.java b/spring-core/src/main/java/org/springframework/core/env/Environment.java index c996040a44..6210d17810 100644 --- a/spring-core/src/main/java/org/springframework/core/env/Environment.java +++ b/spring-core/src/main/java/org/springframework/core/env/Environment.java @@ -57,6 +57,7 @@ package org.springframework.core.env; * * @author Chris Beams * @author Phillip Webb + * @author Sam Brannen * @since 3.1 * @see PropertyResolver * @see EnvironmentCapable @@ -108,20 +109,42 @@ public interface Environment extends PropertyResolver { * whitespace only * @see #getActiveProfiles * @see #getDefaultProfiles + * @see #matchesProfiles(String...) * @see #acceptsProfiles(Profiles) - * @deprecated as of 5.1 in favor of {@link #acceptsProfiles(Profiles)} + * @deprecated as of 5.1 in favor of {@link #acceptsProfiles(Profiles)} or + * {@link #matchesProfiles(String...)} */ @Deprecated boolean acceptsProfiles(String... profiles); + /** + * Determine whether one of the given profile expressions matches the + * {@linkplain #getActiveProfiles() active profiles} — or in the case + * of no explicit active profiles, whether one of the given profile expressions + * matches the {@linkplain #getDefaultProfiles() default profiles}. + *

Profile expressions allow for complex, boolean profile logic to be + * expressed — for example {@code "p1 & p2"}, {@code "(p1 & p2) | p3"}, + * etc. See {@link Profiles#of(String...)} for details on the supported + * expression syntax. + *

This method is a convenient shortcut for + * {@code env.acceptsProfiles(Profiles.of(profileExpressions))}. + * @since 5.3.28 + * @see Profiles#of(String...) + * @see #acceptsProfiles(Profiles) + */ + default boolean matchesProfiles(String... profileExpressions) { + return acceptsProfiles(Profiles.of(profileExpressions)); + } + /** * Determine whether the given {@link Profiles} predicate matches the * {@linkplain #getActiveProfiles() active profiles} — or in the case * of no explicit active profiles, whether the given {@code Profiles} predicate * matches the {@linkplain #getDefaultProfiles() default profiles}. - *

If you wish to check a single profile expression, consider using - * {@link #acceptsProfiles(String)} instead. + *

If you wish provide profile expressions directly as strings, use + * {@link #matchesProfiles(String...)} instead. * @since 5.1 + * @see #matchesProfiles(String...) * @see Profiles#of(String...) */ boolean acceptsProfiles(Profiles profiles); diff --git a/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java b/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java index 466606efcd..36c7915ac0 100644 --- a/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java +++ b/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java @@ -450,4 +450,116 @@ class StandardEnvironmentTests { } + @Nested + class MatchesProfilesTests { + + @Test + @SuppressWarnings("deprecation") + void withEmptyArgumentList() { + assertThatIllegalArgumentException().isThrownBy(environment::matchesProfiles); + } + + @Test + @SuppressWarnings("deprecation") + void withNullArgumentList() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles((String[]) null)); + } + + @Test + @SuppressWarnings("deprecation") + void withNullArgument() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles((String) null)); + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1", null)); + } + + @Test + @SuppressWarnings("deprecation") + void withEmptyArgument() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("")); + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1", "")); + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1", " ")); + } + + @Test + @SuppressWarnings("deprecation") + void withInvalidNotOperator() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1", "!")); + } + + @Test + @SuppressWarnings("deprecation") + void withInvalidCompoundExpressionGrouping() { + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1 | p2 & p3")); + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1 & p2 | p3")); + assertThatIllegalArgumentException().isThrownBy(() -> environment.matchesProfiles("p1 & (p2 | p3) | p4")); + } + + @Test + @SuppressWarnings("deprecation") + void activeProfileSetProgrammatically() { + assertThat(environment.matchesProfiles("p1", "p2")).isFalse(); + + environment.setActiveProfiles("p1"); + assertThat(environment.matchesProfiles("p1", "p2")).isTrue(); + + environment.setActiveProfiles("p2"); + assertThat(environment.matchesProfiles("p1", "p2")).isTrue(); + + environment.setActiveProfiles("p1", "p2"); + assertThat(environment.matchesProfiles("p1", "p2")).isTrue(); + } + + @Test + @SuppressWarnings("deprecation") + void activeProfileSetViaProperty() { + assertThat(environment.matchesProfiles("p1")).isFalse(); + + environment.getPropertySources().addFirst(new MockPropertySource().withProperty(ACTIVE_PROFILES_PROPERTY_NAME, "p1")); + assertThat(environment.matchesProfiles("p1")).isTrue(); + } + + @Test + @SuppressWarnings("deprecation") + void defaultProfile() { + assertThat(environment.matchesProfiles("pd")).isFalse(); + + environment.setDefaultProfiles("pd"); + assertThat(environment.matchesProfiles("pd")).isTrue(); + + environment.setActiveProfiles("p1"); + assertThat(environment.matchesProfiles("pd")).isFalse(); + assertThat(environment.matchesProfiles("p1")).isTrue(); + } + + @Test + @SuppressWarnings("deprecation") + void withNotOperator() { + assertThat(environment.matchesProfiles("p1")).isFalse(); + assertThat(environment.matchesProfiles("!p1")).isTrue(); + + environment.addActiveProfile("p1"); + assertThat(environment.matchesProfiles("p1")).isTrue(); + assertThat(environment.matchesProfiles("!p1")).isFalse(); + } + + @Test + void withProfileExpressions() { + assertThat(environment.matchesProfiles("p1 & p2")).isFalse(); + + environment.addActiveProfile("p1"); + assertThat(environment.matchesProfiles("p1 | p2")).isTrue(); + assertThat(environment.matchesProfiles("p1 & p2")).isFalse(); + + environment.addActiveProfile("p2"); + assertThat(environment.matchesProfiles("p1 & p2")).isTrue(); + assertThat(environment.matchesProfiles("p1 | p2")).isTrue(); + assertThat(environment.matchesProfiles("foo | p1", "p2")).isTrue(); + assertThat(environment.matchesProfiles("foo | p2", "p1")).isTrue(); + assertThat(environment.matchesProfiles("foo | (p2 & p1)")).isTrue(); + assertThat(environment.matchesProfiles("p2 & (foo | p1)")).isTrue(); + assertThat(environment.matchesProfiles("foo", "(p2 & p1)")).isTrue(); + } + + } + }