Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VYidKhdO] The required constraint on import key for import.json is unique #3539

Merged
merged 1 commit into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions core/src/main/java/apoc/export/json/JsonImporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -142,11 +143,11 @@ private void manageRelationship(Map<String, Object> param) {
"label", getType(param));
List<String> 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;
}
Expand All @@ -155,17 +156,22 @@ private void manageRelationship(Map<String, Object> param) {
private void manageNode(Map<String, Object> param) {
List<String> 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<String> 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<String> labels) {
if (labels.isEmpty()) {
return;
}
Expand All @@ -174,7 +180,8 @@ private void checkConstraints(List<String> 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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String> 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<String> 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<String, Object> 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"));
}
);
}
}
17 changes: 17 additions & 0 deletions core/src/test/java/apoc/export/json/ImportJsonTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions test-utils/src/main/java/apoc/util/TestContainerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down