Skip to content

Commit 79e1022

Browse files
injectivesinjectives
committed
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.
1 parent df8b781 commit 79e1022

File tree

15 files changed

+158
-81
lines changed

15 files changed

+158
-81
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandler.java

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.neo4j.driver.Query;
3131
import org.neo4j.driver.Record;
3232
import org.neo4j.driver.Value;
33+
import org.neo4j.driver.exceptions.Neo4jException;
3334
import org.neo4j.driver.internal.InternalRecord;
3435
import org.neo4j.driver.internal.messaging.request.PullAllMessage;
3536
import org.neo4j.driver.internal.spi.Connection;
@@ -92,19 +93,34 @@ public boolean canManageAutoRead()
9293
public synchronized void onSuccess( Map<String,Value> metadata )
9394
{
9495
finished = true;
95-
summary = extractResultSummary( metadata );
96+
Neo4jException exception = null;
97+
try
98+
{
99+
summary = extractResultSummary( metadata, true );
100+
}
101+
catch ( Neo4jException e )
102+
{
103+
exception = e;
104+
}
96105

97-
completionListener.afterSuccess( metadata );
106+
if ( exception == null )
107+
{
108+
completionListener.afterSuccess( metadata );
98109

99-
completeRecordFuture( null );
100-
completeFailureFuture( null );
110+
completeRecordFuture( null );
111+
completeFailureFuture( null );
112+
}
113+
else
114+
{
115+
onFailure( exception );
116+
}
101117
}
102118

103119
@Override
104120
public synchronized void onFailure( Throwable error )
105121
{
106122
finished = true;
107-
summary = extractResultSummary( emptyMap() );
123+
summary = extractResultSummary( emptyMap(), false );
108124

109125
completionListener.afterFailure( error );
110126

@@ -332,10 +348,10 @@ private boolean completeFailureFuture( Throwable error )
332348
return false;
333349
}
334350

335-
private ResultSummary extractResultSummary( Map<String,Value> metadata )
351+
private ResultSummary extractResultSummary( Map<String,Value> metadata, boolean enforceQueryType )
336352
{
337353
long resultAvailableAfter = runResponseHandler.resultAvailableAfter();
338-
return metadataExtractor.extractSummary(query, connection, resultAvailableAfter, metadata );
354+
return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata, enforceQueryType );
339355
}
340356

341357
private void enableAutoRead()

‎driver/src/main/java/org/neo4j/driver/internal/handlers/pulln/BasicPullResponseHandler.java

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.neo4j.driver.Query;
2525
import org.neo4j.driver.Record;
2626
import org.neo4j.driver.Value;
27+
import org.neo4j.driver.exceptions.Neo4jException;
2728
import org.neo4j.driver.internal.InternalRecord;
2829
import org.neo4j.driver.internal.handlers.PullResponseCompletionListener;
2930
import org.neo4j.driver.internal.handlers.RunResponseHandler;
@@ -106,15 +107,24 @@ public synchronized void cancel()
106107
protected void completeWithFailure( Throwable error )
107108
{
108109
completionListener.afterFailure( error );
109-
complete( extractResultSummary( emptyMap() ), error );
110+
complete( extractResultSummary( emptyMap(), false ), error );
110111
}
111112

112-
protected void completeWithSuccess( Map<String,Value> metadata )
113+
protected void completeWithSuccess( Map<String,Value> metadata, boolean enforceQueryType )
113114
{
114115
completionListener.afterSuccess( metadata );
115-
ResultSummary summary = extractResultSummary( metadata );
116-
117-
complete( summary, null );
116+
ResultSummary summary;
117+
Neo4jException exception = null;
118+
try
119+
{
120+
summary = extractResultSummary( metadata, enforceQueryType );
121+
}
122+
catch ( Neo4jException e )
123+
{
124+
summary = extractResultSummary( emptyMap(), false );
125+
exception = e;
126+
}
127+
complete( summary, exception );
118128
}
119129

120130
protected void successHasMore()
@@ -169,10 +179,10 @@ protected boolean isDone()
169179
return state.equals( State.SUCCEEDED_STATE ) || state.equals( State.FAILURE_STATE );
170180
}
171181

172-
private ResultSummary extractResultSummary( Map<String,Value> metadata )
182+
private ResultSummary extractResultSummary( Map<String,Value> metadata, boolean enforceQueryType )
173183
{
174184
long resultAvailableAfter = runResponseHandler.resultAvailableAfter();
175-
return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata );
185+
return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata, enforceQueryType );
176186
}
177187

178188
private void addToRequest( long toAdd )
@@ -247,7 +257,7 @@ enum State
247257
void onSuccess( BasicPullResponseHandler context, Map<String,Value> metadata )
248258
{
249259
context.state( SUCCEEDED_STATE );
250-
context.completeWithSuccess( metadata );
260+
context.completeWithSuccess( metadata, false );
251261
}
252262

