From 37d66034f8eb730ac1763a8122ae192d8af2ff79 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 28 Sep 2023 14:55:14 +0200 Subject: [PATCH] Fix property value conversion in query mapper for nested values. Closes #4510 Original pull request: #4517 --- .../mongodb/core/convert/QueryMapper.java | 116 ++++++++++-------- .../data/mongodb/util/BsonUtils.java | 20 +++ .../core/convert/QueryMapperUnitTests.java | 18 +++ .../data/mongodb/util/json/BsonUtilsTest.java | 47 +++++++ 4 files changed, 148 insertions(+), 53 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index fe6cb2631..8890ed3f4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -449,65 +449,14 @@ public class QueryMapper { if (documentField.getProperty() != null && converter.getCustomConversions().hasValueConverter(documentField.getProperty())) { - MongoConversionContext conversionContext = new MongoConversionContext(new PropertyValueProvider<>() { - @Override - public T getPropertyValue(MongoPersistentProperty property) { - throw new IllegalStateException("No enclosing property available"); - } - }, documentField.getProperty(), converter); PropertyValueConverter> 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 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 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 @@ public class QueryMapper { return createReferenceFor(source, property); } + @Nullable + private Object convertValue(Field documentField, Object sourceValue, Object value, + PropertyValueConverter> valueConverter) { + + MongoConversionContext conversionContext = new MongoConversionContext(new PropertyValueProvider<>() { + @Override + public 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 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}. * diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java index 5ec9ade5d..ca3bc3296 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java @@ -20,9 +20,12 @@ import java.util.Arrays; 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 class BsonUtils { return source.getClass().isArray() ? CollectionUtils.arrayToList(source) : Collections.singleton(source); } + public static Document mapValues(Document source, BiFunction valueMapper) { + return mapEntries(source, Entry::getKey, entry -> valueMapper.apply(entry.getKey(), entry.getValue())); + } + + public static Document mapEntries(Document source, Function,String> keyMapper, Function,Object> valueMapper) { + + if(source.isEmpty()) { + return source; + } + + Map target = new LinkedHashMap<>(source.size(), 1f); + for(Entry entry : source.entrySet()) { + target.put(keyMapper.apply(entry), valueMapper.apply(entry)); + } + return new Document(target); + } + @Nullable private static String toJson(@Nullable Object value) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java index f3a94da11..69953f589 100755 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java @@ -1528,6 +1528,24 @@ public class QueryMapperUnitTests { 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 simpleMap; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java index c43def145..76bc967f1 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java @@ -29,6 +29,7 @@ import java.util.Date; 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 @@ class BsonUtilsTest { } } + @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 fieldNames() { return Stream.of(// Arguments.of(FieldName.path("a"), true), //