More NODE KEY acceptance tests and various related bug-fixes

community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java

@@ -28,7 +28,7 @@ public class ConstraintViolationTransactionFailureException extends TransactionF
 {
     public ConstraintViolationTransactionFailureException( String msg, KernelException cause )
     {
-        super( Status.Schema.ConstraintValidationFailed, msg, cause );
+        super( Status.Schema.ConstraintValidationFailed, cause, msg );
     }
 
     public ConstraintViolationTransactionFailureException( String msg )

community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/schema/UniquePropertyValueValidationException.java

@@ -26,26 +26,26 @@
 import org.neo4j.kernel.api.TokenNameLookup;
 import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException;
 import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor;
-import org.neo4j.kernel.api.schema_new.constaints.UniquenessConstraintDescriptor;
+import org.neo4j.kernel.api.schema_new.constaints.IndexBackedConstraintDescriptor;
 
 public class UniquePropertyValueValidationException extends ConstraintValidationException
 {
     private final Set<IndexEntryConflictException> conflicts;
 
-    public UniquePropertyValueValidationException( UniquenessConstraintDescriptor constraint,
+    public UniquePropertyValueValidationException( IndexBackedConstraintDescriptor constraint,
             ConstraintValidationException.Phase phase, IndexEntryConflictException conflict )
     {
         this( constraint, phase, Collections.singleton( conflict ) );
     }
 
-    public UniquePropertyValueValidationException( UniquenessConstraintDescriptor constraint,
+    public UniquePropertyValueValidationException( IndexBackedConstraintDescriptor constraint,
             ConstraintValidationException.Phase phase, Set<IndexEntryConflictException> conflicts )
     {
         super( constraint, phase, phase == Phase.VERIFICATION ? "Existing data" : "New data" );
         this.conflicts = conflicts;
     }
 
-    public UniquePropertyValueValidationException( UniquenessConstraintDescriptor constraint,
+    public UniquePropertyValueValidationException( IndexBackedConstraintDescriptor constraint,
             ConstraintValidationException.Phase phase, Throwable cause )
     {
         super( constraint, phase, phase == Phase.VERIFICATION ? "Existing data" : "New data", cause );

community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java

@@ -56,6 +56,7 @@
 import org.neo4j.kernel.api.schema_new.RelationTypeSchemaDescriptor;
 import org.neo4j.kernel.api.schema_new.SchemaDescriptor;
 import org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptor;
+import org.neo4j.kernel.api.schema_new.constaints.IndexBackedConstraintDescriptor;
 import org.neo4j.kernel.api.schema_new.constaints.NodeExistenceConstraintDescriptor;
 import org.neo4j.kernel.api.schema_new.constaints.NodeKeyConstraintDescriptor;
 import org.neo4j.kernel.api.schema_new.constaints.RelExistenceConstraintDescriptor;
@@ -91,7 +92,7 @@ public class ConstraintEnforcingEntityOperations implements EntityOperations, Sc
     private final SchemaWriteOperations schemaWriteOperations;
     private final SchemaReadOperations schemaReadOperations;
     private final ConstraintSemantics constraintSemantics;
-    private final NodeSchemaMatcher<UniquenessConstraintDescriptor> nodeSchemaMatcher;
+    private final NodeSchemaMatcher nodeSchemaMatcher;
 
     public ConstraintEnforcingEntityOperations(
             ConstraintSemantics constraintSemantics, EntityWriteOperations entityWriteOperations,
@@ -104,7 +105,7 @@ public ConstraintEnforcingEntityOperations(
         this.entityReadOperations = entityReadOperations;
         this.schemaWriteOperations = schemaWriteOperations;
         this.schemaReadOperations = schemaReadOperations;
-        nodeSchemaMatcher = new NodeSchemaMatcher<>( entityReadOperations );
+        nodeSchemaMatcher = new NodeSchemaMatcher( entityReadOperations );
     }
 
     @Override
@@ -120,7 +121,7 @@ public boolean nodeAddLabel( KernelStatement state, long nodeId, int labelId )
                 ConstraintDescriptor constraint = constraints.next();
                 if ( constraint.type().enforcesUniqueness() )
                 {
-                    UniquenessConstraintDescriptor uniqueConstraint = (UniquenessConstraintDescriptor) constraint;
+                    IndexBackedConstraintDescriptor uniqueConstraint = (IndexBackedConstraintDescriptor) constraint;
                     ExactPredicate[] propertyValues = getAllPropertyValues( state, uniqueConstraint.schema(), node );
                     if ( propertyValues != null )
                     {
@@ -141,15 +142,13 @@ public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProp
         {
             NodeItem node = cursor.get();
             Iterator<ConstraintDescriptor> constraints = getConstraintsInvolvingProperty( state, property.propertyKeyId() );
-            Iterator<UniquenessConstraintDescriptor> uniquenessConstraints =
-                    new CastingIterator<>( constraints, UniquenessConstraintDescriptor.class );
+            Iterator<IndexBackedConstraintDescriptor> uniquenessConstraints =
+                    new CastingIterator<>( constraints, IndexBackedConstraintDescriptor.class );
 
             nodeSchemaMatcher.onMatchingSchema( state, uniquenessConstraints, node, property.propertyKeyId(),
-                    constraint -> {
-                        validateNoExistingNodeWithExactValues( state, constraint,
-                                getAllPropertyValues( state, constraint.schema(), node, property ),
-                                node.id() );
-                    } );
+                    constraint -> validateNoExistingNodeWithExactValues( state, constraint,
+                            getAllPropertyValues( state, constraint.schema(), node, property ),
+                            node.id() ) );
         }
 
         return entityWriteOperations.nodeSetProperty( state, nodeId, property );
@@ -217,7 +216,7 @@ private ExactPredicate[] getAllPropertyValues( KernelStatement state, SchemaDesc
 
     private void validateNoExistingNodeWithExactValues(
             KernelStatement state,
-            UniquenessConstraintDescriptor constraint,
+            IndexBackedConstraintDescriptor constraint,
             ExactPredicate[] propertyValues,
             long modifiedNode
     ) throws ConstraintValidationException
@@ -561,6 +560,10 @@ public void uniqueIndexDrop( KernelStatement state, NewIndexDescriptor descripto
     public NodeKeyConstraintDescriptor nodeKeyConstraintCreate( KernelStatement state, LabelSchemaDescriptor descriptor )
             throws AlreadyConstrainedException, CreateConstraintFailureException, AlreadyIndexedException
     {
+        Iterator<Cursor<NodeItem>> nodes = new NodeLoadingIterator( nodesGetForLabel( state, descriptor.getLabelId() ),
+                ( id ) -> nodeCursorById( state, id ) );
+        constraintSemantics.validateNodeKeyConstraint( nodes, descriptor,
+                (node, propertyKey) -> entityReadOperations.nodeHasProperty(state, node, propertyKey) );
         return schemaWriteOperations.nodeKeyConstraintCreate( state, descriptor );
     }
 

community/kernel/src/main/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcher.java

@@ -32,9 +32,8 @@
 
 /**
  * This class holds functionality to match LabelSchemaDescriptors to nodes
- * @param <SUPPLIER> the type to match. Must implement LabelSchemaSupplier
  */
-public class NodeSchemaMatcher<SUPPLIER extends LabelSchemaSupplier>
+public class NodeSchemaMatcher
 {
     private final EntityReadOperations readOps;
 
@@ -57,10 +56,11 @@ public NodeSchemaMatcher( EntityReadOperations readOps )
      * @param specialPropertyId This property id will always count as a match for the descriptor, regardless of
      *                          whether the node has this property or not
      * @param action The action to take on match
+     * @param <SUPPLIER> the type to match. Must implement LabelSchemaDescriptor.Supplier
      * @param <EXCEPTION> The type of exception that can be thrown when taking the action
      * @throws EXCEPTION This exception is propagated from the action
      */
-    public <EXCEPTION extends Exception> void onMatchingSchema(
+    public <SUPPLIER extends LabelSchemaDescriptor.Supplier,EXCEPTION extends Exception> void onMatchingSchema(
             KernelStatement state,
             Iterator<SUPPLIER> schemaSuppliers,
             NodeItem node,

community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdater.java

@@ -47,13 +47,13 @@ public class IndexTxStateUpdater
 {
     private final SchemaReadOperations schemaReadOps;
     private final EntityReadOperations readOps;
-    private final NodeSchemaMatcher<NewIndexDescriptor> nodeIndexMatcher;
+    private final NodeSchemaMatcher nodeIndexMatcher;
 
     public IndexTxStateUpdater( SchemaReadOperations schemaReadOps, EntityReadOperations readOps )
     {
         this.schemaReadOps = schemaReadOps;
         this.readOps = readOps;
-        this.nodeIndexMatcher = new NodeSchemaMatcher<>( readOps );
+        this.nodeIndexMatcher = new NodeSchemaMatcher( readOps );
     }
 
     // LABEL CHANGES

community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintSemantics.java

@@ -41,6 +41,9 @@
  */
 public interface ConstraintSemantics
 {
+    void validateNodeKeyConstraint( Iterator<Cursor<NodeItem>> allNodes, LabelSchemaDescriptor descriptor,
+            BiPredicate<NodeItem,Integer> hasProperty ) throws CreateConstraintFailureException;
+
     void validateNodePropertyExistenceConstraint( Iterator<Cursor<NodeItem>> allNodes, LabelSchemaDescriptor descriptor,
             BiPredicate<NodeItem,Integer> hasProperty ) throws CreateConstraintFailureException;
 

community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/StandardConstraintSemantics.java

@@ -48,15 +48,23 @@ public void validateNodePropertyExistenceConstraint( Iterator<Cursor<NodeItem>>
             LabelSchemaDescriptor descriptor, BiPredicate<NodeItem,Integer> hasProperty )
             throws CreateConstraintFailureException
     {
-        throw propertyExistenceConstraintsNotAllowed( descriptor, ERROR_MESSAGE_NODE_KEY );
+        throw propertyExistenceConstraintsNotAllowed( descriptor );
     }
 
     @Override
     public void validateRelationshipPropertyExistenceConstraint( Cursor<RelationshipItem> allRelationships,
             RelationTypeSchemaDescriptor descriptor, BiPredicate<RelationshipItem,Integer> hasPropertyCheck )
             throws CreateConstraintFailureException
     {
-        throw propertyExistenceConstraintsNotAllowed( descriptor, ERROR_MESSAGE_EXISTS );
+        throw propertyExistenceConstraintsNotAllowed( descriptor );
+    }
+
+    @Override
+    public void validateNodeKeyConstraint( Iterator<Cursor<NodeItem>> allNodes,
+            LabelSchemaDescriptor descriptor, BiPredicate<NodeItem,Integer> hasProperty )
+            throws CreateConstraintFailureException
+    {
+        throw nodeKeyConstraintsNotAllowed( descriptor );
     }
 
     @Override
@@ -80,12 +88,20 @@ protected ConstraintDescriptor readNonStandardConstraint( ConstraintRule rule, S
         throw new IllegalStateException( errorMessage );
     }
 
-    private CreateConstraintFailureException propertyExistenceConstraintsNotAllowed( SchemaDescriptor descriptor, String errorMessage )
+    private CreateConstraintFailureException propertyExistenceConstraintsNotAllowed( SchemaDescriptor descriptor )
     {
         // When creating a Property Existence Constraint in Community Edition
         return new CreateConstraintFailureException(
-                    ConstraintDescriptorFactory.existsForSchema( descriptor ),
-                    new IllegalStateException( errorMessage ) );
+                ConstraintDescriptorFactory.existsForSchema( descriptor ),
+                new IllegalStateException( ERROR_MESSAGE_EXISTS ) );
+    }
+
+    private CreateConstraintFailureException nodeKeyConstraintsNotAllowed( SchemaDescriptor descriptor )
+    {
+        // When creating a Node Key Constraint in Community Edition
+        return new CreateConstraintFailureException(
+                ConstraintDescriptorFactory.existsForSchema( descriptor ),
+                new IllegalStateException( ERROR_MESSAGE_NODE_KEY ) );
     }
 
     @Override
@@ -99,14 +115,14 @@ public ConstraintRule createUniquenessConstraintRule(
     public ConstraintRule createNodeKeyConstraintRule(
             long ruleId, NodeKeyConstraintDescriptor descriptor, long indexId ) throws CreateConstraintFailureException
     {
-        throw propertyExistenceConstraintsNotAllowed( descriptor.schema(), ERROR_MESSAGE_NODE_KEY );
+        throw nodeKeyConstraintsNotAllowed( descriptor.schema() );
     }
 
     @Override
     public ConstraintRule createExistenceConstraint( long ruleId, ConstraintDescriptor descriptor )
             throws CreateConstraintFailureException
     {
-        throw propertyExistenceConstraintsNotAllowed( descriptor.schema(), ERROR_MESSAGE_EXISTS );
+        throw propertyExistenceConstraintsNotAllowed( descriptor.schema() );
     }
 
     @Override

community/kernel/src/test/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcherTest.java

@@ -69,7 +69,7 @@ public class NodeSchemaMatcherTest
     NewIndexDescriptor indexWithMissingLabel = forLabel( nonExistentLabelId, propId1, propId2 );
     NewIndexDescriptor indexOnSpecialProperty = forLabel( labelId1, propId1, specialPropId );
 
-    private NodeSchemaMatcher<NewIndexDescriptor> nodeSchemaMatcher;
+    private NodeSchemaMatcher nodeSchemaMatcher;
 
     @Before
     public void setup()
@@ -91,7 +91,7 @@ public void setup()
         when( readOps.nodeGetProperty( state, node, propId2 ) ).thenReturn( "hi2" );
         when( readOps.nodeGetProperty( state, node, unIndexedPropId ) ).thenReturn( "hi3" );
 
-        nodeSchemaMatcher = new NodeSchemaMatcher<>( readOps );
+        nodeSchemaMatcher = new NodeSchemaMatcher( readOps );
     }
 
     @Test

enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeConstraintAcceptanceTest.scala

@@ -19,12 +19,15 @@
  */
 package org.neo4j.internal.cypher.acceptance
 
+import org.hamcrest.CoreMatchers.containsString
+import org.junit.Assert.assertThat
 import org.neo4j.cypher.javacompat.internal.GraphDatabaseCypherService
-import org.neo4j.cypher.{CypherExecutionException, ExecutionEngineFunSuite, NewPlannerTestSupport}
-import org.neo4j.graphdb.{ConstraintViolationException, Node}
+import org.neo4j.cypher._
+import org.neo4j.graphdb.ConstraintViolationException
 import org.neo4j.kernel.GraphDatabaseQueryService
-import org.neo4j.test.{TestEnterpriseGraphDatabaseFactory, TestGraphDatabaseFactory}
+import org.neo4j.test.TestEnterpriseGraphDatabaseFactory
 import org.scalatest.matchers.{MatchResult, Matcher}
+import org.neo4j.cypher.internal.frontend.v3_2.helpers.StringHelper._
 
 import scala.collection.JavaConverters._
 
@@ -124,6 +127,84 @@ class CompositeConstraintAcceptanceTest extends ExecutionEngineFunSuite with New
     graph should not(haveConstraints("NODE_KEY:Person(firstname,lastname)"))
   }
 
+  test("trying to add duplicate node when unique constraint exists") {
+    createLabeledNode(Map("name" -> "A"), "Person")
+    executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT person.name IS UNIQUE")
+
+    expectError(
+      "CREATE (n:Person) SET n.name = 'A'",
+      String.format("Node(0) already exists with label `Person` and property `name` = 'A'")
+    )
+  }
+
+  test("trying to add a node key constraint when duplicates exist") {
+    createLabeledNode(Map("name" -> "A"), "Person")
+    createLabeledNode(Map("name" -> "A"), "Person")
+
+    expectError(
+      "CREATE CONSTRAINT ON (person:Person) ASSERT (person.name) IS NODE KEY",
+      String.format("Unable to create CONSTRAINT ON ( person:Person ) ASSERT person.name IS NODE KEY:%n" +
+        "Both Node(0) and Node(1) have the label `Person` and property `name` = 'A'")
+    )
+  }
+
+  test("trying to add duplicate node when node key constraint exists") {
+    createLabeledNode(Map("name" -> "A"), "Person")
+    executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT (person.name) IS NODE KEY")
+
+    expectError(
+      "CREATE (n:Person) SET n.name = 'A'",
+      String.format("Node(0) already exists with label `Person` and property `name` = 'A'")
+    )
+  }
+
+  test("drop a non existent node key constraint") {
+    expectError(
+      "DROP CONSTRAINT ON (person:Person) ASSERT (person.name) IS NODE KEY",
+      "No such constraint"
+    )
+  }
+
+  test("trying to add duplicate node when composite unique constraint exists") {
+    createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person")
+    executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT (person.name, person.surname) IS UNIQUE")
+
+    expectError(
+      "CREATE (n:Person) SET n.name = 'A', n.surname = 'B'",
+      String.format("Node(0) already exists with label `Person` and properties `name` = 'A', `surname` = 'B'")
+    )
+  }
+
+  test("trying to add a composite node key constraint when duplicates exist") {
+    createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person")
+    createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person")
+
+    expectError(
+      "CREATE CONSTRAINT ON (person:Person) ASSERT (person.name, person.surname) IS NODE KEY",
+      String.format("Unable to create CONSTRAINT ON ( person:Person ) ASSERT (person.name, person.surname) IS NODE KEY:%n" +
+        "Both Node(0) and Node(1) have the label `Person` and properties `name` = 'A', `surname` = 'B'")
+    )
+  }
+
+  test("trying to add duplicate node when composite node key constraint exists") {
+    createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person")
+    executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT (person.name, person.surname) IS NODE KEY")
+
+    expectError(
+      "CREATE (n:Person) SET n.name = 'A', n.surname = 'B'",
+      String.format("Node(0) already exists with label `Person` and properties `name` = 'A', `surname` = 'B'")
+    )
+  }
+
+  private def expectError(query: String, expectedError: String) {
+    val error = intercept[CypherException](executeQuery(query))
+    assertThat(error.getMessage, containsString(expectedError))
+  }
+
+  private def executeQuery(query: String) {
+    executeWithCostPlannerAndInterpretedRuntimeOnly(query.fixNewLines).toList
+  }
+
   case class haveConstraints(expectedConstraints: String*) extends Matcher[GraphDatabaseQueryService] {
     def apply(graph: GraphDatabaseQueryService): MatchResult = {
       graph.inTx {

enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/EnterpriseConstraintSemantics.java

@@ -87,6 +87,14 @@ public void validateNodePropertyExistenceConstraint( Iterator<Cursor<NodeItem>>
         }
     }
 
+    @Override
+    public void validateNodeKeyConstraint( Iterator<Cursor<NodeItem>> allNodes,
+            LabelSchemaDescriptor descriptor, BiPredicate<NodeItem,Integer> hasPropertyCheck )
+            throws CreateConstraintFailureException
+    {
+        validateNodePropertyExistenceConstraint( allNodes, descriptor, hasPropertyCheck );
+    }
+
     private void validateNodePropertyExistenceConstraint( NodeItem node, int propertyKey,
         LabelSchemaDescriptor descriptor, BiPredicate<NodeItem, Integer> hasPropertyCheck ) throws
             CreateConstraintFailureException