Skip to content

Commit a81b238

Browse files
lutovichlutovich
committed
Fail early on auth errors in direct driver
This commit makes direct driver test connectivity on creation. It's done by opening and closing a connection. Previously driver did not try to connect during creation and auth errors (incorrect credentials, etc.) popped up only when first query was executed or first transaction was started. Such delayed errors were confusing. Also routing driver behaved differently because it fetches routing table at creation and auth errors come out of constructor.
1 parent 42e4a67 commit a81b238

File tree

15 files changed

+293
-146
lines changed

15 files changed

+293
-146
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/DirectConnectionProvider.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public class DirectConnectionProvider implements ConnectionProvider
3737
{
3838
this.address = address;
3939
this.pool = pool;
40+
41+
verifyConnectivity();
4042
}
4143

4244
@Override
@@ -55,4 +57,13 @@ public BoltServerAddress getAddress()
5557
{
5658
return address;
5759
}
60+
61+
/**
62+
* Acquires and releases a connection to verify connectivity so this connection provider fails fast. This is
63+
* especially valuable when driver was created with incorrect credentials.
64+
*/
65+
private void verifyConnectivity()
66+
{
67+
acquireConnection( AccessMode.READ ).close();
68+
}
5869
}

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

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@
2525
import org.neo4j.driver.internal.spi.PooledConnection;
2626

2727
import static org.junit.Assert.assertEquals;
28+
import static org.junit.Assert.assertNotNull;
2829
import static org.junit.Assert.assertSame;
30+
import static org.junit.Assert.fail;
2931
import static org.mockito.Matchers.any;
32+
import static org.mockito.Mockito.RETURNS_MOCKS;
33+
import static org.mockito.Mockito.doThrow;
3034
import static org.mockito.Mockito.mock;
31-
import static org.mockito.Mockito.only;
3235
import static org.mockito.Mockito.verify;
3336
import static org.mockito.Mockito.when;
3437
import static org.neo4j.driver.v1.AccessMode.READ;
@@ -42,7 +45,7 @@ public void acquiresConnectionsFromThePool()
4245
ConnectionPool pool = mock( ConnectionPool.class );
4346
PooledConnection connection1 = mock( PooledConnection.class );
4447
PooledConnection connection2 = mock( PooledConnection.class );
45-
when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection1 ).thenReturn( connection2 );
48+
when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection1, connection1, connection2 );
4649

4750
DirectConnectionProvider provider = newConnectionProvider( pool );
4851

@@ -53,12 +56,12 @@ public void acquiresConnectionsFromThePool()
5356
@Test
5457
public void closesPool() throws Exception
5558
{
56-
ConnectionPool pool = mock( ConnectionPool.class );
59+
ConnectionPool pool = mock( ConnectionPool.class, RETURNS_MOCKS );
5760
DirectConnectionProvider provider = newConnectionProvider( pool );
5861

5962
provider.close();
6063

61-
verify( pool, only() ).close();
64+
verify( pool ).close();
6265
}
6366

6467
@Test
@@ -71,9 +74,42 @@ public void returnsCorrectAddress()
7174
assertEquals( address, provider.getAddress() );
7275
}
7376

77+
@Test
78+
public void testsConnectivityOnCreation()
79+
{
80+
ConnectionPool pool = mock( ConnectionPool.class );
81+
PooledConnection connection = mock( PooledConnection.class );
82+
when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection );
83+
84+
assertNotNull( newConnectionProvider( pool ) );
85+
86+
verify( pool ).acquire( BoltServerAddress.LOCAL_DEFAULT );
87+
verify( connection ).close();
88+
}
89+
90+
@Test
91+
public void throwsWhenTestConnectionThrows()
92+
{
93+
ConnectionPool pool = mock( ConnectionPool.class );
94+
PooledConnection connection = mock( PooledConnection.class );
95+
RuntimeException error = new RuntimeException();
96+
doThrow( error ).when( connection ).close();
97+
when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection );
98+
99+
try
100+
{
101+
newConnectionProvider( pool );
102+
fail( "Exception expected" );
103+
}
104+
catch ( Exception e )
105+
{
106+
assertSame( error, e );
107+
}
108+
}
109+
74110
private static DirectConnectionProvider newConnectionProvider( BoltServerAddress address )
75111
{
76-
return new DirectConnectionProvider( address, mock( ConnectionPool.class ) );
112+
return new DirectConnectionProvider( address, mock( ConnectionPool.class, RETURNS_MOCKS ) );
77113
}
78114

