From cab9bff4f4cc2eb099af91cd0d56ce9162172213 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Mon, 16 Apr 2018 12:36:09 -0700 Subject: [PATCH] Log included profiles according to the processing order This commit includes some refactoring of active profiles processing. Previously, there was a LIFO Queue for adding active profiles. Profiles that were added last, were processed first. Because of this reverse ordering, profiles were prepended to the environment to preserve the order in which they were logged. This however didn't work for "included" profiles as they were prepended to the environment even though they were processed after the active profile. In this commit, profiles are processed in a FIFO manner and processed as they're found. Fixes gh-11380 --- .../config/ConfigFileApplicationListener.java | 133 ++++++------------ .../ConfigFileApplicationListenerTests.java | 2 + 2 files changed, 47 insertions(+), 88 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java index 6ca646c5c0..50f8208d04 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java @@ -26,7 +26,6 @@ import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Queue; import java.util.Set; import java.util.function.BiConsumer; import java.util.stream.Collectors; @@ -302,7 +301,7 @@ public class ConfigFileApplicationListener private final List propertySourceLoaders; - private Queue profiles; + private LinkedList profiles; private List processedProfiles; @@ -321,7 +320,7 @@ public class ConfigFileApplicationListener } public void load() { - this.profiles = Collections.asLifoQueue(new LinkedList()); + this.profiles = new LinkedList<>(); this.processedProfiles = new LinkedList<>(); this.activatedProfiles = false; this.loaded = new LinkedHashMap<>(); @@ -343,62 +342,62 @@ public class ConfigFileApplicationListener * properties that are already set. */ private void initializeProfiles() { - Set initialActiveProfiles = initializeActiveProfiles(); - this.profiles.addAll(getUnprocessedActiveProfiles(initialActiveProfiles)); - if (this.profiles.isEmpty()) { + //The default profile for these purposes is represented as null. We add it + // first so that it is processed first and has lowest priority. + this.profiles.add(null); + Set activatedViaProperty = getProfilesActivatedViaActiveProfileProperty(); + processOtherActiveProfiles(activatedViaProperty); + // Any pre-existing active activeProfiles set via property sources (e.g. System + // properties) take precedence over those added in config files. + addActiveProfiles(activatedViaProperty); + if (this.profiles.size() == 1) { //only has null profile for (String defaultProfileName : this.environment.getDefaultProfiles()) { - Profile defaultProfile = new Profile(defaultProfileName, true); - if (!this.profiles.contains(defaultProfile)) { - this.profiles.add(defaultProfile); - } + ConfigFileApplicationListener.Profile defaultProfile = new ConfigFileApplicationListener.Profile( + defaultProfileName, true); + this.profiles.add(defaultProfile); } } - // The default profile for these purposes is represented as null. We add it - // last so that it is first out of the queue (active profiles will then - // override any settings in the defaults when the list is reversed later). - this.profiles.add(null); } - private Set initializeActiveProfiles() { + private Set getProfilesActivatedViaActiveProfileProperty() { if (!this.environment.containsProperty(ACTIVE_PROFILES_PROPERTY) && !this.environment.containsProperty(INCLUDE_PROFILES_PROPERTY)) { return Collections.emptySet(); } - // Any pre-existing active profiles set via property sources (e.g. System - // properties) take precedence over those added in config files. Binder binder = Binder.get(this.environment); Set activeProfiles = new LinkedHashSet<>(); activeProfiles.addAll(getProfiles(binder, ACTIVE_PROFILES_PROPERTY)); activeProfiles.addAll(getProfiles(binder, INCLUDE_PROFILES_PROPERTY)); - maybeActivateProfiles(activeProfiles); return activeProfiles; } - /** - * Return the active profiles that have not been processed yet. If a profile is - * enabled via both {@link #ACTIVE_PROFILES_PROPERTY} and - * {@link ConfigurableEnvironment#addActiveProfile(String)} it needs to be - * filtered so that the {@link #ACTIVE_PROFILES_PROPERTY} value takes precedence. - *

- * Concretely, if the "cloud" profile is enabled via the environment, it will take - * less precedence that any profile set via the {@link #ACTIVE_PROFILES_PROPERTY}. - * @param initialActiveProfiles the profiles that have been enabled via - * {@link #ACTIVE_PROFILES_PROPERTY} - * @return the unprocessed active profiles from the environment to enable - */ - private List getUnprocessedActiveProfiles( - Set initialActiveProfiles) { - List unprocessedActiveProfiles = new ArrayList<>(); - for (String profileName : this.environment.getActiveProfiles()) { - Profile profile = new Profile(profileName); - if (!initialActiveProfiles.contains(profile)) { - unprocessedActiveProfiles.add(profile); - } + private void processOtherActiveProfiles(Set activatedViaProperty) { + List otherActiveProfiles = Arrays.stream(this.environment.getActiveProfiles()) + .map(Profile::new) + .filter(o -> !activatedViaProperty.contains(o)).collect(Collectors.toList()); + this.profiles.addAll(otherActiveProfiles); + } + + void addActiveProfiles(Set profiles) { + if (this.activatedProfiles || profiles.isEmpty()) { + return; } - // Reverse them so the order is the same as from getProfilesForValue() - // (last one wins when properties are eventually resolved) - Collections.reverse(unprocessedActiveProfiles); - return unprocessedActiveProfiles; + addProfiles(profiles); + this.logger.debug("Activated activeProfiles " + + StringUtils.collectionToCommaDelimitedString(profiles)); + this.activatedProfiles = true; + removeUnprocessedDefaultProfiles(); + } + + void addProfiles(Set profiles) { + for (Profile profile : profiles) { + this.profiles.add(profile); + addProfileToEnvironment(profile.getName()); + } + } + + private void removeUnprocessedDefaultProfiles() { + this.profiles.removeIf(profile -> (profile != null && profile.isDefaultProfile())); } private DocumentFilter getPositiveProfileFilter(Profile profile) { @@ -520,7 +519,7 @@ public class ConfigFileApplicationListener List loaded = new ArrayList<>(); for (Document document : documents) { if (filter.match(document)) { - maybeActivateProfiles(document.getActiveProfiles()); + addActiveProfiles(document.getActiveProfiles()); addProfiles(document.getIncludeProfiles()); loaded.add(document); } @@ -587,58 +586,16 @@ public class ConfigFileApplicationListener for (String profileName : profileNames) { profiles.add(new Profile(profileName)); } - Collections.reverse(profiles); return new LinkedHashSet<>(profiles); } - private void maybeActivateProfiles(Set profiles) { - if (profiles.isEmpty()) { - return; - } - if (this.activatedProfiles) { - this.logger.debug("Profiles already activated, '" + profiles - + "' will not be applied"); - return; - } - addProfiles(profiles); - this.logger.debug("Activated profiles " - + StringUtils.collectionToCommaDelimitedString(profiles)); - this.activatedProfiles = true; - removeUnprocessedDefaultProfiles(); - } - - private void removeUnprocessedDefaultProfiles() { - this.profiles.removeIf(Profile::isDefaultProfile); - } - - private void addProfiles(Set profiles) { - for (Profile profile : profiles) { - this.profiles.add(profile); - if (!environmentHasActiveProfile(profile.getName())) { - // If it's already accepted we assume the order was set - // intentionally - prependProfile(this.environment, profile); - } - } - } - - private boolean environmentHasActiveProfile(String profile) { + private void addProfileToEnvironment(String profile) { for (String activeProfile : this.environment.getActiveProfiles()) { if (activeProfile.equals(profile)) { - return true; + return; } } - return false; - } - - private void prependProfile(ConfigurableEnvironment environment, - Profile profile) { - Set profiles = new LinkedHashSet<>(); - environment.getActiveProfiles(); // ensure they are initialized - // But this one should go first (last wins in a property key clash) - profiles.add(profile.getName()); - profiles.addAll(Arrays.asList(environment.getActiveProfiles())); - environment.setActiveProfiles(StringUtils.toStringArray(profiles)); + this.environment.addActiveProfile(profile); } private Set getSearchLocations() { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java index 37b064cdf2..7508ac2337 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java @@ -758,6 +758,8 @@ public class ConfigFileApplicationListenerTests { assertThat(environment).has(matchingProfile("morespecific")); assertThat(environment).has(matchingProfile("yetmorespecific")); assertThat(environment).doesNotHave(matchingProfile("missing")); + assertThat(this.out.toString()) + .contains("The following profiles are active: includeprofile,specific,morespecific,yetmorespecific"); } @Test