Correctly convert enum array values.

We now correctly convert array write values. Previously, enum arrays were converted to null as these fell through the entity conversion.

Closes #1593

Author: mp911de

spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.r2dbc.core;
 
+import static org.mockito.Mockito.*;
 import static org.springframework.data.r2dbc.testing.Assertions.*;
 
 import io.r2dbc.postgresql.codec.Interval;
@@ -33,9 +34,13 @@
 import org.springframework.data.convert.ReadingConverter;
 import org.springframework.data.convert.WritingConverter;
 import org.springframework.data.r2dbc.convert.EnumWriteSupport;
+import org.springframework.data.r2dbc.core.StatementMapper.InsertSpec;
 import org.springframework.data.r2dbc.dialect.PostgresDialect;
 import org.springframework.data.r2dbc.mapping.OutboundRow;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
+import org.springframework.r2dbc.core.Parameter;
+import org.springframework.r2dbc.core.PreparedOperation;
+import org.springframework.r2dbc.core.binding.BindTarget;
 
 /**
  * {@link PostgresDialect} specific tests for {@link ReactiveDataAccessStrategy}.
@@ -60,6 +65,20 @@ void shouldConvertPrimitiveMultidimensionArrayToWrapper() {
 		assertThat(row).withColumn("myarray").hasValueInstanceOf(Integer[][].class);
 	}
 
+	@Test // GH-1593
+	void shouldConvertEnumsCorrectly() {
+
+		StatementMapper mapper = strategy.getStatementMapper();
+		MyEnum[] value = { MyEnum.ONE };
+		InsertSpec insert = mapper.createInsert("table").withColumn("my_col", Parameter.from(value));
+		PreparedOperation<?> mappedObject = mapper.getMappedObject(insert);
+
+		BindTarget bindTarget = mock(BindTarget.class);
+		mappedObject.bindTo(bindTarget);
+
+		verify(bindTarget).bind(0, new String[] { "ONE" });
+	}
+
 	@Test // gh-161
 	void shouldConvertNullArrayToDriverArrayType() {
 

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.relational.core.conversion;
 
+import java.lang.reflect.Array;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -164,44 +165,19 @@ public Object writeValue(@Nullable Object value, TypeInformation<?> type) {
 
 		if (getConversions().isSimpleType(value.getClass())) {
 
-			if (TypeInformation.OBJECT != type) {
-
-				if (conversionService.canConvert(value.getClass(), type.getType())) {
-					value = conversionService.convert(value, type.getType());
-				}
+			if (TypeInformation.OBJECT != type && conversionService.canConvert(value.getClass(), type.getType())) {
+				value = conversionService.convert(value, type.getType());
 			}
 
 			return getPotentiallyConvertedSimpleWrite(value);
 		}
 
-		// TODO: We should add conversion support for arrays, however,
-		// these should consider multi-dimensional arrays as well.
-		if (value.getClass().isArray() //
-				&& !value.getClass().getComponentType().isEnum() //
-				&& (TypeInformation.OBJECT.equals(type) //
-						|| type.isCollectionLike()) //
-		) {
-			return value;
+		if (value.getClass().isArray()) {
+			return writeArray(value, type);
 		}
 
 		if (value instanceof Collection<?>) {
-
-			List<Object> mapped = new ArrayList<>();
-
-			TypeInformation<?> component = TypeInformation.OBJECT;
-			if (type.isCollectionLike() && type.getActualType() != null) {
-				component = type.getRequiredComponentType();
-			}
-
-			for (Object o : (Iterable<?>) value) {
-				mapped.add(writeValue(o, component));
-			}
-
-			if (type.getType().isInstance(mapped) || !type.isCollectionLike()) {
-				return mapped;
-			}
-
-			return conversionService.convert(mapped, type.getType());
+			return writeCollection((Iterable<?>) value, type);
 		}
 
 		RelationalPersistentEntity<?> persistentEntity = context.getPersistentEntity(value.getClass());
@@ -215,6 +191,57 @@ public Object writeValue(@Nullable Object value, TypeInformation<?> type) {
 		return conversionService.convert(value, type.getType());
 	}
 
+	private Object writeArray(Object value, TypeInformation<?> type) {
+
+		Class<?> componentType = value.getClass().getComponentType();
+		Optional<Class<?>> optionalWriteTarget = getConversions().getCustomWriteTarget(componentType);
+
+		if (optionalWriteTarget.isEmpty() && !componentType.isEnum()) {
+			return value;
+		}
+
+		Class<?> customWriteTarget = optionalWriteTarget
+				.orElseGet(() -> componentType.isEnum() ? String.class : componentType);
+
+		// optimization: bypass identity conversion
+		if (customWriteTarget.equals(componentType)) {
+			return value;
+		}
+
+		TypeInformation<?> component = TypeInformation.OBJECT;
+		if (type.isCollectionLike() && type.getActualType() != null) {
+			component = type.getRequiredComponentType();
+		}
+
+		int length = Array.getLength(value);
+		Object target = Array.newInstance(customWriteTarget, length);
+		for (int i = 0; i < length; i++) {
+			Array.set(target, i, writeValue(Array.get(value, i), component));
+		}
+
+		return target;
+	}
+
+	private Object writeCollection(Iterable<?> value, TypeInformation<?> type) {
+
+		List<Object> mapped = new ArrayList<>();
+
+		TypeInformation<?> component = TypeInformation.OBJECT;
+		if (type.isCollectionLike() && type.getActualType() != null) {
+			component = type.getRequiredComponentType();
+		}
+
+		for (Object o : value) {
+			mapped.add(writeValue(o, component));
+		}
+
+		if (type.getType().isInstance(mapped) || !type.isCollectionLike()) {
+			return mapped;
+		}
+
+		return conversionService.convert(mapped, type.getType());
+	}
+
 	@Override
 	public EntityInstantiators getEntityInstantiators() {
 		return this.entityInstantiators;

spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/BasicRelationalConverterUnitTests.java

@@ -20,14 +20,15 @@
 import lombok.Data;
 import lombok.Value;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Set;
+import java.util.function.Function;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.springframework.core.convert.converter.GenericConverter;
 import org.springframework.data.convert.ConverterBuilder;
+import org.springframework.data.convert.ConverterBuilder.ConverterAware;
 import org.springframework.data.convert.CustomConversions;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
@@ -49,8 +50,13 @@ class BasicRelationalConverterUnitTests {
 	@BeforeEach
 	public void before() throws Exception {
 
-		Set<GenericConverter> converters = ConverterBuilder.writing(MyValue.class, String.class, MyValue::getFoo)
-				.andReading(MyValue::new).getConverters();
+		List<Object> converters = new ArrayList<>();
+		converters.addAll(ConverterBuilder.writing(MyValue.class, String.class, MyValue::getFoo).andReading(MyValue::new)
+				.getConverters());
+
+		ConverterAware converterAware = ConverterBuilder
+				.writing(MySimpleEnum.class, MySimpleEnum.class, Function.identity()).andReading(mySimpleEnum -> mySimpleEnum);
+		converters.addAll(converterAware.getConverters());
 
 		CustomConversions conversions = new CustomConversions(CustomConversions.StoreConversions.NONE, converters);
 		context.setSimpleTypeHolder(conversions.getSimpleTypeHolder());
@@ -82,7 +88,23 @@ void shouldConvertEnumToString() {
 		assertThat(result).isEqualTo("ON");
 	}
 
-	@Test // DATAJDBC-235
+	@Test
+	void shouldConvertEnumArrayToStringArray() {
+
+		Object result = converter.writeValue(new MyEnum[] { MyEnum.ON }, TypeInformation.OBJECT);
+
+		assertThat(result).isEqualTo(new String[] { "ON" });
+	}
+
+	@Test // GH-1593
+	void shouldRetainEnumArray() {
+
+		Object result = converter.writeValue(new MySimpleEnum[] { MySimpleEnum.ON }, TypeInformation.OBJECT);
+
+		assertThat(result).isEqualTo(new MySimpleEnum[] { MySimpleEnum.ON });
+	}
+
+	@Test // GH-1593
 	void shouldConvertStringToEnum() {
 
 		Object result = converter.readValue("OFF", TypeInformation.of(MyEnum.class));
@@ -93,8 +115,7 @@ void shouldConvertStringToEnum() {
 	@Test // GH-1046
 	void shouldConvertArrayElementsToTargetElementType() throws NoSuchMethodException {
 
-		TypeInformation<?> typeInformation = TypeInformation
-				.fromReturnTypeOf(EntityWithArray.class.getMethod("getFloats"));
+		TypeInformation<?> typeInformation = TypeInformation.fromReturnTypeOf(EntityWithArray.class.getMethod("getFloats"));
 		Double[] value = { 1.2d, 1.3d, 1.4d };
 		Object result = converter.readValue(value, typeInformation);
 		assertThat(result).isEqualTo(Arrays.asList(1.2f, 1.3f, 1.4f));
@@ -157,4 +178,8 @@ static class MyEntityWithConvertibleProperty {
 	enum MyEnum {
 		ON, OFF;
 	}
+
+	enum MySimpleEnum {
+		ON, OFF;
+	}
 }