79115
private static DirectConnectionProvider newConnectionProvider( ConnectionPool pool )

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.neo4j.driver.internal;
2020

21+
import org.junit.After;
22+
import org.junit.ClassRule;
2123
import org.junit.Test;
2224

2325
import java.net.URI;
@@ -28,6 +30,7 @@
2830
import org.neo4j.driver.v1.Record;
2931
import org.neo4j.driver.v1.Session;
3032
import org.neo4j.driver.v1.util.StubServer;
33+
import org.neo4j.driver.v1.util.TestNeo4j;
3134

3235
import static org.hamcrest.Matchers.is;
3336
import static org.hamcrest.core.IsEqual.equalTo;
@@ -40,14 +43,28 @@
4043

4144
public class DirectDriverTest
4245
{
46+
@ClassRule
47+
public static final TestNeo4j neo4j = new TestNeo4j();
48+
49+
private Driver driver;
50+
51+
@After
52+
public void closeDriver() throws Exception
53+
{
54+
if ( driver != null )
55+
{
56+
driver.close();
57+
}
58+
}
59+
4360
@Test
4461
public void shouldUseDefaultPortIfMissing()
4562
{
4663
// Given
4764
URI uri = URI.create( "bolt://localhost" );
4865

4966
// When
50-
Driver driver = GraphDatabase.driver( uri );
67+
driver = GraphDatabase.driver( uri, neo4j.authToken() );
5168

5269
// Then
5370
assertThat( driver, is( directDriverWithAddress( LOCAL_DEFAULT ) ) );
@@ -61,7 +78,7 @@ public void shouldAllowIPv6Address()
6178
BoltServerAddress address = BoltServerAddress.from( uri );
6279

6380
// When
64-
Driver driver = GraphDatabase.driver( uri );
81+
driver = GraphDatabase.driver( uri, neo4j.authToken() );
6582

6683
// Then
6784
assertThat( driver, is( directDriverWithAddress( address ) ) );
@@ -76,7 +93,7 @@ public void shouldRejectInvalidAddress()
7693
// When & Then
7794
try
7895
{
79-
Driver driver = GraphDatabase.driver( uri );
96+
driver = GraphDatabase.driver( uri, neo4j.authToken() );
8097
fail("Expecting error for wrong uri");
8198
}
8299
catch( IllegalArgumentException e )
@@ -93,7 +110,7 @@ public void shouldRegisterSingleServer()
93110
BoltServerAddress address = BoltServerAddress.from( uri );
94111

95112
// When
96-
Driver driver = GraphDatabase.driver( uri );
113+
driver = GraphDatabase.driver( uri, neo4j.authToken() );
97114

98115
// Then
99116
assertThat( driver, is( directDriverWithAddress( address ) ) );

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.neo4j.driver.internal.security.SecurityPlan;
3737
import org.neo4j.driver.internal.spi.ConnectionPool;
3838
import org.neo4j.driver.internal.spi.ConnectionProvider;
39+
import org.neo4j.driver.internal.spi.PooledConnection;
3940
import org.neo4j.driver.v1.AuthToken;
4041
import org.neo4j.driver.v1.AuthTokens;
4142
import org.neo4j.driver.v1.Config;
@@ -45,9 +46,11 @@
4546
import static org.junit.Assert.assertArrayEquals;
4647
import static org.junit.Assert.assertThat;
4748
import static org.junit.Assert.fail;
49+
import static org.mockito.Mockito.any;
4850
import static org.mockito.Mockito.doThrow;
4951
import static org.mockito.Mockito.mock;
5052
import static org.mockito.Mockito.verify;
53+
import static org.mockito.Mockito.when;
5154
import static org.neo4j.driver.v1.AccessMode.READ;
5255
import static org.neo4j.driver.v1.Config.defaultConfig;
5356

@@ -69,7 +72,7 @@ public static List<URI> uris()
6972
@Test
7073
public void connectionPoolClosedWhenDriverCreationFails() throws Exception
7174
{
72-
ConnectionPool connectionPool = mock( ConnectionPool.class );
75+
ConnectionPool connectionPool = connectionPoolMock();
7376
DriverFactory factory = new ThrowingDriverFactory( connectionPool );
7477

7578
try
@@ -87,7 +90,7 @@ public void connectionPoolClosedWhenDriverCreationFails() throws Exception
8790
@Test
8891
public void connectionPoolCloseExceptionIsSupressedWhenDriverCreationFails() throws Exception
8992
{
90-
ConnectionPool connectionPool = mock( ConnectionPool.class );
93+
ConnectionPool connectionPool = connectionPoolMock();
9194
RuntimeException poolCloseError = new RuntimeException( "Pool close error" );
9295
doThrow( poolCloseError ).when( connectionPool ).close();
9396

@@ -142,6 +145,13 @@ private Driver createDriver( DriverFactory driverFactory, Config config )
142145
return driverFactory.newInstance( uri, auth, routingSettings, RetrySettings.DEFAULT, config );
143146
}
144147

148+
private static ConnectionPool connectionPoolMock()
149+
{
150+
ConnectionPool pool = mock( ConnectionPool.class );
151+
when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( mock( PooledConnection.class ) );
152+
return pool;
153+
}
154+
145155
private static class ThrowingDriverFactory extends DriverFactory
146156
{
147157
final ConnectionPool connectionPool;
@@ -196,5 +206,11 @@ protected SessionFactory createSessionFactory( ConnectionProvider connectionProv
196206
capturedSessionFactory = sessionFactory;
197207
return sessionFactory;
198208
}
209+
210+
@Override
211+
protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan securityPlan, Config config )
212+
{
213+
return connectionPoolMock();
214+
}
199215
}
200216
}

‎driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,18 @@
3939
public class GraphDatabaseTest
4040
{
4141
@Test
42-
public void boltSchemeShouldInstantiateDirectDriver()
42+
public void boltSchemeShouldInstantiateDirectDriver() throws Exception
4343
{
4444
// Given
45-
URI uri = URI.create( "bolt://localhost:7687" );
45+
StubServer server = StubServer.start( "dummy_connection.script", 9001 );
46+
URI uri = URI.create( "bolt://localhost:9001" );
4647

4748
// When
48-
Driver driver = GraphDatabase.driver( uri );
49+
Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG );
4950

5051
// Then
5152
assertThat( driver, is( directDriver() ) );
53+
server.exit();
5254
}
5355

5456
@Test

‎driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.neo4j.driver.internal.spi.PooledConnection;
4141
import org.neo4j.driver.internal.util.Clock;
4242
import org.neo4j.driver.v1.AuthToken;
43-
import org.neo4j.driver.v1.AuthTokens;
4443
import org.neo4j.driver.v1.Config;
4544
import org.neo4j.driver.v1.Driver;
4645
import org.neo4j.driver.v1.Logging;
@@ -85,6 +84,7 @@ public void createDriver()
8584
RetrySettings retrySettings = RetrySettings.DEFAULT;
8685
driver = driverFactory.newInstance( neo4j.uri(), auth, routingSettings, retrySettings, defaultConfig() );
8786
connectionPool = driverFactory.connectionPool;
87+
connectionPool.startMemorizing(); // start memorizing connections after driver creation
8888
}
8989

