From 9fcfd7e827323a0a47161779ff7fcad224b473d4 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Sat, 26 May 2012 14:09:21 +0300 Subject: [PATCH] Introduce ConfigurableEnvironment#merge Prior to this change, AbstractApplicationContext#setParent replaced the child context's Environment with the parent's Environment if available. This has the negative effect of potentially changing the type of the child context's Environment, and in any case causes property sources added directly against the child environment to be ignored. This situation could easily occur if a WebApplicationContext child had a non-web ApplicationContext set as its parent. In this case the parent Environment type would (likely) be StandardEnvironment, while the child Environment type would (likely) be StandardServletEnvironment. By directly inheriting the parent environment, critical property sources such as ServletContextPropertySource are lost entirely. This commit introduces the concept of merging an environment through the new ConfigurableEnvironment#merge method. Instead of replacing the child's environment with the parent's, AbstractApplicationContext#setParent now merges property sources as well as active and default profile names from the parent into the child. In this way, distinct environment objects are maintained with specific types and property sources preserved. See #merge Javadoc for additional details. Issue: SPR-9444, SPR-9439 --- .../support/AbstractApplicationContext.java | 15 ++++--- .../core/env/AbstractEnvironment.java | 14 +++++++ .../core/env/ConfigurableEnvironment.java | 20 +++++++++ .../core/env/StandardEnvironmentTests.java | 41 +++++++++++++++++++ .../XmlWebApplicationContextTests.java | 9 +++- 5 files changed, 92 insertions(+), 7 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java index 23b157f1d8a..8f3be1847d1 100644 --- a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java +++ b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java @@ -378,14 +378,19 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader /** * {@inheritDoc} - *

