From deacee100087a9501c98d423f4fde9b0754c205a Mon Sep 17 00:00:00 2001 From: Jean-Pierre Bergamin Date: Sun, 2 Oct 2011 01:49:29 +0200 Subject: [PATCH 1/3] OneToNRelationshipFieldAccessor overrides getDefaultImplementation that returns a HashSet. Makes the previously ignored test shouldWorkWithUninitializedCollectionFieldWithoutUnderlyingState run successfully. Introduced generic types for T and the TARGET entities. --- ...AbstractNodeRelationshipFieldAccessor.java | 2 +- .../NodeRelationshipFieldAccessorFactory.java | 9 +---- .../NodeToNodesRelationshipFieldAccessor.java | 2 +- ...elationshipEntityFieldAccessorFactory.java | 1 - ...neToNRelationshipFieldAccessorFactory.java | 36 +++++++++++-------- ...neToNRelationshipFieldAccessorFactory.java | 17 +++++---- ...ingleRelationshipFieldAccessorFactory.java | 20 +++++------ .../ModificationOutsideOfTransactionTest.java | 1 - 8 files changed, 41 insertions(+), 47 deletions(-) diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/AbstractNodeRelationshipFieldAccessor.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/AbstractNodeRelationshipFieldAccessor.java index 2502ca1b..5d4bc64b 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/AbstractNodeRelationshipFieldAccessor.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/AbstractNodeRelationshipFieldAccessor.java @@ -29,7 +29,7 @@ * @author Michael Hunger * @since 11.09.2010 */ -public abstract class AbstractNodeRelationshipFieldAccessor implements FieldAccessor { +public abstract class AbstractNodeRelationshipFieldAccessor,STATE extends PropertyContainer,TARGET extends GraphBacked,TSTATE extends PropertyContainer> implements FieldAccessor { protected final RelationshipType type; protected final Neo4JPersistentProperty property; protected final Direction direction; diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeRelationshipFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeRelationshipFieldAccessorFactory.java index b2444a2f..efb7abe6 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeRelationshipFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeRelationshipFieldAccessorFactory.java @@ -16,21 +16,14 @@ package org.springframework.data.neo4j.fieldaccess; -import org.neo4j.graphdb.Direction; -import org.neo4j.graphdb.DynamicRelationshipType; -import org.springframework.data.neo4j.annotation.RelatedTo; import org.springframework.data.neo4j.core.NodeBacked; -import org.springframework.data.neo4j.mapping.Neo4JPersistentProperty; -import org.springframework.data.neo4j.support.GenericTypeExtractor; import org.springframework.data.neo4j.support.GraphDatabaseContext; -import java.lang.reflect.Field; - /** * @author Michael Hunger * @since 12.09.2010 */ -public abstract class NodeRelationshipFieldAccessorFactory implements FieldAccessorFactory { +public abstract class NodeRelationshipFieldAccessorFactory implements FieldAccessorFactory { protected GraphDatabaseContext graphDatabaseContext; diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeToNodesRelationshipFieldAccessor.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeToNodesRelationshipFieldAccessor.java index 5aaf1704..7cf501bf 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeToNodesRelationshipFieldAccessor.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/NodeToNodesRelationshipFieldAccessor.java @@ -33,7 +33,7 @@ * @author Michael Hunger * @since 12.09.2010 */ -public abstract class NodeToNodesRelationshipFieldAccessor extends AbstractNodeRelationshipFieldAccessor { +public abstract class NodeToNodesRelationshipFieldAccessor extends AbstractNodeRelationshipFieldAccessor { public NodeToNodesRelationshipFieldAccessor(final Class clazz, final GraphDatabaseContext graphDatabaseContext, final Direction direction, final RelationshipType type, Neo4JPersistentProperty property) { super(clazz, graphDatabaseContext, direction, type,property); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipEntityFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipEntityFieldAccessorFactory.java index 3a65cf1f..6c10f2ea 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipEntityFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipEntityFieldAccessorFactory.java @@ -25,7 +25,6 @@ import org.springframework.data.neo4j.mapping.RelationshipInfo; import org.springframework.data.neo4j.support.GraphDatabaseContext; -import java.lang.reflect.Field; import java.util.HashSet; import java.util.Set; diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipFieldAccessorFactory.java index 0db6d82e..6fb38713 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/OneToNRelationshipFieldAccessorFactory.java @@ -16,6 +16,12 @@ package org.springframework.data.neo4j.fieldaccess; +import static org.springframework.data.neo4j.support.DoReturn.doReturn; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + import org.neo4j.graphdb.Direction; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.RelationshipType; @@ -24,12 +30,7 @@ import org.springframework.data.neo4j.mapping.RelationshipInfo; import org.springframework.data.neo4j.support.GraphDatabaseContext; -import java.util.Collections; -import java.util.Set; - -import static org.springframework.data.neo4j.support.DoReturn.doReturn; - -public class OneToNRelationshipFieldAccessorFactory extends NodeRelationshipFieldAccessorFactory { +public class OneToNRelationshipFieldAccessorFactory extends NodeRelationshipFieldAccessorFactory { public OneToNRelationshipFieldAccessorFactory(GraphDatabaseContext graphDatabaseContext) { super(graphDatabaseContext); @@ -43,19 +44,20 @@ public boolean accept(final Neo4JPersistentProperty property) { } @Override - public FieldAccessor forField(final Neo4JPersistentProperty property) { + public FieldAccessor forField(final Neo4JPersistentProperty property) { final RelationshipInfo relationshipInfo = property.getRelationshipInfo(); - final Class targetType = (Class) relationshipInfo.getTargetType().getType(); - return new OneToNRelationshipFieldAccessor(relationshipInfo.getRelationshipType(), relationshipInfo.getDirection(), targetType, graphDatabaseContext,property); + final Class targetType = (Class) relationshipInfo.getTargetType().getType(); + return new OneToNRelationshipFieldAccessor(relationshipInfo.getRelationshipType(), relationshipInfo.getDirection(), targetType, graphDatabaseContext,property); } - public static class OneToNRelationshipFieldAccessor extends NodeToNodesRelationshipFieldAccessor { + public static class OneToNRelationshipFieldAccessor extends NodeToNodesRelationshipFieldAccessor { - public OneToNRelationshipFieldAccessor(final RelationshipType type, final Direction direction, final Class elementClass, final GraphDatabaseContext graphDatabaseContext, Neo4JPersistentProperty property) { + public OneToNRelationshipFieldAccessor(final RelationshipType type, final Direction direction, final Class elementClass, final GraphDatabaseContext graphDatabaseContext, Neo4JPersistentProperty property) { super(elementClass, graphDatabaseContext, direction, type,property); } - public Object setValue(final NodeBacked entity, final Object newVal) { + @Override + public Object setValue(final T entity, final Object newVal) { final Node node = checkUnderlyingNode(entity); if (newVal == null) { removeMissingRelationships(node, Collections.emptySet()); @@ -64,15 +66,19 @@ public Object setValue(final NodeBacked entity, final Object newVal) { final Set targetNodes = checkTargetIsSetOfNodebacked(newVal); removeMissingRelationships(node, targetNodes); createAddedRelationships(node, targetNodes); - return createManagedSet(entity, (Set) newVal); + return createManagedSet(entity, (Set) newVal); } @Override - public Object getValue(final NodeBacked entity) { + public Object getValue(final T entity) { checkUnderlyingNode(entity); - final Set result = createEntitySetFromRelationshipEndNodes(entity); + final Set result = createEntitySetFromRelationshipEndNodes(entity); return doReturn(createManagedSet(entity, result)); } + @Override + public Object getDefaultImplementation() { + return new HashSet(); + } } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ReadOnlyOneToNRelationshipFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ReadOnlyOneToNRelationshipFieldAccessorFactory.java index 42efa399..bff90c94 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ReadOnlyOneToNRelationshipFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/ReadOnlyOneToNRelationshipFieldAccessorFactory.java @@ -24,9 +24,7 @@ import org.springframework.data.neo4j.mapping.RelationshipInfo; import org.springframework.data.neo4j.support.GraphDatabaseContext; -import java.lang.reflect.Field; - -public class ReadOnlyOneToNRelationshipFieldAccessorFactory extends NodeRelationshipFieldAccessorFactory { +public class ReadOnlyOneToNRelationshipFieldAccessorFactory extends NodeRelationshipFieldAccessorFactory { public ReadOnlyOneToNRelationshipFieldAccessorFactory(GraphDatabaseContext graphDatabaseContext) { super(graphDatabaseContext); @@ -40,23 +38,24 @@ public boolean accept(final Neo4JPersistentProperty f) { } @Override - public FieldAccessor forField(final Neo4JPersistentProperty property) { + public FieldAccessor forField(final Neo4JPersistentProperty property) { final RelationshipInfo relationshipInfo = property.getRelationshipInfo(); - return new ReadOnlyOneToNRelationshipFieldAccessor(relationshipInfo.getRelationshipType(), relationshipInfo.getDirection(), (Class) property.getRelationshipInfo().getTargetType().getType(), graphDatabaseContext,property); + return new ReadOnlyOneToNRelationshipFieldAccessor(relationshipInfo.getRelationshipType(), relationshipInfo.getDirection(), (Class) property.getRelationshipInfo().getTargetType().getType(), graphDatabaseContext,property); } - public static class ReadOnlyOneToNRelationshipFieldAccessor extends OneToNRelationshipFieldAccessorFactory.OneToNRelationshipFieldAccessor { + public static class ReadOnlyOneToNRelationshipFieldAccessor extends OneToNRelationshipFieldAccessorFactory.OneToNRelationshipFieldAccessor { - public ReadOnlyOneToNRelationshipFieldAccessor(final RelationshipType type, final Direction direction, final Class elementClass, final GraphDatabaseContext graphDatabaseContext, Neo4JPersistentProperty field) { + public ReadOnlyOneToNRelationshipFieldAccessor(final RelationshipType type, final Direction direction, final Class elementClass, final GraphDatabaseContext graphDatabaseContext, Neo4JPersistentProperty field) { super(type,direction,elementClass, graphDatabaseContext, field); } @Override - public boolean isWriteable(NodeBacked nodeBacked) { + public boolean isWriteable(T entity) { return false; } - public Object setValue(final NodeBacked entity, final Object newVal) { + @Override + public Object setValue(final T entity, final Object newVal) { throw new InvalidDataAccessApiUsageException("Cannot set read-only relationship entity field."); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/SingleRelationshipFieldAccessorFactory.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/SingleRelationshipFieldAccessorFactory.java index ac5b114a..412dee72 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/SingleRelationshipFieldAccessorFactory.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/SingleRelationshipFieldAccessorFactory.java @@ -19,19 +19,17 @@ import org.neo4j.graphdb.Direction; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.RelationshipType; -import org.springframework.data.neo4j.annotation.RelatedTo; import org.springframework.data.neo4j.core.NodeBacked; import org.springframework.data.neo4j.mapping.Neo4JPersistentProperty; import org.springframework.data.neo4j.mapping.RelationshipInfo; import org.springframework.data.neo4j.support.GraphDatabaseContext; -import java.lang.reflect.Field; import java.util.Collections; import java.util.Set; import static org.springframework.data.neo4j.support.DoReturn.doReturn; -public class SingleRelationshipFieldAccessorFactory extends NodeRelationshipFieldAccessorFactory { +public class SingleRelationshipFieldAccessorFactory extends NodeRelationshipFieldAccessorFactory { public SingleRelationshipFieldAccessorFactory(GraphDatabaseContext graphDatabaseContext) { super(graphDatabaseContext); @@ -43,18 +41,18 @@ public boolean accept(final Neo4JPersistentProperty property) { } @Override - public FieldAccessor forField(final Neo4JPersistentProperty property) { + public FieldAccessor forField(final Neo4JPersistentProperty property) { final RelationshipInfo relationshipInfo = property.getRelationshipInfo(); - return new SingleRelationshipFieldAccessor(relationshipInfo.getRelationshipType(), relationshipInfo.getDirection(), (Class) relationshipInfo.getTargetType().getType(), graphDatabaseContext,property); + return new SingleRelationshipFieldAccessor(relationshipInfo.getRelationshipType(), relationshipInfo.getDirection(), (Class) relationshipInfo.getTargetType().getType(), graphDatabaseContext,property); } - public static class SingleRelationshipFieldAccessor extends NodeToNodesRelationshipFieldAccessor { - public SingleRelationshipFieldAccessor(final RelationshipType type, final Direction direction, final Class clazz, final GraphDatabaseContext graphDatabaseContext, Neo4JPersistentProperty property) { + public static class SingleRelationshipFieldAccessor extends NodeToNodesRelationshipFieldAccessor { + public SingleRelationshipFieldAccessor(final RelationshipType type, final Direction direction, final Class clazz, final GraphDatabaseContext graphDatabaseContext, Neo4JPersistentProperty property) { super(clazz, graphDatabaseContext, direction, type, property); } @Override - public Object setValue(final NodeBacked entity, final Object newVal) { + public Object setValue(final T entity, final Object newVal) { final Node node=checkUnderlyingNode(entity); if (newVal == null) { removeMissingRelationships(node, Collections.emptySet()); @@ -67,10 +65,10 @@ public Object setValue(final NodeBacked entity, final Object newVal) { } @Override - public Object getValue(final NodeBacked entity) { + public Object getValue(final T entity) { checkUnderlyingNode(entity); - final Set result = createEntitySetFromRelationshipEndNodes(entity); - final NodeBacked singleEntity = result.isEmpty() ? null : result.iterator().next(); + final Set result = createEntitySetFromRelationshipEndNodes(entity); + final TARGET singleEntity = result.isEmpty() ? null : result.iterator().next(); return doReturn(singleEntity); } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java index 87a2e481..878831b0 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java @@ -164,7 +164,6 @@ public void testSetPropertyOutsideTransaction() assertEquals( 35, nodeFor( p ).getProperty("age") ); } - @Ignore @Test public void shouldWorkWithUninitializedCollectionFieldWithoutUnderlyingState() { From d5a1f74ab752feb3fcc4ef4d39964c2f1753f9e2 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Bergamin Date: Sun, 2 Oct 2011 02:13:26 +0200 Subject: [PATCH 2/3] added checks to make sure the relationships get created when the node is persisted --- .../ModificationOutsideOfTransactionTest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java index 878831b0..f6b62408 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/ModificationOutsideOfTransactionTest.java @@ -165,7 +165,7 @@ public void testSetPropertyOutsideTransaction() } @Test - public void shouldWorkWithUninitializedCollectionFieldWithoutUnderlyingState() + public void uninitializedCollectionFieldWithoutUnderlyingState() { Group group = new Group(); Collection people = group.getPersons(); @@ -175,10 +175,14 @@ public void shouldWorkWithUninitializedCollectionFieldWithoutUnderlyingState() people.add(p); assertEquals( Collections.singleton(p), group.getPersons() ); + + group.persist(); + assertThat(group.getPersistentState(), hasRelationship("persons", p.getPersistentState())); + assertThat(p.getPersistentState(), hasRelationship("persons", group.getPersistentState())); } @Test - public void shouldWorkWithInitializedCollectionFieldWithoutUnderlyingState() + public void initializedCollectionFieldWithoutUnderlyingState() { Group group = new Group(); group.setPersons(new HashSet()); @@ -189,6 +193,10 @@ public void shouldWorkWithInitializedCollectionFieldWithoutUnderlyingState() people.add(p); assertEquals( Collections.singleton(p), group.getPersons() ); + + group.persist(); + assertThat(group.getPersistentState(), hasRelationship("persons", p.getPersistentState())); + assertThat(p.getPersistentState(), hasRelationship("persons", group.getPersistentState())); } @Test From c4f84d1846a2ae5d8045c5e75afd65fb6f24f49e Mon Sep 17 00:00:00 2001 From: Jean-Pierre Bergamin Date: Wed, 5 Oct 2011 16:44:45 +0200 Subject: [PATCH 3/3] fixes NullPointerException with entities using transient fields in detached mode. --- .../data/neo4j/fieldaccess/DetachedEntityState.java | 13 +++++++++---- .../data/neo4j/support/PropertyTest.java | 8 +++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/DetachedEntityState.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/DetachedEntityState.java index 977d94c4..e7d1498e 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/DetachedEntityState.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/DetachedEntityState.java @@ -43,8 +43,8 @@ public class DetachedEntityState, STATE> imple private final Map dirty = new HashMap(); protected final EntityState delegate; private final static Log log = LogFactory.getLog(DetachedEntityState.class); - private GraphDatabaseContext graphDatabaseContext; - private Neo4JPersistentEntity persistentEntity; + private final GraphDatabaseContext graphDatabaseContext; + private final Neo4JPersistentEntity persistentEntity; public DetachedEntityState(final EntityState delegate, GraphDatabaseContext graphDatabaseContext) { this.delegate = delegate; @@ -134,7 +134,13 @@ private boolean mustCheckConcurrentModification() { } @Override public Object setValue(final Field field, final Object newVal) { - return setValue(property(field),newVal); + Neo4JPersistentProperty property = property(field); + if (property == null) { + // The property maybe null if it is a transient field + return newVal; + } + + return setValue(property, newVal); } private Neo4JPersistentProperty property(Field field) { @@ -146,7 +152,6 @@ public Object setValue(final Neo4JPersistentProperty property, final Object newV if (isDetached()) { final Field field = property.getField(); if (!isDirty(field) && isWritable(field)) { - Object existingValue; if (hasPersistentState()) { addDirty(field, unwrap(delegate.getValue(field)), true); } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/PropertyTest.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/PropertyTest.java index 74ca718f..befa989c 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/PropertyTest.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/support/PropertyTest.java @@ -70,7 +70,13 @@ public void testGetPropertyEnum() { p.getPersistentState().setProperty("personality", "EXTROVERT"); assertEquals("Did not deserialize property value properly.", Personality.EXTROVERT, p.getPersonality()); } - + @Test + @Transactional + public void testSetTransientPropertyFieldInDetachedMode() { + Person p = new Person("Michael", 35); + p.setThought("food"); // caused a NullPointerException + } + @Test(expected = NotFoundException.class) @Transactional public void testSetTransientPropertyFieldNotManaged() {