From ec974feb472b5e8fdfbde49ebc1de1962e8cbb87 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Wed, 17 Mar 2021 23:42:06 +0100 Subject: [PATCH 1/3] Remove use of old reference nodes, reducing deadlocks Also add support for exception on trying to import the same OSM file twice, as this is not yet supported and will cause errors or model corruptions. This change is not backwards compatible, as the spatial model is different. Layers created in earlier verions would not be readible in this one. To protect against that we throw exceptions on all SpatialDatabaseService. layer finding methods, if they detect the old format. The exception requests that the database be upgraded. This can be done in the Java API with the upgradeFromOldModel method, or via procedures using `spatial.upgrade`. --- .../java/org/neo4j/gis/spatial/Constants.java | 83 ++++++++++--------- .../org/neo4j/gis/spatial/DefaultLayer.java | 1 - .../gis/spatial/SpatialDatabaseService.java | 61 ++++++++++---- .../neo4j/gis/spatial/index/IndexManager.java | 41 +++++++-- .../neo4j/gis/spatial/osm/OSMImporter.java | 18 ++-- .../spatial/procedures/SpatialProcedures.java | 21 +++++ .../gis/spatial/utilities/LayerUtilities.java | 3 +- .../gis/spatial/utilities/ReferenceNodes.java | 40 +++++++-- .../neo4j/gis/spatial/OsmAnalysisTest.java | 4 +- .../procedures/SpatialProceduresTest.java | 66 ++++++++++++++- 10 files changed, 250 insertions(+), 88 deletions(-) diff --git a/src/main/java/org/neo4j/gis/spatial/Constants.java b/src/main/java/org/neo4j/gis/spatial/Constants.java index bb4a1bed6..19113b412 100644 --- a/src/main/java/org/neo4j/gis/spatial/Constants.java +++ b/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; + } \ No newline at end of file diff --git a/src/main/java/org/neo4j/gis/spatial/DefaultLayer.java b/src/main/java/org/neo4j/gis/spatial/DefaultLayer.java index 4606e6795..e95c5fb5a 100644 --- a/src/main/java/org/neo4j/gis/spatial/DefaultLayer.java +++ b/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; } diff --git a/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java b/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java index f891a766d..d7fa12f77 100644 --- a/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java +++ b/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java @@ -49,24 +49,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"); } - return tx.getNodeById(spatialRoot); + } + + public List upgradeFromOldModel(Transaction tx) { + ArrayList layersConverted = new ArrayList<>(); + Node oldReferenceNode = ReferenceNodes.findDeprecatedReferenceNode(tx, "spatial_root"); + if (oldReferenceNode != null) { + List 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(); + } + indexManager.makeIndexFor(tx, "SpatialLayers", LABEL_LAYER, PROP_LAYER); + return layersConverted; } public String[] getLayerNames(Transaction tx) { + assertNotOldModel(tx); List names = new ArrayList<>(); - for (Relationship relationship : getSpatialRoot(tx).getRelationships(Direction.OUTGOING, SpatialRelationshipTypes.LAYER)) { - Layer layer = LayerUtilities.makeLayerFromNode(tx, indexManager, relationship.getEndNode()); + ResourceIterator 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 +106,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 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 +118,11 @@ public Layer getLayer(Transaction tx, String name) { } public Layer getDynamicLayer(Transaction tx, String name) { + assertNotOldModel(tx); ArrayList dynamicLayers = new ArrayList<>(); - for (Relationship relationship : getSpatialRoot(tx).getRelationships(Direction.OUTGOING, SpatialRelationshipTypes.LAYER)) { - Node node = relationship.getEndNode(); + ResourceIterator 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) { @@ -300,15 +332,13 @@ public Layer createLayer(Transaction tx, String name, Class 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) { @@ -327,8 +357,7 @@ public void deleteLayer(Transaction tx, String name, Listener monitor) { public static int convertGeometryNameToType(String geometryName) { if (geometryName == null) return GTYPE_GEOMETRY; try { - return convertJtsClassToGeometryType((Class) Class.forName("org.locationtech.jts.geom." - + geometryName)); + return convertJtsClassToGeometryType((Class) Class.forName("org.locationtech.jts.geom." + geometryName)); } catch (ClassNotFoundException e) { System.err.println("Unrecognized geometry '" + geometryName + "': " + e); return GTYPE_GEOMETRY; diff --git a/src/main/java/org/neo4j/gis/spatial/index/IndexManager.java b/src/main/java/org/neo4j/gis/spatial/index/IndexManager.java index b359255cc..0e12f4e37 100644 --- a/src/main/java/org/neo4j/gis/spatial/index/IndexManager.java +++ b/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; } } } diff --git a/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java b/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java index 47a13bb0d..92c07120a 100644 --- a/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java +++ b/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java @@ -930,12 +930,10 @@ private void recoverNode(WrappedNode outOfTx) { } } - private WrappedNode findNode(String name, WrappedNode parent, OSMRelation relType) { - for (Relationship relationship : parent.getRelationships(Direction.OUTGOING, relType)) { - Node node = relationship.getEndNode(); - if (name.equals(node.getProperty("name"))) { - return WrappedNode.fromNode(node); - } + private WrappedNode findNode(Label label, String name) { + Node node = tx.findNode(label, "name", name); + if (node != null) { + return WrappedNode.fromNode(node); } return null; } @@ -1003,13 +1001,12 @@ private IndexDefinition findIndex(Transaction tx, Label label, String propertyKe return null; } - private WrappedNode getOrCreateNode(Label label, String name, String type, WrappedNode parent, OSMRelation relType) { - WrappedNode node = findNode(name, parent, relType); + private WrappedNode getOrCreateNode(Label label, String name, String type) { + WrappedNode node = findNode(label, name); if (node == null) { Node n = tx.createNode(label); n.setProperty("name", name); n.setProperty("type", type); - parent.inner.createRelationshipTo(n, relType); node = checkTx(WrappedNode.fromNode(n)); } return node; @@ -1018,8 +1015,7 @@ private WrappedNode getOrCreateNode(Label label, String name, String type, Wrapp @Override protected WrappedNode getOrCreateOSMDataset(String name) { if (osm_dataset == null) { - Node osm_root = ReferenceNodes.getReferenceNode(tx, "osm_root"); - osm_dataset = getOrCreateNode(LABEL_DATASET, name, "osm", WrappedNode.fromNode(osm_root), OSMRelation.OSM); + osm_dataset = getOrCreateNode(LABEL_DATASET, name, "osm"); } return osm_dataset; } diff --git a/src/main/java/org/neo4j/gis/spatial/procedures/SpatialProcedures.java b/src/main/java/org/neo4j/gis/spatial/procedures/SpatialProcedures.java index aac41d0a3..ee6c433f7 100644 --- a/src/main/java/org/neo4j/gis/spatial/procedures/SpatialProcedures.java +++ b/src/main/java/org/neo4j/gis/spatial/procedures/SpatialProcedures.java @@ -198,6 +198,20 @@ public Stream listProcedures() throws ProcedureException { return builder.build(); } + @Procedure(name = "spatial.upgrade", mode = WRITE) + @Description("Upgrades an older spatial data model and returns a list of layers upgraded") + public Stream upgradeSpatial() { + SpatialDatabaseService sdb = spatial(); + Stream.Builder builder = Stream.builder(); + for (String name : sdb.upgradeFromOldModel(tx)) { + Layer layer = sdb.getLayer(tx, name); + if (layer != null) { + builder.accept(new NameResult(name, layer.getSignature())); + } + } + return builder.build(); + } + @Procedure(value="spatial.layers", mode = WRITE) @Description("Returns name, and details for all layers") public Stream getAllLayers() { @@ -682,6 +696,7 @@ public Stream importOSM( public Stream importOSM( @Name("uri") String uri) throws InterruptedException { String layerName = uri.substring(uri.lastIndexOf(File.separator) + 1); + assertLayerDoesNotExists(tx, spatial(), layerName); // Delegate creating the layer to the inner thread, so we do not pollute the procedure transaction with anything that might conflict. // Since the procedure transaction starts before, and ends after, all inner transactions. BiFunction layerMaker = (tx, name) -> (OSMLayer) spatial().getOrCreateLayer(tx, name, OSMGeometryEncoder.class, OSMLayer.class); @@ -1041,4 +1056,10 @@ private static Layer getLayerOrThrow(Transaction tx, SpatialDatabaseService spat throw new IllegalArgumentException("No such layer '" + name + "'"); } } + + private static void assertLayerDoesNotExists(Transaction tx, SpatialDatabaseService spatial, String name) { + if (spatial.getLayer(tx, name) != null) { + throw new IllegalArgumentException("Layer already exists: '" + name + "'"); + } + } } diff --git a/src/main/java/org/neo4j/gis/spatial/utilities/LayerUtilities.java b/src/main/java/org/neo4j/gis/spatial/utilities/LayerUtilities.java index fa2f365ea..e93d96886 100644 --- a/src/main/java/org/neo4j/gis/spatial/utilities/LayerUtilities.java +++ b/src/main/java/org/neo4j/gis/spatial/utilities/LayerUtilities.java @@ -27,7 +27,7 @@ import org.neo4j.graphdb.Transaction; /** - * Utilities for creating layersfrom nodes. + * Utilities for creating layers from nodes. */ public class LayerUtilities implements Constants { @@ -72,6 +72,7 @@ public static Layer makeLayerAndNode(Transaction tx, IndexManager indexManager, indexClass = LayerRTreeIndex.class; } Node layerNode = tx.createNode(); + layerNode.addLabel(LABEL_LAYER); layerNode.setProperty(PROP_LAYER, name); layerNode.setProperty(PROP_CREATIONTIME, System.currentTimeMillis()); layerNode.setProperty(PROP_GEOMENCODER, geometryEncoderClass.getCanonicalName()); diff --git a/src/main/java/org/neo4j/gis/spatial/utilities/ReferenceNodes.java b/src/main/java/org/neo4j/gis/spatial/utilities/ReferenceNodes.java index e537ac5c9..004cf04ca 100644 --- a/src/main/java/org/neo4j/gis/spatial/utilities/ReferenceNodes.java +++ b/src/main/java/org/neo4j/gis/spatial/utilities/ReferenceNodes.java @@ -20,17 +20,45 @@ package org.neo4j.gis.spatial.utilities; +import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; -import org.neo4j.graphdb.Result; import org.neo4j.graphdb.Transaction; -import org.neo4j.internal.helpers.collection.Iterators; - -import static org.neo4j.internal.helpers.collection.MapUtil.map; +/** + * In Neo4j 0.x and even 1.x it was common to not use an index for the starting point of a search. + * Instead Neo4j had a single reference node id=0 which always existed + * and all models could be connected at some point to that node. + * Finding things was always by traversal. + * This model was used in Neo4j Spatial, and transitioned to using a special spatial reference node. + * However, this kind of thinking became more and more of a problem as we moved to procedures in Neo4j 3.0 + * and in particular the nested transaction model of Neo4j 4.0. Since this node was created on-demand + * even in read-only queries like 'findLayer', it messed with nested transactions where both might + * try create the same node, even if the developer was careful to split read and write aspects of the + * code. + * + * It is time to stop using a root node. This class will remain only for the purpose of helping + * users transition older spatial models away from root nodes. + */ public class ReferenceNodes { + public static final Label LABEL_REFERENCE = Label.label("ReferenceNode"); + public static final String PROP_NAME = "name"; + + @Deprecated public static Node getReferenceNode(Transaction tx, String name) { - Result result = tx.execute("MERGE (ref:ReferenceNode {name:$name}) RETURN ref", map("name", name)); - return Iterators.single(result.columnAs("ref")); + throw new IllegalStateException("It is no longer valid to use a root or reference node in the spatial model"); + } + + public static Node findDeprecatedReferenceNode(Transaction tx, String name) { + return tx.findNode(LABEL_REFERENCE, PROP_NAME, name); + } + + /** + * Should be used for tests only. No attempt is made to ensure no duplicates are created. + */ + public static Node createDeprecatedReferenceNode(Transaction tx, String name) { + Node node = tx.createNode(LABEL_REFERENCE); + node.setProperty(PROP_NAME, name); + return node; } } diff --git a/src/test/java/org/neo4j/gis/spatial/OsmAnalysisTest.java b/src/test/java/org/neo4j/gis/spatial/OsmAnalysisTest.java index f40fb1e91..cbee1a9c8 100644 --- a/src/test/java/org/neo4j/gis/spatial/OsmAnalysisTest.java +++ b/src/test/java/org/neo4j/gis/spatial/OsmAnalysisTest.java @@ -29,6 +29,7 @@ import org.neo4j.gis.spatial.filter.SearchRecords; import org.neo4j.gis.spatial.index.IndexManager; import org.neo4j.gis.spatial.osm.OSMDataset; +import org.neo4j.gis.spatial.osm.OSMImporter; import org.neo4j.gis.spatial.osm.OSMLayer; import org.neo4j.gis.spatial.osm.OSMRelation; import org.neo4j.gis.spatial.rtree.Envelope; @@ -272,8 +273,7 @@ public void testAnalysis(String osm) throws Exception { SortedMap layers; ReferencedEnvelope bbox; try (Transaction tx = graphDb().beginTx()) { - Node osmRoot = ReferenceNodes.getReferenceNode(tx, "osm_root"); - Node osmImport = osmRoot.getSingleRelationship(OSMRelation.OSM, Direction.OUTGOING).getEndNode(); + Node osmImport = tx.findNode(OSMImporter.LABEL_DATASET, "name", osm); Node usersNode = osmImport.getSingleRelationship(OSMRelation.USERS, Direction.OUTGOING).getEndNode(); Map userIndex = collectUserChangesetData(usersNode); diff --git a/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java b/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java index aa220c6fc..62251fbea 100644 --- a/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java +++ b/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java @@ -23,15 +23,28 @@ import org.neo4j.configuration.GraphDatabaseSettings; import org.neo4j.dbms.api.DatabaseManagementService; import org.neo4j.exceptions.KernelException; +import org.neo4j.gis.spatial.Layer; +import org.neo4j.gis.spatial.SimplePointLayer; +import org.neo4j.gis.spatial.SpatialDatabaseService; +import org.neo4j.gis.spatial.SpatialRelationshipTypes; +import org.neo4j.gis.spatial.encoders.SimplePointEncoder; +import org.neo4j.gis.spatial.index.IndexManager; +import org.neo4j.gis.spatial.utilities.ReferenceNodes; import org.neo4j.graphdb.*; import org.neo4j.graphdb.spatial.Geometry; import org.neo4j.graphdb.spatial.Point; import org.neo4j.internal.helpers.collection.Iterators; +import org.neo4j.io.fs.FileUtils; +import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.procedure.GlobalProcedures; +import org.neo4j.kernel.impl.coreapi.InternalTransaction; import org.neo4j.kernel.internal.GraphDatabaseAPI; import org.neo4j.test.TestDatabaseManagementServiceBuilder; import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -47,8 +60,10 @@ public class SpatialProceduresTest { private GraphDatabaseService db; @Before - public void setUp() throws KernelException { - databases = new TestDatabaseManagementServiceBuilder(new File("target/procedures").toPath()).setConfig(GraphDatabaseSettings.procedure_unrestricted, List.of("spatial.*")).impermanent().build(); + public void setUp() throws KernelException, IOException { + Path dbRoot = new File("target/procedures").toPath(); + FileUtils.deleteDirectory(dbRoot); + databases = new TestDatabaseManagementServiceBuilder(dbRoot).setConfig(GraphDatabaseSettings.procedure_unrestricted, List.of("spatial.*")).impermanent().build(); db = databases.database(DEFAULT_DATABASE_NAME); registerProceduresAndFunctions(db, SpatialProcedures.class); } @@ -128,6 +143,43 @@ public static void registerProceduresAndFunctions(GraphDatabaseService db, Class procedures.registerFunction(procedure); } + private void makeOldSpatialModel(Transaction tx, String... layers) { + KernelTransaction ktx = ((InternalTransaction) tx).kernelTransaction(); + SpatialDatabaseService spatial = new SpatialDatabaseService(new IndexManager((GraphDatabaseAPI) db, ktx.securityContext())); + ArrayList layerNodes = new ArrayList<>(); + // First create a set of layers + for (String name : layers) { + Layer layer = spatial.createLayer(tx, name, SimplePointEncoder.class, SimplePointLayer.class); + layerNodes.add(layer.getLayerNode(tx)); + } + // Then downgrade to old format, without label and with reference node and relationships + Node root = ReferenceNodes.createDeprecatedReferenceNode(tx, "spatial_root"); + for (Node node : layerNodes) { + node.removeLabel(LABEL_LAYER); + root.createRelationshipTo(node, SpatialRelationshipTypes.LAYER); + } + } + + @Test + public void old_spatial_model_throws_errors() { + try (Transaction tx = db.beginTx()) { + makeOldSpatialModel(tx, "layer1", "layer2", "layer3"); + tx.commit(); + } + testCallFails(db, "CALL spatial.layers", null, "Old reference node exists - please upgrade the spatial database to the new format"); + } + + @Test + public void old_spatial_model_can_be_upgraded() { + try (Transaction tx = db.beginTx()) { + makeOldSpatialModel(tx, "layer1", "layer2", "layer3"); + tx.commit(); + } + testCallFails(db, "CALL spatial.layers", null, "Old reference node exists - please upgrade the spatial database to the new format"); + testCallCount(db, "CALL spatial.upgrade", null, 3); + testCallCount(db, "CALL spatial.layers", null, 3); + } + @Test public void add_node_to_non_existing_layer() { execute("CALL spatial.addPointLayer('some_name')"); @@ -847,6 +899,14 @@ public void import_osm() { testCallCount(db, "CALL spatial.layers()", null, 1); } + @Test + public void import_osm_twice_should_fail() { + testCountQuery("importOSM", "CALL spatial.importOSM('map.osm')", 55, "count", null); + testCallCount(db, "CALL spatial.layers()", null, 1); + testCallFails(db, "CALL spatial.importOSM('map.osm')", null, "Layer already exists: 'map.osm'"); + testCallCount(db, "CALL spatial.layers()", null, 1); + } + @Test public void import_osm_without_extension() { testCountQuery("importOSM", "CALL spatial.importOSM('map')", 55, "count", null); @@ -856,7 +916,7 @@ public void import_osm_without_extension() { @Test public void import_osm_to_layer() { execute("CALL spatial.addLayer('geom','OSM','')"); - testCountQuery("importShapefileToLayer", "CALL spatial.importOSMToLayer('geom','map.osm')", 55, "count", null); + testCountQuery("importOSMToLayer", "CALL spatial.importOSMToLayer('geom','map.osm')", 55, "count", null); testCallCount(db, "CALL spatial.layers()", null, 1); } From 5b96b36519c931b76141268d13c1db8748d20b54 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Thu, 18 Mar 2021 00:01:03 +0100 Subject: [PATCH 2/3] Removed some warnings and improved a test --- .../gis/spatial/SpatialDatabaseService.java | 10 ++++---- .../procedures/SpatialProceduresTest.java | 23 +++++++++++++------ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java b/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java index d7fa12f77..c9531e565 100644 --- a/src/main/java/org/neo4j/gis/spatial/SpatialDatabaseService.java +++ b/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; @@ -157,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) { @@ -273,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) { @@ -285,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) { @@ -353,7 +352,6 @@ 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 { diff --git a/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java b/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java index 62251fbea..025e13396 100644 --- a/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java +++ b/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java @@ -24,10 +24,8 @@ import org.neo4j.dbms.api.DatabaseManagementService; import org.neo4j.exceptions.KernelException; import org.neo4j.gis.spatial.Layer; -import org.neo4j.gis.spatial.SimplePointLayer; import org.neo4j.gis.spatial.SpatialDatabaseService; import org.neo4j.gis.spatial.SpatialRelationshipTypes; -import org.neo4j.gis.spatial.encoders.SimplePointEncoder; import org.neo4j.gis.spatial.index.IndexManager; import org.neo4j.gis.spatial.utilities.ReferenceNodes; import org.neo4j.graphdb.*; @@ -143,14 +141,27 @@ public static void registerProceduresAndFunctions(GraphDatabaseService db, Class procedures.registerFunction(procedure); } + private Layer makeLayerOfVariousTypes(SpatialDatabaseService spatial, Transaction tx, String name, int index) { + switch (index % 3) { + case 0: + return spatial.getOrCreateSimplePointLayer(tx, name, SpatialDatabaseService.RTREE_INDEX_NAME, "x", "y"); + case 1: + return spatial.getOrCreateNativePointLayer(tx, name, SpatialDatabaseService.RTREE_INDEX_NAME, "location"); + default: + return spatial.getOrCreateDefaultLayer(tx, name); + } + } + private void makeOldSpatialModel(Transaction tx, String... layers) { KernelTransaction ktx = ((InternalTransaction) tx).kernelTransaction(); SpatialDatabaseService spatial = new SpatialDatabaseService(new IndexManager((GraphDatabaseAPI) db, ktx.securityContext())); ArrayList layerNodes = new ArrayList<>(); + int index = 0; // First create a set of layers for (String name : layers) { - Layer layer = spatial.createLayer(tx, name, SimplePointEncoder.class, SimplePointLayer.class); + Layer layer = makeLayerOfVariousTypes(spatial, tx, name, index); layerNodes.add(layer.getLayerNode(tx)); + index++; } // Then downgrade to old format, without label and with reference node and relationships Node root = ReferenceNodes.createDeprecatedReferenceNode(tx, "spatial_root"); @@ -783,9 +794,7 @@ public void add_two_nodes_to_the_spatial_layer() { removeResult.close(); tx.commit(); } - testResult(db, "CALL spatial.withinDistance('geom',{lon:15.0,lat:60.0},100)", res -> { - assertFalse(res.hasNext()); - }); + testResult(db, "CALL spatial.withinDistance('geom',{lon:15.0,lat:60.0},100)", res -> assertFalse(res.hasNext())); } @Test @@ -967,7 +976,7 @@ public void import_osm_and_polygons_withinDistance() { Node node = (Node) r.get("node"); double distance = (Double) r.get("distance"); Node osmNode = (Node) r.get("osmNode"); - Map props = (Map) r.get("props"); + @SuppressWarnings("rawtypes") Map props = (Map) r.get("props"); System.out.println("(node[" + node.getId() + "])<-[:GEOM {distance:" + distance + "}]-(osmNode[" + osmNode.getId() + "] " + props + ") "); assertThat("Node should have either way_osm_id or node_osm_id", props, anyOf(hasKey("node_osm_id"), hasKey("way_osm_id"))); } From 785554da820891670e8710fd81d8b15fea4c1e16 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Sun, 21 Mar 2021 15:25:18 +0100 Subject: [PATCH 3/3] Use unique labels for indexing within multiple OSM layers Essentially when adding nodes of a particular label, we also add a label with a unique name for indexing purposes. The unique name is made of the original name plus a hex suffix made of the MD5 hash of the layer name. For example, the layer `geom1` will have an MD5 hash of its name `9ECE5459EA0D46FC556E5E3F454A0795`. Then when adding an OSM node we label the node with both: * OSMNode * OSMNode_9ECE5459EA0D46FC556E5E3F454A0795 The second label is also used to create a label-property index to be used for looking up the node when building ways. --- README.md | 10 +- pom.xml | 2 +- .../neo4j/gis/spatial/osm/OSMImporter.java | 91 +++++++++++++------ .../spatial/Neo4jSpatialDataStoreTest.java | 2 +- .../procedures/SpatialProceduresTest.java | 16 ++++ 5 files changed, 92 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 22c1f5a95..c270d966b 100755 --- a/README.md +++ b/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: org.neo4j neo4j-spatial - 0.27.2-neo4j-4.2.3 + 0.28.0-neo4j-4.2.3 ~~~ diff --git a/pom.xml b/pom.xml index db64dbce9..e80dea01a 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ 4.0.0 neo4j-spatial org.neo4j - 0.27.2-neo4j-4.2.3 + 0.28.0-neo4j-4.2.3 Neo4j - Spatial Components Spatial utilities and components for Neo4j http://components.neo4j.org/${project.artifactId}/${project.version} diff --git a/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java b/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java index 92c07120a..e5f7a029c 100644 --- a/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java +++ b/src/main/java/org/neo4j/gis/spatial/osm/OSMImporter.java @@ -28,7 +28,6 @@ import org.neo4j.gis.spatial.rtree.Envelope; import org.neo4j.gis.spatial.rtree.Listener; import org.neo4j.gis.spatial.rtree.NullListener; -import org.neo4j.gis.spatial.utilities.ReferenceNodes; import org.neo4j.graphdb.*; import org.neo4j.graphdb.schema.IndexDefinition; import org.neo4j.graphdb.traversal.Evaluators; @@ -41,10 +40,13 @@ import org.neo4j.kernel.impl.traversal.MonoDirectionalTraversalDescription; import org.neo4j.kernel.internal.GraphDatabaseAPI; +import javax.xml.bind.DatatypeConverter; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import java.io.*; import java.nio.charset.Charset; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; @@ -392,7 +394,7 @@ private OSMWriter(StatsManager statsManager, OSMImporter osmImporter) { this.osmImporter = osmImporter; } - static OSMWriter fromGraphDatabase(GraphDatabaseService graphDb, SecurityContext securityContext, StatsManager stats, OSMImporter osmImporter, int txInterval) { + static OSMWriter fromGraphDatabase(GraphDatabaseService graphDb, SecurityContext securityContext, StatsManager stats, OSMImporter osmImporter, int txInterval) throws NoSuchAlgorithmException { return new OSMGraphWriter(graphDb, securityContext, stats, osmImporter, txInterval); } @@ -867,8 +869,10 @@ private static class OSMGraphWriter extends OSMWriter { private IndexDefinition relationIndex; private IndexDefinition changesetIndex; private IndexDefinition userIndex; + private final String layerHash; + private final HashMap hashedLabels = new HashMap<>(); - private OSMGraphWriter(GraphDatabaseService graphDb, SecurityContext securityContext, StatsManager statsManager, OSMImporter osmImporter, int txInterval) { + private OSMGraphWriter(GraphDatabaseService graphDb, SecurityContext securityContext, StatsManager statsManager, OSMImporter osmImporter, int txInterval) throws NoSuchAlgorithmException { super(statsManager, osmImporter); this.graphDb = graphDb; this.securityContext = securityContext; @@ -876,9 +880,18 @@ private OSMGraphWriter(GraphDatabaseService graphDb, SecurityContext securityCon if (this.txInterval < 100) { System.err.println("Warning: Unusually short txInterval, expect bad insert performance"); } + this.layerHash = md5Hash(osmImporter.layerName); checkTx(null); // Opens transaction for future writes } + private static String md5Hash(String text) throws NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("MD5"); + md.update(text.getBytes()); + byte[] digest = md.digest(); + String hashed = DatatypeConverter.printHexBinary(digest).toUpperCase(); + return hashed; + } + private void successTx() { if (tx != null) { tx.commit(); @@ -930,14 +943,19 @@ private void recoverNode(WrappedNode outOfTx) { } } - private WrappedNode findNode(Label label, String name) { - Node node = tx.findNode(label, "name", name); + private WrappedNode findNodeByName(Label label, String name) { + Node node = findNodeByLabelProperty(tx, label, "name", name); if (node != null) { return WrappedNode.fromNode(node); } return null; } + private WrappedNode createNodeWithLabel(Transaction tx, Label label) { + Label hashed = getLabelHashed(label); + return WrappedNode.fromNode(tx.createNode(label, hashed)); + } + @Override protected void startWays() { System.out.println("About to create node index"); @@ -968,12 +986,29 @@ protected void optimize() { } } + private Label getLabelHashed(Label label) { + if (hashedLabels.containsKey(label)) { + return hashedLabels.get(label); + } else { + Label hashed = Label.label(label.name() + "_" + layerHash); + hashedLabels.put(label, hashed); + return hashed; + } + } + + private Node findNodeByLabelProperty(Transaction tx, Label label, String propertyKey, Object value) { + Label hashed = getLabelHashed(label); + return tx.findNode(hashed, propertyKey, value); + } + private IndexDefinition createIndex(Label label, String propertyKey) { - IndexDefinition index = findIndex(tx, label, propertyKey); + Label hashed = getLabelHashed(label); + String indexName = String.format("OSM-%s-%s-%s", osmImporter.layerName, hashed.name(), propertyKey); + IndexDefinition index = findIndex(tx, indexName, hashed, propertyKey); if (index == null) { successTx(); try (Transaction indexTx = beginIndexTx(graphDb)) { - index = indexTx.schema().indexFor(label).on(propertyKey).create(); + index = indexTx.schema().indexFor(hashed).on(propertyKey).withName(indexName).create(); indexTx.commit(); } System.out.println("Created index " + index.getName()); @@ -990,11 +1025,15 @@ private IndexDefinition createIndexIfNotNull(IndexDefinition index, Label label, return index; } - private IndexDefinition findIndex(Transaction tx, Label label, String propertyKey) { + private IndexDefinition findIndex(Transaction tx, String indexName, Label label, String propertyKey) { for (IndexDefinition index : tx.schema().getIndexes(label)) { for (String prop : index.getPropertyKeys()) { if (prop.equals(propertyKey)) { - return index; + if (index.getName().equals(indexName)) { + return index; + } else { + throw new IllegalStateException(String.format("Found pre-existing index '%s' for index '%s'", index.getName(), indexName)); + } } } } @@ -1002,12 +1041,12 @@ private IndexDefinition findIndex(Transaction tx, Label label, String propertyKe } private WrappedNode getOrCreateNode(Label label, String name, String type) { - WrappedNode node = findNode(label, name); + WrappedNode node = findNodeByName(label, name); if (node == null) { - Node n = tx.createNode(label); + WrappedNode n = createNodeWithLabel(tx, label); n.setProperty("name", name); n.setProperty("type", type); - node = checkTx(WrappedNode.fromNode(n)); + node = checkTx(n); } return node; } @@ -1038,9 +1077,9 @@ protected void addNodeTags(WrappedNode node, LinkedHashMap tags, logNodeAddition(tags, type); if (node != null && tags.size() > 0) { statsManager.addToTagStats(type, tags.keySet()); - Node tagsNode = tx.createNode(LABEL_TAGS); - addProperties(tagsNode, tags); - node.createRelationshipTo(new WrappedNode(tagsNode), OSMRelation.TAGS); + WrappedNode tagsNode = createNodeWithLabel(tx, LABEL_TAGS); + addProperties(tagsNode.inner, tags); + node.createRelationshipTo(tagsNode, OSMRelation.TAGS); tags.clear(); } } @@ -1060,12 +1099,12 @@ protected void addNodeGeometry(WrappedNode node, int gtype, Envelope bbox, int v @Override protected WrappedNode addNode(Label label, Map properties, String indexKey) { - Node node = tx.createNode(label); + WrappedNode node = createNodeWithLabel(tx, label); if (indexKey != null && properties.containsKey(indexKey)) { properties.put(indexKey, Long.parseLong(properties.get(indexKey).toString())); } - addProperties(node, properties); - return checkTx(WrappedNode.fromNode(node)); + addProperties(node.inner, properties); + return checkTx(node); } @Override @@ -1085,7 +1124,7 @@ protected long getDatasetId() { @Override protected WrappedNode getSingleNode(Label label, String property, Object value) { - Node node = tx.findNode(LABEL_NODE, property, value); + Node node = findNodeByLabelProperty(tx, LABEL_NODE, property, value); return node == null ? null : WrappedNode.fromNode(node); } @@ -1116,7 +1155,7 @@ protected WrappedNode getOSMNode(long osmId, WrappedNode changesetNode) { WrappedNode node = changesetNodes.get(osmId); if (node == null) { logNodeFoundFrom("node-index"); - node = WrappedNode.fromNode(tx.findNode(LABEL_NODE, PROP_NODE_ID, osmId)); + node = WrappedNode.fromNode(findNodeByLabelProperty(tx, LABEL_NODE, PROP_NODE_ID, osmId)); } else { logNodeFoundFrom(PROP_CHANGESET); } @@ -1157,7 +1196,7 @@ protected WrappedNode getChangesetNode(Map nodeProps, WrappedNod if (changeset != currentChangesetId) { changesetIndex = createIndexIfNotNull(changesetIndex, LABEL_CHANGESET, PROP_CHANGESET); currentChangesetId = changeset; - Node changesetNode = tx.findNode(LABEL_CHANGESET, PROP_CHANGESET, currentChangesetId); + Node changesetNode = findNodeByLabelProperty(tx, LABEL_CHANGESET, PROP_CHANGESET, currentChangesetId); if (changesetNode != null) { currentChangesetNode = WrappedNode.fromNode(changesetNode); } else { @@ -1187,7 +1226,7 @@ protected WrappedNode getUserNode(Map nodeProps) { if (uid != currentUserId) { currentUserId = uid; userIndex = createIndexIfNotNull(userIndex, LABEL_USER, PROP_USER_ID); - Node userNode = tx.findNode(LABEL_USER, PROP_USER_ID, currentUserId); + Node userNode = findNodeByLabelProperty(tx, LABEL_USER, PROP_USER_ID, currentUserId); if (userNode != null) { currentUserNode = WrappedNode.fromNode(userNode); } else { @@ -1198,7 +1237,7 @@ protected WrappedNode getUserNode(Map nodeProps) { currentUserNode = addNode(LABEL_USER, userProps, PROP_USER_ID); userCount++; if (usersNode == null) { - usersNode = WrappedNode.fromNode(tx.createNode(LABEL_USER)); + usersNode = createNodeWithLabel(tx, LABEL_USER); osm_dataset.createRelationshipTo(usersNode, OSMRelation.USERS); } usersNode.createRelationshipTo(currentUserNode, OSMRelation.OSM_USER); @@ -1218,15 +1257,15 @@ public String toString() { } - public void importFile(GraphDatabaseService database, String dataset) throws IOException, XMLStreamException { + public void importFile(GraphDatabaseService database, String dataset) throws Exception { importFile(database, dataset, false, 5000); } - public void importFile(GraphDatabaseService database, String dataset, int txInterval) throws IOException, XMLStreamException { + public void importFile(GraphDatabaseService database, String dataset, int txInterval) throws Exception { importFile(database, dataset, false, txInterval); } - public void importFile(GraphDatabaseService database, String dataset, boolean allPoints, int txInterval) throws IOException, XMLStreamException { + public void importFile(GraphDatabaseService database, String dataset, boolean allPoints, int txInterval) throws Exception { importFile(OSMWriter.fromGraphDatabase(database, securityContext, stats, this, txInterval), dataset, allPoints, charset); } diff --git a/src/test/java/org/neo4j/gis/spatial/Neo4jSpatialDataStoreTest.java b/src/test/java/org/neo4j/gis/spatial/Neo4jSpatialDataStoreTest.java index f9af5f560..6a2f31406 100644 --- a/src/test/java/org/neo4j/gis/spatial/Neo4jSpatialDataStoreTest.java +++ b/src/test/java/org/neo4j/gis/spatial/Neo4jSpatialDataStoreTest.java @@ -35,7 +35,7 @@ public class Neo4jSpatialDataStoreTest { public GraphDatabaseService graph; @Before - public void setup() throws IOException, XMLStreamException { + public void setup() throws Exception { this.databases = new TestDatabaseManagementServiceBuilder(Path.of("target", "test")).impermanent().build(); this.graph = databases.database(DEFAULT_DATABASE_NAME); OSMImporter importer = new OSMImporter("map", new ConsoleListener()); diff --git a/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java b/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java index 025e13396..5aa740111 100644 --- a/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java +++ b/src/test/java/org/neo4j/gis/spatial/procedures/SpatialProceduresTest.java @@ -929,6 +929,22 @@ public void import_osm_to_layer() { testCallCount(db, "CALL spatial.layers()", null, 1); } + @Test + public void import_osm_twice_should_pass_with_different_layers() { + execute("CALL spatial.addLayer('geom1','OSM','')"); + execute("CALL spatial.addLayer('geom2','OSM','')"); + + testCountQuery("importOSM", "CALL spatial.importOSMToLayer('geom1','map.osm')", 55, "count", null); + testCallCount(db, "CALL spatial.layers()", null, 2); + testCallCount(db, "CALL spatial.withinDistance('geom1',{lon:6.3740429666,lat:50.93676351666},10000)", null, 217); + testCallCount(db, "CALL spatial.withinDistance('geom2',{lon:6.3740429666,lat:50.93676351666},10000)", null, 0); + + testCountQuery("importOSM", "CALL spatial.importOSMToLayer('geom2','map.osm')", 55, "count", null); + testCallCount(db, "CALL spatial.layers()", null, 2); + testCallCount(db, "CALL spatial.withinDistance('geom1',{lon:6.3740429666,lat:50.93676351666},10000)", null, 217); + testCallCount(db, "CALL spatial.withinDistance('geom2',{lon:6.3740429666,lat:50.93676351666},10000)", null, 217); + } + @Ignore public void import_cracow_to_layer() { execute("CALL spatial.addLayer('geom','OSM','')");