Removed Transaction#success and Transaction#failure

Changed the behaviour of `AsyncTransaction#commitAsync` and `AsyncTransaction#rollbackAsync` to throw error when committing/rolling back second time.
A transaction can be allowed to commit or rollback once, but closed multiple times.
This bring the behaviour of commit and rollback the same as 4.0 core API, as well as all other language drivers.

Author: Zhen Li

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

@@ -89,7 +89,7 @@ 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()}.
+     * {@link Transaction#close()} or transaction is explicitly 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.
@@ -101,7 +101,7 @@ 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()}.
+     * {@link Transaction#close()} or transaction is explicitly 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.
@@ -114,7 +114,7 @@ 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()}.
+     * {@link Transaction#close()} or transaction is explicitly 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.
@@ -126,7 +126,7 @@ 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()}.
+     * {@link Transaction#close()} or transaction is explicitly 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,7 +130,7 @@ void bookmarkRemainsAfterTxFailure()
 
         Transaction tx = session.beginTransaction();
         tx.run( "RETURN" );
-        tx.success();
+        tx.commit();
 
         assertThrows( ClientException.class, tx::close );
         assertEquals( bookmark, session.lastBookmark() );
@@ -210,7 +210,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,7 +268,7 @@ void connectionUsedForTransactionReturnedToThePoolWhenTransactionFailsToCommitte
 
         // property existence constraints are verified on commit, try to violate it
         tx.run( "CREATE (:Book)" );
-        tx.success();
+        tx.commit();
 
         assertThrows( ClientException.class, tx::close );
 

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,13 +158,12 @@ 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();
+        tx.commit();
 
         // then expect
         ClientException e = assertThrows( ClientException.class, tx::close );