9090
@After
@@ -370,23 +370,34 @@ protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan
370370
private static class MemorizingConnectionPool extends SocketConnectionPool
371371
{
372372
PooledConnection lastAcquiredConnectionSpy;
373+
boolean memorize;
373374

374375
MemorizingConnectionPool( PoolSettings poolSettings, Connector connector, Clock clock, Logging logging )
375376
{
376377
super( poolSettings, connector, clock, logging );
377378
}
378379

380+
void startMemorizing()
381+
{
382+
memorize = true;
383+
}
384+
379385
@Override
380386
public PooledConnection acquire( BoltServerAddress address )
381387
{
382388
PooledConnection connection = super.acquire( address );
383-
// this connection pool returns spies so spies will be returned to the pool
384-
// prevent spying on spies...
385-
if ( !Mockito.mockingDetails( connection ).isSpy() )
389+
390+
if ( memorize )
386391
{
387-
connection = spy( connection );
392+
// this connection pool returns spies so spies will be returned to the pool
393+
// prevent spying on spies...
394+
if ( !Mockito.mockingDetails( connection ).isSpy() )
395+
{
396+
connection = spy( connection );
397+
}
398+
lastAcquiredConnectionSpy = connection;
388399
}
389-
lastAcquiredConnectionSpy = connection;
400+
390401
return connection;
391402
}
392403
}

0 commit comments

Comments
 (0)