Minor cleanups

neo4j · Jul 5, 2017 · fcc578d · fcc578d
1 parent 5ea5cd1
commit fcc578d
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 53 deletions.
diff --git a/driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java b/driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java
@@ -150,19 +150,19 @@ protected LoadBalancer createLoadBalancer( BoltServerAddress address, Connection
             RoutingSettings routingSettings )
     {
         return new LoadBalancer( address, routingSettings, connectionPool, createClock(), config.logging(),
-                loadBalancingStrategy( config, connectionPool ) );
+                createLoadBalancingStrategy( config, connectionPool ) );
     }
 
-    private LoadBalancingStrategy loadBalancingStrategy( Config config,
-            ConnectionPool connectionPool )
+    private LoadBalancingStrategy createLoadBalancingStrategy( Config config, ConnectionPool connectionPool )
     {
-        if ( config.loadBalancingStrategy() == Config.LoadBalancingStrategy.ROUND_ROBIN )
+        switch ( config.loadBalancingStrategy() )
         {
+        case ROUND_ROBIN:
             return new RoundRobinLoadBalancingStrategy();
-        }
-        else
-        {
+        case LEAST_CONNECTED:
             return new LeastConnectedLoadBalancingStrategy( connectionPool );
+        default:
+            throw new IllegalArgumentException( "Unknown load balancing strategy: " + config.loadBalancingStrategy() );
         }
     }
 

diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java
@@ -55,20 +55,18 @@ public class LoadBalancer implements ConnectionProvider, RoutingErrorHandler, Au
     public LoadBalancer( BoltServerAddress initialRouter, RoutingSettings settings, ConnectionPool connections,
             Clock clock, Logging logging, LoadBalancingStrategy loadBalancingStrategy )
     {
-        this( initialRouter, settings, connections, new ClusterRoutingTable( clock, initialRouter ), clock,
-                logging.getLog( LOAD_BALANCER_LOG_NAME ), loadBalancingStrategy );
+        this( connections, new ClusterRoutingTable( clock, initialRouter ),
+                createRediscovery( initialRouter, settings, clock, logging ), loadBalancerLogger( logging ),
+                loadBalancingStrategy );
     }
 
-    private LoadBalancer( BoltServerAddress initialRouter, RoutingSettings settings, ConnectionPool connections,
-            RoutingTable routingTable, Clock clock, Logger log,
-            LoadBalancingStrategy loadBalancingStrategy )
+    // Used only in testing
+    public LoadBalancer( ConnectionPool connections, RoutingTable routingTable, Rediscovery rediscovery, Logger log )
     {
-        this( connections, routingTable, createRediscovery( initialRouter, settings, clock, log ), log,
-                loadBalancingStrategy );
+        this( connections, routingTable, rediscovery, log, new LeastConnectedLoadBalancingStrategy( connections ) );
     }
 
-    // Used only in testing
-    public LoadBalancer( ConnectionPool connections, RoutingTable routingTable, Rediscovery rediscovery, Logger log,
+    private LoadBalancer( ConnectionPool connections, RoutingTable routingTable, Rediscovery rediscovery, Logger log,
             LoadBalancingStrategy loadBalancingStrategy )
     {
         this.connections = connections;
@@ -186,13 +184,19 @@ private BoltServerAddress selectAddress( AccessMode mode, AddressSet servers )
     }
 
     private static Rediscovery createRediscovery( BoltServerAddress initialRouter, RoutingSettings settings,
-            Clock clock, Logger log )
+            Clock clock, Logging logging )
     {
+        Logger log = loadBalancerLogger( logging );
         ClusterCompositionProvider clusterComposition =
                 new RoutingProcedureClusterCompositionProvider( clock, log, settings );
         return new Rediscovery( initialRouter, settings, clock, log, clusterComposition, new DnsResolver( log ) );
     }
 
