Skip to content

Commit d6f1f86

Browse files
zhenlineozhenlineo
authored
Merge pull request #533 from lutovich/1.7-resolver-throws
Fail discovery when custom resolver fails
2 parents b428d48 + 5d9478a commit d6f1f86

File tree

4 files changed

+107
-26
lines changed

4 files changed

+107
-26
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@
3939

4040
import static java.lang.String.format;
4141
import static java.util.Collections.emptySet;
42-
import static java.util.Collections.singletonList;
4342
import static java.util.concurrent.CompletableFuture.completedFuture;
4443
import static java.util.stream.Collectors.toList;
4544
import static org.neo4j.driver.internal.util.Futures.completedWithNull;
45+
import static org.neo4j.driver.internal.util.Futures.failedFuture;
4646

4747
public class Rediscovery
4848
{
@@ -204,7 +204,15 @@ private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable r
204204
private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable routingTable,
205205
ConnectionPool connectionPool, Set<BoltServerAddress> seenServers )
206206
{
207-
List<BoltServerAddress> addresses = resolve( initialRouter );
207+
List<BoltServerAddress> addresses;
208+
try
209+
{
210+
addresses = resolve( initialRouter );
211+
}
212+
catch ( Throwable error )
213+
{
214+
return failedFuture( error );
215+
}
208216
addresses.removeAll( seenServers );
209217

210218
CompletableFuture<ClusterComposition> result = completedWithNull();
@@ -260,17 +268,9 @@ private ClusterComposition handleRoutingProcedureError( Throwable error, Routing
260268

261269
private List<BoltServerAddress> resolve( BoltServerAddress address )
262270
{
263-
try
264-
{
265-
return resolver.resolve( address )
266-
.stream()
267-
.map( BoltServerAddress::from )
268-
.collect( toList() ); // collect to list to preserve the order
269-
}
270-
catch ( Throwable error )
271-
{
272-
logger.error( "Resolver function failed to resolve '" + address + "'. The address will be used as is", error );
273-
return singletonList( address );
274-
}
271+
return resolver.resolve( address )
272+
.stream()
273+
.map( BoltServerAddress::from )
274+
.collect( toList() ); // collect to list to preserve the order
275275
}
276276
}

‎driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java

Lines changed: 89 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@
2323
import java.io.IOException;
2424
import java.net.URI;
2525
import java.util.ArrayList;
26-
import java.util.Arrays;
26+
import java.util.HashSet;
2727
import java.util.List;
28+
import java.util.concurrent.atomic.AtomicBoolean;
2829
import java.util.concurrent.atomic.AtomicInteger;
2930

3031
import org.neo4j.driver.internal.cluster.RoutingSettings;
@@ -45,10 +46,12 @@
4546
import org.neo4j.driver.v1.TransactionWork;
4647
import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
4748
import org.neo4j.driver.v1.exceptions.SessionExpiredException;
49+
import org.neo4j.driver.v1.net.ServerAddress;
50+
import org.neo4j.driver.v1.net.ServerAddressResolver;
4851
import org.neo4j.driver.v1.util.StubServer;
4952

5053
import static java.util.Arrays.asList;
51-
import static java.util.logging.Level.INFO;
54+
import static java.util.Collections.singleton;
5255
import static org.hamcrest.Matchers.hasSize;
5356
import static org.hamcrest.core.IsEqual.equalTo;
5457
import static org.hamcrest.junit.MatcherAssert.assertThat;
@@ -57,13 +60,18 @@
5760
import static org.junit.jupiter.api.Assertions.assertNotNull;
5861
import static org.junit.jupiter.api.Assertions.assertThrows;
5962
import static org.junit.jupiter.api.Assertions.assertTrue;
60-
import static org.neo4j.driver.v1.Logging.console;
63+
import static org.mockito.ArgumentMatchers.any;
64+
import static org.mockito.Mockito.mock;
65+
import static org.mockito.Mockito.verify;
66+
import static org.mockito.Mockito.when;
67+
import static org.neo4j.driver.v1.Logging.none;
6168

6269
class RoutingDriverBoltKitTest
6370
{
6471
private static final Config config = Config.build()
6572
.withoutEncryption()
66-
.withLogging( console( INFO ) ).toConfig();
73+
.withLogging( none() )
74+
.toConfig();
6775

6876
@Test
6977
void shouldHandleAcquireReadSession() throws IOException, InterruptedException, StubServer.ForceKilled
@@ -917,7 +925,7 @@ void shouldSendMultipleBookmarks() throws Exception
917925
StubServer router = StubServer.start( "acquire_endpoints.script", 9001 );
918926
StubServer writer = StubServer.start( "multiple_bookmarks.script", 9007 );
919927

920-
List<String> bookmarks = Arrays.asList( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29",
928+
List<String> bookmarks = asList( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29",
921929
"neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16",
922930
"neo4j:bookmark:v1:tx68" );
923931

@@ -963,10 +971,82 @@ void shouldForgetAddressOnDatabaseUnavailableError() throws Exception
963971
}
964972
finally
965973
{
966-
assertEquals( router1.exitStatus(), 0 );
967-
assertEquals( writer1.exitStatus(), 0 );
968-
assertEquals( router2.exitStatus(), 0 );
969-
assertEquals( writer2.exitStatus(), 0 );
974+
assertEquals( 0, router1.exitStatus() );
975+
assertEquals( 0, writer1.exitStatus() );
976+
assertEquals( 0, router2.exitStatus() );
977+
assertEquals( 0, writer2.exitStatus() );
978+
}
979+
}
980+
981+
@Test
982+
void shouldFailInitialDiscoveryWhenConfiguredResolverThrows()
983+
{
984+
ServerAddressResolver resolver = mock( ServerAddressResolver.class );
985+
when( resolver.resolve( any( ServerAddress.class ) ) ).thenThrow( new RuntimeException( "Resolution failure!" ) );
986+
987+
Config config = Config.build()
988+
.withLogging( none() )
989+
.withoutEncryption()
990+
.withResolver( resolver )
991+
.toConfig();
992+
993+
RuntimeException error = assertThrows( RuntimeException.class, () -> GraphDatabase.driver( "bolt+routing://my.server.com:9001", config ) );
994+
assertEquals( "Resolution failure!", error.getMessage() );
995+
verify( resolver ).resolve( ServerAddress.of( "my.server.com", 9001 ) );
996+
}
997+
998+
@Test
999+
void shouldUseResolverDuringRediscoveryWhenExistingRoutersFail() throws Exception
1000+
{
1001+
StubServer router1 = StubServer.start( "get_routing_table.script", 9001 );
1002+
StubServer router2 = StubServer.start( "acquire_endpoints.script", 9042 );
1003+
StubServer reader = StubServer.start( "read_server.script", 9005 );
1004+
1005+
AtomicBoolean resolverInvoked = new AtomicBoolean();
1006+
ServerAddressResolver resolver = address ->
1007+
{
1008+
if ( resolverInvoked.compareAndSet( false, true ) )
1009+
{
1010+
// return the address first time
1011+
return singleton( address );
1012+
}
1013+
if ( "127.0.0.1".equals( address.host() ) && address.port() == 9001 )
1014+
{
1015+
// return list of addresses where onl 9042 is functional
1016+
return new HashSet<>( asList(
1017+
ServerAddress.of( "127.0.0.1", 9010 ),
1018+
ServerAddress.of( "127.0.0.1", 9011 ),
1019+
ServerAddress.of( "127.0.0.1", 9042 ) ) );
1020+
}
1021+
throw new AssertionError();
1022+
};
1023+
1024+
Config config = Config.build()
1025+
.withLogging( none() )
1026+
.withoutEncryption()
1027+
.withResolver( resolver )
1028+
.toConfig();
1029+
1030+
try ( Driver driver = GraphDatabase.driver( "bolt+routing://127.0.0.1:9001", config ) )
1031+
{
1032+
try ( Session session = driver.session( AccessMode.READ ) )
1033+
{
1034+
// run first query against 9001, which should return result and exit
1035+
List<String> names1 = session.run( "MATCH (n) RETURN n.name AS name" )
1036+
.list( record -> record.get( "name" ).asString() );
1037+
assertEquals( asList( "Alice", "Bob", "Eve" ), names1 );
1038+
1039+
// run second query with retries, it should rediscover using 9042 returned by the resolver and read from 9005
1040+
List<String> names2 = session.readTransaction( tx -> tx.run( "MATCH (n) RETURN n.name" ) )
1041+
.list( record -> record.get( 0 ).asString() );
1042+
assertEquals( asList( "Bob", "Alice", "Tina" ), names2 );
1043+
}
1044+
}
1045+
finally
1046+
{
1047+
assertEquals( 0, router1.exitStatus() );
1048+
assertEquals( 0, router2.exitStatus() );
1049+
assertEquals( 0, reader.exitStatus() );
9701050
}
9711051
}
9721052

‎driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ void shouldResolveInitialRouterAddressUsingCustomResolver()
227227
}
228228

