diff --git a/driver/src/main/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcher.java b/driver/src/main/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcher.java index c1f08de21c..21ecc9652f 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcher.java +++ b/driver/src/main/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcher.java @@ -281,4 +281,10 @@ enum MessageType void run( MessageType messageType ); } + + // For testing only + Logger getLog() + { + return log; + } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/messaging/request/HelloMessage.java b/driver/src/main/java/org/neo4j/driver/internal/messaging/request/HelloMessage.java index 5719e745fa..1de10640f4 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/messaging/request/HelloMessage.java +++ b/driver/src/main/java/org/neo4j/driver/internal/messaging/request/HelloMessage.java @@ -78,7 +78,10 @@ private static Map buildMetadata( String userAgent, Map result = new HashMap<>( authToken ); result.put( USER_AGENT_METADATA_KEY, value( userAgent ) ); - result.put( ROUTING_CONTEXT_METADATA_KEY, value( routingContext ) ); + if ( routingContext != null ) + { + result.put( ROUTING_CONTEXT_METADATA_KEY, value( routingContext ) ); + } return result; } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitIT.java b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitIT.java index a96befe61c..d28e1ee4cc 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitIT.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverBoltKitIT.java @@ -22,7 +22,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -30,7 +29,6 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import java.util.function.Consumer; import org.neo4j.driver.AccessMode; @@ -39,8 +37,6 @@ import org.neo4j.driver.Config; import org.neo4j.driver.Driver; import org.neo4j.driver.GraphDatabase; -import org.neo4j.driver.Logger; -import org.neo4j.driver.Record; import org.neo4j.driver.Result; import org.neo4j.driver.Session; import org.neo4j.driver.Transaction; @@ -67,18 +63,11 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import static org.neo4j.driver.SessionConfig.builder; import static org.neo4j.driver.SessionConfig.forDatabase; -import static org.neo4j.driver.Values.parameters; import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.util.StubServer.INSECURE_CONFIG; import static org.neo4j.driver.util.StubServer.insecureBuilder; -import static org.neo4j.driver.util.TestUtil.asOrderedSet; import static org.neo4j.driver.util.TestUtil.await; class DirectDriverBoltKitIT @@ -97,140 +86,6 @@ public void killServers() stubController.reset(); } - @Test - void shouldBeAbleRunCypher() throws Exception - { - StubServer server = stubController.startStub( "return_x.script", 9001 ); - URI uri = URI.create( "bolt://127.0.0.1:9001" ); - int x; - - try ( Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG ) ) - { - try ( Session session = driver.session() ) - { - Record record = session.run( "RETURN {x}", parameters( "x", 1 ) ).single(); - x = record.get( 0 ).asInt(); - } - } - - assertThat( x, equalTo( 1 ) ); - assertThat( server.exitStatus(), equalTo( 0 ) ); - } - - @Test - void shouldSendMultipleBookmarks() throws Exception - { - StubServer server = stubController.startStub( "multiple_bookmarks.script", 9001 ); - - Bookmark bookmarks = InternalBookmark.parse( asOrderedSet( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", - "neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16", "neo4j:bookmark:v1:tx68" ) ); - - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ); - Session session = driver.session( builder().withBookmarks( bookmarks ).build() ) ) - { - try ( Transaction tx = session.beginTransaction() ) - { - tx.run( "CREATE (n {name:'Bob'})" ); - tx.commit(); - } - - assertEquals( InternalBookmark.parse( "neo4j:bookmark:v1:tx95" ), session.lastBookmark() ); - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - - @Test - void shouldSendNullRoutingContextForBoltUri() throws Exception - { - StubServer server = StubServer.start( "hello_with_routing_context_bolt.script", 9001 ); - - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ); - Session session = driver.session() ) - { - List names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() ); - assertEquals( asList( "Foo", "Bar" ), names ); - - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - - @Test - void shouldLogConnectionIdInDebugMode() throws Exception - { - StubServer server = stubController.startStub( "hello_run_exit.script", 9001 ); - - Logger logger = mock( Logger.class ); - when( logger.isDebugEnabled() ).thenReturn( true ); - - Config config = Config.builder() - .withLogging( ignore -> logger ) - .withoutEncryption() - .build(); - - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", config ); - Session session = driver.session() ) - { - List names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() ); - assertEquals( asList( "Foo", "Bar" ), names ); - - ArgumentCaptor messageCaptor = ArgumentCaptor.forClass( String.class ); - verify( logger, atLeastOnce() ).debug( messageCaptor.capture(), any( Object[].class ) ); - - Optional logMessageWithConnectionId = messageCaptor.getAllValues() - .stream() - .filter( line -> line.contains( "bolt-123456789" ) ) - .findAny(); - - assertTrue( logMessageWithConnectionId.isPresent(), - "Expected log call did not happen. All debug log calls:\n" + String.join( "\n", messageCaptor.getAllValues() ) ); - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - - @Test - void shouldSendReadAccessModeInQueryMetadata() throws Exception - { - StubServer server = stubController.startStub( "hello_run_exit_read.script", 9001 ); - - - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ); - Session session = driver.session( builder().withDefaultAccessMode( AccessMode.READ ).build() ) ) - { - List names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() ); - assertEquals( asList( "Foo", "Bar" ), names ); - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - - @Test - void shouldNotSendWriteAccessModeInQueryMetadata() throws Exception - { - StubServer server = stubController.startStub( "hello_run_exit.script", 9001 ); - - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ); - Session session = driver.session( builder().withDefaultAccessMode( AccessMode.WRITE ).build() ) ) - { - List names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() ); - assertEquals( asList( "Foo", "Bar" ), names ); - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - @Test void shouldCloseChannelWhenResetFails() throws Exception { @@ -597,25 +452,6 @@ void shouldBeAbleHandleNOOPsDuringRunCypher() throws Exception assertThat( server.exitStatus(), equalTo( 0 ) ); } - @Test - void shouldSendCustomerUserAgentInHelloMessage() throws Exception - { - StubServer server = stubController.startStub( "hello_with_custom_user_agent.script", 9001 ); - - Config config = Config.builder().withUserAgent( "AwesomeClient" ).build(); - - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", config ); - Session session = driver.session( builder().withDefaultAccessMode( AccessMode.WRITE ).build() ) ) - { - List names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() ); - assertEquals( asList( "Foo", "Bar" ), names ); - } - finally - { - assertEquals( 0, server.exitStatus() ); - } - } - private static void testTxCloseErrorPropagation( String script, Consumer txAction, String expectedErrorMessage ) throws Exception { diff --git a/driver/src/test/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcherTest.java b/driver/src/test/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcherTest.java index 313f47b4fd..a3c967af99 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcherTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcherTest.java @@ -23,24 +23,38 @@ import io.netty.channel.DefaultChannelId; import io.netty.util.Attribute; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import java.util.HashMap; import java.util.Map; -import org.neo4j.driver.internal.spi.ResponseHandler; -import org.neo4j.driver.internal.value.IntegerValue; +import org.neo4j.driver.Logger; +import org.neo4j.driver.Logging; import org.neo4j.driver.Value; +import org.neo4j.driver.Values; import org.neo4j.driver.exceptions.ClientException; import org.neo4j.driver.exceptions.Neo4jException; +import org.neo4j.driver.internal.logging.ChannelActivityLogger; +import org.neo4j.driver.internal.messaging.Message; +import org.neo4j.driver.internal.messaging.response.FailureMessage; +import org.neo4j.driver.internal.messaging.response.IgnoredMessage; +import org.neo4j.driver.internal.messaging.response.RecordMessage; +import org.neo4j.driver.internal.messaging.response.SuccessMessage; +import org.neo4j.driver.internal.spi.ResponseHandler; +import org.neo4j.driver.internal.value.IntegerValue; import static java.util.Collections.emptyMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.inOrder; @@ -49,9 +63,9 @@ import static org.mockito.Mockito.only; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.neo4j.driver.Values.value; import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.internal.messaging.request.ResetMessage.RESET; -import static org.neo4j.driver.Values.value; class InboundMessageDispatcherTest { @@ -354,6 +368,69 @@ void shouldReEnableAutoReadWhenAutoReadManagingHandlerIsRemoved() verify( channel.config() ).setAutoRead( anyBoolean() ); } + @ParameterizedTest + @ValueSource( classes = {SuccessMessage.class, FailureMessage.class, RecordMessage.class, IgnoredMessage.class} ) + void shouldCreateChannelActivityLoggerAndLogDebugMessageOnMessageHandling( Class message ) + { + // GIVEN + Channel channel = newChannelMock(); + Logging logging = mock( Logging.class ); + Logger logger = mock( Logger.class ); + when( logger.isDebugEnabled() ).thenReturn( true ); + when( logging.getLog( InboundMessageDispatcher.class.getSimpleName() ) ).thenReturn( logger ); + InboundMessageDispatcher dispatcher = new InboundMessageDispatcher( channel, logging ); + ResponseHandler handler = mock( ResponseHandler.class ); + dispatcher.enqueue( handler ); + + // WHEN + if ( SuccessMessage.class.isAssignableFrom( message ) ) + { + dispatcher.handleSuccessMessage( new HashMap<>() ); + } + else if ( FailureMessage.class.isAssignableFrom( message ) ) + { + dispatcher.handleFailureMessage( FAILURE_CODE, FAILURE_MESSAGE ); + } + else if ( RecordMessage.class.isAssignableFrom( message ) ) + { + dispatcher.handleRecordMessage( Values.values() ); + } + else if ( IgnoredMessage.class.isAssignableFrom( message ) ) + { + dispatcher.handleIgnoredMessage(); + } + else + { + fail( "Unexpected message type parameter provided" ); + } + + // THEN + assertTrue( dispatcher.getLog() instanceof ChannelActivityLogger ); + verify( logger ).debug( anyString(), any( Object.class ) ); + } + + @Test + void shouldCreateChannelActivityLoggerAndLogDebugMessageOnChannelError() + { + // GIVEN + Channel channel = newChannelMock(); + Logging logging = mock( Logging.class ); + Logger logger = mock( Logger.class ); + when( logger.isDebugEnabled() ).thenReturn( true ); + when( logging.getLog( InboundMessageDispatcher.class.getSimpleName() ) ).thenReturn( logger ); + InboundMessageDispatcher dispatcher = new InboundMessageDispatcher( channel, logging ); + ResponseHandler handler = mock( ResponseHandler.class ); + dispatcher.enqueue( handler ); + Throwable throwable = mock( Throwable.class ); + + // WHEN + dispatcher.handleChannelError( throwable ); + + // THEN + assertTrue( dispatcher.getLog() instanceof ChannelActivityLogger ); + verify( logger ).debug( anyString(), eq( throwable ) ); + } + private static void verifyFailure( ResponseHandler handler ) { ArgumentCaptor captor = ArgumentCaptor.forClass( Neo4jException.class ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/messaging/encode/HelloMessageEncoderTest.java b/driver/src/test/java/org/neo4j/driver/internal/messaging/encode/HelloMessageEncoderTest.java index 9f011baf2b..ae3a62e52b 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/messaging/encode/HelloMessageEncoderTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/messaging/encode/HelloMessageEncoderTest.java @@ -21,20 +21,18 @@ import org.junit.jupiter.api.Test; import org.mockito.InOrder; -import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.neo4j.driver.Value; import org.neo4j.driver.internal.messaging.ValuePacker; import org.neo4j.driver.internal.messaging.request.HelloMessage; -import org.neo4j.driver.Value; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; -import static org.neo4j.driver.Values.NULL; -import static org.neo4j.driver.internal.messaging.request.PullAllMessage.PULL_ALL; import static org.neo4j.driver.Values.value; +import static org.neo4j.driver.internal.messaging.request.PullAllMessage.PULL_ALL; class HelloMessageEncoderTest { @@ -55,7 +53,6 @@ void shouldEncodeHelloMessage() throws Exception Map expectedMetadata = new HashMap<>( authToken ); expectedMetadata.put( "user_agent", value( "MyDriver" ) ); - expectedMetadata.put( "routing", NULL ); order.verify( packer ).pack( expectedMetadata ); } diff --git a/driver/src/test/resources/hello_run_exit.script b/driver/src/test/resources/hello_run_exit.script deleted file mode 100644 index 76f4004823..0000000000 --- a/driver/src/test/resources/hello_run_exit.script +++ /dev/null @@ -1,13 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO GOODBYE - -C: HELLO {"scheme": "none", "user_agent": "neo4j-java/dev", "routing": null} -S: SUCCESS {"server": "Neo4j/3.5.0", "connection_id": "bolt-123456789"} -C: RUN "MATCH (n) RETURN n.name" {} {} - PULL_ALL -S: SUCCESS {"fields": ["n.name"]} - RECORD ["Foo"] - RECORD ["Bar"] - SUCCESS {} -S: diff --git a/driver/src/test/resources/hello_run_exit_read.script b/driver/src/test/resources/hello_run_exit_read.script deleted file mode 100644 index ce312d002b..0000000000 --- a/driver/src/test/resources/hello_run_exit_read.script +++ /dev/null @@ -1,12 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE - -C: RUN "MATCH (n) RETURN n.name" {} {"mode": "r"} - PULL_ALL -S: SUCCESS {"fields": ["n.name"]} - RECORD ["Foo"] - RECORD ["Bar"] - SUCCESS {} -S: diff --git a/driver/src/test/resources/hello_with_custom_user_agent.script b/driver/src/test/resources/hello_with_custom_user_agent.script deleted file mode 100644 index e1beea7401..0000000000 --- a/driver/src/test/resources/hello_with_custom_user_agent.script +++ /dev/null @@ -1,13 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO GOODBYE - -C: HELLO {"scheme": "none", "user_agent": "AwesomeClient", "routing": null} -S: SUCCESS {"server": "Neo4j/3.5.0", "connection_id": "bolt-123456789"} -C: RUN "MATCH (n) RETURN n.name" {} {} - PULL_ALL -S: SUCCESS {"fields": ["n.name"]} - RECORD ["Foo"] - RECORD ["Bar"] - SUCCESS {} -S: diff --git a/driver/src/test/resources/hello_with_routing_context_bolt.script b/driver/src/test/resources/hello_with_routing_context_bolt.script deleted file mode 100644 index 1f762f2913..0000000000 --- a/driver/src/test/resources/hello_with_routing_context_bolt.script +++ /dev/null @@ -1,12 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO GOODBYE - -C: HELLO {"scheme": "none", "user_agent": "neo4j-java/dev", "routing": null} -S: SUCCESS {"server": "Neo4j/3.5.0", "connection_id": "bolt-123456789"} -C: RUN "MATCH (n) RETURN n.name" {} {} - PULL_ALL -S: SUCCESS {"fields": ["n.name"]} - RECORD ["Foo"] - RECORD ["Bar"] - SUCCESS {} diff --git a/driver/src/test/resources/multiple_bookmarks.script b/driver/src/test/resources/multiple_bookmarks.script deleted file mode 100644 index 3a6df85aec..0000000000 --- a/driver/src/test/resources/multiple_bookmarks.script +++ /dev/null @@ -1,13 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE - -C: BEGIN {"bookmarks": ["neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", "neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16", "neo4j:bookmark:v1:tx68"]} -S: SUCCESS {} -C: RUN "CREATE (n {name:'Bob'})" {} {} - PULL_ALL -S: SUCCESS {} - SUCCESS {} -C: COMMIT -S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx95"} diff --git a/driver/src/test/resources/return_x.script b/driver/src/test/resources/return_x.script deleted file mode 100644 index 1deaf35708..0000000000 --- a/driver/src/test/resources/return_x.script +++ /dev/null @@ -1,10 +0,0 @@ -!: BOLT 3 -!: AUTO RESET -!: AUTO HELLO -!: AUTO GOODBYE - -C: RUN "RETURN {x}" {"x": 1} {} - PULL_ALL -S: SUCCESS {"fields": ["x"]} - RECORD [1] - SUCCESS {} diff --git a/driver/src/test/resources/untrusted_server.script b/driver/src/test/resources/untrusted_server.script index 49716f6784..ee7ced5708 100644 --- a/driver/src/test/resources/untrusted_server.script +++ b/driver/src/test/resources/untrusted_server.script @@ -2,5 +2,5 @@ !: AUTO RESET !: AUTO GOODBYE -C: HELLO {"scheme": "none", "user_agent": "neo4j-java/dev", "routing": null} +C: HELLO {"scheme": "none", "user_agent": "neo4j-java/dev"} S: SUCCESS {"server": "AgentSmith/0.0.1", "connection_id": "bolt-123456789"}