From 682c8d04fd83a5048538ccbd84838a2563c695bf Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 18 May 2017 15:59:03 +0200 Subject: [PATCH] Fix `Transaction#isOpen()` It was previously reporting transactions marked for success/failure as closed which is not correct. Transaction will now be considered closed only after `Transaction#close()` call which executes commit/rollback. --- .../driver/internal/ExplicitTransaction.java | 2 +- .../internal/ExplicitTransactionTest.java | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java b/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java index 29056d9238..078f3ceb56 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java +++ b/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java @@ -205,7 +205,7 @@ public synchronized StatementResult run( Statement statement ) @Override public boolean isOpen() { - return state == State.ACTIVE; + return state != State.SUCCEEDED && state != State.ROLLED_BACK; } private void ensureNotFailed() diff --git a/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java b/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java index 13b199cf72..88b85fce8c 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java @@ -26,8 +26,11 @@ import org.neo4j.driver.internal.spi.Collector; import org.neo4j.driver.internal.spi.Connection; +import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.Value; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -143,4 +146,71 @@ public void shouldSyncWhenBookmarkGiven() inOrder.verify( connection ).pullAll( Collector.NO_OP ); inOrder.verify( connection ).sync(); } + + @Test + public void shouldBeOpenAfterConstruction() + { + Transaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) ); + + assertTrue( tx.isOpen() ); + } + + @Test + public void shouldBeOpenWhenMarkedForSuccess() + { + Transaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) ); + + tx.success(); + + assertTrue( tx.isOpen() ); + } + + @Test + public void shouldBeOpenWhenMarkedForFailure() + { + Transaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) ); + + tx.failure(); + + assertTrue( tx.isOpen() ); + } + + @Test + public void shouldBeOpenWhenMarkedToClose() + { + ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) ); + + tx.markToClose(); + + assertTrue( tx.isOpen() ); + } + + @Test + public void shouldBeClosedAfterCommit() + { + Transaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) ); + + tx.success(); + tx.close(); + + assertFalse( tx.isOpen() ); + } + + @Test + public void shouldBeClosedAfterRollback() + { + Transaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) ); + + tx.failure(); + tx.close(); + + assertFalse( tx.isOpen() ); + } + + private static Connection openConnectionMock() + { + Connection connection = mock( Connection.class ); + when( connection.isOpen() ).thenReturn( true ); + return connection; + } }