root
/
spring-boot
mirror of https://github.com/spring-projects/spring-boot.git
1
0
Fork 0
Code Issues Actions 9 Packages Projects Releases Wiki Activity

Set using collection copies when possible 

Update `Map` and `Collection` binders to create a copy of the existing
collection whenever possible. Prior to this commit the binder would
always mutate the existing value and then call the setter with the
same instance. This could cause issues if the setter expected a
different instance.

Fixes gh-12322
This commit is contained in:
Phillip Webb 2018-03-18 12:46:13 -07:00
parent 6e2ecb8a43
commit c66781a3d3
4 changed files with 131 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/CollectionBinder.java
View File

@ -65,18 +65,27 @@ class CollectionBinder extends IndexedElementsBinder<Collection<Object>> {
try {
existingCollection.clear();
existingCollection.addAll(additional);
return existingCollection;
return copyIfPossible(existingCollection);
}
catch (UnsupportedOperationException ex) {
return createNewCollection(additional);
}
}
private Collection<Object> createNewCollection(Collection<Object> additional) {
Collection<Object> merged = CollectionFactory
.createCollection(additional.getClass(), additional.size());
merged.addAll(additional);
return merged;
private Collection<Object> copyIfPossible(Collection<Object> collection) {
try {
return createNewCollection(collection);
}
catch (Exception ex) {
return collection;
}
}
private Collection<Object> createNewCollection(Collection<Object> collection) {
Collection<Object> result = CollectionFactory
.createCollection(collection.getClass(), collection.size());
result.addAll(collection);
return result;
}
}

14
spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/MapBinder.java
View File

@ -91,7 +91,19 @@ class MapBinder extends AggregateBinder<Map<Object, Object>> {
return additional;
}
existingMap.putAll(additional);
return existingMap;
return copyIfPossible(existingMap);
}
private Map<Object, Object> copyIfPossible(Map<Object, Object> map) {
try {
Map<Object, Object> result = CollectionFactory.createMap(map.getClass(),
map.size());
result.putAll(map);
return result;
}
catch (Exception ex) {
return map;
}
}
private class EntryBinder {

51
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/CollectionBinderTests.java
View File

@ -172,7 +172,6 @@ public class CollectionBinderTests {
List<Integer> 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<String> {
public static class MyCustomNoDefaultConstructorList extends ArrayList<String> {
private List<String> items;
public MyCustomList(List<String> items) {
this.items = items;
public MyCustomNoDefaultConstructorList(List<String> items) {
addAll(items);
}
public List<String> 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<String> {
}
public static class BeanWithNestedCollection {

65
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/MapBinderTests.java
View File

@ -261,7 +261,6 @@ public class MapBinderTests {
.withExistingValue(existing);
Map<String, Integer> 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 <K, V> Bindable<Map<K, V>> getMapBindable(Class<K> 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<String, String> {
public MyCustomNoDefaultConstructorList(Map<String, String> 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<String, String> {
}
}