Skip to content

Commit 84b3335

Browse files
jhoellerjhoeller
committed
Apply array editor to collection of same element type as well
Closes gh-24845
1 parent c3e18bc commit 84b3335

File tree

5 files changed

+43
-19
lines changed

5 files changed

+43
-19
lines changed

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

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.springframework.core.convert.TypeDescriptor;
3636
import org.springframework.lang.Nullable;
3737
import org.springframework.util.ClassUtils;
38+
import org.springframework.util.CollectionUtils;
3839
import org.springframework.util.NumberUtils;
3940
import org.springframework.util.ReflectionUtils;
4041
import org.springframework.util.StringUtils;
@@ -139,13 +140,17 @@ public <T> T convertIfNecessary(@Nullable String propertyName, @Nullable Object
139140

140141
// Value not of required type?
141142
if (editor != null || (requiredType != null && !ClassUtils.isAssignableValue(requiredType, convertedValue))) {
142-
if (typeDescriptor != null && requiredType != null && Collection.class.isAssignableFrom(requiredType) &&
143-
convertedValue instanceof String text) {
143+
if (typeDescriptor != null && requiredType != null && Collection.class.isAssignableFrom(requiredType)) {
144144
TypeDescriptor elementTypeDesc = typeDescriptor.getElementTypeDescriptor();
145145
if (elementTypeDesc != null) {
146146
Class<?> elementType = elementTypeDesc.getType();
147-
if (Class.class == elementType || Enum.class.isAssignableFrom(elementType)) {
148-
convertedValue = StringUtils.commaDelimitedListToStringArray(text);
147+
if (convertedValue instanceof String text) {
148+
if (Class.class == elementType || Enum.class.isAssignableFrom(elementType)) {
149+
convertedValue = StringUtils.commaDelimitedListToStringArray(text);
150+
}
151+
if (editor == null && String.class != elementType) {
152+
editor = findDefaultEditor(Array.newInstance(elementType, 0).getClass());
153+
}
149154
}
150155
}
151156
}
@@ -166,11 +171,23 @@ public <T> T convertIfNecessary(@Nullable String propertyName, @Nullable Object
166171
}
167172
else if (requiredType.isArray()) {
168173
// Array required -> apply appropriate conversion of elements.
169-
if (convertedValue instanceof String text && Enum.class.isAssignableFrom(requiredType.getComponentType())) {
174+
if (convertedValue instanceof String text &&
175+
Enum.class.isAssignableFrom(requiredType.getComponentType())) {
170176
convertedValue = StringUtils.commaDelimitedListToStringArray(text);
171177
}
172178
return (T) convertToTypedArray(convertedValue, propertyName, requiredType.getComponentType());
173179
}
180+
else if (convertedValue.getClass().isArray()) {
181+
if (Array.getLength(convertedValue) == 1) {
182+
convertedValue = Array.get(convertedValue, 0);
183+
standardConversion = true;
184+
}
185+
else if (Collection.class.isAssignableFrom(requiredType)) {
186+
convertedValue = convertToTypedCollection(CollectionUtils.arrayToList(convertedValue),
187+
propertyName, requiredType, typeDescriptor);
188+
standardConversion = true;
189+
}
190+
}
174191
else if (convertedValue instanceof Collection<?> coll) {
175192
// Convert elements to target type, if determined.
176193
convertedValue = convertToTypedCollection(coll, propertyName, requiredType, typeDescriptor);
@@ -181,10 +198,6 @@ else if (convertedValue instanceof Map<?, ?> map) {
181198
convertedValue = convertToTypedMap(map, propertyName, requiredType, typeDescriptor);
182199
standardConversion = true;
183200
}
184-
if (convertedValue.getClass().isArray() && Array.getLength(convertedValue) == 1) {
185-
convertedValue = Array.get(convertedValue, 0);
186-
standardConversion = true;
187-
}
188201
if (String.class == requiredType && ClassUtils.isPrimitiveOrWrapper(convertedValue.getClass())) {
189202
// We can stringify any primitive value...
190203
return (T) convertedValue.toString();
@@ -501,12 +514,11 @@ private Collection<?> convertToTypedCollection(Collection<?> original, @Nullable
501514

502515
Collection<Object> convertedCopy;
503516
try {
504-
if (approximable) {
517+
if (approximable && requiredType.isInstance(original)) {
505518
convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
506519
}
507520
else {
508-
convertedCopy = (Collection<Object>)
509-
ReflectionUtils.accessibleConstructor(requiredType).newInstance();
521+
convertedCopy = CollectionFactory.createCollection(requiredType, original.size());
510522
}
511523
}
512524
catch (Throwable ex) {
@@ -576,12 +588,11 @@ private Collection<?> convertToTypedCollection(Collection<?> original, @Nullable
576588

577589
Map<Object, Object> convertedCopy;
578590
try {
579-
if (approximable) {
591+
if (approximable && requiredType.isInstance(original)) {
580592
convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
581593
}
582594
else {
583-
convertedCopy = (Map<Object, Object>)
584-
ReflectionUtils.accessibleConstructor(requiredType).newInstance();
595+
convertedCopy = CollectionFactory.createMap(requiredType, original.size());
585596
}
586597
}
587598
catch (Throwable ex) {

‎spring-context/src/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,12 @@ void resourceArrayPropertyEditor() throws IOException {
222222
ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext(CONTEXT_WILDCARD);
223223
Service service = ctx.getBean("service", Service.class);
224224
assertThat(service.getResources()).containsExactlyInAnyOrder(contextA, contextB, contextC);
225+
assertThat(service.getResourceSet()).containsExactlyInAnyOrder(contextA, contextB, contextC);
225226
ctx.close();
226227
}
227228

228229
@Test
229-
void childWithProxy() throws Exception {
230+
void childWithProxy() {
230231
ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext(CONTEXT_WILDCARD);
231232
ClassPathXmlApplicationContext child = new ClassPathXmlApplicationContext(
232233
new String[] {CHILD_WITH_PROXY_CONTEXT}, ctx);
@@ -337,8 +338,7 @@ public Resource getResource(String location) {
337338
};
338339
ResourceTestBean resource1 = (ResourceTestBean) ctx.getBean("resource1");
339340
ResourceTestBean resource2 = (ResourceTestBean) ctx.getBean("resource2");
340-
boolean condition = resource1.getResource() instanceof ClassPathResource;
341-
assertThat(condition).isTrue();
341+
assertThat(resource1.getResource()).isInstanceOf(ClassPathResource.class);
342342
StringWriter writer = new StringWriter();
343343
FileCopyUtils.copy(new InputStreamReader(resource1.getResource().getInputStream()), writer);
344344
assertThat(writer.toString()).isEqualTo("contexttest");

‎spring-context/src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private void doTestConversionServiceInApplicationContext(String fileName, Class<
118118
ConfigurableApplicationContext ctx = new ClassPathXmlApplicationContext(fileName, getClass());
119119
ResourceTestBean tb = ctx.getBean("resourceTestBean", ResourceTestBean.class);
120120
assertThat(resourceClass.isInstance(tb.getResource())).isTrue();
121-
assertThat(tb.getResourceArray()).isNotEmpty();
121+
assertThat(tb.getResourceArray()).hasSize(1);
122122
assertThat(resourceClass.isInstance(tb.getResourceArray()[0])).isTrue();
123123
assertThat(tb.getResourceMap()).hasSize(1);
124124
assertThat(resourceClass.isInstance(tb.getResourceMap().get("key1"))).isTrue();

‎spring-context/src/test/java/org/springframework/context/support/Service.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.context.support;
1818

19+
import java.util.Set;
20+
1921
import org.springframework.beans.factory.BeanCreationNotAllowedException;
2022
import org.springframework.beans.factory.DisposableBean;
2123
import org.springframework.context.ApplicationContext;
@@ -37,6 +39,8 @@ public class Service implements ApplicationContextAware, MessageSourceAware, Dis
3739

3840
private Resource[] resources;
3941

42+
private Set<Resource> resourceSet;
43+
4044
private boolean properlyDestroyed = false;
4145

4246

@@ -65,6 +69,14 @@ public Resource[] getResources() {
6569
return resources;
6670
}
6771

72+
public void setResourceSet(Set<Resource> resourceSet) {
73+
this.resourceSet = resourceSet;
74+
}
75+
76+
public Set<Resource> getResourceSet() {
77+
return resourceSet;
78+
}
79+
6880

6981
@Override
7082
public void destroy() {

‎spring-context/src/test/resources/org/springframework/context/support/test/contextA.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
<bean name="service" class="org.springframework.context.support.Service">
1919
<property name="resources" value="/org/springframework/context/support/test/context*.xml"/>
20+
<property name="resourceSet" value="/org/springframework/context/support/test/context*.xml"/>
2021
</bean>
2122

2223
<bean name="service2" class="org.springframework.context.support.Service" autowire="byName" depends-on="service">

0 commit comments

Comments
 (0)