From 47fe45662ed4e8151b0e352b50d77d7392ed6662 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 1 Jun 2021 10:11:52 +0200 Subject: [PATCH 1/3] Prepare issue branch --- pom.xml | 2 +- spring-data-mongodb-benchmarks/pom.xml | 2 +- spring-data-mongodb-distribution/pom.xml | 2 +- spring-data-mongodb/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index a6d5da9170..a72df9d2d3 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3659-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 0033bd11d5..d5244334e3 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 - 3.3.0-SNAPSHOT + 3.3.0-GH-3659-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index f62c8dc7f4..329591b64e 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3659-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index c1efaea420..39e376cfc7 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3659-SNAPSHOT ../pom.xml From cf3e850de603cc39e2631f6499183cea4602a5e7 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 31 May 2021 10:22:03 +0200 Subject: [PATCH 2/3] 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 --- .../mongodb/core/convert/QueryMapper.java | 59 +++++++++++++------ .../core/convert/QueryMapperUnitTests.java | 51 +++++++++++++--- 2 files changed, 83 insertions(+), 27 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 81c1c96ddf..f06cc52ce8 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.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.annotation.Reference; @@ -69,6 +72,8 @@ */ 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); @@ -673,7 +678,8 @@ private Object createReferenceFor(Object source, MongoPersistentProperty propert return (DBRef) source; } - if(property != null && (property.isDocumentReference() || (!property.isDbReference() && property.findAnnotation(Reference.class) != null))) { + if (property != null && (property.isDocumentReference() + || (!property.isDbReference() && property.findAnnotation(Reference.class) != null))) { return converter.toDocumentPointer(source, property).getPointer(); } @@ -1174,8 +1180,8 @@ private PersistentPropertyPath getPath(String pathExpre removePlaceholders(DOT_POSITIONAL_PATTERN, pathExpression)); if (sourceProperty != null && sourceProperty.getOwner().equals(entity)) { - return mappingContext - .getPersistentPropertyPath(PropertyPath.from(Pattern.quote(sourceProperty.getName()), entity.getTypeInformation())); + return mappingContext.getPersistentPropertyPath( + PropertyPath.from(Pattern.quote(sourceProperty.getName()), entity.getTypeInformation())); } PropertyPath path = forName(rawPath); @@ -1183,29 +1189,46 @@ private PersistentPropertyPath getPath(String pathExpre 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 f7b5ec76d7..0cf8b3498a 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.Map; import java.util.Optional; +import lombok.Data; import org.bson.conversions.Bson; import org.bson.types.Code; import org.bson.types.ObjectId; @@ -35,9 +36,10 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; 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 +54,7 @@ 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.mapping.Unwrapped; import org.springframework.data.mongodb.core.query.BasicQuery; @@ -1255,6 +1258,26 @@ void resolvesFieldNameWithUnderscoreOnNestedMappedFieldnameWithUnderscoresCorrec assertThat(document).isEqualTo(new org.bson.Document("double_underscore.renamed", new org.bson.Document("$exists", true))); } + @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; @@ -1486,17 +1509,27 @@ static class WithPropertyUsingUnderscoreInName { String renamed_fieldname_with_underscores; } - static class WithDocumentReferences { + @Document + static class Customer { - @DocumentReference - Sample sample; + @Id + private ObjectId id; + private String name; + private MyAddress address; + } - @DocumentReference - SimpeEntityWithoutId noId; + static class MyAddress { + private String street; + } - @DocumentReference(lookup = "{ 'stringProperty' : ?#{stringProperty} }") - SimpeEntityWithoutId noIdButLookupQuery; + @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; + } } - } From 9e83fbf2689ca3e2e6862964917d1678c8ef1664 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 1 Jun 2021 09:25:15 +0200 Subject: [PATCH 3/3] Polishing. Fix typo in class name and make sure MongoTestTemplate uses the configured simple types. Remove superfluous junit extension. --- .../mongodb/core/convert/QueryMapperUnitTests.java | 10 +++------- .../test/util/MongoTestTemplateConfiguration.java | 3 +++ 2 files changed, 6 insertions(+), 7 deletions(-) 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 0cf8b3498a..0132a705d7 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,7 +28,6 @@ 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; @@ -49,12 +48,10 @@ import org.springframework.data.mongodb.core.geo.GeoJsonPolygon; import org.springframework.data.mongodb.core.mapping.DBRef; import org.springframework.data.mongodb.core.mapping.Document; -import org.springframework.data.mongodb.core.mapping.DocumentReference; 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.mapping.Unwrapped; import org.springframework.data.mongodb.core.query.BasicQuery; @@ -75,7 +72,6 @@ * @author Christoph Strobl * @author Mark Paluch */ -@ExtendWith(MockitoExtension.class) public class QueryMapperUnitTests { private QueryMapper mapper; @@ -1417,18 +1413,18 @@ static class ClassWithGeoTypes { @Field("geoJsonPointWithNameViaFieldAnnotation") GeoJsonPoint namedGeoJsonPoint; } - static class SimpeEntityWithoutId { + static class SimpleEntityWithoutId { String stringProperty; Integer integerProperty; } static class EntityWithComplexValueTypeMap { - Map map; + Map map; } static class EntityWithComplexValueTypeList { - List list; + List list; } static class WithExplicitTargetTypes { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplateConfiguration.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplateConfiguration.java index 0f90bd2b9c..2d2dedc2ee 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplateConfiguration.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplateConfiguration.java @@ -119,6 +119,9 @@ MongoMappingContext mappingContext() { mappingContext = new MongoMappingContext(); mappingContext.setInitialEntitySet(mappingContextConfigurer.initialEntitySet()); mappingContext.setAutoIndexCreation(mappingContextConfigurer.autocreateIndex); + if(mongoConverterConfigurer.customConversions != null) { + mappingContext.setSimpleTypeHolder(mongoConverterConfigurer.customConversions.getSimpleTypeHolder()); + } mappingContext.afterPropertiesSet(); }