Skip to content

Commit

Permalink
Short circuit already covered property sources
Browse files Browse the repository at this point in the history
Add an alternative `PropertySourcesPropertyResolver` that can short
circuit resolution of properties that are already covered by the
`ConfigurationPropertySourcesPropertySource`.

Prior to this commit, calling `getProperty` or `containsProperty` on an
`Environment` that has `ConfigurationPropertySources` attached could
result in two identical calls to the underlying source. The first call
would be via the adapted source, and the second would be direct. Since
we can now plug-in a custom `PropertySourcesPropertyResolver` to the
`Environment`, we can optimize resolution so that calls happen only
once.

Closes gh-17400
  • Loading branch information
philwebb committed Apr 9, 2021
1 parent 1d302f4 commit 6ad100e
Show file tree
Hide file tree
Showing 9 changed files with 347 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package org.springframework.boot;

import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.core.env.ConfigurablePropertyResolver;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.StandardEnvironment;

/**
Expand All @@ -35,4 +38,9 @@ protected String doGetDefaultProfilesProperty() {
return null;
}

@Override
protected ConfigurablePropertyResolver createPropertyResolver(MutablePropertySources propertySources) {
return ConfigurationPropertySources.createPropertyResolver(propertySources);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

package org.springframework.boot;

import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.boot.web.reactive.context.StandardReactiveWebEnvironment;
import org.springframework.core.env.ConfigurablePropertyResolver;
import org.springframework.core.env.MutablePropertySources;

/**
* {@link StandardReactiveWebEnvironment} for typical use in a typical
Expand All @@ -36,4 +39,9 @@ protected String doGetDefaultProfilesProperty() {
return null;
}

@Override
protected ConfigurablePropertyResolver createPropertyResolver(MutablePropertySources propertySources) {
return ConfigurationPropertySources.createPropertyResolver(propertySources);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package org.springframework.boot;

import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.core.env.ConfigurablePropertyResolver;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.web.context.support.StandardServletEnvironment;

/**
Expand All @@ -36,4 +39,9 @@ protected String doGetDefaultProfilesProperty() {
return null;
}

@Override
protected ConfigurablePropertyResolver createPropertyResolver(MutablePropertySources propertySources) {
return ConfigurationPropertySources.createPropertyResolver(propertySources);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2021 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.
Expand All @@ -20,8 +20,10 @@
import java.util.stream.Stream;

import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.ConfigurablePropertyResolver;
import org.springframework.core.env.Environment;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.PropertyResolver;
import org.springframework.core.env.PropertySource;
import org.springframework.core.env.PropertySource.StubPropertySource;
import org.springframework.core.env.PropertySources;
Expand All @@ -44,6 +46,19 @@ public final class ConfigurationPropertySources {
private ConfigurationPropertySources() {
}

/**
* Create a new {@link PropertyResolver} that resolves property values against an
* underlying set of {@link PropertySources}. Provides an
* {@link ConfigurationPropertySource} aware and optimized alternative to
* {@link PropertySourcesPropertyResolver}.
* @param propertySources the set of {@link PropertySource} objects to use
* @return a {@link ConfigurablePropertyResolver} implementation
* @since 2.5.0
*/
public static ConfigurablePropertyResolver createPropertyResolver(MutablePropertySources propertySources) {
return new ConfigurationPropertySourcesPropertyResolver(propertySources);
}

