From b7077e89f625caecad5994aa515f4306fc7f547d Mon Sep 17 00:00:00 2001 From: David Montag Date: Mon, 11 Apr 2011 15:17:47 -0700 Subject: [PATCH] Removed backreferences stuff. Added cascading of persist of dirty fields pointing to node entities or collections thereof. --- .../data/graph/core/EntityState.java | 2 - .../data/graph/core/NodeBacked.java | 3 - .../neo4j/fieldaccess/BackReferences.java | 59 ------------- .../neo4j/fieldaccess/DefaultEntityState.java | 6 -- .../fieldaccess/DetachedEntityState.java | 82 +++++++------------ .../neo4j/support/node/Neo4jNodeBacking.aj | 3 - .../neo4j/support/HasRelationshipMatcher.java | 4 + .../ModificationOutsideOfTransactionTest.java | 55 ++++++++++++- 8 files changed, 85 insertions(+), 129 deletions(-) delete mode 100644 spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/BackReferences.java diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/EntityState.java b/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/EntityState.java index aa895dbe5..594c9ad94 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/EntityState.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/EntityState.java @@ -62,6 +62,4 @@ public interface EntityState,STATE> { STATE getPersistentState(); ENTITY persist(); - - boolean refersTo(GraphBacked target); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/NodeBacked.java b/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/NodeBacked.java index db6fdbec4..50eaa7e97 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/NodeBacked.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/graph/core/NodeBacked.java @@ -147,7 +147,4 @@ public interface NodeBacked extends GraphBacked { * @return the newly created relationship to the target node */ Relationship relateTo(NodeBacked target, String type); - - // will possibly be used for object graphs - boolean refersTo(GraphBacked target); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/BackReferences.java b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/BackReferences.java deleted file mode 100644 index 353896902..000000000 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/BackReferences.java +++ /dev/null @@ -1,59 +0,0 @@ -/** - * 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.graph.neo4j.fieldaccess; - -import org.springframework.data.graph.core.EntityState; -import org.springframework.data.graph.core.GraphBacked; -import org.springframework.data.graph.core.NodeBacked; - -import java.util.*; - -/** - * @author mh - * @since 12.03.11 - */ -public class BackReferences { - private List backrefs=new ArrayList(); - private EntityState entityState; - - public BackReferences(EntityState entityState) { - - this.entityState = entityState; - } - - public void addBackReferences(Collection backReference) { - this.backrefs.addAll(backReference); - } - - - private void pruneInvalidBackRefs() { - GraphBacked entity = entityState.getEntity(); - for (Iterator it = backrefs.iterator(); it.hasNext();) { - NodeBacked backRef = it.next(); - if (backRef.refersTo(entity)) continue; - it.remove(); - } - } - - - public void persistNeighbours() { - pruneInvalidBackRefs(); - for (NodeBacked backref : backrefs) { - backref.persist(); - } - } -} diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DefaultEntityState.java b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DefaultEntityState.java index e9a25b3d7..cd8f42c2d 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DefaultEntityState.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DefaultEntityState.java @@ -108,7 +108,6 @@ public abstract class DefaultEntityState, STAT } } - protected Object getIdFromEntity() { final Field idField = fieldAccessorFactoryProviders.getIdField(); if (idField==null) return null; @@ -120,9 +119,4 @@ public abstract class DefaultEntityState, STAT return null; } } - - @Override - public boolean refersTo(GraphBacked target) { - return false; - } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java index 27a1ad3ef..1f9190d37 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/fieldaccess/DetachedEntityState.java @@ -19,15 +19,17 @@ package org.springframework.data.graph.neo4j.fieldaccess; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.neo4j.graphdb.Transaction; -import org.springframework.data.graph.annotation.RelatedTo; +import org.springframework.data.graph.core.EntityState; import org.springframework.data.graph.core.GraphBacked; import org.springframework.data.graph.core.NodeBacked; -import org.springframework.data.graph.core.EntityState; import org.springframework.data.graph.neo4j.support.GraphDatabaseContext; import org.springframework.util.ObjectUtils; import java.lang.reflect.Field; -import java.util.*; +import java.util.Collection; +import java.util.ConcurrentModificationException; +import java.util.HashMap; +import java.util.Map; import static org.springframework.data.graph.neo4j.support.DoReturn.unwrap; @@ -40,11 +42,9 @@ public class DetachedEntityState, STATE> imple protected final EntityState delegate; private final static Log log = LogFactory.getLog(DetachedEntityState.class); private GraphDatabaseContext graphDatabaseContext; - private final BackReferences backReferences = null; public DetachedEntityState(final EntityState delegate, GraphDatabaseContext graphDatabaseContext) { this.delegate = delegate; this.graphDatabaseContext = graphDatabaseContext; - //this.backReferences = new BackReferences(this); } @Override @@ -152,14 +152,31 @@ public class DetachedEntityState, STATE> imple // createAndAssignState(); throw new IllegalStateException("Flushing detached entity without a persistent state, this had to be created first."); } + if (isDirty()) { - for (final Map.Entry entry : dirty.entrySet()) { - final Field field = entry.getKey(); - if (log.isDebugEnabled()) log.debug("Flushing dirty Entity new node " + entity.getPersistentState() + " field " + field+ " with value "+getValueFromEntity(field)); - checkConcurrentModification(entity, entry, field); - delegate.setValue(field, getValueFromEntity(field)); - } + final Map dirtyCopy = new HashMap(dirty); clearDirty(); + for (final Map.Entry entry : dirtyCopy.entrySet()) { + final Field field = entry.getKey(); + Object valueFromEntity = getValueFromEntity(field); + cascadePersist(valueFromEntity); + if (log.isDebugEnabled()) log.debug("Flushing dirty Entity new node " + entity.getPersistentState() + " field " + field+ " with value "+ valueFromEntity); + checkConcurrentModification(entity, entry, field); + delegate.setValue(field, valueFromEntity); + } + } + } + + private void cascadePersist(Object valueFromEntity) { + if (valueFromEntity instanceof NodeBacked) { + ((NodeBacked) valueFromEntity).persist(); + } + if (valueFromEntity instanceof Collection) { + for (Object o : (Collection)valueFromEntity) { + if (o instanceof NodeBacked) { + ((NodeBacked) o).persist(); + } + } } } @@ -217,7 +234,6 @@ public class DetachedEntityState, STATE> imple Transaction tx = graphDatabaseContext.beginTx(); try { ENTITY result = delegate.persist(); - //persistNeighbours(); flushDirty(); tx.success(); @@ -226,46 +242,4 @@ public class DetachedEntityState, STATE> imple tx.finish(); } } - - private void persistNeighbours() { - backReferences.persistNeighbours(); - for (NodeBacked nodeBacked : getOutboundDirtyNodeEntities()) { - nodeBacked.persist(); - } - } - - private Set getOutboundDirtyNodeEntities() { - HashSet result = new HashSet(); - for (Field field : dirty.keySet()) { - if (handleSingleField(result, field)) continue; - handleOneToMany(result, field); - } - return result; - } - - private boolean handleOneToMany(HashSet result, Field field) { - if ((Collection.class.isAssignableFrom(field.getType())) && field.isAnnotationPresent(RelatedTo.class)) { - result.addAll((Collection) getValueFromEntity(field)); - return true; - } - return false; - } - - - private boolean handleSingleField(HashSet result, Field field) { - if (NodeBacked.class.isAssignableFrom(field.getType())) { - Object value = getValueFromEntity(field); - if (value!=null) { - result.add((NodeBacked) value); - } - return true; - } - return false; - } - public boolean refersTo(GraphBacked target) { - return getOutboundDirtyNodeEntities().contains(target); - } } - - - diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/support/node/Neo4jNodeBacking.aj b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/support/node/Neo4jNodeBacking.aj index e42c1c4a6..2d7675872 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/support/node/Neo4jNodeBacking.aj +++ b/spring-data-neo4j/src/main/java/org/springframework/data/graph/neo4j/support/node/Neo4jNodeBacking.aj @@ -119,9 +119,6 @@ public aspect Neo4jNodeBacking { // extends AbstractTypeAnnotatingMixinFields T NodeBacked.persist() { return (T)this.entityState.persist(); } - public boolean NodeBacked.refersTo(GraphBacked target) { - return this.entityState.refersTo(target); - } public void NodeBacked.setPersistentState(Node n) { if (this.entityState == null) { diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/HasRelationshipMatcher.java b/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/HasRelationshipMatcher.java index 24410ce16..bc2f9780d 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/HasRelationshipMatcher.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/HasRelationshipMatcher.java @@ -67,6 +67,10 @@ class HasRelationshipMatcher extends TypeSafeMatcher { description.appendText( "Expected relationship named " + relationshipTypeName + " to " +(other==null ? "unspecified": other)+"\r\n got: " ); + if (relationships == null) { + description.appendValue("[]"); + return; + } List types = new ArrayList(); for ( Relationship rel : relationships ) { diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/ModificationOutsideOfTransactionTest.java b/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/ModificationOutsideOfTransactionTest.java index a24a49209..17f005dbf 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/ModificationOutsideOfTransactionTest.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/graph/neo4j/support/ModificationOutsideOfTransactionTest.java @@ -23,7 +23,10 @@ import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.neo4j.graphdb.Node; +import org.neo4j.graphdb.NotFoundException; +import org.neo4j.graphdb.NotInTransactionException; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.graph.neo4j.Friendship; import org.springframework.data.graph.neo4j.Group; import org.springframework.data.graph.neo4j.Person; import org.springframework.data.graph.neo4j.repository.DirectGraphRepositoryFactory; @@ -43,7 +46,6 @@ import static org.springframework.data.graph.neo4j.support.HasRelationshipMatche @RunWith( SpringJUnit4ClassRunner.class ) @ContextConfiguration( locations = {"classpath:org/springframework/data/graph/neo4j/support/Neo4jGraphPersistenceTest-context.xml"} ) - public class ModificationOutsideOfTransactionTest { @@ -71,7 +73,6 @@ public class ModificationOutsideOfTransactionTest } @Test - @Ignore("ignored until subgraph persisting is added") public void testCreateSubgraphOutsideOfTransactionPersistInDirectionOfRel() { Person michael = new Person("Michael", 35); Person emil = new Person("Emil", 31); @@ -87,6 +88,48 @@ public class ModificationOutsideOfTransactionTest } + @Test + public void testCreateSubgraphOutsideOfTransactionPersistWithImmediateCycle() { + Person michael = new Person("Michael", 35); + Person emil = new Person("Emil", 31); + + michael.setBoss(emil); + emil.setBoss(michael); + + assertEquals(emil, michael.getBoss()); + assertEquals(michael, emil.getBoss()); + assertFalse(hasPersistentState(michael)); + assertFalse(hasPersistentState(emil)); + michael.persist(); + assertThat(nodeFor(michael), hasRelationship("boss", nodeFor(emil))); + assertThat(nodeFor(emil), hasRelationship("boss", nodeFor(michael))); + } + + @Test + public void testCreateSubgraphOutsideOfTransactionPersistWithCycle() { + Person michael = new Person("Michael", 35); + Person david = new Person("David", 27); + Person emil = new Person("Emil", 31); + + michael.setBoss(emil); + david.setBoss(michael); + emil.setBoss(david); + + assertEquals(emil, michael.getBoss()); + assertEquals(michael, david.getBoss()); + assertEquals(david, emil.getBoss()); + assertFalse(hasPersistentState(michael)); + assertFalse(hasPersistentState(david)); + assertFalse(hasPersistentState(emil)); + michael.persist(); + assertThat(nodeFor(michael), hasRelationship("boss", nodeFor(emil))); + assertThat(nodeFor(michael), hasRelationship("boss", nodeFor(david))); + assertThat(nodeFor(david), hasRelationship("boss", nodeFor(michael))); + assertThat(nodeFor(david), hasRelationship("boss", nodeFor(emil))); + assertThat(nodeFor(emil), hasRelationship("boss", nodeFor(david))); + assertThat(nodeFor(emil), hasRelationship("boss", nodeFor(michael))); + } + @Ignore("ignored until subgraph persisting is added") @Test public void testCreateSubgraphOutsideOfTransactionPersistInReverseDirectionOfRel() { @@ -103,6 +146,14 @@ public class ModificationOutsideOfTransactionTest assertThat(nodeFor(emil), hasRelationship("boss", nodeFor(michael))); } + // TODO: Would be nice if this worked outside of a tx + @Test(expected = NotInTransactionException.class) + public void foo() { + Person p = persistedPerson("Michael", 35); + Person p2 = persistedPerson("David", 26); + Friendship f = p.knows(p2); + } + @Test public void testSetPropertyOutsideTransaction() {