Lost properties in property existence enforcing

Fix error on arrays union that was causing result array missing part of the values. One of the observable effects was that property constraint enforcer missed part of the properties and it was possible to create nodes with properties set that do not satisfy all defined constraints. Add missing tests for node key constraint creation and validation. Move ImpermanentEnterpriseDatabaseRule to correct package.
neo4j · Apr 18, 2017 · 54dfe41 · 54dfe41
1 parent f0475ef
commit 54dfe41
Show file tree

Hide file tree

Showing 7 changed files with 185 additions and 29 deletions.
diff --git a/...y/primitive-collections/src/main/java/org/neo4j/collection/primitive/PrimitiveArrays.java b/...y/primitive-collections/src/main/java/org/neo4j/collection/primitive/PrimitiveArrays.java
@@ -57,7 +57,7 @@ public static int[] union( int[] lhs, int[] rhs )
         int[] merged = null;
         int m = 0;
         int l = 0;
-        for ( int r = 0; l < lhs.length && r < rhs.length; )
+        for ( int r = 0; l <= lhs.length && r < rhs.length; )
         {
             while ( l < lhs.length && lhs[l] < rhs[r] )
             {

diff --git a/...ve-collections/src/test/java/org/neo4j/collection/primitive/PrimitiveArraysUnionTest.java b/...ve-collections/src/test/java/org/neo4j/collection/primitive/PrimitiveArraysUnionTest.java
@@ -19,12 +19,12 @@
  */
 package org.neo4j.collection.primitive;
 
-import java.util.Arrays;
-
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
+import java.util.Arrays;
+
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertSame;
 import static org.neo4j.collection.primitive.PrimitiveArrays.union;
@@ -47,7 +47,9 @@ public static Iterable<Object[]> parameters()
                 lhs( 1, 3, 5 ).rhs( 2, 4, 6 ).expect( 1, 2, 3, 4, 5, 6 ),
                 lhs( 1, 2, 3, 5 ).rhs( 2, 4, 6 ).expect( 1, 2, 3, 4, 5, 6 ),
                 lhs( 2, 3, 4, 7, 8, 9, 12, 16, 19 ).rhs( 4, 6, 9, 11, 12, 15 )
-                        .expect( 2, 3, 4, 6, 7, 8, 9, 11, 12, 15, 16, 19 )
+                        .expect( 2, 3, 4, 6, 7, 8, 9, 11, 12, 15, 16, 19 ),
+                lhs( 10, 13 ).rhs( 13, 18 ).expect( 10, 13, 18 ),
+                lhs( 13, 18 ).rhs( 10, 13 ).expect( 10, 13, 18 )
         );
     }
 
@@ -63,7 +65,7 @@ public PrimitiveArraysUnionTest( Input input )
     }
 
     @Test
