Skip to content
This repository was archived by the owner on Jul 6, 2023. It is now read-only.

Commit c72c0cb

Browse files
sherfertsherfert
authored
Merge pull request #182 from henriknyman/4.0-fix-connection-when-no-default-database
Fix connection when default database is unavailable
2 parents c956452 + 2732f2f commit c72c0cb

File tree

9 files changed

+433
-24
lines changed

9 files changed

+433
-24
lines changed

‎build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ ext {
7070
argparse4jVersion = '0.7.0'
7171
junitVersion = '4.12'
7272
evaluatorVersion = '3.5.4'
73-
neo4jJavaDriverVersion = '4.0.0-rc1'
73+
neo4jJavaDriverVersion = '4.0.0-rc2'
7474
findbugsVersion = '3.0.0'
7575
jansiVersion = '1.13'
7676
jlineVersion = '2.14.6'

‎cypher-shell/src/integration-test/java/org/neo4j/shell/MainIntegrationTest.java‎

Lines changed: 181 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import org.neo4j.driver.exceptions.ClientException;
1616
import org.neo4j.driver.exceptions.ServiceUnavailableException;
17+
import org.neo4j.driver.exceptions.TransientException;
1718
import org.neo4j.shell.cli.CliArgs;
1819
import org.neo4j.shell.commands.CommandHelper;
1920
import org.neo4j.shell.exception.CommandException;
@@ -27,9 +28,11 @@
2728
import static java.lang.String.format;
2829
import static org.hamcrest.CoreMatchers.isA;
2930
import static org.junit.Assert.assertEquals;
31+
import static org.junit.Assert.assertFalse;
3032
import static org.junit.Assert.assertNull;
3133
import static org.junit.Assert.assertTrue;
3234
import static org.junit.Assert.fail;
35+
import static org.junit.Assume.assumeTrue;
3336
import static org.mockito.Matchers.any;
3437
import static org.mockito.Mockito.mock;
3538
import static org.mockito.Mockito.verify;
@@ -102,6 +105,16 @@ private void ensureUser() throws Exception {
102105
}
103106
}
104107

108+
private void ensureDefaultDatabaseStarted() throws Exception {
109+
CliArgs cliArgs = new CliArgs();
110+
cliArgs.setUsername("neo4j", "");
111+
cliArgs.setPassword("neo", "");
112+
cliArgs.setDatabase("system");
113+
ShellAndConnection sac = getShell(cliArgs);
114+
main.connectMaybeInteractively(sac.shell, sac.connectionConfig, true, false);
115+
sac.shell.execute("START DATABASE " + DatabaseManager.DEFAULT_DEFAULT_DB_NAME);
116+
}
117+
105118
@Test
106119
public void promptsOnWrongAuthenticationIfInteractive() throws Exception {
107120
// when
@@ -239,7 +252,7 @@ public void wrongPortWithNeo4j() throws Exception
239252
ConnectionConfig connectionConfig = sac.connectionConfig;
240253

241254
exception.expect( ServiceUnavailableException.class );
242-
exception.expectMessage( "Unable to connect to database, ensure the database is running and that there is a working network connection to it" );
255+
// The error message here may be subject to change and is not stable across versions so let us not assert on it
243256
main.connectMaybeInteractively( shell, connectionConfig, true, true );
244257
}
245258

@@ -374,6 +387,173 @@ public void shouldFailIfInputFileDoesntExistInteractively() throws Exception {
374387
shell.execute( ":source what.cypher" );
375388
}
376389