+    private static Logger loadBalancerLogger( Logging logging )
+    {
+        return logging.getLog( LOAD_BALANCER_LOG_NAME );
+    }
+
     private static RuntimeException unknownMode( AccessMode mode )
     {
         return new IllegalArgumentException( "Mode '" + mode + "' is not supported" );

diff --git a/driver/src/main/java/org/neo4j/driver/v1/Config.java b/driver/src/main/java/org/neo4j/driver/v1/Config.java
@@ -94,16 +94,6 @@ private Config( ConfigBuilder builder)
         this.loadBalancingStrategy = builder.loadBalancingStrategy;
     }
 
-    /**
-     * Load Balancing Strategy
-     *
-     * @return load balancing strategy to use
-     */
-    public LoadBalancingStrategy loadBalancingStrategy()
-    {
-        return loadBalancingStrategy;
-    }
-
     /**
      * Logging provider
      * @return the logging provider to use
@@ -186,6 +176,17 @@ public TrustStrategy trustStrategy()
         return trustStrategy;
     }
 
+    /**
+     * Load balancing strategy
+     *
+     * @return the strategy to use
+     */
+    @Experimental
+    public LoadBalancingStrategy loadBalancingStrategy()
+    {
+        return loadBalancingStrategy;
+    }
+
     /**
      * Return a {@link ConfigBuilder} instance
      * @return a {@link ConfigBuilder} instance
@@ -245,11 +246,10 @@ public ConfigBuilder withLogging( Logging logging )
         }
 
         /**
-         * Provide an alternative load balancing implementation for the driver to use. By default we use
+         * Provide an alternative load balancing strategy for the routing driver to use. By default we use
          * {@link LoadBalancingStrategy#LEAST_CONNECTED}.
          * <p>
-         * We are experimnenting with different strategies before sticking to one. This could be removed in the next
-         * minor version
+         * <b>Note:</b> We are experimenting with different strategies. This could be removed in the next minor version.
          *
          * @param loadBalancingStrategy the strategy to use
          * @return this builder

diff --git a/...test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java b/...test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java
@@ -30,7 +30,6 @@
 import java.util.HashSet;
 import java.util.List;
 
-import org.neo4j.driver.internal.cluster.loadbalancing.LeastConnectedLoadBalancingStrategy;
 import org.neo4j.driver.internal.cluster.loadbalancing.LoadBalancer;
 import org.neo4j.driver.internal.net.BoltServerAddress;
 import org.neo4j.driver.internal.net.pooling.PoolSettings;
@@ -326,8 +325,7 @@ private static LoadBalancer newLoadBalancer( ClusterComposition clusterCompositi
     {
         Rediscovery rediscovery = mock( Rediscovery.class );
         when( rediscovery.lookupClusterComposition( routingTable, connectionPool ) ).thenReturn( clusterComposition );
-        return new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connectionPool ) );
+        return new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER );
     }
 
     private interface ConnectionMethod

diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancerTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancerTest.java
@@ -93,8 +93,7 @@ public void ensureRoutingShouldUpdateRoutingTableAndPurgeConnectionPoolWhenStale
         when( routingTable.update( any( ClusterComposition.class ) ) ).thenReturn( set );
 
         // when
-        LoadBalancer balancer = new LoadBalancer( conns, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( conns ) );
+        LoadBalancer balancer = new LoadBalancer( conns, routingTable, rediscovery, DEV_NULL_LOGGER );
 
         // then
         assertNotNull( balancer );
@@ -110,8 +109,7 @@ public void shouldRefreshRoutingTableOnInitialization() throws Exception
         // given & when
         final AtomicInteger refreshRoutingTableCounter = new AtomicInteger( 0 );
         LoadBalancer balancer = new LoadBalancer( mock( ConnectionPool.class ), mock( RoutingTable.class ),
-                mock( Rediscovery.class ), DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( mock( ConnectionPool.class ) ) )
+                mock( Rediscovery.class ), DEV_NULL_LOGGER )
         {
             @Override
             synchronized void refreshRoutingTable()
@@ -165,8 +163,7 @@ public void shouldForgetAddressAndItsConnectionsOnServiceUnavailableWhileClosing
         RoutingTable routingTable = mock( RoutingTable.class );
         ConnectionPool connectionPool = mock( ConnectionPool.class );
         Rediscovery rediscovery = mock( Rediscovery.class );
-        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connectionPool ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER );
         BoltServerAddress address = new BoltServerAddress( "host", 42 );
 
         PooledConnection connection = newConnectionWithFailingSync( address );
@@ -200,8 +197,7 @@ public void shouldForgetAddressAndItsConnectionsOnServiceUnavailableWhileClosing
         PooledConnection connectionWithFailingSync = newConnectionWithFailingSync( address );
         when( connectionPool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connectionWithFailingSync );
         Rediscovery rediscovery = mock( Rediscovery.class );
-        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connectionPool ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER );
 
         Session session = newSession( loadBalancer );
         // begin transaction to make session obtain a connection
@@ -247,8 +243,7 @@ public void shouldThrowWhenRediscoveryReturnsNoSuitableServers()
         when( routingTable.readers() ).thenReturn( new AddressSet() );
         when( routingTable.writers() ).thenReturn( new AddressSet() );
 
-        LoadBalancer loadBalancer = new LoadBalancer( connections, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connections ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connections, routingTable, rediscovery, DEV_NULL_LOGGER );
 
         try
         {
@@ -288,8 +283,7 @@ public void shouldSelectLeastConnectedAddress()
 
         Rediscovery rediscovery = mock( Rediscovery.class );
 
-        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connectionPool ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER );
 
         Set<BoltServerAddress> seenAddresses = new HashSet<>();
         for ( int i = 0; i < 10; i++ )
@@ -315,8 +309,7 @@ public void shouldRoundRobinWhenNoActiveConnections()
 
         Rediscovery rediscovery = mock( Rediscovery.class );
 
-        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connectionPool ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connectionPool, routingTable, rediscovery, DEV_NULL_LOGGER );
 
         Set<BoltServerAddress> seenAddresses = new HashSet<>();
         for ( int i = 0; i < 10; i++ )
@@ -337,8 +330,7 @@ private void testRediscoveryWhenStale( AccessMode mode )
         RoutingTable routingTable = newStaleRoutingTableMock( mode );
         Rediscovery rediscovery = newRediscoveryMock();
 
-        LoadBalancer loadBalancer = new LoadBalancer( connections, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connections ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connections, routingTable, rediscovery, DEV_NULL_LOGGER );
         verify( rediscovery ).lookupClusterComposition( routingTable, connections );
 
         assertNotNull( loadBalancer.acquireConnection( mode ) );
@@ -354,8 +346,7 @@ private void testNoRediscoveryWhenNotStale( AccessMode staleMode, AccessMode not
         RoutingTable routingTable = newStaleRoutingTableMock( staleMode );
         Rediscovery rediscovery = newRediscoveryMock();
 
-        LoadBalancer loadBalancer = new LoadBalancer( connections, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connections ) );
+        LoadBalancer loadBalancer = new LoadBalancer( connections, routingTable, rediscovery, DEV_NULL_LOGGER );
         verify( rediscovery ).lookupClusterComposition( routingTable, connections );
 
         assertNotNull( loadBalancer.acquireConnection( notStaleMode ) );
@@ -388,8 +379,7 @@ private LoadBalancer setupLoadBalancer( PooledConnection writerConn, PooledConne
         when( routingTable.readers() ).thenReturn( readerAddrs );
         when( routingTable.writers() ).thenReturn( writerAddrs );
 
-        return new LoadBalancer( connPool, routingTable, rediscovery, DEV_NULL_LOGGER,
-                new LeastConnectedLoadBalancingStrategy( connPool ) );
+        return new LoadBalancer( connPool, routingTable, rediscovery, DEV_NULL_LOGGER );
     }
 
     private static Session newSession( LoadBalancer loadBalancer )