Merge pull request #622 from zhenlineo/4.0-commit-rollback

`Transaction#commit` and `Transaction#rollback`

Author: zhenlineo

driver/src/main/java/org/neo4j/driver/Session.java

@@ -88,8 +88,8 @@ public interface Session extends Resource, StatementRunner
     /**
      * Execute given unit of work in a  {@link AccessMode#READ read} transaction.
      * <p>
-     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or from
-     * {@link Transaction#close()} or transaction is explicitly marked for failure via {@link Transaction#failure()}.
+     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or during committing,
+     * or transaction is explicitly committed via {@link Transaction#commit()} or rolled back via {@link Transaction#rollback()}.
      *
      * @param work the {@link TransactionWork} to be applied to a new read transaction.
      * @param <T> the return type of the given unit of work.
@@ -100,8 +100,8 @@ public interface Session extends Resource, StatementRunner
     /**
      * Execute given unit of work in a  {@link AccessMode#READ read} transaction with the specified {@link TransactionConfig configuration}.
      * <p>
-     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or from
-     * {@link Transaction#close()} or transaction is explicitly marked for failure via {@link Transaction#failure()}.
+     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or during committing,
+     * or transaction is explicitly committed via {@link Transaction#commit()} or rolled back via {@link Transaction#rollback()}.
      *
      * @param work the {@link TransactionWork} to be applied to a new read transaction.
      * @param config configuration for all transactions started to execute the unit of work.
@@ -113,8 +113,8 @@ public interface Session extends Resource, StatementRunner
     /**
      * Execute given unit of work in a  {@link AccessMode#WRITE write} transaction.
      * <p>
-     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or from
-     * {@link Transaction#close()} or transaction is explicitly marked for failure via {@link Transaction#failure()}.
+     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or during committing,
+     * or transaction is explicitly committed via {@link Transaction#commit()} or rolled back via {@link Transaction#rollback()}.
      *
      * @param work the {@link TransactionWork} to be applied to a new write transaction.
      * @param <T> the return type of the given unit of work.
@@ -125,8 +125,8 @@ public interface Session extends Resource, StatementRunner
     /**
      * Execute given unit of work in a  {@link AccessMode#WRITE write} transaction with the specified {@link TransactionConfig configuration}.
      * <p>
-     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or from
-     * {@link Transaction#close()} or transaction is explicitly marked for failure via {@link Transaction#failure()}.
+     * Transaction will automatically be committed unless exception is thrown from the unit of work itself or during committing,
+     * or transaction is explicitly committed via {@link Transaction#commit()} or rolled back via {@link Transaction#rollback()}.
      *
      * @param work the {@link TransactionWork} to be applied to a new write transaction.
      * @param config configuration for all transactions started to execute the unit of work.

driver/src/main/java/org/neo4j/driver/Transaction.java

@@ -25,19 +25,17 @@
  * A driver Transaction object corresponds to a server transaction.
  * <p>
  * Transactions are typically wrapped in a try-with-resources block
- * which ensures that <code>COMMIT</code> or <code>ROLLBACK</code>
- * occurs correctly on close. Note that <code>ROLLBACK</code> is the
- * default action unless {@link #success()} is called before closing.
- * <pre class="docTest:TransactionDocIT#classDoc">
+ * which ensures in case of any error in try block, the transaction is
+ * automatically rolled back on close. Note that <code>ROLLBACK</code> is the
+ * default action unless {@link #commit()} is called before closing.
  * {@code
  * try(Transaction tx = session.beginTransaction())
  * {
  *     tx.run("CREATE (a:Person {name: {x}})", parameters("x", "Alice"));
- *     tx.success();
+ *     tx.commit();
  * }
  * }
- * </pre>
- * Blocking calls are: {@link #success()}, {@link #failure()}, {@link #close()}
+ * Blocking calls are: {@link #commit()}, {@link #rollback()}, {@link #close()}
  * and various overloads of {@link #run(Statement)}.
  *
  * @see Session#run
@@ -47,36 +45,60 @@
 public interface Transaction extends Resource, StatementRunner
 {
     /**
-     * Mark this transaction as successful. You must call this method before calling {@link #close()} to have your
-     * transaction committed.
+     * Commit this current transaction.
+     * When this method returns, all outstanding statements in the transaction are guaranteed to
+     * have completed, meaning any writes you performed are guaranteed to be durably stored.
+     * No more statement can be executed inside this transaction once this transaction is committed.
+     * After this method is called, the transaction cannot be committed or rolled back again.
+     * You must call this method before calling {@link #close()} to have your transaction committed.
+     * If a transaction is not committed or rolled back before close,
+     * the transaction will be rolled back by default in {@link #close}.
+     *
+     Example:
+     *
+     * {@code
+     * try(Transaction tx = session.beginTransaction() )
+     * {
+     *     tx.run("CREATE (a:Person {name: {x}})", parameters("x", "Alice"));
+     *     tx.commit();
+     * }
+     * }
      */
