From a8bc968dd9d36e9ce09a11eb1b2b5b293978d3c6 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Mon, 21 Mar 2022 12:21:22 +0000 Subject: [PATCH] Throw ProtocolException when QueryType is unknown or missing This update ensures that `ProtocolException` is thrown when server provides an unknown `QueryType` or when it is missing on `SUCCESS` response that expects it. --- .../LegacyPullAllResponseHandler.java | 30 +++++++++++---- .../pulln/BasicPullResponseHandler.java | 34 +++++++++++------ .../internal/util/MetadataExtractor.java | 23 ++++++++--- .../org/neo4j/driver/summary/QueryType.java | 24 +++++++++++- .../driver/internal/InternalResultTest.java | 4 +- .../LegacyPullAllResponseHandlerTest.java | 5 +-- .../PullAllResponseHandlerTestBase.java | 30 +++++++++------ .../BasicPullResponseHandlerTestBase.java | 38 ++++++++++++------- ...ionPullResponseCompletionListenerTest.java | 5 +-- ...ionPullResponseCompletionListenerTest.java | 3 +- .../messaging/v3/BoltProtocolV3Test.java | 5 ++- .../reactive/util/ListBasedPullHandler.java | 3 +- .../internal/util/MetadataExtractorTest.java | 28 +++++++------- .../java/org/neo4j/driver/util/TestUtil.java | 4 +- .../backend/messages/requests/StartTest.java | 1 - 15 files changed, 157 insertions(+), 80 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandler.java b/driver/src/main/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandler.java index 6cef4dc9d0..4d42d72cfb 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandler.java +++ b/driver/src/main/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandler.java @@ -30,6 +30,7 @@ import org.neo4j.driver.Query; import org.neo4j.driver.Record; import org.neo4j.driver.Value; +import org.neo4j.driver.exceptions.Neo4jException; import org.neo4j.driver.internal.InternalRecord; import org.neo4j.driver.internal.messaging.request.PullAllMessage; import org.neo4j.driver.internal.spi.Connection; @@ -92,19 +93,34 @@ public boolean canManageAutoRead() public synchronized void onSuccess( Map metadata ) { finished = true; - summary = extractResultSummary( metadata ); + Neo4jException exception = null; + try + { + summary = extractResultSummary( metadata, true ); + } + catch ( Neo4jException e ) + { + exception = e; + } - completionListener.afterSuccess( metadata ); + if ( exception == null ) + { + completionListener.afterSuccess( metadata ); - completeRecordFuture( null ); - completeFailureFuture( null ); + completeRecordFuture( null ); + completeFailureFuture( null ); + } + else + { + onFailure( exception ); + } } @Override public synchronized void onFailure( Throwable error ) { finished = true; - summary = extractResultSummary( emptyMap() ); + summary = extractResultSummary( emptyMap(), false ); completionListener.afterFailure( error ); @@ -332,10 +348,10 @@ private boolean completeFailureFuture( Throwable error ) return false; } - private ResultSummary extractResultSummary( Map metadata ) + private ResultSummary extractResultSummary( Map metadata, boolean enforceQueryType ) { long resultAvailableAfter = runResponseHandler.resultAvailableAfter(); - return metadataExtractor.extractSummary(query, connection, resultAvailableAfter, metadata ); + return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata, enforceQueryType ); } private void enableAutoRead() diff --git a/driver/src/main/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandler.java b/driver/src/main/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandler.java index 10234559b5..9559488d9a 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandler.java +++ b/driver/src/main/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandler.java @@ -24,6 +24,7 @@ import org.neo4j.driver.Query; import org.neo4j.driver.Record; import org.neo4j.driver.Value; +import org.neo4j.driver.exceptions.Neo4jException; import org.neo4j.driver.internal.InternalRecord; import org.neo4j.driver.internal.handlers.PullResponseCompletionListener; import org.neo4j.driver.internal.handlers.RunResponseHandler; @@ -106,15 +107,24 @@ public synchronized void cancel() protected void completeWithFailure( Throwable error ) { completionListener.afterFailure( error ); - complete( extractResultSummary( emptyMap() ), error ); + complete( extractResultSummary( emptyMap(), false ), error ); } - protected void completeWithSuccess( Map metadata ) + protected void completeWithSuccess( Map metadata, boolean enforceQueryType ) { completionListener.afterSuccess( metadata ); - ResultSummary summary = extractResultSummary( metadata ); - - complete( summary, null ); + ResultSummary summary; + Neo4jException exception = null; + try + { + summary = extractResultSummary( metadata, enforceQueryType ); + } + catch ( Neo4jException e ) + { + summary = extractResultSummary( emptyMap(), false ); + exception = e; + } + complete( summary, exception ); } protected void successHasMore() @@ -169,10 +179,10 @@ protected boolean isDone() return state.equals( State.SUCCEEDED_STATE ) || state.equals( State.FAILURE_STATE ); } - private ResultSummary extractResultSummary( Map metadata ) + private ResultSummary extractResultSummary( Map metadata, boolean enforceQueryType ) { long resultAvailableAfter = runResponseHandler.resultAvailableAfter(); - return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata ); + return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata, enforceQueryType ); } private void addToRequest( long toAdd ) @@ -247,7 +257,7 @@ enum State void onSuccess( BasicPullResponseHandler context, Map metadata ) { context.state( SUCCEEDED_STATE ); - context.completeWithSuccess( metadata ); + context.completeWithSuccess( metadata, false ); } @Override @@ -290,7 +300,7 @@ void onSuccess( BasicPullResponseHandler context, Map metadata ) else { context.state( SUCCEEDED_STATE ); - context.completeWithSuccess( metadata ); + context.completeWithSuccess( metadata, true ); } } @@ -334,7 +344,7 @@ void onSuccess( BasicPullResponseHandler context, Map metadata ) else { context.state( SUCCEEDED_STATE ); - context.completeWithSuccess( metadata ); + context.completeWithSuccess( metadata, true ); } } @@ -369,7 +379,7 @@ void cancel( BasicPullResponseHandler context ) void onSuccess( BasicPullResponseHandler context, Map metadata ) { context.state( SUCCEEDED_STATE ); - context.completeWithSuccess( metadata ); + context.completeWithSuccess( metadata, false ); } @Override @@ -403,7 +413,7 @@ void cancel( BasicPullResponseHandler context ) void onSuccess( BasicPullResponseHandler context, Map metadata ) { context.state( SUCCEEDED_STATE ); - context.completeWithSuccess( metadata ); + context.completeWithSuccess( metadata, false ); } @Override diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java b/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java index f726478c4f..e4523be60d 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java +++ b/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java @@ -21,10 +21,12 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Function; import org.neo4j.driver.Bookmark; import org.neo4j.driver.Query; import org.neo4j.driver.Value; +import org.neo4j.driver.exceptions.ProtocolException; import org.neo4j.driver.exceptions.UntrustedServerException; import org.neo4j.driver.internal.InternalBookmark; import org.neo4j.driver.internal.spi.Connection; @@ -49,6 +51,10 @@ public class MetadataExtractor { public static final int ABSENT_QUERY_ID = -1; + public static final String MISSING_TYPE_MSG = "No query type has been provided, consider updating the driver"; + private static final String UNEXPECTED_TYPE_MSG_FMT = "Unexpected query type '%s', consider updating the driver"; + private static final Function UNEXPECTED_TYPE_EXCEPTION_SUPPLIER = + ( type ) -> new ProtocolException( String.format( UNEXPECTED_TYPE_MSG_FMT, type ) ); private final String resultAvailableAfterMetadataKey; private final String resultConsumedAfterMetadataKey; @@ -98,14 +104,15 @@ public long extractResultAvailableAfter( Map metadata ) return -1; } - public ResultSummary extractSummary(Query query, Connection connection, long resultAvailableAfter, Map metadata ) + public ResultSummary extractSummary( Query query, Connection connection, long resultAvailableAfter, Map metadata, boolean enforceQueryType ) { ServerInfo serverInfo = new InternalServerInfo( connection.serverAgent(), connection.serverAddress(), connection.protocol().version() ); DatabaseInfo dbInfo = extractDatabaseInfo( metadata ); - return new InternalResultSummary(query, serverInfo, dbInfo, extractQueryType( metadata ), extractCounters( metadata ), extractPlan( metadata ), - extractProfiledPlan( metadata ), extractNotifications( metadata ), resultAvailableAfter, - extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) ); + QueryType queryType = extractQueryType( metadata, enforceQueryType ); + return new InternalResultSummary( query, serverInfo, dbInfo, queryType, extractCounters( metadata ), extractPlan( metadata ), + extractProfiledPlan( metadata ), extractNotifications( metadata ), resultAvailableAfter, + extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) ); } public static DatabaseInfo extractDatabaseInfo( Map metadata ) @@ -146,12 +153,16 @@ public static Value extractServer( Map metadata ) return versionValue; } - private static QueryType extractQueryType( Map metadata ) + private static QueryType extractQueryType( Map metadata, boolean enforce ) { Value typeValue = metadata.get( "type" ); if ( typeValue != null ) { - return QueryType.fromCode( typeValue.asString() ); + return QueryType.fromCode( typeValue.asString(), UNEXPECTED_TYPE_EXCEPTION_SUPPLIER ); + } + else if ( enforce ) + { + throw new ProtocolException( MISSING_TYPE_MSG ); } return null; } diff --git a/driver/src/main/java/org/neo4j/driver/summary/QueryType.java b/driver/src/main/java/org/neo4j/driver/summary/QueryType.java index ef07efe35b..606a2de037 100644 --- a/driver/src/main/java/org/neo4j/driver/summary/QueryType.java +++ b/driver/src/main/java/org/neo4j/driver/summary/QueryType.java @@ -18,10 +18,14 @@ */ package org.neo4j.driver.summary; +import java.util.function.Function; + import org.neo4j.driver.exceptions.ClientException; +import org.neo4j.driver.exceptions.Neo4jException; /** * The type of query executed. + * * @since 1.0 */ public enum QueryType @@ -31,7 +35,16 @@ public enum QueryType WRITE_ONLY, SCHEMA_WRITE; - public static QueryType fromCode(String type ) + private static final String UNEXPECTED_TYPE_MSG_FMT = "Unknown query type: `%s`."; + private static final Function UNEXPECTED_TYPE_EXCEPTION_SUPPLIER = + ( type ) -> new ClientException( String.format( UNEXPECTED_TYPE_MSG_FMT, type ) ); + + public static QueryType fromCode( String type ) + { + return fromCode( type, UNEXPECTED_TYPE_EXCEPTION_SUPPLIER ); + } + + public static QueryType fromCode( String type, Function exceptionFunction ) { switch ( type ) { @@ -44,7 +57,14 @@ public static QueryType fromCode(String type ) case "s": return QueryType.SCHEMA_WRITE; default: - throw new ClientException( "Unknown query type: `" + type + "`." ); + if ( exceptionFunction != null ) + { + throw exceptionFunction.apply( type ); + } + else + { + return null; + } } } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/InternalResultTest.java b/driver/src/test/java/org/neo4j/driver/internal/InternalResultTest.java index 7779cef1b3..0d21c1cffd 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/InternalResultTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/InternalResultTest.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -45,7 +46,6 @@ import org.neo4j.driver.util.Pair; import static java.util.Arrays.asList; -import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.collection.IsCollectionWithSize.hasSize; @@ -369,7 +369,7 @@ private Result createResult(int numberOfRecords ) { pullAllHandler.onRecord( new Value[]{value( "v1-" + i ), value( "v2-" + i )} ); } - pullAllHandler.onSuccess( emptyMap() ); + pullAllHandler.onSuccess( Collections.singletonMap( "type", value( "rw" ) ) ); AsyncResultCursor cursor = new AsyncResultCursorImpl( null, runHandler, pullAllHandler ); return new InternalResult( connection, new DisposableAsyncResultCursor( cursor ) ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandlerTest.java b/driver/src/test/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandlerTest.java index e0a5855f89..5cf98f8968 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandlerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandlerTest.java @@ -32,7 +32,6 @@ import static java.util.Arrays.asList; import static java.util.Collections.emptyList; -import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -107,7 +106,7 @@ void shouldNotDisableAutoReadWhenSummaryRequested() verify( connection, never() ).disableAutoRead(); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertTrue( summaryFuture.isDone() ); ResultSummary summary = await( summaryFuture ); @@ -228,7 +227,7 @@ void shouldEnableAutoReadOnConnectionWhenSummaryRequestedButNotAvailable() throw assertNull( await( handler.nextAsync() ) ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertTrue( summaryFuture.isDone() ); assertNotNull( summaryFuture.get() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/handlers/PullAllResponseHandlerTestBase.java b/driver/src/test/java/org/neo4j/driver/internal/handlers/PullAllResponseHandlerTestBase.java index c65020fc6e..52d50ddc52 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/handlers/PullAllResponseHandlerTestBase.java +++ b/driver/src/test/java/org/neo4j/driver/internal/handlers/PullAllResponseHandlerTestBase.java @@ -22,7 +22,9 @@ import java.io.IOException; import java.nio.channels.ClosedChannelException; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.function.Function; @@ -30,6 +32,7 @@ import org.neo4j.driver.Query; import org.neo4j.driver.Record; import org.neo4j.driver.Value; +import org.neo4j.driver.Values; import org.neo4j.driver.exceptions.ServiceUnavailableException; import org.neo4j.driver.exceptions.SessionExpiredException; import org.neo4j.driver.internal.BoltServerAddress; @@ -62,7 +65,7 @@ public abstract class PullAllResponseHandlerTestBase failureFuture = handler.pullAllFailureAsync().toCompletableFuture(); assertFalse( failureFuture.isDone() ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertTrue( failureFuture.isDone() ); assertNull( await( failureFuture ) ); @@ -342,7 +345,7 @@ void shouldPeekAvailableNothingAfterSuccess() void shouldPeekNothingAfterSuccess() { PullAllResponseHandler handler = newHandler(); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertNull( await( handler.peekAsync() ) ); } @@ -444,7 +447,7 @@ void shouldReturnSingleAvailableRecordInNextAsync() void shouldReturnNoRecordsWhenNoneAvailableInNextAsync() { PullAllResponseHandler handler = newHandler( asList( "key1", "key2" ) ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertNull( await( handler.nextAsync() ) ); } @@ -457,7 +460,7 @@ void shouldReturnNoRecordsWhenSuccessComesAfterNextAsync() CompletableFuture recordFuture = handler.nextAsync().toCompletableFuture(); assertFalse( recordFuture.isDone() ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertTrue( recordFuture.isDone() ); assertNull( await( recordFuture ) ); @@ -473,7 +476,7 @@ void shouldPullAllAvailableRecordsWithNextAsync() handler.onRecord( values( 11, 22, 33 ) ); handler.onRecord( values( 111, 222, 333 ) ); handler.onRecord( values( 1111, 2222, 3333 ) ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); Record record1 = await( handler.nextAsync() ); assertNotNull( record1 ); @@ -612,7 +615,7 @@ void shouldFailListAsyncWhenTransformationFunctionThrows() PullAllResponseHandler handler = newHandler( asList( "key1", "key2" ) ); handler.onRecord( values( 1, 2 ) ); handler.onRecord( values( 3, 4 ) ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); RuntimeException error = new RuntimeException( "Hi!" ); @@ -634,7 +637,7 @@ void shouldReturnEmptyListInListAsyncAfterSuccess() { PullAllResponseHandler handler = newHandler(); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); assertEquals( emptyList(), await( handler.listAsync( Function.identity() ) ) ); } @@ -648,7 +651,7 @@ void shouldReturnTransformedListInListAsync() handler.onRecord( values( 2 ) ); handler.onRecord( values( 3 ) ); handler.onRecord( values( 4 ) ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); List transformedList = await( handler.listAsync( record -> record.get( 0 ).asInt() * 2 ) ); @@ -668,7 +671,7 @@ void shouldReturnNotTransformedListInListAsync() handler.onRecord( fields1 ); handler.onRecord( fields2 ); handler.onRecord( fields3 ); - handler.onSuccess( emptyMap() ); + handler.onSuccess( metadataWithType() ); List list = await( handler.listAsync( Function.identity() ) ); @@ -704,7 +707,7 @@ protected T newHandler( List queryKeys, Connection connection ) return newHandler( new Query( "RETURN 1" ), queryKeys, connection ); } - protected abstract T newHandler(Query query, List queryKeys, Connection connection ); + protected abstract T newHandler( Query query, List queryKeys, Connection connection ); protected Connection connectionMock() { @@ -714,4 +717,9 @@ protected Connection connectionMock() when( connection.serverAgent() ).thenReturn( "Neo4j/4.2.5" ); return connection; } + + protected Map metadataWithType() + { + return Collections.singletonMap( "type", Values.value( "rw" ) ); + } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandlerTestBase.java b/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandlerTestBase.java index 7db9d635ba..e7f441c88c 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandlerTestBase.java +++ b/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandlerTestBase.java @@ -22,12 +22,15 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import java.util.Collections; import java.util.HashMap; +import java.util.Map; import java.util.function.BiConsumer; import java.util.stream.Stream; import org.neo4j.driver.Record; import org.neo4j.driver.Value; +import org.neo4j.driver.Values; import org.neo4j.driver.internal.BoltServerAddress; import org.neo4j.driver.internal.messaging.request.DiscardMessage; import org.neo4j.driver.internal.messaging.request.PullMessage; @@ -58,14 +61,14 @@ protected abstract BasicPullResponseHandler newResponseHandlerWithStatus( Connec // on success with summary @ParameterizedTest @MethodSource( "allStatus" ) - void shouldSuccessWithSummary( BasicPullResponseHandler.State state ) throws Throwable + void shouldSuccessWithSummary( BasicPullResponseHandler.State state ) { shouldHandleSuccessWithSummary( state ); } // on success with has_more @Test - void shouldRequestMoreWithHasMore() throws Throwable + void shouldRequestMoreWithHasMore() { // Given a handler in streaming state Connection conn = mockConnection(); @@ -82,13 +85,14 @@ void shouldRequestMoreWithHasMore() throws Throwable } @Test - void shouldInformSummaryConsumerSuccessWithHasMore() throws Throwable + void shouldInformSummaryConsumerSuccessWithHasMore() { // Given Connection conn = mockConnection(); BiConsumer recordConsumer = mock( BiConsumer.class ); BiConsumer summaryConsumer = mock( BiConsumer.class ); - BasicPullResponseHandler handler = newResponseHandlerWithStatus( conn, recordConsumer, summaryConsumer, BasicPullResponseHandler.State.STREAMING_STATE ); + BasicPullResponseHandler handler = + newResponseHandlerWithStatus( conn, recordConsumer, summaryConsumer, BasicPullResponseHandler.State.STREAMING_STATE ); // When handler.onSuccess( metaWithHasMoreEqualsTrue() ); @@ -101,7 +105,7 @@ void shouldInformSummaryConsumerSuccessWithHasMore() throws Throwable } @Test - void shouldDiscardIfStreamingIsCanceled() throws Throwable + void shouldDiscardIfStreamingIsCanceled() { // Given a handler in streaming state Connection conn = mockConnection(); @@ -116,20 +120,21 @@ void shouldDiscardIfStreamingIsCanceled() throws Throwable // on failure @ParameterizedTest @MethodSource( "allStatus" ) - void shouldErrorToRecordAndSummaryConsumer( BasicPullResponseHandler.State state ) throws Throwable + void shouldErrorToRecordAndSummaryConsumer( BasicPullResponseHandler.State state ) { shouldHandleFailure( state ); } // on record @Test - void shouldReportRecordInStreaming() throws Throwable + void shouldReportRecordInStreaming() { // Given a handler in streaming state Connection conn = mockConnection(); BiConsumer recordConsumer = mock( BiConsumer.class ); BiConsumer summaryConsumer = mock( BiConsumer.class ); - BasicPullResponseHandler handler = newResponseHandlerWithStatus( conn, recordConsumer, summaryConsumer, BasicPullResponseHandler.State.STREAMING_STATE ); + BasicPullResponseHandler handler = + newResponseHandlerWithStatus( conn, recordConsumer, summaryConsumer, BasicPullResponseHandler.State.STREAMING_STATE ); // When handler.onRecord( new Value[0] ); @@ -143,7 +148,7 @@ void shouldReportRecordInStreaming() throws Throwable @ParameterizedTest @MethodSource( "allStatusExceptStreaming" ) - void shouldNotReportRecordWhenNotStreaming( BasicPullResponseHandler.State state ) throws Throwable + void shouldNotReportRecordWhenNotStreaming( BasicPullResponseHandler.State state ) { // Given a handler in streaming state Connection conn = mockConnection(); @@ -162,7 +167,7 @@ void shouldNotReportRecordWhenNotStreaming( BasicPullResponseHandler.State state // request @Test - void shouldStayInStreaming() throws Throwable + void shouldStayInStreaming() { // Given Connection conn = mockConnection(); @@ -176,7 +181,7 @@ void shouldStayInStreaming() throws Throwable } @Test - void shouldPullAndSwitchStreamingInReady() throws Throwable + void shouldPullAndSwitchStreamingInReady() { // Given Connection conn = mockConnection(); @@ -192,7 +197,7 @@ void shouldPullAndSwitchStreamingInReady() throws Throwable // cancel @Test - void shouldStayInCancel() throws Throwable + void shouldStayInCancel() { // Given Connection conn = mockConnection(); @@ -207,7 +212,7 @@ void shouldStayInCancel() throws Throwable } @Test - void shouldSwitchFromStreamingToCancel() throws Throwable + void shouldSwitchFromStreamingToCancel() { // Given Connection conn = mockConnection(); @@ -222,7 +227,7 @@ void shouldSwitchFromStreamingToCancel() throws Throwable } @Test - void shouldSwitchFromReadyToCancel() throws Throwable + void shouldSwitchFromReadyToCancel() { // Given Connection conn = mockConnection(); @@ -245,6 +250,11 @@ static Connection mockConnection() return conn; } + protected Map metadataWithType() + { + return Collections.singletonMap( "type", Values.value( "rw" ) ); + } + private BasicPullResponseHandler newResponseHandlerWithStatus( Connection conn, BasicPullResponseHandler.State state ) { BiConsumer recordConsumer = mock( BiConsumer.class ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/SessionPullResponseCompletionListenerTest.java b/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/SessionPullResponseCompletionListenerTest.java index 3c9cc0ddb6..aaec17b0ff 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/SessionPullResponseCompletionListenerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/SessionPullResponseCompletionListenerTest.java @@ -18,11 +18,10 @@ */ package org.neo4j.driver.internal.handlers.pulln; -import java.util.Collections; import java.util.function.BiConsumer; -import org.neo4j.driver.Record; import org.neo4j.driver.Query; +import org.neo4j.driver.Record; import org.neo4j.driver.internal.BookmarkHolder; import org.neo4j.driver.internal.handlers.RunResponseHandler; import org.neo4j.driver.internal.handlers.SessionPullResponseCompletionListener; @@ -49,7 +48,7 @@ protected void shouldHandleSuccessWithSummary( BasicPullResponseHandler.State st PullResponseHandler handler = newSessionResponseHandler( conn, recordConsumer, summaryConsumer, bookmarkHolder, state ); // When - handler.onSuccess( Collections.emptyMap() ); + handler.onSuccess( metadataWithType() ); // Then // assertThat( handler.status(), equalTo( SUCCEEDED ) ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/TransactionPullResponseCompletionListenerTest.java b/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/TransactionPullResponseCompletionListenerTest.java index 688e910929..3e456be6e9 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/TransactionPullResponseCompletionListenerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/handlers/pulln/TransactionPullResponseCompletionListenerTest.java @@ -18,7 +18,6 @@ */ package org.neo4j.driver.internal.handlers.pulln; -import java.util.Collections; import java.util.function.BiConsumer; import org.neo4j.driver.Query; @@ -50,7 +49,7 @@ protected void shouldHandleSuccessWithSummary( BasicPullResponseHandler.State st BasicPullResponseHandler handler = newResponseHandlerWithStatus( conn, recordConsumer, summaryConsumer, state ); // When - handler.onSuccess( Collections.emptyMap() ); + handler.onSuccess( metadataWithType() ); // Then assertThat( handler.state(), equalTo( BasicPullResponseHandler.State.SUCCEEDED_STATE )); diff --git a/driver/src/test/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3Test.java b/driver/src/test/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3Test.java index 60f34a4f3f..ec4df3f8e3 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3Test.java +++ b/driver/src/test/java/org/neo4j/driver/internal/messaging/v3/BoltProtocolV3Test.java @@ -459,7 +459,10 @@ protected void testSuccessfulRunInAutoCommitTxWithWaitingForResponse( Bookmark b String newBookmarkValue = "neo4j:bookmark:v1:tx98765"; handlers.runHandler.onSuccess( emptyMap() ); - handlers.pullAllHandler.onSuccess( singletonMap( "bookmark", value( newBookmarkValue ) ) ); + Map metadata = new HashMap<>(); + metadata.put( "bookmark", value( newBookmarkValue ) ); + metadata.put( "type", value( "rw" ) ); + handlers.pullAllHandler.onSuccess( metadata ); assertEquals( InternalBookmark.parse( newBookmarkValue ), bookmarkHolder.getBookmark() ); assertTrue( cursorFuture.isDone() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/reactive/util/ListBasedPullHandler.java b/driver/src/test/java/org/neo4j/driver/internal/reactive/util/ListBasedPullHandler.java index b41d33e411..fe310c8c49 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/reactive/util/ListBasedPullHandler.java +++ b/driver/src/test/java/org/neo4j/driver/internal/reactive/util/ListBasedPullHandler.java @@ -37,6 +37,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -68,7 +69,7 @@ private ListBasedPullHandler( List list, Throwable error ) PullResponseCompletionListener.class ) ); this.list = list; this.error = error; - when( super.metadataExtractor.extractSummary( any( Query.class ), any( Connection.class ), anyLong(), any( Map.class ) ) ).thenReturn( + when( super.metadataExtractor.extractSummary( any( Query.class ), any( Connection.class ), anyLong(), any( Map.class ), anyBoolean() ) ).thenReturn( mock( ResultSummary.class ) ); if ( list.size() > 1 ) { diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java index 6c26c27dbc..b60cbc9662 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java @@ -117,7 +117,7 @@ void shouldBuildResultSummaryWithQuery() Query query = new Query( "UNWIND range(10, 100) AS x CREATE (:Node {name: $name, x: x})", singletonMap( "name", "Apa" ) ); - ResultSummary summary = extractor.extractSummary( query, connectionMock(), 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query, connectionMock(), 42, emptyMap(), false ); assertEquals( query, summary.query() ); } @@ -127,7 +127,7 @@ void shouldBuildResultSummaryWithServerAddress() { Connection connection = connectionMock( new BoltServerAddress( "server:42" ) ); - ResultSummary summary = extractor.extractSummary( query(), connection, 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connection, 42, emptyMap(), false ); assertEquals( "server:42", summary.server().address() ); } @@ -162,7 +162,7 @@ void shouldBuildResultSummaryWithCounters() Map metadata = singletonMap( "stats", stats ); - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata, false ); assertEquals( 42, summary.counters().nodesCreated() ); assertEquals( 4242, summary.counters().nodesDeleted() ); @@ -180,7 +180,7 @@ void shouldBuildResultSummaryWithCounters() @Test void shouldBuildResultSummaryWithoutCounters() { - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap(), false ); assertEquals( EMPTY_STATS, summary.counters() ); } @@ -201,7 +201,7 @@ void shouldBuildResultSummaryWithPlan() ) ); Map metadata = singletonMap( "plan", plan ); - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata, false ); assertTrue( summary.hasPlan() ); assertEquals( "Projection", summary.plan().operatorType() ); @@ -221,7 +221,7 @@ void shouldBuildResultSummaryWithPlan() @Test void shouldBuildResultSummaryWithoutPlan() { - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap(), false ); assertFalse( summary.hasPlan() ); assertNull( summary.plan() ); } @@ -248,7 +248,7 @@ void shouldBuildResultSummaryWithProfiledPlan() ) ); Map metadata = singletonMap( "profile", profile ); - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata, false ); assertTrue( summary.hasPlan() ); assertTrue( summary.hasProfile() ); @@ -277,7 +277,7 @@ void shouldBuildResultSummaryWithProfiledPlan() @Test void shouldBuildResultSummaryWithoutProfiledPlan() { - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap(), false ); assertFalse( summary.hasProfile() ); assertNull( summary.profile() ); } @@ -310,7 +310,7 @@ void shouldBuildResultSummaryWithNotifications() Value notifications = value( notification1, notification2 ); Map metadata = singletonMap( "notifications", notifications ); - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata, false ); assertEquals( 2, summary.notifications().size() ); Notification firstNotification = summary.notifications().get( 0 ); @@ -332,7 +332,7 @@ void shouldBuildResultSummaryWithNotifications() @Test void shouldBuildResultSummaryWithoutNotifications() { - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap(), false ); assertEquals( 0, summary.notifications().size() ); } @@ -341,7 +341,7 @@ void shouldBuildResultSummaryWithResultAvailableAfter() { int value = 42_000; - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), value, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), value, emptyMap(), false ); assertEquals( 42, summary.resultAvailableAfter( TimeUnit.SECONDS ) ); assertEquals( value, summary.resultAvailableAfter( TimeUnit.MILLISECONDS ) ); @@ -353,7 +353,7 @@ void shouldBuildResultSummaryWithResultConsumedAfter() int value = 42_000; Map metadata = singletonMap( RESULT_CONSUMED_AFTER_KEY, value( value ) ); - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, metadata, false ); assertEquals( 42, summary.resultConsumedAfter( TimeUnit.SECONDS ) ); assertEquals( value, summary.resultConsumedAfter( TimeUnit.MILLISECONDS ) ); @@ -362,7 +362,7 @@ void shouldBuildResultSummaryWithResultConsumedAfter() @Test void shouldBuildResultSummaryWithoutResultConsumedAfter() { - ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap() ); + ResultSummary summary = extractor.extractSummary( query(), connectionMock(), 42, emptyMap(), false ); assertEquals( -1, summary.resultConsumedAfter( TimeUnit.SECONDS ) ); assertEquals( -1, summary.resultConsumedAfter( TimeUnit.MILLISECONDS ) ); } @@ -467,7 +467,7 @@ void shouldFailToExtractServerVersionFromNonNeo4jProduct() private ResultSummary createWithQueryType( Value typeValue ) { Map metadata = singletonMap( "type", typeValue ); - return extractor.extractSummary( query(), connectionMock(), 42, metadata ); + return extractor.extractSummary( query(), connectionMock(), 42, metadata, false ); } private static Query query() diff --git a/driver/src/test/java/org/neo4j/driver/util/TestUtil.java b/driver/src/test/java/org/neo4j/driver/util/TestUtil.java index 00390b3a50..ae87ff7d3a 100644 --- a/driver/src/test/java/org/neo4j/driver/util/TestUtil.java +++ b/driver/src/test/java/org/neo4j/driver/util/TestUtil.java @@ -37,6 +37,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -56,6 +57,7 @@ import org.neo4j.driver.Result; import org.neo4j.driver.Session; import org.neo4j.driver.SessionConfig; +import org.neo4j.driver.Values; import org.neo4j.driver.exceptions.ServiceUnavailableException; import org.neo4j.driver.internal.BoltServerAddress; import org.neo4j.driver.internal.DefaultBookmarkHolder; @@ -451,7 +453,7 @@ public static void setupSuccessfulRunAndPull( Connection connection ) doAnswer( invocation -> { ResponseHandler pullHandler = invocation.getArgument( 1 ); - pullHandler.onSuccess( emptyMap() ); + pullHandler.onSuccess( Collections.singletonMap( "type", Values.value( "rw" ) ) ); return null; } ).when( connection ).writeAndFlush( any( PullMessage.class ), any() ); } diff --git a/testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/StartTest.java b/testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/StartTest.java index 1f38331450..e8dd5eae43 100644 --- a/testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/StartTest.java +++ b/testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/StartTest.java @@ -41,7 +41,6 @@ public class StartTest implements TestkitRequest static { - COMMON_SKIP_PATTERN_TO_REASON.put( "^.*\\.test_invalid_query_type$", "Does not report type exception" ); COMMON_SKIP_PATTERN_TO_REASON.put( "^.*\\.test_no_notifications$", "An empty list is returned when there are no notifications" ); COMMON_SKIP_PATTERN_TO_REASON.put( "^.*\\.test_no_notification_info$", "An empty list is returned when there are no notifications" ); COMMON_SKIP_PATTERN_TO_REASON.put( "^.*\\.test_notifications_without_position$", "Null value is provided when position is absent" );