Handling collections and map types before running an element converter

spring-data-mongodb/pom.xml

@@ -18,6 +18,7 @@
 	<properties>
 		<objenesis>1.3</objenesis>
 		<equalsverifier>1.7.8</equalsverifier>
+		<vavr>0.10.4</vavr>
 		<java-module-name>spring.data.mongodb</java-module-name>
 		<project.root>${basedir}/..</project.root>
 		<multithreadedtc>1.01</multithreadedtc>
@@ -224,6 +225,13 @@
 			<scope>test</scope>
 		</dependency>
 
+		<dependency>
+			<groupId>io.vavr</groupId>
+			<artifactId>vavr</artifactId>
+			<version>${vavr}</version>
+			<scope>test</scope>
+		</dependency>
+
 		<dependency>
 			<groupId>org.threeten</groupId>
 			<artifactId>threetenbp</artifactId>

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

@@ -1295,7 +1295,8 @@ protected Object readCollectionOrArray(ConversionContext context, Collection<?>
 	 */
 	@Deprecated
 	protected Map<Object, Object> readMap(TypeInformation<?> type, Bson bson, ObjectPath path) {
-		return readMap(getConversionContext(path), bson, type);
+		//noinspection unchecked
+		return (Map<Object, Object>) readMap(getConversionContext(path), bson, type);
 	}
 
 	/**
@@ -1308,15 +1309,19 @@ protected Map<Object, Object> readMap(TypeInformation<?> type, Bson bson, Object
 	 * @return the converted {@link Map}, will never be {@literal null}.
 	 * @since 3.2
 	 */
-	protected Map<Object, Object> readMap(ConversionContext context, Bson bson, TypeInformation<?> targetType) {
+	protected Object readMap(ConversionContext context, Bson bson, TypeInformation<?> targetType) {
 
 		Assert.notNull(bson, "Document must not be null!");
 		Assert.notNull(targetType, "TypeInformation must not be null!");
 
-		Class<?> mapType = getTypeMapper().readType(bson, targetType).getType();
+		TypeInformation<?> typeInformation = getTypeMapper().readType(bson, targetType);
+		Class<?> mapType = typeInformation.isSubTypeOf(Map.class)
+				? targetType.getType()
+				: Map.class;
 
 		TypeInformation<?> keyType = targetType.getComponentType();
-		TypeInformation<?> valueType = targetType.getMapValueType() == null ? ClassTypeInformation.OBJECT
+		TypeInformation<?> valueType = targetType.getMapValueType() == null
+				? ClassTypeInformation.OBJECT
 				: targetType.getRequiredMapValueType();
 
 		Class<?> rawKeyType = keyType != null ? keyType.getType() : Object.class;
@@ -1347,7 +1352,7 @@ protected Map<Object, Object> readMap(ConversionContext context, Bson bson, Type
 
 		});
 
-		return map;
+		return getPotentiallyConvertedSimpleRead(map, targetType.getType());
 	}
 
 	/*
@@ -2032,27 +2037,7 @@ public <S extends Object> S convert(Object source, TypeInformation<? extends S>
 			Assert.notNull(source, "Source must not be null");
 			Assert.notNull(typeHint, "TypeInformation must not be null");
 
-			if (conversions.hasCustomReadTarget(source.getClass(), typeHint.getType())) {
-				return (S) elementConverter.convert(source, typeHint);
-			}
-
-			if (source instanceof Collection) {
-
-				Class<?> rawType = typeHint.getType();
-				if (!Object.class.equals(rawType)) {
-					if (!rawType.isArray() && !ClassUtils.isAssignable(Iterable.class, rawType)) {
-						throw new MappingException(
-								String.format(INCOMPATIBLE_TYPES, source, source.getClass(), rawType, getPath()));
-					}
-				}
-
-				if (typeHint.isCollectionLike() || typeHint.getType().isAssignableFrom(Collection.class)) {
-					return (S) collectionConverter.convert(this, (Collection<?>) source, typeHint);
-				}
-			}
-
 			if (typeHint.isMap()) {
-
 				if (ClassUtils.isAssignable(Document.class, typeHint.getType())) {
 					return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint);
 				}
@@ -2065,6 +2050,14 @@ public <S extends Object> S convert(Object source, TypeInformation<? extends S>
 						String.format("Expected map like structure but found %s", source.getClass()));
 			}
 
+			if (source instanceof Collection) {
+				return (S) collectionConverter.convert(this, (Collection<?>) source, typeHint);
+			}
+
+			if (conversions.hasCustomReadTarget(source.getClass(), typeHint.getType())) {
+				return (S) elementConverter.convert(source, typeHint);
+			}
+
 			if (source instanceof DBRef) {
 				return (S) dbRefConverter.convert(this, (DBRef) source, typeHint);
 			}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomListReadingConvertersUnitTests.java

@@ -0,0 +1,147 @@
+/*
+ * Copyright 2011-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.mongodb.core.convert;
+
+import org.bson.Document;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.core.convert.ConversionFailedException;
+import org.springframework.core.convert.converter.Converter;
+import org.springframework.data.convert.CustomConversions;
+import org.springframework.data.convert.ReadingConverter;
+import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Test case to verify correct usage of custom {@link Converter} implementations for different List implementations to be used.
+ *
+ * @author Jürgen Diez
+ */
+@ExtendWith(MockitoExtension.class)
+class CustomListReadingConvertersUnitTests {
+
+	private MappingMongoConverter converter;
+
+	@Mock JavaListReadingConverter javaListReadingConverter;
+	@Mock IterableListReadingConverter iterableListReadingConverter;
+	@Mock CustomListReadingConverter customListReadingConverter;
+	@Captor ArgumentCaptor<List<TestEnum>> enumListCaptor;
+
+	private Document document;
+
+	@BeforeEach
+	void setUp() {
+		CustomConversions conversions = new MongoCustomConversions(
+				Arrays.asList(javaListReadingConverter, iterableListReadingConverter, customListReadingConverter));
+
+		MongoMappingContext context = new MongoMappingContext();
+		context.setSimpleTypeHolder(conversions.getSimpleTypeHolder());
+		context.initialize();
+
+		converter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, context);
+		converter.setCustomConversions(conversions);
+		converter.afterPropertiesSet();
+
+		document = new Document();
+		document.append("list", Arrays.asList("ENUM_VALUE1", "ENUM_VALUE2"));
+	}
+
+	@Test
+	void invokeJavaListConverterForEnumsAfterResolvingTheListTypes() {
+
+		converter.read(TestJavaList.class, document);
+
+		verify(javaListReadingConverter).convert(enumListCaptor.capture());
+		assertThat(enumListCaptor.getValue()).containsExactly(TestEnum.ENUM_VALUE1, TestEnum.ENUM_VALUE2);
+	}
+
+	@Test
+	void invokeExtendedIterableListConverterForEnumsAfterResolvingTheListTypes() {
+
+		converter.read(TestIterableList.class, document);
+
+		verify(iterableListReadingConverter).convert(enumListCaptor.capture());
+		assertThat(enumListCaptor.getValue()).containsExactly(TestEnum.ENUM_VALUE1, TestEnum.ENUM_VALUE2);
+	}
+
+	@Test
+	void invokeOtherListConverterForEnumsAfterResolvingTheListTypes() {
+
+		converter.read(TestOtherList.class, document);
+
+		verify(customListReadingConverter).convert(enumListCaptor.capture());
+		assertThat(enumListCaptor.getValue()).containsExactly(TestEnum.ENUM_VALUE1, TestEnum.ENUM_VALUE2);
+	}
+
+	@Test
+	void throwExceptionIfNoConverterIsGivenForACustomListImplementation() {
+
+		assertThrows(ConversionFailedException.class, () -> converter.read(TestNoConverterList.class, document));
+	}
+
+
+	@ReadingConverter
+	private interface JavaListReadingConverter extends Converter<List<?>, List<?>> {}
+
+	@ReadingConverter
+	private interface IterableListReadingConverter extends Converter<List<?>, Iterable<?>> {}
+
+	@ReadingConverter
+	private interface CustomListReadingConverter extends Converter<List<?>, OtherList<?>> {}
+
+	private enum TestEnum {
+		ENUM_VALUE1,
+		ENUM_VALUE2
+	}
+
+	private static class TestJavaList {
+		@SuppressWarnings("unused")
+		List<TestEnum> list;
+	}
+
+	private static class TestIterableList {
+		@SuppressWarnings("unused")
+		Iterable<TestEnum> list;
+	}
+
+	private static class TestOtherList {
+		@SuppressWarnings("unused")
+		OtherList<TestEnum> list;
+	}
+
+	private interface OtherList<T> {
+	}
+
+	private static class TestNoConverterList {
+		@SuppressWarnings("unused")
+		NoConverterList<TestEnum> list;
+	}
+
+	private interface NoConverterList<T> {
+	}
+
+}