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 ac953607dd..16f8e71379 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 @@ -65,18 +65,27 @@ class CollectionBinder extends IndexedElementsBinder> { try { existingCollection.clear(); existingCollection.addAll(additional); - return existingCollection; + return copyIfPossible(existingCollection); } catch (UnsupportedOperationException ex) { return createNewCollection(additional); } } - private Collection createNewCollection(Collection additional) { - Collection merged = CollectionFactory - .createCollection(additional.getClass(), additional.size()); - merged.addAll(additional); - return merged; + private Collection copyIfPossible(Collection collection) { + try { + return createNewCollection(collection); + } + catch (Exception ex) { + return collection; + } + } + + private Collection createNewCollection(Collection collection) { + Collection result = CollectionFactory + .createCollection(collection.getClass(), collection.size()); + result.addAll(collection); + return result; } } 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 1d02a1020f..5ad55455b4 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 @@ -91,7 +91,19 @@ class MapBinder extends AggregateBinder> { return additional; } existingMap.putAll(additional); - return existingMap; + return copyIfPossible(existingMap); + } + + private Map copyIfPossible(Map map) { + try { + Map result = CollectionFactory.createMap(map.getClass(), + map.size()); + result.putAll(map); + return result; + } + catch (Exception ex) { + return map; + } } private class EntryBinder { 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 06fe47f936..3ceeff8dd7 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 @@ -172,7 +172,6 @@ public class CollectionBinderTests { List result = this.binder .bind("foo", INTEGER_LIST.withExistingValue(existing)).get(); assertThat(result).isExactlyInstanceOf(LinkedList.class); - assertThat(result).isSameAs(existing); assertThat(result).containsExactly(1); } @@ -309,7 +308,20 @@ public class CollectionBinderTests { MockConfigurationPropertySource source = new MockConfigurationPropertySource(); source.put("foo.items", "a,b,c,c"); this.sources.add(source); - ExampleCustomBean result = this.binder.bind("foo", ExampleCustomBean.class).get(); + ExampleCustomNoDefaultConstructorBean result = this.binder + .bind("foo", ExampleCustomNoDefaultConstructorBean.class).get(); + assertThat(result.getItems()).hasSize(4); + assertThat(result.getItems()).containsExactly("a", "b", "c", "c"); + } + + @Test + public void bindToCollectionWithDefaultConstructor() { + // gh-12322 + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.items", "a,b,c,c"); + this.sources.add(source); + ExampleCustomWithDefaultConstructorBean result = this.binder + .bind("foo", ExampleCustomWithDefaultConstructorBean.class).get(); assertThat(result.getItems()).hasSize(4); assertThat(result.getItems()).containsExactly("a", "b", "c", "c"); } @@ -415,31 +427,46 @@ public class CollectionBinderTests { } } - public static class ExampleCustomBean { + public static class ExampleCustomNoDefaultConstructorBean { - private MyCustomList items = new MyCustomList(Collections.singletonList("foo")); + private MyCustomNoDefaultConstructorList items = new MyCustomNoDefaultConstructorList( + Collections.singletonList("foo")); - public MyCustomList getItems() { + public MyCustomNoDefaultConstructorList getItems() { return this.items; } - public void setItems(MyCustomList items) { + public void setItems(MyCustomNoDefaultConstructorList items) { this.items = items; } + } - public static class MyCustomList extends ArrayList { + public static class MyCustomNoDefaultConstructorList extends ArrayList { - private List items; - - public MyCustomList(List items) { - this.items = items; + public MyCustomNoDefaultConstructorList(List items) { + addAll(items); } - public List getItems() { + } + + public static class ExampleCustomWithDefaultConstructorBean { + + private MyCustomWithDefaultConstructorList items = new MyCustomWithDefaultConstructorList(); + + public MyCustomWithDefaultConstructorList getItems() { return this.items; } + public void setItems(MyCustomWithDefaultConstructorList items) { + this.items.clear(); + this.items.addAll(items); + } + + } + + public static class MyCustomWithDefaultConstructorList extends ArrayList { + } public static class BeanWithNestedCollection { 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 f56fa71667..ea4553e9c3 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 @@ -261,7 +261,6 @@ public class MapBinderTests { .withExistingValue(existing); Map result = this.binder.bind("foo", target).get(); assertThat(result).isExactlyInstanceOf(HashMap.class); - assertThat(result).isSameAs(existing); assertThat(result).hasSize(2); assertThat(result).containsEntry("bar", 1); assertThat(result).containsEntry("baz", 1001); @@ -595,6 +594,27 @@ public class MapBinderTests { assertThat(map).containsExactly(entry("bar", RuntimeException.class)); } + @Test + public void bindToMapWithNoDefaultConstructor() { + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.items.a", "b"); + this.sources.add(source); + ExampleCustomNoDefaultConstructorBean result = this.binder + .bind("foo", ExampleCustomNoDefaultConstructorBean.class).get(); + assertThat(result.getItems()).containsOnly(entry("foo", "bar"), entry("a", "b")); + } + + @Test + public void bindToMapWithDefaultConstructor() { + // gh-12322 + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.items.a", "b"); + this.sources.add(source); + ExampleCustomWithDefaultConstructorBean result = this.binder + .bind("foo", ExampleCustomWithDefaultConstructorBean.class).get(); + assertThat(result.getItems()).containsExactly(entry("a", "b")); + } + private Bindable> getMapBindable(Class keyGeneric, ResolvableType valueType) { ResolvableType keyType = ResolvableType.forClass(keyGeneric); @@ -657,4 +677,47 @@ public class MapBinderTests { } + public static class ExampleCustomNoDefaultConstructorBean { + + private MyCustomNoDefaultConstructorList items = new MyCustomNoDefaultConstructorList( + Collections.singletonMap("foo", "bar")); + + public MyCustomNoDefaultConstructorList getItems() { + return this.items; + } + + public void setItems(MyCustomNoDefaultConstructorList items) { + this.items = items; + } + + } + + public static class MyCustomNoDefaultConstructorList extends HashMap { + + public MyCustomNoDefaultConstructorList(Map items) { + putAll(items); + } + + } + + public static class ExampleCustomWithDefaultConstructorBean { + + private MyCustomWithDefaultConstructorList items = new MyCustomWithDefaultConstructorList(); + + public MyCustomWithDefaultConstructorList getItems() { + return this.items; + } + + public void setItems(MyCustomWithDefaultConstructorList items) { + this.items.clear(); + this.items.putAll(items); + } + + } + + public static class MyCustomWithDefaultConstructorList + extends HashMap { + + } + }