From 09475c50161e69fb92c354dfa0c0e74646dbda7b Mon Sep 17 00:00:00 2001
From: Antonio Barcelos <antonio.barcelos@neo4j.com>
Date: Wed, 4 May 2022 16:05:56 +0200
Subject: [PATCH] Fix lack of ProtocolVersion in the `Result.summary()`

When there are more than one observer subscribed to the ResultStreamObserver, the first notified observed got the summary with the protocol info because the connection holder stills holding the connection. The next observers notified won't have the protocol version since the connection holder will not have the connection in hands at the moment.

This situation is occurring during some tests where result iterator is not fully consumed and the summary is called in the meantime.

Checking for existance of the summary in the result and notifying the observer with the already created summary should solve issue since the first summary created will have the protocol version set.
---
 packages/core/src/result.ts       |  7 +++--
 packages/core/test/result.test.ts | 44 ++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/packages/core/src/result.ts b/packages/core/src/result.ts
index fa4a28e1c..6dce77f86 100644
--- a/packages/core/src/result.ts
+++ b/packages/core/src/result.ts
@@ -436,7 +436,10 @@ class Result implements Promise<QueryResult> {
     const onKeysOriginal = observer.onKeys ?? DEFAULT_ON_KEYS
 
     const onCompletedWrapper = (metadata: any): void => {
-      this._createSummary(metadata).then(summary => {
+      this._releaseConnectionAndGetSummary(metadata).then(summary => {
+        if (this._summary !== null) {
+          return onCompletedOriginal.call(observer, this._summary)
+        }
         this._summary = summary
         return onCompletedOriginal.call(observer, summary)
       }).catch(onErrorOriginal)
@@ -484,7 +487,7 @@ class Result implements Promise<QueryResult> {
    * @param metadata
    * @returns
    */
-  private _createSummary (metadata: any): Promise<ResultSummary> {
+  private _releaseConnectionAndGetSummary (metadata: any): Promise<ResultSummary> {
     const {
       validatedQuery: query,
       params: parameters
diff --git a/packages/core/test/result.test.ts b/packages/core/test/result.test.ts
index c772711ab..9ab679652 100644
--- a/packages/core/test/result.test.ts
+++ b/packages/core/test/result.test.ts
@@ -26,6 +26,7 @@ import {
 } from '../src'
 
 import Result from '../src/result'
+import FakeConnection from './utils/connection.fake'
 
 describe('Result', () => {
   const expectedError = newError('some error')
@@ -146,11 +147,14 @@ describe('Result', () => {
           { query: 'query', parameters: { b: 2 } }
         ]
       ])('when query=%s and parameters=%s', (query, params, expected) => {
+        let connectionHolderMock: connectionHolder.ConnectionHolder
         beforeEach(() => {
+          connectionHolderMock = new connectionHolder.ConnectionHolder({})
           result = new Result(
             Promise.resolve(streamObserverMock),
             query,
-            params
+            params,
+            connectionHolderMock
           )
         })
 
@@ -174,6 +178,44 @@ describe('Result', () => {
           expect(summary).toEqual(expectedSummary)
         })
 
+        it('should resolve pre-existing summary', async () => {
+          const metadata = {
+            resultConsumedAfter: 20,
+            resultAvailableAfter: 124,
+            extraInfo: 'extra'
+          }
+          const protocolVersion = 5.0
+
+          const expectedSummary = new ResultSummary(
+            expected.query,
+            expected.parameters,
+            metadata,
+            protocolVersion
+          )
+
+          jest.spyOn(connectionHolderMock, 'getConnection')
+            .mockImplementationOnce(async () => {
+              const conn = new FakeConnection()
+              conn.protocolVersion = protocolVersion
+              return conn
+            })
+
+          jest.spyOn(connectionHolderMock, 'releaseConnection')
+            .mockImplementationOnce(async () => null)
+
+          const iterator = result[Symbol.asyncIterator]()
+          const next = iterator.next()
+          const summaryPromise = result.summary()
+
+          streamObserverMock.onCompleted(metadata)
+
+          const { value } = await next
+          const summary = await summaryPromise
+
+          expect(value).toEqual(expectedSummary)
+          expect(summary).toEqual(expectedSummary)
+        })
+
         it('should reject a pre-existing error', async () => {
           const expectedError = newError('the expected error')
           streamObserverMock.onError(expectedError)