390+
@Test
391+
public void doesNotStartWhenDefaultDatabaseUnavailableIfInteractive() throws Exception {
392+
shell.setCommandHelper(new CommandHelper(mock(Logger.class), Historian.empty, shell));
393+
inputBuffer.put(String.format("neo4j%nneo%n").getBytes());
394+
395+
assertEquals("", connectionConfig.username());
396+
assertEquals("", connectionConfig.password());
397+
398+
// when
399+
main.connectMaybeInteractively(shell, connectionConfig, true, true);
400+
401+
// Multiple databases are only available from 4.0
402+
assumeTrue( majorVersion( shell.getServerVersion() ) >= 4 );
403+
404+
// then
405+
// should be connected
406+
assertTrue(shell.isConnected());
407+
// should have prompted and set the username and password
408+
String expectedLoginOutput = format( "username: neo4j%npassword: ***%n" );
409+
assertEquals(expectedLoginOutput, baos.toString());
410+
assertEquals("neo4j", connectionConfig.username());
411+
assertEquals("neo", connectionConfig.password());
412+
413+
// Stop the default database
414+
shell.execute(":use " + DatabaseManager.SYSTEM_DB_NAME);
415+
shell.execute("STOP DATABASE " + DatabaseManager.DEFAULT_DEFAULT_DB_NAME);
416+
417+
try {
418+
shell.disconnect();
419+
420+
// Should get exception that database is unavailable when trying to connect
421+
exception.expect(TransientException.class);
422+
exception.expectMessage("Database 'neo4j' is unavailable");
423+
main.connectMaybeInteractively(shell, connectionConfig, true, true);
424+
425+
// then
426+
assertFalse(shell.isConnected());
427+
} finally {
428+
// Start the default database again
429+
ensureDefaultDatabaseStarted();
430+
}
431+
}
432+
433+
@Test
434+
public void startsAgainstSystemDatabaseWhenDefaultDatabaseUnavailableIfInteractive() throws Exception {
435+
shell.setCommandHelper(new CommandHelper(mock(Logger.class), Historian.empty, shell));
436+
437+
assertEquals("", connectionConfig.username());
438+
assertEquals("", connectionConfig.password());
439+
440+
// when
441+
main.connectMaybeInteractively(shell, connectionConfig, true, true);
442+
443+
// Multiple databases are only available from 4.0
444+
assumeTrue( majorVersion( shell.getServerVersion() ) >= 4 );
445+
446+
// then
447+
// should be connected
448+
assertTrue(shell.isConnected());
449+
// should have prompted and set the username and password
450+
String expectedLoginOutput = format( "username: neo4j%npassword: ***%n" );
451+
assertEquals(expectedLoginOutput, baos.toString());
452+
assertEquals("neo4j", connectionConfig.username());
453+
assertEquals("neo", connectionConfig.password());
454+
455+
// Stop the default database
456+
shell.execute(":use " + DatabaseManager.SYSTEM_DB_NAME);
457+
shell.execute("STOP DATABASE " + DatabaseManager.DEFAULT_DEFAULT_DB_NAME);
458+
459+
try {
460+
shell.disconnect();
461+
462+
// Connect to system database
463+
CliArgs cliArgs = new CliArgs();
464+
cliArgs.setUsername("neo4j", "");
465+
cliArgs.setPassword("neo", "");
466+
cliArgs.setDatabase("system");
467+
ShellAndConnection sac = getShell(cliArgs);
468+
// Use the new shell and connection config from here on
469+
shell = sac.shell;
470+
connectionConfig = sac.connectionConfig;
471+
main.connectMaybeInteractively(shell, connectionConfig, true, false);
472+
473+
// then
474+
assertTrue(shell.isConnected());
475+
} finally {
476+
// Start the default database again
477+
ensureDefaultDatabaseStarted();
478+
}
479+
}
480+
481+
@Test
482+
public void switchingToUnavailableDatabaseIfInteractive() throws Exception {
483+
shell.setCommandHelper(new CommandHelper(mock(Logger.class), Historian.empty, shell));
484+
inputBuffer.put(String.format("neo4j%nneo%n").getBytes());
485+
486+
assertEquals("", connectionConfig.username());
487+
assertEquals("", connectionConfig.password());
488+
489+
// when
490+
main.connectMaybeInteractively(shell, connectionConfig, true, true);
491+
492+
// Multiple databases are only available from 4.0
493+
assumeTrue(majorVersion( shell.getServerVersion() ) >= 4);
494+
495+
// then
496+
// should be connected
497+
assertTrue(shell.isConnected());
498+
// should have prompted and set the username and password
499+
String expectedLoginOutput = format( "username: neo4j%npassword: ***%n" );
500+
assertEquals(expectedLoginOutput, baos.toString());
501+
assertEquals("neo4j", connectionConfig.username());
502+
assertEquals("neo", connectionConfig.password());
503+
504+
// Stop the default database
505+
shell.execute(":use " + DatabaseManager.SYSTEM_DB_NAME);
506+
shell.execute("STOP DATABASE " + DatabaseManager.DEFAULT_DEFAULT_DB_NAME);
507+
508+
try {
509+
// Should get exception that database is unavailable when trying to connect
510+
exception.expect(TransientException.class);
511+
exception.expectMessage("Database 'neo4j' is unavailable");
512+
shell.execute(":use " + DatabaseManager.DEFAULT_DEFAULT_DB_NAME);
513+
} finally {
514+
// Start the default database again
515+
ensureDefaultDatabaseStarted();
516+
}
517+
}
518+
519+
@Test
520+
public void switchingToUnavailableDefaultDatabaseIfInteractive() throws Exception {
521+
shell.setCommandHelper(new CommandHelper(mock(Logger.class), Historian.empty, shell));
522+
inputBuffer.put(String.format("neo4j%nneo%n").getBytes());
523+
524+
assertEquals("", connectionConfig.username());
525+
assertEquals("", connectionConfig.password());
526+
527+
// when
528+
main.connectMaybeInteractively(shell, connectionConfig, true, true);
529+
530+
// Multiple databases are only available from 4.0
531+
assumeTrue(majorVersion( shell.getServerVersion() ) >= 4);
532+
533+
// then
534+
// should be connected
535+
assertTrue(shell.isConnected());
536+
// should have prompted and set the username and password
537+
String expectedLoginOutput = format( "username: neo4j%npassword: ***%n" );
538+
assertEquals(expectedLoginOutput, baos.toString());
539+
assertEquals("neo4j", connectionConfig.username());
540+
assertEquals("neo", connectionConfig.password());
541+
542+
// Stop the default database
543+
shell.execute(":use " + DatabaseManager.SYSTEM_DB_NAME);
544+
shell.execute("STOP DATABASE " + DatabaseManager.DEFAULT_DEFAULT_DB_NAME);
545+
546+
try {
547+
// Should get exception that database is unavailable when trying to connect
548+
exception.expect(TransientException.class);
549+
exception.expectMessage("Database 'neo4j' is unavailable");
550+
shell.execute(":use");
551+
} finally {
552+
// Start the default database again
553+
ensureDefaultDatabaseStarted();
554+
}
555+
}
556+
377557
private String executeFileNonInteractively(String filename) throws Exception {
378558
return executeFileNonInteractively(filename, mock(Logger.class));
379559
}