/**
* Determines if the specific {@link PropertySource} is the
* {@link ConfigurationPropertySource} that was {@link #attach(Environment) attached}
Expand Down Expand Up @@ -71,7 +86,7 @@ public static boolean isAttachedConfigurationPropertySource(PropertySource<?> pr
public static void attach(Environment environment) {
Assert.isInstanceOf(ConfigurableEnvironment.class, environment);
MutablePropertySources sources = ((ConfigurableEnvironment) environment).getPropertySources();
PropertySource<?> attached = sources.get(ATTACHED_PROPERTY_SOURCE_NAME);
PropertySource<?> attached = getAttached(sources);
if (attached != null && attached.getSource() != sources) {
sources.remove(ATTACHED_PROPERTY_SOURCE_NAME);
attached = null;
Expand All @@ -82,6 +97,10 @@ public static void attach(Environment environment) {
}
}

static PropertySource<?> getAttached(MutablePropertySources sources) {
return (sources != null) ? sources.get(ATTACHED_PROPERTY_SOURCE_NAME) : null;
}

/**
* Return a set of {@link ConfigurationPropertySource} instances that have previously
* been {@link #attach(Environment) attached} to the {@link Environment}.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* Copyright 2012-2021 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
*
* https://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.context.properties.source;

import org.springframework.core.env.AbstractPropertyResolver;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.PropertySources;
import org.springframework.core.env.PropertySourcesPropertyResolver;

/**
* Alternative {@link PropertySourcesPropertyResolver} implementation that recognizes
* {@link ConfigurationPropertySourcesPropertySource} and saves duplicate calls to the
* underlying sources if the name is a value {@link ConfigurationPropertyName}.
*
* @author Phillip Webb
*/
class ConfigurationPropertySourcesPropertyResolver extends AbstractPropertyResolver {

private final MutablePropertySources propertySources;

private final DefaultResolver defaultResolver;

ConfigurationPropertySourcesPropertyResolver(MutablePropertySources propertySources) {
this.propertySources = propertySources;
this.defaultResolver = new DefaultResolver(propertySources);
}

@Override
public boolean containsProperty(String key) {
ConfigurationPropertySourcesPropertySource attached = getAttached();
if (attached != null) {
ConfigurationPropertyName name = ConfigurationPropertyName.of(key, true);
if (name != null) {
try {
return attached.findConfigurationProperty(name) != null;
}
catch (Exception ex) {
}
}
}
return this.defaultResolver.containsProperty(key);
}

@Override
public String getProperty(String key) {
return getProperty(key, String.class, true);
}

@Override
public <T> T getProperty(String key, Class<T> targetValueType) {
return getProperty(key, targetValueType, true);
}

@Override
protected String getPropertyAsRawString(String key) {
return getProperty(key, String.class, false);
}

private <T> T getProperty(String key, Class<T> targetValueType, boolean resolveNestedPlaceholders) {
Object value = findPropertyValue(key, targetValueType);
if (value == null) {
return null;
}
if (resolveNestedPlaceholders && value instanceof String) {
value = resolveNestedPlaceholders((String) value);
}
return convertValueIfNecessary(value, targetValueType);
}

private Object findPropertyValue(String key, Class<?> targetValueType) {
ConfigurationPropertySourcesPropertySource attached = getAttached();
if (attached != null) {
ConfigurationPropertyName name = ConfigurationPropertyName.of(key, true);
if (name != null) {
try {
ConfigurationProperty configurationProperty = attached.findConfigurationProperty(name);
return (configurationProperty != null) ? configurationProperty.getValue() : null;
}
catch (Exception ex) {
}
}
}
return this.defaultResolver.getProperty(key, targetValueType, false);
}

private ConfigurationPropertySourcesPropertySource getAttached() {
ConfigurationPropertySourcesPropertySource attached = (ConfigurationPropertySourcesPropertySource) ConfigurationPropertySources
.getAttached(this.propertySources);
Iterable<ConfigurationPropertySource> attachedSource = (attached != null) ? attached.getSource() : null;
if ((attachedSource instanceof SpringConfigurationPropertySources)
&& ((SpringConfigurationPropertySources) attachedSource).isUsingSources(this.propertySources)) {
return attached;
}
return null;
}

/**
* Default {@link PropertySourcesPropertyResolver} used if
* {@link ConfigurationPropertySources} is not attached.
*/
static class DefaultResolver extends PropertySourcesPropertyResolver {

DefaultResolver(PropertySources propertySources) {
super(propertySources);
}

@Override
public <T> T getProperty(String key, Class<T> targetValueType, boolean resolveNestedPlaceholders) {
return super.getProperty(key, targetValueType, resolveNestedPlaceholders);
}

}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2021 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.
Expand Down Expand Up @@ -37,6 +37,11 @@ class ConfigurationPropertySourcesPropertySource extends PropertySource<Iterable
super(name, source);
}

@Override
public boolean containsProperty(String name) {
return findConfigurationProperty(name) != null;
}

@Override
public Object getProperty(String name) {
ConfigurationProperty configurationProperty = findConfigurationProperty(name);
Expand All @@ -57,7 +62,7 @@ private ConfigurationProperty findConfigurationProperty(String name) {
}
}

private ConfigurationProperty findConfigurationProperty(ConfigurationPropertyName name) {
ConfigurationProperty findConfigurationProperty(ConfigurationPropertyName name) {
if (name == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ class SpringConfigurationPropertySources implements Iterable<ConfigurationProper
this.sources = sources;
}

boolean isUsingSources(Iterable<PropertySource<?>> sources) {
return this.sources == sources;
}

@Override
public Iterator<ConfigurationPropertySource> iterator() {
return new SourcesIterator(this.sources.iterator(), this::adapt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import org.junit.jupiter.api.Test;

import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.core.env.AbstractEnvironment;
import org.springframework.core.env.ConfigurablePropertyResolver;
import org.springframework.core.env.Environment;
import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.StandardEnvironment;
import org.springframework.mock.env.MockPropertySource;

Expand Down Expand Up @@ -50,6 +53,14 @@ void getDefaultProfilesDoesNotResolveProperty() {
assertThat(environment.getDefaultProfiles()).containsExactly("default");
}

@Test
void propertyResolverIsOptimizedForConfigurationProperties() {
StandardEnvironment environment = createEnvironment();
ConfigurablePropertyResolver expected = ConfigurationPropertySources
.createPropertyResolver(new MutablePropertySources());
assertThat(environment).extracting("propertyResolver").hasSameClassAs(expected);
}

protected abstract StandardEnvironment createEnvironment();

}

0 comments on commit 6ad100e

Please sign in to comment.