Fix property value conversion in query mapper for nested values.

Closes #4510
Original pull request: #4517

Author: christophstrobl

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

@@ -449,65 +449,14 @@ protected Object getMappedValue(Field documentField, Object sourceValue) {
 		if (documentField.getProperty() != null
 				&& converter.getCustomConversions().hasValueConverter(documentField.getProperty())) {
 
-			MongoConversionContext conversionContext = new MongoConversionContext(new PropertyValueProvider<>() {
-				@Override
-				public <T> T getPropertyValue(MongoPersistentProperty property) {
-					throw new IllegalStateException("No enclosing property available");
-				}
-			}, documentField.getProperty(), converter);
 			PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> valueConverter = converter
 					.getCustomConversions().getPropertyValueConversions().getValueConverter(documentField.getProperty());
 
-			/* might be an $in clause with multiple entries */
-			if (!documentField.getProperty().isCollectionLike() && sourceValue instanceof Collection<?> collection) {
-				return collection.stream().map(it -> valueConverter.write(it, conversionContext)).collect(Collectors.toList());
-			}
-
-			return valueConverter.write(value, conversionContext);
+			return convertValue(documentField, sourceValue, value, valueConverter);
 		}
 
 		if (documentField.isIdField() && !documentField.isAssociation()) {
-
-			if (isDBObject(value)) {
-				DBObject valueDbo = (DBObject) value;
-				Document resultDbo = new Document(valueDbo.toMap());
-
-				if (valueDbo.containsField("$in") || valueDbo.containsField("$nin")) {
-					String inKey = valueDbo.containsField("$in") ? "$in" : "$nin";
-					List<Object> ids = new ArrayList<>();
-					for (Object id : (Iterable<?>) valueDbo.get(inKey)) {
-						ids.add(convertId(id, getIdTypeForField(documentField)));
-					}
-					resultDbo.put(inKey, ids);
-				} else if (valueDbo.containsField("$ne")) {
-					resultDbo.put("$ne", convertId(valueDbo.get("$ne"), getIdTypeForField(documentField)));
-				} else {
-					return getMappedObject(resultDbo, Optional.empty());
-				}
-				return resultDbo;
-			}
-
-			else if (isDocument(value)) {
-				Document valueDbo = (Document) value;
-				Document resultDbo = new Document(valueDbo);
-
-				if (valueDbo.containsKey("$in") || valueDbo.containsKey("$nin")) {
-					String inKey = valueDbo.containsKey("$in") ? "$in" : "$nin";
-					List<Object> ids = new ArrayList<>();
-					for (Object id : (Iterable<?>) valueDbo.get(inKey)) {
-						ids.add(convertId(id, getIdTypeForField(documentField)));
-					}
-					resultDbo.put(inKey, ids);
-				} else if (valueDbo.containsKey("$ne")) {
-					resultDbo.put("$ne", convertId(valueDbo.get("$ne"), getIdTypeForField(documentField)));
-				} else {
-					return getMappedObject(resultDbo, Optional.empty());
-				}
-				return resultDbo;
-
-			} else {
-				return convertId(value, getIdTypeForField(documentField));
-			}
+			return convertIdField(documentField, value);
 		}
 
 		if (value == null) {
@@ -708,6 +657,67 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers
 		return createReferenceFor(source, property);
 	}
 
+	@Nullable
+	private Object convertValue(Field documentField, Object sourceValue, Object value,
+			PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> valueConverter) {
+
+		MongoConversionContext conversionContext = new MongoConversionContext(new PropertyValueProvider<>() {
+			@Override
+			public <T> T getPropertyValue(MongoPersistentProperty property) {
+				throw new IllegalStateException("No enclosing property available");
+			}
+		}, documentField.getProperty(), converter);
+
+		/* might be an $in clause with multiple entries */
+		if (!documentField.getProperty().isCollectionLike() && sourceValue instanceof Collection<?> collection) {
+			return collection.stream().map(it -> valueConverter.write(it, conversionContext)).collect(Collectors.toList());
+		}
+
+		if (!documentField.getProperty().isMap() && sourceValue instanceof Document document) {
+
+			return BsonUtils.mapValues(document, (key, val) -> {
+				if (isKeyword(key)) {
+					return getMappedValue(documentField, val);
+				}
+				return val;
+			});
+		}
+
+		return valueConverter.write(value, conversionContext);
+	}
+
+	@Nullable
+	private Object convertIdField(Field documentField, Object source) {
+
+		Object value = source;
+		if (isDBObject(source)) {
+			DBObject valueDbo = (DBObject) source;
+			value = new Document(valueDbo.toMap());
+		}
+
+		if (!isDocument(value)) {
+			return convertId(value, getIdTypeForField(documentField));
+		}
+
+		Document valueDbo = (Document) value;
+		Document resultDbo = new Document(valueDbo);
+
+		if (valueDbo.containsKey("$in") || valueDbo.containsKey("$nin")) {
+			String inKey = valueDbo.containsKey("$in") ? "$in" : "$nin";
+			List<Object> ids = new ArrayList<>();
+			for (Object id : (Iterable<?>) valueDbo.get(inKey)) {
+				ids.add(convertId(id, getIdTypeForField(documentField)));
+			}
+			resultDbo.put(inKey, ids);
+		} else if (valueDbo.containsKey("$ne")) {
+			resultDbo.put("$ne", convertId(valueDbo.get("$ne"), getIdTypeForField(documentField)));
+		} else {
+			return getMappedObject(resultDbo, Optional.empty());
+		}
+		return resultDbo;
+
+	}
+
 	/**
 	 * Checks whether the given value is a {@link Document}.
 	 *

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java

@@ -20,9 +20,12 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.StringJoiner;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.StreamSupport;
 
@@ -716,6 +719,23 @@ public static Collection<?> asCollection(Object source) {
 		return source.getClass().isArray() ? CollectionUtils.arrayToList(source) : Collections.singleton(source);
 	}
 
+	public static Document mapValues(Document source, BiFunction<String, Object, Object> valueMapper) {
+		return mapEntries(source, Entry::getKey, entry -> valueMapper.apply(entry.getKey(), entry.getValue()));
+	}
+
+	public static Document mapEntries(Document source, Function<Entry<String,Object>,String> keyMapper, Function<Entry<String,Object>,Object> valueMapper) {
+
+		if(source.isEmpty()) {
+			return source;
+		}
+
+		Map<String, Object> target = new LinkedHashMap<>(source.size(), 1f);
+		for(Entry<String,Object> entry : source.entrySet()) {
+			target.put(keyMapper.apply(entry), valueMapper.apply(entry));
+		}
+		return new Document(target);
+	}
+
 	@Nullable
 	private static String toJson(@Nullable Object value) {
 

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

@@ -1528,6 +1528,24 @@ void mappingShouldRetainMapKeyOrder() {
 		assertThat(target.get("simpleMap", Map.class)).containsExactlyEntriesOf(sourceMap);
 	}
 
+	@Test // GH-4510
+	void convertsNestedOperatorValueForPropertyThatHasValueConverter() {
+
+		org.bson.Document mappedObject = mapper.getMappedObject(query(where("text").gt("spring").lt( "data")).getQueryObject(),
+				context.getPersistentEntity(WithPropertyValueConverter.class));
+
+		assertThat(mappedObject).isEqualTo("{ 'text' : {  $gt : 'gnirps', $lt : 'atad' } }");
+	}
+
+	@Test // GH-4510
+	void convertsNestedOperatorValueForPropertyContainingListThatHasValueConverter() {
+
+		org.bson.Document mappedObject = mapper.getMappedObject(query(where("text").gt("spring").in( "data")).getQueryObject(),
+				context.getPersistentEntity(WithPropertyValueConverter.class));
+
+		assertThat(mappedObject).isEqualTo("{ 'text' : {  $gt : 'gnirps', $in : [ 'atad' ] } }");
+	}
+
 	class WithSimpleMap {
 		Map<String, String> simpleMap;
 	}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java

@@ -29,6 +29,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.stream.Stream;
 
 import org.bson.BsonArray;
@@ -180,6 +181,52 @@ void resolveValueForField(FieldName fieldName, boolean exists) {
 		}
 	}
 
+	@Test
+	void retainsOrderWhenMappingValues() {
+
+		Document source = new Document();
+		source.append("z", "first-entry");
+		source.append("a", "second-entry");
+		source.append("0", "third-entry");
+		source.append("9", "fourth-entry");
+
+		Document target = BsonUtils.mapValues(source, (key, value) -> value);
+		assertThat(source).isNotSameAs(target).containsExactlyEntriesOf(source);
+	}
+
+	@Test
+	void retainsOrderWhenMappingKeys() {
+
+		Document source = new Document();
+		source.append("z", "first-entry");
+		source.append("a", "second-entry");
+
+		Document target = BsonUtils.mapEntries(source, entry -> entry.getKey().toUpperCase(), Entry::getValue);
+		assertThat(target).containsExactly(Map.entry("Z", "first-entry"), Map.entry("A", "second-entry"));
+	}
+
+	@Test
+	void appliesValueMapping() {
+		Document source = new Document();
+		source.append("z", "first-entry");
+		source.append("a", "second-entry");
+
+		Document target = BsonUtils.mapValues(source,
+				(key, value) -> new StringBuilder(value.toString()).reverse().toString());
+		assertThat(target).containsValues("yrtne-tsrif", "yrtne-dnoces");
+	}
+
+	@Test
+	void appliesKeyMapping() {
+
+		Document source = new Document();
+		source.append("z", "first-entry");
+		source.append("a", "second-entry");
+
+		Document target = BsonUtils.mapEntries(source, entry -> entry.getKey().toUpperCase(), Entry::getValue);
+		assertThat(target).containsKeys("Z", "A");
+	}
+
 	static Stream<Arguments> fieldNames() {
 		return Stream.of(//
 				Arguments.of(FieldName.path("a"), true), //