Throw ProtocolException when QueryType is unknown (#1193)

This update ensures that `ProtocolException` is thrown when server provides an unknown `QueryType`.
neo4j · Mar 29, 2022 · 350435c · 350435c
1 parent a4fb946
commit 350435c
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 16 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
@@ -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,12 +93,27 @@ public boolean canManageAutoRead()
     public synchronized void onSuccess( Map<String,Value> metadata )
     {
         finished = true;
-        summary = extractResultSummary( metadata );
+        Neo4jException exception = null;
+        try
+        {
+            summary = extractResultSummary( metadata );
+        }
+        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
@@ -335,7 +351,7 @@ private boolean completeFailureFuture( Throwable error )
     private ResultSummary extractResultSummary( Map<String,Value> metadata )
     {
         long resultAvailableAfter = runResponseHandler.resultAvailableAfter();
-        return metadataExtractor.extractSummary(query, connection, resultAvailableAfter, metadata );
+        return metadataExtractor.extractSummary( query, connection, resultAvailableAfter, metadata );
     }
 
     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
@@ -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;
@@ -112,9 +113,18 @@ protected void completeWithFailure( Throwable error )
     protected void completeWithSuccess( Map<String,Value> metadata )
     {
         completionListener.afterSuccess( metadata );
-        ResultSummary summary = extractResultSummary( metadata );
-
-        complete( summary, null );
+        ResultSummary summary;
+        Neo4jException exception = null;
+        try
+        {
+            summary = extractResultSummary( metadata );
+        }
+        catch ( Neo4jException e )
+        {
+            summary = extractResultSummary( emptyMap() );
+            exception = e;
+        }
+        complete( summary, exception );
     }
 
     protected void successHasMore()

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
@@ -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,9 @@
 public class MetadataExtractor
 {
     public static final int ABSENT_QUERY_ID = -1;
+    private static final String UNEXPECTED_TYPE_MSG_FMT = "Unexpected query type '%s', consider updating the driver";
+    private static final Function<String,ProtocolException> UNEXPECTED_TYPE_EXCEPTION_SUPPLIER =
+            ( type ) -> new ProtocolException( String.format( UNEXPECTED_TYPE_MSG_FMT, type ) );
     private final String resultAvailableAfterMetadataKey;
     private final String resultConsumedAfterMetadataKey;
 
@@ -98,14 +103,14 @@ public long extractResultAvailableAfter( Map<String,Value> metadata )
         return -1;
     }
 
-    public ResultSummary extractSummary(Query query, Connection connection, long resultAvailableAfter, Map<String,Value> metadata )
+    public ResultSummary extractSummary( Query query, Connection connection, long resultAvailableAfter, Map<String,Value> metadata )
     {
         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 ) );
+        return new InternalResultSummary( query, serverInfo, dbInfo, extractQueryType( metadata ), extractCounters( metadata ), extractPlan( metadata ),
+                                          extractProfiledPlan( metadata ), extractNotifications( metadata ), resultAvailableAfter,
+                                          extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) );
     }
 
     public static DatabaseInfo extractDatabaseInfo( Map<String,Value> metadata )
@@ -151,7 +156,7 @@ private static QueryType extractQueryType( Map<String,Value> metadata )
         Value typeValue = metadata.get( "type" );
         if ( typeValue != null )
         {
-            return QueryType.fromCode( typeValue.asString() );
+            return QueryType.fromCode( typeValue.asString(), UNEXPECTED_TYPE_EXCEPTION_SUPPLIER );
         }
         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
@@ -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<String,ClientException> 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<String,? extends Neo4jException> 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/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" );