From 7c1bf582621314d3143861fb1972cb3f83cc65f7 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 22 Oct 2015 13:46:09 +0200 Subject: [PATCH] Filter duplicate Improve the initial PR to include a filtering of the profiles that were already enabled via the `spring.profiles.active` property. Also add more tests to prove that each profile is loaded only once now. Closes gh-4273 --- .../main/asciidoc/spring-boot-features.adoc | 3 + .../config/ConfigFileApplicationListener.java | 37 ++++++-- .../ConfigFileApplicationListenerTests.java | 85 ++++++++++++++++--- 3 files changed, 109 insertions(+), 16 deletions(-) diff --git a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 6400155c364..4310e0b937a 100644 --- a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -369,6 +369,9 @@ Profile specific properties are loaded from the same locations as standard ones irrespective of whether the profile-specific files are inside or outside your packaged jar. +If several profiles are specified, a last wins strategy applies. For example, profiles +specified by the `spring.active.profiles` property are added after those configured via +the `SpringApplication` API and therefore take precedence. [[boot-features-external-config-placeholders-in-properties]] diff --git a/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java b/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java index b3c8ea160aa..4578f2367e4 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java @@ -89,6 +89,7 @@ import org.springframework.validation.BindException; * * @author Dave Syer * @author Phillip Webb + * @author Stephane Nicoll */ public class ConfigFileApplicationListener implements ApplicationListener, Ordered { @@ -308,17 +309,19 @@ public class ConfigFileApplicationListener this.propertiesLoader = new PropertySourcesLoader(); this.profiles = Collections.asLifoQueue(new LinkedList()); this.activatedProfiles = false; + + Set initialActiveProfiles = null; if (this.environment.containsProperty(ACTIVE_PROFILES_PROPERTY)) { // Any pre-existing active profiles set via property sources (e.g. System // properties) take precedence over those added in config files. - maybeActivateProfiles( + initialActiveProfiles = maybeActivateProfiles( this.environment.getProperty(ACTIVE_PROFILES_PROPERTY)); } // Pre-existing active profiles set via Environment.setActiveProfiles() // are additional profiles and config files are allowed to add more if // they want to, so don't call addActiveProfiles() here. - List list = new ArrayList( - Arrays.asList(this.environment.getActiveProfiles())); + List list = filterEnvironmentProfiles(initialActiveProfiles != null + ? initialActiveProfiles : Collections.emptySet()); // Reverse them so the order is the same as from getProfilesForValue() // (last one wins when properties are eventually resolved) Collections.reverse(list); @@ -404,13 +407,36 @@ public class ConfigFileApplicationListener return propertySource; } - private void maybeActivateProfiles(Object value) { + /** + * 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 additional profiles from the environment to enable + */ + private List filterEnvironmentProfiles(Set initialActiveProfiles) { + List additionalProfiles = new ArrayList(); + for (String profile : this.environment.getActiveProfiles()) { + if (!initialActiveProfiles.contains(profile)) { + additionalProfiles.add(profile); + } + } + return additionalProfiles; + } + + private Set maybeActivateProfiles(Object value) { if (this.activatedProfiles) { if (value != null) { this.debug.add("Profiles already activated, '" + value + "' will not be applied"); } - return; + return Collections.emptySet(); } Set profiles = getProfilesForValue(value); @@ -420,6 +446,7 @@ public class ConfigFileApplicationListener + StringUtils.collectionToCommaDelimitedString(profiles)); this.activatedProfiles = true; } + return profiles; } private void addIncludeProfiles(Object value) { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java b/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java index a273cdb5756..a649fbddc0c 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/config/ConfigFileApplicationListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2010-2014 the original author or authors. + * Copyright 2012-2015 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. @@ -27,20 +27,28 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import ch.qos.logback.classic.BasicConfigurator; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.TypeSafeDiagnosingMatcher; import org.junit.After; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.slf4j.LoggerFactory; import org.springframework.boot.SpringApplication; import org.springframework.boot.context.config.ConfigFileApplicationListener.ConfigurationPropertySources; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; +import org.springframework.boot.context.event.ApplicationPreparedEvent; import org.springframework.boot.env.EnumerableCompositePropertySource; import org.springframework.boot.test.EnvironmentTestUtils; +import org.springframework.boot.test.OutputCapture; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Profile; import org.springframework.context.annotation.PropertySource; @@ -55,13 +63,8 @@ import org.springframework.core.io.ResourceLoader; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; /** * Tests for {@link ConfigFileApplicationListener}. @@ -81,6 +84,17 @@ public class ConfigFileApplicationListenerTests { @Rule public ExpectedException expected = ExpectedException.none(); + @Rule + public OutputCapture out = new OutputCapture(); + + @Before + public void resetLogging() { + LoggerContext loggerContext = ((Logger) LoggerFactory.getLogger(getClass())) + .getLoggerContext(); + loggerContext.reset(); + BasicConfigurator.configure(loggerContext); + } + @After public void cleanup() { System.clearProperty("the.property"); @@ -343,14 +357,63 @@ public class ConfigFileApplicationListenerTests { @Test public void profilesAddedToEnvironmentAndViaProperty() throws Exception { + // External profile takes precedence over profile added via the environment EnvironmentTestUtils.addEnvironment(this.environment, - "spring.profiles.active:foo"); + "spring.profiles.active:other"); this.environment.addActiveProfile("dev"); this.initializer.onApplicationEvent(this.event); - assertThat(this.environment.getActiveProfiles(), - equalTo(new String[] { "foo", "dev" })); + assertThat(Arrays.asList(this.environment.getActiveProfiles()), containsInAnyOrder("dev", "other")); + assertThat(this.environment.getProperty("my.property"), + equalTo("fromotherpropertiesfile")); + validateProfilePrecedence(null, "dev", "other"); + } + + @Test + public void profilesAddedToEnvironmentAndViaPropertyDuplicate() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment, + "spring.profiles.active:dev,other"); + this.environment.addActiveProfile("dev"); + this.initializer.onApplicationEvent(this.event); + assertThat(Arrays.asList(this.environment.getActiveProfiles()), containsInAnyOrder("dev", "other")); + assertThat(this.environment.getProperty("my.property"), + equalTo("fromotherpropertiesfile")); + validateProfilePrecedence(null, "dev", "other"); + } + + @Test + public void profilesAddedToEnvironmentAndViaPropertyDuplicateEnvironmentWins() throws Exception { + EnvironmentTestUtils.addEnvironment(this.environment, + "spring.profiles.active:other,dev"); + this.environment.addActiveProfile("other"); + this.initializer.onApplicationEvent(this.event); + assertThat(Arrays.asList(this.environment.getActiveProfiles()), containsInAnyOrder("dev", "other")); assertThat(this.environment.getProperty("my.property"), equalTo("fromdevpropertiesfile")); + validateProfilePrecedence(null, "other", "dev"); + } + + private void validateProfilePrecedence(String... profiles) { + this.initializer.onApplicationEvent(new ApplicationPreparedEvent( + new SpringApplication(), new String[0], new AnnotationConfigApplicationContext())); + String log = this.out.toString(); + + // First make sure that each profile got processed only once + for (String profile : profiles) { + assertThat("Wrong number of occurrences for profile '" + profile + "' --> " + log, + StringUtils.countOccurrencesOf(log, createLogForProfile(profile)), equalTo(1)); + } + // Make sure the order of loading is the right one + for (String profile : profiles) { + String line = createLogForProfile(profile); + int index = log.indexOf(line); + assertTrue("Loading profile '" + profile + "' not found in '" + log + "'", index != -1); + log = log.substring(index + line.length(), log.length()); + } + } + + private String createLogForProfile(String profile) { + String suffix = profile != null ? "-" + profile : ""; + return "Loaded config file 'classpath:/application" + suffix + ".properties'"; } @Test