253263
@Override
@@ -290,7 +300,7 @@ void onSuccess( BasicPullResponseHandler context, Map<String,Value> metadata )
290300
else
291301
{
292302
context.state( SUCCEEDED_STATE );
293-
context.completeWithSuccess( metadata );
303+
context.completeWithSuccess( metadata, true );
294304
}
295305
}
296306

@@ -334,7 +344,7 @@ void onSuccess( BasicPullResponseHandler context, Map<String,Value> metadata )
334344
else
335345
{
336346
context.state( SUCCEEDED_STATE );
337-
context.completeWithSuccess( metadata );
347+
context.completeWithSuccess( metadata, true );
338348
}
339349
}
340350

@@ -369,7 +379,7 @@ void cancel( BasicPullResponseHandler context )
369379
void onSuccess( BasicPullResponseHandler context, Map<String,Value> metadata )
370380
{
371381
context.state( SUCCEEDED_STATE );
372-
context.completeWithSuccess( metadata );
382+
context.completeWithSuccess( metadata, false );
373383
}
374384

375385
@Override
@@ -403,7 +413,7 @@ void cancel( BasicPullResponseHandler context )
403413
void onSuccess( BasicPullResponseHandler context, Map<String,Value> metadata )
404414
{
405415
context.state( SUCCEEDED_STATE );
406-
context.completeWithSuccess( metadata );
416+
context.completeWithSuccess( metadata, false );
407417
}
408418

409419
@Override

‎driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@
2121
import java.util.Collections;
2222
import java.util.List;
2323
import java.util.Map;
24+
import java.util.function.Function;
2425

2526
import org.neo4j.driver.Bookmark;
2627
import org.neo4j.driver.Query;
2728
import org.neo4j.driver.Value;
29+
import org.neo4j.driver.exceptions.ProtocolException;
2830
import org.neo4j.driver.exceptions.UntrustedServerException;
2931
import org.neo4j.driver.internal.InternalBookmark;
3032
import org.neo4j.driver.internal.spi.Connection;
@@ -49,6 +51,10 @@
4951
public class MetadataExtractor
5052
{
5153
public static final int ABSENT_QUERY_ID = -1;
54+
public static final String MISSING_TYPE_MSG = "No query type has been provided, consider updating the driver";
55+
private static final String UNEXPECTED_TYPE_MSG_FMT = "Unexpected query type '%s', consider updating the driver";
56+
private static final Function<String,ProtocolException> UNEXPECTED_TYPE_EXCEPTION_SUPPLIER =
57+
( type ) -> new ProtocolException( String.format( UNEXPECTED_TYPE_MSG_FMT, type ) );
5258
private final String resultAvailableAfterMetadataKey;
5359
private final String resultConsumedAfterMetadataKey;
5460

@@ -98,14 +104,15 @@ public long extractResultAvailableAfter( Map<String,Value> metadata )
98104
return -1;
99105
}
100106

101-
public ResultSummary extractSummary(Query query, Connection connection, long resultAvailableAfter, Map<String,Value> metadata )
107+
public ResultSummary extractSummary( Query query, Connection connection, long resultAvailableAfter, Map<String,Value> metadata, boolean enforceQueryType )
102108
{
103109
ServerInfo serverInfo =
104110
new InternalServerInfo( connection.serverAgent(), connection.serverAddress(), connection.protocol().version() );
105111
DatabaseInfo dbInfo = extractDatabaseInfo( metadata );
106-
return new InternalResultSummary(query, serverInfo, dbInfo, extractQueryType( metadata ), extractCounters( metadata ), extractPlan( metadata ),
107-
extractProfiledPlan( metadata ), extractNotifications( metadata ), resultAvailableAfter,
108-
extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) );
112+
QueryType queryType = extractQueryType( metadata, enforceQueryType );
113+
return new InternalResultSummary( query, serverInfo, dbInfo, queryType, extractCounters( metadata ), extractPlan( metadata ),
114+
extractProfiledPlan( metadata ), extractNotifications( metadata ), resultAvailableAfter,
115+
extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) );
109116
}
110117

111118
public static DatabaseInfo extractDatabaseInfo( Map<String,Value> metadata )
@@ -146,12 +153,16 @@ public static Value extractServer( Map<String,Value> metadata )
146153
return versionValue;
147154
}
148155

