Apply array editor to collection of same element type as well

Closes gh-24845
spring-projects · Aug 3, 2023 · 84b3335 · 84b3335
1 parent c3e18bc
commit 84b3335
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 19 deletions.
diff --git a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java
@@ -35,6 +35,7 @@
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.lang.Nullable;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.NumberUtils;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
@@ -139,13 +140,17 @@ public <T> T convertIfNecessary(@Nullable String propertyName, @Nullable Object
 
 		// Value not of required type?
 		if (editor != null || (requiredType != null && !ClassUtils.isAssignableValue(requiredType, convertedValue))) {
-			if (typeDescriptor != null && requiredType != null && Collection.class.isAssignableFrom(requiredType) &&
-					convertedValue instanceof String text) {
+			if (typeDescriptor != null && requiredType != null && Collection.class.isAssignableFrom(requiredType)) {
 				TypeDescriptor elementTypeDesc = typeDescriptor.getElementTypeDescriptor();
 				if (elementTypeDesc != null) {
 					Class<?> elementType = elementTypeDesc.getType();
-					if (Class.class == elementType || Enum.class.isAssignableFrom(elementType)) {
-						convertedValue = StringUtils.commaDelimitedListToStringArray(text);
+					if (convertedValue instanceof String text) {
+						if (Class.class == elementType || Enum.class.isAssignableFrom(elementType)) {
+							convertedValue = StringUtils.commaDelimitedListToStringArray(text);
+						}
+						if (editor == null && String.class != elementType) {
+							editor = findDefaultEditor(Array.newInstance(elementType, 0).getClass());
+						}
 					}
 				}
 			}
@@ -166,11 +171,23 @@ public <T> T convertIfNecessary(@Nullable String propertyName, @Nullable Object
 				}
 				else if (requiredType.isArray()) {
 					// Array required -> apply appropriate conversion of elements.
-					if (convertedValue instanceof String text && Enum.class.isAssignableFrom(requiredType.getComponentType())) {
+					if (convertedValue instanceof String text &&
+							Enum.class.isAssignableFrom(requiredType.getComponentType())) {
 						convertedValue = StringUtils.commaDelimitedListToStringArray(text);
 					}
 					return (T) convertToTypedArray(convertedValue, propertyName, requiredType.getComponentType());
 				}
+				else if (convertedValue.getClass().isArray()) {
+					if (Array.getLength(convertedValue) == 1) {
+						convertedValue = Array.get(convertedValue, 0);
+						standardConversion = true;
+					}
+					else if (Collection.class.isAssignableFrom(requiredType)) {
+						convertedValue = convertToTypedCollection(CollectionUtils.arrayToList(convertedValue),
+								propertyName, requiredType, typeDescriptor);
+						standardConversion = true;
+					}
+				}
 				else if (convertedValue instanceof Collection<?> coll) {
 					// Convert elements to target type, if determined.
 					convertedValue = convertToTypedCollection(coll, propertyName, requiredType, typeDescriptor);
@@ -181,10 +198,6 @@ else if (convertedValue instanceof Map<?, ?> map) {
 					convertedValue = convertToTypedMap(map, propertyName, requiredType, typeDescriptor);
 					standardConversion = true;
 				}
-				if (convertedValue.getClass().isArray() && Array.getLength(convertedValue) == 1) {
-					convertedValue = Array.get(convertedValue, 0);
-					standardConversion = true;
-				}
 				if (String.class == requiredType && ClassUtils.isPrimitiveOrWrapper(convertedValue.getClass())) {
 					// We can stringify any primitive value...
 					return (T) convertedValue.toString();
@@ -501,12 +514,11 @@ private Collection<?> convertToTypedCollection(Collection<?> original, @Nullable
 
 		Collection<Object> convertedCopy;
 		try {
-			if (approximable) {
+			if (approximable && requiredType.isInstance(original)) {
 				convertedCopy = CollectionFactory.createApproximateCollection(original, original.size());
 			}
 			else {
-				convertedCopy = (Collection<Object>)
-						ReflectionUtils.accessibleConstructor(requiredType).newInstance();
+				convertedCopy = CollectionFactory.createCollection(requiredType, original.size());
 			}
 		}
 		catch (Throwable ex) {
@@ -576,12 +588,11 @@ private Collection<?> convertToTypedCollection(Collection<?> original, @Nullable
 
 		Map<Object, Object> convertedCopy;
 		try {
-			if (approximable) {
+			if (approximable && requiredType.isInstance(original)) {
 				convertedCopy = CollectionFactory.createApproximateMap(original, original.size());
 			}
 			else {
-				convertedCopy = (Map<Object, Object>)
-						ReflectionUtils.accessibleConstructor(requiredType).newInstance();
+				convertedCopy = CollectionFactory.createMap(requiredType, original.size());
 			}
 		}
 		catch (Throwable ex) {

diff --git a/...rc/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java b/...rc/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java
@@ -222,11 +222,12 @@ void resourceArrayPropertyEditor() throws IOException {
 		ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext(CONTEXT_WILDCARD);
 		Service service = ctx.getBean("service", Service.class);
 		assertThat(service.getResources()).containsExactlyInAnyOrder(contextA, contextB, contextC);
+		assertThat(service.getResourceSet()).containsExactlyInAnyOrder(contextA, contextB, contextC);
 		ctx.close();
 	}
 
 	@Test
-	void childWithProxy() throws Exception {
+	void childWithProxy() {
 		ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext(CONTEXT_WILDCARD);
 		ClassPathXmlApplicationContext child = new ClassPathXmlApplicationContext(
 				new String[] {CHILD_WITH_PROXY_CONTEXT}, ctx);
@@ -337,8 +338,7 @@ public Resource getResource(String location) {
 		};
 		ResourceTestBean resource1 = (ResourceTestBean) ctx.getBean("resource1");
 		ResourceTestBean resource2 = (ResourceTestBean) ctx.getBean("resource2");
-		boolean condition = resource1.getResource() instanceof ClassPathResource;
-		assertThat(condition).isTrue();
+		assertThat(resource1.getResource()).isInstanceOf(ClassPathResource.class);
 		StringWriter writer = new StringWriter();
 		FileCopyUtils.copy(new InputStreamReader(resource1.getResource().getInputStream()), writer);
 		assertThat(writer.toString()).isEqualTo("contexttest");

diff --git a/.../src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java b/.../src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java
@@ -118,7 +118,7 @@ private void doTestConversionServiceInApplicationContext(String fileName, Class<
 		ConfigurableApplicationContext ctx = new ClassPathXmlApplicationContext(fileName, getClass());
 		ResourceTestBean tb = ctx.getBean("resourceTestBean", ResourceTestBean.class);
 		assertThat(resourceClass.isInstance(tb.getResource())).isTrue();
-		assertThat(tb.getResourceArray()).isNotEmpty();
+		assertThat(tb.getResourceArray()).hasSize(1);
 		assertThat(resourceClass.isInstance(tb.getResourceArray()[0])).isTrue();
 		assertThat(tb.getResourceMap()).hasSize(1);
 		assertThat(resourceClass.isInstance(tb.getResourceMap().get("key1"))).isTrue();

diff --git a/spring-context/src/test/java/org/springframework/context/support/Service.java b/spring-context/src/test/java/org/springframework/context/support/Service.java
@@ -16,6 +16,8 @@
 
 package org.springframework.context.support;
 
+import java.util.Set;
+
 import org.springframework.beans.factory.BeanCreationNotAllowedException;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.context.ApplicationContext;
@@ -37,6 +39,8 @@ public class Service implements ApplicationContextAware, MessageSourceAware, Dis
 
 	private Resource[] resources;
 
+	private Set<Resource> resourceSet;
+
 	private boolean properlyDestroyed = false;
 
 
@@ -65,6 +69,14 @@ public Resource[] getResources() {
 		return resources;
 	}
 
+	public void setResourceSet(Set<Resource> resourceSet) {
+		this.resourceSet = resourceSet;
+	}
+
+	public Set<Resource> getResourceSet() {
+		return resourceSet;
+	}
+
 
 	@Override
 	public void destroy() {

diff --git a/spring-context/src/test/resources/org/springframework/context/support/test/contextA.xml b/spring-context/src/test/resources/org/springframework/context/support/test/contextA.xml
@@ -17,6 +17,7 @@
 
 	<bean name="service" class="org.springframework.context.support.Service">
 		<property name="resources" value="/org/springframework/context/support/test/context*.xml"/>
+		<property name="resourceSet" value="/org/springframework/context/support/test/context*.xml"/>
 	</bean>
 
 	<bean name="service2" class="org.springframework.context.support.Service" autowire="byName" depends-on="service">