-    public void testMerge() throws Exception
+    public void testUnion() throws Exception
     {
         int[] actual = union( lhs, rhs );
         if ( lhs == expected || rhs == expected )

diff --git a/enterprise/kernel/src/test/java/org/neo4j/SchemaHelper.java b/enterprise/kernel/src/test/java/org/neo4j/SchemaHelper.java
@@ -19,13 +19,17 @@
  */
 package org.neo4j;
 
+import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 
 import org.neo4j.graphdb.GraphDatabaseService;
 import org.neo4j.graphdb.Label;
 import org.neo4j.graphdb.RelationshipType;
 import org.neo4j.graphdb.Transaction;
 
+import static java.lang.String.format;
+import static java.util.stream.Collectors.joining;
+
 public final class SchemaHelper
 {
     private SchemaHelper()
@@ -40,7 +44,7 @@ public static void createIndex( GraphDatabaseService db, Label label, String pro
 
     public static void createIndex( GraphDatabaseService db, String label, String property )
     {
-        db.execute( String.format( "CREATE INDEX ON :`%s`(`%s`)", label, property ) );
+        db.execute( format( "CREATE INDEX ON :`%s`(`%s`)", label, property ) );
     }
 
     public static void createUniquenessConstraint( GraphDatabaseService db, Label label, String property )
@@ -50,17 +54,20 @@ public static void createUniquenessConstraint( GraphDatabaseService db, Label la
 
     public static void createUniquenessConstraint( GraphDatabaseService db, String label, String property )
     {
-        db.execute( String.format( "CREATE CONSTRAINT ON (n:`%s`) ASSERT n.`%s` IS UNIQUE", label, property ) );
+        db.execute( format( "CREATE CONSTRAINT ON (n:`%s`) ASSERT n.`%s` IS UNIQUE", label, property ) );
     }
 
-    public static void createNodeKeyConstraint( GraphDatabaseService db, Label label, String property )
+    public static void createNodeKeyConstraint( GraphDatabaseService db, Label label, String... properties )
     {
-        createNodeKeyConstraint( db, label.name(), property );
+        createNodeKeyConstraint( db, label.name(), properties );
     }
 
-    public static void createNodeKeyConstraint( GraphDatabaseService db, String label, String property )
+    public static void createNodeKeyConstraint( GraphDatabaseService db, String label, String... properties )
     {
-        db.execute( String.format( "CREATE CONSTRAINT ON (n:`%s`) ASSERT (n.`%s`) IS NODE KEY", label, property ) );
+        String keyProperties = Arrays.stream( properties )
+                .map( property -> format("n.`%s`", property))
+                .collect( joining( "," ) );
+        db.execute( format( "CREATE CONSTRAINT ON (n:`%s`) ASSERT (%s) IS NODE KEY", label, keyProperties ) );
     }
 
     public static void createNodePropertyExistenceConstraint( GraphDatabaseService db, Label label, String property )
@@ -70,7 +77,7 @@ public static void createNodePropertyExistenceConstraint( GraphDatabaseService d
 
     public static void createNodePropertyExistenceConstraint( GraphDatabaseService db, String label, String property )
     {
-        db.execute( String.format( "CREATE CONSTRAINT ON (n:`%s`) ASSERT exists(n.`%s`)", label, property ) );
+        db.execute( format( "CREATE CONSTRAINT ON (n:`%s`) ASSERT exists(n.`%s`)", label, property ) );
     }
 
     public static void createRelPropertyExistenceConstraint( GraphDatabaseService db, RelationshipType type,
@@ -81,7 +88,7 @@ public static void createRelPropertyExistenceConstraint( GraphDatabaseService db
 
     public static void createRelPropertyExistenceConstraint( GraphDatabaseService db, String type, String property )
     {
-        db.execute( String.format( "CREATE CONSTRAINT ON ()-[r:`%s`]-() ASSERT exists(r.`%s`)", type, property ) );
+        db.execute( format( "CREATE CONSTRAINT ON ()-[r:`%s`]-() ASSERT exists(r.`%s`)", type, property ) );
     }
 
     public static void awaitIndexes( GraphDatabaseService db )

diff --git a/...rnel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java b/...rnel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java
@@ -19,6 +19,11 @@
  */
 package org.neo4j.kernel.enterprise.builtinprocs;
 
+import org.hamcrest.Matcher;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+
 import java.io.PrintWriter;
 import java.util.HashSet;
 import java.util.List;
@@ -30,18 +35,13 @@
 import java.util.function.BiConsumer;
 import java.util.function.Supplier;
 
-import org.hamcrest.Matcher;
-import org.junit.Ignore;
-import org.junit.Rule;
-import org.junit.Test;
-
 import org.neo4j.graphdb.Node;
 import org.neo4j.graphdb.Result;
 import org.neo4j.graphdb.Transaction;
 import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
-import org.neo4j.kernel.enterprise.ImpermanentEnterpriseDatabaseRule;
 import org.neo4j.kernel.impl.factory.GraphDatabaseFacade;
 import org.neo4j.test.rule.DatabaseRule;
+import org.neo4j.test.rule.ImpermanentEnterpriseDatabaseRule;
 import org.neo4j.test.rule.concurrent.ThreadingRule;
 
 import static java.util.Collections.singletonMap;

diff --git a/.../src/test/java/org/neo4j/kernel/impl/api/integrationtest/NodeKeyConstraintCreationIT.java b/.../src/test/java/org/neo4j/kernel/impl/api/integrationtest/NodeKeyConstraintCreationIT.java
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.kernel.impl.api.integrationtest;
+
+import org.neo4j.SchemaHelper;
+import org.neo4j.graphdb.GraphDatabaseService;
+import org.neo4j.graphdb.Label;
+import org.neo4j.graphdb.Node;
+import org.neo4j.graphdb.ResourceIterator;
+import org.neo4j.kernel.api.SchemaWriteOperations;
+import org.neo4j.kernel.api.TokenWriteOperations;
+import org.neo4j.kernel.api.exceptions.KernelException;
+import org.neo4j.kernel.api.schema.LabelSchemaDescriptor;
+import org.neo4j.kernel.api.schema.SchemaDescriptorFactory;
+import org.neo4j.kernel.api.schema.constaints.ConstraintDescriptorFactory;
+import org.neo4j.kernel.api.schema.constaints.NodeKeyConstraintDescriptor;
+
+public class NodeKeyConstraintCreationIT extends AbstractConstraintCreationIT<NodeKeyConstraintDescriptor,LabelSchemaDescriptor>
+{
+    @Override
+    int initializeLabelOrRelType( TokenWriteOperations tokenWriteOperations, String name ) throws KernelException
+    {
+        return tokenWriteOperations.labelGetOrCreateForName( name );
+    }
+
+    @Override
+    NodeKeyConstraintDescriptor createConstraint( SchemaWriteOperations writeOps, LabelSchemaDescriptor descriptor )
+            throws Exception
+    {
+        return writeOps.nodeKeyConstraintCreate( descriptor );
+    }
+
+    @Override
+    void createConstraintInRunningTx( GraphDatabaseService db, String type, String property )
+    {
+        SchemaHelper.createNodeKeyConstraint( db, type, property );
+    }
+
+    @Override
+    NodeKeyConstraintDescriptor newConstraintObject( LabelSchemaDescriptor descriptor )
+    {
+        return ConstraintDescriptorFactory.nodeKeyForSchema( descriptor );
+    }
+
+    @Override
+    void dropConstraint( SchemaWriteOperations writeOps, NodeKeyConstraintDescriptor constraint )
+            throws Exception
+    {
+        writeOps.constraintDrop( constraint );
+    }
+
+    @Override
+    void createOffendingDataInRunningTx( GraphDatabaseService db )
+    {
+        db.createNode( Label.label( KEY ) );
+    }
+
+    @Override
+    void removeOffendingDataInRunningTx( GraphDatabaseService db )
+    {
+        try ( ResourceIterator<Node> nodes = db.findNodes( Label.label( KEY ) ) )
+        {
+            while ( nodes.hasNext() )
+            {
+                nodes.next().delete();
+            }
+        }
+    }
+
+    @Override
+    LabelSchemaDescriptor makeDescriptor( int typeId, int propertyKeyId )
+    {
+        return SchemaDescriptorFactory.forLabel( typeId, propertyKeyId );
+    }
+}
diff --git a/...pertyExistenceConstraintValidationIT.java → ...ntest/PropertyConstraintValidationIT.java b/...pertyExistenceConstraintValidationIT.java → ...ntest/PropertyConstraintValidationIT.java
@@ -26,8 +26,13 @@
 
 import java.util.UUID;
 
+import org.neo4j.SchemaHelper;
 import org.neo4j.collection.primitive.PrimitiveLongCollections;
+import org.neo4j.graphdb.ConstraintViolationException;
 import org.neo4j.graphdb.GraphDatabaseService;
+import org.neo4j.graphdb.Label;
+import org.neo4j.graphdb.Node;
+import org.neo4j.graphdb.Transaction;
 import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
 import org.neo4j.kernel.api.DataWriteOperations;
 import org.neo4j.kernel.api.ReadOperations;
@@ -41,6 +46,7 @@
 import org.neo4j.kernel.api.properties.DefinedProperty;
 import org.neo4j.kernel.api.security.AnonymousContext;
 import org.neo4j.test.TestEnterpriseGraphDatabaseFactory;
+import org.neo4j.test.assertion.Assert;
 
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertEquals;
@@ -49,17 +55,67 @@
 import static org.neo4j.kernel.api.properties.Property.property;
 import static org.neo4j.kernel.api.schema.SchemaDescriptorFactory.forLabel;
 import static org.neo4j.kernel.api.schema.SchemaDescriptorFactory.forRelType;
-import static org.neo4j.kernel.impl.api.integrationtest.PropertyExistenceConstraintValidationIT.NodePropertyExistenceExistenceConstraintValidationIT;
+import static org.neo4j.kernel.impl.api.integrationtest.PropertyConstraintValidationIT.NodeKeyConstraintValidationIT;
-import static org.neo4j.kernel.impl.api.integrationtest.PropertyExistenceConstraintValidationIT.RelationshipPropertyExistenceExistenceConstraintValidationIT;
+import static org.neo4j.kernel.impl.api.integrationtest.PropertyConstraintValidationIT.NodePropertyExistenceConstraintValidationIT;
+import static org.neo4j.kernel.impl.api.integrationtest.PropertyConstraintValidationIT.RelationshipPropertyExistenceConstraintValidationIT;
 
 @RunWith( Suite.class )
 @SuiteClasses( {
-        NodePropertyExistenceExistenceConstraintValidationIT.class,
+        NodePropertyExistenceConstraintValidationIT.class,
-        RelationshipPropertyExistenceExistenceConstraintValidationIT.class
+        RelationshipPropertyExistenceConstraintValidationIT.class,
+        NodeKeyConstraintValidationIT.class
 } )
-public class PropertyExistenceConstraintValidationIT
+public class PropertyConstraintValidationIT
 {
-    public static class NodePropertyExistenceExistenceConstraintValidationIT
+    public static class NodeKeyConstraintValidationIT extends NodePropertyExistenceConstraintValidationIT
+    {
+        @Override
+        void createConstraint( String key, String property ) throws KernelException
+        {
+            TokenWriteOperations tokenWriteOperations = tokenWriteOperationsInNewTransaction();
+            int label = tokenWriteOperations.labelGetOrCreateForName( key );
+            int propertyKey = tokenWriteOperations.propertyKeyGetOrCreateForName( property );
+            commit();
+
+            SchemaWriteOperations schemaWrite = schemaWriteOperationsInNewTransaction();
+            schemaWrite.nodeKeyConstraintCreate( forLabel( label, propertyKey ) );
+            commit();
+        }
+
+        @Test
+        public void requirePropertyFromMultipleNodeKeys() throws Exception
+        {
+            Label label = Label.label( "multiNodeKeyLabel" );
+            SchemaHelper.createNodeKeyConstraint( db, label,  "property1", "property2" );
+            SchemaHelper.createNodeKeyConstraint( db, label,  "property2", "property3" );
+            SchemaHelper.createNodeKeyConstraint( db, label,  "property3", "property4" );
+
+            Assert.assertException( () ->
+            {
+                try ( Transaction transaction = db.beginTx() )
+                {
+                    Node node = db.createNode( label );
+                    node.setProperty( "property1", "1" );
+                    node.setProperty( "property2", "2" );
+                    transaction.success();
+                }
+            }, ConstraintViolationException.class, "Node(0) with label `multiNodeKeyLabel` must have the properties `property2, property3`" );
+
+            Assert.assertException( () ->
+            {
+                try ( Transaction transaction = db.beginTx() )
+                {
+                    Node node = db.createNode( label );
+                    node.setProperty( "property1", "1" );
+                    node.setProperty( "property2", "2" );
+                    node.setProperty( "property3", "3" );
+                    transaction.success();
+                }
+            }, ConstraintViolationException.class, "Node(1) with label `multiNodeKeyLabel` must have the properties `property3, property4`" );
+        }
+    }
+
+    public static class NodePropertyExistenceConstraintValidationIT
             extends AbstractPropertyExistenceConstraintValidationIT
     {
         @Test
@@ -155,7 +211,7 @@ int entityCount() throws TransactionFailureException
         }
     }
 
-    public static class RelationshipPropertyExistenceExistenceConstraintValidationIT
+    public static class RelationshipPropertyExistenceConstraintValidationIT
             extends AbstractPropertyExistenceConstraintValidationIT
     {
         @Override
@@ -360,7 +416,7 @@ public void shouldAllowCreationOfNonConflictingData() throws Exception
             createEntity( statement, "key1", "value1" );
             createEntity( statement, "Type2" );
             createEntity( statement, "Type1", "key1", "value2" );
-            createEntity( statement, "Type1", "key1", "value1" );
+            createEntity( statement, "Type1", "key1", "value3" );
 
             commit();
 

diff --git a/...se/ImpermanentEnterpriseDatabaseRule.java → ...le/ImpermanentEnterpriseDatabaseRule.java b/...se/ImpermanentEnterpriseDatabaseRule.java → ...le/ImpermanentEnterpriseDatabaseRule.java
@@ -17,11 +17,10 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
-package org.neo4j.kernel.enterprise;
+package org.neo4j.test.rule;
 
 import org.neo4j.graphdb.factory.GraphDatabaseFactory;
 import org.neo4j.test.TestEnterpriseGraphDatabaseFactory;
-import org.neo4j.test.rule.ImpermanentDatabaseRule;
 
 public class ImpermanentEnterpriseDatabaseRule extends ImpermanentDatabaseRule
 {