From 83d57d7fcd94baa5c4e0e2d852c6eef2dbf0f9ff Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Thu, 22 Aug 2019 17:02:18 +0200 Subject: [PATCH] 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. --- .../main/java/org/neo4j/driver/Session.java | 16 +- .../java/org/neo4j/driver/Transaction.java | 66 +++-- .../driver/internal/InternalSession.java | 17 +- .../driver/internal/InternalTransaction.java | 10 +- .../internal/async/ExplicitTransaction.java | 37 +-- .../internal/async/InternalAsyncSession.java | 3 +- .../neo4j/driver/integration/BookmarkIT.java | 7 +- .../integration/ConnectionHandlingIT.java | 7 +- .../driver/integration/ConnectionPoolIT.java | 3 +- .../org/neo4j/driver/integration/ErrorIT.java | 6 +- .../integration/ExplicitTransactionIT.java | 26 -- .../driver/integration/NestedQueries.java | 6 +- .../driver/integration/ResultStreamIT.java | 2 +- .../integration/RoutingDriverBoltKitTest.java | 23 +- .../driver/integration/SessionBoltV3IT.java | 7 +- .../neo4j/driver/integration/SessionIT.java | 105 +++++--- .../driver/integration/SessionResetIT.java | 24 +- .../integration/TransactionBoltV3IT.java | 2 +- .../driver/integration/TransactionIT.java | 242 ++++++++++++------ .../integration/async/AsyncTransactionIT.java | 19 +- .../integration/reactive/RxTransactionIT.java | 92 ++----- .../internal/DirectDriverBoltKitTest.java | 98 +++---- .../internal/InternalTransactionTest.java | 22 +- .../async/ExplicitTransactionTest.java | 79 ------ .../internal/async/NetworkSessionTest.java | 9 +- .../driver/internal/util/BookmarkUtils.java | 11 + .../driver/stress/BlockingReadQueryInTx.java | 2 +- .../driver/stress/BlockingWriteQueryInTx.java | 2 +- ...lockingWriteQueryUsingReadSessionInTx.java | 2 +- .../driver/stress/CausalClusteringIT.java | 25 +- .../org/neo4j/driver/util/Neo4jSettings.java | 2 - .../neo4j/docs/driver/CypherErrorExample.java | 1 + 32 files changed, 457 insertions(+), 516 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/Session.java b/driver/src/main/java/org/neo4j/driver/Session.java index 53193aed26..176b8f5ea0 100644 --- a/driver/src/main/java/org/neo4j/driver/Session.java +++ b/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. *

- * 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 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}. *

- * 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. *

- * 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 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}. *

- * 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. diff --git a/driver/src/main/java/org/neo4j/driver/Transaction.java b/driver/src/main/java/org/neo4j/driver/Transaction.java index c2b279ea5c..65301c637c 100644 --- a/driver/src/main/java/org/neo4j/driver/Transaction.java +++ b/driver/src/main/java/org/neo4j/driver/Transaction.java @@ -25,19 +25,17 @@ * A driver Transaction object corresponds to a server transaction. *

* Transactions are typically wrapped in a try-with-resources block - * which ensures that COMMIT or ROLLBACK - * occurs correctly on close. Note that ROLLBACK is the - * default action unless {@link #success()} is called before closing. - * 

+ * which ensures in case of any error in try block, the transaction is
+ * automatically rolled back on close. Note that ROLLBACK 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();
  * }
  * }
- *
- * 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: * - * 
      * {@code
      * try(Transaction tx = session.beginTransaction() )
      * {
      *     tx.run("CREATE (a:Person {name: {x}})", parameters("x", "Alice"));
-     *     tx.failure();
+     *     tx.rollback();
      * }
      * }
