From 6616761f50ebacbbc55b760b3d75248034fc663e Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 6 Dec 2011 18:49:41 +0100 Subject: [PATCH] DATAMONGO-322 - MongoTemplates refuses to save entities with unset id if not auto-generateable. If an entity is handed into the template to be saved or inserted we now check that the auto-generated ObjectId can actually be applied to the id property after saving the object. --- .../data/mongodb/core/MongoTemplate.java | 26 ++++++ .../core/mapping/MongoSimpleTypes.java | 4 +- .../support/MongoRepositoryFactory.java | 25 ++---- .../mongodb/core/MongoTemplateUnitTests.java | 80 +++++++++++++++++-- .../MongoRepositoryFactoryUnitTests.java | 16 +--- 5 files changed, 110 insertions(+), 41 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 3daba8b01..2d89fa24f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -66,6 +66,7 @@ import org.springframework.data.mongodb.core.index.MongoPersistentEntityIndexCre 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.MongoSimpleTypes; import org.springframework.data.mongodb.core.mapping.event.AfterConvertEvent; import org.springframework.data.mongodb.core.mapping.event.AfterLoadEvent; import org.springframework.data.mongodb.core.mapping.event.AfterSaveEvent; @@ -609,6 +610,9 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { } protected void doInsert(String collectionName, T objectToSave, MongoWriter writer) { + + assertUpdateableIdIfNotSet(objectToSave); + BasicDBObject dbDoc = new BasicDBObject(); maybeEmitEvent(new BeforeConvertEvent(objectToSave)); @@ -693,6 +697,9 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { } protected void doSave(String collectionName, T objectToSave, MongoWriter writer) { + + assertUpdateableIdIfNotSet(objectToSave); + BasicDBObject dbDoc = new BasicDBObject(); maybeEmitEvent(new BeforeConvertEvent(objectToSave)); @@ -882,6 +889,25 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { return new Query(where(idProp.getFieldName()).is(idProperty)); } + private void assertUpdateableIdIfNotSet(Object entity) { + + MongoPersistentEntity persistentEntity = mappingContext.getPersistentEntity(entity.getClass()); + MongoPersistentProperty idProperty = persistentEntity.getIdProperty(); + + if (idProperty == null) { + return; + } + + ConversionService service = mongoConverter.getConversionService(); + Object idValue = BeanWrapper.create(entity, service).getProperty(idProperty, Object.class, true); + + if (idValue == null && !MongoSimpleTypes.AUTOGENERATED_ID_TYPES.contains(idProperty.getType())) { + throw new InvalidDataAccessApiUsageException(String.format( + "Cannot autogenerate id of type %s for entity of type %s!", idProperty.getType().getName(), entity.getClass() + .getName())); + } + } + public void remove(Query query, Class entityClass) { Assert.notNull(query); doRemove(determineCollectionName(entityClass), query, entityClass); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java index e553ee911..5f8482790 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoSimpleTypes.java @@ -35,14 +35,14 @@ import com.mongodb.DBRef; */ public abstract class MongoSimpleTypes { - public static final Set> SUPPORTED_ID_CLASSES; + public static final Set> AUTOGENERATED_ID_TYPES; static { Set> classes = new HashSet>(); classes.add(ObjectId.class); classes.add(String.class); classes.add(BigInteger.class); - SUPPORTED_ID_CLASSES = Collections.unmodifiableSet(classes); + AUTOGENERATED_ID_TYPES = Collections.unmodifiableSet(classes); Set> simpleTypes = new HashSet>(); simpleTypes.add(DBRef.class); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactory.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactory.java index c02cbce66..9a78014b2 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactory.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactory.java @@ -15,7 +15,7 @@ */ package org.springframework.data.mongodb.repository.support; -import static org.springframework.data.querydsl.QueryDslUtils.QUERY_DSL_PRESENT; +import static org.springframework.data.querydsl.QueryDslUtils.*; import java.io.Serializable; import java.lang.reflect.Method; @@ -23,7 +23,6 @@ import java.lang.reflect.Method; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.MongoTemplate; -import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes; import org.springframework.data.mongodb.repository.MongoRepository; import org.springframework.data.mongodb.repository.query.EntityInformationCreator; import org.springframework.data.mongodb.repository.query.MongoEntityInformation; @@ -38,7 +37,6 @@ import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryLookupStrategy.Key; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.util.Assert; -import org.springframework.util.StringUtils; /** * Factory to create {@link MongoRepository} instances. @@ -60,7 +58,8 @@ public class MongoRepositoryFactory extends RepositoryFactorySupport { Assert.notNull(mongoOperations); this.mongoOperations = mongoOperations; - this.entityInformationCreator = new DefaultEntityInformationCreator(mongoOperations.getConverter().getMappingContext()); + this.entityInformationCreator = new DefaultEntityInformationCreator(mongoOperations.getConverter() + .getMappingContext()); } /* @@ -111,13 +110,13 @@ public class MongoRepositoryFactory extends RepositoryFactorySupport { * @author Oliver Gierke */ private class MongoQueryLookupStrategy implements QueryLookupStrategy { - + /* * (non-Javadoc) * @see org.springframework.data.repository.query.QueryLookupStrategy#resolveQuery(java.lang.reflect.Method, org.springframework.data.repository.core.RepositoryMetadata, org.springframework.data.repository.core.NamedQueries) */ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata, NamedQueries namedQueries) { - + MongoQueryMethod queryMethod = new MongoQueryMethod(method, metadata, entityInformationCreator); String namedQueryName = queryMethod.getNamedQueryName(); @@ -132,20 +131,6 @@ public class MongoRepositoryFactory extends RepositoryFactorySupport { } } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.support.RepositoryFactorySupport#validate(org.springframework.data.repository.support.RepositoryMetadata) - */ - @Override - protected void validate(RepositoryMetadata metadata) { - - Class idClass = metadata.getIdClass(); - if (!MongoSimpleTypes.SUPPORTED_ID_CLASSES.contains(idClass)) { - throw new IllegalArgumentException(String.format("Unsupported id class! Only %s are supported!", - StringUtils.collectionToCommaDelimitedString(MongoSimpleTypes.SUPPORTED_ID_CLASSES))); - } - } - /* * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFactorySupport#getEntityInformation(java.lang.Class) diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index c185f7541..048d02f91 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -15,23 +15,37 @@ */ package org.springframework.data.mongodb.core; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; import static org.mockito.Mockito.*; -import com.mongodb.DB; -import com.mongodb.Mongo; -import com.mongodb.MongoException; +import java.math.BigInteger; + +import org.bson.types.ObjectId; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import org.springframework.context.support.GenericApplicationContext; import org.springframework.dao.DataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.annotation.Id; import org.springframework.data.mongodb.core.convert.MappingMongoConverter; +import org.springframework.data.mongodb.core.convert.MongoWriter; import org.springframework.test.util.ReflectionTestUtils; +import com.mongodb.BasicDBObject; +import com.mongodb.DB; +import com.mongodb.DBCollection; +import com.mongodb.DBObject; +import com.mongodb.Mongo; +import com.mongodb.MongoException; +import com.mongodb.WriteResult; + /** * Unit tests for {@link MongoTemplate}. * @@ -48,9 +62,15 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { @Mock DB db; + @Mock + DBCollection collection; + @Before public void setUp() { this.template = new MongoTemplate(mongo, "database"); + + when(mongo.getDB("database")).thenReturn(db); + when(db.getCollection(Mockito.any(String.class))).thenReturn(collection); } @Test(expected = IllegalArgumentException.class) @@ -82,7 +102,57 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { template.setApplicationContext(new GenericApplicationContext()); template.mapReduce("foo", "classpath:doesNotExist.js", "function() {}", Person.class); } - + + /** + * @see DATAMONGO-322 + */ + @Test(expected = InvalidDataAccessApiUsageException.class) + public void rejectsEntityWithNullIdIfNotSupportedIdType() { + + Object entity = new NotAutogenerateableId(); + template.save(entity); + } + + /** + * @see DATAMONGO-322 + */ + @Test + public void storesEntityWithSetIdAlthoughNotAutogenerateable() { + + NotAutogenerateableId entity = new NotAutogenerateableId(); + entity.id = 1; + + template.save(entity); + } + + /** + * @see DATAMONGO-322 + */ + @Test + public void autogeneratesIdForEntityWithAutogeneratableId() { + + MongoTemplate template = spy(this.template); + doReturn(new ObjectId()).when(template).saveDBObject(Mockito.any(String.class), Mockito.any(DBObject.class), + Mockito.any(Class.class)); + + AutogenerateableId entity = new AutogenerateableId(); + template.save(entity); + + assertThat(entity.id, is(notNullValue())); + } + + class AutogenerateableId { + + @Id + BigInteger id; + } + + class NotAutogenerateableId { + + @Id + Integer id; + } + /** * Mocks out the {@link MongoTemplate#getDb()} method to return the {@link DB} mock instead of executing the actual * behaviour. diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactoryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactoryUnitTests.java index fd757c9ed..cc491a877 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactoryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/MongoRepositoryFactoryUnitTests.java @@ -30,10 +30,8 @@ import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; -import org.springframework.data.mongodb.repository.MongoRepository; import org.springframework.data.mongodb.repository.Person; import org.springframework.data.mongodb.repository.query.MongoEntityInformation; -import org.springframework.data.mongodb.repository.support.MappingMongoEntityInformation; /** * Unit test for {@link MongoRepositoryFactory}. @@ -45,7 +43,7 @@ public class MongoRepositoryFactoryUnitTests { @Mock MongoTemplate template; - + @Mock MongoConverter converter; @@ -55,7 +53,7 @@ public class MongoRepositoryFactoryUnitTests { @Mock @SuppressWarnings("rawtypes") MongoPersistentEntity entity; - + @Before @SuppressWarnings({ "rawtypes", "unchecked" }) public void setUp() { @@ -63,12 +61,6 @@ public class MongoRepositoryFactoryUnitTests { when(converter.getMappingContext()).thenReturn((MappingContext) mappingContext); } - @Test(expected = IllegalArgumentException.class) - public void rejectsInvalidIdType() throws Exception { - MongoRepositoryFactory factory = new MongoRepositoryFactory(template); - factory.getRepository(SampleRepository.class); - } - @Test @SuppressWarnings("unchecked") public void usesMappingMongoEntityInformationIfMappingContextSet() { @@ -80,8 +72,4 @@ public class MongoRepositoryFactoryUnitTests { MongoEntityInformation entityInformation = factory.getEntityInformation(Person.class); assertTrue(entityInformation instanceof MappingMongoEntityInformation); } - - private interface SampleRepository extends MongoRepository { - - } }