DATAMONGO-1070 - Fixed a few glitches in DBRef binding for repository query methods.

The QueryMapping for derived repository queries pointing to the identifier of the referenced document. We now reduce the query field's key from reference.id to reference so that the generated DBRef is applied correctly and also take care that the id's are potentially converted to ObjectIds. This is mainly achieved by using the AssociationConverter pulled up from UpdateMapper in ObjectMapper.getMappedKey().

MongoQueryCreator now refrains from translating the field keys as that will fail the QueryMapper to correctly detect id properties.

Fixed DBRef handling for StringBasedMongoQuery which previously didn't parse the DBRef instance created after JSON parsing for placeholders.

Author: odrotbohm

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java

@@ -334,7 +334,8 @@ protected boolean isAssociationConversionNecessary(Field documentField, Object v
 		}
 
 		MongoPersistentEntity<?> entity = documentField.getPropertyEntity();
-		return entity.hasIdProperty() && entity.getIdProperty().getActualType().isAssignableFrom(type);
+		return entity.hasIdProperty()
+				&& (type.equals(DBRef.class) || entity.getIdProperty().getActualType().isAssignableFrom(type));
 	}
 
 	/**
@@ -382,10 +383,16 @@ protected Object convertAssociation(Object source, Field field) {
 	 */
 	protected Object convertAssociation(Object source, MongoPersistentProperty property) {
 
-		if (property == null || source == null || source instanceof DBRef || source instanceof DBObject) {
+		if (property == null || source == null || source instanceof DBObject) {
 			return source;
 		}
 
+		if (source instanceof DBRef) {
+
+			DBRef ref = (DBRef) source;
+			return new DBRef(ref.getDB(), ref.getRef(), convertId(ref.getId()));
+		}
+
 		if (source instanceof Iterable) {
 			BasicDBList result = new BasicDBList();
 			for (Object element : (Iterable<?>) source) {
@@ -785,7 +792,8 @@ private final Association<MongoPersistentProperty> findAssociation() {
 		 */
 		@Override
 		public String getMappedKey() {
-			return path == null ? name : path.toDotPath(getPropertyConverter());
+			return path == null ? name : path.toDotPath(isAssociation() ? new AssociationConverter(getAssociation())
+					: getPropertyConverter());
 		}
 
 		protected PersistentPropertyPath<MongoPersistentProperty> getPath() {
@@ -838,4 +846,44 @@ protected Converter<MongoPersistentProperty, String> getPropertyConverter() {
 			return PropertyToFieldNameConverter.INSTANCE;
 		}
 	}
+
+	/**
+	 * Converter to skip all properties after an association property was rendered.
+	 * 
+	 * @author Oliver Gierke
+	 */
+	protected static class AssociationConverter implements Converter<MongoPersistentProperty, String> {
+
+		private final MongoPersistentProperty property;
+		private boolean associationFound;
+
+		/**
+		 * Creates a new {@link AssociationConverter} for the given {@link Association}.
+		 * 
+		 * @param association must not be {@literal null}.
+		 */
+		public AssociationConverter(Association<MongoPersistentProperty> association) {
+
+			Assert.notNull(association, "Association must not be null!");
+			this.property = association.getInverse();
+		}
+
+		/* 
+		 * (non-Javadoc)
+		 * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object)
+		 */
+		@Override
+		public String convert(MongoPersistentProperty source) {
+
+			if (associationFound) {
+				return null;
+			}
+
+			if (property.equals(source)) {
+				associationFound = true;
+			}
+
+			return source.getFieldName();
+		}
+	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java

@@ -20,7 +20,6 @@
 import java.util.Map.Entry;
 
 import org.springframework.core.convert.converter.Converter;
-import org.springframework.data.mapping.Association;
 import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
@@ -197,46 +196,6 @@ protected Converter<MongoPersistentProperty, String> getPropertyConverter() {
 			return isAssociation() ? new AssociationConverter(getAssociation()) : new UpdatePropertyConverter(key);
 		}
 
-		/**
-		 * Converter to skip all properties after an association property was rendered.
-		 * 
-		 * @author Oliver Gierke
-		 */
-		private static class AssociationConverter implements Converter<MongoPersistentProperty, String> {
-
-			private final MongoPersistentProperty property;
-			private boolean associationFound;
-
-			/**
-			 * Creates a new {@link AssociationConverter} for the given {@link Association}.
-			 * 
-			 * @param association must not be {@literal null}.
-			 */
-			public AssociationConverter(Association<MongoPersistentProperty> association) {
-
-				Assert.notNull(association, "Association must not be null!");
-				this.property = association.getInverse();
-			}
-
-			/* 
-			 * (non-Javadoc)
-			 * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object)
-			 */
-			@Override
-			public String convert(MongoPersistentProperty source) {
-
-				if (associationFound) {
-					return null;
-				}
-
-				if (property.equals(source)) {
-					associationFound = true;
-				}
-
-				return source.getFieldName();
-			}
-		}
-
 		/**
 		 * Special {@link Converter} for {@link MongoPersistentProperty} instances that will concatenate the {@literal $}
 		 * contained in the source update key.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryCreator.java

@@ -102,9 +102,7 @@ protected Criteria create(Part part, Iterator<Object> iterator) {
 
 		PersistentPropertyPath<MongoPersistentProperty> path = context.getPersistentPropertyPath(part.getProperty());
 		MongoPersistentProperty property = path.getLeafProperty();
-		Criteria criteria = from(part, property,
-				where(path.toDotPath(MongoPersistentProperty.PropertyToFieldNameConverter.INSTANCE)),
-				(PotentiallyConvertingIterator) iterator);
+		Criteria criteria = from(part, property, where(path.toDotPath()), (PotentiallyConvertingIterator) iterator);
 
 		return criteria;
 	}
@@ -123,9 +121,7 @@ protected Criteria and(Part part, Criteria base, Iterator<Object> iterator) {
 		PersistentPropertyPath<MongoPersistentProperty> path = context.getPersistentPropertyPath(part.getProperty());
 		MongoPersistentProperty property = path.getLeafProperty();
 
-		return from(part, property,
-				base.and(path.toDotPath(MongoPersistentProperty.PropertyToFieldNameConverter.INSTANCE)),
-				(PotentiallyConvertingIterator) iterator);
+		return from(part, property, base.and(path.toDotPath()), (PotentiallyConvertingIterator) iterator);
 	}
 
 	/*

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java

@@ -29,6 +29,7 @@
 import org.springframework.util.StringUtils;
 
 import com.mongodb.DBObject;
+import com.mongodb.DBRef;
 import com.mongodb.util.JSON;
 
 /**
@@ -199,6 +200,7 @@ private static enum ParameterBindingParser {
 		 * {@link Collections#emptyList()}.
 		 * 
 		 * @param input
+		 * @param conversionService must not be {@literal null}.
 		 * @return
 		 */
 		public List<ParameterBinding> parseParameterBindingsFrom(String input) {
@@ -229,14 +231,7 @@ private void collectParameterReferencesIntoBindings(List<ParameterBinding> bindi
 			if (value instanceof String) {
 
 				String string = ((String) value).trim();
-
-				Matcher valueMatcher = PARSEABLE_BINDING_PATTERN.matcher(string);
-				while (valueMatcher.find()) {
-					int paramIndex = Integer.parseInt(valueMatcher.group(PARAMETER_INDEX_GROUP));
-					boolean quoted = (string.startsWith("'") && string.endsWith("'"))
-							|| (string.startsWith("\"") && string.endsWith("\""));
-					bindings.add(new ParameterBinding(paramIndex, quoted));
-				}
+				potentiallyAddBinding(string, bindings);
 
 			} else if (value instanceof Pattern) {
 
@@ -255,6 +250,13 @@ private void collectParameterReferencesIntoBindings(List<ParameterBinding> bindi
 					bindings.add(new ParameterBinding(paramIndex, quoted));
 				}
 
+			} else if (value instanceof DBRef) {
+
+				DBRef dbref = (DBRef) value;
+
+				potentiallyAddBinding(dbref.getRef(), bindings);
+				potentiallyAddBinding(dbref.getId().toString(), bindings);
+
 			} else if (value instanceof DBObject) {
 
 				DBObject dbo = (DBObject) value;
@@ -264,6 +266,20 @@ private void collectParameterReferencesIntoBindings(List<ParameterBinding> bindi
 				}
 			}
 		}
+
+		private void potentiallyAddBinding(String source, List<ParameterBinding> bindings) {
+
+			Matcher valueMatcher = PARSEABLE_BINDING_PATTERN.matcher(source);
+
+			while (valueMatcher.find()) {
+
+				int paramIndex = Integer.parseInt(valueMatcher.group(PARAMETER_INDEX_GROUP));
+				boolean quoted = (source.startsWith("'") && source.endsWith("'"))
+						|| (source.startsWith("\"") && source.endsWith("\""));
+
+				bindings.add(new ParameterBinding(paramIndex, quoted));
+			}
+		}
 	}
 
 	/**

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java

@@ -658,6 +658,22 @@ public void getMappedSortIgnoresTextScoreWhenNotSortedByScore() {
 		assertThat(dbo, equalTo(new BasicDBObjectBuilder().add("_id", 1).get()));
 	}
 
+	/**
+	 * @see DATAMONGO-1070
+	 */
+	@Test
+	public void mapsIdReferenceToDBRefCorrectly() {
+
+		ObjectId id = new ObjectId();
+
+		DBObject query = new BasicDBObject("reference.id", new com.mongodb.DBRef(null, "reference", id.toString()));
+		DBObject result = mapper.getMappedObject(query, context.getPersistentEntity(WithDBRef.class));
+
+		assertThat(result.containsField("reference"), is(true));
+		com.mongodb.DBRef reference = getTypedValue(result, "reference", com.mongodb.DBRef.class);
+		assertThat(reference.getId(), is(instanceOf(ObjectId.class)));
+	}
+
 	@Document
 	public class Foo {
 		@Id private ObjectId id;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/MongoQueryCreatorUnitTests.java

@@ -167,19 +167,6 @@ public void createsGreaterThanEqualQueryCorrectly() throws Exception {
 		assertThat(creator.createQuery(), is(reference));
 	}
 
-	/**
-	 * @see DATAMONGO-291
-	 */
-	@Test
-	public void honoursMappingInformationForPropertyPaths() {
-
-		PartTree partTree = new PartTree("findByUsername", User.class);
-
-		MongoQueryCreator creator = new MongoQueryCreator(partTree, getAccessor(converter, "Oliver"), context);
-		Query reference = query(where("foo").is("Oliver"));
-		assertThat(creator.createQuery(), is(reference));
-	}
-
 	/**
 	 * @see DATAMONGO-338
 	 */
@@ -268,7 +255,7 @@ public void createsQueryWithStartingWithPredicateCorrectly() {
 		MongoQueryCreator creator = new MongoQueryCreator(tree, getAccessor(converter, "Matt"), context);
 		Query query = creator.createQuery();
 
-		assertThat(query, is(query(where("foo").regex("^Matt"))));
+		assertThat(query, is(query(where("username").regex("^Matt"))));
 	}
 
 	/**
@@ -281,7 +268,7 @@ public void createsQueryWithEndingWithPredicateCorrectly() {
 		MongoQueryCreator creator = new MongoQueryCreator(tree, getAccessor(converter, "ews"), context);
 		Query query = creator.createQuery();
 
-		assertThat(query, is(query(where("foo").regex("ews$"))));
+		assertThat(query, is(query(where("username").regex("ews$"))));
 	}
 
 	/**
@@ -294,7 +281,7 @@ public void createsQueryWithContainingPredicateCorrectly() {
 		MongoQueryCreator creator = new MongoQueryCreator(tree, getAccessor(converter, "thew"), context);
 		Query query = creator.createQuery();
 
-		assertThat(query, is(query(where("foo").regex(".*thew.*"))));
+		assertThat(query, is(query(where("username").regex(".*thew.*"))));
 	}
 
 	private void assertBindsDistanceToQuery(Point point, Distance distance, Query reference) throws Exception {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java

@@ -29,6 +29,7 @@
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.data.mongodb.core.DBObjectTestUtils;
 import org.springframework.data.mongodb.core.MongoOperations;
 import org.springframework.data.mongodb.core.convert.DbRefResolver;
 import org.springframework.data.mongodb.core.convert.DefaultMongoTypeMapper;
@@ -43,6 +44,7 @@
 
 import com.mongodb.BasicDBObject;
 import com.mongodb.DBObject;
+import com.mongodb.DBRef;
 
 /**
  * Unit tests for {@link StringBasedMongoQuery}.
@@ -255,6 +257,22 @@ public void bindsSimplePropertyWithRegexCorrectly() throws Exception {
 		assertThat(query.getQueryObject(), is(reference.getQueryObject()));
 	}
 
+	/**
+	 * @see DATAMONGO-1070
+	 */
+	@Test
+	public void parsesDbRefDeclarationsCorrectly() throws Exception {
+
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("methodWithManuallyDefinedDbRef", String.class);
+		ConvertingParameterAccessor parameterAccessor = StubParameterAccessor.getAccessor(converter, "myid");
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(parameterAccessor);
+
+		DBRef dbRef = DBObjectTestUtils.getTypedValue(query.getQueryObject(), "reference", DBRef.class);
+		assertThat(dbRef.getId(), is((Object) "myid"));
+		assertThat(dbRef.getRef(), is("reference"));
+	}
+
 	private StringBasedMongoQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
 
 		Method method = SampleRepository.class.getMethod(name, parameters);
@@ -291,7 +309,10 @@ private interface SampleRepository {
 		@Query("{'title': { $regex : '^?0', $options : 'i'}}")
 		List<DBObject> findByTitleBeginsWithExplicitQuoting(String title);
 
-		@Query(value = "{$where: 'return this.date.getUTCMonth() == ?2 && this.date.getUTCDay() == ?3;'}")
+		@Query("{$where: 'return this.date.getUTCMonth() == ?2 && this.date.getUTCDay() == ?3;'}")
 		List<DBObject> findByQueryWithParametersInExpression(int param1, int param2, int param3, int param4);
+
+		@Query("{ 'reference' : { $ref : 'reference', $id : ?0 }}")
+		Object methodWithManuallyDefinedDbRef(String id);
 	}
 }