DATAMONGO-404 - Polishing of DBRef creation in Update clauses.
Refactored the internals of UpdateMapper to simplify the code a little. Removed the special converter in favor of handling the mapped key generation directly. This can be removed again, once DATACMNS-444 is fixed. MetadataBackedField.getPath(String) now also rejects PersistentPropertyPaths the refer to anything else but the id property in case it traverses an association. Changed MetadataBackedField to return the association property in calls to ….getProperty() as it is the PersistentProperty to hand to the mapping infrastructure for object conversion. Changed MappingMongoConverter to also check, whether the given source object handed into DBRef creation is of the ID type and simply use that for DBRef creation. This allows creating DBRefs from ids as well.
This commit is contained in:
@@ -304,7 +304,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
Assert.isTrue(annotation != null, "The referenced property has to be mapped with @DBRef!");
|
||||
}
|
||||
|
||||
return createDBRef(object, annotation);
|
||||
return createDBRef(object, referingProperty);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -447,7 +447,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
}
|
||||
|
||||
if (prop.isDbReference()) {
|
||||
DBRef dbRefObj = createDBRef(obj, prop.getDBRef());
|
||||
DBRef dbRefObj = createDBRef(obj, prop);
|
||||
if (null != dbRefObj) {
|
||||
accessor.put(prop, dbRefObj);
|
||||
return;
|
||||
@@ -516,7 +516,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
continue;
|
||||
}
|
||||
|
||||
DBRef dbRef = createDBRef(element, property.getDBRef());
|
||||
DBRef dbRef = createDBRef(element, property);
|
||||
dbList.add(dbRef);
|
||||
}
|
||||
|
||||
@@ -549,7 +549,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
if (conversions.isSimpleType(key.getClass())) {
|
||||
|
||||
String simpleKey = potentiallyEscapeMapKey(key.toString());
|
||||
dbObject.put(simpleKey, value != null ? createDBRef(value, property.getDBRef()) : null);
|
||||
dbObject.put(simpleKey, value != null ? createDBRef(value, property) : null);
|
||||
|
||||
} else {
|
||||
throw new MappingException("Cannot use a complex object as a key value.");
|
||||
@@ -742,7 +742,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
return target.isAssignableFrom(value.getClass()) ? value : conversionService.convert(value, target);
|
||||
}
|
||||
|
||||
protected DBRef createDBRef(Object target, org.springframework.data.mongodb.core.mapping.DBRef dbref) {
|
||||
protected DBRef createDBRef(Object target, MongoPersistentProperty property) {
|
||||
|
||||
Assert.notNull(target);
|
||||
|
||||
@@ -751,6 +751,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
}
|
||||
|
||||
MongoPersistentEntity<?> targetEntity = mappingContext.getPersistentEntity(target.getClass());
|
||||
targetEntity = targetEntity == null ? targetEntity = mappingContext.getPersistentEntity(property) : targetEntity;
|
||||
|
||||
if (null == targetEntity) {
|
||||
throw new MappingException("No mapping metadata found for " + target.getClass());
|
||||
@@ -762,14 +763,21 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
|
||||
throw new MappingException("No id property found on class " + targetEntity.getType());
|
||||
}
|
||||
|
||||
BeanWrapper<MongoPersistentEntity<Object>, Object> wrapper = BeanWrapper.create(target, conversionService);
|
||||
Object id = wrapper.getProperty(idProperty, Object.class, useFieldAccessOnly);
|
||||
Object id = null;
|
||||
|
||||
if (target.getClass().equals(idProperty.getType())) {
|
||||
id = target;
|
||||
} else {
|
||||
BeanWrapper<MongoPersistentEntity<Object>, Object> wrapper = BeanWrapper.create(target, conversionService);
|
||||
id = wrapper.getProperty(idProperty, Object.class, useFieldAccessOnly);
|
||||
}
|
||||
|
||||
if (null == id) {
|
||||
throw new MappingException("Cannot create a reference to an object with a NULL id.");
|
||||
}
|
||||
|
||||
return dbRefResolver.createDbRef(dbref, targetEntity, idMapper.convertId(id));
|
||||
return dbRefResolver.createDbRef(property == null ? null : property.getDBRef(), targetEntity,
|
||||
idMapper.convertId(id));
|
||||
}
|
||||
|
||||
protected Object getValueInternal(MongoPersistentProperty prop, DBObject dbo, SpELExpressionEvaluator eval,
|
||||
|
||||
@@ -18,6 +18,7 @@ package org.springframework.data.mongodb.core.convert;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Map.Entry;
|
||||
import java.util.Set;
|
||||
@@ -32,6 +33,7 @@ import org.springframework.data.mapping.PropertyPath;
|
||||
import org.springframework.data.mapping.PropertyReferenceException;
|
||||
import org.springframework.data.mapping.context.MappingContext;
|
||||
import org.springframework.data.mapping.context.PersistentPropertyPath;
|
||||
import org.springframework.data.mapping.model.MappingException;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty.PropertyToFieldNameConverter;
|
||||
@@ -301,7 +303,7 @@ public class QueryMapper {
|
||||
* @param property
|
||||
* @return
|
||||
*/
|
||||
private Object convertAssociation(Object source, MongoPersistentProperty property) {
|
||||
protected Object convertAssociation(Object source, MongoPersistentProperty property) {
|
||||
|
||||
if (property == null || source == null || source instanceof DBRef) {
|
||||
return source;
|
||||
@@ -481,7 +483,9 @@ public class QueryMapper {
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the underlying {@link MongoPersistentProperty} backing the field.
|
||||
* Returns the underlying {@link MongoPersistentProperty} backing the field. For path traversals this will be the
|
||||
* property that represents the value to handle. This means it'll be the leaf property for plain paths or the
|
||||
* association property in case we refer to an association somewhere in the path.
|
||||
*
|
||||
* @return
|
||||
*/
|
||||
@@ -517,20 +521,15 @@ public class QueryMapper {
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if property is part of database reference.
|
||||
* Returns whether the field references an association in case it refers to a nested field.
|
||||
*
|
||||
* @return
|
||||
*/
|
||||
public boolean isPartOfAssociation() {
|
||||
public boolean containsAssociation() {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get {@link Association} pointing to property
|
||||
*
|
||||
* @return null if not available
|
||||
*/
|
||||
public Association<MongoPersistentProperty> findAssociation() {
|
||||
public Association<MongoPersistentProperty> getAssociation() {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -543,10 +542,13 @@ public class QueryMapper {
|
||||
*/
|
||||
protected static class MetadataBackedField extends Field {
|
||||
|
||||
private static final String INVALID_ASSOCIATION_REFERENCE = "Invalid path reference %s! Associations can only be pointed to directly or via their id property!";
|
||||
|
||||
private final MongoPersistentEntity<?> entity;
|
||||
private final MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext;
|
||||
private final MongoPersistentProperty property;
|
||||
private final PersistentPropertyPath<MongoPersistentProperty> path;
|
||||
private final Association<MongoPersistentProperty> association;
|
||||
|
||||
/**
|
||||
* Creates a new {@link MetadataBackedField} with the given name, {@link MongoPersistentEntity} and
|
||||
@@ -568,6 +570,7 @@ public class QueryMapper {
|
||||
|
||||
this.path = getPath(name);
|
||||
this.property = path == null ? null : path.getLeafProperty();
|
||||
this.association = findAssociation();
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -601,7 +604,7 @@ public class QueryMapper {
|
||||
*/
|
||||
@Override
|
||||
public MongoPersistentProperty getProperty() {
|
||||
return property;
|
||||
return association == null ? property : association.getInverse();
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -620,36 +623,33 @@ public class QueryMapper {
|
||||
*/
|
||||
@Override
|
||||
public boolean isAssociation() {
|
||||
|
||||
MongoPersistentProperty property = getProperty();
|
||||
return property == null ? false : property.isAssociation();
|
||||
return association != null;
|
||||
}
|
||||
|
||||
/*
|
||||
/*
|
||||
* (non-Javadoc)
|
||||
* @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#isPartOfAssociation()
|
||||
* @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#getAssociation()
|
||||
*/
|
||||
@Override
|
||||
public boolean isPartOfAssociation() {
|
||||
return findAssociation() != null;
|
||||
public Association<MongoPersistentProperty> getAssociation() {
|
||||
return association;
|
||||
}
|
||||
|
||||
/*
|
||||
* (non-Javadoc)
|
||||
* @see org.springframework.data.mongodb.core.convert.QueryMapper.Field#findAssociation()
|
||||
/**
|
||||
* Finds the association property in the {@link PersistentPropertyPath}.
|
||||
*
|
||||
* @return
|
||||
*/
|
||||
@Override
|
||||
public Association<MongoPersistentProperty> findAssociation() {
|
||||
if (isAssociation()) {
|
||||
return property.getAssociation();
|
||||
}
|
||||
private final Association<MongoPersistentProperty> findAssociation() {
|
||||
|
||||
if (this.path != null) {
|
||||
for (MongoPersistentProperty p : this.path) {
|
||||
if (p != null && p.isAssociation()) {
|
||||
if (p.isAssociation()) {
|
||||
return p.getAssociation();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -662,6 +662,10 @@ public class QueryMapper {
|
||||
return path == null ? name : path.toDotPath(getPropertyConverter());
|
||||
}
|
||||
|
||||
protected PersistentPropertyPath<MongoPersistentProperty> getPath() {
|
||||
return path;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the {@link PersistentPropertyPath} for the given <code>pathExpression</code>.
|
||||
*
|
||||
@@ -671,8 +675,28 @@ public class QueryMapper {
|
||||
private PersistentPropertyPath<MongoPersistentProperty> getPath(String pathExpression) {
|
||||
|
||||
try {
|
||||
|
||||
PropertyPath path = PropertyPath.from(pathExpression, entity.getTypeInformation());
|
||||
return mappingContext.getPersistentPropertyPath(path);
|
||||
PersistentPropertyPath<MongoPersistentProperty> propertyPath = mappingContext.getPersistentPropertyPath(path);
|
||||
|
||||
Iterator<MongoPersistentProperty> iterator = propertyPath.iterator();
|
||||
boolean associationDetected = false;
|
||||
|
||||
while (iterator.hasNext()) {
|
||||
|
||||
MongoPersistentProperty property = iterator.next();
|
||||
|
||||
if (property.isAssociation()) {
|
||||
associationDetected = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (associationDetected && !property.isIdProperty()) {
|
||||
throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression));
|
||||
}
|
||||
}
|
||||
|
||||
return propertyPath;
|
||||
} catch (PropertyReferenceException e) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -22,7 +22,6 @@ import java.util.Map.Entry;
|
||||
|
||||
import org.springframework.core.convert.converter.Converter;
|
||||
import org.springframework.data.mapping.Association;
|
||||
import org.springframework.data.mapping.PersistentEntity;
|
||||
import org.springframework.data.mapping.context.MappingContext;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
|
||||
@@ -34,7 +33,6 @@ import org.springframework.util.Assert;
|
||||
|
||||
import com.mongodb.BasicDBObject;
|
||||
import com.mongodb.DBObject;
|
||||
import com.mongodb.DBRef;
|
||||
|
||||
/**
|
||||
* A subclass of {@link QueryMapper} that retains type information on the mongo types.
|
||||
@@ -105,25 +103,13 @@ public class UpdateMapper extends QueryMapper {
|
||||
return Collections.singletonMap(field.getMappedKey(), value).entrySet().iterator().next();
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Object convertAssociation(Object source, Field field) {
|
||||
|
||||
if (source instanceof DBRef || field.isAssociation() || !field.isPartOfAssociation()) {
|
||||
return super.convertAssociation(source, field);
|
||||
}
|
||||
|
||||
MongoPersistentProperty property = field.getProperty();
|
||||
PersistentEntity<?, MongoPersistentProperty> owner = property.getOwner();
|
||||
if (owner instanceof MongoPersistentEntity) {
|
||||
return new DBRef(null, ((MongoPersistentEntity<?>) owner).getCollection(), source);
|
||||
}
|
||||
|
||||
throw new IllegalArgumentException(String.format("Expected MongoPeristentEntity but found '%s'.", owner.getClass()));
|
||||
}
|
||||
|
||||
/*
|
||||
* (non-Javadoc)
|
||||
* @see org.springframework.data.mongodb.core.convert.QueryMapper#isAssociationConversionNecessary(org.springframework.data.mongodb.core.convert.QueryMapper.Field, java.lang.Object)
|
||||
*/
|
||||
@Override
|
||||
protected boolean isAssociationConversionNecessary(Field documentField, Object value) {
|
||||
return super.isAssociationConversionNecessary(documentField, value) || documentField.isPartOfAssociation();
|
||||
return super.isAssociationConversionNecessary(documentField, value) || documentField.containsAssociation();
|
||||
}
|
||||
|
||||
private boolean isUpdateModifier(Object value) {
|
||||
@@ -183,39 +169,47 @@ public class UpdateMapper extends QueryMapper {
|
||||
*/
|
||||
@Override
|
||||
protected Converter<MongoPersistentProperty, String> getPropertyConverter() {
|
||||
|
||||
if (this.isPartOfAssociation()) {
|
||||
return new AssociationPropertyConverter(key, this.findAssociation());
|
||||
}
|
||||
return new UpdatePropertyConverter(key);
|
||||
return isAssociation() ? new AssociationConverter(getAssociation()) : new UpdatePropertyConverter(key);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getMappedKey() {
|
||||
/**
|
||||
* Converter to skip all properties after an association property was rendered.
|
||||
*
|
||||
* @author Oliver Gierke
|
||||
*/
|
||||
private static class AssociationConverter implements Converter<MongoPersistentProperty, String> {
|
||||
|
||||
if (isPartOfAssociation()) {
|
||||
return getPropertyConverter().convert(null);
|
||||
}
|
||||
return super.getMappedKey();
|
||||
}
|
||||
private final MongoPersistentProperty property;
|
||||
private boolean associationFound;
|
||||
|
||||
private static class AssociationPropertyConverter implements Converter<MongoPersistentProperty, String> {
|
||||
/**
|
||||
* Creates a new {@link AssociationConverter} for the given {@link Association}.
|
||||
*
|
||||
* @param association must not be {@literal null}.
|
||||
*/
|
||||
public AssociationConverter(Association<MongoPersistentProperty> association) {
|
||||
|
||||
Association<MongoPersistentProperty> association;
|
||||
String key;
|
||||
|
||||
public AssociationPropertyConverter(String key, Association<MongoPersistentProperty> field) {
|
||||
this.key = key;
|
||||
this.association = field;
|
||||
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) {
|
||||
String fieldName = association.getInverse().getFieldName();
|
||||
String newKey = key.substring(0, key.indexOf(fieldName) + fieldName.length());
|
||||
return newKey;
|
||||
}
|
||||
|
||||
if (associationFound) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (property.equals(source)) {
|
||||
associationFound = true;
|
||||
}
|
||||
|
||||
return source.getFieldName();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -58,6 +58,7 @@ import org.springframework.data.mongodb.core.convert.DBObjectAccessorUnitTests.P
|
||||
import org.springframework.data.mongodb.core.mapping.Document;
|
||||
import org.springframework.data.mongodb.core.mapping.Field;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
|
||||
import org.springframework.data.mongodb.core.mapping.PersonPojoStringId;
|
||||
import org.springframework.data.util.ClassTypeInformation;
|
||||
import org.springframework.test.util.ReflectionTestUtils;
|
||||
@@ -1243,9 +1244,9 @@ public class MappingMongoConverterUnitTests {
|
||||
DB db = mock(DB.class);
|
||||
DBRef dbRef = new DBRef(db, "collection", "id");
|
||||
|
||||
org.springframework.data.mongodb.core.mapping.DBRef annotation = mock(org.springframework.data.mongodb.core.mapping.DBRef.class);
|
||||
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
|
||||
|
||||
assertThat(converter.createDBRef(dbRef, annotation), is(dbRef));
|
||||
assertThat(converter.createDBRef(dbRef, property), is(dbRef));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -32,7 +32,9 @@ import org.mockito.Mock;
|
||||
import org.mockito.Mockito;
|
||||
import org.mockito.runners.MockitoJUnitRunner;
|
||||
import org.springframework.core.convert.converter.Converter;
|
||||
import org.springframework.data.annotation.Id;
|
||||
import org.springframework.data.convert.WritingConverter;
|
||||
import org.springframework.data.mapping.model.MappingException;
|
||||
import org.springframework.data.mongodb.MongoDbFactory;
|
||||
import org.springframework.data.mongodb.core.mapping.Field;
|
||||
import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
|
||||
@@ -41,6 +43,7 @@ import org.springframework.data.mongodb.core.query.Update;
|
||||
import com.mongodb.BasicDBList;
|
||||
import com.mongodb.BasicDBObject;
|
||||
import com.mongodb.DBObject;
|
||||
import com.mongodb.DBRef;
|
||||
|
||||
/**
|
||||
* Unit tests for {@link UpdateMapper}.
|
||||
@@ -300,6 +303,62 @@ public class UpdateMapperUnitTests {
|
||||
verify(writingConverterSpy, times(3)).convert(Mockito.any(NestedEntity.class));
|
||||
}
|
||||
|
||||
/**
|
||||
* @see DATAMONGO-404
|
||||
*/
|
||||
@Test
|
||||
public void createsDbRefForEntityIdOnPulls() {
|
||||
|
||||
Update update = new Update().pull("dbRefAnnotatedList.id", "2");
|
||||
|
||||
DBObject mappedObject = mapper.getMappedObject(update.getUpdateObject(),
|
||||
context.getPersistentEntity(DocumentWithDBRefCollection.class));
|
||||
|
||||
DBObject pullClause = getAsDBObject(mappedObject, "$pull");
|
||||
assertThat(pullClause.get("dbRefAnnotatedList"), is((Object) new DBRef(null, "entity", "2")));
|
||||
}
|
||||
|
||||
/**
|
||||
* @see DATAMONGO-404
|
||||
*/
|
||||
@Test
|
||||
public void createsDbRefForEntityOnPulls() {
|
||||
|
||||
Entity entity = new Entity();
|
||||
entity.id = "5";
|
||||
|
||||
Update update = new Update().pull("dbRefAnnotatedList", entity);
|
||||
DBObject mappedObject = mapper.getMappedObject(update.getUpdateObject(),
|
||||
context.getPersistentEntity(DocumentWithDBRefCollection.class));
|
||||
|
||||
DBObject pullClause = getAsDBObject(mappedObject, "$pull");
|
||||
assertThat(pullClause.get("dbRefAnnotatedList"), is((Object) new DBRef(null, "entity", entity.id)));
|
||||
}
|
||||
|
||||
/**
|
||||
* @see DATAMONGO-404
|
||||
*/
|
||||
@Test(expected = MappingException.class)
|
||||
public void rejectsInvalidFieldReferenceForDbRef() {
|
||||
|
||||
Update update = new Update().pull("dbRefAnnotatedList.name", "NAME");
|
||||
mapper.getMappedObject(update.getUpdateObject(), context.getPersistentEntity(DocumentWithDBRefCollection.class));
|
||||
}
|
||||
|
||||
/**
|
||||
* @see DATAMONGO-404
|
||||
*/
|
||||
@Test
|
||||
public void rendersNestedDbRefCorrectly() {
|
||||
|
||||
Update update = new Update().pull("nested.dbRefAnnotatedList.id", "2");
|
||||
DBObject mappedObject = mapper
|
||||
.getMappedObject(update.getUpdateObject(), context.getPersistentEntity(Wrapper.class));
|
||||
|
||||
DBObject pullClause = getAsDBObject(mappedObject, "$pull");
|
||||
assertThat(pullClause.containsField("mapped.dbRefAnnotatedList"), is(true));
|
||||
}
|
||||
|
||||
static interface Model {}
|
||||
|
||||
static class ModelImpl implements Model {
|
||||
@@ -385,4 +444,23 @@ public class UpdateMapperUnitTests {
|
||||
return new BasicDBObject();
|
||||
}
|
||||
}
|
||||
|
||||
static class DocumentWithDBRefCollection {
|
||||
|
||||
@Id public String id;
|
||||
|
||||
@org.springframework.data.mongodb.core.mapping.DBRef//
|
||||
public List<Entity> dbRefAnnotatedList;
|
||||
}
|
||||
|
||||
static class Entity {
|
||||
|
||||
@Id public String id;
|
||||
String name;
|
||||
}
|
||||
|
||||
static class Wrapper {
|
||||
|
||||
@Field("mapped") DocumentWithDBRefCollection nested;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user