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

lutovich · web-flow · commit 30cc1a476634 · 2017-05-19T12:15:41.000+02:00
Fix `Transaction#isOpen()` after errors
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;
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -135,6 +135,12 @@ else if ( state == State.MARKED_FAILED \|\| state == State.ACTIVE )`
`135`	`135`	`{`
`136`	`136`	`rollbackTx();`
`137`	`137`	`}`
	`138`	`+ else if ( state == State.FAILED )`
	`139`	`+ {`
	`140`	`+ // unrecoverable error happened, transaction should've been rolled back on the server`
	`141`	`+ // update state so that this transaction does not remain open`
	`142`	`+ state = State.ROLLED_BACK;`
	`143`	`+ }`
`138`	`144`	`}`
`139`	`145`	`}`
`140`	`146`	`finally`