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.
This commit is contained in:
Oliver Gierke
2014-10-15 10:13:53 +02:00
parent 17c342895a
commit 831d667896
7 changed files with 118 additions and 75 deletions

View File

@@ -334,7 +334,8 @@ public class QueryMapper {
}
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 @@ public class QueryMapper {
*/
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 @@ public class QueryMapper {
*/
@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 @@ public class QueryMapper {
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();
}
}
}

View File

@@ -20,7 +20,6 @@ import java.util.Iterator;
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 @@ public class UpdateMapper extends QueryMapper {
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.

View File

@@ -102,9 +102,7 @@ class MongoQueryCreator extends AbstractQueryCreator<Query, Criteria> {
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 @@ class MongoQueryCreator extends AbstractQueryCreator<Query, Criteria> {
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);
}
/*

View File

@@ -29,6 +29,7 @@ import org.springframework.data.mongodb.core.query.Query;
import org.springframework.util.StringUtils;
import com.mongodb.DBObject;
import com.mongodb.DBRef;
import com.mongodb.util.JSON;
/**
@@ -199,6 +200,7 @@ public class StringBasedMongoQuery extends AbstractMongoQuery {
* {@link Collections#emptyList()}.
*
* @param input
* @param conversionService must not be {@literal null}.
* @return
*/
public List<ParameterBinding> parseParameterBindingsFrom(String input) {
@@ -229,14 +231,7 @@ public class StringBasedMongoQuery extends AbstractMongoQuery {
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 @@ public class StringBasedMongoQuery extends AbstractMongoQuery {
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 @@ public class StringBasedMongoQuery extends AbstractMongoQuery {
}
}
}
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));
}
}
}
/**

View File

@@ -658,6 +658,22 @@ public class QueryMapperUnitTests {
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;

View File

@@ -167,19 +167,6 @@ public class MongoQueryCreatorUnitTests {
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 class MongoQueryCreatorUnitTests {
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 class MongoQueryCreatorUnitTests {
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 class MongoQueryCreatorUnitTests {
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 {

View File

@@ -29,6 +29,7 @@ import org.junit.Test;
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 org.springframework.data.repository.core.RepositoryMetadata;
import com.mongodb.BasicDBObject;
import com.mongodb.DBObject;
import com.mongodb.DBRef;
/**
* Unit tests for {@link StringBasedMongoQuery}.
@@ -255,6 +257,22 @@ public class StringBasedMongoQueryUnitTests {
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 @@ public class StringBasedMongoQueryUnitTests {
@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);
}
}