From 20f87ab98da8a813c0251979286286e5e3776435 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Wed, 27 Jun 2012 17:01:44 +0200 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-9446 Backport-Issue: SPR-9444, SPR-9439 Backport-Commit: 9fcfd7e827323a0a47161779ff7fcad224b473d4 --- .../support/AbstractApplicationContext.java | 19 +++-- .../core/env/AbstractEnvironment.java | 20 +++++- .../core/env/ConfigurableEnvironment.java | 42 +++++++---- .../core/env/StandardEnvironmentTests.java | 69 ++++++++++++++----- .../XmlWebApplicationContextTests.java | 25 ++++--- 5 files changed, 126 insertions(+), 49 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java b/org.springframework.context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java index 84831884b7..833e43b441 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java +++ b/org.springframework.context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -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); + } } } @@ -506,7 +511,7 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader /** *

Replace any stub property sources with actual instances. * @see org.springframework.core.env.PropertySource.StubPropertySource - * @see org.springframework.web.context.support.WebApplicationContextUtils#initSerlvetPropertySources + * @see org.springframework.web.context.support.WebApplicationContextUtils#initServletPropertySources */ protected void initPropertySources() { // For subclasses: do nothing by default. diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java b/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java index 4e93c5a278..65574834a9 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java +++ b/org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java @@ -41,9 +41,9 @@ import static org.springframework.util.StringUtils.*; * *

Concrete subclasses differ primarily on which {@link PropertySource} objects they * add by default. {@code AbstractEnvironment} adds none. Subclasses should contribute - * property sources through the protected {@link #customizePropertySources()} hook, while - * clients should customize using {@link ConfigurableEnvironment#getPropertySources()} and - * working against the {@link MutablePropertySources} API. See + * property sources through the protected {@link #customizePropertySources(MutablePropertySources)} + * hook, while clients should customize using {@link ConfigurableEnvironment#getPropertySources()} + * and working against the {@link MutablePropertySources} API. See * {@link ConfigurableEnvironment} Javadoc for usage examples. * * @author Chris Beams @@ -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/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java b/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java index 8e8aad80ea..0972d962e7 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java +++ b/org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -55,13 +55,14 @@ import java.util.Map; * propertySources.replace(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, mockEnvVars); * * - * When an {@link Environment} is being used by an ApplicationContext, it is important - * that any such PropertySource manipulations be performed before the context's - * {@link org.springframework.context.support.AbstractApplicationContext#refresh() - * refresh()} method is called. This ensures that all property sources are available - * during the container bootstrap process, including use by - * {@linkplain org.springframework.context.support.PropertySourcesPlaceholderConfigurer - * property placeholder configurers}. + * When an {@link Environment} is being used by an {@code ApplicationContext}, it is + * important that any such {@code PropertySource} manipulations be performed + * before the context's {@link + * org.springframework.context.support.AbstractApplicationContext#refresh() refresh()} + * method is called. This ensures that all property sources are available during the + * container bootstrap process, including use by {@linkplain + * org.springframework.context.support.PropertySourcesPlaceholderConfigurer property + * placeholder configurers}. * * * @author Chris Beams @@ -78,7 +79,6 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper *

Any existing active profiles will be replaced with the given arguments; call * with zero arguments to clear the current set of active profiles. Use * {@link #addActiveProfile} to add a profile while preserving the existing set. - * * @see #addActiveProfile * @see #setDefaultProfiles * @see org.springframework.context.annotation.Profile @@ -123,12 +123,10 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper * Return the value of {@link System#getenv()} if allowed by the current * {@link SecurityManager}, otherwise return a map implementation that will attempt * to access individual keys using calls to {@link System#getenv(String)}. - * *

Note that most {@link Environment} implementations will include this system * environment map as a default {@link PropertySource} to be searched. Therefore, it * is recommended that this method not be used directly unless bypassing other * property sources is expressly intended. - * *

Calls to {@link Map#get(Object)} on the Map returned will never throw * {@link IllegalAccessException}; in cases where the SecurityManager forbids access * to a property, {@code null} will be returned and an INFO-level log message will be @@ -140,12 +138,10 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper * Return the value of {@link System#getProperties()} if allowed by the current * {@link SecurityManager}, otherwise return a map implementation that will attempt * to access individual keys using calls to {@link System#getProperty(String)}. - * *

Note that most {@code Environment} implementations will include this system * properties map as a default {@link PropertySource} to be searched. Therefore, it is * recommended that this method not be used directly unless bypassing other property * sources is expressly intended. - * *

Calls to {@link Map#get(Object)} on the Map returned will never throw * {@link IllegalAccessException}; in cases where the SecurityManager forbids access * to a property, {@code null} will be returned and an INFO-level log message will be @@ -153,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/org.springframework.core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java b/org.springframework.core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java index 4a5881dcb7..8fca6ec6d8 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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,22 +16,6 @@ package org.springframework.core.env; -import static java.lang.String.format; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.CoreMatchers.notNullValue; -import static org.hamcrest.CoreMatchers.nullValue; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; -import static org.junit.matchers.JUnitMatchers.hasItem; -import static org.junit.matchers.JUnitMatchers.hasItems; -import static org.springframework.core.env.AbstractEnvironment.ACTIVE_PROFILES_PROPERTY_NAME; -import static org.springframework.core.env.AbstractEnvironment.DEFAULT_PROFILES_PROPERTY_NAME; -import static org.springframework.core.env.AbstractEnvironment.RESERVED_DEFAULT_PROFILE_NAME; - import java.lang.reflect.Field; import java.security.AccessControlException; import java.security.Permission; @@ -40,8 +24,18 @@ import java.util.Collections; import java.util.Map; import org.junit.Test; + import org.springframework.mock.env.MockPropertySource; +import static java.lang.String.*; + +import static org.hamcrest.CoreMatchers.*; + +import static org.junit.Assert.*; +import static org.junit.matchers.JUnitMatchers.*; + +import static org.springframework.core.env.AbstractEnvironment.*; + /** * Unit tests for {@link StandardEnvironment}. * @@ -62,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/org.springframework.web.servlet/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java index b20b13220b..09525632d6 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java @@ -1,12 +1,12 @@ /* - * Copyright 2002-2005 the original author or authors. - * + * Copyright 2002-2012 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. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -16,9 +16,6 @@ package org.springframework.web.context; -import static org.hamcrest.CoreMatchers.sameInstance; -import static org.junit.Assert.assertThat; - import java.util.Locale; import javax.servlet.ServletException; @@ -36,6 +33,10 @@ import org.springframework.context.TestListener; import org.springframework.mock.web.MockServletContext; import org.springframework.web.context.support.XmlWebApplicationContext; +import static org.hamcrest.CoreMatchers.*; + +import static org.junit.Assert.*; + /** * @author Rod Johnson * @author Juergen Hoeller @@ -47,12 +48,14 @@ 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"}); root.addBeanFactoryPostProcessor(new BeanFactoryPostProcessor() { public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) { beanFactory.addBeanPostProcessor(new BeanPostProcessor() { + @SuppressWarnings("unchecked") public Object postProcessBeforeInitialization(Object bean, String name) throws BeansException { if (bean instanceof TestBean) { ((TestBean) bean).getFriends().add("myFriend"); @@ -67,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"); @@ -75,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)); } /**