-    void success();
+    void commit();
 
     /**
-     * Mark this transaction as failed. When you call {@link #close()}, the transaction will value rolled back.
-     *
-     * After this method has been called, there is nothing that can be done to "un-mark" it. This is a safety feature
-     * to make sure no other code calls {@link #success()} and makes a transaction commit that was meant to be rolled
-     * back.
+     * Roll back this current transaction.
+     * No more statement can be executed inside this transaction once this transaction is committed.
+     * After this method has been called, the transaction cannot be committed or rolled back again.
+     * If a transaction is not committed or rolled back before close,
+     * the transaction will be rolled back by default in {@link #close}.
      *
      * Example:
      *
-     * <pre class="docTest:TransactionDocIT#failure">
      * {@code
      * try(Transaction tx = session.beginTransaction() )
      * {
      *     tx.run("CREATE (a:Person {name: {x}})", parameters("x", "Alice"));
-     *     tx.failure();
+     *     tx.rollback();
      * }
      * }
-     * </pre>
      */
-    void failure();
+    void rollback();
 
     /**
-     * Closing the transaction will complete it - it will commit if {@link #success()} has been called.
-     * When this method returns, all outstanding statements in the transaction are guaranteed to
-     * have completed, meaning any writes you performed are guaranteed to be durably stored.
+     * Close the transaction.
+     * If the transaction has been {@link #commit() committed} or {@link #rollback() rolled back},
+     * the close is optional and no operation is performed inside.
+     * Otherwise, the transaction will be rolled back by default by this method.
+     *
+     * Example:
+     *
+     * {@code
+     * try(Transaction tx = session.beginTransaction() )
+     * {
+     *     tx.run("CREATE (a:Person {name: {x}})", parameters("x", "Alice"));
+     * }
+     * }
      */
     @Override
     void close();

driver/src/main/java/org/neo4j/driver/internal/InternalSession.java

@@ -145,19 +145,14 @@ private <T> T transaction( AccessMode mode, TransactionWork<T> work, Transaction
         return session.retryLogic().retry( () -> {
             try ( Transaction tx = beginTransaction( mode, config ) )
             {
-                try
-                {
-                    T result = work.execute( tx );
-                    tx.success();
-                    return result;
-                }
-                catch ( Throwable t )
+
+                T result = work.execute( tx );
+                if ( tx.isOpen() )
                 {
-                    // mark transaction for failure if the given unit of work threw exception
-                    // this will override any success marks that were made by the unit of work
-                    tx.failure();
-                    throw t;
+                    // commit tx if a user has not explicitly committed or rolled back the transaction
+                    tx.commit();
                 }
+                return result;
             }
         } );
     }

driver/src/main/java/org/neo4j/driver/internal/InternalTransaction.java

@@ -34,15 +34,17 @@ public InternalTransaction( ExplicitTransaction tx )
     }
 
     @Override
-    public void success()
+    public void commit()
     {
-        tx.success();
+        Futures.blockingGet( tx.commitAsync(),
+                () -> terminateConnectionOnThreadInterrupt( "Thread interrupted while committing the transaction" ) );
     }
 
     @Override
-    public void failure()
+    public void rollback()
     {
-        tx.failure();
+        Futures.blockingGet( tx.rollbackAsync(),
+                () -> terminateConnectionOnThreadInterrupt( "Thread interrupted while rolling back the transaction" ) );
     }
 
     @Override

driver/src/main/java/org/neo4j/driver/internal/async/ExplicitTransaction.java

@@ -45,12 +45,6 @@ private enum State
         /** The transaction is running with no explicit success or failure marked */
         ACTIVE,
 
