Another one bites the dust

Author: technige

driver/src/main/java/org/neo4j/driver/internal/InternalResultCursor.java

@@ -186,7 +186,7 @@ private void receiveOne()
         {
             if (transaction != null)
             {
-                transaction.failure();
+                transaction.defunct();
             }
             throw ex;
         }

driver/src/main/java/org/neo4j/driver/internal/InternalTransaction.java

@@ -33,8 +33,8 @@
 
 public class InternalTransaction implements Transaction
 {
-    private final Connection conn;
     private final Runnable cleanup;
+    private Connection conn;
 
     private enum State
     {
@@ -90,25 +90,36 @@ public void failure()
         }
     }
 
+    @Override
+    public void defunct()
+    {
+        state = State.ROLLED_BACK;
+        conn = null;
+    }
+
     @Override
     public void close()
     {
         try
         {
-            if ( state == State.MARKED_SUCCESS )
-            {
-                conn.run( "COMMIT", Collections.<String, Value>emptyMap(), null );
-                conn.discardAll();
-                conn.sync();
-                state = State.SUCCEEDED;
-            }
-            else if ( state == State.MARKED_FAILED || state == State.ACTIVE )
+            if ( conn != null && conn.isOpen() )
             {
-                // If alwaysValid of the things we've put in the queue have been sent off, there is no need to
-                // do this, we could just clear the queue. Future optimization.
-                conn.run( "ROLLBACK", Collections.<String, Value>emptyMap(), null );
-                conn.discardAll();
-                state = State.ROLLED_BACK;
+                if ( state == State.MARKED_SUCCESS )
+                {
+                    conn.run( "COMMIT", Collections.<String, Value>emptyMap(), null );
+                    conn.discardAll();
+                    conn.sync();
+                    state = State.SUCCEEDED;
+                }
+                else if ( state == State.MARKED_FAILED || state == State.ACTIVE )
+                {
+                    // If alwaysValid of the things we've put in the queue have been sent off, there is no need to
+                    // do this, we could just clear the queue. Future optimization.
+                    conn.run( "ROLLBACK", Collections.<String, Value>emptyMap(), null );
+                    conn.discardAll();
+                    conn.sync();
+                    state = State.ROLLED_BACK;
+                }
             }
         }
         finally
@@ -158,7 +169,7 @@ public boolean isOpen()
 
     private void ensureNotFailed()
     {
-        if ( state == State.FAILED || state == State.MARKED_FAILED )
+        if ( state == State.FAILED || state == State.MARKED_FAILED || state == State.ROLLED_BACK )
         {
             throw new ClientException(
                 "Cannot run more statements in this transaction, because previous statements in the " +

driver/src/main/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandler.java

@@ -85,7 +85,7 @@ public void handleSuccessMessage( Map<String,Value> meta )
     public void handleRecordMessage( Value[] fields )
     {
         super.handleRecordMessage( fields );
-        logger.debug( "S: RecordMessage{%s}", Arrays.asList( fields ) );
+        logger.debug( "S: [RECORD %s]", Arrays.asList( fields ) );
     }
 
     @Override

driver/src/main/java/org/neo4j/driver/internal/connector/socket/SocketClient.java

@@ -102,6 +102,7 @@ public int sendAll( Queue<Message> messages ) throws IOException
             }
             else
             {
+                logger.debug( "C: %s", message );
                 writer.write( message );
                 messageCount += 1;
             }

driver/src/main/java/org/neo4j/driver/internal/connector/socket/SocketConnection.java

@@ -187,7 +187,6 @@ else if ( e instanceof SocketTimeoutException )
     private void queueMessage( Message msg, StreamCollector collector )
     {
         pendingMessages.add( msg );
-        logger.debug( "C: %s", msg );
         responseHandler.appendResultCollector( collector );
     }
 
@@ -202,9 +201,4 @@ public boolean isOpen()
     {
         return socket.isOpen();
     }
-
-    private int nextRequestId()
-    {
-        return (requestCounter++);
-    }
 }

driver/src/main/java/org/neo4j/driver/internal/messaging/RecordMessage.java

@@ -41,7 +41,7 @@ public void dispatch( MessageHandler handler ) throws IOException
     @Override
     public String toString()
     {
-        return "RecordMessage{" + Arrays.toString( fields ) + '}';
+        return "[RECORD " + Arrays.toString( fields ) + ']';
     }
 
     @Override

driver/src/main/java/org/neo4j/driver/v1/Transaction.java

@@ -65,4 +65,6 @@ public interface Transaction extends Resource, StatementRunner
      * </pre>
      */
     void failure();
+
+    void defunct();
 }

driver/src/test/java/org/neo4j/driver/internal/InternalTransactionTest.java

@@ -30,14 +30,16 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
 
 public class InternalTransactionTest
 {
     @Test
-    public void shouldRollbackOnNoExplicitSuccess() throws Throwable
+    public void shouldRollbackOnImplicitFailure() throws Throwable
     {
         // Given
         Connection conn = mock( Connection.class );
+        when( conn.isOpen() ).thenReturn( true );
         Runnable cleanup = mock( Runnable.class );
         InternalTransaction tx = new InternalTransaction( conn, cleanup );
 
@@ -48,8 +50,10 @@ public void shouldRollbackOnNoExplicitSuccess() throws Throwable
         InOrder order = inOrder( conn );
         order.verify( conn ).run( "BEGIN", Collections.<String, Value>emptyMap(), null );
         order.verify( conn ).discardAll();
+        order.verify( conn ).isOpen();
         order.verify( conn ).run( "ROLLBACK", Collections.<String, Value>emptyMap(), null );
         order.verify( conn ).discardAll();
+        order.verify( conn ).sync();
         verify( cleanup ).run();
         verifyNoMoreInteractions( conn, cleanup );
     }
@@ -59,21 +63,23 @@ public void shouldRollbackOnExplicitFailure() throws Throwable
     {
         // Given
         Connection conn = mock( Connection.class );
+        when( conn.isOpen() ).thenReturn( true );
         Runnable cleanup = mock( Runnable.class );
         InternalTransaction tx = new InternalTransaction( conn, cleanup );
 
+        // When
         tx.failure();
         tx.success(); // even if success is called after the failure call!
-
-        // When
         tx.close();
 
         // Then
         InOrder order = inOrder( conn );
         order.verify( conn ).run( "BEGIN", Collections.<String, Value>emptyMap(), null );
         order.verify( conn ).discardAll();
+        order.verify( conn ).isOpen();
         order.verify( conn ).run( "ROLLBACK", Collections.<String, Value>emptyMap(), null );
         order.verify( conn ).discardAll();
+        order.verify( conn ).sync();
         verify( cleanup ).run();
         verifyNoMoreInteractions( conn, cleanup );
     }
@@ -83,18 +89,20 @@ public void shouldCommitOnSuccess() throws Throwable
     {
         // Given
         Connection conn = mock( Connection.class );
+        when( conn.isOpen() ).thenReturn( true );
         Runnable cleanup = mock( Runnable.class );
         InternalTransaction tx = new InternalTransaction( conn, cleanup );
 
-        tx.success();
-
         // When
+        tx.success();
         tx.close();
 
         // Then
+
         InOrder order = inOrder( conn );
         order.verify( conn ).run( "BEGIN", Collections.<String, Value>emptyMap(), null );
         order.verify( conn ).discardAll();
+        order.verify( conn ).isOpen();
         order.verify( conn ).run( "COMMIT", Collections.<String, Value>emptyMap(), null );
         order.verify( conn ).discardAll();
         order.verify( conn ).sync();

driver/src/test/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandlerTest.java

@@ -131,7 +131,7 @@ public void shouldLogRecordMessage() throws Throwable
         handler.handleRecordMessage( new Value[]{} );
 
         // Then
-        assertEquals( "S: RecordMessage{[]}", log );
+        assertEquals( "S: [RECORD []]", log );
         assertEquals( format( new RecordMessage( new Value[]{} ) ), log );
     }
 

driver/src/test/java/org/neo4j/driver/v1/util/Neo4jRunner.java

@@ -22,8 +22,10 @@
 import java.io.IOException;
 import java.net.URI;
 import java.util.Map;
+import java.util.logging.Level;
 
 import org.neo4j.driver.internal.connector.socket.SocketClient;
+import org.neo4j.driver.internal.logging.ConsoleLogging;
 import org.neo4j.driver.internal.logging.DevNullLogger;
 import org.neo4j.driver.v1.Config;
 import org.neo4j.driver.v1.Driver;