Port to Neo4j 4.1.7

Most some internal API changes:
* Transaction type names
* IndexSeek API change
* Stricter checks on parameters containing Node objects

This last change means that if we return a Node from one transaction, we cannot pass it into a spatial procedure in another transaction. Users have to either lookup the node again in the new transaction, or use some new procedures we provide here:

* spatial.addNode.byId
* spatial.addNodes.byId
* spatial.removeNode.byId
* spatial.removeNodes.byId

Also the removeNode procedures now return `nodeId` instead of `node`

README.md

@@ -64,10 +64,14 @@ This has meant that the spatial library needed a major refactoring to work with
   It was therefor necessary to upgrade the GeoTools libraries to version 24.2.
   This in turn required a re-write of the Neo4jDataStore interface since the older API had
   long been deprecated, and was entirely unavailable in newer versions.
+* Neo4j 4.1 was slightly stricter with regards to passing nodes as parameters, requiring the nodes
+  objects to have been created in the current transaction.
+  To work around this we added `.byId` versions of the `spatial.addNode` and `spatial.removeNode` procedures.
+  We also changed the `spatial.removeNode` procedures to return `nodeId` instead of `node`.
 
-Consequences of this port:
+Consequences of the port to Neo4j 4.x:
 
-* The large number of changes mean that the 0.27.0 version should be considered very alpha.
+* The large number of changes mean that the 0.27.x versions should be considered very alpha.
 * Many API's have changed and client code might need to be adapted to take the changes into account.
 * The new DataStore API is entirely untested in GeoServer, besides the existing unit and integration tests.
 * The need to manage threads and create schema indexes results in the procedures requiring
