Merge pull request #9169 from pontusmelke/3.2-error-when-already-an-i…

…ndex

Better error message for existing index

community/cypher/cypher/src/test/scala/org/neo4j/cypher/ErrorMessagesTest.scala

@@ -238,6 +238,12 @@ class ErrorMessagesTest extends ExecutionEngineFunSuite with CypherSerializer {
       "Expected exactly one statement per query but got: 2")
   }
 
+  test("should give proper error message when trying to use Node Key constraint on community") {
+    expectError("CREATE CONSTRAINT ON (n:Person) ASSERT (n.firstname) IS NODE KEY",
+                """Unable to create CONSTRAINT ON ( person:Person ) ASSERT exists(person.firstname):
+                  |Node Key constraint requires Neo4j Enterprise Edition""".stripMargin)
+  }
+
   private def expectError(query: String, expectedError: String) {
     val error = intercept[CypherException](executeQuery(query))
     assertThat(error.getMessage, containsString(expectedError))

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

@@ -32,7 +32,7 @@ public class AlreadyConstrainedException extends SchemaKernelException
 
     private static final String ALREADY_CONSTRAINED_MESSAGE_PREFIX = "Constraint already exists: ";
 
-    private static final String INDEX_CONTEXT_FORMAT = "Label '%s' and property '%s' have a unique constraint defined on them, so an index is " +
+    private static final String INDEX_CONTEXT_FORMAT = "Label '%s' and %s have a unique constraint defined on them, so an index is " +
                                                        "already created that matches this.";
 
     private final ConstraintDescriptor constraint;

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

@@ -27,8 +27,8 @@ public class AlreadyIndexedException extends SchemaKernelException
 {
     private static final String NO_CONTEXT_FORMAT = "Already indexed %s.";
 
-    private static final String INDEX_CONTEXT_FORMAT = "There already exists an index for label '%s' on property '%s'.";
-    private static final String CONSTRAINT_CONTEXT_FORMAT = "There already exists an index for label '%s' on property '%s'. " +
+    private static final String INDEX_CONTEXT_FORMAT = "There already exists an index for label '%s' on %s.";
+    private static final String CONSTRAINT_CONTEXT_FORMAT = "There already exists an index for label '%s' on %s. " +
                                                             "A constraint cannot be created until the index has been dropped.";
 
     private final LabelSchemaDescriptor descriptor;

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

@@ -28,11 +28,20 @@ public class CreateConstraintFailureException extends SchemaKernelException
 {
     private final ConstraintDescriptor constraint;
 
+    private final String cause;
     public CreateConstraintFailureException( ConstraintDescriptor constraint, Throwable cause )
     {
         super( Status.Schema.ConstraintCreationFailed, cause, "Unable to create constraint %s: %s", constraint,
                 cause.getMessage() );
         this.constraint = constraint;
+        this.cause = null;
+    }
+
+    public CreateConstraintFailureException( ConstraintDescriptor constraint, String cause )
+    {
+        super( Status.Schema.ConstraintCreationFailed, null, "Unable to create constraint %s: %s", constraint, cause );
+        this.constraint = constraint;
+        this.cause = cause;
     }
 
     public ConstraintDescriptor constraint()
@@ -44,6 +53,10 @@ public ConstraintDescriptor constraint()
     public String getUserMessage( TokenNameLookup tokenNameLookup )
     {
         String message = "Unable to create " + constraint.prettyPrint( tokenNameLookup );
+        if (cause != null)
+        {
+            message = String.format( "%s:%n%s", message, cause );
+        }
         if ( getCause() instanceof KernelException )
         {
             KernelException cause = (KernelException) getCause();

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

@@ -19,6 +19,9 @@
  */
 package org.neo4j.kernel.api.exceptions.schema;
 
+import java.util.Arrays;
+import java.util.stream.Collectors;
+
 import org.neo4j.kernel.api.TokenNameLookup;
 import org.neo4j.kernel.api.exceptions.KernelException;
 import org.neo4j.kernel.api.exceptions.Status;
@@ -50,20 +53,29 @@ public SchemaKernelException( Status statusCode, String message )
         super( statusCode, message );
     }
 
-    protected static String messageWithLabelAndPropertyName( TokenNameLookup tokenNameLookup, String formatString,
+    static String messageWithLabelAndPropertyName( TokenNameLookup tokenNameLookup, String formatString,
             LabelSchemaDescriptor descriptor )
     {
+        int[] propertyIds = descriptor.getPropertyIds();
+
         if ( tokenNameLookup != null )
         {
+            String propertyString = propertyIds.length == 1 ?
+                                    "property '" + tokenNameLookup.propertyKeyGetName( propertyIds[0]) + "'" :
+                                    "properties " + Arrays.stream( propertyIds )
+                                            .mapToObj( (i) -> "'" + tokenNameLookup.propertyKeyGetName( i ) + "'")
+                                            .collect( Collectors.joining( " and " ));
             return String.format( formatString,
-                    tokenNameLookup.labelGetName( descriptor.getLabelId() ),
-                    tokenNameLookup.propertyKeyGetName( descriptor.getPropertyId() ) );
+                    tokenNameLookup.labelGetName( descriptor.getLabelId() ), propertyString);
         }
         else
         {
+            String keyString = propertyIds.length == 1 ? "key[" + propertyIds[0] + "]" :
+                               "keys[" + Arrays.stream( propertyIds )
+                                       .mapToObj( Integer::toString )
+                                       .collect( Collectors.joining( ", " )) + "]";
             return String.format( formatString,
-                    "label[" + descriptor.getLabelId() + "]",
-                    "key[" + descriptor.getPropertyId() + "]" );
+                    "label[" + descriptor.getLabelId() + "]", keyString );
         }
     }
 }

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

@@ -92,16 +92,14 @@ private CreateConstraintFailureException propertyExistenceConstraintsNotAllowed(
     {
         // When creating a Property Existence Constraint in Community Edition
         return new CreateConstraintFailureException(
-                ConstraintDescriptorFactory.existsForSchema( descriptor ),
-                new IllegalStateException( ERROR_MESSAGE_EXISTS ) );
+                ConstraintDescriptorFactory.existsForSchema( descriptor ), 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 ) );
+                ConstraintDescriptorFactory.existsForSchema( descriptor ), ERROR_MESSAGE_NODE_KEY );
     }
 
     @Override

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

@@ -204,6 +204,26 @@ class CompositeNodeKeyConstraintAcceptanceTest extends ExecutionEngineFunSuite w
     )
   }
 
+  test("should give appropriate error message when there is already an index") {
+    // Given
+    exec("CREATE INDEX ON :Person(firstname, lastname)")
+
+    // then
+    expectError("CREATE CONSTRAINT ON (n:Person) ASSERT (n.firstname,n.lastname) IS NODE KEY",
+                "There already exists an index for label 'Person' on properties 'firstname' and 'lastname'. " +
+                  "A constraint cannot be created until the index has been dropped.")
+  }
+
+  test("should give appropriate error message when there is already a constraint") {
+    // Given
+    exec("CREATE CONSTRAINT ON (n:Person) ASSERT (n.firstname,n.lastname) IS NODE KEY")
+
+    // then
+    expectError("CREATE INDEX ON :Person(firstname, lastname)",
+                "Label 'Person' and properties 'firstname' and 'lastname' have a unique constraint defined on them, " +
+                  "so an index is already created that matches this.")
+  }
+
   private def expectError(query: String, expectedError: String) {
     val error = intercept[CypherException](exec(query))
     assertThat(error.getMessage, containsString(expectedError))