-        /** Running, user marked for success, meaning it'll value committed */
-        MARKED_SUCCESS,
-
-        /** User marked as failed, meaning it'll be rolled back. */
-        MARKED_FAILED,
-
         /**
          * This transaction has been terminated either because of explicit {@link Session#reset()} or because of a
          * fatal connection error.
@@ -94,29 +88,9 @@ public CompletionStage<ExplicitTransaction> beginAsync( InternalBookmark initial
                 } );
     }
 
-    public void success()
-    {
-        if ( state == State.ACTIVE )
-        {
-            state = State.MARKED_SUCCESS;
-        }
-    }
-
-    public void failure()
-    {
-        if ( state == State.ACTIVE || state == State.MARKED_SUCCESS )
-        {
-            state = State.MARKED_FAILED;
-        }
-    }
-
     public CompletionStage<Void> closeAsync()
     {
-        if ( state == State.MARKED_SUCCESS )
-        {
-            return commitAsync();
-        }
-        else if ( state != State.COMMITTED && state != State.ROLLED_BACK )
+        if ( isOpen() )
         {
             return rollbackAsync();
         }
@@ -130,7 +104,7 @@ public CompletionStage<Void> commitAsync()
     {
         if ( state == State.COMMITTED )
         {
-            return completedWithNull();
+            return failedFuture( new ClientException( "Can't commit, transaction has been committed" ) );
         }
         else if ( state == State.ROLLED_BACK )
         {
@@ -152,7 +126,7 @@ public CompletionStage<Void> rollbackAsync()
         }
         else if ( state == State.ROLLED_BACK )
         {
-            return completedWithNull();
+            return failedFuture( new ClientException( "Can't rollback, transaction has been rolled back" ) );
         }
         else
         {
@@ -205,11 +179,6 @@ else if ( state == State.ROLLED_BACK )
         {
             throw new ClientException( "Cannot run more statements in this transaction, it has been rolled back" );
         }
-        else if ( state == State.MARKED_FAILED )
-        {
-            throw new ClientException( "Cannot run more statements in this transaction, it has been marked for failure. " +
-                    "Please either rollback or close this transaction" );
-        }
         else if ( state == State.TERMINATED )
         {
             throw new ClientException( "Cannot run more statements in this transaction, " +

driver/src/main/java/org/neo4j/driver/internal/async/InternalAsyncSession.java

@@ -196,8 +196,7 @@ private <T> void closeTxAfterSucceededTransactionWork( ExplicitTransaction tx, C
     {
         if ( tx.isOpen() )
         {
-            tx.success();
-            tx.closeAsync().whenComplete( ( ignore, completionError ) -> {
+            tx.commitAsync().whenComplete( ( ignore, completionError ) -> {
                 Throwable commitError = Futures.completionExceptionCause( completionError );
                 if ( commitError != null )
                 {

driver/src/test/java/org/neo4j/driver/integration/BookmarkIT.java

@@ -112,7 +112,7 @@ void bookmarkRemainsAfterRolledBackTx()
         try ( Transaction tx = session.beginTransaction() )
         {
             tx.run( "CREATE (a:Person)" );
-            tx.failure();
+            tx.rollback();
         }
 
         assertEquals( bookmark, session.lastBookmark() );
@@ -130,9 +130,8 @@ void bookmarkRemainsAfterTxFailure()
 
         Transaction tx = session.beginTransaction();
         tx.run( "RETURN" );
-        tx.success();
 
-        assertThrows( ClientException.class, tx::close );
+        assertThrows( ClientException.class, tx::commit );
         assertEquals( bookmark, session.lastBookmark() );
     }
 
@@ -210,7 +209,7 @@ private static void createNodeInTx( Session session )
         try ( Transaction tx = session.beginTransaction() )
         {
             tx.run( "CREATE (a:Person)" );
-            tx.success();
+            tx.commit();
         }
     }
 }

driver/src/test/java/org/neo4j/driver/integration/ConnectionHandlingIT.java

@@ -219,7 +219,7 @@ void connectionUsedForTransactionReturnedToThePoolWhenTransactionCommitted()
         verify( connection1, never() ).release();
 
         StatementResult result = createNodes( 5, tx );
-        tx.success();
+        tx.commit();
         tx.close();
 
         Connection connection2 = connectionPool.lastAcquiredConnectionSpy;
@@ -240,7 +240,7 @@ void connectionUsedForTransactionReturnedToThePoolWhenTransactionRolledBack()
         verify( connection1, never() ).release();
 
         StatementResult result = createNodes( 8, tx );
-        tx.failure();
+        tx.rollback();
         tx.close();
 
         Connection connection2 = connectionPool.lastAcquiredConnectionSpy;
@@ -268,9 +268,8 @@ void connectionUsedForTransactionReturnedToThePoolWhenTransactionFailsToCommitte
 
         // property existence constraints are verified on commit, try to violate it
         tx.run( "CREATE (:Book)" );
-        tx.success();
 
-        assertThrows( ClientException.class, tx::close );
+        assertThrows( ClientException.class, tx::commit );
 
         // connection should have been released after failed node creation
         verify( connection2 ).release();

driver/src/test/java/org/neo4j/driver/integration/ConnectionPoolIT.java

@@ -177,8 +177,7 @@ private static void startAndCloseTransactions( Driver driver, int txCount )
             }
             for ( Transaction tx : transactions )
             {
-                tx.success();
-                tx.close();
+                tx.commit();
             }
             for ( Session session : sessions )
             {

driver/src/test/java/org/neo4j/driver/integration/ErrorIT.java

@@ -158,16 +158,14 @@ void shouldHandleFailureAtCommitTime()
         // given
         Transaction tx = session.beginTransaction();
         tx.run( "CREATE CONSTRAINT ON (a:`" + label + "`) ASSERT a.name IS UNIQUE" );
-        tx.success();
-        tx.close();
+        tx.commit();
 
         // and
         tx = session.beginTransaction();
         tx.run( "CREATE INDEX ON :`" + label + "`(name)" );
-        tx.success();
 
         // then expect
-        ClientException e = assertThrows( ClientException.class, tx::close );
+        ClientException e = assertThrows( ClientException.class, tx::commit );
         assertThat( e.getMessage(), containsString( label ) );
         assertThat( e.getMessage(), containsString( "name" ) );
     }