From a8882ab066de7f3b36354123dcef6b81c0c800db Mon Sep 17 00:00:00 2001 From: Nigel Small Date: Wed, 17 May 2017 20:44:21 +0100 Subject: [PATCH 1/2] Added method and test --- .../org/neo4j/driver/v1/GraphDatabase.java | 42 +++++++++++++++++++ .../v1/integration/CausalClusteringIT.java | 42 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java b/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java index 845919544a..ac215b9d4a 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java +++ b/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java @@ -19,10 +19,12 @@ package org.neo4j.driver.v1; import java.net.URI; +import java.util.List; import org.neo4j.driver.internal.DriverFactory; import org.neo4j.driver.internal.cluster.RoutingSettings; import org.neo4j.driver.internal.retry.RetrySettings; +import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; /** * Creates {@link Driver drivers}, optionally letting you {@link #driver(URI, Config)} to configure them. @@ -131,4 +133,44 @@ public static Driver driver( URI uri, AuthToken authToken, Config config ) return new DriverFactory().newInstance( uri, authToken, routingSettings, retrySettings, config ); } + + /** + * Try to create a bolt+routing driver from the first available address. + * This is wrapper for the {@link #driver} method that finds the first + * server to respond positively. + * + * @param addresses a list of server addresses for Neo4j instances + * @param authToken authentication to use, see {@link AuthTokens} + * @param config user defined configuration + * @return a new driver instance + */ + public static Driver routingDriverFromFirstAvailableAddress( List addresses, AuthToken authToken, Config config ) + { + for( String address: addresses ) + { + try + { + return driver( "bolt+routing://" + address, authToken, config ); + } + catch( ServiceUnavailableException e ) + { + // try the next one + } + } + throw new ServiceUnavailableException( "Failed to discover an available server" ); + } + + /** + * Try to create a bolt+routing driver from the first available address. + * This is wrapper for the {@link #driver} method that finds the first + * server to respond positively. + * + * @param addresses a list of server addresses for Neo4j instances + * @param authToken authentication to use, see {@link AuthTokens} + * @return a new driver instance + */ + public static Driver routingDriverFromFirstAvailableAddress( List addresses, AuthToken authToken ) + { + return routingDriverFromFirstAvailableAddress( addresses, authToken, Config.defaultConfig() ); + } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java index 0b502985e7..dbf8e552e9 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java @@ -22,6 +22,7 @@ import org.junit.Test; import java.net.URI; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; @@ -85,6 +86,16 @@ public void shouldExecuteReadAndWritesWhenDriverSuppliedWithAddressOfLeader() th assertEquals( 1, count ); } + @Test + public void shouldExecuteReadAndWritesWhenRouterIsDiscovered() throws Exception + { + Cluster cluster = clusterRule.getCluster(); + + int count = executeWriteAndReadThroughBoltOnFirstAvailableAddress( cluster.anyReadReplica(), cluster.leader() ); + + assertEquals( 1, count ); + } + @Test public void shouldExecuteReadAndWritesWhenDriverSuppliedWithAddressOfFollower() throws Exception { @@ -446,6 +457,19 @@ private int executeWriteAndReadThroughBolt( ClusterMember member ) throws Timeou } } + private int executeWriteAndReadThroughBoltOnFirstAvailableAddress( ClusterMember... members ) throws TimeoutException, InterruptedException + { + List addresses = new ArrayList<>( members.length ); + for ( ClusterMember member : members ) + { + addresses.add( member.getRoutingUri().getAuthority() ); + } + try ( Driver driver = discoverDriver( addresses ) ) + { + return inExpirableSession( driver, createWritableSession( null ), executeWriteAndRead() ); + } + } + private Function createSession() { return new Function() @@ -592,6 +616,24 @@ public Logger getLog( String name ) return GraphDatabase.driver( boltUri, clusterRule.getDefaultAuthToken(), config ); } + private Driver discoverDriver( List addresses ) + { + Logging devNullLogging = new Logging() + { + @Override + public Logger getLog( String name ) + { + return DevNullLogger.DEV_NULL_LOGGER; + } + }; + + Config config = Config.build() + .withLogging( devNullLogging ) + .toConfig(); + + return GraphDatabase.routingDriverFromFirstAvailableAddress( addresses, clusterRule.getDefaultAuthToken(), config ); + } + private static void createNodesInDifferentThreads( int count, final Driver driver ) throws Exception { final CountDownLatch beforeRunLatch = new CountDownLatch( count ); From 2dab779671a43aa32f2025c288621aff8234da28 Mon Sep 17 00:00:00 2001 From: lutovich Date: Tue, 23 May 2017 15:05:30 +0200 Subject: [PATCH 2/2] Address review comments * use shorter name `routingDriver` instead of `routingDriverFromFirstAvailableAddress` * no overloads * take `Iterable` of server URIs and verify 'bolt+routing' scheme * introduce constants for URIs schemes --- .../neo4j/driver/internal/DriverFactory.java | 8 ++-- .../org/neo4j/driver/v1/GraphDatabase.java | 41 ++++++++++--------- .../v1/integration/CausalClusteringIT.java | 20 +++------ .../v1/util/cc/LocalOrRemoteClusterRule.java | 4 +- 4 files changed, 36 insertions(+), 37 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 index 4e4c0f380e..6397e826ec 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java +++ b/driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java @@ -47,10 +47,12 @@ import static java.lang.String.format; import static org.neo4j.driver.internal.security.SecurityPlan.insecure; -import static org.neo4j.driver.v1.Config.EncryptionLevel.REQUIRED; public class DriverFactory { + public static final String BOLT_URI_SCHEME = "bolt"; + public static final String BOLT_ROUTING_URI_SCHEME = "bolt+routing"; + public final Driver newInstance( URI uri, AuthToken authToken, RoutingSettings routingSettings, RetrySettings retrySettings, Config config ) { @@ -86,10 +88,10 @@ private Driver createDriver( URI uri, BoltServerAddress address, ConnectionPool String scheme = uri.getScheme().toLowerCase(); switch ( scheme ) { - case "bolt": + case BOLT_URI_SCHEME: assertNoRoutingContext( uri, routingSettings ); return createDirectDriver( address, connectionPool, config, securityPlan, retryLogic ); - case "bolt+routing": + case BOLT_ROUTING_URI_SCHEME: return createRoutingDriver( address, connectionPool, config, routingSettings, securityPlan, retryLogic ); default: throw new ClientException( format( "Unsupported URI scheme: %s", scheme ) ); diff --git a/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java b/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java index ac215b9d4a..2536736638 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java +++ b/driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java @@ -19,13 +19,14 @@ package org.neo4j.driver.v1; import java.net.URI; -import java.util.List; import org.neo4j.driver.internal.DriverFactory; import org.neo4j.driver.internal.cluster.RoutingSettings; import org.neo4j.driver.internal.retry.RetrySettings; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; +import static org.neo4j.driver.internal.DriverFactory.BOLT_ROUTING_URI_SCHEME; + /** * Creates {@link Driver drivers}, optionally letting you {@link #driver(URI, Config)} to configure them. * @see Driver @@ -135,42 +136,44 @@ public static Driver driver( URI uri, AuthToken authToken, Config config ) } /** - * Try to create a bolt+routing driver from the first available address. - * This is wrapper for the {@link #driver} method that finds the first + * Try to create a bolt+routing driver from the first available address. + * This is wrapper for the {@link #driver} method that finds the first * server to respond positively. * - * @param addresses a list of server addresses for Neo4j instances + * @param routingUris an {@link Iterable} of server {@link URI}s for Neo4j instances. All given URIs should + * have 'bolt+routing' scheme. * @param authToken authentication to use, see {@link AuthTokens} * @param config user defined configuration * @return a new driver instance */ - public static Driver routingDriverFromFirstAvailableAddress( List addresses, AuthToken authToken, Config config ) + public static Driver routingDriver( Iterable routingUris, AuthToken authToken, Config config ) { - for( String address: addresses ) + assertRoutingUris( routingUris ); + + for ( URI uri : routingUris ) { try { - return driver( "bolt+routing://" + address, authToken, config ); + return driver( uri, authToken, config ); } - catch( ServiceUnavailableException e ) + catch ( ServiceUnavailableException e ) { // try the next one } } + throw new ServiceUnavailableException( "Failed to discover an available server" ); } - /** - * Try to create a bolt+routing driver from the first available address. - * This is wrapper for the {@link #driver} method that finds the first - * server to respond positively. - * - * @param addresses a list of server addresses for Neo4j instances - * @param authToken authentication to use, see {@link AuthTokens} - * @return a new driver instance - */ - public static Driver routingDriverFromFirstAvailableAddress( List addresses, AuthToken authToken ) + private static void assertRoutingUris( Iterable uris ) { - return routingDriverFromFirstAvailableAddress( addresses, authToken, Config.defaultConfig() ); + for ( URI uri : uris ) + { + if ( !BOLT_ROUTING_URI_SCHEME.equals( uri.getScheme() ) ) + { + throw new IllegalArgumentException( + "Illegal URI scheme, expected '" + BOLT_ROUTING_URI_SCHEME + "' in '" + uri + "'" ); + } + } } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java index dbf8e552e9..42ab5ed4fe 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/CausalClusteringIT.java @@ -67,6 +67,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.v1.Values.parameters; public class CausalClusteringIT @@ -459,10 +460,10 @@ private int executeWriteAndReadThroughBolt( ClusterMember member ) throws Timeou private int executeWriteAndReadThroughBoltOnFirstAvailableAddress( ClusterMember... members ) throws TimeoutException, InterruptedException { - List addresses = new ArrayList<>( members.length ); + List addresses = new ArrayList<>( members.length ); for ( ClusterMember member : members ) { - addresses.add( member.getRoutingUri().getAuthority() ); + addresses.add( member.getRoutingUri() ); } try ( Driver driver = discoverDriver( addresses ) ) { @@ -616,22 +617,13 @@ public Logger getLog( String name ) return GraphDatabase.driver( boltUri, clusterRule.getDefaultAuthToken(), config ); } - private Driver discoverDriver( List addresses ) + private Driver discoverDriver( List routingUris ) { - Logging devNullLogging = new Logging() - { - @Override - public Logger getLog( String name ) - { - return DevNullLogger.DEV_NULL_LOGGER; - } - }; - Config config = Config.build() - .withLogging( devNullLogging ) + .withLogging( DEV_NULL_LOGGING ) .toConfig(); - return GraphDatabase.routingDriverFromFirstAvailableAddress( addresses, clusterRule.getDefaultAuthToken(), config ); + return GraphDatabase.routingDriver( routingUris, clusterRule.getDefaultAuthToken(), config ); } private static void createNodesInDifferentThreads( int count, final Driver driver ) throws Exception diff --git a/driver/src/test/java/org/neo4j/driver/v1/util/cc/LocalOrRemoteClusterRule.java b/driver/src/test/java/org/neo4j/driver/v1/util/cc/LocalOrRemoteClusterRule.java index 68e51bb2b4..be28cc49b5 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/util/cc/LocalOrRemoteClusterRule.java +++ b/driver/src/test/java/org/neo4j/driver/v1/util/cc/LocalOrRemoteClusterRule.java @@ -25,6 +25,8 @@ import org.neo4j.driver.v1.AuthToken; import org.neo4j.driver.v1.AuthTokens; +import static org.neo4j.driver.internal.DriverFactory.BOLT_ROUTING_URI_SCHEME; + public class LocalOrRemoteClusterRule extends ExternalResource { private static final String CLUSTER_URI_SYSTEM_PROPERTY_NAME = "externalClusterUri"; @@ -88,7 +90,7 @@ private static void assertValidSystemPropertiesDefined() "Both cluster uri and 'neo4j' user password system properties should be set. " + "Uri: '" + uri + "', Password: '" + password + "'" ); } - if ( uri != null && !"bolt+routing".equals( uri.getScheme() ) ) + if ( uri != null && !BOLT_ROUTING_URI_SCHEME.equals( uri.getScheme() ) ) { throw new IllegalStateException( "CLuster uri should have bolt+routing scheme: '" + uri + "'" ); }