Providing protocol version in Result get by Transaction.run

The result summary was not filled with the protocol version because the transactions were creating
the Result object with a empty connection holder. Since the Result releases the connection from holder
when it completes, the original holder could not be passed to it.

The ReadOnlyConnectionHolder provides a safe way to pass the holder forward without let the consumer initialize,
close or release the connection by simulating this methods without do the action and delegating part of the requests to the original ConnectionHolder.

These changes also made shared-neo4j cluster friendly.

Author: bigmontz

src/internal/connection-holder-readonly.js

@@ -0,0 +1,77 @@
+/**
+ * Copyright (c) 2002-2020 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import ConnectionHolder from './connection-holder'
+
+/**
+ * Provides a interaction with a ConnectionHolder without change it state by
+ * releasing or initilizing
+ */
+export default class ReadOnlyConnectionHolder extends ConnectionHolder {
+  /**
+   * Contructor
+   * @param {ConnectionHolder} connectionHolder the connection holder which will treat the requests
+   */
+  constructor (connectionHolder) {
+    super({
+      mode: connectionHolder._mode,
+      database: connectionHolder._database,
+      bookmark: connectionHolder._bookmark,
+      connectionProvider: connectionHolder._connectionProvider
+    })
+    this._connectionHolder = connectionHolder
+  }
+
+  /**
+   * Return the true if the connection is suppose to be initilized with the command.
+   *
+   * @return {boolean}
+   */
+  initializeConnection () {
+    if (this._connectionHolder._referenceCount === 0) {
+      return false
+    }
+    return true
+  }
+
+  /**
+   * Get the current connection promise.
+   * @return {Promise<Connection>} promise resolved with the current connection.
+   */
+  getConnection () {
+    return this._connectionHolder.getConnection()
+  }
+
+  /**
+   * Get the current connection promise, doesn't performs the release
+   * @return {Promise<Connection>} promise with the resolved current connection
+   */
+  releaseConnection () {
+    return this._connectionHolder.getConnection().catch(() => Promise.resolve())
+  }
+
+  /**
+   * Get the current connection promise, doesn't performs the connection close
+   * @return {Promise<Connection>} promise with the resolved current connection
+   */
+  close () {
+    return this._connectionHolder
+      .getConnection()
+      .catch(() => () => Promise.resolve())
+  }
+}

src/result.js

@@ -166,26 +166,34 @@ class Result {
       const query = this._query
       const parameters = this._parameters
 
-      function release (protocolVersion) {
+      function complete (protocolVersion) {
+        onCompletedOriginal.call(
+          observer,
+          new ResultSummary(query, parameters, metadata, protocolVersion)
+        )
+      }
+
+      function release () {
         // notify connection holder that the used connection is not needed any more because result has
         // been fully consumed; call the original onCompleted callback after that
-        connectionHolder.releaseConnection().then(() => {
-          onCompletedOriginal.call(
-            observer,
-            new ResultSummary(query, parameters, metadata, protocolVersion)
-          )
-        })
+        return connectionHolder.releaseConnection()
       }
 
       connectionHolder.getConnection().then(
         // onFulfilled:
         connection => {
-          release(connection ? connection.protocol().version : undefined)
+          release().then(() =>
+            complete(
+              connection !== undefined
+                ? connection.protocol().version
+                : undefined
+            )
+          )
         },
 
         // onRejected:
         _ => {
-          release()
+          complete()
         }
       )
     }

src/transaction.js

@@ -21,6 +21,7 @@ import { validateQueryAndParameters } from './internal/util'
 import ConnectionHolder, {
   EMPTY_CONNECTION_HOLDER
 } from './internal/connection-holder'
+import ReadOnlyConnectionHolder from './internal/connection-holder-readonly'
 import Bookmark from './internal/bookmark'
 import TxConfig from './internal/tx-config'
 
@@ -257,7 +258,12 @@ const _states = {
         })
         .catch(error => new FailedObserver({ error, onError }))
 
-      return newCompletedResult(observerPromise, query, parameters)
+      return newCompletedResult(
+        observerPromise,
+        query,
+        parameters,
+        connectionHolder
+      )
     }
   },
 
@@ -274,14 +280,20 @@ const _states = {
             onError
           }),
           'COMMIT',
-          {}
+          {},
+          connectionHolder
         ),
         state: _states.FAILED
       }
     },
     rollback: ({ connectionHolder, onError, onComplete }) => {
       return {
-        result: newCompletedResult(new CompletedObserver(), 'ROLLBACK', {}),
+        result: newCompletedResult(
+          new CompletedObserver(),
+          'ROLLBACK',
+          {},
+          connectionHolder
+        ),
         state: _states.FAILED
       }
     },
@@ -294,7 +306,8 @@ const _states = {
           onError
         }),
         query,
-        parameters
+        parameters,
+        connectionHolder
       )
     }
   },
@@ -313,7 +326,8 @@ const _states = {
           'COMMIT',
           {}
         ),
-        state: _states.SUCCEEDED
+        state: _states.SUCCEEDED,
+        connectionHolder
       }
     },
     rollback: ({ connectionHolder, onError, onComplete }) => {
@@ -328,7 +342,8 @@ const _states = {
           'ROLLBACK',
           {}
         ),
-        state: _states.SUCCEEDED
+        state: _states.SUCCEEDED,
+        connectionHolder
       }
     },
     run: (query, parameters, { connectionHolder, onError, onComplete }) => {
@@ -340,7 +355,8 @@ const _states = {
           onError
         }),
         query,
-        parameters
+        parameters,
+        connectionHolder
       )
     }
   },
@@ -357,7 +373,8 @@ const _states = {
             onError
           }),
           'COMMIT',
-          {}
+          {},
+          connectionHolder
         ),
         state: _states.ROLLED_BACK
       }
@@ -371,7 +388,8 @@ const _states = {
             )
           }),
           'ROLLBACK',
-          {}
+          {},
+          connectionHolder
         ),
         state: _states.ROLLED_BACK
       }
@@ -385,7 +403,8 @@ const _states = {
           onError
         }),
         query,
-        parameters
+        parameters,
+        connectionHolder
       )
     }
   }
@@ -446,15 +465,21 @@ function finishTransaction (
  * @param {ResultStreamObserver} observer - an observer for the created result.
  * @param {string} query - the cypher query that produced the result.
  * @param {Object} parameters - the parameters for cypher query that produced the result.
+ * @param {ConnectionHolder} connectionHolder - the connection holder used to get the result
  * @return {Result} new result.
  * @private
  */
-function newCompletedResult (observerPromise, query, parameters) {
+function newCompletedResult (
+  observerPromise,
+  query,
+  parameters,
+  connectionHolder = EMPTY_CONNECTION_HOLDER
+) {
   return new Result(
     Promise.resolve(observerPromise),
     query,
     parameters,
-    EMPTY_CONNECTION_HOLDER
+    new ReadOnlyConnectionHolder(connectionHolder || EMPTY_CONNECTION_HOLDER)
   )
 }