From 8b1625b41d2f87911ac7b3b592b495c5a7e26718 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 1 May 2017 22:07:57 -0700 Subject: [PATCH] Short circuit binding if possible Update the `Binder` to short circuit potentially expensive bean binding if there are no known child properties. The shortcut can only be applied when all used `ConfigurationPropertySources` return a non empty `containsDescendantOf` result. This should be the case for most Spring Boot applications, the exception being any apps that are running in a security restricted environment. Fixes gh-9023 --- .../context/properties/bind/BeanBinder.java | 12 ++--- .../context/properties/bind/BindContext.java | 13 +++++- .../boot/context/properties/bind/Binder.java | 45 ++++++++++--------- .../properties/bind/JavaBeanBinder.java | 8 +++- 4 files changed, 48 insertions(+), 30 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BeanBinder.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BeanBinder.java index 3f06b8ba11..e8863cabdc 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BeanBinder.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BeanBinder.java @@ -16,6 +16,7 @@ package org.springframework.boot.context.properties.bind; +import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertySource; /** @@ -29,15 +30,16 @@ interface BeanBinder { /** * Return a bound bean instance or {@code null} if the {@link BeanBinder} does not * support the specified {@link Bindable}. - * @param target the bindable to bind - * @param hasKnownBindableProperties if this binder has known bindable elements. If - * names from underlying {@link ConfigurationPropertySource} cannot be iterated this - * method can be {@code false}, even though binding may ultimately succeed. + * @param name the name being bound + * @param target the bindable to bind names from underlying + * {@link ConfigurationPropertySource} cannot be iterated this method can be + * {@code false}, even though binding may ultimately succeed. + * @param context the bind context * @param propertyBinder property binder * @param The source type * @return a bound instance or {@code null} */ - T bind(Bindable target, boolean hasKnownBindableProperties, + T bind(ConfigurationPropertyName name, Bindable target, BindContext context, BeanPropertyBinder propertyBinder); } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindContext.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindContext.java index b4f0001c3c..30ecd61c0f 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindContext.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindContext.java @@ -16,6 +16,8 @@ package org.springframework.boot.context.properties.bind; +import java.util.stream.Stream; + import org.springframework.boot.context.properties.bind.convert.BinderConversionService; import org.springframework.boot.context.properties.source.ConfigurationProperty; import org.springframework.boot.context.properties.source.ConfigurationPropertySource; @@ -38,12 +40,19 @@ public interface BindContext { int getDepth(); /** - * Return the {@link ConfigurationPropertySource sources} being used by the - * {@link Binder}. + * Return an {@link Iterable} of the {@link ConfigurationPropertySource sources} being + * used by the {@link Binder}. * @return the sources */ Iterable getSources(); + /** + * Return a {@link Stream} of the {@link ConfigurationPropertySource sources} being + * used by the {@link Binder}. + * @return the sources + */ + Stream streamSources(); + /** * Return the {@link ConfigurationProperty} actually being bound or {@code null} if * the property has not yet been determined. diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java index 8970710770..f8c11a9bc7 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java @@ -36,7 +36,6 @@ import org.springframework.boot.context.properties.source.ConfigurationProperty; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertySource; import org.springframework.boot.context.properties.source.ConfigurationPropertySources; -import org.springframework.boot.context.properties.source.IterableConfigurationPropertySource; import org.springframework.core.convert.ConversionService; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; @@ -289,8 +288,8 @@ public class Binder { private Object bindBean(ConfigurationPropertyName name, Bindable target, BindHandler handler, Context context) { - boolean hasKnownBindableProperties = hasKnownBindableProperties(name, context); - if (!hasKnownBindableProperties && isUnbindableBean(target)) { + if (containsNoDescendantOf(context.streamSources(), name) + || isUnbindableBean(name, target, context)) { return null; } BeanPropertyBinder propertyBinder = (propertyName, propertyTarget) -> bind( @@ -300,22 +299,19 @@ public class Binder { return null; } return context.withBean(type, () -> { - Stream boundBeans = BEAN_BINDERS.stream().map( - (b) -> b.bind(target, hasKnownBindableProperties, propertyBinder)); + Stream boundBeans = BEAN_BINDERS.stream() + .map((b) -> b.bind(name, target, context, propertyBinder)); return boundBeans.filter(Objects::nonNull).findFirst().orElse(null); }); } - private boolean hasKnownBindableProperties(ConfigurationPropertyName name, + private boolean isUnbindableBean(ConfigurationPropertyName name, Bindable target, Context context) { - Stream sources = context.streamSources() - .filter(IterableConfigurationPropertySource.class::isInstance) - .map(IterableConfigurationPropertySource.class::cast); - return sources.flatMap((s) -> s.filter(name::isAncestorOf).stream()).findAny() - .isPresent(); - } - - private boolean isUnbindableBean(Bindable target) { + if (context.streamSources().map((s) -> s.containsDescendantOf(name).orElse(false)) + .anyMatch(Boolean.TRUE::equals)) { + // We know there are properties to bind so we can't bypass anything + return false; + } Class resolved = target.getType().resolve(); if (resolved.isPrimitive() || NON_BEAN_CLASSES.contains(resolved)) { return true; @@ -324,6 +320,12 @@ public class Binder { return packageName.startsWith("java."); } + private boolean containsNoDescendantOf(Stream sources, + ConfigurationPropertyName name) { + return sources.map((s) -> s.containsDescendantOf(name).orElse(true)) + .allMatch(Boolean.FALSE::equals); + } + /** * Create a new {@link Binder} instance from the specified environment. * @param environment the environment (must be a {@link ConfigurableEnvironment}) @@ -400,13 +402,6 @@ public class Binder { } } - private Stream streamSources() { - if (this.sourcePushCount > 0) { - return this.source.stream(); - } - return StreamSupport.stream(Binder.this.sources.spliterator(), false); - } - @Override public Iterable getSources() { if (this.sourcePushCount > 0) { @@ -415,6 +410,14 @@ public class Binder { return Binder.this.sources; } + @Override + public Stream streamSources() { + if (this.sourcePushCount > 0) { + return this.source.stream(); + } + return StreamSupport.stream(Binder.this.sources.spliterator(), false); + } + public boolean hasBoundBean(Class bean) { return this.beans.contains(bean); } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java index ddd07fe373..cebf897949 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.function.Supplier; import org.springframework.beans.BeanUtils; +import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.core.ResolvableType; /** @@ -37,8 +38,11 @@ import org.springframework.core.ResolvableType; class JavaBeanBinder implements BeanBinder { @Override - public T bind(Bindable target, boolean hasKnownBindableProperties, - BeanPropertyBinder propertyBinder) { + public T bind(ConfigurationPropertyName name, Bindable target, + BindContext context, BeanPropertyBinder propertyBinder) { + boolean hasKnownBindableProperties = context.streamSources() + .map((s) -> s.containsDescendantOf(name).orElse(false)) + .anyMatch(Boolean.TRUE::equals); Bean bean = Bean.get(target, hasKnownBindableProperties); if (bean == null) { return null;