From 559db5af8154311ee6c564ea452ef34f163decb5 Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 30 Mar 2017 00:25:53 +0200 Subject: [PATCH] Routing context in URI Routing context is a map of string key-value pairs. It is send to the database when calling `dbms.cluster.routing.getRoutingTable` procedure as it's single argument. This is only supported by 3.2+ servers. Previously routing context was defined on the config. It is problematic because routing context in `bolt+routing` specific and thus makes no sense for single instance database. This commit makes routing context part of the `bolt+routing` URI. It lives in the query part of the URI as regular query parameters like: `bolt+routing://localhost:7687/?policy=my_policy®ion=china`. Clustering-specific configuration method is removed. --- .../neo4j/driver/internal/DriverFactory.java | 24 +++- .../driver/internal/cluster/LoadBalancer.java | 2 +- .../driver/internal/cluster/Rediscovery.java | 4 +- .../internal/cluster/RoutingContext.java | 86 ++++++++++++ ...gProcedureClusterCompositionProvider.java} | 35 ++--- ...unner.java => RoutingProcedureRunner.java} | 23 ++-- .../internal/cluster/RoutingSettings.java | 37 +++++- .../main/java/org/neo4j/driver/v1/Config.java | 24 +--- .../internal/RoutingDriverBoltKitTest.java | 63 +++++++++ .../driver/internal/RoutingDriverTest.java | 4 +- .../ClusterCompositionProviderTest.java | 35 +++-- .../internal/cluster/RoutingContextTest.java | 125 ++++++++++++++++++ ...t.java => RoutingProcedureRunnerTest.java} | 44 +++--- .../neo4j/driver/v1/GraphDatabaseTest.java | 14 ++ .../v1/integration/CausalClusteringIT.java | 3 +- .../driver/v1/integration/SessionIT.java | 4 +- .../test/resources/get_routing_table.script | 17 +++ .../get_routing_table_with_context.script | 16 +++ .../rediscover_and_read_with_init.script | 16 +++ 19 files changed, 468 insertions(+), 108 deletions(-) create mode 100644 driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingContext.java rename driver/src/main/java/org/neo4j/driver/internal/cluster/{GetServersProcedureClusterCompositionProvider.java => RoutingProcedureClusterCompositionProvider.java} (69%) rename driver/src/main/java/org/neo4j/driver/internal/cluster/{GetServersProcedureRunner.java => RoutingProcedureRunner.java} (75%) create mode 100644 driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingContextTest.java rename driver/src/test/java/org/neo4j/driver/internal/cluster/{GetServersProcedureRunnerTest.java => RoutingProcedureRunnerTest.java} (66%) create mode 100644 driver/src/test/resources/get_routing_table.script create mode 100644 driver/src/test/resources/get_routing_table_with_context.script create mode 100644 driver/src/test/resources/rediscover_and_read_with_init.script 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 54d8bd64b6..f3c038777e 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java +++ b/driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java @@ -23,6 +23,7 @@ import java.security.GeneralSecurityException; import org.neo4j.driver.internal.cluster.LoadBalancer; +import org.neo4j.driver.internal.cluster.RoutingContext; import org.neo4j.driver.internal.cluster.RoutingSettings; import org.neo4j.driver.internal.net.BoltServerAddress; import org.neo4j.driver.internal.net.SocketConnector; @@ -54,14 +55,14 @@ public final Driver newInstance( URI uri, AuthToken authToken, RoutingSettings r RetrySettings retrySettings, Config config ) { BoltServerAddress address = BoltServerAddress.from( uri ); + RoutingSettings newRoutingSettings = routingSettings.withRoutingContext( new RoutingContext( uri ) ); SecurityPlan securityPlan = createSecurityPlan( address, config ); ConnectionPool connectionPool = createConnectionPool( authToken, securityPlan, config ); RetryLogic retryLogic = createRetryLogic( retrySettings, config.logging() ); try { - return createDriver( address, uri.getScheme(), connectionPool, config, routingSettings, securityPlan, - retryLogic ); + return createDriver( uri, address, connectionPool, config, newRoutingSettings, securityPlan, retryLogic ); } catch ( Throwable driverError ) { @@ -78,12 +79,15 @@ public final Driver newInstance( URI uri, AuthToken authToken, RoutingSettings r } } - private Driver createDriver( BoltServerAddress address, String scheme, ConnectionPool connectionPool, - Config config, RoutingSettings routingSettings, SecurityPlan securityPlan, RetryLogic retryLogic ) + private Driver createDriver( URI uri, BoltServerAddress address, ConnectionPool connectionPool, + Config config, RoutingSettings routingSettings, SecurityPlan securityPlan, + RetryLogic retryLogic ) { - switch ( scheme.toLowerCase() ) + String scheme = uri.getScheme().toLowerCase(); + switch ( scheme ) { case "bolt": + assertNoRoutingContext( uri, routingSettings ); return createDirectDriver( address, connectionPool, config, securityPlan, retryLogic ); case "bolt+routing": return createRoutingDriver( address, connectionPool, config, routingSettings, securityPlan, retryLogic ); @@ -260,4 +264,14 @@ private static SecurityPlan createSecurityPlanImpl( BoltServerAddress address, C return insecure(); } } + + private static void assertNoRoutingContext( URI uri, RoutingSettings routingSettings ) + { + RoutingContext routingContext = routingSettings.routingContext(); + if ( routingContext.isDefined() ) + { + throw new IllegalArgumentException( + "Routing parameters are not supported with scheme 'bolt'. Given URI: '" + uri + "'" ); + } + } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java index 8d9e0fbee0..e88b84b9ec 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java @@ -157,7 +157,7 @@ private static Rediscovery createRediscovery( BoltServerAddress initialRouter, R Clock clock, Logger log ) { ClusterCompositionProvider clusterComposition = - new GetServersProcedureClusterCompositionProvider( clock, log, settings ); + new RoutingProcedureClusterCompositionProvider( clock, log, settings ); return new Rediscovery( initialRouter, settings, clock, log, clusterComposition, new DnsResolver( log ) ); } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java index baddf7abb9..2958a9b0c9 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java @@ -59,7 +59,7 @@ public ClusterComposition lookupClusterComposition( ConnectionPool connections, { int failures = 0; - for ( long start = clock.millis(), delay = 0; ; delay = Math.max( settings.retryTimeoutDelay, delay * 2 ) ) + for ( long start = clock.millis(), delay = 0; ; delay = Math.max( settings.retryTimeoutDelay(), delay * 2 ) ) { long waitTime = start + delay - clock.millis(); sleep( waitTime ); @@ -71,7 +71,7 @@ public ClusterComposition lookupClusterComposition( ConnectionPool connections, return composition; } - if ( ++failures >= settings.maxRoutingFailures ) + if ( ++failures >= settings.maxRoutingFailures() ) { throw new ServiceUnavailableException( NO_ROUTERS_AVAILABLE ); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingContext.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingContext.java new file mode 100644 index 0000000000..b473a6275f --- /dev/null +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingContext.java @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal.cluster; + +import java.net.URI; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.unmodifiableMap; + +public class RoutingContext +{ + public static final RoutingContext EMPTY = new RoutingContext(); + + private final Map context; + + private RoutingContext() + { + this.context = emptyMap(); + } + + public RoutingContext( URI uri ) + { + this.context = unmodifiableMap( parseParameters( uri ) ); + } + + public boolean isDefined() + { + return !context.isEmpty(); + } + + public Map asMap() + { + return context; + } + + private static Map parseParameters( URI uri ) + { + String query = uri.getQuery(); + + if ( query == null || query.isEmpty() ) + { + return emptyMap(); + } + + Map parameters = new HashMap<>(); + String[] pairs = query.split( "&" ); + for ( String pair : pairs ) + { + String[] keyValue = pair.split( "=" ); + if ( keyValue.length != 2 ) + { + throw new IllegalArgumentException( + "Invalid parameters: '" + pair + "' in URI '" + uri + "'" ); + } + + String key = keyValue[0]; + String value = keyValue[1]; + String previousValue = parameters.put( key, value ); + + if ( previousValue != null ) + { + throw new IllegalArgumentException( + "Duplicated query parameters with key '" + key + "' in URI '" + uri + "'" ); + } + } + return parameters; + } +} diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/GetServersProcedureClusterCompositionProvider.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureClusterCompositionProvider.java similarity index 69% rename from driver/src/main/java/org/neo4j/driver/internal/cluster/GetServersProcedureClusterCompositionProvider.java rename to driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureClusterCompositionProvider.java index b5e0211b63..c6b9f5f732 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/GetServersProcedureClusterCompositionProvider.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureClusterCompositionProvider.java @@ -24,6 +24,7 @@ import org.neo4j.driver.internal.util.Clock; import org.neo4j.driver.v1.Logger; import org.neo4j.driver.v1.Record; +import org.neo4j.driver.v1.Statement; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.ProtocolException; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; @@ -31,21 +32,20 @@ import static java.lang.String.format; -public class GetServersProcedureClusterCompositionProvider implements ClusterCompositionProvider +public class RoutingProcedureClusterCompositionProvider implements ClusterCompositionProvider { - - private final String PROTOCOL_ERROR_MESSAGE = "Failed to parse `%s' result received from server due to "; + private static final String PROTOCOL_ERROR_MESSAGE = "Failed to parse '%s' result received from server due to "; private final Clock clock; private final Logger log; - private final GetServersProcedureRunner getServersRunner; + private final RoutingProcedureRunner getServersRunner; - public GetServersProcedureClusterCompositionProvider( Clock clock, Logger log, RoutingSettings settings ) + public RoutingProcedureClusterCompositionProvider( Clock clock, Logger log, RoutingSettings settings ) { - this( clock, log, new GetServersProcedureRunner( settings.routingParameters ) ); + this( clock, log, new RoutingProcedureRunner( settings.routingContext() ) ); } - GetServersProcedureClusterCompositionProvider( Clock clock, Logger log, GetServersProcedureRunner getServersRunner ) + RoutingProcedureClusterCompositionProvider( Clock clock, Logger log, RoutingProcedureRunner getServersRunner ) { this.clock = clock; this.log = log; @@ -67,7 +67,7 @@ public ClusterCompositionResponse getClusterComposition( Connection connection ) return new ClusterCompositionResponse.Failure( new ServiceUnavailableException( format( "Failed to run '%s' on server. " + "Please make sure that there is a Neo4j 3.1+ causal cluster up running.", - getServersRunner.procedureCalled() ), e + invokedProcedureString() ), e ) ); } @@ -78,9 +78,8 @@ public ClusterCompositionResponse getClusterComposition( Connection connection ) if ( records.size() != 1 ) { return new ClusterCompositionResponse.Failure( new ProtocolException( format( - PROTOCOL_ERROR_MESSAGE + - "records received '%s' is too few or too many.", getServersRunner.procedureCalled(), - records.size() ) ) ); + PROTOCOL_ERROR_MESSAGE + "records received '%s' is too few or too many.", + invokedProcedureString(), records.size() ) ) ); } // failed to parse the record @@ -92,19 +91,25 @@ public ClusterCompositionResponse getClusterComposition( Connection connection ) catch ( ValueException e ) { return new ClusterCompositionResponse.Failure( new ProtocolException( format( - PROTOCOL_ERROR_MESSAGE + - "unparsable record received.", getServersRunner.procedureCalled() ), e ) ); + PROTOCOL_ERROR_MESSAGE + "unparsable record received.", + invokedProcedureString() ), e ) ); } // the cluster result is not a legal reply if ( !cluster.hasRoutersAndReaders() ) { return new ClusterCompositionResponse.Failure( new ProtocolException( format( - PROTOCOL_ERROR_MESSAGE + - "no router or reader found in response.", getServersRunner.procedureCalled() ) ) ); + PROTOCOL_ERROR_MESSAGE + "no router or reader found in response.", + invokedProcedureString() ) ) ); } // all good return new ClusterCompositionResponse.Success( cluster ); } + + private String invokedProcedureString() + { + Statement statement = getServersRunner.invokedProcedure(); + return statement.text() + " " + statement.parameters(); + } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/GetServersProcedureRunner.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunner.java similarity index 75% rename from driver/src/main/java/org/neo4j/driver/internal/cluster/GetServersProcedureRunner.java rename to driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunner.java index 9408b96ec5..7b9d0da22c 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/GetServersProcedureRunner.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunner.java @@ -20,7 +20,6 @@ package org.neo4j.driver.internal.cluster; import java.util.List; -import java.util.Map; import org.neo4j.driver.internal.NetworkSession; import org.neo4j.driver.internal.spi.Connection; @@ -32,33 +31,33 @@ import static org.neo4j.driver.internal.util.ServerVersion.version; import static org.neo4j.driver.v1.Values.parameters; -public class GetServersProcedureRunner +public class RoutingProcedureRunner { static final String GET_SERVERS = "dbms.cluster.routing.getServers"; static final String GET_ROUTING_TABLE_PARAM = "context"; static final String GET_ROUTING_TABLE = "dbms.cluster.routing.getRoutingTable({" + GET_ROUTING_TABLE_PARAM + "})"; - private final Map routingContext; - private Statement procedureCalled; + private final RoutingContext context; + private Statement invokedProcedure; - public GetServersProcedureRunner( Map context ) + public RoutingProcedureRunner( RoutingContext context ) { - this.routingContext = context; + this.context = context; } public List run( Connection connection ) { if( version( connection.server().version() ).greaterThanOrEqual( v3_2_0 ) ) { - procedureCalled = new Statement( "CALL " + GET_ROUTING_TABLE, - parameters(GET_ROUTING_TABLE_PARAM, routingContext ) ); + invokedProcedure = new Statement( "CALL " + GET_ROUTING_TABLE, + parameters( GET_ROUTING_TABLE_PARAM, context.asMap() ) ); } else { - procedureCalled = new Statement("CALL " + GET_SERVERS ); + invokedProcedure = new Statement( "CALL " + GET_SERVERS ); } - return runProcedure( connection, procedureCalled ); + return runProcedure( connection, invokedProcedure ); } List runProcedure( Connection connection, Statement procedure ) @@ -66,8 +65,8 @@ List runProcedure( Connection connection, Statement procedure ) return NetworkSession.run( connection, procedure, NO_OP ).list(); } - Statement procedureCalled() + Statement invokedProcedure() { - return procedureCalled; + return invokedProcedure; } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java index 5da014f79f..2555172d0c 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java @@ -18,18 +18,41 @@ */ package org.neo4j.driver.internal.cluster; -import java.util.Map; - public class RoutingSettings { - final int maxRoutingFailures; - final long retryTimeoutDelay; - final Map routingParameters; + private final int maxRoutingFailures; + private final long retryTimeoutDelay; + private final RoutingContext routingContext; + + public RoutingSettings( int maxRoutingFailures, long retryTimeoutDelay ) + { + this( maxRoutingFailures, retryTimeoutDelay, RoutingContext.EMPTY ); + } - public RoutingSettings( int maxRoutingFailures, long retryTimeoutDelay, Map routingParameters ) + public RoutingSettings( int maxRoutingFailures, long retryTimeoutDelay, RoutingContext routingContext ) { this.maxRoutingFailures = maxRoutingFailures; this.retryTimeoutDelay = retryTimeoutDelay; - this.routingParameters = routingParameters; + this.routingContext = routingContext; + } + + public RoutingSettings withRoutingContext( RoutingContext newRoutingContext ) + { + return new RoutingSettings( maxRoutingFailures, retryTimeoutDelay, newRoutingContext ); + } + + public int maxRoutingFailures() + { + return maxRoutingFailures; + } + + public long retryTimeoutDelay() + { + return retryTimeoutDelay; + } + + public RoutingContext routingContext() + { + return routingContext; } } diff --git a/driver/src/main/java/org/neo4j/driver/v1/Config.java b/driver/src/main/java/org/neo4j/driver/v1/Config.java index 79247fc145..2eea9f359f 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/Config.java +++ b/driver/src/main/java/org/neo4j/driver/v1/Config.java @@ -19,7 +19,6 @@ package org.neo4j.driver.v1; import java.io.File; -import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.logging.Level; @@ -33,7 +32,6 @@ import org.neo4j.driver.v1.util.Immutable; import org.neo4j.driver.v1.util.Resource; -import static java.util.Collections.EMPTY_MAP; import static org.neo4j.driver.v1.Config.TrustStrategy.trustAllCertificates; /** @@ -76,8 +74,6 @@ public class Config private final int connectionTimeoutMillis; private final RetrySettings retrySettings; - private final Map routingContext; - private Config( ConfigBuilder builder) { this.logging = builder.logging; @@ -92,8 +88,6 @@ private Config( ConfigBuilder builder) this.routingRetryDelayMillis = builder.routingRetryDelayMillis; this.connectionTimeoutMillis = builder.connectionTimeoutMillis; this.retrySettings = builder.retrySettings; - - this.routingContext = builder.routingContext; } /** @@ -188,7 +182,7 @@ public static Config defaultConfig() RoutingSettings routingSettings() { - return new RoutingSettings( routingFailureLimit, routingRetryDelayMillis, routingContext ); + return new RoutingSettings( routingFailureLimit, routingRetryDelayMillis ); } RetrySettings retrySettings() @@ -211,7 +205,6 @@ public static class ConfigBuilder private long routingRetryDelayMillis = TimeUnit.SECONDS.toMillis( 5 ); private int connectionTimeoutMillis = (int) TimeUnit.SECONDS.toMillis( 5 ); private RetrySettings retrySettings = RetrySettings.DEFAULT; - private Map routingContext = EMPTY_MAP; private ConfigBuilder() {} @@ -480,21 +473,6 @@ public ConfigBuilder withMaxTransactionRetryTime( long value, TimeUnit unit ) return this; } - /** - * Specify routing context that would be passed to server in getRoutingTable Procedure call for customized - * routing table reply. - * This parameter is only valid for the routing driver, a.k.a. the driver created use bolt+routing in URI - * scheme with 3.2+ Neo4j Casual Cluster servers. - * @param context The routing context to pass to getRoutingTable Procedure - * @since 1.3 - * @return this builder - */ - public ConfigBuilder withRoutingContext( Map context ) - { - this.routingContext = context; - return this; - } - /** * Create a config instance from this builder. * @return a {@link Config} instance diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java index fc1f950922..c40d981163 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java @@ -852,6 +852,69 @@ public void shouldUseInitialRouterForRediscoveryWhenAllOtherRoutersAreDead() thr assertEquals( 0, router.exitStatus() ); } + @Test + public void shouldInvokeProcedureGetRoutingTableWhenServerVersionPermits() throws Exception + { + // stub server is both a router and reader + StubServer server = StubServer.start( "get_routing_table.script", 9001 ); + + try ( Driver driver = GraphDatabase.driver( "bolt+routing://127.0.0.1:9001", config ); + Session session = driver.session() ) + { + List records = session.run( "MATCH (n) RETURN n.name AS name" ).list(); + assertEquals( 3, records.size() ); + assertEquals( "Alice", records.get( 0 ).get( "name" ).asString() ); + assertEquals( "Bob", records.get( 1 ).get( "name" ).asString() ); + assertEquals( "Eve", records.get( 2 ).get( "name" ).asString() ); + } + finally + { + assertEquals( 0, server.exitStatus() ); + } + } + + @Test + public void shouldSendRoutingContextToServer() throws Exception + { + // stub server is both a router and reader + StubServer server = StubServer.start( "get_routing_table_with_context.script", 9001 ); + + URI uri = URI.create( "bolt+routing://127.0.0.1:9001/?policy=my_policy®ion=china" ); + try ( Driver driver = GraphDatabase.driver( uri, config ); + Session session = driver.session() ) + { + List records = session.run( "MATCH (n) RETURN n.name AS name" ).list(); + assertEquals( 2, records.size() ); + assertEquals( "Alice", records.get( 0 ).get( "name" ).asString() ); + assertEquals( "Bob", records.get( 1 ).get( "name" ).asString() ); + } + finally + { + assertEquals( 0, server.exitStatus() ); + } + } + + @Test + public void shouldIgnoreRoutingContextWhenServerDoesNotSupportIt() throws Exception + { + // stub server is both a router and reader + StubServer server = StubServer.start( "rediscover_and_read_with_init.script", 9001 ); + + URI uri = URI.create( "bolt+routing://127.0.0.1:9001/?policy=my_policy" ); + try ( Driver driver = GraphDatabase.driver( uri, config ); + Session session = driver.session() ) + { + List records = session.run( "MATCH (n) RETURN n.name" ).list(); + assertEquals( 2, records.size() ); + assertEquals( "Bob", records.get( 0 ).get( 0 ).asString() ); + assertEquals( "Tina", records.get( 1 ).get( 0 ).asString() ); + } + finally + { + assertEquals( 0, server.exitStatus() ); + } + } + private static Driver newDriverWithSleeplessClock( String uriString ) { DriverFactory driverFactory = new DriverFactoryWithClock( new SleeplessClock() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java index 3b1a0312f7..97403a5115 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java @@ -149,8 +149,8 @@ public void shouldFailIfNoRouting() // Then catch ( ServiceUnavailableException e ) { - assertThat( e.getMessage(), containsString( "Failed to run " + - "'Statement{text='CALL dbms.cluster.routing.getServers', parameters={}}' on server." ) ); + assertThat( e.getMessage(), + containsString( "Failed to run 'CALL dbms.cluster.routing.getServers {}' on server." ) ); } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterCompositionProviderTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterCompositionProviderTest.java index f1ac12dcc5..b1fece48c7 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterCompositionProviderTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterCompositionProviderTest.java @@ -31,6 +31,7 @@ import org.neo4j.driver.internal.util.Clock; import org.neo4j.driver.internal.value.StringValue; import org.neo4j.driver.v1.Record; +import org.neo4j.driver.v1.Statement; import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.exceptions.ProtocolException; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; @@ -53,8 +54,8 @@ public class ClusterCompositionProviderTest public void shouldProtocolErrorWhenNoRecord() throws Throwable { // Given - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mock( Clock.class ), + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mock( Clock.class ), DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -82,8 +83,8 @@ public void shouldProtocolErrorWhenNoRecord() throws Throwable public void shouldProtocolErrorWhenMoreThanOneRecord() throws Throwable { // Given - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mock( Clock.class ), + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mock( Clock.class ), DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -111,8 +112,8 @@ public void shouldProtocolErrorWhenMoreThanOneRecord() throws Throwable public void shouldProtocolErrorWhenUnparsableRecord() throws Throwable { // Given - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mock( Clock.class ), + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mock( Clock.class ), DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -140,9 +141,9 @@ public void shouldProtocolErrorWhenUnparsableRecord() throws Throwable public void shouldProtocolErrorWhenNoRouters() throws Throwable { // Given - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); Clock mockedClock = mock( Clock.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mockedClock, + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mockedClock, DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -175,9 +176,9 @@ public void shouldProtocolErrorWhenNoRouters() throws Throwable public void shouldProtocolErrorWhenNoReaders() throws Throwable { // Given - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); Clock mockedClock = mock( Clock.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mockedClock, + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mockedClock, DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -211,8 +212,8 @@ public void shouldProtocolErrorWhenNoReaders() throws Throwable public void shouldPropagateConnectionFailureExceptions() throws Exception { // Given - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mock( Clock.class ), + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mock( Clock.class ), DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -242,8 +243,8 @@ public void shouldReturnSuccessResultWhenNoError() throws Throwable { // Given Clock mockedClock = mock( Clock.class ); - GetServersProcedureRunner mockedRunner = mock( GetServersProcedureRunner.class ); - ClusterCompositionProvider provider = new GetServersProcedureClusterCompositionProvider( mockedClock, + RoutingProcedureRunner mockedRunner = newProcedureRunnerMock(); + ClusterCompositionProvider provider = new RoutingProcedureClusterCompositionProvider( mockedClock, DEV_NULL_LOGGER, mockedRunner ); PooledConnection mockedConn = mock( PooledConnection.class ); @@ -286,4 +287,10 @@ private static Set serverSet( String... addresses ) return result; } + private static RoutingProcedureRunner newProcedureRunnerMock() + { + RoutingProcedureRunner mock = mock( RoutingProcedureRunner.class ); + when( mock.invokedProcedure() ).thenReturn( new Statement( "procedure" ) ); + return mock; + } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingContextTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingContextTest.java new file mode 100644 index 0000000000..2d14840360 --- /dev/null +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingContextTest.java @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal.cluster; + +import org.junit.Test; + +import java.net.URI; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class RoutingContextTest +{ + @Test + public void emptyContextIsNotDefined() + { + assertFalse( RoutingContext.EMPTY.isDefined() ); + } + + @Test + public void emptyContextInEmptyMap() + { + assertTrue( RoutingContext.EMPTY.asMap().isEmpty() ); + } + + @Test + public void uriWithoutQueryIsParsedToEmptyContext() + { + URI uri = URI.create( "bolt+routing://localhost:7687/" ); + RoutingContext context = new RoutingContext( uri ); + + assertFalse( context.isDefined() ); + assertTrue( context.asMap().isEmpty() ); + } + + @Test + public void uriWithQueryIsParsed() + { + URI uri = URI.create( "bolt+routing://localhost:7687/?key1=value1&key2=value2&key3=value3" ); + RoutingContext context = new RoutingContext( uri ); + + assertTrue( context.isDefined() ); + Map expectedMap = new HashMap<>(); + expectedMap.put( "key1", "value1" ); + expectedMap.put( "key2", "value2" ); + expectedMap.put( "key3", "value3" ); + assertEquals( expectedMap, context.asMap() ); + } + + @Test + public void throwsForInvalidUriQuery() + { + URI uri = URI.create( "bolt+routing://localhost:7687/?justKey" ); + + try + { + new RoutingContext( uri ); + fail( "Exception expected" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( IllegalArgumentException.class ) ); + } + } + + @Test + public void throwsForDuplicatedUriQueryParameters() + { + URI uri = URI.create( "bolt+routing://localhost:7687/?key1=value1&key2=value2&key1=value2" ); + + try + { + new RoutingContext( uri ); + fail( "Exception expected" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( IllegalArgumentException.class ) ); + } + } + + @Test + public void mapRepresentationIsUnmodifiable() + { + URI uri = URI.create( "bolt+routing://localhost:7687/?key1=value1" ); + RoutingContext context = new RoutingContext( uri ); + + assertEquals( singletonMap( "key1", "value1" ), context.asMap() ); + + try + { + context.asMap().put( "key2", "value2" ); + fail( "Exception expected" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( UnsupportedOperationException.class ) ); + } + + assertEquals( singletonMap( "key1", "value1" ), context.asMap() ); + } +} diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/GetServersProcedureRunnerTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunnerTest.java similarity index 66% rename from driver/src/test/java/org/neo4j/driver/internal/cluster/GetServersProcedureRunnerTest.java rename to driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunnerTest.java index aa4508ce04..c30735d4f9 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/GetServersProcedureRunnerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunnerTest.java @@ -20,33 +20,33 @@ import org.junit.Test; -import java.util.HashMap; +import java.net.URI; import java.util.List; -import java.util.Map; import org.neo4j.driver.internal.net.BoltServerAddress; import org.neo4j.driver.internal.spi.Connection; import org.neo4j.driver.internal.summary.InternalServerInfo; import org.neo4j.driver.v1.Record; import org.neo4j.driver.v1.Statement; +import org.neo4j.driver.v1.Value; import static java.util.Collections.EMPTY_MAP; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsEqual.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.neo4j.driver.internal.cluster.GetServersProcedureRunner.GET_ROUTING_TABLE; -import static org.neo4j.driver.internal.cluster.GetServersProcedureRunner.GET_ROUTING_TABLE_PARAM; -import static org.neo4j.driver.internal.cluster.GetServersProcedureRunner.GET_SERVERS; +import static org.neo4j.driver.internal.cluster.RoutingProcedureRunner.GET_ROUTING_TABLE; +import static org.neo4j.driver.internal.cluster.RoutingProcedureRunner.GET_ROUTING_TABLE_PARAM; +import static org.neo4j.driver.internal.cluster.RoutingProcedureRunner.GET_SERVERS; import static org.neo4j.driver.v1.Values.parameters; -public class GetServersProcedureRunnerTest +public class RoutingProcedureRunnerTest { @Test public void shouldCallGetRoutingTableWithEmptyMap() throws Throwable { // Given - GetServersProcedureRunner runner = new TestGetServersProcedureRunner( EMPTY_MAP ); + RoutingProcedureRunner runner = new TestRoutingProcedureRunner( RoutingContext.EMPTY ); Connection mock = mock( Connection.class ); when( mock.server() ).thenReturn( new InternalServerInfo( new BoltServerAddress( "123:45" ), "Neo4j/3.2.1" ) ); @@ -54,7 +54,7 @@ public void shouldCallGetRoutingTableWithEmptyMap() throws Throwable runner.run( mock ); // Then - assertThat( runner.procedureCalled(), equalTo( + assertThat( runner.invokedProcedure(), equalTo( new Statement( "CALL " + GET_ROUTING_TABLE, parameters( GET_ROUTING_TABLE_PARAM, EMPTY_MAP ) ) ) ); } @@ -62,10 +62,9 @@ public void shouldCallGetRoutingTableWithEmptyMap() throws Throwable public void shouldCallGetRoutingTableWithParam() throws Throwable { // Given - HashMap param = new HashMap<>(); - param.put( "key1", "value1" ); - param.put( "key2", "value2" ); - GetServersProcedureRunner runner = new TestGetServersProcedureRunner( param ); + URI uri = URI.create( "bolt+routing://localhost/?key1=value1&key2=value2" ); + RoutingContext context = new RoutingContext( uri ); + RoutingProcedureRunner runner = new TestRoutingProcedureRunner( context ); Connection mock = mock( Connection.class ); when( mock.server() ).thenReturn( new InternalServerInfo( new BoltServerAddress( "123:45" ), "Neo4j/3.2.1" ) ); @@ -73,18 +72,18 @@ public void shouldCallGetRoutingTableWithParam() throws Throwable runner.run( mock ); // Then - assertThat( runner.procedureCalled(), equalTo( - new Statement( "CALL " + GET_ROUTING_TABLE, parameters( GET_ROUTING_TABLE_PARAM, param ) ) ) ); + Value expectedParams = parameters( GET_ROUTING_TABLE_PARAM, context.asMap() ); + assertThat( runner.invokedProcedure(), equalTo( + new Statement( "CALL " + GET_ROUTING_TABLE, expectedParams ) ) ); } @Test public void shouldCallGetServers() throws Throwable { // Given - HashMap param = new HashMap<>(); - param.put( "key1", "value1" ); - param.put( "key2", "value2" ); - GetServersProcedureRunner runner = new TestGetServersProcedureRunner( param ); + URI uri = URI.create( "bolt+routing://localhost/?key1=value1&key2=value2" ); + RoutingContext context = new RoutingContext( uri ); + RoutingProcedureRunner runner = new TestRoutingProcedureRunner( context ); Connection mock = mock( Connection.class ); when( mock.server() ).thenReturn( new InternalServerInfo( new BoltServerAddress( "123:45" ), "Neo4j/3.1.8" ) ); @@ -92,16 +91,15 @@ public void shouldCallGetServers() throws Throwable runner.run( mock ); // Then - assertThat( runner.procedureCalled(), equalTo( + assertThat( runner.invokedProcedure(), equalTo( new Statement( "CALL " + GET_SERVERS ) ) ); } - private static class TestGetServersProcedureRunner extends GetServersProcedureRunner + private static class TestRoutingProcedureRunner extends RoutingProcedureRunner { - - TestGetServersProcedureRunner( Map parameters ) + TestRoutingProcedureRunner( RoutingContext context ) { - super( parameters ); + super( context ); } @Override diff --git a/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java b/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java index 9a0afe2634..791c7595cf 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java +++ b/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java @@ -90,4 +90,18 @@ public void boltPlusDiscoverySchemeShouldNotSupportTrustOnFirstUse() assertThat( e, instanceOf( IllegalArgumentException.class ) ); } } + + @Test + public void throwsWhenBoltSchemeUsedWithRoutingParams() + { + try + { + GraphDatabase.driver( "bolt://localhost:7687/?policy=my_policy" ); + fail( "Exception expected" ); + } + catch ( Exception e ) + { + assertThat( e, instanceOf( IllegalArgumentException.class ) ); + } + } } 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 bee737376f..72eabb9d07 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 @@ -105,8 +105,7 @@ public void sessionCreationShouldFailIfCallingDiscoveryProcedureOnEdgeServer() t } catch ( ServiceUnavailableException ex ) { - assertThat( ex.getMessage(), containsString( - "Failed to run 'Statement{text='CALL dbms.cluster.routing." ) ); + assertThat( ex.getMessage(), containsString( "Failed to run 'CALL dbms.cluster.routing" ) ); } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java index 0dd85d53cf..e8b0599f30 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java @@ -23,7 +23,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -37,6 +36,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.neo4j.driver.internal.DriverFactory; +import org.neo4j.driver.internal.cluster.RoutingContext; import org.neo4j.driver.internal.cluster.RoutingSettings; import org.neo4j.driver.internal.logging.DevNullLogging; import org.neo4j.driver.internal.retry.RetrySettings; @@ -1341,7 +1341,7 @@ private Driver newDriverWithoutRetries() private Driver newDriverWithFixedRetries( int maxRetriesCount ) { DriverFactory driverFactory = new DriverFactoryWithFixedRetryLogic( maxRetriesCount ); - RoutingSettings routingConf = new RoutingSettings( 1, 1, Collections.emptyMap() ); + RoutingSettings routingConf = new RoutingSettings( 1, 1, RoutingContext.EMPTY ); AuthToken auth = AuthTokens.none(); return driverFactory.newInstance( neo4j.uri(), auth, routingConf, RetrySettings.DEFAULT, noLoggingConfig() ); } diff --git a/driver/src/test/resources/get_routing_table.script b/driver/src/test/resources/get_routing_table.script new file mode 100644 index 0000000000..aa686e5941 --- /dev/null +++ b/driver/src/test/resources/get_routing_table.script @@ -0,0 +1,17 @@ +!: AUTO RESET +!: AUTO PULL_ALL + +C: INIT "neo4j-java/dev" +S: SUCCESS {"server": "Neo4j/3.2.2"} +C: RUN "CALL dbms.cluster.routing.getRoutingTable({context})" {"context": {}} + PULL_ALL +S: SUCCESS {"fields": ["ttl", "servers"]} + RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9001", "127.0.0.1:9002"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9001", "127.0.0.1:9002"], "role": "READ"},{"addresses": ["127.0.0.1:9001", "127.0.0.1:9002"], "role": "ROUTE"}]] + SUCCESS {} +C: RUN "MATCH (n) RETURN n.name AS name" {} + PULL_ALL +S: SUCCESS {"fields": ["name"]} + RECORD ["Alice"] + RECORD ["Bob"] + RECORD ["Eve"] + SUCCESS {} diff --git a/driver/src/test/resources/get_routing_table_with_context.script b/driver/src/test/resources/get_routing_table_with_context.script new file mode 100644 index 0000000000..38c8cacd75 --- /dev/null +++ b/driver/src/test/resources/get_routing_table_with_context.script @@ -0,0 +1,16 @@ +!: AUTO RESET +!: AUTO PULL_ALL + +C: INIT "neo4j-java/dev" +S: SUCCESS {"server": "Neo4j/3.2.3"} +C: RUN "CALL dbms.cluster.routing.getRoutingTable({context})" {"context": {"policy": "my_policy", "region": "china"}} + PULL_ALL +S: SUCCESS {"fields": ["ttl", "servers"]} + RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9001", "127.0.0.1:9002"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9001", "127.0.0.1:9002"], "role": "READ"},{"addresses": ["127.0.0.1:9001", "127.0.0.1:9002"], "role": "ROUTE"}]] + SUCCESS {} +C: RUN "MATCH (n) RETURN n.name AS name" {} + PULL_ALL +S: SUCCESS {"fields": ["name"]} + RECORD ["Alice"] + RECORD ["Bob"] + SUCCESS {} diff --git a/driver/src/test/resources/rediscover_and_read_with_init.script b/driver/src/test/resources/rediscover_and_read_with_init.script new file mode 100644 index 0000000000..cb63d479e3 --- /dev/null +++ b/driver/src/test/resources/rediscover_and_read_with_init.script @@ -0,0 +1,16 @@ +!: AUTO RESET +!: AUTO PULL_ALL + +C: INIT "neo4j-java/dev" +S: SUCCESS {"server": "Neo4j/3.1.0"} +C: RUN "CALL dbms.cluster.routing.getServers" {} + PULL_ALL +S: SUCCESS {"fields": ["ttl", "servers"]} + RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9001","127.0.0.1:9002"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9001","127.0.0.1:9002"], "role": "READ"},{"addresses": ["127.0.0.1:9001","127.0.0.1:9002"], "role": "ROUTE"}]] + SUCCESS {} +C: RUN "MATCH (n) RETURN n.name" {} + PULL_ALL +S: SUCCESS {"fields": ["n.name"]} + RECORD ["Bob"] + RECORD ["Tina"] + SUCCESS {}