From f3373238197ba1f9ec5c213c276596df84b95b0d Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 10 Oct 2017 21:41:53 -0700 Subject: [PATCH] Allow recursive binding in Maps Update `Binder` so that Maps containing references to themselves may be bound. The existing stack-overflow protection (required when binding a bean to a non enumerable source) now only applies to bean properties. Fixes gh-9801 --- .../boot/context/properties/bind/Binder.java | 21 +++++----- .../context/properties/bind/BinderTests.java | 38 +++++++++++++++++++ .../properties/bind/MapBinderTests.java | 34 +++++++++++++++++ 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java index 5e132737180..6cdf28479cb 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java @@ -183,18 +183,18 @@ public class Binder { Assert.notNull(target, "Target must not be null"); handler = (handler != null ? handler : BindHandler.DEFAULT); Context context = new Context(); - T bound = bind(name, target, handler, context); + T bound = bind(name, target, handler, context, false); return BindResult.of(bound); } protected final T bind(ConfigurationPropertyName name, Bindable target, - BindHandler handler, Context context) { + BindHandler handler, Context context, boolean skipIfHasBoundBean) { context.clearConfigurationProperty(); try { if (!handler.onStart(name, target, context)) { return null; } - Object bound = bindObject(name, target, handler, context); + Object bound = bindObject(name, target, handler, context, skipIfHasBoundBean); return handleBindResult(name, target, handler, context, bound); } catch (Exception ex) { @@ -234,7 +234,8 @@ public class Binder { } private Object bindObject(ConfigurationPropertyName name, Bindable target, - BindHandler handler, Context context) throws Exception { + BindHandler handler, Context context, boolean skipIfHasBoundBean) + throws Exception { ConfigurationProperty property = findProperty(name, context); if (property == null && containsNoDescendantOf(context.streamSources(), name)) { return null; @@ -246,7 +247,7 @@ public class Binder { if (property != null) { return bindProperty(name, target, handler, context, property); } - return bindBean(name, target, handler, context); + return bindBean(name, target, handler, context, skipIfHasBoundBean); } private AggregateBinder getAggregateBinder(Bindable target, Context context) { @@ -266,7 +267,8 @@ public class Binder { private Object bindAggregate(ConfigurationPropertyName name, Bindable target, BindHandler handler, Context context, AggregateBinder aggregateBinder) { AggregateElementBinder elementBinder = (itemName, itemTarget, source) -> { - Supplier supplier = () -> bind(itemName, itemTarget, handler, context); + Supplier supplier = () -> bind(itemName, itemTarget, handler, context, + false); return context.withSource(source, supplier); }; return context.withIncreasedDepth( @@ -290,15 +292,16 @@ public class Binder { } private Object bindBean(ConfigurationPropertyName name, Bindable target, - BindHandler handler, Context context) { + BindHandler handler, Context context, boolean skipIfHasBoundBean) { if (containsNoDescendantOf(context.streamSources(), name) || isUnbindableBean(name, target, context)) { return null; } BeanPropertyBinder propertyBinder = (propertyName, propertyTarget) -> bind( - name.append(propertyName), propertyTarget, handler, context); + name.append(propertyName), propertyTarget, handler, context, true); Class type = target.getType().resolve(); - if (context.hasBoundBean(type)) { + if (skipIfHasBoundBean && context.hasBoundBean(type)) { + System.err.println(type + " " + name); return null; } return context.withBean(type, () -> { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java index 41fe5dabad7..fb21fc4e1fb 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BinderTests.java @@ -251,10 +251,12 @@ public class BinderTests { @Test public void bindToValidatedBeanWithResourceAndNonEnumerablePropertySource() { ConfigurationPropertySources.from(new PropertySource("test") { + @Override public Object getProperty(String name) { return null; } + }).forEach(this.sources::add); Validator validator = new SpringValidatorAdapter(Validation.byDefaultProvider() .configure().buildValidatorFactory().getValidator()); @@ -262,6 +264,14 @@ public class BinderTests { new ValidationBindHandler(validator)); } + @Test + public void bindToBeanWithCycle() throws Exception { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + this.sources.add(source.nonIterable()); + Bindable target = Bindable.of(CycleBean1.class); + this.binder.bind("foo", target); + } + public static class JavaBean { private String value; @@ -303,4 +313,32 @@ public class BinderTests { } + public static class CycleBean1 { + + private CycleBean2 two; + + public CycleBean2 getTwo() { + return this.two; + } + + public void setTwo(CycleBean2 two) { + this.two = two; + } + + } + + public static class CycleBean2 { + + private CycleBean1 one; + + public CycleBean1 getOne() { + return this.one; + } + + public void setOne(CycleBean1 one) { + this.one = one; + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/MapBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/MapBinderTests.java index c962c042e10..6944c54100b 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/MapBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/MapBinderTests.java @@ -19,6 +19,7 @@ package org.springframework.boot.context.properties.bind; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Properties; @@ -515,6 +516,19 @@ public class MapBinderTests { assertThat(map).containsEntry("x [B] y", "[ball]"); } + @Test + public void nestedMapsShouldNotBindToNull() throws Exception { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.value", "one"); + source.put("foo.foos.foo1.value", "two"); + source.put("foo.foos.foo2.value", "three"); + this.sources.add(source); + BindResult foo = this.binder.bind("foo", NestableFoo.class); + assertThat(foo.get().getValue()).isNotNull(); + assertThat(foo.get().getFoos().get("foo1").getValue()).isEqualTo("two"); + assertThat(foo.get().getFoos().get("foo2").getValue()).isEqualTo("three"); + } + private Bindable> getMapBindable(Class keyGeneric, ResolvableType valueType) { ResolvableType keyType = ResolvableType.forClass(keyGeneric); @@ -547,4 +561,24 @@ public class MapBinderTests { } + static class NestableFoo { + + private Map foos = new LinkedHashMap<>(); + + private String value; + + public Map getFoos() { + return this.foos; + } + + public String getValue() { + return this.value; + } + + public void setValue(String value) { + this.value = value; + } + + } + }