149-
private static QueryType extractQueryType( Map<String,Value> metadata )
156+
private static QueryType extractQueryType( Map<String,Value> metadata, boolean enforce )
150157
{
151158
Value typeValue = metadata.get( "type" );
152159
if ( typeValue != null )
153160
{
154-
return QueryType.fromCode( typeValue.asString() );
161+
return QueryType.fromCode( typeValue.asString(), UNEXPECTED_TYPE_EXCEPTION_SUPPLIER );
162+
}
163+
else if ( enforce )
164+
{
165+
throw new ProtocolException( MISSING_TYPE_MSG );
155166
}
156167
return null;
157168
}

‎driver/src/main/java/org/neo4j/driver/summary/QueryType.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@
1818
*/
1919
package org.neo4j.driver.summary;
2020

21+
import java.util.function.Function;
22+
2123
import org.neo4j.driver.exceptions.ClientException;
24+
import org.neo4j.driver.exceptions.Neo4jException;
2225

2326
/**
2427
* The type of query executed.
28+
*
2529
* @since 1.0
2630
*/
2731
public enum QueryType
@@ -31,7 +35,16 @@ public enum QueryType
3135
WRITE_ONLY,
3236
SCHEMA_WRITE;
3337

34-
public static QueryType fromCode(String type )
38+
private static final String UNEXPECTED_TYPE_MSG_FMT = "Unknown query type: `%s`.";
39+
private static final Function<String,ClientException> UNEXPECTED_TYPE_EXCEPTION_SUPPLIER =
40+
( type ) -> new ClientException( String.format( UNEXPECTED_TYPE_MSG_FMT, type ) );
41+
42+
public static QueryType fromCode( String type )
43+
{
44+
return fromCode( type, UNEXPECTED_TYPE_EXCEPTION_SUPPLIER );
45+
}
46+
47+
public static QueryType fromCode( String type, Function<String,? extends Neo4jException> exceptionFunction )
3548
{
3649
switch ( type )
3750
{
@@ -44,7 +57,14 @@ public static QueryType fromCode(String type )
4457
case "s":
4558
return QueryType.SCHEMA_WRITE;
4659
default:
47-
throw new ClientException( "Unknown query type: `" + type + "`." );
60+
if ( exceptionFunction != null )
61+
{
62+
throw exceptionFunction.apply( type );
63+
}
64+
else
65+
{
66+
return null;
67+
}
4868
}
4969
}
5070
}

‎driver/src/test/java/org/neo4j/driver/internal/InternalResultTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.util.ArrayList;
2424
import java.util.Arrays;
25+
import java.util.Collections;
2526
import java.util.List;
2627
import java.util.concurrent.CompletableFuture;
2728

@@ -45,7 +46,6 @@
4546
import org.neo4j.driver.util.Pair;
4647

4748
import static java.util.Arrays.asList;
48-
import static java.util.Collections.emptyMap;
4949
import static java.util.Collections.singletonMap;
5050
import static org.hamcrest.CoreMatchers.equalTo;
5151
import static org.hamcrest.collection.IsCollectionWithSize.hasSize;
@@ -369,7 +369,7 @@ private Result createResult(int numberOfRecords )
369369
{
370370
pullAllHandler.onRecord( new Value[]{value( "v1-" + i ), value( "v2-" + i )} );
371371
}
372-
pullAllHandler.onSuccess( emptyMap() );
372+
pullAllHandler.onSuccess( Collections.singletonMap( "type", value( "rw" ) ) );
373373

374374
AsyncResultCursor cursor = new AsyncResultCursorImpl( null, runHandler, pullAllHandler );
375375
return new InternalResult( connection, new DisposableAsyncResultCursor( cursor ) );

‎driver/src/test/java/org/neo4j/driver/internal/handlers/LegacyPullAllResponseHandlerTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
import static java.util.Arrays.asList;
3434
import static java.util.Collections.emptyList;
35-
import static java.util.Collections.emptyMap;
3635
import static java.util.Collections.singletonMap;
3736
import static org.junit.jupiter.api.Assertions.assertEquals;
3837
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -107,7 +106,7 @@ void shouldNotDisableAutoReadWhenSummaryRequested()
107106

108107
verify( connection, never() ).disableAutoRead();
109108

110-
handler.onSuccess( emptyMap() );
109+
handler.onSuccess( metadataWithType() );
111110
assertTrue( summaryFuture.isDone() );
112111

113112
ResultSummary summary = await( summaryFuture );
@@ -228,7 +227,7 @@ void shouldEnableAutoReadOnConnectionWhenSummaryRequestedButNotAvailable() throw
228227

229228
assertNull( await( handler.nextAsync() ) );
230229

231-
handler.onSuccess( emptyMap() );
230+
handler.onSuccess( metadataWithType() );
232231

233232
assertTrue( summaryFuture.isDone() );
234233
assertNotNull( summaryFuture.get() );

0 commit comments

Comments
 (0)