Merge pull request #379 from neo4j-contrib/0.28-neo4j-4.2

Initial support for multiple OSM imports

README.md

@@ -74,6 +74,13 @@ This has meant that the spatial library needed a major refactoring to work with
   and simply add on the rights to create tokens and indexes. In 0.27.2 we instead use `RestrictedAccessMode`
   to restrict the users access right to the built in `AccessModel.Static.SCHEMA` and then boost to enable
   index and token writes. The difference is subtle and should only be possible to notice in Enterprise Edition.
+* 0.28.0 tackles the ability to import multiple OSM files. The initial solution for Neo4j 4.x made use
+  of schema indexes keyed by the label and property. However, that means that all OSM imports would share
+  the same index. If they are completely disjointed data sets, this would not matter. But if you import
+  overlapping OSM files or different versions of the same file file, a mangled partial merger would result.
+  0.28.0 solves this by using different indexes, and keeping all imports completely separate.
+  The more complex problems of importing newer versions, and stitching together overlapping areas, are not
+  yet solved.
 
 Consequences of the port to Neo4j 4.x:
 
@@ -347,6 +354,7 @@ The Neo4j Spatial Plugin is available for inclusion in the server version of Neo
   * [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)
   * [v0.27.2 for Neo4j 4.2.3](https://github.com/neo4j-contrib/m2/blob/master/releases/org/neo4j/neo4j-spatial/0.27.2-neo4j-4.2.3/neo4j-spatial-0.27.2-neo4j-4.2.3-server-plugin.jar?raw=true)
+  * [v0.28.0 for Neo4j 4.2.3](https://github.com/neo4j-contrib/m2/blob/master/releases/org/neo4j/neo4j-spatial/0.28.0-neo4j-4.2.3/neo4j-spatial-0.28.0-neo4j-4.2.3-server-plugin.jar?raw=true)
 
 For versions up to 0.15-neo4j-2.3.4:
 
@@ -463,7 +471,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.2-neo4j-4.2.3</version>
+        <version>0.28.0-neo4j-4.2.3</version>
     </dependency>
 ~~~
 

pom.xml

@@ -23,7 +23,7 @@
     <modelVersion>4.0.0</modelVersion>
     <artifactId>neo4j-spatial</artifactId>
     <groupId>org.neo4j</groupId>
-    <version>0.27.2-neo4j-4.2.3</version>
+    <version>0.28.0-neo4j-4.2.3</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/Constants.java

@@ -19,51 +19,54 @@
  */
 package org.neo4j.gis.spatial;
 
+import org.neo4j.graphdb.Label;
+
 /**
  * @author Davide Savazzi
  */
 public interface Constants {
 
-	// Node properties
-	
-	String PROP_BBOX = "bbox";
-	String PROP_LAYER = "layer";
-	String PROP_LAYERNODEEXTRAPROPS = "layerprops";
-	String PROP_CRS = "layercrs";
-	String PROP_CREATIONTIME = "ctime";
-	String PROP_GEOMENCODER = "geomencoder";
-	String PROP_INDEX_CLASS = "index_class";
-	String PROP_GEOMENCODER_CONFIG = "geomencoder_config";
-	String PROP_INDEX_CONFIG = "index_config";
+    // Node properties
+
+    String PROP_BBOX = "bbox";
+    String PROP_LAYER = "layer";
+    String PROP_LAYERNODEEXTRAPROPS = "layerprops";
+    String PROP_CRS = "layercrs";
+    String PROP_CREATIONTIME = "ctime";
+    String PROP_GEOMENCODER = "geomencoder";
+    String PROP_INDEX_CLASS = "index_class";
+    String PROP_GEOMENCODER_CONFIG = "geomencoder_config";
+    String PROP_INDEX_CONFIG = "index_config";
     String PROP_LAYER_CLASS = "layer_class";
 
-	String PROP_TYPE = "gtype";
-	String PROP_QUERY = "query";
-	String PROP_WKB = "wkb";
-	String PROP_WKT = "wkt";
-	String PROP_GEOM = "geometry";
-
-	String[] RESERVED_PROPS = new String[] { 
-			PROP_BBOX,
-			PROP_LAYER, 
-			PROP_LAYERNODEEXTRAPROPS, 
-			PROP_CRS, 
-			PROP_CREATIONTIME, 
-			PROP_TYPE,
-			PROP_WKB,
-			PROP_WKT,
-			PROP_GEOM
-	};
-
-
-	// OpenGIS geometry type numbers 
-
-	int GTYPE_GEOMETRY = 0;
-	int GTYPE_POINT = 1;
-	int GTYPE_LINESTRING = 2; 
-	int GTYPE_POLYGON = 3;
-	int GTYPE_MULTIPOINT = 4; 	
-	int GTYPE_MULTILINESTRING = 5; 
-	int GTYPE_MULTIPOLYGON = 6; 
-
+    String PROP_TYPE = "gtype";
+    String PROP_QUERY = "query";
+    String PROP_WKB = "wkb";
+    String PROP_WKT = "wkt";
+    String PROP_GEOM = "geometry";
+
+    String[] RESERVED_PROPS = new String[]{
+            PROP_BBOX,
+            PROP_LAYER,
+            PROP_LAYERNODEEXTRAPROPS,
+            PROP_CRS,
+            PROP_CREATIONTIME,
+            PROP_TYPE,
+            PROP_WKB,
+            PROP_WKT,
+            PROP_GEOM
+    };
+
+    Label LABEL_LAYER = Label.label("SpatialLayer");
+
+    // OpenGIS geometry type numbers
+
+    int GTYPE_GEOMETRY = 0;
+    int GTYPE_POINT = 1;
+    int GTYPE_LINESTRING = 2;
+    int GTYPE_POLYGON = 3;
+    int GTYPE_MULTIPOINT = 4;
+    int GTYPE_MULTILINESTRING = 5;
+    int GTYPE_MULTIPOLYGON = 6;
+
 }

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

@@ -259,7 +259,6 @@ public Node getLayerNode(Transaction tx) {
     public void delete(Transaction tx, Listener monitor) {
         indexWriter.removeAll(tx, true, monitor);
         Node layerNode = getLayerNode(tx);
-        layerNode.getSingleRelationship(SpatialRelationshipTypes.LAYER, Direction.INCOMING).delete();
         layerNode.delete();
         layerNodeId = -1L;
     }

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

@@ -19,9 +19,9 @@
  */
 package org.neo4j.gis.spatial;
 
-import org.locationtech.jts.geom.*;
 import org.geotools.referencing.crs.AbstractCRS;
 import org.geotools.referencing.crs.DefaultGeographicCRS;
+import org.locationtech.jts.geom.*;
 import org.neo4j.gis.spatial.encoders.Configurable;
 import org.neo4j.gis.spatial.encoders.NativePointEncoder;
 import org.neo4j.gis.spatial.encoders.SimplePointEncoder;
@@ -32,7 +32,6 @@
 import org.neo4j.gis.spatial.utilities.LayerUtilities;
 import org.neo4j.gis.spatial.utilities.ReferenceNodes;
 import org.neo4j.graphdb.*;
-import org.neo4j.kernel.internal.GraphDatabaseAPI;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 
 import java.util.ArrayList;
@@ -49,24 +48,52 @@
 public class SpatialDatabaseService implements Constants {
 
     public final IndexManager indexManager;
-    private long spatialRoot = -1;
 
     public SpatialDatabaseService(IndexManager indexManager) {
         this.indexManager = indexManager;
     }
 
-    private Node getSpatialRoot(Transaction tx) {
-        if (spatialRoot < 0) {
-            spatialRoot = ReferenceNodes.getReferenceNode(tx, "spatial_root").getId();
+    public static void assertNotOldModel(Transaction tx) {
+        Node oldReferenceNode = ReferenceNodes.findDeprecatedReferenceNode(tx, "spatial_root");
+        if (oldReferenceNode != null) {
+            throw new IllegalStateException("Old reference node exists - please upgrade the spatial database to the new format");
+        }
+    }
+
+    public List<String> upgradeFromOldModel(Transaction tx) {
+        ArrayList<String> layersConverted = new ArrayList<>();
+        Node oldReferenceNode = ReferenceNodes.findDeprecatedReferenceNode(tx, "spatial_root");
+        if (oldReferenceNode != null) {
+            List<Node> layers = new ArrayList<>();
+
+            for (Relationship relationship : oldReferenceNode.getRelationships(Direction.OUTGOING, SpatialRelationshipTypes.LAYER)) {
+                layers.add(relationship.getEndNode());
+            }
+
+            for (Node layer : layers) {
+                Relationship fromRoot = layer.getSingleRelationship(SpatialRelationshipTypes.LAYER, Direction.INCOMING);
+                fromRoot.delete();
+                layer.addLabel(LABEL_LAYER);
+                layersConverted.add((String) layer.getProperty(PROP_LAYER));
+            }
+
+            if (oldReferenceNode.getRelationships().iterator().hasNext()) {
+                throw new IllegalStateException("Cannot upgrade - ReferenceNode 'spatial_root' still has relationships other than layers");
+            }
+
+            oldReferenceNode.delete();
         }
-        return tx.getNodeById(spatialRoot);
+        indexManager.makeIndexFor(tx, "SpatialLayers", LABEL_LAYER, PROP_LAYER);
+        return layersConverted;
     }
 
     public String[] getLayerNames(Transaction tx) {
+        assertNotOldModel(tx);
         List<String> names = new ArrayList<>();
 
-        for (Relationship relationship : getSpatialRoot(tx).getRelationships(Direction.OUTGOING, SpatialRelationshipTypes.LAYER)) {
-            Layer layer = LayerUtilities.makeLayerFromNode(tx, indexManager, relationship.getEndNode());
+        ResourceIterator<Node> layers = tx.findNodes(LABEL_LAYER);
+        while (layers.hasNext()) {
+            Layer layer = LayerUtilities.makeLayerFromNode(tx, indexManager, layers.next());
             if (layer instanceof DynamicLayer) {
                 names.addAll(((DynamicLayer) layer).getLayerNames(tx));
             } else {
@@ -78,8 +105,10 @@ public String[] getLayerNames(Transaction tx) {
     }
 
     public Layer getLayer(Transaction tx, String name) {
-        for (Relationship relationship : getSpatialRoot(tx).getRelationships(Direction.OUTGOING, SpatialRelationshipTypes.LAYER)) {
-            Node node = relationship.getEndNode();
+        assertNotOldModel(tx);
+        ResourceIterator<Node> layers = tx.findNodes(LABEL_LAYER);
+        while (layers.hasNext()) {
+            Node node = layers.next();
             if (name.equals(node.getProperty(PROP_LAYER))) {
                 return LayerUtilities.makeLayerFromNode(tx, indexManager, node);
             }
@@ -88,9 +117,11 @@ public Layer getLayer(Transaction tx, String name) {
     }
 
     public Layer getDynamicLayer(Transaction tx, String name) {
+        assertNotOldModel(tx);
         ArrayList<DynamicLayer> dynamicLayers = new ArrayList<>();
-        for (Relationship relationship : getSpatialRoot(tx).getRelationships(Direction.OUTGOING, SpatialRelationshipTypes.LAYER)) {
-            Node node = relationship.getEndNode();
+        ResourceIterator<Node> layers = tx.findNodes(LABEL_LAYER);
+        while (layers.hasNext()) {
+            Node node = layers.next();
             if (!node.getProperty(PROP_LAYER_CLASS, "").toString().startsWith("DefaultLayer")) {
                 Layer layer = LayerUtilities.makeLayerFromNode(tx, indexManager, node);
                 if (layer instanceof DynamicLayer) {
@@ -125,7 +156,7 @@ public DynamicLayer asDynamicLayer(Transaction tx, Layer layer) {
     }
 
     public DefaultLayer getOrCreateDefaultLayer(Transaction tx, String name) {
-        return (DefaultLayer) getOrCreateLayer(tx, name, WKBGeometryEncoder.class, DefaultLayer.class, "");
+        return (DefaultLayer) getOrCreateLayer(tx, name, WKBGeometryEncoder.class, EditableLayerImpl.class, "");
     }
 
     public EditableLayer getOrCreateEditableLayer(Transaction tx, String name, String format, String propertyNameConfig) {
@@ -241,7 +272,7 @@ public Layer createWKBLayer(Transaction tx, String name) {
     }
 
     public SimplePointLayer createSimplePointLayer(Transaction tx, String name) {
-        return createSimplePointLayer(tx, name, null);
+        return createSimplePointLayer(tx, name, (String[]) null);
     }
 
     public SimplePointLayer createSimplePointLayer(Transaction tx, String name, String xProperty, String yProperty) {
@@ -253,7 +284,7 @@ public SimplePointLayer createSimplePointLayer(Transaction tx, String name, Stri
     }
 
     public SimplePointLayer createNativePointLayer(Transaction tx, String name) {
-        return createNativePointLayer(tx, name, null);
+        return createNativePointLayer(tx, name, (String[]) null);
     }
 
     public SimplePointLayer createNativePointLayer(Transaction tx, String name, String locationProperty, String bboxProperty) {
@@ -300,15 +331,13 @@ public Layer createLayer(Transaction tx, String name, Class<? extends GeometryEn
             throw new SpatialDatabaseException("Layer " + name + " already exists");
 
         Layer layer = LayerUtilities.makeLayerAndNode(tx, indexManager, name, geometryEncoderClass, layerClass, indexClass);
-        getSpatialRoot(tx).createRelationshipTo(layer.getLayerNode(tx), SpatialRelationshipTypes.LAYER);
         if (encoderConfig != null && encoderConfig.length() > 0) {
             GeometryEncoder encoder = layer.getGeometryEncoder();
             if (encoder instanceof Configurable) {
                 ((Configurable) encoder).setConfiguration(encoderConfig);
                 layer.getLayerNode(tx).setProperty(PROP_GEOMENCODER_CONFIG, encoderConfig);
             } else {
-                System.out.println("Warning: encoder configuration '" + encoderConfig
-                        + "' passed to non-configurable encoder: " + geometryEncoderClass);
+                System.out.println("Warning: encoder configuration '" + encoderConfig + "' passed to non-configurable encoder: " + geometryEncoderClass);
             }
         }
         if (crs != null && layer instanceof EditableLayer) {
@@ -323,12 +352,10 @@ public void deleteLayer(Transaction tx, String name, Listener monitor) {
         layer.delete(tx, monitor);
     }
 
-    @SuppressWarnings("unchecked")
     public static int convertGeometryNameToType(String geometryName) {
         if (geometryName == null) return GTYPE_GEOMETRY;
         try {
-            return convertJtsClassToGeometryType((Class<? extends Geometry>) Class.forName("org.locationtech.jts.geom."
-                    + geometryName));
+            return convertJtsClassToGeometryType((Class<? extends Geometry>) Class.forName("org.locationtech.jts.geom." + geometryName));
         } catch (ClassNotFoundException e) {
             System.err.println("Unrecognized geometry '" + geometryName + "': " + e);
             return GTYPE_GEOMETRY;

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

@@ -47,7 +47,28 @@ public IndexManager(GraphDatabaseAPI db, SecurityContext securityContext) {
         this.securityContext = IndexAccessMode.withIndexCreate(securityContext);
     }
 
+    /**
+     * Blocking call that spawns a thread to create an index and then waits for that thread to finish.
+     * This is highly likely to cause deadlocks on index checks, so be careful where it is used.
+     * Best used if you can commit any other outer transaction first, then run this, and after that
+     * start a new transaction. For example, see the OSMImport approaching to batching transactions.
+     * It is possible to use this in procedures with outer transactions if you can ensure the outer
+     * transactions are read-only.
+     */
     public IndexDefinition indexFor(Transaction tx, String indexName, Label label, String propertyKey) {
+        return indexFor(tx, indexName, label, propertyKey, true);
+    }
+
+    /**
+     * Non-blocking call that spawns a thread to create an index and then waits for that thread to finish.
+     * Use this especially on indexes that are not immediately needed. Also use it if you have an outer
+     * transaction that cannot be committed before making this call.
+     */
+    public void makeIndexFor(Transaction tx, String indexName, Label label, String propertyKey) {
+        indexFor(tx, indexName, label, propertyKey, false);
+    }
+
+    private IndexDefinition indexFor(Transaction tx, String indexName, Label label, String propertyKey, boolean waitFor) {
         for (IndexDefinition exists : tx.schema().getIndexes(label)) {
             if (exists.getName().equals(indexName)) {
                 return exists;
@@ -60,15 +81,19 @@ public IndexDefinition indexFor(Transaction tx, String indexName, Label label, S
         } else {
             IndexMaker indexMaker = new IndexMaker(indexName, label, propertyKey);
             Thread indexMakerThread = new Thread(indexMaker, name);
-            indexMakerThread.start();
-            try {
-                indexMakerThread.join();
-                if (indexMaker.e != null) {
-                    throw new RuntimeException("Failed to make index " + indexMaker.description(), indexMaker.e);
+            if (waitFor) {
+                indexMakerThread.start();
+                try {
+                    indexMakerThread.join();
+                    if (indexMaker.e != null) {
+                        throw new RuntimeException("Failed to make index " + indexMaker.description(), indexMaker.e);
+                    }
+                    return indexMaker.index;
+                } catch (InterruptedException e) {
+                    throw new RuntimeException("Failed to make index " + indexMaker.description(), e);
                 }
-                return indexMaker.index;
-            } catch (InterruptedException e) {
-                throw new RuntimeException("Failed to make index " + indexMaker.description(), e);
+            } else {
+                return null;
             }
         }
     }