‎cypher-shell/src/main/java/org/neo4j/shell/CypherShell.java‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package org.neo4j.shell;
22

3+
import org.neo4j.driver.exceptions.DiscoveryException;
34
import org.neo4j.driver.exceptions.Neo4jException;
5+
import org.neo4j.driver.exceptions.ServiceUnavailableException;
46
import org.neo4j.shell.commands.Command;
57
import org.neo4j.shell.commands.CommandExecutable;
68
import org.neo4j.shell.commands.CommandHelper;
@@ -98,7 +100,7 @@ private void executeCypher(@Nonnull final String cypher) throws CommandException
98100
});
99101
lastNeo4jErrorCode = null;
100102
} catch (Neo4jException e) {
101-
lastNeo4jErrorCode = e.code();
103+
lastNeo4jErrorCode = getErrorCode(e);
102104
throw e;
103105
}
104106
}
@@ -160,7 +162,7 @@ public Optional<List<BoltResult>> commitTransaction() throws CommandException {
160162
lastNeo4jErrorCode = null;
161163
return results;
162164
} catch (Neo4jException e) {
163-
lastNeo4jErrorCode = e.code();
165+
lastNeo4jErrorCode = getErrorCode(e);
164166
throw e;
165167
}
166168
}
@@ -175,7 +177,7 @@ public boolean isTransactionOpen() {
175177
return boltStateHandler.isTransactionOpen();
176178
}
177179

178-
void setCommandHelper(@Nonnull CommandHelper commandHelper) {
180+
public void setCommandHelper(@Nonnull CommandHelper commandHelper) {
179181
this.commandHelper = commandHelper;
180182
}
181183

@@ -195,7 +197,7 @@ public void setActiveDatabase(String databaseName) throws CommandException
195197
boltStateHandler.setActiveDatabase(databaseName);
196198
lastNeo4jErrorCode = null;
197199
} catch (Neo4jException e) {
198-
lastNeo4jErrorCode = e.code();
200+
lastNeo4jErrorCode = getErrorCode(e);
199201
throw e;
200202
}
201203
}
@@ -230,4 +232,23 @@ public void changePassword(@Nonnull ConnectionConfig connectionConfig) {
230232
public void disconnect() {
231233
boltStateHandler.disconnect();
232234
}
235+
236+
private String getErrorCode(Neo4jException e) {
237+
Neo4jException statusException = e;
238+
239+
// If we encountered a later suppressed Neo4jException we use that as the basis for the status instead
240+
Throwable[] suppressed = e.getSuppressed();
241+
for (Throwable s : suppressed) {
242+
if (s instanceof Neo4jException) {
243+
statusException = (Neo4jException) s;
244+
break;
245+
}
246+
}
247+
248+
if (statusException instanceof ServiceUnavailableException || statusException instanceof DiscoveryException) {
249+
// Treat this the same way as a DatabaseUnavailable error for now.
250+
return DATABASE_UNAVAILABLE_ERROR_CODE;
251+
}
252+
return statusException.code();
253+
}
233254
}

