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

Fix getting server version and database name on failure #178

Merged
merged 1 commit into from
Nov 22, 2019
Merged

Fix getting server version and database name on failure #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,8 @@ private void reconnect(boolean keepBookmark) {
driver.verifyConnectivity();

SessionConfig.Builder builder = SessionConfig.builder();
builder.withDefaultAccessMode( AccessMode.WRITE );
if ( !ABSENT_DB_NAME.equals( activeDatabaseNameAsSetByUser ) )
{
builder.withDefaultAccessMode(AccessMode.WRITE);
if (!ABSENT_DB_NAME.equals(activeDatabaseNameAsSetByUser)) {
builder.withDatabase( activeDatabaseNameAsSetByUser );
}
if (session != null && keepBookmark) {
Expand All @@ -184,9 +183,17 @@ private void reconnect(boolean keepBookmark) {

resetActualDbName(); // Set this to null first in case run throws an exception
StatementResult run = session.run(query);
ResultSummary summary = run.consume();
this.version = summary.server().version();
updateActualDbName(summary);
ResultSummary summary = null;
try {
summary = run.consume();
} finally {
// Since run.consume() can throw the first time we have to go through this extra hoop to get the summary
Copy link
Collaborator

@pontusmelke pontusmelke Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Is this known/documented behavior?

Copy link
Collaborator Author

@henriknyman henriknyman Nov 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it throws because it is not authorized to execute the ping query when password change is required in 4.0 anymore. It just started happening when packaged with the very latest Neo4j, but I wonder wether this really changed that recently. Maybe the neo4j docker image that is used to run integration tests is too far behind or there is some problem with the Team City test config parameters for 4.0.

Copy link
Collaborator Author

@henriknyman henriknyman Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pontusmelke Ideally we want to get the version and database name by the simplest possible Bolt message or command that is always allowed to run (at least if authenticated), instead of having to execute an actual query, but this should do until we get Neo4j server support and/or driver support.

if (summary == null) {
summary = run.consume();
}
this.version = summary.server().version();
updateActualDbName(summary);
}
}

@Nonnull
Expand Down
3 changes: 3 additions & 0 deletions cypher-shell/src/main/java/org/neo4j/shell/util/Versions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ public static Version version(String version) {
if (version == null) {
throw new AssertionError("null is not a valid version string");
}
if (version.isEmpty()) {
return new Version(0, 0, 0);
}
//remove -alpha, and -beta etc
int offset = version.indexOf("-");
if (offset > 0) {
Expand Down
34 changes: 34 additions & 0 deletions cypher-shell/src/test/java/org/neo4j/shell/util/VersionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.neo4j.shell.util;

import org.junit.Test;

import static org.junit.Assert.assertEquals;

public class VersionsTest
{
@Test
public void shouldWorkForEmptyString() throws Exception {
assertEquals(0, Versions.version("").compareTo(Versions.version("0.0.0")));
assertEquals(0, Versions.majorVersion(""));
assertEquals(0, Versions.minorVersion(""));
assertEquals(0, Versions.patch(""));
}

@Test
public void shouldWorkForReleaseVersion() throws Exception {
String versionString = "3.4.5";
assertEquals(0, Versions.version(versionString).compareTo(Versions.version("3.4.5")));
assertEquals(3, Versions.majorVersion(versionString));
assertEquals(4, Versions.minorVersion(versionString));
assertEquals(5, Versions.patch(versionString));
}

@Test
public void shouldWorkForPreReleaseVersion() throws Exception {
String versionString = "3.4.55-beta99";
assertEquals(0, Versions.version(versionString).compareTo(Versions.version("3.4.55")));
assertEquals(3, Versions.majorVersion(versionString));
assertEquals(4, Versions.minorVersion(versionString));
assertEquals(55, Versions.patch(versionString));
}
}