diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java index 5a8fff2340bce..e961765ef3ad9 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Write.java +++ b/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 true if a label was added otherwise false */ - 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 true if node was removed otherwise false */ - void nodeRemoveLabel( long node, int nodeLabel ); + boolean nodeRemoveLabel( long node, int nodeLabel ) throws KernelException; /** * Set a property on a node diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeWriteTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeWriteTestBase.java index f963dd4bed157..8bf6f5d6ba889 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeWriteTestBase.java +++ b/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 extends KernelAPIWriteTestBase { + @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() ) ); + } + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java index cd995ad62eb93..25e7cc406ab14 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java +++ b/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 explicitIndexes; AllStoreHolder( StorageEngine engine, - StorageStatement statement, - Supplier explicitIndexes, - Cursors cursors ) + StorageStatement statement, + Supplier 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; + } + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java index 7e8c526016b0b..375469136d9b6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java +++ b/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 diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexCursorFilterTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexCursorFilterTest.java index eafb501ccbe5d..35f14446cfb8e 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/IndexCursorFilterTest.java +++ b/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; }