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

Author: snicoll

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]]

spring-boot/src/main/java/org/springframework/boot/context/config/ConfigFileApplicationListener.java

@@ -89,6 +89,7 @@
  *
  * @author Dave Syer
  * @author Phillip Webb
+ * @author Stephane Nicoll
  */
 public class ConfigFileApplicationListener
 		implements ApplicationListener<ApplicationEvent>, Ordered {
@@ -308,17 +309,19 @@ public void load() throws IOException {
 			this.propertiesLoader = new PropertySourcesLoader();
 			this.profiles = Collections.asLifoQueue(new LinkedList<String>());
 			this.activatedProfiles = false;
+
+			Set<String> 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<String> list = new ArrayList<String>(
-					Arrays.asList(this.environment.getActiveProfiles()));
+			List<String> list = filterEnvironmentProfiles(initialActiveProfiles != null
+					? initialActiveProfiles : Collections.<String>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 @@ private PropertySource<?> loadIntoGroup(String identifier, String location,
 			return propertySource;
 		}
 
-		private void maybeActivateProfiles(Object value) {
+		/**
+		 * Return the active profiles that have not been processed yet.
+		 * <p>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.
+		 * <p>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<String> filterEnvironmentProfiles(Set<String> initialActiveProfiles) {
+			List<String> additionalProfiles = new ArrayList<String>();
+			for (String profile : this.environment.getActiveProfiles()) {
+				if (!initialActiveProfiles.contains(profile)) {
+					additionalProfiles.add(profile);
+				}
+			}
+			return additionalProfiles;
+		}
+
+		private Set<String> 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<String> profiles = getProfilesForValue(value);
@@ -420,6 +446,7 @@ private void maybeActivateProfiles(Object value) {
 						+ StringUtils.collectionToCommaDelimitedString(profiles));
 				this.activatedProfiles = true;
 			}
+			return profiles;
 		}
 
 		private void addIncludeProfiles(Object value) {

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.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.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 void profilePropertiesUsedInPlaceholders() throws Exception {
 
 	@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