Skip to content

Commit 5b30269

Browse files
wilkinsonawilkinsona
committed
Polish "Copy conversion service when performing environment conversion"
Closes gh-9246
1 parent a424081 commit 5b30269

File tree

4 files changed

+135
-149
lines changed

4 files changed

+135
-149
lines changed

‎spring-boot/src/main/java/org/springframework/boot/EnvironmentConverter.java

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,24 @@
2121
import java.util.Set;
2222

2323
import org.springframework.core.env.ConfigurableEnvironment;
24+
import org.springframework.core.env.Environment;
2425
import org.springframework.core.env.MutablePropertySources;
2526
import org.springframework.core.env.PropertySource;
2627
import org.springframework.core.env.StandardEnvironment;
28+
import org.springframework.util.ClassUtils;
29+
import org.springframework.web.context.ConfigurableWebEnvironment;
2730
import org.springframework.web.context.support.StandardServletEnvironment;
2831

2932
/**
30-
* Utility class for converting one environment type to another.
33+
* Utility class for converting one type of {@link Environment} to another.
3134
*
3235
* @author Ethan Rubinson
33-
* @since 1.5.4
36+
* @author Andy Wilkinson
3437
*/
3538
final class EnvironmentConverter {
3639

40+
private static final String CONFIGURABLE_WEB_ENVIRONMENT_CLASS = "org.springframework.web.context.ConfigurableWebEnvironment";
41+
3742
private static final Set<String> SERVLET_ENVIRONMENT_SOURCE_NAMES;
3843

3944
static {
@@ -44,42 +49,68 @@ final class EnvironmentConverter {
4449
SERVLET_ENVIRONMENT_SOURCE_NAMES = Collections.unmodifiableSet(names);
4550
}
4651

47-
private EnvironmentConverter() {
52+
private final ClassLoader classLoader;
4853

54+
/**
55+
* Creates a new {@link EnvironmentConverter} that will use the given
56+
* {@code classLoader} during conversion.
57+
* @param classLoader the class loader to use
58+
*/
59+
EnvironmentConverter(ClassLoader classLoader) {
60+
this.classLoader = classLoader;
4961
}
5062

5163
/**
52-
* Converts the specified environment to a {@link StandardEnvironment}.
64+
* Converts the given {@code environment} to a {@link StandardEnvironment}. If the
65+
* environment is already a {@code StandardEnvironment} and is not a
66+
* {@link ConfigurableWebEnvironment} no conversion is performed and it is returned
67+
* unchanged.
5368
*
54-
* @param environment The environment to convert.
55-
* @return The converted environment.
69+
* @param environment The Environment to convert
70+
* @return The converted Environment
5671
*/
57-
protected static ConfigurableEnvironment convertToStandardEnvironment(
72+
StandardEnvironment convertToStandardEnvironmentIfNecessary(
5873
ConfigurableEnvironment environment) {
59-
final StandardEnvironment result = new StandardEnvironment();
74+
if (environment instanceof StandardEnvironment
75+
&& !isWebEnvironment(environment, this.classLoader)) {
76+
return (StandardEnvironment) environment;
77+
}
78+
return convertToStandardEnvironment(environment);
79+
}
6080

61-
/* Copy the profiles */
62-
result.setActiveProfiles(environment.getActiveProfiles());
81+
private boolean isWebEnvironment(ConfigurableEnvironment environment,
82+
ClassLoader classLoader) {
83+
try {
84+
Class<?> webEnvironmentClass = ClassUtils
85+
.forName(CONFIGURABLE_WEB_ENVIRONMENT_CLASS, classLoader);
86+
return (webEnvironmentClass.isInstance(environment));
87+
}
88+
catch (Throwable ex) {
89+
return false;
90+
}
91+
}
6392

64-
/* Copy the conversion service */
93+
private StandardEnvironment convertToStandardEnvironment(
94+
ConfigurableEnvironment environment) {
95+
StandardEnvironment result = new StandardEnvironment();
96+
result.setActiveProfiles(environment.getActiveProfiles());
6597
result.setConversionService(environment.getConversionService());
98+
copyNonServletPropertySources(environment, result);
99+
return result;
100+
}
66101

67-
/*
68-
* Copy over all of the property sources except those unrelated to a standard
69-
* environment
70-
*/
71-
removeAllPropertySources(result.getPropertySources());
72-
for (PropertySource<?> propertySource : environment.getPropertySources()) {
102+
private void copyNonServletPropertySources(ConfigurableEnvironment source,
103+
StandardEnvironment target) {
104+
removeAllPropertySources(target.getPropertySources());
105+
for (PropertySource<?> propertySource : source.getPropertySources()) {
73106
if (!SERVLET_ENVIRONMENT_SOURCE_NAMES.contains(propertySource.getName())) {
74-
result.getPropertySources().addLast(propertySource);
107+
target.getPropertySources().addLast(propertySource);
75108
}
76109
}
77-
78-
return result;
79110
}
80111

81-
private static void removeAllPropertySources(MutablePropertySources propertySources) {
82-
final Set<String> names = new HashSet<String>();
112+
private void removeAllPropertySources(MutablePropertySources propertySources) {
113+
Set<String> names = new HashSet<String>();
83114
for (PropertySource<?> propertySource : propertySources) {
84115
names.add(propertySource.getName());
85116
}

‎spring-boot/src/main/java/org/springframework/boot/SpringApplication.java

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,6 @@ public class SpringApplication {
174174
*/
175175
public static final String BANNER_LOCATION_PROPERTY = SpringApplicationBannerPrinter.BANNER_LOCATION_PROPERTY;
176176

177-
private static final String CONFIGURABLE_WEB_ENVIRONMENT_CLASS = "org.springframework.web.context.ConfigurableWebEnvironment";
178-
179177
private static final String SYSTEM_PROPERTY_JAVA_AWT_HEADLESS = "java.awt.headless";
180178

181179
private static final Log logger = LogFactory.getLog(SpringApplication.class);
@@ -325,8 +323,9 @@ private ConfigurableEnvironment prepareEnvironment(
325323
ConfigurableEnvironment environment = getOrCreateEnvironment();
326324
configureEnvironment(environment, applicationArguments.getSourceArgs());
327325
listeners.environmentPrepared(environment);
328-
if (isWebEnvironment(environment) && !this.webEnvironment) {
329-
environment = EnvironmentConverter.convertToStandardEnvironment(environment);
326+
if (!this.webEnvironment) {
327+
environment = new EnvironmentConverter(getClassLoader())
328+
.convertToStandardEnvironmentIfNecessary(environment);
330329
}
331330
return environment;
332331
}
@@ -445,17 +444,6 @@ protected void configureEnvironment(ConfigurableEnvironment environment,
445444
configureProfiles(environment, args);
446445
}
447446

448-
private boolean isWebEnvironment(ConfigurableEnvironment environment) {
449-
try {
450-
Class<?> webEnvironmentClass = ClassUtils
451-
.forName(CONFIGURABLE_WEB_ENVIRONMENT_CLASS, getClassLoader());
452-
return (webEnvironmentClass.isInstance(environment));
453-
}
454-
catch (Throwable ex) {
455-
return false;
456-
}
457-
}
458-
459447
/**
460448
* Add, remove or re-order any {@link PropertySource}s in this application's
461449
* environment.

‎spring-boot/src/test/java/org/springframework/boot/EnvironmentConverterTest.java

Lines changed: 0 additions & 112 deletions
This file was deleted.

‎spring-boot/src/test/java/org/springframework/boot/EnvironmentConverterTests.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Copyright 2012-2017 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot;
18+
19+
import org.junit.Test;
20+
21+
import org.springframework.core.convert.support.ConfigurableConversionService;
22+
import org.springframework.core.env.AbstractEnvironment;
23+
import org.springframework.core.env.StandardEnvironment;
24+
import org.springframework.mock.env.MockEnvironment;
25+
import org.springframework.web.context.support.StandardServletEnvironment;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
import static org.mockito.Mockito.mock;
29+
30+
/**
31+
* Tests for {@link EnvironmentConverter}.
32+
*
33+
* @author Ethan Rubinson
34+
* @author Andy Wilkinson
35+
*/
36+
public class EnvironmentConverterTests {
37+
38+
private final EnvironmentConverter environmentConverter = new EnvironmentConverter(
39+
getClass().getClassLoader());
40+
41+
@Test
42+
public void convertedEnvironmentHasSameActiveProfiles() {
43+
AbstractEnvironment originalEnvironment = new MockEnvironment();
44+
originalEnvironment.setActiveProfiles("activeProfile1", "activeProfile2");
45+
StandardEnvironment convertedEnvironment = this.environmentConverter
46+
.convertToStandardEnvironmentIfNecessary(originalEnvironment);
47+
assertThat(convertedEnvironment.getActiveProfiles())
48+
.containsExactly("activeProfile1", "activeProfile2");
49+
}
50+
51+
@Test
52+
public void convertedEnvironmentHasSameConversionService() {
53+
AbstractEnvironment originalEnvironment = new MockEnvironment();
54+
ConfigurableConversionService conversionService = mock(
55+
ConfigurableConversionService.class);
56+
originalEnvironment.setConversionService(conversionService);
57+
StandardEnvironment convertedEnvironment = this.environmentConverter
58+
.convertToStandardEnvironmentIfNecessary(originalEnvironment);
59+
assertThat(convertedEnvironment.getConversionService())
60+
.isEqualTo(conversionService);
61+
}
62+
63+
@Test
64+
public void standardEnvironmentIsReturnedUnconverted() {
65+
StandardEnvironment standardEnvironment = new StandardEnvironment();
66+
StandardEnvironment convertedEnvironment = this.environmentConverter
67+
.convertToStandardEnvironmentIfNecessary(standardEnvironment);
68+
assertThat(convertedEnvironment).isSameAs(standardEnvironment);
69+
}
70+
71+
@Test
72+
public void standardServletEnvironmentIsConverted() {
73+
StandardServletEnvironment standardServletEnvironment = new StandardServletEnvironment();
74+
StandardEnvironment convertedEnvironment = this.environmentConverter
75+
.convertToStandardEnvironmentIfNecessary(standardServletEnvironment);
76+
assertThat(convertedEnvironment).isNotSameAs(standardServletEnvironment);
77+
}
78+
79+
}

0 commit comments

Comments
 (0)