Skip to content

Commit c333946

Browse files
sbrannensbrannen
committed
Restore property binding support for a Map that implements Iterable
The changes in commit c20a2e4 introduced a regression with regard to binding to a Map property when the Map also happens to implement Iterable. Although that is perhaps not a very common scenario, this commit reorders the if-blocks in AbstractNestablePropertyAccessor's getPropertyValue(PropertyTokenHolder) method so that a Map is considered before an Iterable, thereby allowing an Iterable-Map to be accessed as a Map. See gh-907 Closes gh-34332 (cherry picked from commit b9e43d0)
1 parent b0a8a3e commit c333946

File tree

3 files changed

+67
-11
lines changed

3 files changed

+67
-11
lines changed

‎spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -658,6 +658,14 @@ else if (value instanceof List list) {
658658
growCollectionIfNecessary(list, index, indexedPropertyName.toString(), ph, i + 1);
659659
value = list.get(index);
660660
}
661+
else if (value instanceof Map map) {
662+
Class<?> mapKeyType = ph.getResolvableType().getNested(i + 1).asMap().resolveGeneric(0);
663+
// IMPORTANT: Do not pass full property name in here - property editors
664+
// must not kick in for map keys but rather only for map values.
665+
TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(mapKeyType);
666+
Object convertedMapKey = convertIfNecessary(null, null, key, mapKeyType, typeDescriptor);
667+
value = map.get(convertedMapKey);
668+
}
661669
else if (value instanceof Iterable iterable) {
662670
// Apply index to Iterator in case of a Set/Collection/Iterable.
663671
int index = Integer.parseInt(key);
@@ -685,14 +693,6 @@ else if (value instanceof Iterable iterable) {
685693
currIndex + ", accessed using property path '" + propertyName + "'");
686694
}
687695
}
688-
else if (value instanceof Map map) {
689-
Class<?> mapKeyType = ph.getResolvableType().getNested(i + 1).asMap().resolveGeneric(0);
690-
// IMPORTANT: Do not pass full property name in here - property editors
691-
// must not kick in for map keys but rather only for map values.
692-
TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(mapKeyType);
693-
Object convertedMapKey = convertIfNecessary(null, null, key, mapKeyType, typeDescriptor);
694-
value = map.get(convertedMapKey);
695-
}
696696
else {
697697
throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName,
698698
"Property referenced in indexed property path '" + propertyName +

‎spring-beans/src/test/java/org/springframework/beans/AbstractPropertyAccessorTests.java

+28-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -139,13 +139,15 @@ void isReadableWritableForIndexedProperties() {
139139
assertThat(accessor.isReadableProperty("list")).isTrue();
140140
assertThat(accessor.isReadableProperty("set")).isTrue();
141141
assertThat(accessor.isReadableProperty("map")).isTrue();
142+
assertThat(accessor.isReadableProperty("iterableMap")).isTrue();
142143
assertThat(accessor.isReadableProperty("myTestBeans")).isTrue();
143144
assertThat(accessor.isReadableProperty("xxx")).isFalse();
144145

145146
assertThat(accessor.isWritableProperty("array")).isTrue();
146147
assertThat(accessor.isWritableProperty("list")).isTrue();
147148
assertThat(accessor.isWritableProperty("set")).isTrue();
148149
assertThat(accessor.isWritableProperty("map")).isTrue();
150+
assertThat(accessor.isWritableProperty("iterableMap")).isTrue();
149151
assertThat(accessor.isWritableProperty("myTestBeans")).isTrue();
150152
assertThat(accessor.isWritableProperty("xxx")).isFalse();
151153

@@ -161,6 +163,14 @@ void isReadableWritableForIndexedProperties() {
161163
assertThat(accessor.isReadableProperty("map[key4][0].name")).isTrue();
162164
assertThat(accessor.isReadableProperty("map[key4][1]")).isTrue();
163165
assertThat(accessor.isReadableProperty("map[key4][1].name")).isTrue();
166+
assertThat(accessor.isReadableProperty("map[key999]")).isTrue();
167+
assertThat(accessor.isReadableProperty("iterableMap[key1]")).isTrue();
168+
assertThat(accessor.isReadableProperty("iterableMap[key1].name")).isTrue();
169+
assertThat(accessor.isReadableProperty("iterableMap[key2][0]")).isTrue();
170+
assertThat(accessor.isReadableProperty("iterableMap[key2][0].name")).isTrue();
171+
assertThat(accessor.isReadableProperty("iterableMap[key2][1]")).isTrue();
172+
assertThat(accessor.isReadableProperty("iterableMap[key2][1].name")).isTrue();
173+
assertThat(accessor.isReadableProperty("iterableMap[key999]")).isTrue();
164174
assertThat(accessor.isReadableProperty("myTestBeans[0]")).isTrue();
165175
assertThat(accessor.isReadableProperty("myTestBeans[1]")).isFalse();
166176
assertThat(accessor.isReadableProperty("array[key1]")).isFalse();
@@ -177,6 +187,14 @@ void isReadableWritableForIndexedProperties() {
177187
assertThat(accessor.isWritableProperty("map[key4][0].name")).isTrue();
178188
assertThat(accessor.isWritableProperty("map[key4][1]")).isTrue();
179189
assertThat(accessor.isWritableProperty("map[key4][1].name")).isTrue();
190+
assertThat(accessor.isWritableProperty("map[key999]")).isTrue();
191+
assertThat(accessor.isWritableProperty("iterableMap[key1]")).isTrue();
192+
assertThat(accessor.isWritableProperty("iterableMap[key1].name")).isTrue();
193+
assertThat(accessor.isWritableProperty("iterableMap[key2][0]")).isTrue();
194+
assertThat(accessor.isWritableProperty("iterableMap[key2][0].name")).isTrue();
195+
assertThat(accessor.isWritableProperty("iterableMap[key2][1]")).isTrue();
196+
assertThat(accessor.isWritableProperty("iterableMap[key2][1].name")).isTrue();
197+
assertThat(accessor.isWritableProperty("iterableMap[key999]")).isTrue();
180198
assertThat(accessor.isReadableProperty("myTestBeans[0]")).isTrue();
181199
assertThat(accessor.isReadableProperty("myTestBeans[1]")).isFalse();
182200
assertThat(accessor.isWritableProperty("array[key1]")).isFalse();
@@ -1394,6 +1412,9 @@ void getAndSetIndexedProperties() {
13941412
assertThat(accessor.getPropertyValue("map[key5[foo]].name")).isEqualTo("name8");
13951413
assertThat(accessor.getPropertyValue("map['key5[foo]'].name")).isEqualTo("name8");
13961414
assertThat(accessor.getPropertyValue("map[\"key5[foo]\"].name")).isEqualTo("name8");
1415+
assertThat(accessor.getPropertyValue("iterableMap[key1].name")).isEqualTo("nameC");
1416+
assertThat(accessor.getPropertyValue("iterableMap[key2][0].name")).isEqualTo("nameA");
1417+
assertThat(accessor.getPropertyValue("iterableMap[key2][1].name")).isEqualTo("nameB");
13971418
assertThat(accessor.getPropertyValue("myTestBeans[0].name")).isEqualTo("nameZ");
13981419

13991420
MutablePropertyValues pvs = new MutablePropertyValues();
@@ -1408,6 +1429,9 @@ void getAndSetIndexedProperties() {
14081429
pvs.add("map[key4][0].name", "nameA");
14091430
pvs.add("map[key4][1].name", "nameB");
14101431
pvs.add("map[key5[foo]].name", "name10");
1432+
pvs.add("iterableMap[key1].name", "newName1");
1433+
pvs.add("iterableMap[key2][0].name", "newName2A");
1434+
pvs.add("iterableMap[key2][1].name", "newName2B");
14111435
pvs.add("myTestBeans[0].name", "nameZZ");
14121436
accessor.setPropertyValues(pvs);
14131437
assertThat(tb0.getName()).isEqualTo("name5");
@@ -1427,6 +1451,9 @@ void getAndSetIndexedProperties() {
14271451
assertThat(accessor.getPropertyValue("map[key4][0].name")).isEqualTo("nameA");
14281452
assertThat(accessor.getPropertyValue("map[key4][1].name")).isEqualTo("nameB");
14291453
assertThat(accessor.getPropertyValue("map[key5[foo]].name")).isEqualTo("name10");
1454+
assertThat(accessor.getPropertyValue("iterableMap[key1].name")).isEqualTo("newName1");
1455+
assertThat(accessor.getPropertyValue("iterableMap[key2][0].name")).isEqualTo("newName2A");
1456+
assertThat(accessor.getPropertyValue("iterableMap[key2][1].name")).isEqualTo("newName2B");
14301457
assertThat(accessor.getPropertyValue("myTestBeans[0].name")).isEqualTo("nameZZ");
14311458
}
14321459

‎spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/IndexedTestBean.java

+30-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@
2121
import java.util.Collection;
2222
import java.util.HashMap;
2323
import java.util.Iterator;
24+
import java.util.LinkedHashMap;
2425
import java.util.List;
2526
import java.util.Map;
2627
import java.util.Set;
@@ -49,6 +50,8 @@ public class IndexedTestBean {
4950

5051
private SortedMap sortedMap;
5152

53+
private IterableMap iterableMap;
54+
5255
private MyTestBeans myTestBeans;
5356

5457

@@ -73,6 +76,9 @@ public void populate() {
7376
TestBean tb6 = new TestBean("name6", 0);
7477
TestBean tb7 = new TestBean("name7", 0);
7578
TestBean tb8 = new TestBean("name8", 0);
79+
TestBean tbA = new TestBean("nameA", 0);
80+
TestBean tbB = new TestBean("nameB", 0);
81+
TestBean tbC = new TestBean("nameC", 0);
7682
TestBean tbX = new TestBean("nameX", 0);
7783
TestBean tbY = new TestBean("nameY", 0);
7884
TestBean tbZ = new TestBean("nameZ", 0);
@@ -88,6 +94,12 @@ public void populate() {
8894
this.map.put("key2", tb5);
8995
this.map.put("key.3", tb5);
9096
List list = new ArrayList();
97+
list.add(tbA);
98+
list.add(tbB);
99+
this.iterableMap = new IterableMap<>();
100+
this.iterableMap.put("key1", tbC);
101+
this.iterableMap.put("key2", list);
102+
list = new ArrayList();
91103
list.add(tbX);
92104
list.add(tbY);
93105
this.map.put("key4", list);
@@ -152,6 +164,14 @@ public void setSortedMap(SortedMap sortedMap) {
152164
this.sortedMap = sortedMap;
153165
}
154166

167+
public IterableMap getIterableMap() {
168+
return this.iterableMap;
169+
}
170+
171+
public void setIterableMap(IterableMap iterableMap) {
172+
this.iterableMap = iterableMap;
173+
}
174+
155175
public MyTestBeans getMyTestBeans() {
156176
return myTestBeans;
157177
}
@@ -161,6 +181,15 @@ public void setMyTestBeans(MyTestBeans myTestBeans) {
161181
}
162182

163183

184+
@SuppressWarnings("serial")
185+
public static class IterableMap<K,V> extends LinkedHashMap<K,V> implements Iterable<V> {
186+
187+
@Override
188+
public Iterator<V> iterator() {
189+
return values().iterator();
190+
}
191+
}
192+
164193
public static class MyTestBeans implements Iterable<TestBean> {
165194

166195
private final Collection<TestBean> testBeans;

0 commit comments

Comments
 (0)