‎cypher-shell/src/main/java/org/neo4j/shell/cli/InteractiveShellRunner.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public class InteractiveShellRunner implements ShellRunner, SignalHandler {
3939
private final static String TRANSACTION_PROMPT = "# ";
4040
private final static String USERNAME_DB_DELIMITER = "@";
4141
private final static int ONELINE_PROMPT_MAX_LENGTH = 50;
42-
private static final String UNRESOLVED_DEFAULT_DB_PROPMPT_TEXT = "<default_database>";
43-
private static final String DATABASE_UNAVAILABLE_ERROR_PROMPT_TEXT = "[UNAVAILABLE]";
42+
static final String UNRESOLVED_DEFAULT_DB_PROPMPT_TEXT = "<default_database>";
43+
static final String DATABASE_UNAVAILABLE_ERROR_PROMPT_TEXT = "[UNAVAILABLE]";
4444

4545
// Need to know if we are currently executing when catch Ctrl-C, needs to be atomic due to
4646
// being called from different thread
@@ -166,14 +166,11 @@ AnsiFormattedText updateAndGetPrompt() {
166166
}
167167

168168
String databaseName = databaseManager.getActualDatabaseAsReportedByServer();
169-
if (databaseName == null) {
169+
if (databaseName == null || ABSENT_DB_NAME.equals(databaseName)) {
170170
// We have failed to get a successful response from the connection ping query
171171
// Build the prompt from the db name as set by the user + a suffix indicating that we are in a disconnected state
172172
String dbNameSetByUser = databaseManager.getActiveDatabaseAsSetByUser();
173173
databaseName = ABSENT_DB_NAME.equals(dbNameSetByUser)? UNRESOLVED_DEFAULT_DB_PROPMPT_TEXT : dbNameSetByUser;
174-
} else if (ABSENT_DB_NAME.equals(databaseName)) {
175-
// The driver did not give us a database name in the response from the connection ping query
176-
databaseName = UNRESOLVED_DEFAULT_DB_PROPMPT_TEXT;
177174
}
178175

179176
String errorSuffix = getErrorPrompt(executer.lastNeo4jErrorCode());

‎cypher-shell/src/main/java/org/neo4j/shell/log/AnsiLogger.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import org.fusesource.jansi.Ansi;
44
import org.fusesource.jansi.AnsiConsole;
55
import org.neo4j.driver.exceptions.ClientException;
6+
import org.neo4j.driver.exceptions.DiscoveryException;
7+
import org.neo4j.driver.exceptions.ServiceUnavailableException;
68
import org.neo4j.shell.cli.Format;
79
import org.neo4j.shell.exception.AnsiFormattedException;
810

@@ -132,10 +134,23 @@ String getFormattedMessage(@Nonnull final Throwable e) {
132134
.append("\nor as environment variable(s), NEO4J_USERNAME, and NEO4J_PASSWORD respectively.")
133135
.append("\nSee --help for more info.");
134136
} else {
135-
if (e.getMessage() != null) {
136-
msg = msg.append(e.getMessage());
137+
Throwable cause = e;
138+
139+
// Get the suppressed root cause of ServiceUnavailableExceptions
140+
if (e instanceof ServiceUnavailableException) {
141+
Throwable[] suppressed = e.getSuppressed();
142+
for (Throwable s : suppressed) {
143+
if (s instanceof DiscoveryException ) {
144+
cause = getRootCause(s);
145+
break;
146+
}
147+
}
148+
}
149+
150+
if (cause.getMessage() != null) {
151+
msg = msg.append(cause.getMessage());
137152
} else {
138-
msg = msg.append(e.getClass().getSimpleName());
153+
msg = msg.append(cause.getClass().getSimpleName());
139154
}
140155
}
141156
}

‎cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void setActiveDatabase(String databaseName) throws CommandException
8383
try {
8484
reconnect(true);
8585
}
86-
catch (ClientException e2) {
86+
catch (Exception e2) {
8787
e.addSuppressed(e2);
8888
}
8989
}
@@ -174,9 +174,6 @@ private void reconnect() {
174174
}
175175

176176
private void reconnect(boolean keepBookmark) {
177-
// This will already throw an exception if there is no connectivity
178-
driver.verifyConnectivity();
179-
180177
SessionConfig.Builder builder = SessionConfig.builder();
181178
builder.withDefaultAccessMode(AccessMode.WRITE);
182179
if (!ABSENT_DB_NAME.equals(activeDatabaseNameAsSetByUser)) {
@@ -268,9 +265,6 @@ public void changePassword(@Nonnull ConnectionConfig connectionConfig) {
268265
try {
269266
driver = getDriver(connectionConfig, authToken);
270267

271-
// This will already throw an exception if there is no connectivity
272-
driver.verifyConnectivity();
273-
274268
SessionConfig.Builder builder = SessionConfig.builder()
275269
.withDefaultAccessMode(AccessMode.WRITE)
276270
.withDatabase(SYSTEM_DB_NAME);

0 commit comments

Comments
 (0)