Merge pull request #375 from lutovich/1.1-tx-isOpen-reset

Fix `Transaction#isOpen()` after errors
neo4j · May 19, 2017 · 30cc1a4 · 30cc1a4
2 parents 85760c8 + de1faa6
commit 30cc1a4
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 0 deletions.
diff --git a/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java b/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java
@@ -135,6 +135,12 @@ else if ( state == State.MARKED_FAILED || state == State.ACTIVE )
                 {
                     rollbackTx();
                 }
+                else if ( state == State.FAILED )
+                {
+                    // unrecoverable error happened, transaction should've been rolled back on the server
+                    // update state so that this transaction does not remain open
+                    state = State.ROLLED_BACK;
+                }
             }
         }
         finally

diff --git a/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java b/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java
@@ -207,6 +207,17 @@ public void shouldBeClosedAfterRollback()
         assertFalse( tx.isOpen() );
     }
 
+    @Test
+    public void shouldBeClosedWhenMarkedToCloseAndClosed()
+    {
+        ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( Runnable.class ) );
+
+        tx.markToClose();
+        tx.close();
+
+        assertFalse( tx.isOpen() );
+    }
+
     private static Connection openConnectionMock()
     {
         Connection connection = mock( Connection.class );

diff --git a/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java b/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java
@@ -30,7 +30,9 @@
 import static junit.framework.Assert.fail;
 import static junit.framework.TestCase.assertNotNull;
 import static org.hamcrest.CoreMatchers.equalTo;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -164,4 +166,38 @@ public void shouldGetExceptionIfTryingToCloseSessionMoreThanOnce() throws Throwa
            assertThat( e.getMessage(), equalTo("This session has already been closed." ));
         }
     }
+
+    @Test
+    public void transactionShouldBeOpenAfterSessionReset()
+    {
+        NetworkSession session = new NetworkSession( openConnectionMock() );
+        Transaction tx = session.beginTransaction();
+
+        assertTrue( tx.isOpen() );
+
+        session.reset();
+        assertTrue( tx.isOpen() );
+    }
+
+    @Test
+    public void transactionShouldBeClosedAfterSessionResetAndClose()
+    {
+        NetworkSession session = new NetworkSession( openConnectionMock() );
+        Transaction tx = session.beginTransaction();
+
+        assertTrue( tx.isOpen() );
+
+        session.reset();
+        assertTrue( tx.isOpen() );
+
+        tx.close();
+        assertFalse( tx.isOpen() );
+    }
+
+    private static Connection openConnectionMock()
+    {
+        Connection connection = mock( Connection.class );
+        when( connection.isOpen() ).thenReturn( true );
+        return connection;
+    }
 }