229229
@Test
230-
void shouldUseInitialRouterAddressAsIsWhenResolverFails()
230+
void shouldPropagateFailureWhenResolverFails()
231231
{
232232
ClusterComposition expectedComposition = new ClusterComposition( 42,
233233
asOrderedSet( A, B ), asOrderedSet( A, B ), asOrderedSet( A, B ) );
@@ -242,9 +242,9 @@ void shouldUseInitialRouterAddressAsIsWhenResolverFails()
242242
Rediscovery rediscovery = newRediscovery( A, compositionProvider, resolver );
243243
RoutingTable table = routingTableMock();
244244

245-
ClusterComposition actualComposition = await( rediscovery.lookupClusterComposition( table, pool ) );
245+
RuntimeException error = assertThrows( RuntimeException.class, () -> await( rediscovery.lookupClusterComposition( table, pool ) ) );
246+
assertEquals( "Resolver fails!", error.getMessage() );
246247

247-
assertEquals( expectedComposition, actualComposition );
248248
verify( resolver ).resolve( A );
249249
verify( table, never() ).forget( any() );
250250
}

‎driver/src/test/resources/get_routing_table.script

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ S: SUCCESS {"fields": ["name"]}
1515
RECORD ["Bob"]
1616
RECORD ["Eve"]
1717
SUCCESS {}
18+
S: <EXIT>

0 commit comments

Comments
 (0)