Merge pull request #10833 from fickludd/3.4-tx-state-aware-rel-cursor

Tx-state aware RelationshipScanCursor and relationship writes

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

@@ -75,10 +75,10 @@ void nodeIndexSeek( IndexReference index, NodeValueIndexCursor cursor, IndexOrde
 
     /**
      * Checks if a node exists in the database
-     * @param id The id of the node to check
+     * @param reference The reference of the node to check
      * @return <tt>true</tt> if the node exists, otherwise <tt>false</tt>
      */
-    boolean nodeExists( long id );
+    boolean nodeExists( long reference );
 
     /**
      * @param reference

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

@@ -49,13 +49,13 @@ public interface Write
      * @param targetNode the target internal node id
      * @return the internal id of the created relationship
      */
-    long relationshipCreate( long sourceNode, int relationshipLabel, long targetNode );
+    long relationshipCreate( long sourceNode, int relationshipLabel, long targetNode ) throws KernelException;
 
     /**
      * Delete a relationship
      * @param relationship the internal id of the relationship to delete
      */
-    void relationshipDelete( long relationship );
+    boolean relationshipDelete( long relationship ) throws AutoIndexingKernelException;
 
     /**
      * Add a label to a node

community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java → community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeTransactionStateTestBase.java

@@ -37,7 +37,7 @@
 import static org.neo4j.values.storable.Values.stringValue;
 
 @SuppressWarnings( "Duplicates" )
-public abstract class TransactionStateTestBase<G extends KernelAPIWriteTestSupport> extends KernelAPIWriteTestBase<G>
+public abstract class NodeTransactionStateTestBase<G extends KernelAPIWriteTestSupport> extends KernelAPIWriteTestBase<G>
 {
     @Test
     public void shouldSeeNodeInTransaction() throws Exception

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

@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2002-2018 "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 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.internal.kernel.api;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+@SuppressWarnings( "Duplicates" )
+public abstract class RelationshipTransactionStateTestBase<G extends KernelAPIWriteTestSupport> extends KernelAPIWriteTestBase<G>
+{
+    @Test
+    public void shouldSeeSingleRelationshipInTransaction() throws Exception
+    {
+        int label;
+        long n1, n2;
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            n1 = tx.dataWrite().nodeCreate();
+            n2 = tx.dataWrite().nodeCreate();
+
+            // setup extra relationship to challenge the implementation
+            long decoyNode = tx.dataWrite().nodeCreate();
+            label = tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" );
+            tx.dataWrite().relationshipCreate( n2, label, decoyNode );
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            long r = tx.dataWrite().relationshipCreate( n1, label, n2 );
+            try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor() )
+            {
+                tx.dataRead().singleRelationship( r, relationship );
+                assertTrue( "should find relationship", relationship.next() );
+
+                assertEquals( label, relationship.label() );
+                assertEquals( n1, relationship.sourceNodeReference() );
+                assertEquals( n2, relationship.targetNodeReference() );
+                assertEquals( r, relationship.relationshipReference() );
+
+                assertFalse( "should only find one relationship", relationship.next() );
+            }
+            tx.success();
+        }
+    }
+
+    @Test
+    public void shouldNotSeeSingleRelationshipWhichWasDeletedInTransaction() throws Exception
+    {
+        int label;
+        long n1, n2, r;
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            n1 = tx.dataWrite().nodeCreate();
+            n2 = tx.dataWrite().nodeCreate();
+            label = tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" );
+
+            long decoyNode = tx.dataWrite().nodeCreate();
+            tx.dataWrite().relationshipCreate( n2, label, decoyNode ); // to have >1 relationship in the db
+
+            r = tx.dataWrite().relationshipCreate( n1, label, n2 );
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            assertTrue( "should delete relationship", tx.dataWrite().relationshipDelete( r ) );
+            try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor() )
+            {
+                tx.dataRead().singleRelationship( r, relationship );
+                assertFalse( "should not find relationship", relationship.next() );
+            }
+            tx.success();
+        }
+    }
+}

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

@@ -0,0 +1,193 @@
+/*
+ * Copyright (c) 2002-2018 "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 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.internal.kernel.api;
+
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import java.util.List;
+
+import org.neo4j.graphdb.Node;
+import org.neo4j.graphdb.Relationship;
+import org.neo4j.graphdb.RelationshipType;
+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.neo4j.graphdb.Label.label;
+import static org.neo4j.values.storable.Values.NO_VALUE;
+import static org.neo4j.values.storable.Values.intValue;
+import static org.neo4j.values.storable.Values.stringValue;
+
+@SuppressWarnings( "Duplicates" )
+public abstract class RelationshipWriteTestBase<G extends KernelAPIWriteTestSupport> extends KernelAPIWriteTestBase<G>
+{
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
+    @Test
+    public void shouldCreateRelationship() throws Exception
+    {
+        long n1, n2;
+        try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
+        {
+            n1 = graphDb.createNode().getId();
+            n2 = graphDb.createNode().getId();
+            tx.success();
+        }
+
+        long r;
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            int label = session.token().relationshipTypeGetOrCreateForName( "R" );
+            r = tx.dataWrite().relationshipCreate( n1, label, n2 );
+            tx.success();
+        }
+
+        try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() )
+        {
+            List<Relationship> relationships = Iterables.asList( graphDb.getNodeById( n1 ).getRelationships() );
+            assertEquals( 1, relationships.size() );
+            assertEquals( relationships.get( 0 ).getId(), r );
+        }
+    }
+
+    @Test
+    public void shouldCreateRelationshipBetweenInTransactionNodes() throws Exception
+    {
+        long n1, n2, r;
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            n1 = tx.dataWrite().nodeCreate();
+            n2 = tx.dataWrite().nodeCreate();
+            int label = session.token().relationshipTypeGetOrCreateForName( "R" );
+            r = tx.dataWrite().relationshipCreate( n1, label, n2 );
+            tx.success();
+        }
+
+        try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() )
+        {
+            List<Relationship> relationships = Iterables.asList( graphDb.getNodeById( n1 ).getRelationships() );
+            assertEquals( 1, relationships.size() );
+            assertEquals( relationships.get( 0 ).getId(), r );
+        }
+    }
+
+    @Test
+    public void shouldRollbackRelationshipOnFailure() throws Exception
+    {
+        long n1, n2;
+        try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
+        {
+            n1 = graphDb.createNode().getId();
+            n2 = graphDb.createNode().getId();
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            int label = session.token().relationshipTypeGetOrCreateForName( "R" );
+            tx.dataWrite().relationshipCreate( n1, label, n2 );
+            tx.failure();
+        }
+
+        try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() )
+        {
+            assertEquals( 0, graphDb.getNodeById( n1 ).getDegree() );
+        }
+    }
+
+    @Test
+    public void shouldDeleteRelationship() throws Exception
+    {
+        long n1, r;
+        try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
+        {
+            Node node1 = graphDb.createNode();
+            Node node2 = graphDb.createNode();
+
+            n1 = node1.getId();
+            r = node1.createRelationshipTo( node2, RelationshipType.withName( "R" ) ).getId();
+
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            assertTrue( "should delete relationship", tx.dataWrite().relationshipDelete( r ) );
+            tx.success();
+        }
+
+        try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() )
+        {
+            assertEquals( 0, graphDb.getNodeById( n1 ).getDegree() );
+        }
+    }
+
+    @Test
+    public void shouldNotDeleteRelationshipThatDoesNotExist() throws Exception
+    {
+        long relationship = 0;
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            assertFalse( tx.dataWrite().relationshipDelete( relationship ) );
+            tx.failure();
+        }
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            assertFalse( tx.dataWrite().relationshipDelete( relationship ) );
+            tx.success();
+        }
+        // should not crash
+    }
+
+    @Test
+    public void shouldDeleteRelationshipAddedInTransaction() throws Exception
+    {
+        long n1, n2;
+        try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
+        {
+            n1 = graphDb.createNode().getId();
+            n2 = graphDb.createNode().getId();
+            tx.success();
+        }
+
+        try ( Transaction tx = session.beginTransaction() )
+        {
+            int label = session.token().relationshipTypeGetOrCreateForName( "R" );
+            long r = tx.dataWrite().relationshipCreate( n1, label, n2 );
+
+            assertTrue( tx.dataWrite().relationshipDelete( r ) );
+            tx.success();
+        }
+
+        try ( org.neo4j.graphdb.Transaction ignore = graphDb.beginTx() )
+        {
+            assertEquals( 0, graphDb.getNodeById( n1 ).getDegree() );
+        }
+    }
+}

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

@@ -93,7 +93,7 @@
  * into one component. Now that that work is done, this class should be refactored to increase transparency in how it
  * works.
  */
-public final class TxState implements TransactionState, RelationshipVisitor.Home
+public class TxState implements TransactionState, RelationshipVisitor.Home
 {
     private static final LabelState.Defaults LABEL_STATE = new TransactionLabelState();
     private static final NodeStateImpl.Defaults NODE_STATE = new TransactionNodeState();

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

@@ -95,23 +95,23 @@ public AllStoreHolder( StorageEngine engine,
     }
 
     @Override
-    public boolean nodeExists( long id )
+    public boolean nodeExists( long reference )
     {
         ktx.assertOpen();
 
         if ( hasTxStateWithChanges() )
         {
             TransactionState txState = txState();
-            if ( txState.nodeIsDeletedInThisTx( id ) )
+            if ( txState.nodeIsDeletedInThisTx( reference ) )
             {
                 return false;
             }
-            else if ( txState.nodeIsAddedInThisTx( id ) )
+            else if ( txState.nodeIsAddedInThisTx( reference ) )
             {
                 return true;
             }
         }
-        return storeReadLayer.nodeExists( id );
+        return storeReadLayer.nodeExists( reference );
     }
 
     @Override

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

@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2002-2018 "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 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.kernel.impl.newapi;
+
+enum HasChanges
+{
+    MAYBE,
+    YES,
+    NO
+}