DATACMNS-92 - PersistentEntities are not held in cache if verify() fails.

Documented verify() to throw a MappingException in case verification fails. Let this exception flying cause the PersistentEntity already added to the cache be removed from it in turn.

Fixed execution of integration tests and tests in classes starting with Abstract* along the way. Upgraded to Mockito 1.8.5.

spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java

@@ -235,7 +235,7 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 			final E entity = createPersistentEntity(typeInformation);
 
 			// Eagerly cache the entity as we might have to find it during recursive lookups.
-			persistentEntities.put(entity.getTypeInformation(), entity);
+			persistentEntities.put(typeInformation, entity);
 
 			BeanInfo info = Introspector.getBeanInfo(type);
 
@@ -251,7 +251,12 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 						}
 					});
 
-			entity.verify();
+			try {
+				entity.verify();
+			} catch (MappingException e) {
+				persistentEntities.remove(typeInformation);
+				throw e;
+			}
 
 			// Inform listeners
 			if (null != applicationEventPublisher) {

spring-data-commons-core/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java

@@ -50,6 +50,8 @@ public interface MutablePersistentEntity<T, P extends PersistentProperty<P>> ext
 	/**
 	 * Callback method to trigger validation of the {@link PersistentEntity}. As {@link MutablePersistentEntity} is not
 	 * immutable there might be some verification steps necessary after the object has reached is final state.
+	 * 
+	 * @throws MappingException in case the entity is invalid
 	 */
-	void verify();
+	void verify() throws MappingException;
 }

spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTests.java

@@ -19,6 +19,7 @@ import static org.mockito.Mockito.*;
 
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.Field;
+import java.util.Collections;
 
 import org.junit.Test;
 import org.springframework.data.mapping.PersistentEntity;
@@ -29,11 +30,11 @@ import org.springframework.data.mapping.model.SimpleTypeHolder;
 import org.springframework.data.util.TypeInformation;
 
 /**
- * Unit tests for {@link AbstractMappingContext}.
+ * Integration tests for {@link AbstractMappingContext}.
  * 
  * @author Oliver Gierke
  */
