Restore property binding support for a Map that implements Iterable

The changes in commit c20a2e4763 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
This commit is contained in:
Sam Brannen 2025-01-29 17:33:46 +01:00
parent d80de043ce
commit b9e43d05bd
3 changed files with 66 additions and 11 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2024 the original author or authors. * Copyright 2002-2025 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -658,6 +658,14 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA
growCollectionIfNecessary(list, index, indexedPropertyName.toString(), ph, i + 1); growCollectionIfNecessary(list, index, indexedPropertyName.toString(), ph, i + 1);
value = list.get(index); value = list.get(index);
} }
else if (value instanceof Map map) {
Class<?> mapKeyType = ph.getResolvableType().getNested(i + 1).asMap().resolveGeneric(0);
// IMPORTANT: Do not pass full property name in here - property editors
// must not kick in for map keys but rather only for map values.
TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(mapKeyType);
Object convertedMapKey = convertIfNecessary(null, null, key, mapKeyType, typeDescriptor);
value = map.get(convertedMapKey);
}
else if (value instanceof Iterable iterable) { else if (value instanceof Iterable iterable) {
// Apply index to Iterator in case of a Set/Collection/Iterable. // Apply index to Iterator in case of a Set/Collection/Iterable.
int index = Integer.parseInt(key); int index = Integer.parseInt(key);
@ -685,14 +693,6 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA
currIndex + ", accessed using property path '" + propertyName + "'"); currIndex + ", accessed using property path '" + propertyName + "'");
} }
} }
else if (value instanceof Map map) {
Class<?> mapKeyType = ph.getResolvableType().getNested(i + 1).asMap().resolveGeneric(0);
// IMPORTANT: Do not pass full property name in here - property editors
// must not kick in for map keys but rather only for map values.
TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(mapKeyType);
Object convertedMapKey = convertIfNecessary(null, null, key, mapKeyType, typeDescriptor);
value = map.get(convertedMapKey);
}
else { else {
throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName, throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName,
"Property referenced in indexed property path '" + propertyName + "Property referenced in indexed property path '" + propertyName +

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2024 the original author or authors. * Copyright 2002-2025 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -139,6 +139,7 @@ abstract class AbstractPropertyAccessorTests {
assertThat(accessor.isReadableProperty("list")).isTrue(); assertThat(accessor.isReadableProperty("list")).isTrue();
assertThat(accessor.isReadableProperty("set")).isTrue(); assertThat(accessor.isReadableProperty("set")).isTrue();
assertThat(accessor.isReadableProperty("map")).isTrue(); assertThat(accessor.isReadableProperty("map")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap")).isTrue();
assertThat(accessor.isReadableProperty("myTestBeans")).isTrue(); assertThat(accessor.isReadableProperty("myTestBeans")).isTrue();
assertThat(accessor.isReadableProperty("xxx")).isFalse(); assertThat(accessor.isReadableProperty("xxx")).isFalse();
@ -146,6 +147,7 @@ abstract class AbstractPropertyAccessorTests {
assertThat(accessor.isWritableProperty("list")).isTrue(); assertThat(accessor.isWritableProperty("list")).isTrue();
assertThat(accessor.isWritableProperty("set")).isTrue(); assertThat(accessor.isWritableProperty("set")).isTrue();
assertThat(accessor.isWritableProperty("map")).isTrue(); assertThat(accessor.isWritableProperty("map")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap")).isTrue();
assertThat(accessor.isWritableProperty("myTestBeans")).isTrue(); assertThat(accessor.isWritableProperty("myTestBeans")).isTrue();
assertThat(accessor.isWritableProperty("xxx")).isFalse(); assertThat(accessor.isWritableProperty("xxx")).isFalse();
@ -161,6 +163,14 @@ abstract class AbstractPropertyAccessorTests {
assertThat(accessor.isReadableProperty("map[key4][0].name")).isTrue(); assertThat(accessor.isReadableProperty("map[key4][0].name")).isTrue();
assertThat(accessor.isReadableProperty("map[key4][1]")).isTrue(); assertThat(accessor.isReadableProperty("map[key4][1]")).isTrue();
assertThat(accessor.isReadableProperty("map[key4][1].name")).isTrue(); assertThat(accessor.isReadableProperty("map[key4][1].name")).isTrue();
assertThat(accessor.isReadableProperty("map[key999]")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key1]")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key1].name")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key2][0]")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key2][0].name")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key2][1]")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key2][1].name")).isTrue();
assertThat(accessor.isReadableProperty("iterableMap[key999]")).isTrue();
assertThat(accessor.isReadableProperty("myTestBeans[0]")).isTrue(); assertThat(accessor.isReadableProperty("myTestBeans[0]")).isTrue();
assertThat(accessor.isReadableProperty("myTestBeans[1]")).isFalse(); assertThat(accessor.isReadableProperty("myTestBeans[1]")).isFalse();
assertThat(accessor.isReadableProperty("array[key1]")).isFalse(); assertThat(accessor.isReadableProperty("array[key1]")).isFalse();
@ -177,6 +187,14 @@ abstract class AbstractPropertyAccessorTests {
assertThat(accessor.isWritableProperty("map[key4][0].name")).isTrue(); assertThat(accessor.isWritableProperty("map[key4][0].name")).isTrue();
assertThat(accessor.isWritableProperty("map[key4][1]")).isTrue(); assertThat(accessor.isWritableProperty("map[key4][1]")).isTrue();
assertThat(accessor.isWritableProperty("map[key4][1].name")).isTrue(); assertThat(accessor.isWritableProperty("map[key4][1].name")).isTrue();
assertThat(accessor.isWritableProperty("map[key999]")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key1]")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key1].name")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key2][0]")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key2][0].name")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key2][1]")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key2][1].name")).isTrue();
assertThat(accessor.isWritableProperty("iterableMap[key999]")).isTrue();
assertThat(accessor.isReadableProperty("myTestBeans[0]")).isTrue(); assertThat(accessor.isReadableProperty("myTestBeans[0]")).isTrue();
assertThat(accessor.isReadableProperty("myTestBeans[1]")).isFalse(); assertThat(accessor.isReadableProperty("myTestBeans[1]")).isFalse();
assertThat(accessor.isWritableProperty("array[key1]")).isFalse(); assertThat(accessor.isWritableProperty("array[key1]")).isFalse();
@ -1395,6 +1413,9 @@ abstract class AbstractPropertyAccessorTests {
assertThat(accessor.getPropertyValue("map[key5[foo]].name")).isEqualTo("name8"); assertThat(accessor.getPropertyValue("map[key5[foo]].name")).isEqualTo("name8");
assertThat(accessor.getPropertyValue("map['key5[foo]'].name")).isEqualTo("name8"); assertThat(accessor.getPropertyValue("map['key5[foo]'].name")).isEqualTo("name8");
assertThat(accessor.getPropertyValue("map[\"key5[foo]\"].name")).isEqualTo("name8"); assertThat(accessor.getPropertyValue("map[\"key5[foo]\"].name")).isEqualTo("name8");
assertThat(accessor.getPropertyValue("iterableMap[key1].name")).isEqualTo("nameC");
assertThat(accessor.getPropertyValue("iterableMap[key2][0].name")).isEqualTo("nameA");
assertThat(accessor.getPropertyValue("iterableMap[key2][1].name")).isEqualTo("nameB");
assertThat(accessor.getPropertyValue("myTestBeans[0].name")).isEqualTo("nameZ"); assertThat(accessor.getPropertyValue("myTestBeans[0].name")).isEqualTo("nameZ");
MutablePropertyValues pvs = new MutablePropertyValues(); MutablePropertyValues pvs = new MutablePropertyValues();
@ -1409,6 +1430,9 @@ abstract class AbstractPropertyAccessorTests {
pvs.add("map[key4][0].name", "nameA"); pvs.add("map[key4][0].name", "nameA");
pvs.add("map[key4][1].name", "nameB"); pvs.add("map[key4][1].name", "nameB");
pvs.add("map[key5[foo]].name", "name10"); pvs.add("map[key5[foo]].name", "name10");
pvs.add("iterableMap[key1].name", "newName1");
pvs.add("iterableMap[key2][0].name", "newName2A");
pvs.add("iterableMap[key2][1].name", "newName2B");
pvs.add("myTestBeans[0].name", "nameZZ"); pvs.add("myTestBeans[0].name", "nameZZ");
accessor.setPropertyValues(pvs); accessor.setPropertyValues(pvs);
assertThat(tb0.getName()).isEqualTo("name5"); assertThat(tb0.getName()).isEqualTo("name5");
@ -1428,6 +1452,9 @@ abstract class AbstractPropertyAccessorTests {
assertThat(accessor.getPropertyValue("map[key4][0].name")).isEqualTo("nameA"); assertThat(accessor.getPropertyValue("map[key4][0].name")).isEqualTo("nameA");
assertThat(accessor.getPropertyValue("map[key4][1].name")).isEqualTo("nameB"); assertThat(accessor.getPropertyValue("map[key4][1].name")).isEqualTo("nameB");
assertThat(accessor.getPropertyValue("map[key5[foo]].name")).isEqualTo("name10"); assertThat(accessor.getPropertyValue("map[key5[foo]].name")).isEqualTo("name10");
assertThat(accessor.getPropertyValue("iterableMap[key1].name")).isEqualTo("newName1");
assertThat(accessor.getPropertyValue("iterableMap[key2][0].name")).isEqualTo("newName2A");
assertThat(accessor.getPropertyValue("iterableMap[key2][1].name")).isEqualTo("newName2B");
assertThat(accessor.getPropertyValue("myTestBeans[0].name")).isEqualTo("nameZZ"); assertThat(accessor.getPropertyValue("myTestBeans[0].name")).isEqualTo("nameZZ");
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2023 the original author or authors. * Copyright 2002-2025 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -21,6 +21,7 @@ import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.HashMap; import java.util.HashMap;
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@ -49,6 +50,8 @@ public class IndexedTestBean {
private SortedMap sortedMap; private SortedMap sortedMap;
private IterableMap iterableMap;
private MyTestBeans myTestBeans; private MyTestBeans myTestBeans;
@ -73,6 +76,9 @@ public class IndexedTestBean {
TestBean tb6 = new TestBean("name6", 0); TestBean tb6 = new TestBean("name6", 0);
TestBean tb7 = new TestBean("name7", 0); TestBean tb7 = new TestBean("name7", 0);
TestBean tb8 = new TestBean("name8", 0); TestBean tb8 = new TestBean("name8", 0);
TestBean tbA = new TestBean("nameA", 0);
TestBean tbB = new TestBean("nameB", 0);
TestBean tbC = new TestBean("nameC", 0);
TestBean tbX = new TestBean("nameX", 0); TestBean tbX = new TestBean("nameX", 0);
TestBean tbY = new TestBean("nameY", 0); TestBean tbY = new TestBean("nameY", 0);
TestBean tbZ = new TestBean("nameZ", 0); TestBean tbZ = new TestBean("nameZ", 0);
@ -88,6 +94,12 @@ public class IndexedTestBean {
this.map.put("key2", tb5); this.map.put("key2", tb5);
this.map.put("key.3", tb5); this.map.put("key.3", tb5);
List list = new ArrayList(); List list = new ArrayList();
list.add(tbA);
list.add(tbB);
this.iterableMap = new IterableMap<>();
this.iterableMap.put("key1", tbC);
this.iterableMap.put("key2", list);
list = new ArrayList();
list.add(tbX); list.add(tbX);
list.add(tbY); list.add(tbY);
this.map.put("key4", list); this.map.put("key4", list);
@ -152,6 +164,14 @@ public class IndexedTestBean {
this.sortedMap = sortedMap; this.sortedMap = sortedMap;
} }
public IterableMap getIterableMap() {
return this.iterableMap;
}
public void setIterableMap(IterableMap iterableMap) {
this.iterableMap = iterableMap;
}
public MyTestBeans getMyTestBeans() { public MyTestBeans getMyTestBeans() {
return myTestBeans; return myTestBeans;
} }
@ -161,6 +181,14 @@ public class IndexedTestBean {
} }
public static class IterableMap<K,V> extends LinkedHashMap<K,V> implements Iterable<V> {
@Override
public Iterator<V> iterator() {
return values().iterator();
}
}
public static class MyTestBeans implements Iterable<TestBean> { public static class MyTestBeans implements Iterable<TestBean> {
private final Collection<TestBean> testBeans; private final Collection<TestBean> testBeans;