From 0b04251bbe19445f7d5f8244c455c07329f09818 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 2 Dec 2011 12:44:28 +0100 Subject: [PATCH] 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. --- .../context/AbstractMappingContext.java | 9 +- .../model/MutablePersistentEntity.java | 4 +- ...stractMappingContextIntegrationTests.java} | 6 +- .../AbstractMappingContextUnitTest.java | 68 ------------ .../AbstractMappingContextUnitTests.java | 104 ++++++++++++++++++ ...efaultPersistenPropertyPathUnitTests.java} | 6 +- ... AbstractRepositoryMetadataUnitTests.java} | 2 +- spring-data-commons-parent/pom.xml | 6 +- 8 files changed, 121 insertions(+), 84 deletions(-) rename spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/{AbstractMappingContextIntegrationTest.java => AbstractMappingContextIntegrationTests.java} (92%) delete mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTest.java create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java rename spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/{DefaultPersistenPropertyPathUnitTest.java => DefaultPersistenPropertyPathUnitTests.java} (92%) rename spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/{AbstractRepositoryMetadataUnitTest.java => AbstractRepositoryMetadataUnitTests.java} (98%) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index ebba36525..9815dd53c 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -235,7 +235,7 @@ public abstract class AbstractMappingContext> 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; } \ No newline at end of file diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTests.java similarity index 92% rename from spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java rename to spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTests.java index bf66f5203..0fe95d4d2 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextIntegrationTest.java +++ b/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> { +public class AbstractMappingContextIntegrationTests> { @Test public void foo() throws InterruptedException { @@ -89,6 +90,7 @@ public class AbstractMappingContextIntegrationTest> { - - final SimpleTypeHolder holder = new SimpleTypeHolder(); - - @Test - public void doesNotTryToLookupPersistentEntityForLeafProperty() { - - DummyMappingContext context = new DummyMappingContext(); - context.setSimpleTypeHolder(holder); - PersistentPropertyPath path = context.getPersistentPropertyPath(PropertyPath.from("name", Person.class)); - org.junit.Assert.assertThat(path, is(notNull())); - } - - class Person { - String name; - } - - - class DummyMappingContext extends AbstractMappingContext, T> { - - @Override - @SuppressWarnings("unchecked") - protected BasicPersistentEntity createPersistentEntity(TypeInformation typeInformation) { - return new BasicPersistentEntity((TypeInformation) 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 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; - } - } -} diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java new file mode 100644 index 000000000..5e4c7cf8e --- /dev/null +++ b/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 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, DummyPersistenProperty> { + + @Override + @SuppressWarnings("unchecked") + protected BasicPersistentEntity createPersistentEntity(TypeInformation typeInformation) { + return new BasicPersistentEntity((TypeInformation) 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 owner, final SimpleTypeHolder simpleTypeHolder) { + + return new DummyPersistenProperty(field, descriptor, owner, simpleTypeHolder); + } + } + + class DummyPersistenProperty extends AbstractPersistentProperty { + + public DummyPersistenProperty(Field field, PropertyDescriptor propertyDescriptor, + BasicPersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { + super(field, propertyDescriptor, owner, simpleTypeHolder); + } + + public boolean isIdProperty() { + return false; + } + + protected Association createAssociation() { + return new Association(this, null); + } + } +} diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/DefaultPersistenPropertyPathUnitTest.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/DefaultPersistenPropertyPathUnitTests.java similarity index 92% rename from spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/DefaultPersistenPropertyPathUnitTest.java rename to spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/DefaultPersistenPropertyPathUnitTests.java index 81088b1c0..b39006880 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/DefaultPersistenPropertyPathUnitTest.java +++ b/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> { +public class DefaultPersistenPropertyPathUnitTests> { @Mock T first, second; @@ -45,14 +44,12 @@ public class DefaultPersistenPropertyPathUnitTest converter; - PersistentPropertyPath noLeg; PersistentPropertyPath oneLeg; PersistentPropertyPath twoLegs; @Before @SuppressWarnings("unchecked") public void setUp() { - noLeg = new DefaultPersistentPropertyPath(Collections. emptyList()); oneLeg = new DefaultPersistentPropertyPath(Arrays.asList(first)); twoLegs = new DefaultPersistentPropertyPath(Arrays.asList(first, second)); } @@ -121,7 +118,6 @@ public class DefaultPersistenPropertyPathUnitTest 4.8.1 1.2.16 - 1.8.4 + 1.8.5 3.0.6.RELEASE 4.0.0.RELEASE [${org.springframework.version.30}, ${org.springframework.version.40}] @@ -271,10 +271,6 @@ **/*Tests.java - - **/Abstract*.java - **/*IntegrationTests.java - junit:junit