From 7e9fa65848d125303ca8813f8fc8e274278dc6e0 Mon Sep 17 00:00:00 2001 From: Gemma Lamont Date: Tue, 18 Apr 2023 16:56:28 +0200 Subject: [PATCH] [VYidKhdO] The required constraint on import key for import.json is unique --- .../java/apoc/export/json/JsonImporter.java | 19 ++- .../ImportJsonEnterpriseFeaturesTest.java | 124 ++++++++++++++++++ .../java/apoc/export/json/ImportJsonTest.java | 17 +++ .../java/apoc/util/TestContainerUtil.java | 1 + 4 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 core/src/test/java/apoc/export/json/ImportJsonEnterpriseFeaturesTest.java diff --git a/core/src/main/java/apoc/export/json/JsonImporter.java b/core/src/main/java/apoc/export/json/JsonImporter.java index a77610cacb..427d4e3955 100644 --- a/core/src/main/java/apoc/export/json/JsonImporter.java +++ b/core/src/main/java/apoc/export/json/JsonImporter.java @@ -7,6 +7,7 @@ import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Transaction; +import org.neo4j.graphdb.schema.ConstraintType; import org.neo4j.graphdb.schema.Schema; import org.neo4j.values.storable.DurationValue; import org.neo4j.values.storable.PointValue; @@ -142,11 +143,11 @@ private void manageRelationship(Map param) { "label", getType(param)); List allLabels = Stream.concat(startLabels.stream(), endLabels.stream()).collect(Collectors.toList()); if (lastRelTypes == null) { - checkConstraints(allLabels); + checkUniquenessConstraints(allLabels); lastRelTypes = relType; } if (!relType.equals(lastRelTypes)) { - checkConstraints(allLabels); + checkUniquenessConstraints(allLabels); flush(); lastRelTypes = relType; } @@ -155,17 +156,22 @@ private void manageRelationship(Map param) { private void manageNode(Map param) { List labels = getLabels(param); if (lastLabels == null) { - checkConstraints(labels); + checkUniquenessConstraints(labels); lastLabels = labels; } if (!labels.equals(lastLabels)) { - checkConstraints(labels); + checkUniquenessConstraints(labels); flush(); lastLabels = labels; } } - private void checkConstraints(List labels) { + /** + * The constraint for the import name should be unique to avoid duplicated imports. + * Node keys are a combination of a uniqueness constraint and en existence constraint, so can also be used. + * The constraint should not be composite. + */ + private void checkUniquenessConstraints(List labels) { if (labels.isEmpty()) { return; } @@ -174,7 +180,8 @@ private void checkConstraints(List labels) { final String importIdName = importJsonConfig.getImportIdName(); final String missingConstraint = labels.stream().filter(label -> StreamSupport.stream(schema.getConstraints(Label.label(label)).spliterator(), false) - .noneMatch(constraint -> Iterables.contains(constraint.getPropertyKeys(), importIdName)) + .filter(constraint -> constraint.isConstraintType(ConstraintType.UNIQUENESS) || constraint.isConstraintType(ConstraintType.NODE_KEY)) + .noneMatch(constraint -> Iterables.contains(constraint.getPropertyKeys(), importIdName) && Iterables.size(constraint.getPropertyKeys()) == 1) ).findAny() .orElse(null); if (missingConstraint != null) { diff --git a/core/src/test/java/apoc/export/json/ImportJsonEnterpriseFeaturesTest.java b/core/src/test/java/apoc/export/json/ImportJsonEnterpriseFeaturesTest.java new file mode 100644 index 0000000000..5d57094a4a --- /dev/null +++ b/core/src/test/java/apoc/export/json/ImportJsonEnterpriseFeaturesTest.java @@ -0,0 +1,124 @@ +package apoc.export.json; + +import apoc.util.Neo4jContainerExtension; +import junit.framework.TestCase; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.neo4j.driver.Session; + +import java.util.List; +import java.util.Map; + +import static apoc.export.json.JsonImporter.MISSING_CONSTRAINT_ERROR_MSG; +import static apoc.util.MapUtil.map; +import static apoc.util.TestContainerUtil.createEnterpriseDB; +import static apoc.util.TestContainerUtil.testResult; +import static java.lang.String.format; + +public class ImportJsonEnterpriseFeaturesTest { + + private static Neo4jContainerExtension neo4jContainer; + private static Session session; + + @BeforeClass + public static void beforeAll() { + neo4jContainer = createEnterpriseDB(true); + neo4jContainer.start(); + session = neo4jContainer.getSession(); + + session.writeTransaction(tx -> { + tx.run("CREATE (u:User {name: \"Fred\"})"); + tx.commit(); + return null; + }); + + // First export a file that we can import + String filename = "all.json"; + testResult(session, "CALL apoc.export.json.all($file,null)", + map("file", filename), + (r) -> Assert.assertTrue(r.hasNext()) + ); + + // Remove node so we can import it after + session.writeTransaction(tx -> { + tx.run("MATCH (n) DETACH DELETE n"); + tx.commit(); + return null; + }); + } + + @Before + public void cleanUpDb() { + // Remove all current constraints/indexes + session.writeTransaction(tx -> { + final List constraints = tx.run("SHOW CONSTRAINTS YIELD name").list(i -> i.get("name").asString()); + constraints.forEach(name -> tx.run(String.format("DROP CONSTRAINT %s", name))); + + final List indexes = tx.run("SHOW INDEXES YIELD name").list(i -> i.get("name").asString()); + indexes.forEach(name -> tx.run(String.format("DROP INDEX %s", name))); + tx.commit(); + return null; + }); + + // Make sure we have no other nodes so all uniqueness constraints work + session.writeTransaction(tx -> { + tx.run("MATCH (n) DETACH DELETE n"); + tx.commit(); + return null; + }); + } + + @AfterClass + public static void afterAll() { + session.close(); + neo4jContainer.close(); + } + + @Test + public void shouldFailBecauseOfMissingUniqueConstraintException() { + session.writeTransaction(tx -> { + tx.run("CREATE CONSTRAINT FOR (n:User) REQUIRE n.neo4jImportId IS NOT NULL"); + tx.run("CREATE CONSTRAINT FOR (n:User) REQUIRE (n.neo4jImportId, n.name) IS NODE KEY"); + tx.commit(); + return null; + }); + + String filename = "all.json"; + Exception e = Assert.assertThrows(Exception.class, + () -> testResult(session, "CALL apoc.import.json($file, {})", map("file", filename), (result) -> {}) + ); + + String expectedMsg = format(MISSING_CONSTRAINT_ERROR_MSG, "User", "neo4jImportId"); + TestCase.assertTrue(e.getMessage().contains(expectedMsg)); + } + + @Test + public void shouldWorkWithSingularNodeKeyAsConstraint() { + // Add unique constraint (test if single node key works) + session.writeTransaction(tx -> { + tx.run("CREATE CONSTRAINT FOR (n:User) REQUIRE (n.neo4jImportId) IS NODE KEY"); + tx.commit(); + return null; + }); + + // Test import procedure + String filename = "all.json"; + testResult(session, "CALL apoc.import.json($file, {})", + map("file", filename), + (result) -> { + Map r = result.next(); + Assert.assertEquals("all.json", r.get("file")); + Assert.assertEquals("file", r.get("source")); + Assert.assertEquals("json", r.get("format")); + Assert.assertEquals(1L, r.get("nodes")); + Assert.assertEquals(0L, r.get("relationships")); + Assert.assertEquals(2L, r.get("properties")); + Assert.assertEquals(1L, r.get("rows")); + Assert.assertEquals(true, r.get("done")); + } + ); + } +} diff --git a/core/src/test/java/apoc/export/json/ImportJsonTest.java b/core/src/test/java/apoc/export/json/ImportJsonTest.java index 58d3c152c7..6dde9b1623 100644 --- a/core/src/test/java/apoc/export/json/ImportJsonTest.java +++ b/core/src/test/java/apoc/export/json/ImportJsonTest.java @@ -296,6 +296,23 @@ public void shouldImportAllNodesAndRels() { assertEntities(NODES_BIG_JSON, RELS_BIG_JSON); } + + @Test + public void shouldFailBecauseOfMissingUniquenessConstraintException() { + db.executeTransactionally("CREATE CONSTRAINT FOR (n:User) REQUIRE (n.neo4jImportId, n.name) IS UNIQUE;"); + assertEntities(0L, 0L); + + String filename = "all.json"; + try { + TestUtil.testCall(db, "CALL apoc.import.json($file, {})", + map("file", filename), + (r) -> fail("Should fail due to missing constraint") + ); + } catch (Exception e) { + String expectedMsg = format(MISSING_CONSTRAINT_ERROR_MSG, "User", "neo4jImportId"); + assertRootMessage(expectedMsg, e); + } + } @Test public void shouldFailBecauseOfMissingSecondConstraintException() { diff --git a/test-utils/src/main/java/apoc/util/TestContainerUtil.java b/test-utils/src/main/java/apoc/util/TestContainerUtil.java index db460930d0..c0c3853f0c 100644 --- a/test-utils/src/main/java/apoc/util/TestContainerUtil.java +++ b/test-utils/src/main/java/apoc/util/TestContainerUtil.java @@ -103,6 +103,7 @@ public static Neo4jContainerExtension createEnterpriseDB(File baseDir, boolean w .withEnv("NEO4J_dbms_memory_heap_max__size", "512M") .withEnv("NEO4J_dbms_memory_pagecache_size", "256M") .withEnv("apoc.export.file.enabled", "true") + .withEnv("apoc.import.file.enabled", "true") .withNeo4jConfig("dbms.security.procedures.unrestricted", "apoc.*") .withFileSystemBind(canonicalPath, "/var/lib/neo4j/import") // map the "target/import" dir as the Neo4j's import dir .withEnv("NEO4J_ACCEPT_LICENSE_AGREEMENT", "yes")