@@ -335,6 +339,7 @@ The Neo4j Spatial Plugin is available for inclusion in the server version of Neo
   * [v0.26.2 for Neo4j 3.5.2](https://github.com/neo4j-contrib/m2/blob/master/releases/org/neo4j/neo4j-spatial/0.26.2-neo4j-3.5.2/neo4j-spatial-0.26.2-neo4j-3.5.2-server-plugin.jar?raw=true)
 * Using GeoTools 24.2 (for GeoServer 2.18.x):
   * [v0.27.0 for Neo4j 4.0.3](https://github.com/neo4j-contrib/m2/blob/master/releases/org/neo4j/neo4j-spatial/0.27.0-neo4j-4.0.3/neo4j-spatial-0.27.0-neo4j-4.0.3-server-plugin.jar?raw=true)
+  * [v0.27.1 for Neo4j 4.1.7](https://github.com/neo4j-contrib/m2/blob/master/releases/org/neo4j/neo4j-spatial/0.27.1-neo4j-4.1.7/neo4j-spatial-0.27.1-neo4j-4.1.7-server-plugin.jar?raw=true)
 
 For versions up to 0.15-neo4j-2.3.4:
 
@@ -451,7 +456,7 @@ Add the following repositories and dependency to your project's pom.xml:
     <dependency>
         <groupId>org.neo4j</groupId>
         <artifactId>neo4j-spatial</artifactId>
-        <version>0.27.0-neo4j-4.0.3</version>
+        <version>0.27.1-neo4j-4.1.7</version>
     </dependency>
 ~~~
 

pom.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
     <properties>
-        <neo4j.version>4.0.3</neo4j.version>
+        <neo4j.version>4.1.7</neo4j.version>
         <lucene.version>8.2.0</lucene.version>
         <!-- make sure lucene version is the same as the one the current neo4j depends on -->
         <neo4j.java.version>11</neo4j.java.version>
@@ -23,7 +23,7 @@
     <modelVersion>4.0.0</modelVersion>
     <artifactId>neo4j-spatial</artifactId>
     <groupId>org.neo4j</groupId>
-    <version>0.27.0-neo4j-4.0.3</version>
+    <version>0.27.1-neo4j-4.1.7</version>
     <name>Neo4j - Spatial Components</name>
     <description>Spatial utilities and components for Neo4j</description>
     <url>http://components.neo4j.org/${project.artifactId}/${project.version}</url>

src/main/java/org/neo4j/gis/spatial/AbstractGeometryEncoder.java

@@ -19,7 +19,7 @@
  */
 package org.neo4j.gis.spatial;
 
-import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang3.ArrayUtils;
 import org.neo4j.gis.spatial.rtree.Envelope;
 import org.neo4j.graphdb.Node;
 import org.neo4j.graphdb.Entity;

src/main/java/org/neo4j/gis/spatial/index/IndexManager.java

@@ -150,14 +150,14 @@ private IndexMaker(String indexName, Label label, String propertyKey) {
         @Override
         public void run() {
             try {
-                try (Transaction tx = db.beginTransaction(KernelTransaction.Type.explicit, securityContext)) {
+                try (Transaction tx = db.beginTransaction(KernelTransaction.Type.EXPLICIT, securityContext)) {
                     index = findIndex(tx);
                     if (index == null) {
                         index = tx.schema().indexFor(label).withName(indexName).on(propertyKey).create();
                     }
                     tx.commit();
                 }
-                try (Transaction tx = db.beginTransaction(KernelTransaction.Type.explicit, securityContext)) {
+                try (Transaction tx = db.beginTransaction(KernelTransaction.Type.EXPLICIT, securityContext)) {
                     tx.schema().awaitIndexOnline(indexName, 30, TimeUnit.SECONDS);
                 }
             } catch (Exception e) {
@@ -208,7 +208,7 @@ private IndexRemover(IndexDefinition index) {
         @Override
         public void run() {
             try {
-                try (Transaction tx = db.beginTransaction(KernelTransaction.Type.explicit, securityContext)) {
+                try (Transaction tx = db.beginTransaction(KernelTransaction.Type.EXPLICIT, securityContext)) {
                     // Need to find and drop in the same transaction due to saved state in the index definition implementation
                     IndexDefinition found = tx.schema().getIndexByName(index.getName());
                     if (found != null) {

src/main/java/org/neo4j/gis/spatial/index/LayerSpaceFillingCurvePointIndex.java

@@ -26,13 +26,16 @@
 import org.neo4j.gis.spatial.index.curves.StandardConfiguration;
 import org.neo4j.gis.spatial.rtree.filter.AbstractSearchEnvelopeIntersection;
 import org.neo4j.gis.spatial.rtree.filter.SearchFilter;
-import org.neo4j.graphdb.*;
+import org.neo4j.graphdb.Label;
+import org.neo4j.graphdb.Node;
+import org.neo4j.graphdb.ResourceIterator;
+import org.neo4j.graphdb.Transaction;
 import org.neo4j.internal.helpers.collection.Iterators;
 import org.neo4j.internal.kernel.api.*;
 import org.neo4j.internal.schema.IndexDescriptor;
-import org.neo4j.internal.schema.IndexOrder;
 import org.neo4j.internal.schema.IndexType;
 import org.neo4j.internal.schema.SchemaDescriptor;
+import org.neo4j.io.pagecache.tracing.cursor.PageCursorTracer;
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.kernel.impl.core.NodeEntity;
 import org.neo4j.kernel.impl.coreapi.internal.NodeCursorResourceIterator;
@@ -142,9 +145,9 @@ private ResourceIterator<Node> nodesByLabelAndProperty(KernelTransaction transac
                 // Ha! We found an index - let's use it to find matching nodes
                 try
                 {
-                    NodeValueIndexCursor cursor = transaction.cursors().allocateNodeValueIndexCursor();
+                    NodeValueIndexCursor cursor = transaction.cursors().allocateNodeValueIndexCursor(PageCursorTracer.NULL);
                     IndexReadSession indexSession = read.indexReadSession( index );
-                    read.nodeIndexSeek( indexSession, cursor, IndexOrder.NONE, false, query );
+                    read.nodeIndexSeek( indexSession, cursor, IndexQueryConstraints.unordered(false), query );
 
                     return new NodeCursorResourceIterator<>( cursor, (id) -> new NodeEntity(transaction.internalTransaction(), id) );
                 }

src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java

@@ -33,13 +33,11 @@
 import org.neo4j.graphdb.schema.IndexDefinition;
 import org.neo4j.graphdb.traversal.Evaluators;
 import org.neo4j.graphdb.traversal.TraversalDescription;
-import org.neo4j.internal.kernel.api.security.AccessMode;
 import org.neo4j.internal.kernel.api.security.SecurityContext;
 import org.neo4j.io.fs.FileUtils;
 import org.neo4j.io.layout.DatabaseLayout;
 import org.neo4j.io.layout.Neo4jLayout;
 import org.neo4j.kernel.api.KernelTransaction;
-import org.neo4j.kernel.impl.api.security.OverriddenAccessMode;
 import org.neo4j.kernel.impl.traversal.MonoDirectionalTraversalDescription;
 import org.neo4j.kernel.internal.GraphDatabaseAPI;
 
@@ -202,7 +200,7 @@ private Transaction beginTx(GraphDatabaseService database) {
         if (!(database instanceof GraphDatabaseAPI)) {
             throw new IllegalArgumentException("database must implement GraphDatabaseAPI");
         }
-        return ((GraphDatabaseAPI) database).beginTransaction(KernelTransaction.Type.explicit, securityContext);
+        return ((GraphDatabaseAPI) database).beginTransaction(KernelTransaction.Type.EXPLICIT, securityContext);
     }
 
     public long reIndex(GraphDatabaseService database) {
@@ -902,7 +900,7 @@ private static Transaction beginTx(GraphDatabaseService database, SecurityContex
             if (!(database instanceof GraphDatabaseAPI)) {
                 throw new IllegalArgumentException("database must implement GraphDatabaseAPI");
             }
-            return ((GraphDatabaseAPI) database).beginTransaction(KernelTransaction.Type.explicit, securityContext);
+            return ((GraphDatabaseAPI) database).beginTransaction(KernelTransaction.Type.EXPLICIT, securityContext);
         }
 
         private void beginTx() {

src/main/java/org/neo4j/gis/spatial/procedures/SpatialProcedures.java

@@ -64,6 +64,7 @@
 import java.util.*;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static org.neo4j.gis.spatial.SpatialDatabaseService.RTREE_INDEX_NAME;
@@ -106,6 +107,14 @@ public NodeResult(Node node) {
         }
     }
 
+    public static class NodeIdResult {
+        public final long nodeId;
+
+        public NodeIdResult(long nodeId) {
+            this.nodeId = nodeId;
+        }
+    }
+
     public static class CountResult {
         public final long count;
 
@@ -490,6 +499,10 @@ private Stream<NodeResult> streamNode(Node node) {
         return Stream.of(new NodeResult(node));
     }
 
+    private Stream<NodeIdResult> streamNode(long nodeId) {
+        return Stream.of(new NodeIdResult(nodeId));
+    }
+
     @Procedure(value="spatial.addWKTLayer", mode=WRITE)
     @Description("Adds a new WKT layer with the given node property to hold the WKT string, returns the layer root node")
     public Stream<NodeResult> addWKTLayer(@Name("name") String name,
@@ -540,12 +553,27 @@ public Stream<CountResult> addNodesToLayer(@Name("layerName") String name, @Name
         return Stream.of(new CountResult(layer.addAll(tx, nodes)));
     }
 
+    @Procedure(value="spatial.addNode.byId", mode=WRITE)
+    @Description("Adds the given node to the layer, returns the geometry-node")
+    public Stream<NodeResult> addNodeIdToLayer(@Name("layerName") String name, @Name("nodeId") long nodeId) {
+        EditableLayer layer = getEditableLayerOrThrow(tx, spatial(), name);
+        return streamNode(layer.add(tx, tx.getNodeById(nodeId)).getGeomNode());
+    }
+
+    @Procedure(value="spatial.addNodes.byId", mode=WRITE)
+    @Description("Adds the given nodes list to the layer, returns the count")
+    public Stream<CountResult> addNodeIdsToLayer(@Name("layerName") String name, @Name("nodeIds") List<Long> nodeIds) {
+        EditableLayer layer = getEditableLayerOrThrow(tx, spatial(), name);
+        List<Node> nodes = nodeIds.stream().map(id -> tx.getNodeById(id)).collect(Collectors.toList());
+        return Stream.of(new CountResult(layer.addAll(tx, nodes)));
+    }
+
     @Procedure(value="spatial.removeNode", mode=WRITE)
     @Description("Removes the given node from the layer, returns the geometry-node")
-    public Stream<NodeResult> removeNodeFromLayer(@Name("layerName") String name, @Name("node") Node node) {
+    public Stream<NodeIdResult> removeNodeFromLayer(@Name("layerName") String name, @Name("node") Node node) {
         EditableLayer layer = getEditableLayerOrThrow(tx, spatial(), name);
         layer.removeFromIndex(tx, node.getId());
-        return streamNode(node);
+        return streamNode(node.getId());
     }
 
     @Procedure(value="spatial.removeNodes", mode=WRITE)
@@ -561,6 +589,27 @@ public Stream<CountResult> removeNodesFromLayer(@Name("layerName") String name,
         return Stream.of(new CountResult(before - after));
     }
 
+    @Procedure(value="spatial.removeNode.byId", mode=WRITE)
+    @Description("Removes the given node from the layer, returns the geometry-node")
+    public Stream<NodeIdResult> removeNodeFromLayer(@Name("layerName") String name, @Name("nodeId") long nodeId) {
+        EditableLayer layer = getEditableLayerOrThrow(tx, spatial(), name);
+        layer.removeFromIndex(tx, nodeId);
+        return streamNode(nodeId);
+    }
+
+    @Procedure(value="spatial.removeNodes.byId", mode=WRITE)
+    @Description("Removes the given nodes from the layer, returns the count of nodes removed")
+    public Stream<CountResult> removeNodeIdsFromLayer(@Name("layerName") String name, @Name("nodeIds") List<Long> nodeIds) {
+        EditableLayer layer = getEditableLayerOrThrow(tx, spatial(), name);
+        //TODO optimize bulk node removal from RTree like we have done for node additions
+        int before = layer.getIndex().count(tx);
+        for (long nodeId : nodeIds) {
+            layer.removeFromIndex(tx, nodeId);
+        }
+        int after = layer.getIndex().count(tx);
+        return Stream.of(new CountResult(before - after));
+    }
+
     @Procedure(value="spatial.addWKT", mode=WRITE)
     @Description("Adds the given WKT string to the layer, returns the created geometry node")
     public Stream<NodeResult> addGeometryWKTToLayer(@Name("layerName") String name, @Name("geometry") String geometryWKT) throws ParseException {
@@ -682,7 +731,7 @@ long getResult() {
         public void run() {
             // Create the layer in the same thread as doing the import, otherwise we have an outer thread doing a create,
             // and the inner thread repeating it, resulting in duplicates
-            try (Transaction tx = db.beginTransaction(KernelTransaction.Type.explicit, securityContext)) {
+            try (Transaction tx = db.beginTransaction(KernelTransaction.Type.EXPLICIT, securityContext)) {
                 layerMaker.apply(tx, layerName);
                 tx.commit();
             }

src/test/java/org/neo4j/gis/spatial/Neo4jTestCase.java

@@ -56,8 +56,6 @@ public abstract class Neo4jTestCase {
         //NORMAL_CONFIG.put( GraphDatabaseSettings.strings_mapped_memory_size.name(), "200M" );
         //NORMAL_CONFIG.put( GraphDatabaseSettings.arrays_mapped_memory_size.name(), "0M" );
         NORMAL_CONFIG.put(GraphDatabaseSettings.pagecache_memory.name(), "200M");
-        NORMAL_CONFIG.put(GraphDatabaseSettings.batch_inserter_batch_size.name(), "2");
-        NORMAL_CONFIG.put(GraphDatabaseSettings.dump_configuration.name(), "false");
     }
 
     static final Map<String, String> LARGE_CONFIG = new HashMap<>();
@@ -69,8 +67,6 @@ public abstract class Neo4jTestCase {
         //LARGE_CONFIG.put( GraphDatabaseSettings.strings_mapped_memory_size.name(), "800M" );
         //LARGE_CONFIG.put( GraphDatabaseSettings.arrays_mapped_memory_size.name(), "10M" );
         LARGE_CONFIG.put(GraphDatabaseSettings.pagecache_memory.name(), "100M");
-        LARGE_CONFIG.put(GraphDatabaseSettings.batch_inserter_batch_size.name(), "2");
-        LARGE_CONFIG.put(GraphDatabaseSettings.dump_configuration.name(), "true");
     }
 
     private static final File basePath = new File("target/var");

src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java

@@ -132,7 +132,7 @@ public static void registerProceduresAndFunctions(GraphDatabaseService db, Class
     public void add_node_to_non_existing_layer() {
         execute("CALL spatial.addPointLayer('some_name')");
         Node node = createNode("CREATE (n:Point {latitude:60.1,longitude:15.2}) RETURN n", "n");
-        testCallFails(db, "CALL spatial.addNode('wrong_name',$node)", map("node", node), "No such layer 'wrong_name'");
+        testCallFails(db, "CALL spatial.addNode.byId('wrong_name',$nodeId)", map("nodeId", node.getId()), "No such layer 'wrong_name'");
     }
 
     @Test
@@ -694,38 +694,46 @@ public void add_a_node_to_the_spatial_index_short_with_geohash() {
     @Test
     public void add_two_nodes_to_the_spatial_layer() {
         execute("CALL spatial.addPointLayerXY('geom','lon','lat')");
-        Node node1;
-        Node node2;
+        long node1;
+        long node2;
         try (Transaction tx = db.beginTx()) {
             Result result = tx.execute("CREATE (n1:Node {lat:60.1,lon:15.2}),(n2:Node {lat:60.1,lon:15.3}) WITH n1,n2 CALL spatial.addNodes('geom',[n1,n2]) YIELD count RETURN n1,n2,count");
             Map<String, Object> row = result.next();
-            node1 = (Node) row.get("n1");
-            node2 = (Node) row.get("n2");
+            node1 = ((Node) row.get("n1")).getId();
+            node2 = ((Node) row.get("n2")).getId();
             long count = (Long) row.get("count");
             Assert.assertEquals(2L, count);
             result.close();
             tx.commit();
         }
         testResult(db, "CALL spatial.withinDistance('geom',{lon:15.0,lat:60.0},100)", res -> {
-                    assertTrue(res.hasNext());
-                    assertEquals(node1, res.next().get("node"));
-                    assertTrue(res.hasNext());
-                    assertEquals(node2, res.next().get("node"));
-                    assertFalse(res.hasNext());
-                }
-        );
+            assertTrue(res.hasNext());
+            assertEquals(node1, ((Node) res.next().get("node")).getId());
+            assertTrue(res.hasNext());
+            assertEquals(node2, ((Node) res.next().get("node")).getId());
+            assertFalse(res.hasNext());
+        });
         try (Transaction tx = db.beginTx()) {
-            Result removeResult = tx.execute("CALL spatial.removeNode('geom',$node) YIELD node RETURN node", map("node", node1));
-            Assert.assertEquals(node1, removeResult.next().get("node"));
+            Node node = (Node) tx.execute("MATCH (node) WHERE id(node) = $nodeId RETURN node", map("nodeId", node1)).next().get("node");
+            Result removeResult = tx.execute("CALL spatial.removeNode('geom',$node) YIELD nodeId RETURN nodeId", map("node", node));
+            Assert.assertEquals(node1, removeResult.next().get("nodeId"));
             removeResult.close();
             tx.commit();
         }
         testResult(db, "CALL spatial.withinDistance('geom',{lon:15.0,lat:60.0},100)", res -> {
-                    assertTrue(res.hasNext());
-                    assertEquals(node2, res.next().get("node"));
-                    assertFalse(res.hasNext());
-                }
-        );
+            assertTrue(res.hasNext());
+            assertEquals(node2, ((Node) res.next().get("node")).getId());
+            assertFalse(res.hasNext());
+        });
+        try (Transaction tx = db.beginTx()) {
+            Result removeResult = tx.execute("CALL spatial.removeNode.byId('geom',$nodeId) YIELD nodeId RETURN nodeId", map("nodeId", node2));
+            Assert.assertEquals(node2, removeResult.next().get("nodeId"));
+            removeResult.close();
+            tx.commit();
+        }
+        testResult(db, "CALL spatial.withinDistance('geom',{lon:15.0,lat:60.0},100)", res -> {
+            assertFalse(res.hasNext());
+        });
     }
 
     @Test
@@ -793,9 +801,9 @@ private void testRemoveNode(String layer, int count) {
         String remove = "UNWIND range(1,$count) as i\n" +
                 "MATCH (n:Point {id:i})\n" +
                 "WITH n\n" +
-                "CALL spatial.removeNode('" + layer + "',n) YIELD node\n" +
-                "RETURN count(node)";
-        testCountQuery("removeNode", remove, count / 2, "count(node)", map("count", count / 2));
+                "CALL spatial.removeNode('" + layer + "',n) YIELD nodeId\n" +
+                "RETURN count(nodeId)";
+        testCountQuery("removeNode", remove, count / 2, "count(nodeId)", map("count", count / 2));
         // Check that only half remain
         testCountQuery("withinDistance", "CALL spatial.withinDistance('" + layer + "',{lon:15.0,lat:60.0},1000) YIELD node RETURN count(node)", count / 2, "count(node)", null);
     }