From 7656efe7e8d8e65cc57bfd3724021017629b6aa6 Mon Sep 17 00:00:00 2001 From: Gregory Woods Date: Thu, 16 Jul 2020 14:08:15 +0100 Subject: [PATCH] Revert "Since Server v4.0 the server agent received in response to the Hello message is unreliable. Instead, since the server and Bolt protocol versions are aligned, we use the protocol version instead for all connections to Server 4.0 and higher (#708) (#718)" This reverts commit 31d3495a16aaf02a78d1ebfc70b0a73f0f554f46. --- .../handlers/HelloResponseHandler.java | 16 ++-------- .../internal/messaging/v3/BoltProtocolV3.java | 2 +- .../handlers/HelloResponseHandlerTest.java | 31 +++++-------------- 3 files changed, 12 insertions(+), 37 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/handlers/HelloResponseHandler.java b/driver/src/main/java/org/neo4j/driver/internal/handlers/HelloResponseHandler.java index c1acdb7131..18f669ead0 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/handlers/HelloResponseHandler.java +++ b/driver/src/main/java/org/neo4j/driver/internal/handlers/HelloResponseHandler.java @@ -37,13 +37,11 @@ public class HelloResponseHandler implements ResponseHandler private final ChannelPromise connectionInitializedPromise; private final Channel channel; - private final int protocolVersion; - public HelloResponseHandler( ChannelPromise connectionInitializedPromise, int protocolVersion ) + public HelloResponseHandler( ChannelPromise connectionInitializedPromise ) { this.connectionInitializedPromise = connectionInitializedPromise; this.channel = connectionInitializedPromise.channel(); - this.protocolVersion = protocolVersion; } @Override @@ -51,16 +49,8 @@ public void onSuccess( Map metadata ) { try { - // From Server V4 extracting server from metadata in the success message is unreliable - // so we fix the Server version against the Bolt Protocol version for Server V4 and above. - if ( protocolVersion == 4 ) - { - setServerVersion( channel, ServerVersion.v4_0_0 ); - } - else - { - setServerVersion( channel, extractNeo4jServerVersion( metadata ) ); - } + ServerVersion serverVersion = extractNeo4jServerVersion( metadata ); + setServerVersion( channel, serverVersion ); String connectionId = extractConnectionId( metadata ); setConnectionId( channel, connectionId ); diff --git a/driver/src/main/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3.java b/driver/src/main/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3.java index 565255ffe4..c6f951382c 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3.java +++ b/driver/src/main/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3.java @@ -80,7 +80,7 @@ public void initializeChannel( String userAgent, Map authToken, Ch Channel channel = channelInitializedPromise.channel(); HelloMessage message = new HelloMessage( userAgent, authToken ); - HelloResponseHandler handler = new HelloResponseHandler( channelInitializedPromise, version() ); + HelloResponseHandler handler = new HelloResponseHandler( channelInitializedPromise ); messageDispatcher( channel ).enqueue( handler ); channel.writeAndFlush( message, channel.voidPromise() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/handlers/HelloResponseHandlerTest.java b/driver/src/test/java/org/neo4j/driver/internal/handlers/HelloResponseHandlerTest.java index 88d6394ed3..7a142e03b5 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/handlers/HelloResponseHandlerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/handlers/HelloResponseHandlerTest.java @@ -37,7 +37,6 @@ import org.neo4j.driver.internal.async.inbound.InboundMessageDispatcher; import org.neo4j.driver.internal.async.outbound.OutboundMessageHandler; import org.neo4j.driver.internal.messaging.v1.MessageFormatV1; -import org.neo4j.driver.internal.util.ServerVersion; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -74,7 +73,7 @@ void tearDown() void shouldSetServerVersionOnChannel() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( anyServerVersion(), "bolt-1" ); handler.onSuccess( metadata ); @@ -87,7 +86,7 @@ void shouldSetServerVersionOnChannel() void shouldThrowWhenServerVersionNotReturned() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( null, "bolt-1" ); assertThrows( UntrustedServerException.class, () -> handler.onSuccess( metadata ) ); @@ -100,7 +99,7 @@ void shouldThrowWhenServerVersionNotReturned() void shouldThrowWhenServerVersionIsNull() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( Values.NULL, "bolt-x" ); assertThrows( UntrustedServerException.class, () -> handler.onSuccess( metadata ) ); @@ -113,7 +112,7 @@ void shouldThrowWhenServerVersionIsNull() void shouldThrowWhenServerVersionCantBeParsed() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( "WrongServerVersion", "bolt-x" ); assertThrows( IllegalArgumentException.class, () -> handler.onSuccess( metadata ) ); @@ -122,25 +121,11 @@ void shouldThrowWhenServerVersionCantBeParsed() assertTrue( channel.closeFuture().isDone() ); // channel was closed } - @Test - void shouldUseProtocolVersionForServerVersionWhenConnectedWithBoltV4() - { - ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 4 ); - - // server used in metadata should be ignored - Map metadata = metadata( ServerVersion.vInDev, "bolt-1" ); - handler.onSuccess( metadata ); - - assertTrue( channelPromise.isSuccess() ); - assertEquals( ServerVersion.v4_0_0, serverVersion( channel ) ); - } - @Test void shouldSetConnectionIdOnChannel() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( anyServerVersion(), "bolt-42" ); handler.onSuccess( metadata ); @@ -153,7 +138,7 @@ void shouldSetConnectionIdOnChannel() void shouldThrowWhenConnectionIdNotReturned() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( anyServerVersion(), null ); assertThrows( IllegalStateException.class, () -> handler.onSuccess( metadata ) ); @@ -166,7 +151,7 @@ void shouldThrowWhenConnectionIdNotReturned() void shouldThrowWhenConnectionIdIsNull() { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); Map metadata = metadata( anyServerVersion(), Values.NULL ); assertThrows( IllegalStateException.class, () -> handler.onSuccess( metadata ) ); @@ -179,7 +164,7 @@ void shouldThrowWhenConnectionIdIsNull() void shouldCloseChannelOnFailure() throws Exception { ChannelPromise channelPromise = channel.newPromise(); - HelloResponseHandler handler = new HelloResponseHandler( channelPromise, 3 ); + HelloResponseHandler handler = new HelloResponseHandler( channelPromise ); RuntimeException error = new RuntimeException( "Hi!" ); handler.onFailure( error );