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.

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 <T> void doInsert(String collectionName, T objectToSave, MongoWriter<T> writer) {
+
+		assertUpdateableIdIfNotSet(objectToSave);
+
 		BasicDBObject dbDoc = new BasicDBObject();
 
 		maybeEmitEvent(new BeforeConvertEvent<T>(objectToSave));
@@ -693,6 +697,9 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
 	}
 
 	protected <T> void doSave(String collectionName, T objectToSave, MongoWriter<T> writer) {
+
+		assertUpdateableIdIfNotSet(objectToSave);
+
 		BasicDBObject dbDoc = new BasicDBObject();
 
 		maybeEmitEvent(new BeforeConvertEvent<T>(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 <T> void remove(Query query, Class<T> entityClass) {
 		Assert.notNull(query);
 		doRemove(determineCollectionName(entityClass), query, entityClass);

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<Class<?>> SUPPORTED_ID_CLASSES;
+	public static final Set<Class<?>> AUTOGENERATED_ID_TYPES;
 	
 	static {
 		Set<Class<?>> classes = new HashSet<Class<?>>();
 		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<Class<?>> simpleTypes = new HashSet<Class<?>>();
 		simpleTypes.add(DBRef.class);

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)

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.

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<Person, Serializable> entityInformation = factory.getEntityInformation(Person.class);
 		assertTrue(entityInformation instanceof MappingMongoEntityInformation);
 	}
-
-	private interface SampleRepository extends MongoRepository<Person, Long> {
-
-	}
 }