From 01141502a01d6365dd2b0bb49c3946afb8199dfd Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 31 May 2021 10:22:03 +0200 Subject: [PATCH] Fix query mapper path resolution for types considered simple ones. spring-projects/spring-data-commons#2293 changed how PersistentProperty paths get resolved and considers potentially registered converters for those, which made the path resolution fail in during the query mapping process. This commit makes sure to capture the according exception and continue with the given user input. Fixes: #3659 Original pull request: #3661. --- .../mongodb/core/convert/QueryMapper.java | 52 +++++++++++++------ .../core/convert/QueryMapperUnitTests.java | 50 ++++++++++++++++++ 2 files changed, 87 insertions(+), 15 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 739a33ff6..3fb89dea6 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 @@ -19,11 +19,14 @@ import java.util.*; import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.bson.BsonValue; import org.bson.Document; import org.bson.conversions.Bson; import org.bson.types.ObjectId; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.Converter; import org.springframework.data.domain.Example; @@ -66,6 +69,8 @@ import com.mongodb.DBRef; */ public class QueryMapper { + protected static final Logger LOGGER = LoggerFactory.getLogger(QueryMapper.class); + private static final List DEFAULT_ID_NAMES = Arrays.asList("id", "_id"); private static final Document META_TEXT_SCORE = new Document("$meta", "textScore"); static final ClassTypeInformation NESTED_DOCUMENT = ClassTypeInformation.from(NestedDocument.class); @@ -1099,29 +1104,46 @@ public class QueryMapper { return null; } - try { + PersistentPropertyPath propertyPath = tryToResolvePersistentPropertyPath(path); - PersistentPropertyPath propertyPath = mappingContext.getPersistentPropertyPath(path); + if (propertyPath == null) { - Iterator iterator = propertyPath.iterator(); - boolean associationDetected = false; + if (QueryMapper.LOGGER.isInfoEnabled()) { + + String types = StringUtils.collectionToDelimitedString( + path.stream().map(it -> it.getType().getSimpleName()).collect(Collectors.toList()), " -> "); + QueryMapper.LOGGER.info( + "Could not map '{}'. Maybe a fragment in '{}' is considered a simple type. Mapper continues with {}.", + path, types, pathExpression); + } + return null; + } - while (iterator.hasNext()) { + Iterator iterator = propertyPath.iterator(); + boolean associationDetected = false; - MongoPersistentProperty property = iterator.next(); + while (iterator.hasNext()) { - if (property.isAssociation()) { - associationDetected = true; - continue; - } + MongoPersistentProperty property = iterator.next(); - if (associationDetected && !property.isIdProperty()) { - throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression)); - } + if (property.isAssociation()) { + associationDetected = true; + continue; } - return propertyPath; - } catch (InvalidPersistentPropertyPath e) { + if (associationDetected && !property.isIdProperty()) { + throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression)); + } + } + + return propertyPath; + } + + private PersistentPropertyPath tryToResolvePersistentPropertyPath(PropertyPath path) { + + try { + return mappingContext.getPersistentPropertyPath(path); + } catch (MappingException e) { return null; } } 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 d1d0aaa62..0e6df2776 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 @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import lombok.Data; import org.bson.conversions.Bson; import org.bson.types.Code; import org.bson.types.ObjectId; @@ -36,7 +37,10 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; +import org.springframework.data.annotation.Transient; +import org.springframework.data.convert.WritingConverter; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.geo.Point; @@ -52,6 +56,7 @@ import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.mapping.FieldType; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import org.springframework.data.mongodb.core.mapping.TextScore; import org.springframework.data.mongodb.core.query.BasicQuery; import org.springframework.data.mongodb.core.query.Criteria; @@ -1101,6 +1106,28 @@ public class QueryMapperUnitTests { .isThrownBy(() -> mapper.getMappedObject(query.getQueryObject(), context.getPersistentEntity(Foo.class))); } + @Test // GH-3659 + void allowsUsingFieldPathsForPropertiesHavingCustomConversionRegistered() { + + Query query = query(where("address.street").is("1007 Mountain Drive")); + + MongoCustomConversions mongoCustomConversions = new MongoCustomConversions( + Collections.singletonList(new MyAddressToDocumentConverter())); + + this.context = new MongoMappingContext(); + this.context.setSimpleTypeHolder(mongoCustomConversions.getSimpleTypeHolder()); + this.context.afterPropertiesSet(); + + this.converter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, context); + this.converter.setCustomConversions(mongoCustomConversions); + this.converter.afterPropertiesSet(); + + this.mapper = new QueryMapper(converter); + + assertThat(mapper.getMappedSort(query.getQueryObject(), context.getPersistentEntity(Customer.class))) + .isEqualTo(new org.bson.Document("address.street", "1007 Mountain Drive")); + } + class WithDeepArrayNesting { List level0; @@ -1297,4 +1324,27 @@ public class QueryMapperUnitTests { @Field("renamed") String renamed_fieldname_with_underscores; } + + @Document + static class Customer { + + @Id private ObjectId id; + private String name; + private MyAddress address; + } + + static class MyAddress { + private String street; + } + + @WritingConverter + public static class MyAddressToDocumentConverter implements Converter { + + @Override + public org.bson.Document convert(MyAddress address) { + org.bson.Document doc = new org.bson.Document(); + doc.put("street", address.street); + return doc; + } + } }