Sad path tests for label creation and deletion

community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java

@@ -19,6 +19,7 @@
  */
 package org.neo4j.internal.kernel.api;
 
+import org.neo4j.internal.kernel.api.exceptions.KernelException;
 import org.neo4j.values.storable.Value;
 
 /**
@@ -58,15 +59,17 @@ public interface Write
      * Add a label to a node
      * @param node the internal node id
      * @param nodeLabel the internal id of the label to add
+     * @return <tt>true</tt> if a label was added otherwise <tt>false</tt>
      */
-    void nodeAddLabel( long node, int nodeLabel );
+    boolean nodeAddLabel( long node, int nodeLabel ) throws KernelException;
 
     /**
      * Remove a label from a node
      * @param node the internal node id
      * @param nodeLabel the internal id of the label to remove
+     * @return <tt>true</tt> if node was removed otherwise <tt>false</tt>
      */
-    void nodeRemoveLabel( long node, int nodeLabel );
+    boolean nodeRemoveLabel( long node, int nodeLabel ) throws KernelException;
 
     /**
      * Set a property on a node

community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeWriteTestBase.java

@@ -19,20 +19,27 @@
  */
 package org.neo4j.internal.kernel.api;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import org.neo4j.graphdb.NotFoundException;
 import org.neo4j.helpers.collection.Iterables;