-public class AbstractMappingContextIntegrationTest<T extends PersistentProperty<T>> {
+public class AbstractMappingContextIntegrationTests<T extends PersistentProperty<T>> {
 
 	@Test
 	public void foo() throws InterruptedException {
@@ -89,6 +90,7 @@ public class AbstractMappingContextIntegrationTest<T extends PersistentProperty<
 			
 			when(prop.getTypeInformation()).thenReturn((TypeInformation) owner.getTypeInformation());
 			when(prop.getName()).thenReturn(field.getName());
+			when(prop.getPersistentEntityType()).thenReturn(Collections.EMPTY_SET);
 
 			try {
 				Thread.sleep(800);

spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTest.java

@@ -1,68 +0,0 @@
-package org.springframework.data.mapping.context;
-
-import static org.hamcrest.CoreMatchers.*;
-import static org.mockito.Matchers.*;
-import static org.mockito.Mockito.*;
-
-import java.beans.PropertyDescriptor;
-import java.lang.reflect.Field;
-
-import org.junit.Test;
-import org.springframework.data.mapping.PersistentProperty;
-import org.springframework.data.mapping.PropertyPath;
-import org.springframework.data.mapping.model.BasicPersistentEntity;
-import org.springframework.data.mapping.model.SimpleTypeHolder;
-import org.springframework.data.util.ClassTypeInformation;
-import org.springframework.data.util.TypeInformation;
-import org.springframework.util.Assert;
-
-/**
- *
- * @author Oliver Gierke
- */
-public class AbstractMappingContextUnitTest<T extends PersistentProperty<T>> {
-	
-	final SimpleTypeHolder holder = new SimpleTypeHolder();
-	
-	@Test
-	public void doesNotTryToLookupPersistentEntityForLeafProperty() {
-		
-		DummyMappingContext context = new DummyMappingContext();
-		context.setSimpleTypeHolder(holder);
-		PersistentPropertyPath<T> path = context.getPersistentPropertyPath(PropertyPath.from("name", Person.class));
-		org.junit.Assert.assertThat(path, is(notNull()));
-	}
-
-	class Person {
-		String name;
-	}
-	
-	
-	class DummyMappingContext extends AbstractMappingContext<BasicPersistentEntity<Object, T>, T> {
-
-		@Override
-		@SuppressWarnings("unchecked")
-		protected <S> BasicPersistentEntity<Object, T> createPersistentEntity(TypeInformation<S> typeInformation) {
-			return new BasicPersistentEntity<Object, T>((TypeInformation<Object>) typeInformation) {
-				
-				@Override
-				public void verify() {
-					Assert.isTrue(!holder.isSimpleType(getType()));
-				}
-			};
-		}
-
-		@Override
-		@SuppressWarnings({ "rawtypes", "unchecked" })
-		protected T createPersistentProperty(final Field field, final PropertyDescriptor descriptor,
-				final BasicPersistentEntity<Object, T> owner, final SimpleTypeHolder simpleTypeHolder) {
-			
-			PersistentProperty prop = mock(PersistentProperty.class);
-			
-			when(prop.getTypeInformation()).thenReturn(ClassTypeInformation.from(field.getType()));
-			when(prop.getName()).thenReturn(field.getName());
-
-			return (T) prop;
-		}
-	}
-}

spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java

@@ -0,0 +1,104 @@
+package org.springframework.data.mapping.context;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
+import java.beans.PropertyDescriptor;
+import java.lang.reflect.Field;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.data.mapping.Association;
+import org.springframework.data.mapping.PropertyPath;
+import org.springframework.data.mapping.model.AbstractPersistentProperty;
+import org.springframework.data.mapping.model.BasicPersistentEntity;
+import org.springframework.data.mapping.model.MappingException;
+import org.springframework.data.mapping.model.SimpleTypeHolder;
+import org.springframework.data.util.TypeInformation;
+
+/**
+ * Unit test for {@link AbstractMappingContext}.
+ *
+ * @author Oliver Gierke
+ */
+public class AbstractMappingContextUnitTests {
+	
+	final SimpleTypeHolder holder = new SimpleTypeHolder();
+	DummyMappingContext context;
+	
+	@Before
+	public void setUp() {
+		 context = new DummyMappingContext();
+		 context.setSimpleTypeHolder(holder);
+	}
+	
+	@Test
+	public void doesNotTryToLookupPersistentEntityForLeafProperty() {
+		PersistentPropertyPath<DummyPersistenProperty> path = context.getPersistentPropertyPath(PropertyPath.from("name", Person.class));
+		assertThat(path, is(notNullValue()));
+	}
+
+	/**
+	 * @see DATACMNS-92
+	 */
+	@Test(expected = MappingException.class)
+	public void doesNotAddInvalidEntity() {
+
+		try {
+			context.getPersistentEntity(Unsupported.class);
+		} catch (MappingException e) {
+			// expected
+		}
+		
+		context.getPersistentEntity(Unsupported.class);
+	}
+	
+	class Person {
+		String name;
+	}
+	
+	class Unsupported {
+		
+	}
+	
+	
+	class DummyMappingContext extends AbstractMappingContext<BasicPersistentEntity<Object, DummyPersistenProperty>, DummyPersistenProperty> {
+
+		@Override
+		@SuppressWarnings("unchecked")
+		protected <S> BasicPersistentEntity<Object, DummyPersistenProperty> createPersistentEntity(TypeInformation<S> typeInformation) {
+			return new BasicPersistentEntity<Object, DummyPersistenProperty>((TypeInformation<Object>) typeInformation) {
+				
+				@Override
+				public void verify() {
+					if (holder.isSimpleType(getType()) || Unsupported.class.equals(getType())) {
+						throw new MappingException("Invalid!");
+					}
+				}
+			};
+		}
+
+		@Override
+		protected DummyPersistenProperty createPersistentProperty(final Field field, final PropertyDescriptor descriptor,
+				final BasicPersistentEntity<Object, DummyPersistenProperty> owner, final SimpleTypeHolder simpleTypeHolder) {
+			
+			return new DummyPersistenProperty(field, descriptor, owner, simpleTypeHolder);
+		}
+	}
+	
+	class DummyPersistenProperty extends AbstractPersistentProperty<DummyPersistenProperty> {
+
+		public DummyPersistenProperty(Field field, PropertyDescriptor propertyDescriptor,
+				BasicPersistentEntity<?, DummyPersistenProperty> owner, SimpleTypeHolder simpleTypeHolder) {
+			super(field, propertyDescriptor, owner, simpleTypeHolder);
+		}
+
+		public boolean isIdProperty() {
+			return false;
+		}
+
+		protected Association<DummyPersistenProperty> createAssociation() {
+			return new Association<DummyPersistenProperty>(this, null);
+		}
+	}
+}

spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/DefaultPersistenPropertyPathUnitTests.java

@@ -21,7 +21,6 @@ import static org.mockito.Matchers.*;
 import static org.mockito.Mockito.*;
 
 import java.util.Arrays;
-import java.util.Collections;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -37,7 +36,7 @@ import org.springframework.data.mapping.PersistentProperty;
  * @author Oliver Gierke
  */
 @RunWith(MockitoJUnitRunner.class)
-public class DefaultPersistenPropertyPathUnitTest<T extends PersistentProperty<T>> {
+public class DefaultPersistenPropertyPathUnitTests<T extends PersistentProperty<T>> {
 
 	@Mock
 	T first, second;
@@ -45,14 +44,12 @@ public class DefaultPersistenPropertyPathUnitTest<T extends PersistentProperty<T
 	@Mock
 	Converter<T, String> converter;
 
-	PersistentPropertyPath<T> noLeg;
 	PersistentPropertyPath<T> oneLeg;
 	PersistentPropertyPath<T> twoLegs;
 
 	@Before
 	@SuppressWarnings("unchecked")
 	public void setUp() {
-		noLeg = new DefaultPersistentPropertyPath<T>(Collections.<T> emptyList());
 		oneLeg = new DefaultPersistentPropertyPath<T>(Arrays.asList(first));
 		twoLegs = new DefaultPersistentPropertyPath<T>(Arrays.asList(first, second));
 	}
@@ -121,7 +118,6 @@ public class DefaultPersistenPropertyPathUnitTest<T extends PersistentProperty<T
 
 	@Test
 	public void pathReturnsCorrectSize() {
-		assertThat(noLeg.getLength(), is(0));
 		assertThat(oneLeg.getLength(), is(1));
 		assertThat(twoLegs.getLength(), is(2));
 	}

spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTests.java

@@ -35,7 +35,7 @@ import org.springframework.data.repository.core.RepositoryMetadata;
  * 
  * @author Oliver Gierke
  */
-public class AbstractRepositoryMetadataUnitTest {
+public class AbstractRepositoryMetadataUnitTests {
 
 	@Test
 	public void discoversSimpleReturnTypeCorrectly() throws Exception {

spring-data-commons-parent/pom.xml

@@ -13,7 +13,7 @@
 		<!-- versions for commonly-used dependencies -->
 		<junit.version>4.8.1</junit.version>
 		<log4j.version>1.2.16</log4j.version>
-		<org.mockito.version>1.8.4</org.mockito.version>
+		<org.mockito.version>1.8.5</org.mockito.version>
 		<org.springframework.version.30>3.0.6.RELEASE</org.springframework.version.30>
 		<org.springframework.version.40>4.0.0.RELEASE</org.springframework.version.40>
 		<org.springframework.version>[${org.springframework.version.30}, ${org.springframework.version.40}]</org.springframework.version>
@@ -271,10 +271,6 @@
 					<includes>
 						<include>**/*Tests.java</include>
 					</includes>
-					<excludes>
-						<exclude>**/Abstract*.java</exclude>
-						<exclude>**/*IntegrationTests.java</exclude>
-					</excludes>
 					<junitArtifactName>junit:junit</junitArtifactName>
 				</configuration>
 			</plugin>