diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index b73302f23..9da46304c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -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, 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, 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, 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 ffa7da5ff..a0e07a13c 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 @@ -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 findAssociation() { + public Association 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, MongoPersistentProperty> mappingContext; private final MongoPersistentProperty property; private final PersistentPropertyPath path; + private final Association 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 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 findAssociation() { - if (isAssociation()) { - return property.getAssociation(); - } + private final Association 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 getPath() { + return path; + } + /** * Returns the {@link PersistentPropertyPath} for the given pathExpression. * @@ -671,8 +675,28 @@ public class QueryMapper { private PersistentPropertyPath getPath(String pathExpression) { try { + PropertyPath path = PropertyPath.from(pathExpression, entity.getTypeInformation()); - return mappingContext.getPersistentPropertyPath(path); + PersistentPropertyPath propertyPath = mappingContext.getPersistentPropertyPath(path); + + Iterator 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; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java index 81ec716b1..2e687a282 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java @@ -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 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 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 { - if (isPartOfAssociation()) { - return getPropertyConverter().convert(null); - } - return super.getMappedKey(); - } + private final MongoPersistentProperty property; + private boolean associationFound; - private static class AssociationPropertyConverter implements Converter { + /** + * Creates a new {@link AssociationConverter} for the given {@link Association}. + * + * @param association must not be {@literal null}. + */ + public AssociationConverter(Association association) { - Association association; - String key; - - public AssociationPropertyConverter(String key, Association 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(); + } } /** diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index d0e534039..0c2bb85ab 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java @@ -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)); } /** diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java index 0f1f19c57..1412b7aef 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java @@ -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 dbRefAnnotatedList; + } + + static class Entity { + + @Id public String id; + String name; + } + + static class Wrapper { + + @Field("mapped") DocumentWithDBRefCollection nested; + } }