The parent {@linkplain #getEnvironment() environment} is - * delegated to this (child) context if the parent is a - * {@link ConfigurableApplicationContext} implementation. + *

The parent {@linkplain ApplicationContext#getEnvironment() environment} is + * {@linkplain ConfigurableEnvironment#merge(ConfigurableEnvironment) merged} with + * this (child) application context environment if the parent is non-{@code null} and + * its environment is an instance of {@link ConfigurableEnvironment}. + * @see ConfigurableEnvironment#merge(ConfigurableEnvironment) */ public void setParent(ApplicationContext parent) { this.parent = parent; - if (parent instanceof ConfigurableApplicationContext) { - this.setEnvironment(((ConfigurableApplicationContext)parent).getEnvironment()); + if (parent != null) { + Object parentEnvironment = parent.getEnvironment(); + if (parentEnvironment instanceof ConfigurableEnvironment) { + this.environment.merge((ConfigurableEnvironment)parentEnvironment); + } } } diff --git a/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java b/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java index b9798525db4..65574834a9a 100644 --- a/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java +++ b/spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java @@ -387,6 +387,20 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment { return systemProperties; } + public void merge(ConfigurableEnvironment parent) { + for (PropertySource ps : parent.getPropertySources()) { + if (!this.propertySources.contains(ps.getName())) { + this.propertySources.addLast(ps); + } + } + for (String profile : parent.getActiveProfiles()) { + this.activeProfiles.add(profile); + } + for (String profile : parent.getDefaultProfiles()) { + this.defaultProfiles.add(profile); + } + } + //--------------------------------------------------------------------- // Implementation of ConfigurablePropertyResolver interface diff --git a/spring-core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java b/spring-core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java index 38f4c36275c..0972d962e70 100644 --- a/spring-core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java +++ b/spring-core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java @@ -149,4 +149,24 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper */ Map getSystemProperties(); + /** + * Append the given parent environment's active profiles, default profiles and + * property sources to this (child) environment's respective collections of each. + *

For any identically-named {@code PropertySource} instance existing in both + * parent and child, the child instance is to be preserved and the parent instance + * discarded. This has the effect of allowing overriding of property sources by the + * child as well as avoiding redundant searches through common property source types, + * e.g. system environment and system properties. + *

Active and default profile names are also filtered for duplicates, to avoid + * confusion and redundant storage. + *

The parent environment remains unmodified in any case. Note that any changes to + * the parent environment occurring after the call to {@code merge} will not be + * reflected in the child. Therefore, care should be taken to configure parent + * property sources and profile information prior to calling {@code merge}. + * @param parent the environment to merge with + * @since 3.2 + * @see org.springframework.context.support.AbstractApplicationContext#setParent + */ + void merge(ConfigurableEnvironment parent); + } 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 f793eefe61c..8fca6ec6d83 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 @@ -56,6 +56,47 @@ public class StandardEnvironmentTests { private ConfigurableEnvironment environment = new StandardEnvironment(); + @Test + public void merge() { + ConfigurableEnvironment child = new StandardEnvironment(); + child.setActiveProfiles("c1", "c2"); + child.getPropertySources().addLast( + new MockPropertySource("childMock") + .withProperty("childKey", "childVal") + .withProperty("bothKey", "childBothVal")); + + ConfigurableEnvironment parent = new StandardEnvironment(); + parent.setActiveProfiles("p1", "p2"); + parent.getPropertySources().addLast( + new MockPropertySource("parentMock") + .withProperty("parentKey", "parentVal") + .withProperty("bothKey", "parentBothVal")); + + assertThat(child.getProperty("childKey"), is("childVal")); + assertThat(child.getProperty("parentKey"), nullValue()); + assertThat(child.getProperty("bothKey"), is("childBothVal")); + + assertThat(parent.getProperty("childKey"), nullValue()); + assertThat(parent.getProperty("parentKey"), is("parentVal")); + assertThat(parent.getProperty("bothKey"), is("parentBothVal")); + + assertThat(child.getActiveProfiles(), equalTo(new String[]{"c1","c2"})); + assertThat(parent.getActiveProfiles(), equalTo(new String[]{"p1","p2"})); + + child.merge(parent); + + assertThat(child.getProperty("childKey"), is("childVal")); + assertThat(child.getProperty("parentKey"), is("parentVal")); + assertThat(child.getProperty("bothKey"), is("childBothVal")); + + assertThat(parent.getProperty("childKey"), nullValue()); + assertThat(parent.getProperty("parentKey"), is("parentVal")); + assertThat(parent.getProperty("bothKey"), is("parentBothVal")); + + assertThat(child.getActiveProfiles(), equalTo(new String[]{"c1","c2","p1","p2"})); + assertThat(parent.getActiveProfiles(), equalTo(new String[]{"p1","p2"})); + } + @Test public void propertySourceOrder() { ConfigurableEnvironment env = new StandardEnvironment(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java b/spring-webmvc/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java index 8d6660baf36..c2fb218aad1 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java @@ -48,6 +48,7 @@ public class XmlWebApplicationContextTests extends AbstractApplicationContextTes protected ConfigurableApplicationContext createContext() throws Exception { InitAndIB.constructed = false; root = new XmlWebApplicationContext(); + root.getEnvironment().addActiveProfile("rootProfile1"); MockServletContext sc = new MockServletContext(""); root.setServletContext(sc); root.setConfigLocations(new String[] {"/org/springframework/web/context/WEB-INF/applicationContext.xml"}); @@ -69,6 +70,7 @@ public class XmlWebApplicationContextTests extends AbstractApplicationContextTes }); root.refresh(); XmlWebApplicationContext wac = new XmlWebApplicationContext(); + wac.getEnvironment().addActiveProfile("wacProfile1"); wac.setParent(root); wac.setServletContext(sc); wac.setNamespace("test-servlet"); @@ -77,8 +79,11 @@ public class XmlWebApplicationContextTests extends AbstractApplicationContextTes return wac; } - public void testEnvironmentInheritance() { - assertThat(this.applicationContext.getEnvironment(), sameInstance(this.root.getEnvironment())); + public void testEnvironmentMerge() { + assertThat(this.root.getEnvironment().acceptsProfiles("rootProfile1"), is(true)); + assertThat(this.root.getEnvironment().acceptsProfiles("wacProfile1"), is(false)); + assertThat(this.applicationContext.getEnvironment().acceptsProfiles("rootProfile1"), is(true)); + assertThat(this.applicationContext.getEnvironment().acceptsProfiles("wacProfile1"), is(true)); } /**