From 57ade2ea69f3f1dbbc2dc790ffd7b486f5ac6168 Mon Sep 17 00:00:00 2001 From: Michael Hunger Date: Wed, 1 Oct 2014 14:39:52 +0200 Subject: [PATCH] DATAGRAPH-253 Better abstraction layer for interfacing with Remote Neo4j dedicated API to sync a set of relationships --- .../java/org/neo4j/rest/graphdb/RestAPI.java | 4 + .../neo4j/rest/graphdb/RestAPICypherImpl.java | 84 +++++++++- .../org/neo4j/rest/graphdb/RestAPIImpl.java | 43 ++++- .../rest/graphdb/query/CypherTransaction.java | 9 + .../query/RestCypherTransactionManager.java | 6 +- .../rest/graphdb/traversal/RestDirection.java | 3 +- .../rest/SpringCypherRestGraphDatabase.java | 8 +- .../neo4j/rest/SpringRestGraphDatabase.java | 5 + .../org/neo4j/rest/graphdb/RestTestBase.java | 6 +- .../UpdateRelationshipRestApiImplTest.java | 16 ++ .../rest/graphdb/UpdateRelationshipTest.java | 154 ++++++++++++++++++ .../data/neo4j/core/GraphDatabase.java | 2 + .../neo4j/fieldaccess/RelationshipHelper.java | 98 +++++++---- .../support/DelegatingGraphDatabase.java | 12 ++ .../data/neo4j/support/Neo4jTemplate.java | 13 +- .../support/mapping/EntityStateHandler.java | 2 +- .../support/mapping/Neo4jEntityPersister.java | 4 + .../data/neo4j/template/Neo4jOperations.java | 2 + 18 files changed, 417 insertions(+), 54 deletions(-) create mode 100644 spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipRestApiImplTest.java create mode 100644 spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipTest.java diff --git a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPI.java b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPI.java index 4bb3a2da3..ccd7f610f 100644 --- a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPI.java +++ b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPI.java @@ -68,6 +68,8 @@ public interface RestAPI extends RestAPIIndex, RestAPIInternal { Transaction beginTx(); + Iterable updateRelationships(Node start, Collection endNodes, RelationshipType type, Direction direction, String targetLabel); + Collection getAllLabelNames(); Iterable getRelationshipTypes(); @@ -91,4 +93,6 @@ public interface RestAPI extends RestAPIIndex, RestAPIInternal { void removeFromCache(long id); void close(); + + Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props); } diff --git a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPICypherImpl.java b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPICypherImpl.java index 55811c4fb..5f7632dce 100644 --- a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPICypherImpl.java +++ b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPICypherImpl.java @@ -21,7 +21,7 @@ package org.neo4j.rest.graphdb; import org.neo4j.graphdb.*; import org.neo4j.graphdb.index.IndexHits; -import org.neo4j.graphdb.traversal.TraversalDescription; +import org.neo4j.helpers.Pair; import org.neo4j.helpers.collection.IterableWrapper; import org.neo4j.helpers.collection.IteratorUtil; import org.neo4j.helpers.collection.MapUtil; @@ -34,7 +34,6 @@ import org.neo4j.rest.graphdb.index.IndexInfo; import org.neo4j.rest.graphdb.index.RestIndex; import org.neo4j.rest.graphdb.index.RestIndexManager; import org.neo4j.rest.graphdb.query.*; -import org.neo4j.rest.graphdb.transaction.RemoteCypherTransaction; import org.neo4j.rest.graphdb.traversal.RestTraversalDescription; import org.neo4j.rest.graphdb.traversal.RestTraverser; import org.neo4j.rest.graphdb.util.QueryResult; @@ -44,7 +43,10 @@ import org.neo4j.rest.graphdb.util.ResultConverter; import javax.ws.rs.core.Response.Status; import java.util.*; +import static java.util.Arrays.asList; import static org.neo4j.helpers.collection.MapUtil.map; +import static org.neo4j.rest.graphdb.query.CypherTransaction.ResultType.row; +import static org.neo4j.rest.graphdb.query.CypherTransaction.Statement; public class RestAPICypherImpl implements RestAPI { @@ -168,8 +170,7 @@ public class RestAPICypherImpl implements RestAPI { @Override public RestNode createNode(Map props, Collection labels) { - Map data = props == null ? Collections.emptyMap() : props; - Iterator> result = query(createNodeQuery(labels), map("props", data)).getData().iterator(); + Iterator> result = query(createNodeQuery(labels), map("props", props(props))).getData().iterator(); if (result.hasNext()) { return addToCache(toNode(result.next())); } @@ -177,9 +178,10 @@ public class RestAPICypherImpl implements RestAPI { } @Override - public RestNode merge(String labelName, String key, Object value, final Map nodeProperties, Collection labels) { + public RestNode merge(String labelName, String key, Object value, Map nodeProperties, Collection labels) { if (labelName == null || key == null || value == null) throw new IllegalArgumentException("Label " + labelName + " key " + key + " and value must not be null"); + nodeProperties = props(nodeProperties); Map props = nodeProperties.containsKey(key) ? nodeProperties : MapUtil.copyAndPut(nodeProperties, key, value); Map params = map("props", props, "value", value); Iterator> result = query(mergeQuery(labelName, key, labels), params).getData().iterator(); @@ -196,7 +198,7 @@ public class RestAPICypherImpl implements RestAPI { @Override public RestRelationship createRelationship(Node startNode, Node endNode, RelationshipType type, Map props) { String statement = MATCH_NODE_QUERY("n") + MATCH_NODE_QUERY("m") + " CREATE (n)-[r:`" + type.name() + "`]->(m) SET r={props} " + _QUERY_RETURN_REL; - Map params = map("id_n", startNode.getId(), "id_m", endNode.getId(), "props", props == null ? Collections.emptyMap() : props); + Map params = map("id_n", startNode.getId(), "id_m", endNode.getId(), "props", props(props)); CypherTransaction.Result result = runQuery(statement, params); if (!result.hasData()) throw new RuntimeException("Error creating relationship from " + startNode + " to " + endNode + " type " + type.name()); @@ -268,6 +270,7 @@ public class RestAPICypherImpl implements RestAPI { if (types == null || types.length == 0) return ""; StringBuilder typeString = new StringBuilder(); for (RelationshipType type : types) { + if (type==null) continue; if (typeString.length() > 0) typeString.append("|"); typeString.append(':').append('`').append(type.name()).append("`"); } @@ -423,6 +426,18 @@ public class RestAPICypherImpl implements RestAPI { return new CypherTxResult(runQuery(statement, params)); } + private List runQueries(Collection statements) { + if (!txManager.isActive()) { + CypherTransaction tx = newCypherTransaction(); + tx.addAll(statements); + return tx.commit(); + } else { + CypherTransaction tx = txManager.getCypherTransaction(); + tx.addAll(statements); + return tx.send(); + } + } + private CypherTransaction.Result runQuery(String statement, Map params) { if (!txManager.isActive()) { return newCypherTransaction().commit(statement, params); @@ -431,7 +446,7 @@ public class RestAPICypherImpl implements RestAPI { } public CypherTransaction newCypherTransaction() { - return new CypherTransaction(this, CypherTransaction.ResultType.row); + return new CypherTransaction(this, row); } public QueryResult> query(String statement, Map params, ResultConverter resultConverter) { @@ -471,6 +486,61 @@ public class RestAPICypherImpl implements RestAPI { restAPI.close(); } + @Override + public Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props) { +/* + final Iterable existingRelationships = start.getRelationships(type, direction); + for (final Relationship existingRelationship : existingRelationships) { + if (existingRelationship != null && existingRelationship.getOtherNode(start).equals(end)) + return existingRelationship; + } + if (direction == Direction.INCOMING) { + return end.createRelationshipTo(start, type); + } else { + return start.createRelationshipTo(end, type); + } + + */ + String relPattern = relPattern(direction, type); + String statement = MATCH_NODE_QUERY("n") + MATCH_NODE_QUERY("m") + " MERGE (n)"+relPattern+"(m) ON CREATE SET r={props}" + _QUERY_RETURN_REL; + CypherTransaction.Result result = runQuery(statement, map("id_n", start.getId(), "id_m", end.getId(),"props", props(props))); + if (!result.hasData()) + throw new RuntimeException("Error creating relationship from " + start + " to " + end + " type " + type.name() +" direction "+direction); + return toRel(result.getRows().iterator().next()); + } + + @Override + public Iterable updateRelationships(Node start, Collection endNodes, RelationshipType type, Direction direction, String targetLabel) { + String targetLabelPredicate = targetLabel == null ? "" : " AND (m:`"+targetLabel+"` OR m:`_"+targetLabel+"`)"; + String relPattern = relPattern(direction, type); + String statement1 = "MATCH (n)"+relPattern+"(m) WHERE id(n) = {id_n} "+targetLabelPredicate+" AND NOT id(m) IN {ids_m} DELETE r RETURN id(r) as id_r"; + String statement2 = MATCH_NODE_QUERY("n") + " MATCH (m) WHERE id(m) IN {ids_m} MERGE (n)"+relPattern+"(m)" + _QUERY_RETURN_REL; + Map params = map("id_n", start.getId(), "ids_m", nodeIds(endNodes)); + List results = runQueries(asList( + new Statement(statement1, params, row), + new Statement(statement2, params, row))); + Iterable> mergeResults = results.get(1).getRows(); + return new IterableWrapper>(mergeResults) { + @Override + protected Relationship underlyingObjectToObject(List row) { + return toRel(row); + } + }; + } + + private long[] nodeIds(Collection nodes) { + long[] ids = new long[nodes.size()]; + int i=0; + for (Node node : nodes) { + ids[i++] = node.getId(); + } + return ids; + } + + public Map props(Map props) { + return props == null ? Collections.emptyMap() : props; + } + @Override public boolean isAutoIndexingEnabled(Class clazz) { return restAPI.isAutoIndexingEnabled(clazz); diff --git a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPIImpl.java b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPIImpl.java index ab0dd6093..29d7e65de 100644 --- a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPIImpl.java +++ b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/RestAPIImpl.java @@ -21,11 +21,9 @@ package org.neo4j.rest.graphdb; import org.neo4j.graphdb.*; import org.neo4j.graphdb.index.IndexHits; -import org.neo4j.graphdb.traversal.TraversalDescription; import org.neo4j.helpers.collection.IterableWrapper; import org.neo4j.helpers.collection.MapUtil; import org.neo4j.index.lucene.ValueContext; -import org.neo4j.rest.graphdb.batch.BatchRestAPI; import org.neo4j.rest.graphdb.entity.RestEntityCache; import org.neo4j.rest.graphdb.query.CypherRestResult; import org.neo4j.rest.graphdb.converter.RelationshipIterableConverter; @@ -49,6 +47,8 @@ import org.neo4j.rest.graphdb.traversal.RestTraverser; import org.neo4j.rest.graphdb.util.JsonHelper; import org.neo4j.rest.graphdb.util.QueryResult; import org.neo4j.rest.graphdb.util.ResultConverter; +import org.springframework.data.neo4j.mapping.Neo4jPersistentEntity; +import org.springframework.data.neo4j.support.mapping.Neo4jMappingContext; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; @@ -58,6 +58,7 @@ import java.io.UnsupportedEncodingException; import java.util.*; import java.util.concurrent.TimeUnit; +import static java.lang.String.format; import static javax.ws.rs.core.Response.Status.CREATED; import static org.neo4j.helpers.collection.MapUtil.map; import static org.neo4j.rest.graphdb.ExecutingRestRequest.encode; @@ -261,6 +262,44 @@ public class RestAPIImpl implements RestAPI { ExecutingRestRequest.shutdown(); } + // TODO + @Override + public Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props) { + final Iterable existingRelationships = start.getRelationships(type, direction); + for (final Relationship existingRelationship : existingRelationships) { + if (existingRelationship != null && existingRelationship.getOtherNode(start).equals(end)) + return existingRelationship; + } + if (direction == Direction.INCOMING) { + return createRelationship(end, start, type, props); + } else { + return createRelationship(start,end,type,props); + } + } + + protected Collection removeMissingRelationships(Node node, Collection targetNodes, + RelationshipType type, Direction direction, Label targetLabel) { + targetNodes = new ArrayList<>(targetNodes); + for (Relationship relationship : node.getRelationships( type, direction ) ) { + Node otherNode = relationship.getOtherNode(node); + if ( !targetNodes.remove(otherNode) ) { + if (targetLabel != null && !relationship.getOtherNode(node).hasLabel(targetLabel)) continue; + relationship.delete(); + } + } + return targetNodes; + } + @Override + public Iterable updateRelationships(final Node start, Collection endNodes, final RelationshipType type, final Direction direction, String targetLabelName) { + Label targetLabel = targetLabelName==null ? null : DynamicLabel.label(targetLabelName); + Collection remainingEndNodes = removeMissingRelationships(start, endNodes, type, direction, targetLabel); + List result=new ArrayList<>(remainingEndNodes.size()); + for (Node endNode : remainingEndNodes) { + result.add(getOrCreateRelationship(start, endNode,type,direction,null)); + } + return result; + } + @Override public RestNode merge(String labelName, String key, Object value, final Map nodeProperties, Collection labels) { if (labelName ==null || key == null || value==null) throw new IllegalArgumentException("Label "+ labelName +" key "+key+" and value must not be null"); diff --git a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/CypherTransaction.java b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/CypherTransaction.java index 920ac25e9..d8b116c7f 100644 --- a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/CypherTransaction.java +++ b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/CypherTransaction.java @@ -1,6 +1,7 @@ package org.neo4j.rest.graphdb.query; import com.sun.jersey.api.client.ClientResponse; +import org.neo4j.helpers.Pair; import org.neo4j.helpers.collection.IterableWrapper; import org.neo4j.helpers.collection.IteratorWrapper; import org.neo4j.rest.graphdb.*; @@ -136,6 +137,14 @@ public class CypherTransaction { private final RestRequest request; private final List statements = new ArrayList<>(10); + public void addAll(Statement...statements) { + this.statements.addAll(asList(statements)); + } + + public void addAll(Collection statements) { + this.statements.addAll(statements); + } + public void add(String statement, Map params) { statements.add(new Statement(statement,params,type)); } diff --git a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/RestCypherTransactionManager.java b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/RestCypherTransactionManager.java index eaa9b9890..66d503c2e 100644 --- a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/RestCypherTransactionManager.java +++ b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/query/RestCypherTransactionManager.java @@ -76,8 +76,10 @@ public class RestCypherTransactionManager implements TransactionManager, Transac @Override public void rollback() throws IllegalStateException, SecurityException, SystemException { RemoteCypherTransaction tx = getRemoteCypherTransaction(); - tx.failure(); - tx.close(); + if (tx!=null) { + tx.failure(); + tx.close(); + } } @Override diff --git a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/traversal/RestDirection.java b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/traversal/RestDirection.java index a620bff33..6f9f629ea 100644 --- a/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/traversal/RestDirection.java +++ b/spring-data-neo4j-rest/src/main/java/org/neo4j/rest/graphdb/traversal/RestDirection.java @@ -37,9 +37,10 @@ public enum RestDirection { } public static RestDirection from( Direction direction ) { + if (direction == null) return BOTH; for ( RestDirection restDirection : values() ) { if ( restDirection.direction == direction ) return restDirection; } throw new RuntimeException( "No Rest-Direction for " + direction ); } -} \ No newline at end of file +} diff --git a/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringCypherRestGraphDatabase.java b/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringCypherRestGraphDatabase.java index af2d5e7d0..fbfb19b9e 100644 --- a/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringCypherRestGraphDatabase.java +++ b/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringCypherRestGraphDatabase.java @@ -23,9 +23,6 @@ import org.neo4j.rest.graphdb.RestAPIImpl; import org.neo4j.rest.graphdb.entity.RestNode; import org.neo4j.rest.graphdb.index.RestIndex; import org.neo4j.rest.graphdb.index.RestIndexManager; -import org.neo4j.rest.graphdb.transaction.NullTransaction; -import org.neo4j.rest.graphdb.transaction.NullTransactionManager; -import org.neo4j.rest.graphdb.util.Config; import org.springframework.core.convert.ConversionService; import org.springframework.data.neo4j.conversion.DefaultConverter; import org.springframework.data.neo4j.conversion.ResultConverter; @@ -138,6 +135,11 @@ public class SpringCypherRestGraphDatabase extends CypherRestGraphDatabase imple return super.getTxManager(); } + @Override + public Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props) { + return getRestAPI().getOrCreateRelationship(start, end, type, direction, props); + } + @Override public void remove(Node node) { removeFromIndexes(node); // todo should we do this by default? diff --git a/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringRestGraphDatabase.java b/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringRestGraphDatabase.java index ef861771e..9b080262d 100644 --- a/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringRestGraphDatabase.java +++ b/spring-data-neo4j-rest/src/main/java/org/springframework/data/neo4j/rest/SpringRestGraphDatabase.java @@ -105,6 +105,11 @@ public class SpringRestGraphDatabase extends org.neo4j.rest.graphdb.RestGraphDat return getRestAPI().getOrCreateRelationship(relIndex,key,value,(RestNode) startNode,(RestNode) endNode,type, properties); } + @Override + public Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props) { + return getRestAPI().getOrCreateRelationship(start, end, type, direction,props); + } + @Override public Relationship createRelationship(Node startNode, Node endNode, RelationshipType type, Map properties) { return super.getRestAPI().createRelationship(startNode, endNode, type, properties); diff --git a/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/RestTestBase.java b/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/RestTestBase.java index 18b407b97..8ba43b93b 100644 --- a/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/RestTestBase.java +++ b/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/RestTestBase.java @@ -82,7 +82,7 @@ public class RestTestBase { public void setUp() throws Exception { System.setProperty(Config.CONFIG_BATCH_TRANSACTION,"false"); neoServer.cleanDb(); - restGraphDb = new RestGraphDatabase(SERVER_ROOT_URI); + restGraphDb = createRestGraphDatabase(); GraphDatabaseService db = getGraphDatabase(); try (Transaction tx = db.beginTx()) { @@ -94,6 +94,10 @@ public class RestTestBase { } + protected GraphDatabaseService createRestGraphDatabase() { + return new RestGraphDatabase(SERVER_ROOT_URI); + } + @After public void tearDown() throws Exception { restGraphDb.shutdown(); diff --git a/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipRestApiImplTest.java b/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipRestApiImplTest.java new file mode 100644 index 000000000..323d120c5 --- /dev/null +++ b/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipRestApiImplTest.java @@ -0,0 +1,16 @@ +package org.neo4j.rest.graphdb; + +import org.neo4j.graphdb.GraphDatabaseService; + +/** + * @author mh + * @since 30.09.14 + */ +public class UpdateRelationshipRestApiImplTest extends UpdateRelationshipTest { + + @Override + protected GraphDatabaseService createRestGraphDatabase() { + restAPI = new RestAPIImpl(SERVER_ROOT_URI); + return new RestGraphDatabase(restAPI); + } +} diff --git a/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipTest.java b/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipTest.java new file mode 100644 index 000000000..2cd66c341 --- /dev/null +++ b/spring-data-neo4j-rest/src/test/java/org/neo4j/rest/graphdb/UpdateRelationshipTest.java @@ -0,0 +1,154 @@ +package org.neo4j.rest.graphdb; + +import org.junit.Before; +import org.junit.Test; +import org.neo4j.graphdb.*; +import org.neo4j.rest.graphdb.query.CypherResult; +import org.neo4j.rest.graphdb.query.CypherTransactionExecutionException; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static java.util.Arrays.asList; +import static org.junit.Assert.assertEquals; +import static org.neo4j.helpers.collection.IteratorUtil.count; +import static org.neo4j.helpers.collection.MapUtil.map; + +public class UpdateRelationshipTest extends RestTestBase { + + public static final DynamicRelationshipType KNOWS = DynamicRelationshipType.withName("KNOWS"); + public static final Label FRIEND = DynamicLabel.label("Friend"); + private DynamicRelationshipType type = KNOWS; + public static final DynamicRelationshipType LIKES = DynamicRelationshipType.withName("LIKES"); + private Direction direction = Direction.OUTGOING; + private Node remove; + private Node keep; + private Node add; + protected RestAPI restAPI; + private List expected; + private List updateTo; + private String targetLabel = null; + + @Override + protected GraphDatabaseService createRestGraphDatabase() { + restAPI = new RestAPICypherImpl(new RestAPIImpl(SERVER_ROOT_URI)); + return new CypherRestGraphDatabase(restAPI); + } + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + + GraphDatabaseService db = getRestGraphDb(); + remove = db.createNode(); + keep = db.createNode(); + add = db.createNode(); + } + + @Test + public void testUpdateRelationshipsRemoveAddNoDirection() throws Exception { + Node node = node(); + remove.createRelationshipTo(node, KNOWS); + node.createRelationshipTo(keep, KNOWS); + direction = null; + updateTo = asList(keep,add); + updateRelationships(); + } + + @Test + public void testUpdateRelationshipsRemoveAddBothDirection() throws Exception { + Node node = node(); + remove.createRelationshipTo(node, KNOWS); + node.createRelationshipTo(keep, KNOWS); + direction = Direction.BOTH; + updateTo = asList(keep,add); + updateRelationships(); + } + + @Test(expected = CypherTransactionExecutionException.class) + public void testUpdateRelationshipsRemoveAddNoType() throws Exception { + Node node = node(); + remove.createRelationshipTo(node, LIKES); + node.createRelationshipTo(keep, KNOWS); + type = null; + updateTo = asList(keep); + updateRelationships(); + } + + @Test + public void testUpdateRelationshipsRemoveAdd() throws Exception { + Node node = node(); + node.createRelationshipTo(remove, KNOWS); + node.createRelationshipTo(keep, KNOWS); + updateTo = asList(keep, add); + updateRelationships(); + } + + @Test + public void testUpdateRelationshipsRemoveAddKeepOtherType() throws Exception { + Node node = node(); + node.createRelationshipTo(remove, LIKES); + node.createRelationshipTo(remove, KNOWS); + node.createRelationshipTo(keep, KNOWS); + updateTo = asList(keep, add); + expected = asList(remove, keep, add); + updateRelationships(); + } + @Test + public void testUpdateRelationshipsRemoveAddKeepOtherDirection() throws Exception { + Node node = node(); + remove.createRelationshipTo(node, KNOWS); + node.createRelationshipTo(remove, KNOWS); + node.createRelationshipTo(keep, KNOWS); + updateTo = asList(keep, add); + expected = asList(remove, keep, add); + updateRelationships(); + } + @Test + public void testUpdateRelationshipsRemoveAddKeepOtherLabel() throws Exception { + Node node = node(); + remove.addLabel(FRIEND); + node.createRelationshipTo(remove, KNOWS); + node.createRelationshipTo(keep, KNOWS); + updateTo = asList(); + expected = asList(keep); + targetLabel = FRIEND.name(); + updateRelationships(); + } + + @Test + public void testUpdateRelationshipNothing() throws Exception { + updateTo = asList(); + updateRelationships(); + } + @Test + public void testUpdateRelationshipsAdd() throws Exception { + updateTo = asList(add); + updateRelationships(); + } + + @Test + public void testUpdateRelationshipsRemove() throws Exception { + Node node = node(); + node.createRelationshipTo(remove, KNOWS); + updateTo = asList(); + updateRelationships(); + } + + protected void updateRelationships() { + if (expected==null) expected = updateTo; + Iterable rels = restAPI.updateRelationships(node(), updateTo, type, direction, targetLabel); + CypherResult result = restAPI.query("MATCH (n)--(m) WHERE id(n) = {id} WITH id(m) as id ORDER BY id RETURN collect(id) as ids", map("id", node().getId())); + assertEquals(1, count(result.getData())); + assertEquals(toSortedIdList(expected), result.getData().iterator().next().get(0)); + } + + private List toSortedIdList(List endNodes) { + List expected = new ArrayList<>(endNodes.size()); + for (Node node : endNodes) expected.add((int) node.getId()); + Collections.sort(expected); + return expected; + } +} diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/core/GraphDatabase.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/core/GraphDatabase.java index c68365921..ea63e53dc 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/core/GraphDatabase.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/core/GraphDatabase.java @@ -136,4 +136,6 @@ public interface GraphDatabase { void shutdown(); Collection getAllLabelNames(); + + Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props); } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/RelationshipHelper.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/RelationshipHelper.java index 6836b13fe..bfe039840 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/RelationshipHelper.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/fieldaccess/RelationshipHelper.java @@ -24,8 +24,9 @@ import org.springframework.data.neo4j.support.mapping.Neo4jMappingContext; import org.springframework.data.neo4j.support.typerepresentation.LabelBasedNodeTypeRepresentationStrategy; import org.springframework.util.Assert; +import java.util.Collections; import java.util.HashSet; -import java.util.Iterator; +import java.util.Map; import java.util.Set; import static java.lang.String.format; @@ -47,20 +48,26 @@ public class RelationshipHelper { } private Iterable getOtherNodes(Node node) { - final Set result = new HashSet(); + final Set result = new HashSet<>(); for (final Relationship rel : node.getRelationships(type, direction)) { result.add(rel.getOtherNode(node)); } return result; } - protected Relationship obtainSingleRelationship(final Node start, final Node end) { - final Iterable existingRelationships = start.getRelationships(type, direction); - for (final Relationship existingRelationship : existingRelationships) { - if (existingRelationship != null && existingRelationship.getOtherNode(start).equals(end)) - return existingRelationship; + private Long getOtherNodeId(Node node, Relationship rel) { + long id = node.getId(); + if (rel.getStartNode().getId() == id) return rel.getEndNode().getId(); + if (rel.getEndNode().getId() == id) return rel.getStartNode().getId(); + throw new IllegalStateException("Node "+node+" is not connected to Relationship "+rel); + } + + private Iterable getOtherNodeIds(Node node) { + final Set result = new HashSet<>(); + for (final Relationship rel : node.getRelationships(type, direction)) { + result.add(getOtherNodeId(node, rel)); } - return start.createRelationshipTo(end, type); + return result; } protected Node checkAndGetNode(Object entity) { @@ -70,20 +77,44 @@ public class RelationshipHelper { throw new IllegalStateException("Entity must have a backing Node"); } + /* + + MATCH (n) WHERE id(n) = {id_n} + OPTIONAL MATCH (n)-[r]-(m) + + WITH n, collect(r) as rels, + FILTER(good in collect(r) WHERE id(startNode(r)) IN {ids_m} OR id(endNode(r)) IN {ids_m}) as keep, + FILTER(id in {ids_m} WHERE NOT (id(startNode(r)) = id OR id(endNode(r)) = id) as new + FOREACH (r in rels WHERE NOT r in keep | DELETE r) + // alternative + FOREACH (r in rels WHERE (startNode(r) = n AND id(endNode(r)) NOT IN {ids_m}) OR (endNode(r) = n AND id(startNode(r)) NOT IN {ids_m})) | DELETE r) + + WITH n,count(*), new + UNWIND new as id_m + MATCH (m) WHERE id(m) = id_m + CREATE (n)-[r:TYPE]-(m) + RETURN r + + + MATCH (n) WHERE id(n) = {id_n} + OPTIONAL MATCH (n)-[r]-(m) + WHERE not id(m) IN {ids_m} + DELETE r + + MATCH (n) WHERE id(n) = {id_n} + MATCH (m) WHERE id(m) IN {ids_m} + MERGE (n)-[:TYPE]->(m) + + */ protected void removeMissingRelationshipsInStoreAndKeepOnlyNewRelationShipsInSet( Node node, Set targetNodes, Class targetType ) { Neo4jMappingContext mappingContext = template.getInfrastructure().getMappingContext(); for ( Relationship relationship : node.getRelationships( type, direction ) ) { - if ( !targetNodes.remove( relationship.getOtherNode( node ) ) ) { + Node otherNode = relationship.getOtherNode(node); + if ( !targetNodes.remove(otherNode) ) { if ( targetType != null ) { - Object actualTargetType = tryDetermineTypeAssumingIndexBasedStrategy(relationship, node); - if (actualTargetType == null) { - actualTargetType = tryDetermineTypeAssumingLabelBasedStrategy(relationship, node); - } - if (actualTargetType == null) { - throw new RuntimeException("Neither a property or Label could be found to work out what the type of the node is at the other end of the relationship "); - } + Object actualTargetType = determineEndNodeType(otherNode); try { Neo4jPersistentEntity persistentEntity = mappingContext.getPersistentEntity(actualTargetType); if (! targetType.isAssignableFrom(persistentEntity.getType())) continue; @@ -97,8 +128,19 @@ public class RelationshipHelper { } } - private Object tryDetermineTypeAssumingLabelBasedStrategy(Relationship relationship,Node node) { - for (Label l : relationship.getOtherNode(node).getLabels()) { + private Object determineEndNodeType(Node otherNode) { + Object actualTargetType = otherNode.getProperty("__type__", null); + if (actualTargetType == null) { + actualTargetType = tryDetermineTypeAssumingLabelBasedStrategy(otherNode); + } + if (actualTargetType == null) { + throw new RuntimeException("Neither a property or Label could be found to work out what the type of the node is at the other end of the relationship "); + } + return actualTargetType; + } + + private Object tryDetermineTypeAssumingLabelBasedStrategy(Node otherNode) { + for (Label l : otherNode.getLabels()) { if (l.name().startsWith(LabelBasedNodeTypeRepresentationStrategy.LABELSTRATEGY_PREFIX)) { return l.name().substring(LabelBasedNodeTypeRepresentationStrategy.LABELSTRATEGY_PREFIX.length()); } @@ -107,10 +149,6 @@ public class RelationshipHelper { } - private Object tryDetermineTypeAssumingIndexBasedStrategy(Relationship relationship,Node node) { - return relationship.getOtherNode( node ).getProperty( "__type__" , null); - } - protected void createAddedRelationships(Node node, Set targetNodes) { for (Node targetNode : targetNodes) { createSingleRelationship(node, targetNode); @@ -134,8 +172,8 @@ public class RelationshipHelper { } protected Node getOrCreateState(Object value) { - final Node Node = getNode(value); - if (Node != null) return Node; + final Node node = getNode(value); + if (node != null) return node; final Object saved = template.save(value); final Node newState = getNode(saved); Assert.notNull(newState); @@ -156,16 +194,8 @@ public class RelationshipHelper { @SuppressWarnings("unchecked") protected Relationship createSingleRelationship(Node start, Node end) { if (end == null) return null; - switch (direction) { - case OUTGOING: - case BOTH: { // TODO both should actually check in both directions, perhaps have the obtain method get the direction instead and figure out what to do itself - return obtainSingleRelationship(start, end); - } - case INCOMING: - return obtainSingleRelationship(end, start); - default: - throw new InvalidDataAccessApiUsageException("invalid direction " + direction); - } + Map props = Collections.emptyMap(); + return template.getOrCreateRelationship(start, end, type, direction, props); } protected Iterable getStatesFromEntity(final Object entity) { diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/DelegatingGraphDatabase.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/DelegatingGraphDatabase.java index f28eb59ce..166e33d64 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/DelegatingGraphDatabase.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/DelegatingGraphDatabase.java @@ -279,6 +279,18 @@ public class DelegatingGraphDatabase implements GraphDatabase { return labels; } + @Override + public Relationship getOrCreateRelationship(Node start, Node end, RelationshipType type, Direction direction, Map props) { + final Iterable existingRelationships = start.getRelationships(type, direction); + for (final Relationship existingRelationship : existingRelationships) { + if (existingRelationship != null && existingRelationship.getOtherNode(start).equals(end)) + return existingRelationship; + } + Relationship rel = direction == Direction.INCOMING ? end.createRelationshipTo(start, type) : start.createRelationshipTo(end, type); + setProperties(rel, props); + return rel; + } + public GraphDatabaseService getGraphDatabaseService() { return delegate; } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jTemplate.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jTemplate.java index 2c470a1e1..21f2be689 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jTemplate.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/Neo4jTemplate.java @@ -241,15 +241,18 @@ public class Neo4jTemplate implements Neo4jOperations, ApplicationContextAware { return infrastructure.getEntityPersister().projectTo(entity, targetType, mappingPolicy, this); } - /** - * just sets the persistent state (i.e. Node or id) to the entity, doesn't copy any values/properties. - */ @Override public S getPersistentState(Object entity) { notNull(entity, "entity"); return infrastructure.getEntityPersister().getPersistentState(entity); } + @Override + public Number getId(Object entity) { + notNull(entity, "entity"); + return infrastructure.getEntityPersister().getId(entity); + } + public T setPersistentState(T entity, S state) { notNull(entity, "entity", state, "node or relationship"); infrastructure.getEntityPersister().setPersistentState(entity, state); @@ -759,4 +762,8 @@ public class Neo4jTemplate implements Neo4jOperations, ApplicationContextAware { public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { this.applicationContext = applicationContext; } + + public Relationship getOrCreateRelationship(final Node start, final Node end, RelationshipType type, Direction direction, Map props) { + return getGraphDatabase().getOrCreateRelationship(start, end, type, direction,props); + } } diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java index a47d2f7ea..525fdde4b 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/EntityStateHandler.java @@ -83,7 +83,7 @@ public class EntityStateHandler { return getId(entity) != null; } - private Number getId(Object entity) { + public Number getId(Object entity) { final Class type = entity.getClass(); final Neo4jPersistentEntityImpl persistentEntity = mappingContext.getPersistentEntity(type); final Object id = persistentEntity.getPersistentId(entity); diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityPersister.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityPersister.java index 305413a8d..33d7e4482 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityPersister.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/support/mapping/Neo4jEntityPersister.java @@ -221,6 +221,10 @@ public class Neo4jEntityPersister implements EntityPersister, Neo4jEntityConvert return entityStateHandler.getPersistentState(entity); } + public Number getId(Object entity) { + return entityStateHandler.getId(entity); + } + public Object persist( Object entity, final MappingPolicy mappingPolicy, final Neo4jTemplate template, RelationshipType annotationProvidedRelationshipType ) { diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/template/Neo4jOperations.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/template/Neo4jOperations.java index e8a190b2a..edf03c323 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/template/Neo4jOperations.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/template/Neo4jOperations.java @@ -261,6 +261,8 @@ public interface Neo4jOperations { */ T save(T entity); + Number getId(Object entity); + /** * Removes the given node or relationship entity or node or relationship from the graph, the entity is first removed * from all indexes and then deleted.