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.

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.annotation.Reference;
@@ -69,6 +72,8 @@ import com.mongodb.DBRef;
  */
 public class QueryMapper {
 
+	protected static final Logger LOGGER = LoggerFactory.getLogger(QueryMapper.class);
+
 	private static final List<String> 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);
@@ -677,7 +682,8 @@ public class QueryMapper {
 			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();
 		}
 
@@ -1178,8 +1184,8 @@ public class QueryMapper {
 					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);
@@ -1187,29 +1193,46 @@ public class QueryMapper {
 				return null;
 			}
 
-			try {
+			PersistentPropertyPath<MongoPersistentProperty> propertyPath = tryToResolvePersistentPropertyPath(path);
 
-				PersistentPropertyPath<MongoPersistentProperty> propertyPath = mappingContext.getPersistentPropertyPath(path);
+			if (propertyPath == null) {
 
-				Iterator<MongoPersistentProperty> iterator = propertyPath.iterator();
-				boolean associationDetected = false;
+				if (QueryMapper.LOGGER.isInfoEnabled()) {
 
-				while (iterator.hasNext()) {
+					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;
+			}
 
-					MongoPersistentProperty property = iterator.next();
+			Iterator<MongoPersistentProperty> iterator = propertyPath.iterator();
+			boolean associationDetected = false;
 
-					if (property.isAssociation()) {
-						associationDetected = true;
-						continue;
-					}
+			while (iterator.hasNext()) {
 
-					if (associationDetected && !property.isIdProperty()) {
-						throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression));
-					}
+				MongoPersistentProperty property = iterator.next();
+
+				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<MongoPersistentProperty> tryToResolvePersistentPropertyPath(PropertyPath path) {
+
+			try {
+				return mappingContext.getPersistentPropertyPath(path);
+			} catch (MappingException e) {
 				return null;
 			}
 		}

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;
@@ -35,9 +36,10 @@ import org.junit.jupiter.api.BeforeEach;
 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.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;
@@ -1285,6 +1288,26 @@ 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<WithNestedArray> level0;
@@ -1516,17 +1539,27 @@ public class QueryMapperUnitTests {
 		String renamed_fieldname_with_underscores;
 	}
 
-	static class WithDocumentReferences {
-
-		@DocumentReference
-		Sample sample;
-
-		@DocumentReference
-		SimpeEntityWithoutId noId;
-
-		@DocumentReference(lookup = "{ 'stringProperty' : ?#{stringProperty} }")
-		SimpeEntityWithoutId noIdButLookupQuery;
+	@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<MyAddress, org.bson.Document> {
+
+		@Override
+		public org.bson.Document convert(MyAddress address) {
+			org.bson.Document doc = new org.bson.Document();
+			doc.put("street", address.street);
+			return doc;
+		}
+	}
 }