From 49177201dc2c53e035fdbd4e29dbbb0502af4c9a Mon Sep 17 00:00:00 2001 From: "Rubinson,Ethan(erubinson)" Date: Wed, 17 May 2017 16:58:03 -0700 Subject: [PATCH] Environment Conversion Fix. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When converting one environment to another as much of the original environment’s configuration should be copied over as possible. This fix copies the default profiles and the property resolver configuration which was not occurring previously. --- .../boot/EnvironmentConverter.java | 178 ++++++++++++++++++ .../boot/SpringApplication.java | 36 +--- .../boot/EnvironmentConverterTest.java | 177 +++++++++++++++++ 3 files changed, 357 insertions(+), 34 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/EnvironmentConverter.java create mode 100644 spring-boot/src/test/java/org/springframework/boot/EnvironmentConverterTest.java diff --git a/spring-boot/src/main/java/org/springframework/boot/EnvironmentConverter.java b/spring-boot/src/main/java/org/springframework/boot/EnvironmentConverter.java new file mode 100644 index 000000000000..33fe74c83a60 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/EnvironmentConverter.java @@ -0,0 +1,178 @@ +/* + * Copyright 2012-2017 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.core.env.AbstractEnvironment; +import org.springframework.core.env.AbstractPropertyResolver; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.ConfigurablePropertyResolver; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.web.context.support.StandardServletEnvironment; + +/** + * Utility class for converting one environment type to another. + * + * @author Ethan Rubinson + * @since 1.5.4 + */ +final class EnvironmentConverter { + + private static final Log logger = LogFactory.getLog(EnvironmentConverter.class); + + private static final Set SERVLET_ENVIRONMENT_SOURCE_NAMES; + + static { + final Set names = new HashSet(); + names.add(StandardServletEnvironment.SERVLET_CONTEXT_PROPERTY_SOURCE_NAME); + names.add(StandardServletEnvironment.SERVLET_CONFIG_PROPERTY_SOURCE_NAME); + names.add(StandardServletEnvironment.JNDI_PROPERTY_SOURCE_NAME); + SERVLET_ENVIRONMENT_SOURCE_NAMES = Collections.unmodifiableSet(names); + } + + private EnvironmentConverter() { + + } + + /** + * Converts the specified environment to a {@link StandardEnvironment}. + * + * @param environment The environment to convert. + * @return The converted environment. + */ + protected static ConfigurableEnvironment convertToStandardEnvironment( + ConfigurableEnvironment environment) { + final StandardEnvironment result = new StandardEnvironment(); + + /* Handle specific environment type conversions */ + if (environment instanceof AbstractEnvironment) { + cloneAbstractEnvironmentSpecificConfigurations( + (AbstractEnvironment) environment, result); + } + + /* Copy the profiles */ + result.setActiveProfiles(environment.getActiveProfiles()); + result.setDefaultProfiles(environment.getDefaultProfiles()); + + /* Copy the conversion service */ + result.setConversionService(environment.getConversionService()); + + /* + * Copy over all of the property sources except those unrelated to a standard + * environment + */ + removeAllPropertySources(result.getPropertySources()); + for (PropertySource propertySource : environment.getPropertySources()) { + if (!SERVLET_ENVIRONMENT_SOURCE_NAMES.contains(propertySource.getName())) { + result.getPropertySources().addLast(propertySource); + } + } + + return result; + } + + private static void cloneAbstractEnvironmentSpecificConfigurations( + AbstractEnvironment oldEnv, AbstractEnvironment newEnv) { + clonePropertyResolverConfiguration(oldEnv, newEnv); + } + + private static void clonePropertyResolverConfiguration(AbstractEnvironment oldEnv, + AbstractEnvironment newEnv) { + try { + final Field newPropertyResolverField = AbstractEnvironment.class + .getDeclaredField("propertyResolver"); + newPropertyResolverField.setAccessible(true); + final ConfigurablePropertyResolver newPropertyResolver = (ConfigurablePropertyResolver) newPropertyResolverField + .get(newEnv); + + /* Attempt to get the current property resolver of the abstract environment */ + ConfigurablePropertyResolver oldPropertyResolver = null; + if (oldEnv instanceof AbstractEnvironment) { + final Field oldPropertyResolverField = AbstractEnvironment.class + .getDeclaredField("propertyResolver"); + oldPropertyResolverField.setAccessible(true); + oldPropertyResolver = (ConfigurablePropertyResolver) oldPropertyResolverField + .get(oldEnv); + } + + if (oldPropertyResolver instanceof AbstractPropertyResolver) { + final Field ignoreUnresolvableNestedPlaceholdersField = AbstractPropertyResolver.class + .getDeclaredField("ignoreUnresolvableNestedPlaceholders"); + final Field placeholderPrefixField = AbstractPropertyResolver.class + .getDeclaredField("placeholderPrefix"); + final Field placeholderSuffixField = AbstractPropertyResolver.class + .getDeclaredField("placeholderSuffix"); + final Field valueSeparatorField = AbstractPropertyResolver.class + .getDeclaredField("valueSeparator"); + final Field requiredPropertiesField = AbstractPropertyResolver.class + .getDeclaredField("requiredProperties"); + + ignoreUnresolvableNestedPlaceholdersField.setAccessible(true); + placeholderPrefixField.setAccessible(true); + placeholderSuffixField.setAccessible(true); + valueSeparatorField.setAccessible(true); + requiredPropertiesField.setAccessible(true); + + final boolean ignoreUnresolvableNestedPlaceholders = ignoreUnresolvableNestedPlaceholdersField + .getBoolean(oldPropertyResolver); + final String placeholderPrefix = (String) placeholderPrefixField + .get(oldPropertyResolver); + final String placeholderSuffix = (String) placeholderSuffixField + .get(oldPropertyResolver); + final String valueSeparator = (String) valueSeparatorField + .get(oldPropertyResolver); + final Set requiredProperties = (Set) requiredPropertiesField + .get(oldPropertyResolver); + + newPropertyResolver + .setIgnoreUnresolvableNestedPlaceholders(ignoreUnresolvableNestedPlaceholders); + newPropertyResolver.setPlaceholderPrefix(placeholderPrefix); + newPropertyResolver.setPlaceholderSuffix(placeholderSuffix); + newPropertyResolver.setValueSeparator(valueSeparator); + newPropertyResolver.setRequiredProperties(requiredProperties + .toArray(new String[0])); + } + } + catch (Exception conversionEx) { + final String errMsg = "Failed to clone the environment's property resolver configuration"; + if (logger.isErrorEnabled()) { + logger.error(errMsg, conversionEx); + } + throw new RuntimeException(errMsg, conversionEx); + } + } + + private static void removeAllPropertySources(MutablePropertySources propertySources) { + final Set names = new HashSet(); + for (PropertySource propertySource : propertySources) { + names.add(propertySource.getName()); + } + for (String name : names) { + propertySources.remove(name); + } + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index a7234a97ef8e..38eef24fe2e2 100644 --- a/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -140,6 +140,7 @@ * @author Jeremy Rickard * @author Craig Burke * @author Michael Simons + * @author Ethan Rubinson * @see #run(Object, String[]) * @see #run(Object[], String[]) * @see #SpringApplication(Object...) @@ -177,16 +178,6 @@ public class SpringApplication { private static final String SYSTEM_PROPERTY_JAVA_AWT_HEADLESS = "java.awt.headless"; - private static final Set SERVLET_ENVIRONMENT_SOURCE_NAMES; - - static { - Set names = new HashSet(); - names.add(StandardServletEnvironment.SERVLET_CONTEXT_PROPERTY_SOURCE_NAME); - names.add(StandardServletEnvironment.SERVLET_CONFIG_PROPERTY_SOURCE_NAME); - names.add(StandardServletEnvironment.JNDI_PROPERTY_SOURCE_NAME); - SERVLET_ENVIRONMENT_SOURCE_NAMES = Collections.unmodifiableSet(names); - } - private static final Log logger = LogFactory.getLog(SpringApplication.class); private final Set sources = new LinkedHashSet(); @@ -335,7 +326,7 @@ private ConfigurableEnvironment prepareEnvironment( configureEnvironment(environment, applicationArguments.getSourceArgs()); listeners.environmentPrepared(environment); if (isWebEnvironment(environment) && !this.webEnvironment) { - environment = convertToStandardEnvironment(environment); + environment = EnvironmentConverter.convertToStandardEnvironment(environment); } return environment; } @@ -465,29 +456,6 @@ private boolean isWebEnvironment(ConfigurableEnvironment environment) { } } - private ConfigurableEnvironment convertToStandardEnvironment( - ConfigurableEnvironment environment) { - StandardEnvironment result = new StandardEnvironment(); - removeAllPropertySources(result.getPropertySources()); - result.setActiveProfiles(environment.getActiveProfiles()); - for (PropertySource propertySource : environment.getPropertySources()) { - if (!SERVLET_ENVIRONMENT_SOURCE_NAMES.contains(propertySource.getName())) { - result.getPropertySources().addLast(propertySource); - } - } - return result; - } - - private void removeAllPropertySources(MutablePropertySources propertySources) { - Set names = new HashSet(); - for (PropertySource propertySource : propertySources) { - names.add(propertySource.getName()); - } - for (String name : names) { - propertySources.remove(name); - } - } - /** * Add, remove or re-order any {@link PropertySource}s in this application's * environment. diff --git a/spring-boot/src/test/java/org/springframework/boot/EnvironmentConverterTest.java b/spring-boot/src/test/java/org/springframework/boot/EnvironmentConverterTest.java new file mode 100644 index 000000000000..3c567a4cae61 --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/EnvironmentConverterTest.java @@ -0,0 +1,177 @@ +/* + * Copyright 2012-2017 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import java.lang.reflect.Field; +import java.util.HashSet; +import java.util.Set; + +import org.bouncycastle.util.Arrays; +import org.junit.Assert; +import org.junit.Test; + +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.converter.ConverterFactory; +import org.springframework.core.convert.converter.GenericConverter; +import org.springframework.core.convert.support.ConfigurableConversionService; +import org.springframework.core.env.AbstractEnvironment; +import org.springframework.core.env.AbstractPropertyResolver; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.ConfigurablePropertyResolver; +import org.springframework.mock.env.MockEnvironment; + +/** + * Tests for the {@link EnvironmentConverter} methods + * + * @author Ethan Rubinson + */ +public class EnvironmentConverterTest { + + @Test + public void testConvertAbstractEnvironmentToStandardEnvironment() throws Exception { + final AbstractEnvironment baseEnv = new MockEnvironment(); + final CustomConversionService customConverterServce = new CustomConversionService( + baseEnv.getConversionService()); + final String[] activeProfiles = new String[] { "activeProfile1", "activeProfile2" }; + final String[] defaultProfiles = new String[] { "defaultProfile1", + "defaultProfile2" }; + final boolean ignoreUnresolvableNestedPlaceholders = true; + final String placeholderPrefix = "PREFIX-{"; + final String placeholderSuffix = "}-SUFFIX"; + final String valueSeparator = ":::::"; + final Set requiredProps = new HashSet(3); + requiredProps.add("required1"); + requiredProps.add("required2"); + requiredProps.add("required3"); + + baseEnv.setActiveProfiles(activeProfiles); + baseEnv.setDefaultProfiles(defaultProfiles); + baseEnv.setConversionService(customConverterServce); + baseEnv.setIgnoreUnresolvableNestedPlaceholders(ignoreUnresolvableNestedPlaceholders); + baseEnv.setPlaceholderPrefix(placeholderPrefix); + baseEnv.setPlaceholderSuffix(placeholderSuffix); + baseEnv.setValueSeparator(valueSeparator); + baseEnv.setRequiredProperties(requiredProps.toArray(new String[0])); + + ConfigurableEnvironment convertedEnv = EnvironmentConverter + .convertToStandardEnvironment(baseEnv); + final Field propertyResolverField = AbstractEnvironment.class + .getDeclaredField("propertyResolver"); + propertyResolverField.setAccessible(true); + ConfigurablePropertyResolver propertyResolver = (ConfigurablePropertyResolver) propertyResolverField + .get(convertedEnv); + + final Field ignoreUnresolvableNestedPlaceholdersField = AbstractPropertyResolver.class + .getDeclaredField("ignoreUnresolvableNestedPlaceholders"); + final Field placeholderPrefixField = AbstractPropertyResolver.class + .getDeclaredField("placeholderPrefix"); + final Field placeholderSuffixField = AbstractPropertyResolver.class + .getDeclaredField("placeholderSuffix"); + final Field valueSeparatorField = AbstractPropertyResolver.class + .getDeclaredField("valueSeparator"); + final Field requiredPropertiesField = AbstractPropertyResolver.class + .getDeclaredField("requiredProperties"); + + ignoreUnresolvableNestedPlaceholdersField.setAccessible(true); + placeholderPrefixField.setAccessible(true); + placeholderSuffixField.setAccessible(true); + valueSeparatorField.setAccessible(true); + requiredPropertiesField.setAccessible(true); + + final boolean convertedIgnoreUnresolvableNestedPlaceholders = ignoreUnresolvableNestedPlaceholdersField + .getBoolean(propertyResolver); + final String convertedPlaceholderPrefix = (String) placeholderPrefixField + .get(propertyResolver); + final String convertedPlaceholderSuffix = (String) placeholderSuffixField + .get(propertyResolver); + final String convertedValueSeparator = (String) valueSeparatorField + .get(propertyResolver); + final Set convertedRequiredProperties = (Set) requiredPropertiesField + .get(propertyResolver); + + Assert.assertTrue(Arrays.areEqual(activeProfiles, + convertedEnv.getActiveProfiles())); + Assert.assertTrue(Arrays.areEqual(defaultProfiles, + convertedEnv.getDefaultProfiles())); + Assert.assertEquals(customConverterServce, convertedEnv.getConversionService()); + Assert.assertEquals(ignoreUnresolvableNestedPlaceholders, + convertedIgnoreUnresolvableNestedPlaceholders); + Assert.assertEquals(placeholderPrefix, convertedPlaceholderPrefix); + Assert.assertEquals(placeholderSuffix, convertedPlaceholderSuffix); + Assert.assertEquals(valueSeparator, convertedValueSeparator); + Assert.assertEquals(requiredProps, convertedRequiredProperties); + } + + private class CustomConversionService implements ConfigurableConversionService { + + private final ConfigurableConversionService delegate; + + CustomConversionService(ConfigurableConversionService delegate) { + this.delegate = delegate; + } + + @Override + public boolean canConvert(Class sourceType, Class targetType) { + return this.delegate.canConvert(sourceType, targetType); + } + + @Override + public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { + return this.delegate.canConvert(sourceType, targetType); + } + + @Override + public T convert(Object source, Class targetType) { + return this.delegate.convert(source, targetType); + } + + @Override + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + return this.delegate.convert(source, sourceType, targetType); + } + + @Override + public void addConverter(Converter converter) { + this.delegate.addConverter(converter); + } + + @Override + public void addConverter(Class sourceType, Class targetType, + Converter converter) { + this.delegate.addConverter(sourceType, targetType, converter); + } + + @Override + public void addConverter(GenericConverter converter) { + this.delegate.addConverter(converter); + } + + @Override + public void addConverterFactory(ConverterFactory factory) { + this.delegate.addConverterFactory(factory); + } + + @Override + public void removeConvertible(Class sourceType, Class targetType) { + this.delegate.removeConvertible(sourceType, targetType); + } + + } + +}