diff --git a/pom.xml b/pom.xml index a67ad87168..9c7ed1366b 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.3.0-SNAPSHOT + 4.3.x-4510-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 34d95eb205..473381168b 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 4.3.0-SNAPSHOT + 4.3.x-4510-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 124a6bf5ad..81adb719cd 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 4.3.0-SNAPSHOT + 4.3.x-4510-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index e7282be0fa..30e7b9f714 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 4.3.0-SNAPSHOT + 4.3.x-4510-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java index 5172746cc5..c1a478fa77 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java @@ -32,19 +32,19 @@ public class MongoConversionContext implements ValueConversionContext { private final PropertyValueProvider accessor; // TODO: generics - private final MongoPersistentProperty persistentProperty; + private final @Nullable MongoPersistentProperty persistentProperty; private final MongoConverter mongoConverter; - @Nullable - private final SpELContext spELContext; + @Nullable private final SpELContext spELContext; public MongoConversionContext(PropertyValueProvider accessor, - MongoPersistentProperty persistentProperty, MongoConverter mongoConverter) { + @Nullable MongoPersistentProperty persistentProperty, MongoConverter mongoConverter) { this(accessor, persistentProperty, mongoConverter, null); } public MongoConversionContext(PropertyValueProvider accessor, - MongoPersistentProperty persistentProperty, MongoConverter mongoConverter, @Nullable SpELContext spELContext) { + @Nullable MongoPersistentProperty persistentProperty, MongoConverter mongoConverter, + @Nullable SpELContext spELContext) { this.accessor = accessor; this.persistentProperty = persistentProperty; @@ -54,12 +54,17 @@ public MongoConversionContext(PropertyValueProvider acc @Override public MongoPersistentProperty getProperty() { + + if (persistentProperty == null) { + throw new IllegalStateException("No underlying MongoPersistentProperty available"); + } + return persistentProperty; } @Nullable public Object getValue(String propertyPath) { - return accessor.getPropertyValue(persistentProperty.getOwner().getRequiredPersistentProperty(propertyPath)); + return accessor.getPropertyValue(getProperty().getOwner().getRequiredPersistentProperty(propertyPath)); } @Override 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 fe6cb26311..2e03627474 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 @@ -15,8 +15,17 @@ */ package org.springframework.data.mongodb.core.convert; -import java.util.*; +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -27,6 +36,7 @@ import org.bson.Document; import org.bson.conversions.Bson; import org.bson.types.ObjectId; + import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Reference; @@ -76,6 +86,7 @@ * @author Mark Paluch * @author David Julia * @author Divya Srivastava + * @author Gyungrai Wang */ public class QueryMapper { @@ -449,65 +460,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 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) { @@ -547,7 +507,7 @@ protected boolean isAssociationConversionNecessary(Field documentField, @Nullabl Assert.notNull(documentField, "Document field must not be null"); - if (value == null) { + if (value == null || documentField.getProperty() == null) { return false; } @@ -603,10 +563,6 @@ protected Object convertSimpleOrDocument(Object source, @Nullable MongoPersisten return getMappedObject((Document) source, entity); } - if (source instanceof BasicDBList) { - return delegateConvertToMongoType(source, entity); - } - if (isDBObject(source)) { return getMappedObject((BasicDBObject) source, entity); } @@ -615,20 +571,20 @@ protected Object convertSimpleOrDocument(Object source, @Nullable MongoPersisten return source; } - if (source instanceof Map sourceMap) { + if (source instanceof Map sourceMap) { Map map = new LinkedHashMap<>(sourceMap.size(), 1F); - sourceMap.entrySet().forEach(it -> { + for (Entry entry : sourceMap.entrySet()) { - String key = ObjectUtils.nullSafeToString(converter.convertToMongoType(it.getKey())); + String key = ObjectUtils.nullSafeToString(converter.convertToMongoType(entry.getKey())); - if (it.getValue() instanceof Document document) { + if (entry.getValue() instanceof Document document) { map.put(key, getMappedObject(document, entity)); } else { - map.put(key, delegateConvertToMongoType(it.getValue(), entity)); + map.put(key, delegateConvertToMongoType(entry.getValue(), entity)); } - }); + } return map; } @@ -654,6 +610,7 @@ protected Object delegateConvertToMongoType(Object source, @Nullable MongoPersis return converter.convertToMongoType(source, entity == null ? null : entity.getTypeInformation()); } + @Nullable protected Object convertAssociation(Object source, Field field) { Object value = convertAssociation(source, field.getProperty()); if (value != null && field.isIdField() && field.getFieldType() != value.getClass()) { @@ -678,8 +635,7 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers if (source instanceof DBRef ref) { - Object id = convertId(ref.getId(), - property != null && property.isIdProperty() ? property.getFieldType() : ObjectId.class); + Object id = convertId(ref.getId(), property.isIdProperty() ? property.getFieldType() : ObjectId.class); if (StringUtils.hasText(ref.getDatabaseName())) { return new DBRef(ref.getDatabaseName(), ref.getCollectionName(), id); @@ -696,9 +652,8 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers return result; } - if (property.isMap()) { + if (property.isMap() && source instanceof Document dbObject) { Document result = new Document(); - Document dbObject = (Document) source; for (String key : dbObject.keySet()) { result.put(key, createReferenceFor(dbObject.get(key), property)); } @@ -708,6 +663,78 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers return createReferenceFor(source, property); } + @Nullable + private Object convertValue(Field documentField, Object sourceValue, Object value, + PropertyValueConverter> valueConverter) { + + MongoPersistentProperty property = documentField.getProperty(); + MongoConversionContext conversionContext = new MongoConversionContext(NoPropertyPropertyValueProvider.INSTANCE, + property, converter); + + /* might be an $in clause with multiple entries */ + if (property != null && !property.isCollectionLike() && sourceValue instanceof Collection collection) { + + if (collection.isEmpty()) { + return collection; + } + + List converted = new ArrayList<>(collection.size()); + for (Object o : collection) { + converted.add(valueConverter.write(o, conversionContext)); + } + + return converted; + } + + if (property != null && !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 + @SuppressWarnings("unchecked") + 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); + + for (Entry entry : valueDbo.entrySet()) { + + String key = entry.getKey(); + if ("$nin".equals(key) || "$in".equals(key)) { + List ids = new ArrayList<>(); + for (Object id : (Iterable) valueDbo.get(key)) { + ids.add(convertId(id, getIdTypeForField(documentField))); + } + resultDbo.put(key, ids); + } else if (isKeyword(key)) { + resultDbo.put(key, convertIdField(documentField, entry.getValue())); + } else { + resultDbo.put(key, getMappedValue(documentField, entry.getValue())); + } + } + + return resultDbo; + } + /** * Checks whether the given value is a {@link Document}. * @@ -1444,7 +1471,6 @@ static class KeyMapper { private final Iterator iterator; private int currentIndex; - private String currentPropertyRoot; private final List pathParts; public KeyMapper(String key, @@ -1452,7 +1478,6 @@ public KeyMapper(String key, this.pathParts = Arrays.asList(key.split("\\.")); this.iterator = pathParts.iterator(); - this.currentPropertyRoot = iterator.next(); this.currentIndex = 0; } @@ -1568,4 +1593,14 @@ public MappingContext, MongoPersistentPropert public MongoConverter getConverter() { return converter; } + + private enum NoPropertyPropertyValueProvider implements PropertyValueProvider { + + INSTANCE; + + @Override + public T getPropertyValue(MongoPersistentProperty property) { + throw new IllegalStateException("No enclosing property source available"); + } + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java index c6c26c9127..357428cd87 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java @@ -21,6 +21,7 @@ import org.springframework.data.annotation.Id; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; /** @@ -191,6 +192,8 @@ enum PropertyToFieldNameConverter implements Converter asCollection(Object source) { 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 f3a94da114..cde5e0ff60 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 @@ -72,6 +72,7 @@ * @author Christoph Strobl * @author Mark Paluch * @author David Julia + * @author Gyungrai Wang */ public class QueryMapperUnitTests { @@ -129,6 +130,80 @@ void handlesObjectIdCapableBigIntegerIdsCorrectly() { assertThat(result).containsEntry("_id", id); } + @Test // GH-4490 + void translates$GtCorrectly() { + + Criteria criteria = where("id").gt(new ObjectId().toString()); + + org.bson.Document query = new org.bson.Document("id", new ObjectId().toString()); + org.bson.Document result = mapper.getMappedObject(criteria.getCriteriaObject(), + context.getPersistentEntity(IdWrapper.class)); + Object object = result.get("_id"); + assertThat(object).isInstanceOf(org.bson.Document.class); + org.bson.Document document = (org.bson.Document) object; + assertThat(document.get("$gt")).isInstanceOf(ObjectId.class); + } + + @Test // GH-4490 + void translates$GteCorrectly() { + + Criteria criteria = where("id").gte(new ObjectId().toString()); + + org.bson.Document query = new org.bson.Document("id", new ObjectId().toString()); + org.bson.Document result = mapper.getMappedObject(criteria.getCriteriaObject(), + context.getPersistentEntity(IdWrapper.class)); + Object object = result.get("_id"); + assertThat(object).isInstanceOf(org.bson.Document.class); + org.bson.Document document = (org.bson.Document) object; + assertThat(document.get("$gte")).isInstanceOf(ObjectId.class); + } + + @Test // GH-4490 + void translates$LteCorrectly() { + + Criteria criteria = where("id").lte(new ObjectId().toString()); + + org.bson.Document query = new org.bson.Document("id", new ObjectId().toString()); + org.bson.Document result = mapper.getMappedObject(criteria.getCriteriaObject(), + context.getPersistentEntity(IdWrapper.class)); + Object object = result.get("_id"); + assertThat(object).isInstanceOf(org.bson.Document.class); + org.bson.Document document = (org.bson.Document) object; + assertThat(document.get("$lte")).isInstanceOf(ObjectId.class); + } + + @Test // GH-4490 + void translates$LtCorrectly() { + + Criteria criteria = where("id").lt(new ObjectId().toString()); + + org.bson.Document query = new org.bson.Document("id", new ObjectId().toString()); + org.bson.Document result = mapper.getMappedObject(criteria.getCriteriaObject(), + context.getPersistentEntity(IdWrapper.class)); + Object object = result.get("_id"); + assertThat(object).isInstanceOf(org.bson.Document.class); + org.bson.Document document = (org.bson.Document) object; + assertThat(document.get("$lt")).isInstanceOf(ObjectId.class); + } + + @Test // GH-4490 + void translatesMultipleCompareOperatorsCorrectly() { + + Criteria criteria = where("id").lt(new ObjectId().toString()).lte(new ObjectId().toString()) + .gt(new ObjectId().toString()).gte(new ObjectId().toString()); + + org.bson.Document query = new org.bson.Document("id", new ObjectId().toString()); + org.bson.Document result = mapper.getMappedObject(criteria.getCriteriaObject(), + context.getPersistentEntity(IdWrapper.class)); + Object object = result.get("_id"); + assertThat(object).isInstanceOf(org.bson.Document.class); + org.bson.Document document = (org.bson.Document) object; + assertThat(document.get("$lt")).isInstanceOf(ObjectId.class); + assertThat(document.get("$lte")).isInstanceOf(ObjectId.class); + assertThat(document.get("$gt")).isInstanceOf(ObjectId.class); + assertThat(document.get("$gte")).isInstanceOf(ObjectId.class); + } + @Test // DATAMONGO-278 void translates$NeCorrectly() { @@ -1528,6 +1603,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 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 c43def1459..76bc967f1b 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.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 fieldNames() { return Stream.of(// Arguments.of(FieldName.path("a"), true), //