From 8ce13c765beb8865311f98192473cc2231d8b79b Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Fri, 1 Jun 2018 14:34:16 -0700 Subject: [PATCH 1/2] Support binding to immutable maps Closes gh-13323 --- .../context/properties/bind/MapBinder.java | 22 ++++++++++++++----- .../properties/bind/MapBinderTests.java | 16 ++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java index 7efdf055b4..7cb92a141d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java @@ -90,22 +90,32 @@ class MapBinder extends AggregateBinder> { if (existingMap == null) { return additional; } - existingMap.putAll(additional); - return copyIfPossible(existingMap); + try { + existingMap.putAll(additional); + return copyIfPossible(existingMap); + } + catch (UnsupportedOperationException ex) { + Map result = createNewMap(additional.getClass(), existingMap); + result.putAll(additional); + return result; + } } private Map copyIfPossible(Map map) { try { - Map result = CollectionFactory.createMap(map.getClass(), - map.size()); - result.putAll(map); - return result; + return createNewMap(map.getClass(), map); } catch (Exception ex) { return map; } } + private Map createNewMap(Class mapClass, Map map) { + Map result = CollectionFactory.createMap(mapClass, map.size()); + result.putAll(map); + return result; + } + private class EntryBinder { private final ConfigurationPropertyName root; 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 5920a31b3d..e75493c024 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 @@ -615,6 +615,22 @@ public class MapBinderTests { assertThat(result.getItems()).containsExactly(entry("a", "b")); } + @Test + public void bindToImmutableMapShouldReturnPopulatedCollection() { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.values.c", "d"); + source.put("foo.values.e", "f"); + this.sources.add(source); + Map result = this.binder + .bind("foo.values", + STRING_STRING_MAP + .withExistingValue(Collections.singletonMap("a", "b"))) + .get(); + assertThat(result).hasSize(3); + assertThat(result.entrySet()).containsExactly(entry("a", "b"), entry("c", "d"), + entry("e", "f")); + } + private Bindable> getMapBindable(Class keyGeneric, ResolvableType valueType) { ResolvableType keyType = ResolvableType.forClass(keyGeneric); From 95174a077386222a311ee225b044815e2f81a8d0 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Fri, 1 Jun 2018 14:45:36 -0700 Subject: [PATCH 2/2] Don't fail if aggregate merge can't get existing value Fixes gh-13303 --- .../properties/bind/CollectionBinder.java | 13 ++++++++-- .../context/properties/bind/MapBinder.java | 13 ++++++++-- .../bind/CollectionBinderTests.java | 24 +++++++++++++++++++ .../properties/bind/MapBinderTests.java | 24 +++++++++++++++++++ 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java index ad735cf314..8c2d283e60 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java @@ -55,10 +55,9 @@ class CollectionBinder extends IndexedElementsBinder> { } @Override - @SuppressWarnings("unchecked") protected Collection merge(Supplier existing, Collection additional) { - Collection existingCollection = (Collection) existing.get(); + Collection existingCollection = getExistingIfPossible(existing); if (existingCollection == null) { return additional; } @@ -72,6 +71,16 @@ class CollectionBinder extends IndexedElementsBinder> { } } + @SuppressWarnings("unchecked") + private Collection getExistingIfPossible(Supplier existing) { + try { + return (Collection) existing.get(); + } + catch (Exception ex) { + return null; + } + } + private Collection copyIfPossible(Collection collection) { try { return createNewCollection(collection); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java index 7cb92a141d..724ad529fe 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java @@ -83,10 +83,9 @@ class MapBinder extends AggregateBinder> { } @Override - @SuppressWarnings("unchecked") protected Map merge(Supplier existing, Map additional) { - Map existingMap = (Map) existing.get(); + Map existingMap = getExistingIfPossible(existing); if (existingMap == null) { return additional; } @@ -101,6 +100,16 @@ class MapBinder extends AggregateBinder> { } } + @SuppressWarnings("unchecked") + private Map getExistingIfPossible(Supplier existing) { + try { + return (Map) existing.get(); + } + catch (Exception ex) { + return null; + } + } + private Map copyIfPossible(Map map) { try { return createNewMap(map.getClass(), map); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java index 25dade2c2b..cd057173ef 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java @@ -417,6 +417,16 @@ public class CollectionBinderTests { assertThat(bean.getBar()).containsExactly("hello"); } + @Test + public void bindToBeanWithExceptionInGetterForExistingValue() { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.values", "a,b,c"); + this.sources.add(source); + BeanWithGetterException result = this.binder + .bind("foo", Bindable.of(BeanWithGetterException.class)).get(); + assertThat(result.getValues()).containsExactly("a", "b", "c"); + } + public static class ExampleCollectionBean { private List items = new ArrayList<>(); @@ -521,4 +531,18 @@ public class CollectionBinderTests { } + public static class BeanWithGetterException { + + private List values; + + public void setValues(List values) { + this.values = values; + } + + public List getValues() { + return Collections.unmodifiableList(this.values); + } + + } + } 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 e75493c024..6e9607777f 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 @@ -631,6 +631,16 @@ public class MapBinderTests { entry("e", "f")); } + @Test + public void bindToBeanWithExceptionInGetterForExistingValue() { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.values.a", "b"); + this.sources.add(source); + BeanWithGetterException result = this.binder + .bind("foo", Bindable.of(BeanWithGetterException.class)).get(); + assertThat(result.getValues()).containsExactly(entry("a", "b")); + } + private Bindable> getMapBindable(Class keyGeneric, ResolvableType valueType) { ResolvableType keyType = ResolvableType.forClass(keyGeneric); @@ -736,4 +746,18 @@ public class MapBinderTests { } + public static class BeanWithGetterException { + + private Map values; + + public void setValues(Map values) { + this.values = values; + } + + public Map getValues() { + return Collections.unmodifiableMap(this.values); + } + + } + }