Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix property value conversion in query mapper for nested values. #4517

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>4.3.0-SNAPSHOT</version>
<version>4.3.x-4510-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data MongoDB</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-benchmarks/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>4.3.0-SNAPSHOT</version>
<version>4.3.x-4510-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-distribution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>4.3.0-SNAPSHOT</version>
<version>4.3.x-4510-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>4.3.0-SNAPSHOT</version>
<version>4.3.x-4510-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@
public class MongoConversionContext implements ValueConversionContext<MongoPersistentProperty> {

private final PropertyValueProvider<MongoPersistentProperty> 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<MongoPersistentProperty> accessor,
MongoPersistentProperty persistentProperty, MongoConverter mongoConverter) {
@Nullable MongoPersistentProperty persistentProperty, MongoConverter mongoConverter) {
this(accessor, persistentProperty, mongoConverter, null);
}

public MongoConversionContext(PropertyValueProvider<MongoPersistentProperty> accessor,
MongoPersistentProperty persistentProperty, MongoConverter mongoConverter, @Nullable SpELContext spELContext) {
@Nullable MongoPersistentProperty persistentProperty, MongoConverter mongoConverter,
@Nullable SpELContext spELContext) {

this.accessor = accessor;
this.persistentProperty = persistentProperty;
Expand All @@ -54,12 +54,17 @@ public MongoConversionContext(PropertyValueProvider<MongoPersistentProperty> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -76,6 +86,7 @@
* @author Mark Paluch
* @author David Julia
* @author Divya Srivastava
* @author Gyungrai Wang
*/
public class QueryMapper {

Expand Down Expand Up @@ -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> 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) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -615,20 +571,20 @@ protected Object convertSimpleOrDocument(Object source, @Nullable MongoPersisten
return source;
}

if (source instanceof Map<?,?> sourceMap) {
if (source instanceof Map<?, ?> sourceMap) {

Map<String, Object> 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;
}
Expand All @@ -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()) {
Expand All @@ -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);
Expand All @@ -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));
}
Expand All @@ -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<Object, Object, ValueConversionContext<MongoPersistentProperty>> 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<Object> 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<String, Object> entry : valueDbo.entrySet()) {

String key = entry.getKey();
if ("$nin".equals(key) || "$in".equals(key)) {
List<Object> 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}.
*
Expand Down Expand Up @@ -1444,15 +1471,13 @@ static class KeyMapper {

private final Iterator<String> iterator;
private int currentIndex;
private String currentPropertyRoot;
private final List<String> pathParts;

public KeyMapper(String key,
MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext) {

this.pathParts = Arrays.asList(key.split("\\."));
this.iterator = pathParts.iterator();
this.currentPropertyRoot = iterator.next();
this.currentIndex = 0;
}

Expand Down Expand Up @@ -1568,4 +1593,14 @@ public MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentPropert
public MongoConverter getConverter() {
return converter;
}

private enum NoPropertyPropertyValueProvider implements PropertyValueProvider<MongoPersistentProperty> {

INSTANCE;

@Override
public <T> T getPropertyValue(MongoPersistentProperty property) {
throw new IllegalStateException("No enclosing property source available");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -191,6 +192,8 @@ enum PropertyToFieldNameConverter implements Converter<MongoPersistentProperty,

INSTANCE;

@NonNull
@Override
public String convert(MongoPersistentProperty source) {
if (!source.isUnwrapped()) {
return source.getFieldName();
Expand Down
Loading