Skip to content

Commit 20f87ab

Browse files
committed
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: 9fcfd7e
1 parent c25a1bc commit 20f87ab

File tree

5 files changed

+126
-49
lines changed

5 files changed

+126
-49
lines changed

org.springframework.context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2010 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -378,14 +378,19 @@ protected ResourcePatternResolver getResourcePatternResolver() {
378378

379379
/**
380380
* {@inheritDoc}
381-
* <p>The parent {@linkplain #getEnvironment() environment} is
382-
* delegated to this (child) context if the parent is a
383-
* {@link ConfigurableApplicationContext} implementation.
381+
* <p>The parent {@linkplain ApplicationContext#getEnvironment() environment} is
382+
* {@linkplain ConfigurableEnvironment#merge(ConfigurableEnvironment) merged} with
383+
* this (child) application context environment if the parent is non-{@code null} and
384+
* its environment is an instance of {@link ConfigurableEnvironment}.
385+
* @see ConfigurableEnvironment#merge(ConfigurableEnvironment)
384386
*/
385387
public void setParent(ApplicationContext parent) {
386388
this.parent = parent;
387-
if (parent instanceof ConfigurableApplicationContext) {
388-
this.setEnvironment(((ConfigurableApplicationContext)parent).getEnvironment());
389+
if (parent != null) {
390+
Object parentEnvironment = parent.getEnvironment();
391+
if (parentEnvironment instanceof ConfigurableEnvironment) {
392+
this.environment.merge((ConfigurableEnvironment)parentEnvironment);
393+
}
389394
}
390395
}
391396

@@ -506,7 +511,7 @@ protected void prepareRefresh() {
506511
/**
507512
* <p>Replace any stub property sources with actual instances.
508513
* @see org.springframework.core.env.PropertySource.StubPropertySource
509-
* @see org.springframework.web.context.support.WebApplicationContextUtils#initSerlvetPropertySources
514+
* @see org.springframework.web.context.support.WebApplicationContextUtils#initServletPropertySources
510515
*/
511516
protected void initPropertySources() {
512517
// For subclasses: do nothing by default.

org.springframework.core/src/main/java/org/springframework/core/env/AbstractEnvironment.java

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@
4141
*
4242
* <p>Concrete subclasses differ primarily on which {@link PropertySource} objects they
4343
* add by default. {@code AbstractEnvironment} adds none. Subclasses should contribute
44-
* property sources through the protected {@link #customizePropertySources()} hook, while
45-
* clients should customize using {@link ConfigurableEnvironment#getPropertySources()} and
46-
* working against the {@link MutablePropertySources} API. See
44+
* property sources through the protected {@link #customizePropertySources(MutablePropertySources)}
45+
* hook, while clients should customize using {@link ConfigurableEnvironment#getPropertySources()}
46+
* and working against the {@link MutablePropertySources} API. See
4747
* {@link ConfigurableEnvironment} Javadoc for usage examples.
4848
*
4949
* @author Chris Beams
@@ -387,6 +387,20 @@ protected String getSystemAttribute(String propertyName) {
387387
return systemProperties;
388388
}
389389

390+
public void merge(ConfigurableEnvironment parent) {
391+
for (PropertySource<?> ps : parent.getPropertySources()) {
392+
if (!this.propertySources.contains(ps.getName())) {
393+
this.propertySources.addLast(ps);
394+
}
395+
}
396+
for (String profile : parent.getActiveProfiles()) {
397+
this.activeProfiles.add(profile);
398+
}
399+
for (String profile : parent.getDefaultProfiles()) {
400+
this.defaultProfiles.add(profile);
401+
}
402+
}
403+
390404

391405
//---------------------------------------------------------------------
392406
// Implementation of ConfigurablePropertyResolver interface

org.springframework.core/src/main/java/org/springframework/core/env/ConfigurableEnvironment.java

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -55,13 +55,14 @@
5555
* propertySources.replace(StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, mockEnvVars);
5656
* </pre>
5757
*
58-
* When an {@link Environment} is being used by an ApplicationContext, it is important
59-
* that any such PropertySource manipulations be performed <em>before</em> the context's
60-
* {@link org.springframework.context.support.AbstractApplicationContext#refresh()
61-
* refresh()} method is called. This ensures that all property sources are available
62-
* during the container bootstrap process, including use by
63-
* {@linkplain org.springframework.context.support.PropertySourcesPlaceholderConfigurer
64-
* property placeholder configurers}.
58+
* When an {@link Environment} is being used by an {@code ApplicationContext}, it is
59+
* important that any such {@code PropertySource} manipulations be performed
60+
* <em>before</em> the context's {@link
61+
* org.springframework.context.support.AbstractApplicationContext#refresh() refresh()}
62+
* method is called. This ensures that all property sources are available during the
63+
* container bootstrap process, including use by {@linkplain
64+
* org.springframework.context.support.PropertySourcesPlaceholderConfigurer property
65+
* placeholder configurers}.
6566
*
6667
*
6768
* @author Chris Beams
@@ -78,7 +79,6 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper
7879
* <p>Any existing active profiles will be replaced with the given arguments; call
7980
* with zero arguments to clear the current set of active profiles. Use
8081
* {@link #addActiveProfile} to add a profile while preserving the existing set.
81-
*
8282
* @see #addActiveProfile
8383
* @see #setDefaultProfiles
8484
* @see org.springframework.context.annotation.Profile
@@ -123,12 +123,10 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper
123123
* Return the value of {@link System#getenv()} if allowed by the current
124124
* {@link SecurityManager}, otherwise return a map implementation that will attempt
125125
* to access individual keys using calls to {@link System#getenv(String)}.
126-
*
127126
* <p>Note that most {@link Environment} implementations will include this system
128127
* environment map as a default {@link PropertySource} to be searched. Therefore, it
129128
* is recommended that this method not be used directly unless bypassing other
130129
* property sources is expressly intended.
131-
*
132130
* <p>Calls to {@link Map#get(Object)} on the Map returned will never throw
133131
* {@link IllegalAccessException}; in cases where the SecurityManager forbids access
134132
* to a property, {@code null} will be returned and an INFO-level log message will be
@@ -140,17 +138,35 @@ public interface ConfigurableEnvironment extends Environment, ConfigurableProper
140138
* Return the value of {@link System#getProperties()} if allowed by the current
141139
* {@link SecurityManager}, otherwise return a map implementation that will attempt
142140
* to access individual keys using calls to {@link System#getProperty(String)}.
143-
*
144141
* <p>Note that most {@code Environment} implementations will include this system
145142
* properties map as a default {@link PropertySource} to be searched. Therefore, it is
146143
* recommended that this method not be used directly unless bypassing other property
147144
* sources is expressly intended.
148-
*
149145
* <p>Calls to {@link Map#get(Object)} on the Map returned will never throw
150146
* {@link IllegalAccessException}; in cases where the SecurityManager forbids access
151147
* to a property, {@code null} will be returned and an INFO-level log message will be
152148
* issued noting the exception.
153149
*/
154150
Map<String, Object> getSystemProperties();
155151

152+
/**
153+
* Append the given parent environment's active profiles, default profiles and
154+
* property sources to this (child) environment's respective collections of each.
155+
* <p>For any identically-named {@code PropertySource} instance existing in both
156+
* parent and child, the child instance is to be preserved and the parent instance
157+
* discarded. This has the effect of allowing overriding of property sources by the
158+
* child as well as avoiding redundant searches through common property source types,
159+
* e.g. system environment and system properties.
160+
* <p>Active and default profile names are also filtered for duplicates, to avoid
161+
* confusion and redundant storage.
162+
* <p>The parent environment remains unmodified in any case. Note that any changes to
163+
* the parent environment occurring after the call to {@code merge} will not be
164+
* reflected in the child. Therefore, care should be taken to configure parent
165+
* property sources and profile information prior to calling {@code merge}.
166+
* @param parent the environment to merge with
167+
* @since 3.2
168+
* @see org.springframework.context.support.AbstractApplicationContext#setParent
169+
*/
170+
void merge(ConfigurableEnvironment parent);
171+
156172
}

org.springframework.core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,22 +16,6 @@
1616

1717
package org.springframework.core.env;
1818

19-
import static java.lang.String.format;
20-
import static org.hamcrest.CoreMatchers.equalTo;
21-
import static org.hamcrest.CoreMatchers.instanceOf;
22-
import static org.hamcrest.CoreMatchers.is;
23-
import static org.hamcrest.CoreMatchers.not;
24-
import static org.hamcrest.CoreMatchers.notNullValue;
25-
import static org.hamcrest.CoreMatchers.nullValue;
26-
import static org.junit.Assert.assertSame;
27-
import static org.junit.Assert.assertThat;
28-
import static org.junit.Assert.fail;
29-
import static org.junit.matchers.JUnitMatchers.hasItem;
30-
import static org.junit.matchers.JUnitMatchers.hasItems;
31-
import static org.springframework.core.env.AbstractEnvironment.ACTIVE_PROFILES_PROPERTY_NAME;
32-
import static org.springframework.core.env.AbstractEnvironment.DEFAULT_PROFILES_PROPERTY_NAME;
33-
import static org.springframework.core.env.AbstractEnvironment.RESERVED_DEFAULT_PROFILE_NAME;
34-
3519
import java.lang.reflect.Field;
3620
import java.security.AccessControlException;
3721
import java.security.Permission;
@@ -40,8 +24,18 @@
4024
import java.util.Map;
4125

4226
import org.junit.Test;
27+
4328
import org.springframework.mock.env.MockPropertySource;
4429

30+
import static java.lang.String.*;
31+
32+
import static org.hamcrest.CoreMatchers.*;
33+
34+
import static org.junit.Assert.*;
35+
import static org.junit.matchers.JUnitMatchers.*;
36+
37+
import static org.springframework.core.env.AbstractEnvironment.*;
38+
4539
/**
4640
* Unit tests for {@link StandardEnvironment}.
4741
*
@@ -62,6 +56,47 @@ public class StandardEnvironmentTests {
6256

6357
private ConfigurableEnvironment environment = new StandardEnvironment();
6458

59+
@Test
60+
public void merge() {
61+
ConfigurableEnvironment child = new StandardEnvironment();
62+
child.setActiveProfiles("c1", "c2");
63+
child.getPropertySources().addLast(
64+
new MockPropertySource("childMock")
65+
.withProperty("childKey", "childVal")
66+
.withProperty("bothKey", "childBothVal"));
67+
68+
ConfigurableEnvironment parent = new StandardEnvironment();
69+
parent.setActiveProfiles("p1", "p2");
70+
parent.getPropertySources().addLast(
71+
new MockPropertySource("parentMock")
72+
.withProperty("parentKey", "parentVal")
73+
.withProperty("bothKey", "parentBothVal"));
74+
75+
assertThat(child.getProperty("childKey"), is("childVal"));
76+
assertThat(child.getProperty("parentKey"), nullValue());
77+
assertThat(child.getProperty("bothKey"), is("childBothVal"));
78+
79+
assertThat(parent.getProperty("childKey"), nullValue());
80+
assertThat(parent.getProperty("parentKey"), is("parentVal"));
81+
assertThat(parent.getProperty("bothKey"), is("parentBothVal"));
82+
83+
assertThat(child.getActiveProfiles(), equalTo(new String[]{"c1","c2"}));
84+
assertThat(parent.getActiveProfiles(), equalTo(new String[]{"p1","p2"}));
85+
86+
child.merge(parent);
87+
88+
assertThat(child.getProperty("childKey"), is("childVal"));
89+
assertThat(child.getProperty("parentKey"), is("parentVal"));
90+
assertThat(child.getProperty("bothKey"), is("childBothVal"));
91+
92+
assertThat(parent.getProperty("childKey"), nullValue());
93+
assertThat(parent.getProperty("parentKey"), is("parentVal"));
94+
assertThat(parent.getProperty("bothKey"), is("parentBothVal"));
95+
96+
assertThat(child.getActiveProfiles(), equalTo(new String[]{"c1","c2","p1","p2"}));
97+
assertThat(parent.getActiveProfiles(), equalTo(new String[]{"p1","p2"}));
98+
}
99+
65100
@Test
66101
public void propertySourceOrder() {
67102
ConfigurableEnvironment env = new StandardEnvironment();

org.springframework.web.servlet/src/test/java/org/springframework/web/context/XmlWebApplicationContextTests.java

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/*
2-
* Copyright 2002-2005 the original author or authors.
3-
*
2+
* Copyright 2002-2012 the original author or authors.
3+
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
7-
*
7+
*
88
* http://www.apache.org/licenses/LICENSE-2.0
9-
*
9+
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
1212
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -16,9 +16,6 @@
1616

1717
package org.springframework.web.context;
1818

19-
import static org.hamcrest.CoreMatchers.sameInstance;
20-
import static org.junit.Assert.assertThat;
21-
2219
import java.util.Locale;
2320

2421
import javax.servlet.ServletException;
@@ -36,6 +33,10 @@
3633
import org.springframework.mock.web.MockServletContext;
3734
import org.springframework.web.context.support.XmlWebApplicationContext;
3835

36+
import static org.hamcrest.CoreMatchers.*;
37+
38+
import static org.junit.Assert.*;
39+
3940
/**
4041
* @author Rod Johnson
4142
* @author Juergen Hoeller
@@ -47,12 +48,14 @@ public class XmlWebApplicationContextTests extends AbstractApplicationContextTes
4748
protected ConfigurableApplicationContext createContext() throws Exception {
4849
InitAndIB.constructed = false;
4950
root = new XmlWebApplicationContext();
51+
root.getEnvironment().addActiveProfile("rootProfile1");
5052
MockServletContext sc = new MockServletContext("");
5153
root.setServletContext(sc);
5254
root.setConfigLocations(new String[] {"/org/springframework/web/context/WEB-INF/applicationContext.xml"});
5355
root.addBeanFactoryPostProcessor(new BeanFactoryPostProcessor() {
5456
public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) {
5557
beanFactory.addBeanPostProcessor(new BeanPostProcessor() {
58+
@SuppressWarnings("unchecked")
5659
public Object postProcessBeforeInitialization(Object bean, String name) throws BeansException {
5760
if (bean instanceof TestBean) {
5861
((TestBean) bean).getFriends().add("myFriend");
@@ -67,6 +70,7 @@ public Object postProcessAfterInitialization(Object bean, String name) throws Be
6770
});
6871
root.refresh();
6972
XmlWebApplicationContext wac = new XmlWebApplicationContext();
73+
wac.getEnvironment().addActiveProfile("wacProfile1");
7074
wac.setParent(root);
7175
wac.setServletContext(sc);
7276
wac.setNamespace("test-servlet");
@@ -75,8 +79,11 @@ public Object postProcessAfterInitialization(Object bean, String name) throws Be
7579
return wac;
7680
}
7781

78-
public void testEnvironmentInheritance() {
79-
assertThat(this.applicationContext.getEnvironment(), sameInstance(this.root.getEnvironment()));
82+
public void testEnvironmentMerge() {
83+
assertThat(this.root.getEnvironment().acceptsProfiles("rootProfile1"), is(true));
84+
assertThat(this.root.getEnvironment().acceptsProfiles("wacProfile1"), is(false));
85+
assertThat(this.applicationContext.getEnvironment().acceptsProfiles("rootProfile1"), is(true));
86+
assertThat(this.applicationContext.getEnvironment().acceptsProfiles("wacProfile1"), is(true));
8087
}
8188

8289
/**

0 commit comments

Comments
 (0)