Merge pull request #629 from zhenlineo/1.7-conn-timeout

Changing default conneciton timeout to be 30s

Author: zhenlineo

driver/src/main/java/org/neo4j/driver/internal/messaging/request/BeginMessage.java

@@ -38,7 +38,7 @@ public BeginMessage( Bookmarks bookmarks, TransactionConfig config, AccessMode m
 
     public BeginMessage( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
     {
-        super( bookmarks, txTimeout, txMetadata, mode );
+        super( buildMetadata( bookmarks, txTimeout, txMetadata, mode ) );
     }
 
     @Override

driver/src/main/java/org/neo4j/driver/internal/messaging/request/RunWithMetadataMessage.java

@@ -24,25 +24,41 @@
 
 import org.neo4j.driver.internal.Bookmarks;
 import org.neo4j.driver.v1.AccessMode;
+import org.neo4j.driver.v1.Statement;
 import org.neo4j.driver.v1.TransactionConfig;
 import org.neo4j.driver.v1.Value;
 
+import static java.util.Collections.emptyMap;
+import static org.neo4j.driver.v1.Values.ofValue;
+
 public class RunWithMetadataMessage extends TransactionStartingMessage
 {
     public final static byte SIGNATURE = 0x10;
 
     private final String statement;
     private final Map<String,Value> parameters;
 
-    public RunWithMetadataMessage( String statement, Map<String,Value> parameters, Bookmarks bookmarks, TransactionConfig config, AccessMode mode )
+    public static RunWithMetadataMessage autoCommitTxRunMessage( Statement statement, TransactionConfig config, AccessMode mode,
+            Bookmarks bookmark )
+    {
+        return autoCommitTxRunMessage( statement.text(), statement.parameters().asMap( ofValue() ), config.timeout(), config.metadata(), mode, bookmark );
+    }
+
+    public static RunWithMetadataMessage autoCommitTxRunMessage( String statement, Map<String,Value> parameters, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode,
+            Bookmarks bookmark )
+    {
+        Map<String,Value> metadata = buildMetadata( bookmark, txTimeout, txMetadata, mode );
+        return new RunWithMetadataMessage( statement, parameters, metadata );
+    }
+
+    public static RunWithMetadataMessage explicitTxRunMessage( Statement statement )
     {
-        this( statement, parameters, bookmarks, config.timeout(), config.metadata(), mode );
+        return new RunWithMetadataMessage( statement.text(), statement.parameters().asMap( ofValue() ), emptyMap() );
     }
 
-    public RunWithMetadataMessage( String statement, Map<String,Value> parameters, Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata,
-            AccessMode mode )
+    public RunWithMetadataMessage( String statement, Map<String,Value> parameters, Map<String,Value> metadata )
     {
-        super( bookmarks, txTimeout, txMetadata, mode );
+        super( metadata );
         this.statement = statement;
         this.parameters = parameters;
     }

driver/src/main/java/org/neo4j/driver/internal/messaging/request/TransactionStartingMessage.java

@@ -28,6 +28,7 @@
 import org.neo4j.driver.v1.Value;
 
 import static java.util.Collections.emptyMap;
+import static java.util.Objects.requireNonNull;
 import static org.neo4j.driver.v1.Values.value;
 
 abstract class TransactionStartingMessage implements Message
@@ -40,17 +41,17 @@ abstract class TransactionStartingMessage implements Message
 
     final Map<String,Value> metadata;
 
-    TransactionStartingMessage( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
+    TransactionStartingMessage( Map<String,Value> metadata )
     {
-        this.metadata = buildMetadata( bookmarks, txTimeout, txMetadata, mode );
+        this.metadata = requireNonNull( metadata );
     }
 
     public final Map<String,Value> metadata()
     {
         return metadata;
     }
 
-    private static Map<String,Value> buildMetadata( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
+    static Map<String,Value> buildMetadata( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
     {
         boolean bookmarksPresent = bookmarks != null && !bookmarks.isEmpty();
         boolean txTimeoutPresent = txTimeout != null;

driver/src/main/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3.java

@@ -44,7 +44,6 @@
 import org.neo4j.driver.internal.messaging.request.BeginMessage;
 import org.neo4j.driver.internal.messaging.request.GoodbyeMessage;
 import org.neo4j.driver.internal.messaging.request.HelloMessage;
-import org.neo4j.driver.internal.messaging.request.RunWithMetadataMessage;
 import org.neo4j.driver.internal.spi.Connection;
 import org.neo4j.driver.internal.util.Futures;
 import org.neo4j.driver.internal.util.MetadataExtractor;
@@ -57,6 +56,8 @@
 import static org.neo4j.driver.internal.messaging.request.CommitMessage.COMMIT;
 import static org.neo4j.driver.internal.messaging.request.PullAllMessage.PULL_ALL;
 import static org.neo4j.driver.internal.messaging.request.RollbackMessage.ROLLBACK;
+import static org.neo4j.driver.internal.messaging.request.RunWithMetadataMessage.autoCommitTxRunMessage;
+import static org.neo4j.driver.internal.messaging.request.RunWithMetadataMessage.explicitTxRunMessage;
 import static org.neo4j.driver.v1.Values.ofValue;
 
 public class BoltProtocolV3 implements BoltProtocol
@@ -131,24 +132,25 @@ public CompletionStage<Void> rollbackTransaction( Connection connection )
     public CompletionStage<InternalStatementResultCursor> runInAutoCommitTransaction( Connection connection, Statement statement,
             BookmarksHolder bookmarksHolder, TransactionConfig config, boolean waitForRunResponse )
     {
-        return runStatement( connection, statement, bookmarksHolder, null, config, waitForRunResponse );
+        Message runMessage = autoCommitTxRunMessage( statement, config, connection.mode(), bookmarksHolder.getBookmarks() );
+        return runStatement( connection, statement, bookmarksHolder, null, waitForRunResponse, runMessage );
     }
 
     @Override
     public CompletionStage<InternalStatementResultCursor> runInExplicitTransaction( Connection connection, Statement statement, ExplicitTransaction tx,
             boolean waitForRunResponse )
     {
-        return runStatement( connection, statement, BookmarksHolder.NO_OP, tx, TransactionConfig.empty(), waitForRunResponse );
+        Message runMessage = explicitTxRunMessage( statement );
+        return runStatement( connection, statement, BookmarksHolder.NO_OP, tx, waitForRunResponse, runMessage );
     }
 
-    private static CompletionStage<InternalStatementResultCursor> runStatement( Connection connection, Statement statement,
-            BookmarksHolder bookmarksHolder, ExplicitTransaction tx, TransactionConfig config, boolean waitForRunResponse )
+    private static CompletionStage<InternalStatementResultCursor> runStatement( Connection connection, Statement statement, BookmarksHolder bookmarksHolder,
+            ExplicitTransaction tx, boolean waitForRunResponse, Message runMessage )
     {
         String query = statement.text();
         Map<String,Value> params = statement.parameters().asMap( ofValue() );
 
         CompletableFuture<Void> runCompletedFuture = new CompletableFuture<>();
-        Message runMessage = new RunWithMetadataMessage( query, params, bookmarksHolder.getBookmarks(), config, connection.mode() );
         RunResponseHandler runHandler = new RunResponseHandler( runCompletedFuture, METADATA_EXTRACTOR );
         PullAllResponseHandler pullAllHandler = newPullAllHandler( statement, runHandler, connection, bookmarksHolder, tx );
 

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

@@ -297,7 +297,7 @@ public static class ConfigBuilder
         private LoadBalancingStrategy loadBalancingStrategy = LoadBalancingStrategy.LEAST_CONNECTED;
         private int routingFailureLimit = RoutingSettings.DEFAULT.maxRoutingFailures();
         private long routingRetryDelayMillis = RoutingSettings.DEFAULT.retryTimeoutDelay();
-        private int connectionTimeoutMillis = (int) TimeUnit.SECONDS.toMillis( 5 );
+        private int connectionTimeoutMillis = (int) TimeUnit.SECONDS.toMillis( 30 );
         private RetrySettings retrySettings = RetrySettings.DEFAULT;
         private ServerAddressResolver resolver;
 
@@ -687,7 +687,7 @@ public ConfigBuilder withRoutingRetryDelay( long delay, TimeUnit unit )
          * Timeout value should be greater or equal to zero and represent a valid {@code int} value when converted to
          * {@link TimeUnit#MILLISECONDS milliseconds}.
          * <p>
-         * The default value of this parameter is {@code 5 SECONDS}.
+         * The default value of this parameter is {@code 30 SECONDS}.
          *
          * @param value the timeout duration
          * @param unit the unit in which duration is given

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

@@ -37,7 +37,8 @@
  * The <a href="https://tools.ietf.org/html/rfc3986">URI</a> passed to
  * this method determines the type of Driver created.
  * <br>
- * <table border="1" cellpadding="4" style="border-collapse: collapse" summary="Available schemes and drivers">
+ * <table border="1" style="border-collapse: collapse">
+ *     <caption>Available schemes and drivers</caption>
  *     <thead>
  *         <tr><th>URI Scheme</th><th>Driver</th></tr>
  *     </thead>

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

@@ -45,7 +45,8 @@
  * <p>
  * Driver logging API defines the following log levels: ERROR, INFO, WARN, DEBUG and TRACE. They are similar to levels defined by SLF4J but different from
  * log levels defined for {@link java.util.logging}. The following mapping takes place:
- * <table border="1" cellpadding="4" summary="Driver and JUL log levels">
+ * <table border="1" style="border-collapse: collapse">
+ * <caption>Driver and JUL log levels</caption>
  * <tr>
  * <th>Driver</th>
  * <th>java.util.logging</th>

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

@@ -117,7 +117,7 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue
     /**
      * If this value represents a list or map, test if the collection is empty.
      *
-     * @return <tt>true</tt> if size() is 0, otherwise <tt>false</tt>
+     * @return {@code true} if size() is 0, otherwise {@code false}
      */
     boolean isEmpty();
 
@@ -154,17 +154,17 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue
     boolean hasType( Type type );
 
     /**
-     * @return <tt>true</tt> if the value is a Boolean value and has the value True.
+     * @return {@code true} if the value is a Boolean value and has the value True.
      */
     boolean isTrue();
 
     /**
-     * @return <tt>true</tt> if the value is a Boolean value and has the value False.
+     * @return {@code true} if the value is a Boolean value and has the value False.
      */
     boolean isFalse();
 
     /**
-     * @return <tt>true</tt> if the value is a Null, otherwise <tt>false</tt>
+     * @return {@code true} if the value is a Null, otherwise {@code false}
      */
     boolean isNull();
 

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

@@ -679,7 +679,7 @@ public static Function<Value,List<Object>> ofList()
     }
 
     /**
-     * Converts values to {@link List} of <tt>T</tt>.
+     * Converts values to {@link List} of {@code T}.
      *
      * @param innerMap converter for the values inside the list
      * @param <T> the type of values inside the list

driver/src/main/java/org/neo4j/driver/v1/types/MapAccessor.java

@@ -46,7 +46,7 @@ public interface MapAccessor
      * Check if the list of keys contains the given key
      *
      * @param key the key
-     * @return <tt>true</tt> if this map keys contains the given key otherwise <tt>false</tt>
+     * @return {@code true} if this map keys contains the given key otherwise {@code false}
      */
     boolean containsKey( String key );
 

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

@@ -25,6 +25,7 @@
 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;
@@ -150,7 +151,7 @@ void shouldSendReadAccessModeInStatementMetadata() throws Exception
         StubServer server = StubServer.start( "hello_run_exit_read.script", 9001 );
 
         try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
-                Session session = driver.session( AccessMode.READ ) )
+              Session session = driver.session( AccessMode.READ ) )
         {
             List<String> names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() );
             assertEquals( asList( "Foo", "Bar" ), names );
@@ -167,7 +168,7 @@ void shouldNotSendWriteAccessModeInStatementMetadata() throws Exception
         StubServer server = StubServer.start( "hello_run_exit.script", 9001 );
 
         try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
-                Session session = driver.session( AccessMode.WRITE ) )
+              Session session = driver.session( AccessMode.WRITE ) )
         {
             List<String> names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() );
             assertEquals( asList( "Foo", "Bar" ), names );
@@ -209,36 +210,62 @@ 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", tx -> {
+            tx.success();
+            tx.close();
+        }, "Unable to commit" );
     }
 
     @Test
-    void shouldThrowCommitErrorWhenTransactionClosed() throws Exception
+    void shouldThrowRollbackErrorWhenTransactionRollback() throws Exception
     {
-        testTxCloseErrorPropagation( "commit_error.script", true, "Unable to commit" );
+        testTxCloseErrorPropagation( "rollback_error.script", tx -> {
+            tx.failure();
+            tx.close();
+        }, "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
     {
         StubServer server = StubServer.start( "database_shutdown.script", 9001 );
 
         try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
-                Session session = driver.session( "neo4j:bookmark:v1:tx0" );
+              Session session = driver.session( "neo4j:bookmark:v1:tx0" );
                 // has to enforce to flush BEGIN to have tx started.
                 Transaction transaction = session.beginTransaction() )
         {
@@ -260,7 +287,7 @@ void shouldThrowCorrectErrorOnCommitFailure() throws Throwable
         StubServer server = StubServer.start( "database_shutdown_at_commit.script", 9001 );
 
         try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
-                Session session = driver.session() )
+              Session session = driver.session() )
         {
             Transaction transaction = session.beginTransaction();
             StatementResult result = transaction.run( "CREATE (n {name:'Bob'})" );
@@ -276,41 +303,7 @@ void shouldThrowCorrectErrorOnCommitFailure() 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<Transaction> txAction, String expectedErrorMessage )
             throws Exception
     {
         StubServer server = StubServer.start( script, 9001 );
@@ -323,16 +316,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() );
             }