+import org.neo4j.internal.kernel.api.exceptions.KernelException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.neo4j.graphdb.Label.label;
 
 public abstract class NodeWriteTestBase<G extends KernelAPIWriteTestSupport> extends KernelAPIWriteTestBase<G>
 {
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
     @Test
     public void shouldCreateNode() throws Exception
     {
@@ -120,17 +127,51 @@ public void shouldNotRemoveNodeThatDoesNotExist() throws Exception
     @Test
     public void shouldAddLabelNode() throws Exception
     {
+        // Given
         long node;
         int labelId;
         final String labelName = "Town";
+        try ( org.neo4j.graphdb.Transaction ctx = graphDb.beginTx() )
+        {
+            node = graphDb.createNode( label( labelName ) ).getId();
+            ctx.success();
+        }
+
+        // When
         try ( Transaction tx = session.beginTransaction() )
         {
-            node = tx.dataWrite().nodeCreate();
             labelId = session.token().labelGetOrCreateForName( labelName );
             tx.dataWrite().nodeAddLabel( node, labelId );
             tx.success();
         }
 
+        // Then
+        try ( org.neo4j.graphdb.Transaction ctx = graphDb.beginTx() )
+        {
+            assertThat( graphDb.getNodeById( node ).getLabels(), equalTo( Iterables.iterable( label( labelName ) ) ) );
+        }
+    }
+
+    @Test
+    public void shouldAddLabelNodeOnce() throws Exception
+    {
+        long node;
+        int labelId;
+        final String labelName = "Town";
+
+        try ( org.neo4j.graphdb.Transaction ctx = graphDb.beginTx() )
+        {
+            node = graphDb.createNode( label( labelName ) ).getId();
+            ctx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            labelId = session.token().labelGetOrCreateForName( labelName );
+            assertFalse( tx.dataWrite().nodeAddLabel( node, labelId ) );
+            tx.success();
+        }
+
         try ( org.neo4j.graphdb.Transaction ctx = graphDb.beginTx() )
         {
             assertThat( graphDb.getNodeById( node ).getLabels(), equalTo( Iterables.iterable( label( labelName ) ) ) );
@@ -163,5 +204,50 @@ public void shouldRemoveLabel() throws Exception
         }
     }
 
+    @Test
+    public void shouldNotAddLabelToNonExistingNode() throws Exception
+    {
+        long node = 1337L;
+        int labelId;
+        final String labelName = "Town";
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            labelId = session.token().labelGetOrCreateForName( labelName );
+            exception.expect( KernelException.class );
+            tx.dataWrite().nodeAddLabel( node, labelId );
+        }
+    }
 
+    @Test
+    public void shouldRemoveLabelOnce() throws Exception
+    {
+        long nodeId;
+        int labelId;
+        final String labelName = "Town";
+
+        try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
+        {
+            nodeId = graphDb.createNode( label( labelName ) ).getId();
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            labelId = session.token().labelGetOrCreateForName( labelName );
+            assertTrue( tx.dataWrite().nodeRemoveLabel( nodeId, labelId ) );
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            labelId = session.token().labelGetOrCreateForName( labelName );
+            assertFalse( tx.dataWrite().nodeRemoveLabel( nodeId, labelId ) );
+            tx.success();
+        }
+
+        try ( org.neo4j.graphdb.Transaction ctx = graphDb.beginTx() )
+        {
+            assertThat( graphDb.getNodeById( nodeId ).getLabels(), equalTo( Iterables.empty() ) );
+        }
+    }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java

@@ -26,10 +26,12 @@
 import org.neo4j.function.Suppliers.Lazy;
 import org.neo4j.internal.kernel.api.CapableIndexReference;
 import org.neo4j.internal.kernel.api.IndexCapability;
+import org.neo4j.internal.kernel.api.LabelSet;
 import org.neo4j.internal.kernel.api.Token;
 import org.neo4j.internal.kernel.api.exceptions.KernelException;
 import org.neo4j.io.pagecache.PageCursor;
 import org.neo4j.kernel.api.ExplicitIndex;
+import org.neo4j.kernel.api.exceptions.EntityNotFoundException;
 import org.neo4j.kernel.api.exceptions.explicitindex.ExplicitIndexNotFoundKernelException;
 import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
 import org.neo4j.kernel.api.schema.LabelSchemaDescriptor;
@@ -38,14 +40,13 @@
 import org.neo4j.kernel.api.txstate.ExplicitIndexTransactionState;
 import org.neo4j.kernel.impl.api.store.PropertyUtil;
 import org.neo4j.kernel.impl.store.RecordCursor;
-import org.neo4j.kernel.impl.store.RecordStore;
-import org.neo4j.kernel.impl.store.record.AbstractBaseRecord;
 import org.neo4j.kernel.impl.store.record.DynamicRecord;
 import org.neo4j.kernel.impl.store.record.NodeRecord;
 import org.neo4j.kernel.impl.store.record.PropertyRecord;
 import org.neo4j.kernel.impl.store.record.RecordLoad;
 import org.neo4j.kernel.impl.store.record.RelationshipGroupRecord;
 import org.neo4j.kernel.impl.store.record.RelationshipRecord;
+import org.neo4j.storageengine.api.EntityType;
 import org.neo4j.storageengine.api.StorageEngine;
 import org.neo4j.storageengine.api.StorageStatement;
 import org.neo4j.storageengine.api.StoreReadLayer;
@@ -56,8 +57,6 @@
 import org.neo4j.values.storable.TextValue;
 import org.neo4j.values.storable.Values;
 
-import static org.neo4j.kernel.impl.store.record.RecordLoad.NORMAL;
-
 class AllStoreHolder extends Read implements Token
 {
     private final StorageStatement.Nodes nodes;
@@ -69,9 +68,9 @@ class AllStoreHolder extends Read implements Token
     private final Lazy<ExplicitIndexTransactionState> explicitIndexes;
 
     AllStoreHolder( StorageEngine engine,
-                    StorageStatement statement,
-                    Supplier<ExplicitIndexTransactionState> explicitIndexes,
-                    Cursors cursors )
+            StorageStatement statement,
+            Supplier<ExplicitIndexTransactionState> explicitIndexes,
+            Cursors cursors )
     {
         super( cursors );
         this.read = engine.storeReadLayer();
@@ -296,4 +295,25 @@ public boolean nodeExists( long id )
     {
         return read.nodeExists( id );
     }
+
+    public boolean nodeHasLabel( long node, int nodeLabel ) throws KernelException
+    {
+        try ( org.neo4j.internal.kernel.api.NodeCursor nodes = cursors.allocateNodeCursor() )
+        {
+            singleNode( node, nodes );
+            if ( !nodes.next() )
+            {
+                throw new EntityNotFoundException( EntityType.NODE, node );
+            }
+            LabelSet labels = nodes.labels();
+            for ( int i = 0; i < labels.numberOfLabels(); i++ )
+            {
+                if ( labels.label( i ) == nodeLabel )
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+    }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java

@@ -364,18 +364,31 @@ public void relationshipDelete( long relationship )
     }
 
     @Override
-    public void nodeAddLabel( long node, int nodeLabel )
+    public boolean nodeAddLabel( long node, int nodeLabel ) throws KernelException
     {
         assertOpen();
+        if ( allStoreHolder.nodeHasLabel( node, nodeLabel ) )
+        {
+            return false;
+        }
+        //TODO indexTxStateUpdater.onLabelChange
+
         ktx.txState().nodeDoAddLabel( nodeLabel, node );
+        return true;
     }
 
     @Override
-    public void nodeRemoveLabel( long node, int nodeLabel )
+    public boolean nodeRemoveLabel( long node, int nodeLabel ) throws KernelException
     {
         assertOpen();
+        //TODO indexTxStateUpdater.onLabelChange
+        if ( !allStoreHolder.nodeHasLabel( node, nodeLabel ) )
+        {
+            return false;
+        }
 
         ktx.txState().nodeDoRemoveLabel( nodeLabel, node );
+        return true;
     }
 
     @Override

community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexCursorFilterTest.java

@@ -27,6 +27,7 @@
 import java.util.List;
 
 import org.neo4j.internal.kernel.api.IndexQuery;
+
 import org.neo4j.storageengine.api.schema.IndexProgressor;
 import org.neo4j.storageengine.api.schema.IndexProgressor.NodeValueClient;
 import org.neo4j.values.storable.Value;
@@ -128,7 +129,7 @@ public void shouldNotAcceptNodeWithoutMatchingProperty() throws Exception
     private NodeValueClientFilter initializeFilter( int[] keys, IndexQuery... filters )
     {
         NodeValueClientFilter filter = new NodeValueClientFilter(
-                this, new NodeCursor(), new PropertyCursor(), null, filters );  //TODO: change null to actual Read
+                this, new NodeCursor(), new PropertyCursor(), store, filters );
         filter.initialize( this, keys );
         return filter;
     }