diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jExceptionTranslator.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jExceptionTranslator.java index 573564308..194cbdff8 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jExceptionTranslator.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jExceptionTranslator.java @@ -28,6 +28,7 @@ import org.neo4j.kernel.impl.transaction.IllegalResourceException; import org.neo4j.kernel.impl.transaction.LockException; import org.springframework.dao.*; import org.springframework.dao.support.PersistenceExceptionTranslator; +import org.springframework.data.neo4j.mapping.InvalidEntityTypeException; /** * @author mh @@ -40,6 +41,9 @@ public class Neo4jExceptionTranslator implements PersistenceExceptionTranslator try { throw ex; } catch(IllegalArgumentException iae) { + if (iae.getCause() != null && iae.getCause() instanceof InvalidEntityTypeException) { + throw (InvalidEntityTypeException)iae.getCause(); + } throw new InvalidDataAccessApiUsageException(iae.getMessage(),iae); } catch(DataAccessException dae) { throw dae; diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/AbstractConstructorEntityInstantiator.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/AbstractConstructorEntityInstantiator.java index a42767d42..8ebeb2111 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/AbstractConstructorEntityInstantiator.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/AbstractConstructorEntityInstantiator.java @@ -18,6 +18,7 @@ package org.springframework.data.neo4j.support.mapping; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.neo4j.mapping.EntityInstantiator; +import org.springframework.data.neo4j.mapping.InvalidEntityTypeException; import org.springframework.data.neo4j.mapping.MappingPolicy; import org.springframework.data.persistence.StateBackedCreator; import org.springframework.data.persistence.StateProvider; @@ -26,6 +27,7 @@ import sun.reflect.ReflectionFactory; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Modifier; import java.util.HashMap; import java.util.Map; @@ -38,6 +40,7 @@ public abstract class AbstractConstructorEntityInstantiator implements En private final static Logger log = LoggerFactory.getLogger(EntityInstantiator.class); + private final Map, Boolean> invalidInstantiationCheck = new HashMap, Boolean>(); private final Map, StateBackedCreator> cache = new HashMap, StateBackedCreator>(); @SuppressWarnings("unchecked") @@ -56,15 +59,30 @@ public abstract class AbstractConstructorEntityInstantiator implements En return creator.create(n, c); } } catch (IllegalArgumentException e) { - throw e; + throw e; } catch (InvocationTargetException e) { throw new IllegalArgumentException(e.getTargetException()); + } catch (InstantiationException e) { + if (isAbstractOrInterface(c)) { + // This is the same exception that is being used in the TypeSafetyPolicy + throw new InvalidEntityTypeException("Unable to legally create entity : abstract/interface class specified : " + c); + } + throw new IllegalArgumentException(e); } catch (Exception e) { throw new IllegalArgumentException(e); } } - public void setInstantiators( + protected boolean isAbstractOrInterface(Class c) { + Boolean result = invalidInstantiationCheck.get(c); + if (result == null) { + result = Modifier.isAbstract(c.getModifiers()) || Modifier.isInterface(c.getModifiers()); + invalidInstantiationCheck.put(c, result); + } + return result; + } + + public void setInstantiators( Map, StateBackedCreator> instantiators) { this.cache.putAll(instantiators); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityConverterImpl.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityConverterImpl.java index 611580204..8b8b01681 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityConverterImpl.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityConverterImpl.java @@ -118,7 +118,7 @@ public class Neo4jEntityConverterImpl implements private boolean storedAndRequestedTypesMatch(Class requestedType, S source) { TypeInformation storedType = typeMapper.readType(source); - return storedType.getType().isAssignableFrom(requestedType); + return requestedType.isAssignableFrom(storedType.getType()); } private void cascadeFetch(Neo4jPersistentEntityImpl persistentEntity, final BeanWrapper, R> wrapper, final MappingPolicy policy, final Neo4jTemplate template) { diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/AbstractNodeEntity.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/AbstractNodeEntity.java new file mode 100644 index 000000000..91e93aa80 --- /dev/null +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/AbstractNodeEntity.java @@ -0,0 +1,24 @@ +package org.springframework.data.neo4j.model; + +import org.springframework.data.neo4j.annotation.GraphId; +import org.springframework.data.neo4j.annotation.NodeEntity; + +/** + * + */ +@NodeEntity +public abstract class AbstractNodeEntity { + + @GraphId + public Long id; + + public String name; + + public AbstractNodeEntity() { + this(null); + } + + public AbstractNodeEntity(String name) { + this.name = name; + } +} \ No newline at end of file diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/Concrete1NodeEntity.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/Concrete1NodeEntity.java new file mode 100644 index 000000000..91fb345c1 --- /dev/null +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/Concrete1NodeEntity.java @@ -0,0 +1,30 @@ +package org.springframework.data.neo4j.model; + +/** + * A concrete version (1) of AbstractNodeEntity + */ +public class Concrete1NodeEntity extends AbstractNodeEntity { + + public Concrete1NodeEntity() { + super(); + } + + public Concrete1NodeEntity(String name) { + super(name); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Concrete1NodeEntity otherNode = (Concrete1NodeEntity) o; + if (id == null) return super.equals(o); + return id.equals(otherNode.id); + } + + @Override + public int hashCode() { + return id != null ? id.hashCode() : super.hashCode(); + } +} \ No newline at end of file diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/Concrete2NodeEntity.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/Concrete2NodeEntity.java new file mode 100644 index 000000000..da2649e12 --- /dev/null +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/model/Concrete2NodeEntity.java @@ -0,0 +1,31 @@ +package org.springframework.data.neo4j.model; + +/** + * A concrete version (2) of AbstractNodeEntity + */ +public class Concrete2NodeEntity extends AbstractNodeEntity { + + public Concrete2NodeEntity() { + super(); + } + + public Concrete2NodeEntity(String name) { + super(name); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Concrete2NodeEntity otherNode = (Concrete2NodeEntity) o; + if (id == null) return super.equals(o); + return id.equals(otherNode.id); + } + + @Override + public int hashCode() { + return id != null ? id.hashCode() : super.hashCode(); + } + +} \ No newline at end of file diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/AbstractEntityBasedGraphRepositoryTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/AbstractEntityBasedGraphRepositoryTests.java new file mode 100644 index 000000000..f4eb78d87 --- /dev/null +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/AbstractEntityBasedGraphRepositoryTests.java @@ -0,0 +1,99 @@ +/** + * Copyright 2011 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.data.neo4j.repository; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.neo4j.mapping.InvalidEntityTypeException; +import org.springframework.data.neo4j.model.AbstractNodeEntity; +import org.springframework.data.neo4j.model.Concrete1NodeEntity; +import org.springframework.data.neo4j.model.Concrete2NodeEntity; +import org.springframework.data.neo4j.model.Person; +import org.springframework.data.neo4j.support.Neo4jTemplate; +import org.springframework.data.neo4j.support.node.Neo4jHelper; +import org.springframework.test.context.CleanContextCacheTestExecutionListener; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestExecutionListeners; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; +import org.springframework.test.context.transaction.BeforeTransaction; +import org.springframework.test.context.transaction.TransactionalTestExecutionListener; +import org.springframework.transaction.annotation.Transactional; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; + +/** + * Tests for repositories which are defined against an Abstract Entity. + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration +@TestExecutionListeners({CleanContextCacheTestExecutionListener.class, DependencyInjectionTestExecutionListener.class, TransactionalTestExecutionListener.class}) +public class AbstractEntityBasedGraphRepositoryTests { + + protected final Logger log = LoggerFactory.getLogger(getClass()); + + @Autowired + private Neo4jTemplate neo4jTemplate; + @Autowired + private PersonRepository personRepository; + @Autowired + AbstractNodeEntityRepository abstractNodeEntityRepository; + @BeforeTransaction + public void cleanDb() { + Neo4jHelper.cleanDb(neo4jTemplate); + } + + + @Transactional + @Test(expected = InvalidEntityTypeException.class) // DATAGRAPH-298 + public void testInvalidEntityLoadAttemptThroughAbstractRepoThrowsAppropriateException() { + + Person person = personRepository.save(new Person("someone",30)); + + // We are trying to load a Person Node Entity, using a completely different + // and abstract defined Node entity - this should fail + AbstractNodeEntity shouldNotWork = abstractNodeEntityRepository.findOne(person.getId()); + + } + + @Test + @Transactional // DATAGRAPH-298 + public void testConcreteEntityLoadedThroughAbstractRepoLoadsCorrectType() { + + Concrete1NodeEntity origConcrete1 = new Concrete1NodeEntity("concrete1A"); + Concrete2NodeEntity origConcrete2 = new Concrete2NodeEntity("concrete2A"); + abstractNodeEntityRepository.save(origConcrete1); + abstractNodeEntityRepository.save(origConcrete2); + + Concrete1NodeEntity loadedConcrete1 = (Concrete1NodeEntity)abstractNodeEntityRepository.findOne(origConcrete1.id); + assertNotNull(loadedConcrete1); + assertThat(loadedConcrete1, is(origConcrete1)); + + Concrete2NodeEntity loadedConcrete2 = (Concrete2NodeEntity)abstractNodeEntityRepository.findOne(origConcrete2.id); + assertNotNull(loadedConcrete2); + assertThat(loadedConcrete2, is(origConcrete2)); + + } + + + +} diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/AbstractNodeEntityRepository.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/AbstractNodeEntityRepository.java new file mode 100644 index 000000000..b03ad0a41 --- /dev/null +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/AbstractNodeEntityRepository.java @@ -0,0 +1,11 @@ +package org.springframework.data.neo4j.repository; + +import org.springframework.data.neo4j.model.AbstractNodeEntity; +import org.springframework.data.neo4j.repository.GraphRepository; + +/** + * This Repository has specifically been created against the abstract node entity + * class, to be able to test loading concrete specific node entities. + */ +public interface AbstractNodeEntityRepository extends GraphRepository { +} diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ExceptionThrowingTypeSafetyNeo4jTemplateTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ExceptionThrowingTypeSafetyNeo4jTemplateTests.java index 28c4125e0..df4eb0f7f 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ExceptionThrowingTypeSafetyNeo4jTemplateTests.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ExceptionThrowingTypeSafetyNeo4jTemplateTests.java @@ -21,13 +21,13 @@ import org.junit.runner.RunWith; import org.neo4j.graphdb.NotFoundException; import org.springframework.dao.DataRetrievalFailureException; import org.springframework.data.neo4j.mapping.InvalidEntityTypeException; -import org.springframework.data.neo4j.model.Group; -import org.springframework.data.neo4j.model.Person; +import org.springframework.data.neo4j.model.*; import org.springframework.data.neo4j.template.Neo4jOperations; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.annotation.Transactional; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -64,4 +64,36 @@ public class ExceptionThrowingTypeSafetyNeo4jTemplateTests extends EntityTestBas public void testFindOneWithNonExistingIdThrowsDataRetrievalFailureException() throws Exception { neo4jOperations.findOne(Long.MAX_VALUE, Person.class); } + + @Test(expected = InvalidEntityTypeException.class) + @Transactional + public void testFindOneWithAbstractWrongTypeThrowsInvalidEntityTypeException() throws Exception { + neo4jOperations.findOne(testTeam.michael.getId(), AbstractNodeEntity.class); + } + + @Test + @Transactional + public void testFindOneWithConcreteEntityAndConcreteTypeReturnsConcreteEntity() throws Exception { + AbstractNodeEntity origConcrete1NodeEntity = neo4jOperations.save(new Concrete1NodeEntity("concrete1")); + AbstractNodeEntity readConcrete1NodeEntity = neo4jOperations.findOne(origConcrete1NodeEntity.id, Concrete1NodeEntity.class); + assertNotNull(readConcrete1NodeEntity); + assertEquals(origConcrete1NodeEntity,readConcrete1NodeEntity); + } + + @Test + @Transactional + public void testFindOneWithConcreteEntityAndAbstractTypeReturnsConcreteEntity() throws Exception { + Concrete1NodeEntity origConcrete1NodeEntity = neo4jOperations.save(new Concrete1NodeEntity("concrete1")); + Concrete1NodeEntity readConcrete1NodeEntity = (Concrete1NodeEntity)neo4jOperations.findOne(origConcrete1NodeEntity.id, AbstractNodeEntity.class); + assertNotNull(readConcrete1NodeEntity); + assertEquals(origConcrete1NodeEntity,readConcrete1NodeEntity); + } + + @Test(expected = InvalidEntityTypeException.class) + @Transactional + public void testFindOneWithDifferentConcreteEntitiesThrowsInvalidEntityTypeException() throws Exception { + Concrete2NodeEntity origConcrete2NodeEntity = neo4jOperations.save(new Concrete2NodeEntity("concrete2")); + AbstractNodeEntity readConcrete2NodeEntity = neo4jOperations.findOne(origConcrete2NodeEntity.id, Concrete1NodeEntity.class); + } + } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/NullReturningTypeSafetyNeo4jTemplateTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/NullReturningTypeSafetyNeo4jTemplateTests.java index fcd6afa75..c249145d4 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/NullReturningTypeSafetyNeo4jTemplateTests.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/NullReturningTypeSafetyNeo4jTemplateTests.java @@ -21,13 +21,13 @@ import org.junit.runner.RunWith; import org.neo4j.graphdb.NotFoundException; import org.springframework.dao.DataRetrievalFailureException; import org.springframework.data.neo4j.mapping.InvalidEntityTypeException; -import org.springframework.data.neo4j.model.Group; -import org.springframework.data.neo4j.model.Person; +import org.springframework.data.neo4j.model.*; import org.springframework.data.neo4j.template.Neo4jOperations; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.annotation.Transactional; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -65,4 +65,38 @@ public class NullReturningTypeSafetyNeo4jTemplateTests extends EntityTestBase { public void testFindOneWithNonExistingIdThrowsDataRetrievalFailureException() throws Exception { neo4jOperations.findOne(Long.MAX_VALUE, Person.class); } + + @Test + @Transactional + public void testFindOneWithAbstractWrongTypeReturnsNull() throws Exception { + AbstractNodeEntity entity = neo4jOperations.findOne(testTeam.michael.getId(), AbstractNodeEntity.class); + assertNull(entity); + } + + @Test + @Transactional + public void testFindOneWithConcreteEntityAndConcreteTypeReturnsConcreteEntity() throws Exception { + AbstractNodeEntity origConcrete1NodeEntity = neo4jOperations.save(new Concrete1NodeEntity("concrete1")); + AbstractNodeEntity readConcrete1NodeEntity = neo4jOperations.findOne(origConcrete1NodeEntity.id, Concrete1NodeEntity.class); + assertNotNull(readConcrete1NodeEntity); + assertEquals(origConcrete1NodeEntity,readConcrete1NodeEntity); + } + + @Test + @Transactional + public void testFindOneWithConcreteEntityAndAbstractTypeReturnsConcreteEntity() throws Exception { + Concrete1NodeEntity origConcrete1NodeEntity = neo4jOperations.save(new Concrete1NodeEntity("concrete1")); + Concrete1NodeEntity readConcrete1NodeEntity = (Concrete1NodeEntity)neo4jOperations.findOne(origConcrete1NodeEntity.id, AbstractNodeEntity.class); + assertNotNull(readConcrete1NodeEntity); + assertEquals(origConcrete1NodeEntity,readConcrete1NodeEntity); + } + + @Test + @Transactional + public void testFindOneWithDifferentConcreteEntitiesReturnsNull() throws Exception { + Concrete2NodeEntity origConcrete2NodeEntity = neo4jOperations.save(new Concrete2NodeEntity("concrete2")); + AbstractNodeEntity readConcrete2NodeEntity = neo4jOperations.findOne(origConcrete2NodeEntity.id, Concrete1NodeEntity.class); + assertNull(readConcrete2NodeEntity); + } + } diff --git a/spring-data-neo4j/src/test/resources/org/springframework/data/neo4j/repository/AbstractEntityBasedGraphRepositoryTests-context.xml b/spring-data-neo4j/src/test/resources/org/springframework/data/neo4j/repository/AbstractEntityBasedGraphRepositoryTests-context.xml new file mode 100644 index 000000000..08d6d11c3 --- /dev/null +++ b/spring-data-neo4j/src/test/resources/org/springframework/data/neo4j/repository/AbstractEntityBasedGraphRepositoryTests-context.xml @@ -0,0 +1,13 @@ + + + + + + + \ No newline at end of file