-     *
*/ - 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(); diff --git a/driver/src/main/java/org/neo4j/driver/internal/InternalSession.java b/driver/src/main/java/org/neo4j/driver/internal/InternalSession.java index e92766f1d8..1a95dc85c4 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/InternalSession.java +++ b/driver/src/main/java/org/neo4j/driver/internal/InternalSession.java @@ -145,19 +145,14 @@ private T transaction( AccessMode mode, TransactionWork 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; } } ); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/InternalTransaction.java b/driver/src/main/java/org/neo4j/driver/internal/InternalTransaction.java index 60d26b6947..6cb12bda67 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/InternalTransaction.java +++ b/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 diff --git a/driver/src/main/java/org/neo4j/driver/internal/async/ExplicitTransaction.java b/driver/src/main/java/org/neo4j/driver/internal/async/ExplicitTransaction.java index 5c4c272810..e8d613fa81 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/async/ExplicitTransaction.java +++ b/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 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 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 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 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, " + diff --git a/driver/src/main/java/org/neo4j/driver/internal/async/InternalAsyncSession.java b/driver/src/main/java/org/neo4j/driver/internal/async/InternalAsyncSession.java index 220070f4ff..fb82cbe351 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/async/InternalAsyncSession.java +++ b/driver/src/main/java/org/neo4j/driver/internal/async/InternalAsyncSession.java @@ -196,8 +196,7 @@ private 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 ) { diff --git a/driver/src/test/java/org/neo4j/driver/integration/BookmarkIT.java b/driver/src/test/java/org/neo4j/driver/integration/BookmarkIT.java index 35d612c6d0..4f0472c152 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/BookmarkIT.java +++ b/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(); } } } diff --git a/driver/src/test/java/org/neo4j/driver/integration/ConnectionHandlingIT.java b/driver/src/test/java/org/neo4j/driver/integration/ConnectionHandlingIT.java index 3b2b91ebcd..ade0f13158 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/ConnectionHandlingIT.java +++ b/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(); diff --git a/driver/src/test/java/org/neo4j/driver/integration/ConnectionPoolIT.java b/driver/src/test/java/org/neo4j/driver/integration/ConnectionPoolIT.java index 589e32a00a..5bfae403b8 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/ConnectionPoolIT.java +++ b/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 ) { diff --git a/driver/src/test/java/org/neo4j/driver/integration/ErrorIT.java b/driver/src/test/java/org/neo4j/driver/integration/ErrorIT.java index cd3f195ab9..a47f6e9d47 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/ErrorIT.java +++ b/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" ) ); } diff --git a/driver/src/test/java/org/neo4j/driver/integration/ExplicitTransactionIT.java b/driver/src/test/java/org/neo4j/driver/integration/ExplicitTransactionIT.java index 56c7255ae1..6c70ce0c21 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/ExplicitTransactionIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/ExplicitTransactionIT.java @@ -167,17 +167,6 @@ void shouldRollbackAfterTermination() assertFalse( tx.isOpen() ); } - @Test - void shouldFailToRunQueryWhenMarkedForFailure() - { - ExplicitTransaction tx = beginTransaction(); - txRun( tx, "CREATE (:MyLabel)" ); - tx.failure(); - - ClientException e = assertThrows( ClientException.class, () -> txRun( tx, "CREATE (:MyOtherLabel)" ) ); - assertThat( e.getMessage(), startsWith( "Cannot run more statements in this transaction" ) ); - } - @Test void shouldFailToRunQueryWhenTerminated() { @@ -189,21 +178,6 @@ void shouldFailToRunQueryWhenTerminated() assertThat( e.getMessage(), startsWith( "Cannot run more statements in this transaction" ) ); } - @Test - void shouldAllowQueriesWhenMarkedForSuccess() - { - ExplicitTransaction tx = beginTransaction(); - txRun( tx, "CREATE (:MyLabel)" ); - - tx.success(); - - txRun( tx, "CREATE (:MyLabel)" ); - assertNull( await( tx.commitAsync() ) ); - - StatementResultCursor cursor = sessionRun( session, new Statement( "MATCH (n:MyLabel) RETURN count(n)" ) ); - assertEquals( 2, await( cursor.singleAsync() ).get( 0 ).asInt() ); - } - @Test void shouldBePossibleToRunMoreTransactionsAfterOneIsTerminated() { diff --git a/driver/src/test/java/org/neo4j/driver/integration/NestedQueries.java b/driver/src/test/java/org/neo4j/driver/integration/NestedQueries.java index 2ea1840783..0cc5f86d11 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/NestedQueries.java +++ b/driver/src/test/java/org/neo4j/driver/integration/NestedQueries.java @@ -46,7 +46,7 @@ default void shouldAllowNestedQueriesInTransactionConsumedAsIterators() throws E try ( Session session = newSession( AccessMode.READ ); Transaction tx = session.beginTransaction() ) { testNestedQueriesConsumedAsIterators( tx ); - tx.success(); + tx.commit(); } } @@ -56,7 +56,7 @@ default void shouldAllowNestedQueriesInTransactionConsumedAsLists() throws Excep try ( Session session = newSession( AccessMode.READ ); Transaction tx = session.beginTransaction() ) { testNestedQueriesConsumedAsLists( tx ); - tx.success(); + tx.commit(); } } @@ -66,7 +66,7 @@ default void shouldAllowNestedQueriesInTransactionConsumedAsIteratorAndList() th try ( Session session = newSession( AccessMode.READ ); Transaction tx = session.beginTransaction() ) { testNestedQueriesConsumedAsIteratorAndList( tx ); - tx.success(); + tx.commit(); } } diff --git a/driver/src/test/java/org/neo4j/driver/integration/ResultStreamIT.java b/driver/src/test/java/org/neo4j/driver/integration/ResultStreamIT.java index 6e35003b16..1c87176bef 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/ResultStreamIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/ResultStreamIT.java @@ -174,7 +174,7 @@ void shouldBeAbleToAccessSummaryAfterTransactionFailure() { StatementResult result = tx.run( "UNWIND [1,2,0] AS x RETURN 10/x" ); resultRef.set( result ); - tx.success(); + tx.commit(); } } ); diff --git a/driver/src/test/java/org/neo4j/driver/integration/RoutingDriverBoltKitTest.java b/driver/src/test/java/org/neo4j/driver/integration/RoutingDriverBoltKitTest.java index e296fcc123..48e447411b 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/RoutingDriverBoltKitTest.java +++ b/driver/src/test/java/org/neo4j/driver/integration/RoutingDriverBoltKitTest.java @@ -140,7 +140,7 @@ void shouldHandleAcquireReadSessionAndTransaction() throws IOException, Interrup List result = tx.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( "n.name" ).asString() ); assertThat( result, equalTo( asList( "Bob", "Alice", "Tina" ) ) ); - tx.success(); + tx.commit(); } // Finally assertThat( server.exitStatus(), equalTo( 0 ) ); @@ -194,7 +194,7 @@ void shouldRoundRobinReadServersWhenUsingTransaction() throws IOException, Inter { assertThat( tx.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( "n.name" ).asString() ), equalTo( asList( "Bob", "Alice", "Tina" ) ) ); - tx.success(); + tx.commit(); } } } @@ -243,7 +243,7 @@ void shouldThrowSessionExpiredIfReadServerDisappearsWhenUsingTransaction() throw Transaction tx = session.beginTransaction() ) { tx.run( "MATCH (n) RETURN n.name" ); - tx.success(); + tx.commit(); } } ); assertEquals( "Server at 127.0.0.1:9005 is no longer available", e.getMessage() ); @@ -290,7 +290,6 @@ void shouldThrowSessionExpiredIfWriteServerDisappearsWhenUsingTransaction() thro Transaction tx = session.beginTransaction() ) { assertThrows( SessionExpiredException.class, () -> tx.run( "MATCH (n) RETURN n.name" ).consume() ); - tx.success(); } finally { @@ -350,7 +349,7 @@ void shouldHandleAcquireWriteSessionAndTransaction() throws IOException, Interru Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } // Finally assertThat( server.exitStatus(), equalTo( 0 ) ); @@ -400,7 +399,7 @@ void shouldRoundRobinWriteSessionsInTransaction() throws Exception try ( Session session = driver.session(); Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } } } @@ -527,7 +526,7 @@ void shouldSendInitialBookmark() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } assertEquals( parse( "NewBookmark" ), session.lastBookmark() ); @@ -549,7 +548,7 @@ void shouldUseWriteSessionModeAndInitialBookmark() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } assertEquals( parse( "NewBookmark" ), session.lastBookmark() ); @@ -574,7 +573,7 @@ void shouldUseReadSessionModeAndInitialBookmark() throws Exception assertEquals( 2, records.size() ); assertEquals( "Bob", records.get( 0 ).get( "name" ).asString() ); assertEquals( "Alice", records.get( 1 ).get( "name" ).asString() ); - tx.success(); + tx.commit(); } assertEquals( parse( "NewBookmark" ), session.lastBookmark() ); @@ -596,7 +595,7 @@ void shouldPassBookmarkFromTransactionToTransaction() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } assertEquals( parse( "BookmarkB" ), session.lastBookmark() ); @@ -606,7 +605,7 @@ void shouldPassBookmarkFromTransactionToTransaction() throws Exception List records = tx.run( "MATCH (n) RETURN n.name AS name" ).list(); assertEquals( 1, records.size() ); assertEquals( "Bob", records.get( 0 ).get( "name" ).asString() ); - tx.success(); + tx.commit(); } assertEquals( parse( "BookmarkC" ), session.lastBookmark() ); @@ -991,7 +990,7 @@ void shouldSendMultipleBookmarks() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } assertEquals( parse( "neo4j:bookmark:v1:tx95" ), session.lastBookmark() ); diff --git a/driver/src/test/java/org/neo4j/driver/integration/SessionBoltV3IT.java b/driver/src/test/java/org/neo4j/driver/integration/SessionBoltV3IT.java index ca3018afd9..475f53f3a4 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/SessionBoltV3IT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/SessionBoltV3IT.java @@ -224,7 +224,7 @@ void shouldUseBookmarksForAutoCommitAndExplicitTransactions() try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE ()" ); - tx.success(); + tx.commit(); } Bookmark bookmark1 = session.lastBookmark(); assertNotNull( bookmark1 ); @@ -239,7 +239,7 @@ void shouldUseBookmarksForAutoCommitAndExplicitTransactions() try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE ()" ); - tx.success(); + tx.commit(); } Bookmark bookmark3 = session.lastBookmark(); assertNotNull( bookmark3 ); @@ -297,8 +297,7 @@ void shouldSendGoodbyeWhenClosingDriver() Transaction tx = txs.get( i ); tx.run( "CREATE ()" ); - tx.success(); - tx.close(); + tx.commit(); session.close(); } } diff --git a/driver/src/test/java/org/neo4j/driver/integration/SessionIT.java b/driver/src/test/java/org/neo4j/driver/integration/SessionIT.java index 27047c1ead..a3960aa2a8 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/SessionIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/SessionIT.java @@ -70,6 +70,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.is; @@ -92,6 +93,7 @@ import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.internal.util.BookmarkUtils.assertBookmarkContainsSingleValue; import static org.neo4j.driver.internal.util.BookmarkUtils.assertBookmarkIsEmpty; +import static org.neo4j.driver.internal.util.BookmarkUtils.assertBookmarkIsNotEmpty; import static org.neo4j.driver.internal.util.Matchers.arithmeticError; import static org.neo4j.driver.internal.util.Matchers.connectionAcquisitionTimeoutError; import static org.neo4j.driver.internal.util.Neo4jFeature.BOLT_V4; @@ -370,7 +372,7 @@ void readTxRolledBackWithTxFailure() long answer = session.readTransaction( tx -> { StatementResult result = tx.run( "RETURN 42" ); - tx.failure(); + tx.rollback(); return result.single().get( 0 ).asLong(); } ); assertEquals( 42, answer ); @@ -390,7 +392,7 @@ void writeTxRolledBackWithTxFailure() int answer = session.writeTransaction( tx -> { tx.run( "CREATE (:Person {name: 'Natasha Romanoff'})" ); - tx.failure(); + tx.rollback(); return 42; } ); @@ -458,53 +460,85 @@ void readTxRolledBackWhenMarkedBothSuccessAndFailure() try ( Driver driver = newDriverWithoutRetries(); Session session = driver.session() ) { - assertBookmarkIsEmpty( session.lastBookmark() ); - - long answer = session.readTransaction( tx -> - { + ClientException error = assertThrows( ClientException.class, () -> session.readTransaction( tx -> { StatementResult result = tx.run( "RETURN 42" ); - tx.success(); - tx.failure(); + tx.commit(); + tx.rollback(); return result.single().get( 0 ).asLong(); - } ); - assertEquals( 42, answer ); - - // bookmark should remain null after rollback - assertBookmarkIsEmpty( session.lastBookmark() ); + } ) ); + assertThat( error.getMessage(), startsWith( "Can't rollback, transaction has been committed" ) ); } } @Test - void writeTxRolledBackWhenMarkedBothSuccessAndFailure() + void writeTxFailWhenBothCommitAndRollback() { try ( Driver driver = newDriverWithoutRetries() ) { try ( Session session = driver.session() ) { - int answer = session.writeTransaction( tx -> - { + ClientException error = assertThrows( ClientException.class, () -> session.writeTransaction( tx -> { tx.run( "CREATE (:Person {name: 'Natasha Romanoff'})" ); - tx.success(); - tx.failure(); + tx.commit(); + tx.rollback(); return 42; - } ); + } ) ); - assertEquals( 42, answer ); + assertThat( error.getMessage(), startsWith( "Can't rollback, transaction has been committed" ) ); + } + } + } + + @Test + void readTxCommittedWhenCommitAndThrowsException() + { + try ( Driver driver = newDriverWithoutRetries(); + Session session = driver.session() ) + { + assertBookmarkIsEmpty( session.lastBookmark() ); + + assertThrows( IllegalStateException.class, () -> + session.readTransaction( tx -> + { + tx.run( "RETURN 42" ); + tx.commit(); + throw new IllegalStateException(); + } ) ); + + // We successfully committed + assertBookmarkIsNotEmpty( session.lastBookmark() ); + } + } + + @Test + void writeTxCommittedWhenCommitAndThrowsException() + { + try ( Driver driver = newDriverWithoutRetries() ) + { + try ( Session session = driver.session() ) + { + assertThrows( IllegalStateException.class, () -> + session.writeTransaction( tx -> + { + tx.run( "CREATE (:Person {name: 'Natasha Romanoff'})" ); + tx.commit(); + throw new IllegalStateException(); + } ) ); } try ( Session session = driver.session() ) { StatementResult result = session.run( "MATCH (p:Person {name: 'Natasha Romanoff'}) RETURN count(p)" ); - assertEquals( 0, result.single().get( 0 ).asInt() ); + assertEquals( 1, result.single().get( 0 ).asInt() ); } } } @Test - void readTxRolledBackWhenMarkedAsSuccessAndThrowsException() + void readRolledBackWhenRollbackAndThrowsException() { try ( Driver driver = newDriverWithoutRetries(); - Session session = driver.session() ) + Session session = driver.session() ) { assertBookmarkIsEmpty( session.lastBookmark() ); @@ -512,7 +546,7 @@ void readTxRolledBackWhenMarkedAsSuccessAndThrowsException() session.readTransaction( tx -> { tx.run( "RETURN 42" ); - tx.success(); + tx.rollback(); throw new IllegalStateException(); } ) ); @@ -522,7 +556,7 @@ void readTxRolledBackWhenMarkedAsSuccessAndThrowsException() } @Test - void writeTxRolledBackWhenMarkedAsSuccessAndThrowsException() + void writeTxRolledBackWhenRollbackAndThrowsException() { try ( Driver driver = newDriverWithoutRetries() ) { @@ -532,7 +566,7 @@ void writeTxRolledBackWhenMarkedAsSuccessAndThrowsException() session.writeTransaction( tx -> { tx.run( "CREATE (:Person {name: 'Natasha Romanoff'})" ); - tx.success(); + tx.rollback(); throw new IllegalStateException(); } ) ); } @@ -573,7 +607,7 @@ void transactionRunShouldFailOnDeadlocks() throws Exception // lock second node updateNodeId( tx, nodeId2, newNodeId1 ).consume(); - tx.success(); + tx.commit(); } return null; } ); @@ -592,7 +626,7 @@ void transactionRunShouldFailOnDeadlocks() throws Exception // lock first node updateNodeId( tx, nodeId1, newNodeId2 ).consume(); - tx.success(); + tx.commit(); } return null; } ); @@ -639,7 +673,7 @@ void writeTransactionFunctionShouldRetryDeadlocks() throws Exception // lock second node updateNodeId( tx, nodeId2, newNodeId1 ).consume(); - tx.success(); + tx.commit(); } return null; } ); @@ -1033,8 +1067,7 @@ void shouldAllowLongRunningQueryWithConnectTimeout() throws Exception // verify that query is still executing and has not failed because of the read timeout assertFalse( updateFuture.isDone() ); - tx.success(); - tx.close(); + tx.commit(); long hulkPower = updateFuture.get( 10, TimeUnit.SECONDS ); assertEquals( 100, hulkPower ); @@ -1151,7 +1184,7 @@ void shouldCloseOpenTransactionWhenClosed() tx.run( "CREATE (:Node {id: 123})" ); tx.run( "CREATE (:Node {id: 456})" ); - tx.success(); + tx.commit(); } assertEquals( 1, countNodesWithId( 123 ) ); @@ -1167,7 +1200,7 @@ void shouldRollbackOpenTransactionWhenClosed() tx.run( "CREATE (:Node {id: 123})" ); tx.run( "CREATE (:Node {id: 456})" ); - tx.failure(); + tx.rollback(); } assertEquals( 0, countNodesWithId( 123 ) ); @@ -1369,7 +1402,7 @@ private void testExecuteWriteTx( AccessMode sessionMode ) String material = session.writeTransaction( tx -> { StatementResult result = tx.run( "CREATE (s:Shield {material: 'Vibranium'}) RETURN s" ); - tx.success(); + tx.commit(); Record record = result.single(); return record.get( 0 ).asNode().get( "material" ).asString(); } ); @@ -1397,7 +1430,7 @@ private void testTxRollbackWhenFunctionThrows( AccessMode sessionMode ) tx.run( "CREATE (:Person {name: 'Thanos'})" ); // trigger division by zero error: tx.run( "UNWIND range(0, 1) AS i RETURN 10/i" ); - tx.success(); + tx.commit(); return null; } ) ); } @@ -1541,7 +1574,7 @@ public Record execute( Transaction tx ) { throw new ServiceUnavailableException( "" ); } - tx.success(); + tx.commit(); return result.single(); } } diff --git a/driver/src/test/java/org/neo4j/driver/integration/SessionResetIT.java b/driver/src/test/java/org/neo4j/driver/integration/SessionResetIT.java index 0ee3dd699e..084ad9192c 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/SessionResetIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/SessionResetIT.java @@ -49,7 +49,6 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; -import org.neo4j.driver.internal.util.EnabledOnNeo4jWith; import org.neo4j.driver.Driver; import org.neo4j.driver.Session; import org.neo4j.driver.StatementResult; @@ -235,7 +234,7 @@ void shouldBeAbleToBeginTxAfterResetFailureIsConsumed() throws Throwable try ( Transaction tx2 = session.beginTransaction() ) { tx2.run( "CREATE (n:FirstNode)" ); - tx2.success(); + tx2.commit(); } StatementResult result = session.run( "MATCH (n) RETURN count(n)" ); @@ -358,7 +357,7 @@ void shouldAllowMoreTxAfterSessionReset() try ( Transaction tx = session.beginTransaction() ) { tx.run( "RETURN 1" ); - tx.success(); + tx.commit(); } // When reset the state of this session @@ -368,7 +367,7 @@ void shouldAllowMoreTxAfterSessionReset() try ( Transaction tx = session.beginTransaction() ) { tx.run( "RETURN 2" ); - tx.success(); + tx.commit(); } } } @@ -387,8 +386,7 @@ void shouldMarkTxAsFailedAndDisallowRunAfterSessionReset() Exception e = assertThrows( Exception.class, () -> { tx.run( "RETURN 1" ); - tx.success(); - tx.close(); + tx.commit(); } ); assertThat( e.getMessage(), startsWith( "Cannot run more statements in this transaction" ) ); } @@ -410,7 +408,7 @@ void shouldAllowMoreTxAfterSessionResetInTx() try ( Transaction tx = session.beginTransaction() ) { tx.run( "RETURN 2" ); - tx.success(); + tx.commit(); } } } @@ -497,8 +495,7 @@ void shouldBeAbleToRunMoreStatementsAfterResetOnNoErrorState() // When Transaction tx = session.beginTransaction(); tx.run( "CREATE (n:FirstNode)" ); - tx.success(); - tx.close(); + tx.commit(); // Then the outcome of both statements should be visible StatementResult result = session.run( "MATCH (n) RETURN count(n)" ); @@ -538,8 +535,7 @@ void shouldHandleResetFromMultipleThreads() throws Throwable // session has been reset, it should not be possible to commit the transaction try { - tx1.success(); - tx1.close(); + tx1.commit(); } catch ( Neo4jException ignore ) { @@ -548,7 +544,7 @@ void shouldHandleResetFromMultipleThreads() throws Throwable try ( Transaction tx2 = session.beginTransaction() ) { tx2.run( "CREATE (n:SecondNode)" ); - tx2.success(); + tx2.commit(); } return null; @@ -590,7 +586,6 @@ private void testResetOfQueryWaitingForLock( NodeIdUpdater nodeIdUpdater ) throw StatementResult result = updateNodeId( tx, nodeId, newNodeId2 ); result.consume(); - tx.success(); nodeLocked.countDown(); // give separate thread some time to block on a lock @@ -598,6 +593,7 @@ private void testResetOfQueryWaitingForLock( NodeIdUpdater nodeIdUpdater ) throw otherSessionRef.get().reset(); assertTransactionTerminated( txResult ); + tx.commit(); } try ( Session session = neo4j.driver().session() ) @@ -732,7 +728,7 @@ private static void runQuery( Session session, String query, boolean autoCommit try ( Transaction tx = session.beginTransaction() ) { tx.run( query ); - tx.success(); + tx.commit(); } } } diff --git a/driver/src/test/java/org/neo4j/driver/integration/TransactionBoltV3IT.java b/driver/src/test/java/org/neo4j/driver/integration/TransactionBoltV3IT.java index 61949664df..2392755c68 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/TransactionBoltV3IT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/TransactionBoltV3IT.java @@ -125,7 +125,7 @@ void shouldSetTransactionTimeout() try ( Transaction tx = session.beginTransaction( config ) ) { tx.run( "MATCH (n:Node) SET n.prop = 2" ); - tx.success(); + tx.commit(); } } ); diff --git a/driver/src/test/java/org/neo4j/driver/integration/TransactionIT.java b/driver/src/test/java/org/neo4j/driver/integration/TransactionIT.java index 6f65871109..b94c427ee0 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/TransactionIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/TransactionIT.java @@ -24,10 +24,8 @@ import java.util.List; import java.util.Map; +import java.util.function.Consumer; -import org.neo4j.driver.internal.cluster.RoutingSettings; -import org.neo4j.driver.internal.util.io.ChannelTrackingDriverFactory; -import org.neo4j.driver.internal.util.Clock; import org.neo4j.driver.Config; import org.neo4j.driver.Driver; import org.neo4j.driver.Record; @@ -37,6 +35,9 @@ import org.neo4j.driver.Value; import org.neo4j.driver.exceptions.ClientException; import org.neo4j.driver.exceptions.ServiceUnavailableException; +import org.neo4j.driver.internal.cluster.RoutingSettings; +import org.neo4j.driver.internal.util.Clock; +import org.neo4j.driver.internal.util.io.ChannelTrackingDriverFactory; import org.neo4j.driver.util.ParallelizableIT; import org.neo4j.driver.util.SessionExtension; import org.neo4j.driver.util.TestUtil; @@ -44,9 +45,9 @@ import static java.util.Arrays.asList; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.junit.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -63,20 +64,21 @@ class TransactionIT static final SessionExtension session = new SessionExtension(); @Test - void shouldRunAndCommit() + void shouldAllowRunRollbackAndClose() { - // When - try ( Transaction tx = session.beginTransaction() ) - { - tx.run( "CREATE (n:FirstNode)" ); - tx.run( "CREATE (n:SecondNode)" ); - tx.success(); - } + shouldRunAndCloseAfterAction( Transaction::rollback, false ); + } - // Then the outcome of both statements should be visible - StatementResult result = session.run( "MATCH (n) RETURN count(n)" ); - long nodes = result.single().get( "count(n)" ).asLong(); - assertThat( nodes, equalTo( 2L ) ); + @Test + void shouldAllowRunCommitAndClose() + { + shouldRunAndCloseAfterAction( Transaction::commit, true ); + } + + @Test + void shouldAllowRunCloseAndClose() + { + shouldRunAndCloseAfterAction( Transaction::close, false ); } @Test @@ -120,26 +122,84 @@ void shouldNotAllowSessionLevelStatementsWhenThereIsATransaction() } @Test - void shouldBeClosedAfterRollback() + void shouldFailToRunQueryAfterTxIsCommitted() + { + shouldFailToRunQueryAfterTxAction( Transaction::commit ); + } + + @Test + void shouldFailToRunQueryAfterTxIsRolledBack() + { + shouldFailToRunQueryAfterTxAction( Transaction::rollback ); + } + + @Test + void shouldFailToRunQueryAfterTxIsClosed() + { + shouldFailToRunQueryAfterTxAction( Transaction::close ); + } + + @Test + void shouldFailToCommitAfterRolledBack() { - // When Transaction tx = session.beginTransaction(); - tx.close(); + tx.run( "CREATE (:MyLabel)" ); + tx.rollback(); - // Then - assertFalse( tx.isOpen() ); + ClientException e = assertThrows( ClientException.class, tx::commit ); + assertThat( e.getMessage(), startsWith( "Can't commit, transaction has been rolled back" ) ); } @Test - void shouldBeClosedAfterCommit() + void shouldFailToRollbackAfterTxIsCommitted() { - // When Transaction tx = session.beginTransaction(); - tx.success(); - tx.close(); + tx.run( "CREATE (:MyLabel)" ); + tx.commit(); - // Then - assertFalse( tx.isOpen() ); + ClientException e = assertThrows( ClientException.class, tx::rollback ); + assertThat( e.getMessage(), startsWith( "Can't rollback, transaction has been committed" ) ); + } + + @Test + void shouldFailToCommitAfterCommit() throws Throwable + { + Transaction tx = session.beginTransaction(); + tx.run( "CREATE (:MyLabel)" ); + tx.commit(); + + ClientException e = assertThrows( ClientException.class, tx::commit ); + assertThat( e.getMessage(), startsWith( "Can't commit, transaction has been committed" ) ); + } + + @Test + void shouldFailToRollbackAfterRollback() throws Throwable + { + Transaction tx = session.beginTransaction(); + tx.run( "CREATE (:MyLabel)" ); + tx.rollback(); + + ClientException e = assertThrows( ClientException.class, tx::rollback ); + assertThat( e.getMessage(), startsWith( "Can't rollback, transaction has been rolled back" ) ); + } + + + @Test + void shouldBeClosedAfterClose() + { + shouldBeClosedAfterAction( Transaction::close ); + } + + @Test + void shouldBeClosedAfterRollback() + { + shouldBeClosedAfterAction( Transaction::rollback ); + } + + @Test + void shouldBeClosedAfterCommit() + { + shouldBeClosedAfterAction( Transaction::commit ); } @Test @@ -171,7 +231,7 @@ void shouldHandleFailureAfterClosingTransaction() Transaction tx = session.beginTransaction(); StatementResult result = tx.run( "CREATE (n) RETURN n" ); result.consume(); - tx.success(); + tx.commit(); tx.close(); // WHEN when running a malformed query in the original session @@ -187,7 +247,7 @@ void shouldHandleNullRecordParameters() { Record params = null; tx.run( "CREATE (n:FirstNode)", params ); - tx.success(); + tx.commit(); } // Then it wasn't the end of the world as we know it @@ -202,7 +262,7 @@ void shouldHandleNullValueParameters() { Value params = null; tx.run( "CREATE (n:FirstNode)", params ); - tx.success(); + tx.commit(); } // Then it wasn't the end of the world as we know it @@ -217,7 +277,7 @@ void shouldHandleNullMapParameters() { Map params = null; tx.run( "CREATE (n:FirstNode)", params ); - tx.success(); + tx.commit(); } // Then it wasn't the end of the world as we know it @@ -229,9 +289,8 @@ void shouldRollBackTxIfErrorWithoutConsume() // Given Transaction tx = session.beginTransaction(); tx.run( "invalid" ); // send run, pull_all - tx.success(); - assertThrows( ClientException.class, tx::close ); + assertThrows( ClientException.class, tx::commit ); try ( Transaction anotherTx = session.beginTransaction() ) { @@ -249,7 +308,6 @@ void shouldRollBackTxIfErrorWithConsume() try ( Transaction tx = session.beginTransaction() ) { StatementResult result = tx.run( "invalid" ); - tx.success(); result.consume(); } } ); @@ -320,14 +378,13 @@ void shouldBeResponsiveToThreadInterruptWhenWaitingForCommit() // now 'Beta Ray Bill' node is locked tx2.run( "MATCH (n:Person {name: 'Beta Ray Bill'}) SET n.hammer = 'Stormbreaker'" ); - tx2.success(); // setup other thread to interrupt current thread when it blocks TestUtil.interruptWhenInWaitingState( Thread.currentThread() ); try { - assertThrows( ServiceUnavailableException.class, tx2::close ); + assertThrows( ServiceUnavailableException.class, tx2::commit ); } finally { @@ -340,13 +397,48 @@ void shouldBeResponsiveToThreadInterruptWhenWaitingForCommit() @Test void shouldThrowWhenConnectionKilledDuringTransaction() { - testFailWhenConnectionKilledDuringTransaction( false ); + ChannelTrackingDriverFactory factory = new ChannelTrackingDriverFactory( 1, Clock.SYSTEM ); + Config config = Config.builder().withLogging( DEV_NULL_LOGGING ).build(); + + try ( Driver driver = factory.newInstance( session.uri(), session.authToken(), RoutingSettings.DEFAULT, DEFAULT, config ) ) + { + ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class, () -> + { + try ( Session session1 = driver.session(); + Transaction tx = session1.beginTransaction() ) + { + tx.run( "CREATE (:MyNode {id: 1})" ).consume(); + + // kill all network channels + for ( Channel channel: factory.channels() ) + { + channel.close().syncUninterruptibly(); + } + + tx.run( "CREATE (:MyNode {id: 1})" ).consume(); + } + } ); + + assertThat( e.getMessage(), containsString( "Connection to the database terminated" ) ); + } + + assertEquals( 0, session.run( "MATCH (n:MyNode {id: 1}) RETURN count(n)" ).single().get( 0 ).asInt() ); } @Test - void shouldThrowWhenConnectionKilledDuringTransactionMarkedForSuccess() + void shouldFailToCommitAfterFailure() throws Throwable { - testFailWhenConnectionKilledDuringTransaction( true ); + try ( Transaction tx = session.beginTransaction() ) + { + List xs = tx.run( "UNWIND [1,2,3] AS x CREATE (:Node) RETURN x" ).list( record -> record.get( 0 ).asInt() ); + assertEquals( asList( 1, 2, 3 ), xs ); + + ClientException error1 = assertThrows( ClientException.class, () -> tx.run( "RETURN unknown" ).consume() ); + assertThat( error1.code(), containsString( "SyntaxError" ) ); + + ClientException error2 = assertThrows( ClientException.class, tx::commit ); + assertThat( error2.getMessage(), startsWith( "Transaction can't be committed. It has been rolled back" ) ); + } } @Test @@ -365,8 +457,6 @@ void shouldDisallowQueriesAfterFailureWhenResultsAreConsumed() ClientException error3 = assertThrows( ClientException.class, () -> tx.run( "RETURN 42" ).consume() ); assertThat( error3.getMessage(), startsWith( "Cannot run more statements in this transaction" ) ); - - tx.success(); } assertEquals( 0, countNodesByLabel( "Node" ) ); @@ -386,7 +476,7 @@ void shouldRollbackWhenMarkedSuccessfulButOneStatementFails() tx.run( "CREATE (:Node3)" ); tx.run( "CREATE (:Node4)" ); - tx.success(); + tx.commit(); } } ); @@ -402,43 +492,51 @@ void shouldRollbackWhenMarkedSuccessfulButOneStatementFails() assertEquals( 0, countNodesByLabel( "Node4" ) ); } - private static int countNodesByLabel( String label ) + private void shouldRunAndCloseAfterAction( Consumer txConsumer, boolean isCommit ) { - return session.run( "MATCH (n:" + label + ") RETURN count(n)" ).single().get( 0 ).asInt(); - } - - private static void testFailWhenConnectionKilledDuringTransaction( boolean markForSuccess ) - { - ChannelTrackingDriverFactory factory = new ChannelTrackingDriverFactory( 1, Clock.SYSTEM ); - Config config = Config.builder().withLogging( DEV_NULL_LOGGING ).build(); + // When + try ( Transaction tx = session.beginTransaction() ) + { + tx.run( "CREATE (n:FirstNode)" ); + tx.run( "CREATE (n:SecondNode)" ); + txConsumer.accept( tx ); + } - try ( Driver driver = factory.newInstance( session.uri(), session.authToken(), RoutingSettings.DEFAULT, DEFAULT, config ) ) + // Then the outcome of both statements should be visible + StatementResult result = session.run( "MATCH (n) RETURN count(n)" ); + long nodes = result.single().get( "count(n)" ).asLong(); + if (isCommit) { - ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class, () -> - { - try ( Session session = driver.session(); - Transaction tx = session.beginTransaction() ) - { - tx.run( "CREATE (:MyNode {id: 1})" ).consume(); + assertThat( nodes, equalTo( 2L ) ); + } + else + { + assertThat( nodes, equalTo( 0L ) ); + } + } - if ( markForSuccess ) - { - tx.success(); - } + private void shouldBeClosedAfterAction( Consumer txConsumer ) + { + // When + Transaction tx = session.beginTransaction(); + txConsumer.accept( tx ); - // kill all network channels - for ( Channel channel: factory.channels() ) - { - channel.close().syncUninterruptibly(); - } + // Then + assertFalse( tx.isOpen() ); + } - tx.run( "CREATE (:MyNode {id: 1})" ).consume(); - } - } ); + private void shouldFailToRunQueryAfterTxAction( Consumer txConsumer ) + { + Transaction tx = session.beginTransaction(); + tx.run( "CREATE (:MyLabel)" ); + txConsumer.accept( tx ); - assertThat( e.getMessage(), containsString( "Connection to the database terminated" ) ); - } + ClientException e = assertThrows( ClientException.class, () -> tx.run( "CREATE (:MyOtherLabel)" ) ); + assertThat( e.getMessage(), startsWith( "Cannot run more statements in this transaction" ) ); + } - assertEquals( 0, session.run( "MATCH (n:MyNode {id: 1}) RETURN count(n)" ).single().get( 0 ).asInt() ); + private static int countNodesByLabel( String label ) + { + return session.run( "MATCH (n:" + label + ") RETURN count(n)" ).single().get( 0 ).asInt(); } } diff --git a/driver/src/test/java/org/neo4j/driver/integration/async/AsyncTransactionIT.java b/driver/src/test/java/org/neo4j/driver/integration/async/AsyncTransactionIT.java index d7c546b0dc..10eaf00adc 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/async/AsyncTransactionIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/async/AsyncTransactionIT.java @@ -307,30 +307,27 @@ void shouldFailBoBeginTxWithInvalidBookmark() } @Test - void shouldBePossibleToCommitWhenCommitted() + void shouldFailToCommitWhenCommitted() { AsyncTransaction tx = await( session.beginTransactionAsync() ); tx.runAsync( "CREATE ()" ); assertNull( await( tx.commitAsync() ) ); - CompletionStage secondCommit = tx.commitAsync(); - // second commit should return a completed future - assertTrue( secondCommit.toCompletableFuture().isDone() ); - assertNull( await( secondCommit ) ); + // should not be possible to commit after commit + ClientException e = assertThrows( ClientException.class, () -> await( tx.commitAsync() ) ); + assertThat( e.getMessage(), containsString( "transaction has been committed" ) ); } @Test - void shouldBePossibleToRollbackWhenRolledBack() + void shouldFailToRollbackWhenRolledBack() { AsyncTransaction tx = await( session.beginTransactionAsync() ); tx.runAsync( "CREATE ()" ); assertNull( await( tx.rollbackAsync() ) ); - CompletionStage secondRollback = tx.rollbackAsync(); - // second rollback should return a completed future - assertTrue( secondRollback.toCompletableFuture().isDone() ); - assertNull( await( secondRollback ) ); - } + // should not be possible to rollback after rollback + ClientException e = assertThrows( ClientException.class, () -> await( tx.rollbackAsync() ) ); + assertThat( e.getMessage(), containsString( "transaction has been rolled back" ) ); } @Test void shouldFailToCommitWhenRolledBack() diff --git a/driver/src/test/java/org/neo4j/driver/integration/reactive/RxTransactionIT.java b/driver/src/test/java/org/neo4j/driver/integration/reactive/RxTransactionIT.java index cc1173ac9d..80002e1eb4 100644 --- a/driver/src/test/java/org/neo4j/driver/integration/reactive/RxTransactionIT.java +++ b/driver/src/test/java/org/neo4j/driver/integration/reactive/RxTransactionIT.java @@ -34,7 +34,6 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; @@ -45,8 +44,8 @@ import org.neo4j.driver.exceptions.ServiceUnavailableException; import org.neo4j.driver.internal.Bookmark; import org.neo4j.driver.internal.util.EnabledOnNeo4jWith; -import org.neo4j.driver.reactive.RxStatementResult; import org.neo4j.driver.reactive.RxSession; +import org.neo4j.driver.reactive.RxStatementResult; import org.neo4j.driver.reactive.RxTransaction; import org.neo4j.driver.summary.ResultSummary; import org.neo4j.driver.summary.StatementType; @@ -58,6 +57,7 @@ import static java.util.Collections.emptyMap; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.junit.MatcherAssert.assertThat; @@ -68,8 +68,8 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.neo4j.driver.Values.parameters; import static org.neo4j.driver.SessionConfig.builder; +import static org.neo4j.driver.Values.parameters; import static org.neo4j.driver.internal.InternalBookmark.parse; import static org.neo4j.driver.internal.util.Iterables.single; import static org.neo4j.driver.internal.util.Matchers.containsResultAvailableAfterAndResultConsumedAfter; @@ -256,7 +256,7 @@ void shouldFailBoBeginTxWithInvalidBookmark() } @Test - void shouldBePossibleToCommitWhenCommitted() + void shouldFailToCommitWhenCommitted() { RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); assertCanRunCreate( tx ); @@ -264,12 +264,15 @@ void shouldBePossibleToCommitWhenCommitted() Mono secondCommit = Mono.from( tx.commit() ); // second commit should wrap around a completed future - StepVerifier.create( secondCommit ).verifyComplete(); + StepVerifier.create( secondCommit ).expectErrorSatisfies( error -> { + assertThat( error, instanceOf( ClientException.class ) ); + assertThat( error.getMessage(), startsWith( "Can't commit, transaction has been committed" ) ); + } ).verify(); } @Test - void shouldBePossibleToRollbackWhenRolledBack() + void shouldFailToRollbackWhenRolledBack() { RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); assertCanRunCreate( tx ); @@ -277,7 +280,10 @@ void shouldBePossibleToRollbackWhenRolledBack() Mono secondRollback = Mono.from( tx.rollback() ); // second rollback should wrap around a completed future - StepVerifier.create( secondRollback ).verifyComplete(); + StepVerifier.create( secondRollback ).expectErrorSatisfies( error -> { + assertThat( error, instanceOf( ClientException.class ) ); + assertThat( error.getMessage(), startsWith( "Can't rollback, transaction has been rolled back" ) ); + } ).verify(); } @Test @@ -302,7 +308,6 @@ void shouldFailToRollbackWhenCommitted() // should not be possible to rollback after commit ClientException e = assertThrows( ClientException.class, () -> await( tx.rollback() ) ); assertThat( e.getMessage(), containsString( "transaction has been committed" ) ); - } @Test @@ -598,67 +603,6 @@ void shouldConsumeNonEmptyCursor() testConsume( "RETURN 42" ); } - @Test - void shouldDoNothingWhenCommittedSecondTime() - { - RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); - assertCanCommit( tx ); - - CompletableFuture future = Mono.from( tx.commit() ).toFuture(); - assertTrue( future.isDone() ); - } - - @Test - void shouldFailToCommitAfterRollback() - { - RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); - assertCanRollback( tx ); - - ClientException e = assertThrows( ClientException.class, () -> await( tx.commit() ) ); - assertEquals( "Can't commit, transaction has been rolled back", e.getMessage() ); - - } - - @Test - void shouldFailToCommitAfterTermination() - { - RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); - assertFailToRunWrongStatement( tx ); - - ClientException e = assertThrows( ClientException.class, () -> await( tx.commit() ) ); - assertThat( e.getMessage(), startsWith( "Transaction can't be committed" ) ); - assertCanRollback( tx ); - } - - @Test - void shouldDoNothingWhenRolledBackSecondTime() - { - RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); - assertCanRollback( tx ); - - CompletableFuture future = Mono.from( tx.rollback() ).toFuture(); - assertTrue( future.isDone() ); - - } - - @Test - void shouldFailToRollbackAfterCommit() - { - RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); - assertCanCommit( tx ); - - ClientException e = assertThrows( ClientException.class, () -> await( tx.rollback() ) ); - assertEquals( "Can't rollback, transaction has been committed", e.getMessage() ); - } - - @Test - void shouldRollbackAfterTermination() - { - RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); - assertFailToRunWrongStatement( tx ); - assertCanRollback( tx ); - } - @ParameterizedTest @MethodSource( "commit" ) void shouldFailToRunQueryAfterCommit( boolean commit ) @@ -783,6 +727,16 @@ void shouldFailToCommitWhenPullAllFailureIsConsumed() ClientException e2 = assertThrows( ClientException.class, () -> await( tx.commit() ) ); assertThat( e2.getMessage(), startsWith( "Transaction can't be committed" ) ); + } + + @Test + void shouldBeAbleToRollbackWhenPullAllFailureIsConsumed() + { + RxTransaction tx = await( Mono.from( session.beginTransaction() ) ); + RxStatementResult result = tx.run( "FOREACH (value IN [1,2, 'aaa'] | CREATE (:Person {name: 10 / value}))" ); + + ClientException e1 = assertThrows( ClientException.class, () -> await( result.records() ) ); + assertThat( e1.code(), containsString( "TypeError" ) ); assertCanRollback( tx ); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitTest.java b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitTest.java index 52a91174db..2bc29451b6 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitTest.java @@ -25,11 +25,8 @@ import java.net.URI; import java.util.List; import java.util.Optional; +import java.util.function.Consumer; -import org.neo4j.driver.internal.cluster.RoutingSettings; -import org.neo4j.driver.internal.retry.RetrySettings; -import org.neo4j.driver.internal.util.io.ChannelTrackingDriverFactory; -import org.neo4j.driver.internal.util.Clock; import org.neo4j.driver.AccessMode; import org.neo4j.driver.AuthTokens; import org.neo4j.driver.Config; @@ -41,6 +38,10 @@ import org.neo4j.driver.StatementResult; import org.neo4j.driver.Transaction; import org.neo4j.driver.exceptions.TransientException; +import org.neo4j.driver.internal.cluster.RoutingSettings; +import org.neo4j.driver.internal.retry.RetrySettings; +import org.neo4j.driver.internal.util.Clock; +import org.neo4j.driver.internal.util.io.ChannelTrackingDriverFactory; import org.neo4j.driver.util.StubServer; import static java.util.Arrays.asList; @@ -58,8 +59,8 @@ import static org.mockito.Mockito.when; import static org.neo4j.driver.SessionConfig.builder; import static org.neo4j.driver.SessionConfig.forDatabase; -import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.Values.parameters; +import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.util.StubServer.INSECURE_CONFIG; class DirectDriverBoltKitTest @@ -99,7 +100,7 @@ void shouldSendMultipleBookmarks() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (n {name:'Bob'})" ); - tx.success(); + tx.commit(); } assertEquals( InternalBookmark.parse( "neo4j:bookmark:v1:tx95" ), session.lastBookmark() ); @@ -212,29 +213,49 @@ void shouldCloseChannelWhenResetFails() throws Exception } @Test - void shouldPropagateTransactionCommitErrorWhenSessionClosed() throws Exception + void shouldPropagateTransactionRollbackErrorWhenSessionClosed() throws Exception { - testTransactionCloseErrorPropagationWhenSessionClosed( "commit_error.script", true, "Unable to commit" ); + StubServer server = StubServer.start( "rollback_error.script", 9001 ); + try + { + try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ) ) + { + Session session = driver.session(); + + Transaction tx = session.beginTransaction(); + StatementResult result = tx.run( "CREATE (n {name:'Alice'}) RETURN n.name AS name" ); + assertEquals( "Alice", result.single().get( "name" ).asString() ); + + TransientException e = assertThrows( TransientException.class, session::close ); + assertEquals( "Neo.TransientError.General.DatabaseUnavailable", e.code() ); + assertEquals( "Unable to rollback", e.getMessage() ); + } + } + finally + { + assertEquals( 0, server.exitStatus() ); + } } @Test - void shouldPropagateTransactionRollbackErrorWhenSessionClosed() throws Exception + void shouldThrowCommitErrorWhenTransactionCommit() throws Exception { - testTransactionCloseErrorPropagationWhenSessionClosed( "rollback_error.script", false, "Unable to rollback" ); + testTxCloseErrorPropagation( "commit_error.script", Transaction::commit, "Unable to commit" ); } @Test - void shouldThrowCommitErrorWhenTransactionClosed() throws Exception + void shouldThrowRollbackErrorWhenTransactionRollback() throws Exception { - testTxCloseErrorPropagation( "commit_error.script", true, "Unable to commit" ); + testTxCloseErrorPropagation( "rollback_error.script", Transaction::rollback, "Unable to rollback" ); } @Test - void shouldThrowRollbackErrorWhenTransactionClosed() throws Exception + void shouldThrowRollbackErrorWhenTransactionClose() throws Exception { - testTxCloseErrorPropagation( "rollback_error.script", false, "Unable to rollback" ); + testTxCloseErrorPropagation( "rollback_error.script", Transaction::close, "Unable to rollback" ); } + @Test void shouldThrowCorrectErrorOnRunFailure() throws Throwable { @@ -269,9 +290,8 @@ void shouldThrowCorrectErrorOnCommitFailure() throws Throwable Transaction transaction = session.beginTransaction(); StatementResult result = transaction.run( "CREATE (n {name:'Bob'})" ); result.consume(); - transaction.success(); - TransientException error = assertThrows( TransientException.class, transaction::close ); + TransientException error = assertThrows( TransientException.class, transaction::commit ); assertThat( error.code(), equalTo( "Neo.TransientError.General.DatabaseUnavailable" ) ); } finally @@ -313,41 +333,7 @@ void shouldAllowDatabaseNameInBeginTransaction() throws Throwable } } - private static void testTransactionCloseErrorPropagationWhenSessionClosed( String script, boolean commit, - String expectedErrorMessage ) throws Exception - { - StubServer server = StubServer.start( script, 9001 ); - try - { - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ) ) - { - Session session = driver.session(); - - Transaction tx = session.beginTransaction(); - StatementResult result = tx.run( "CREATE (n {name:'Alice'}) RETURN n.name AS name" ); - assertEquals( "Alice", result.single().get( "name" ).asString() ); - - if ( commit ) - { - tx.success(); - } - else - { - tx.failure(); - } - - TransientException e = assertThrows( TransientException.class, session::close ); - assertEquals( "Neo.TransientError.General.DatabaseUnavailable", e.code() ); - assertEquals( expectedErrorMessage, e.getMessage() ); - } - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - - private static void testTxCloseErrorPropagation( String script, boolean commit, String expectedErrorMessage ) + private static void testTxCloseErrorPropagation( String script, Consumer txAction, String expectedErrorMessage ) throws Exception { StubServer server = StubServer.start( script, 9001 ); @@ -360,16 +346,8 @@ private static void testTxCloseErrorPropagation( String script, boolean commit, StatementResult result = tx.run( "CREATE (n {name:'Alice'}) RETURN n.name AS name" ); assertEquals( "Alice", result.single().get( "name" ).asString() ); - if ( commit ) - { - tx.success(); - } - else - { - tx.failure(); - } + TransientException e = assertThrows( TransientException.class, () -> txAction.accept( tx ) ); - TransientException e = assertThrows( TransientException.class, tx::close ); assertEquals( "Neo.TransientError.General.DatabaseUnavailable", e.code() ); assertEquals( expectedErrorMessage, e.getMessage() ); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/InternalTransactionTest.java b/driver/src/test/java/org/neo4j/driver/internal/InternalTransactionTest.java index 0493d8bf17..8b9e9093ee 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/InternalTransactionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/InternalTransactionTest.java @@ -23,6 +23,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Stream; @@ -100,7 +101,7 @@ void shouldFlushOnRun( Function runReturnOne ) thro @Test void shouldCommit() throws Throwable { - tx.success(); + tx.commit(); tx.close(); verifyCommitTx( connection ); @@ -119,7 +120,7 @@ void shouldRollbackByDefault() throws Throwable @Test void shouldRollback() throws Throwable { - tx.failure(); + tx.rollback(); tx.close(); verifyRollbackTx( connection ); @@ -132,7 +133,6 @@ void shouldRollbackWhenFailedRun() throws Throwable setupFailingRun( connection, new RuntimeException( "Bang!" ) ); assertThrows( RuntimeException.class, () -> tx.run( "RETURN 1" ).consume() ); - tx.success(); tx.close(); verify( connection ).release(); @@ -143,8 +143,7 @@ void shouldRollbackWhenFailedRun() throws Throwable void shouldReleaseConnectionWhenFailedToCommit() throws Throwable { setupFailingCommit( connection ); - tx.success(); - assertThrows( Exception.class, () -> tx.close() ); + assertThrows( Exception.class, () -> tx.commit() ); verify( connection ).release(); assertFalse( tx.isOpen() ); @@ -152,9 +151,20 @@ void shouldReleaseConnectionWhenFailedToCommit() throws Throwable @Test void shouldReleaseConnectionWhenFailedToRollback() throws Throwable + { + shouldReleaseConnectionWhenFailedToAction( Transaction::rollback ); + } + + @Test + void shouldReleaseConnectionWhenFailedToClose() throws Throwable + { + shouldReleaseConnectionWhenFailedToAction( Transaction::close ); + } + + private void shouldReleaseConnectionWhenFailedToAction( Consumer txAction ) { setupFailingRollback( connection ); - assertThrows( Exception.class, () -> tx.close() ); + assertThrows( Exception.class, () -> txAction.accept( tx ) ); verify( connection ).release(); assertFalse( tx.isOpen() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/async/ExplicitTransactionTest.java b/driver/src/test/java/org/neo4j/driver/internal/async/ExplicitTransactionTest.java index d85dabafa0..e6770e985c 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/async/ExplicitTransactionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/async/ExplicitTransactionTest.java @@ -106,43 +106,6 @@ void shouldRollbackOnImplicitFailure() order.verify( connection ).release(); } - @Test - void shouldRollbackOnExplicitFailure() - { - // Given - Connection connection = connectionMock(); - ExplicitTransaction tx = beginTx( connection ); - - // When - tx.failure(); - tx.success(); // even if success is called after the failure call! - await( tx.closeAsync() ); - - // Then - InOrder order = inOrder( connection ); - order.verify( connection ).write( eq( new RunMessage( "BEGIN" ) ), any(), eq( PullAllMessage.PULL_ALL ), any() ); - order.verify( connection ).writeAndFlush( eq( new RunMessage( "ROLLBACK" ) ), any(), eq( PullAllMessage.PULL_ALL ), any() ); - order.verify( connection ).release(); - } - - @Test - void shouldCommitOnSuccess() - { - // Given - Connection connection = connectionMock(); - ExplicitTransaction tx = beginTx( connection ); - - // When - tx.success(); - await( tx.closeAsync() ); - - // Then - InOrder order = inOrder( connection ); - order.verify( connection ).write( eq( new RunMessage( "BEGIN" ) ), any(), eq( PullAllMessage.PULL_ALL ), any() ); - order.verify( connection ).writeAndFlush( eq( new RunMessage( "COMMIT" ) ), any(), eq( PullAllMessage.PULL_ALL ), any() ); - order.verify( connection ).release(); - } - @Test void shouldOnlyQueueMessagesWhenNoBookmarkGiven() { @@ -174,26 +137,6 @@ void shouldBeOpenAfterConstruction() assertTrue( tx.isOpen() ); } - @Test - void shouldBeOpenWhenMarkedForSuccess() - { - ExplicitTransaction tx = beginTx( connectionMock() ); - - tx.success(); - - assertTrue( tx.isOpen() ); - } - - @Test - void shouldBeOpenWhenMarkedForFailure() - { - ExplicitTransaction tx = beginTx( connectionMock() ); - - tx.failure(); - - assertTrue( tx.isOpen() ); - } - @Test void shouldBeClosedWhenMarkedAsTerminated() { @@ -204,28 +147,6 @@ void shouldBeClosedWhenMarkedAsTerminated() assertTrue( tx.isOpen() ); } - @Test - void shouldBeClosedAfterCommit() - { - ExplicitTransaction tx = beginTx( connectionMock() ); - - tx.success(); - await( tx.closeAsync() ); - - assertFalse( tx.isOpen() ); - } - - @Test - void shouldBeClosedAfterRollback() - { - ExplicitTransaction tx = beginTx( connectionMock() ); - - tx.failure(); - await( tx.closeAsync() ); - - assertFalse( tx.isOpen() ); - } - @Test void shouldBeClosedWhenMarkedTerminatedAndClosed() { diff --git a/driver/src/test/java/org/neo4j/driver/internal/async/NetworkSessionTest.java b/driver/src/test/java/org/neo4j/driver/internal/async/NetworkSessionTest.java index cdad603b14..84cef6e080 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/async/NetworkSessionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/async/NetworkSessionTest.java @@ -242,8 +242,7 @@ void updatesBookmarkWhenTxIsClosed() InternalBookmark bookmark = (InternalBookmark) session.lastBookmark(); assertTrue( bookmark.isEmpty() ); - tx.success(); - await( tx.closeAsync() ); + await( tx.commitAsync() ); assertEquals( bookmarkAfterCommit, session.lastBookmark() ); } @@ -288,14 +287,12 @@ void bookmarkIsPropagatedBetweenTransactions() when( connection.protocol() ).thenReturn( protocol ); ExplicitTransaction tx1 = beginTransaction( session ); - tx1.success(); - await( tx1.closeAsync() ); + await( tx1.commitAsync() ); assertEquals( bookmark1, session.lastBookmark() ); ExplicitTransaction tx2 = beginTransaction( session ); verifyBeginTx( connection, bookmark1 ); - tx2.success(); - await( tx2.closeAsync() ); + await( tx2.commitAsync() ); assertEquals( bookmark2, session.lastBookmark() ); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/BookmarkUtils.java b/driver/src/test/java/org/neo4j/driver/internal/util/BookmarkUtils.java index 5903450133..8742fec5b0 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/util/BookmarkUtils.java +++ b/driver/src/test/java/org/neo4j/driver/internal/util/BookmarkUtils.java @@ -31,6 +31,7 @@ import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.hamcrest.junit.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.neo4j.driver.internal.util.Iterables.asList; @@ -47,6 +48,16 @@ public static void assertBookmarkIsEmpty( Bookmark bookmark ) assertTrue( ((InternalBookmark) bookmark).isEmpty() ); } + /** + * Bookmark is not empty but I do not care what value it has. + */ + public static void assertBookmarkIsNotEmpty( Bookmark bookmark ) + { + assertNotNull( bookmark ); + assertThat( bookmark, instanceOf( InternalBookmark.class ) ); + assertFalse( ((InternalBookmark) bookmark).isEmpty() ); + } + /** * Bookmark contains one single value */ diff --git a/driver/src/test/java/org/neo4j/driver/stress/BlockingReadQueryInTx.java b/driver/src/test/java/org/neo4j/driver/stress/BlockingReadQueryInTx.java index 889975a5e6..ac2f0467a6 100644 --- a/driver/src/test/java/org/neo4j/driver/stress/BlockingReadQueryInTx.java +++ b/driver/src/test/java/org/neo4j/driver/stress/BlockingReadQueryInTx.java @@ -54,7 +54,7 @@ public void execute( C context ) } context.readCompleted( result.summary() ); - tx.success(); + tx.commit(); } } } diff --git a/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryInTx.java b/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryInTx.java index e939cbd96f..55aebee212 100644 --- a/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryInTx.java +++ b/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryInTx.java @@ -47,7 +47,7 @@ public void execute( C context ) try ( Transaction tx = beginTransaction( session, context ) ) { result = tx.run( "CREATE ()" ); - tx.success(); + tx.commit(); } context.setBookmark( session.lastBookmark() ); diff --git a/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryUsingReadSessionInTx.java b/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryUsingReadSessionInTx.java index 894b085ac1..76e3617ef3 100644 --- a/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryUsingReadSessionInTx.java +++ b/driver/src/test/java/org/neo4j/driver/stress/BlockingWriteQueryUsingReadSessionInTx.java @@ -49,7 +49,7 @@ public void execute( C context ) { StatementResult result = tx.run( "CREATE ()" ); resultRef.set( result ); - tx.success(); + tx.commit(); } } ); assertNotNull( resultRef.get() ); diff --git a/driver/src/test/java/org/neo4j/driver/stress/CausalClusteringIT.java b/driver/src/test/java/org/neo4j/driver/stress/CausalClusteringIT.java index 00c5dcfd7b..ecf2edfe60 100644 --- a/driver/src/test/java/org/neo4j/driver/stress/CausalClusteringIT.java +++ b/driver/src/test/java/org/neo4j/driver/stress/CausalClusteringIT.java @@ -192,7 +192,7 @@ void bookmarksShouldWorkWithDriverPinnedToSingleServer() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (p:Person {name: {name} })", Values.parameters( "name", "Alistair" ) ); - tx.success(); + tx.commit(); } return session.lastBookmark(); @@ -205,7 +205,7 @@ void bookmarksShouldWorkWithDriverPinnedToSingleServer() throws Exception { Record record = tx.run( "MATCH (n:Person) RETURN COUNT(*) AS count" ).next(); assertEquals( 1, record.get( "count" ).asInt() ); - tx.success(); + tx.commit(); } } } @@ -230,7 +230,7 @@ void shouldUseBookmarkFromAReadSessionInAWriteSession() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "MATCH (n:Person) RETURN COUNT(*) AS count" ).next(); - tx.success(); + tx.commit(); } bookmark = session.lastBookmark(); @@ -243,7 +243,7 @@ void shouldUseBookmarkFromAReadSessionInAWriteSession() throws Exception try ( Transaction tx = session.beginTransaction() ) { tx.run( "CREATE (p:Person {name: {name} })", Values.parameters( "name", "Alistair" ) ); - tx.success(); + tx.commit(); } return null; @@ -340,9 +340,8 @@ void shouldHandleGracefulLeaderSwitch() throws Exception tx1.run( "CREATE (person:Person {name: {name}, title: {title}})", parameters( "name", "Webber", "title", "Mr" ) ); - tx1.success(); - assertThrows( (Class) SessionExpiredException.class, ((AutoCloseable) tx1)::close ); + assertThrows( (Class) SessionExpiredException.class, tx1::commit ); session1.close(); Bookmark bookmark = inExpirableSession( driver, Driver::session, session -> @@ -351,7 +350,7 @@ void shouldHandleGracefulLeaderSwitch() throws Exception { tx.run( "CREATE (person:Person {name: {name}, title: {title}})", parameters( "name", "Webber", "title", "Mr" ) ); - tx.success(); + tx.commit(); } return session.lastBookmark(); } ); @@ -360,7 +359,7 @@ void shouldHandleGracefulLeaderSwitch() throws Exception Transaction tx2 = session2.beginTransaction() ) { Record record = tx2.run( "MATCH (n:Person) RETURN COUNT(*) AS count" ).next(); - tx2.success(); + tx2.commit(); assertEquals( 1, record.get( "count" ).asInt() ); } } @@ -579,8 +578,8 @@ RoutingSettings.DEFAULT, RetrySettings.DEFAULT, configWithoutLogging() ) ) driverFactory.setNextRunFailure( error ); assertUnableToRunMoreStatementsInTx( tx2, error ); - closeTx( tx2 ); - closeTx( tx1 ); + tx2.close(); + tx1.commit(); try ( Session session3 = driver.session( builder().withBookmarks( session1.lastBookmark() ).build() ) ) { @@ -706,12 +705,6 @@ void shouldKeepOperatingWhenConnectionsBreak() throws Exception } } - private static void closeTx( Transaction tx ) - { - tx.success(); - tx.close(); - } - private static void assertUnableToRunMoreStatementsInTx( Transaction tx, ServiceUnavailableException cause ) { SessionExpiredException e = assertThrows( SessionExpiredException.class, () -> tx.run( "CREATE (n:Node3 {name: 'Node3'})" ).consume() ); diff --git a/driver/src/test/java/org/neo4j/driver/util/Neo4jSettings.java b/driver/src/test/java/org/neo4j/driver/util/Neo4jSettings.java index 721c2e6fb7..6a44966837 100644 --- a/driver/src/test/java/org/neo4j/driver/util/Neo4jSettings.java +++ b/driver/src/test/java/org/neo4j/driver/util/Neo4jSettings.java @@ -38,7 +38,6 @@ public class Neo4jSettings private static final String DEFAULT_CERT_DIR = "certificates"; public static final String DEFAULT_TLS_CERT_PATH = DEFAULT_CERT_DIR + "/neo4j.cert"; public static final String DEFAULT_TLS_KEY_PATH = DEFAULT_CERT_DIR + "/neo4j.key"; - public static final String DEFAULT_PAGE_CACHE_SIZE = "512m"; public static final String DEFAULT_BOLT_TLS_LEVEL = BoltTlsLevel.OPTIONAL.toString(); public static final String DEFAULT_DATA_DIR = "data"; @@ -59,7 +58,6 @@ public class Neo4jSettings private final Set excludes; public static final Neo4jSettings TEST_SETTINGS = new Neo4jSettings( map( - "dbms.backup.enabled", "false", "dbms.connector.http.listen_address", ":" + CURRENT_HTTP_PORT, "dbms.connector.https.listen_address", ":" + CURRENT_HTTPS_PORT, "dbms.connector.bolt.listen_address", ":" + CURRENT_BOLT_PORT, diff --git a/examples/src/main/java/org/neo4j/docs/driver/CypherErrorExample.java b/examples/src/main/java/org/neo4j/docs/driver/CypherErrorExample.java index 195294b194..70bc679a17 100644 --- a/examples/src/main/java/org/neo4j/docs/driver/CypherErrorExample.java +++ b/examples/src/main/java/org/neo4j/docs/driver/CypherErrorExample.java @@ -61,6 +61,7 @@ private static int selectEmployee( Transaction tx, String name ) } catch ( ClientException ex ) { + tx.rollback(); System.err.println( ex.getMessage() ); return -1; }