From 4dd771accd2f4186aab346619c2cb9df75d5f7ba Mon Sep 17 00:00:00 2001
From: Antonio Barcelos <antonio.barcelos@neo4j.com>
Date: Tue, 6 Apr 2021 16:53:42 +0200
Subject: [PATCH 1/2] Fix HELLO message RoutingContext

The routing context was missing information about the initial routing server. This information is important for the server side routing.
---
 .../src/bolt/bolt-protocol-v4x0.js            |   2 +-
 .../src/bolt/bolt-protocol-v4x3.js            |   5 +-
 bolt-connection/src/bolt/request-message.js   |   2 +-
 .../connection-provider-routing.js            |   5 +-
 .../src/rediscovery/rediscovery.js            |   5 +-
 .../test/bolt/bolt-protocol-v4x0.test.js      |   2 +-
 .../test/bolt/bolt-protocol-v4x3.test.js      |   2 +-
 bolt-connection/test/fake-connection.js       | 143 ++++++++++++++++++
 .../test/rediscovery}/rediscovery.test.js     |   9 +-
 test/internal/fake-connection.js              |   1 -
 10 files changed, 156 insertions(+), 20 deletions(-)
 create mode 100644 bolt-connection/test/fake-connection.js
 rename {test/internal => bolt-connection/test/rediscovery}/rediscovery.test.js (96%)

diff --git a/bolt-connection/src/bolt/bolt-protocol-v4x0.js b/bolt-connection/src/bolt/bolt-protocol-v4x0.js
index 7fce5934e..e47daaaa8 100644
--- a/bolt-connection/src/bolt/bolt-protocol-v4x0.js
+++ b/bolt-connection/src/bolt/bolt-protocol-v4x0.js
@@ -156,7 +156,7 @@ export default class BoltProtocol extends BoltProtocolV3 {
     const resultObserver = this.run(
       CALL_GET_ROUTING_TABLE_MULTI_DB,
       {
-        [CONTEXT]: { ...routingContext, address: initialAddress },
+        [CONTEXT]: routingContext,
         [DATABASE]: databaseName
       },
       { ...sessionContext, txConfig: TxConfig.empty() }
diff --git a/bolt-connection/src/bolt/bolt-protocol-v4x3.js b/bolt-connection/src/bolt/bolt-protocol-v4x3.js
index e809b248e..ef2cd81f6 100644
--- a/bolt-connection/src/bolt/bolt-protocol-v4x3.js
+++ b/bolt-connection/src/bolt/bolt-protocol-v4x3.js
@@ -56,10 +56,7 @@ export default class BoltProtocol extends BoltProtocolV42 {
     })
 
     this.write(
-      RequestMessage.route(
-        { ...routingContext, address: initialAddress },
-        databaseName
-      ),
+      RequestMessage.route(routingContext, databaseName),
       observer,
       true
     )
diff --git a/bolt-connection/src/bolt/request-message.js b/bolt-connection/src/bolt/request-message.js
index f2f18ed53..7f10fef12 100644
--- a/bolt-connection/src/bolt/request-message.js
+++ b/bolt-connection/src/bolt/request-message.js
@@ -108,7 +108,7 @@ export default class RequestMessage {
    */
   static hello (userAgent, authToken, routing = null) {
     const metadata = Object.assign({ user_agent: userAgent }, authToken)
-    if (routing != null) {
+    if (routing) {
       metadata.routing = routing
     }
     return new RequestMessage(
diff --git a/bolt-connection/src/connection-provider/connection-provider-routing.js b/bolt-connection/src/connection-provider/connection-provider-routing.js
index e40eead9a..c5458da8d 100644
--- a/bolt-connection/src/connection-provider/connection-provider-routing.js
+++ b/bolt-connection/src/connection-provider/connection-provider-routing.js
@@ -65,12 +65,13 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
         this._config,
         this._createConnectionErrorHandler(),
         this._log,
-        routingContext || {}
+        this._routingContext
       )
     })
 
+    this._routingContext = { ...routingContext, address: address.toString() }
     this._seedRouter = address
-    this._rediscovery = new Rediscovery(routingContext, address.toString())
+    this._rediscovery = new Rediscovery(this._routingContext)
     this._loadBalancingStrategy = new LeastConnectedLoadBalancingStrategy(
       this._connectionPool
     )
diff --git a/bolt-connection/src/rediscovery/rediscovery.js b/bolt-connection/src/rediscovery/rediscovery.js
index 6113eea76..e9a5c5bc6 100644
--- a/bolt-connection/src/rediscovery/rediscovery.js
+++ b/bolt-connection/src/rediscovery/rediscovery.js
@@ -28,11 +28,9 @@ export default class Rediscovery {
   /**
    * @constructor
    * @param {object} routingContext
-   * @param {string} initialAddress
    */
-  constructor (routingContext, initialAddress) {
+  constructor (routingContext) {
     this._routingContext = routingContext
-    this._initialAddress = initialAddress
   }
 
   /**
@@ -66,7 +64,6 @@ export default class Rediscovery {
     return new Promise((resolve, reject) => {
       connection.protocol().requestRoutingInformation({
         routingContext: this._routingContext,
-        initialAddress: this._initialAddress,
         databaseName: database,
         sessionContext: {
           bookmark: session._lastBookmark,
diff --git a/bolt-connection/test/bolt/bolt-protocol-v4x0.test.js b/bolt-connection/test/bolt/bolt-protocol-v4x0.test.js
index 46473d379..b34efe86c 100644
--- a/bolt-connection/test/bolt/bolt-protocol-v4x0.test.js
+++ b/bolt-connection/test/bolt/bolt-protocol-v4x0.test.js
@@ -146,7 +146,7 @@ describe('#unit BoltProtocolV4x0', () => {
     expect(protocol._run[0]).toEqual([
       'CALL dbms.routing.getRoutingTable($context, $database)',
       {
-        context: { ...routingContext, address: initialAddress },
+        context: routingContext,
         database: databaseName
       },
       { ...sessionContext, txConfig: TxConfig.empty() }
diff --git a/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js b/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js
index 756ed15ac..069afc8e3 100644
--- a/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js
+++ b/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js
@@ -49,7 +49,7 @@ describe('#unit BoltProtocolV4x3', () => {
 
     protocol.verifyMessageCount(1)
     expect(protocol.messages[0]).toBeMessage(
-      RequestMessage.route({ ...routingContext, address: null }, databaseName)
+      RequestMessage.route(routingContext, databaseName)
     )
     expect(protocol.observers).toEqual([observer])
     expect(observer).toEqual(jasmine.any(RouteObserver))
diff --git a/bolt-connection/test/fake-connection.js b/bolt-connection/test/fake-connection.js
new file mode 100644
index 000000000..af5cfc870
--- /dev/null
+++ b/bolt-connection/test/fake-connection.js
@@ -0,0 +1,143 @@
+/**
+ * Copyright (c) "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 Connection from '../src/connection/connection'
+
+/**
+ * This class is like a mock of {@link Connection} that tracks invocations count.
+ * It tries to maintain same "interface" as {@link Connection}.
+ * It could be replaced with a proper mock by a library like testdouble.
+ * At the time of writing such libraries require {@link Proxy} support but browser tests execute in
+ * PhantomJS which does not support proxies.
+ */
+export default class FakeConnection extends Connection {
+  constructor () {
+    super(null)
+
+    this._open = true
+    this._id = 0
+    this._databaseId = null
+    this._requestRoutingInformationMock = null
+    this.creationTimestamp = Date.now()
+
+    this.resetInvoked = 0
+    this.releaseInvoked = 0
+    this.seenQueries = []
+    this.seenParameters = []
+    this.seenProtocolOptions = []
+    this._server = {}
+    this.protocolVersion = undefined
+    this.protocolErrorsHandled = 0
+    this.seenProtocolErrors = []
+    this.seenRequestRoutingInformation = []
+  }
+
+  get id () {
+    return this._id
+  }
+
+  get databaseId () {
+    return this._databaseId
+  }
+
+  set databaseId (value) {
+    this._databaseId = value
+  }
+
+  get server () {
+    return this._server
+  }
+
+  get version () {
+    return this._server.version
+  }
+
+  set version (value) {
+    this._server.version = value
+  }
+
+  protocol () {
+    // return fake protocol object that simply records seen queries and parameters
+    return {
+      run: (query, parameters, protocolOptions) => {
+        this.seenQueries.push(query)
+        this.seenParameters.push(parameters)
+        this.seenProtocolOptions.push(protocolOptions)
+      },
+      requestRoutingInformation: params => {
+        this.seenRequestRoutingInformation.push(params)
+        if (this._requestRoutingInformationMock) {
+          this._requestRoutingInformationMock(params)
+        }
+      },
+      version: this.protocolVersion
+    }
+  }
+
+  resetAndFlush () {
+    this.resetInvoked++
+    return Promise.resolve()
+  }
+
+  _release () {
+    this.releaseInvoked++
+    return Promise.resolve()
+  }
+
+  isOpen () {
+    return this._open
+  }
+
+  isNeverReleased () {
+    return this.isReleasedTimes(0)
+  }
+
+  isReleasedOnce () {
+    return this.isReleasedTimes(1)
+  }
+
+  isReleasedTimes (times) {
+    return this.resetInvoked === times && this.releaseInvoked === times
+  }
+
+  _handleProtocolError (message) {
+    this.protocolErrorsHandled++
+    this.seenProtocolErrors.push(message)
+  }
+
+  withProtocolVersion (version) {
+    this.protocolVersion = version
+    return this
+  }
+
+  withCreationTimestamp (value) {
+    this.creationTimestamp = value
+    return this
+  }
+
+  withRequestRoutingInformationMock (requestRoutingInformationMock) {
+    this._requestRoutingInformationMock = requestRoutingInformationMock
+    return this
+  }
+
+  closed () {
+    this._open = false
+    return this
+  }
+}
diff --git a/test/internal/rediscovery.test.js b/bolt-connection/test/rediscovery/rediscovery.test.js
similarity index 96%
rename from test/internal/rediscovery.test.js
rename to bolt-connection/test/rediscovery/rediscovery.test.js
index 0257c52a8..12b34b926 100644
--- a/test/internal/rediscovery.test.js
+++ b/bolt-connection/test/rediscovery/rediscovery.test.js
@@ -17,10 +17,10 @@
  * limitations under the License.
  */
 
-import { RawRoutingTable } from '../../bolt-connection/lib/bolt'
-import Rediscovery from '../../bolt-connection/lib/rediscovery'
-import RoutingTable from '../../bolt-connection/lib/rediscovery/routing-table'
-import FakeConnection from './fake-connection'
+import { RawRoutingTable } from '../../src/bolt'
+import Rediscovery from '../../src/rediscovery'
+import RoutingTable from '../../src/rediscovery/routing-table'
+import FakeConnection from '../fake-connection'
 import lolex from 'lolex'
 import { newError, error, int, internal } from 'neo4j-driver-core'
 
@@ -106,7 +106,6 @@ describe('#unit Rediscovery', () => {
     const requestParams = connection.seenRequestRoutingInformation[0]
     expect(requestParams.routingContext).toEqual(routingContext)
     expect(requestParams.databaseName).toEqual(database)
-    expect(requestParams.initialAddress).toEqual(initialAddress)
     expect(requestParams.sessionContext).toEqual({
       bookmark: session._lastBookmark,
       mode: session._mode,
diff --git a/test/internal/fake-connection.js b/test/internal/fake-connection.js
index b197dd609..2a9738f25 100644
--- a/test/internal/fake-connection.js
+++ b/test/internal/fake-connection.js
@@ -18,7 +18,6 @@
  */
 
 import Connection from '../../bolt-connection/lib/connection/connection'
-import { textChangeRangeIsUnchanged } from 'typescript'
 import {
   ServerVersion,
   VERSION_3_4_0,

From 85fbe740fa6968d61cda9ed0b7826a4a8f4a208f Mon Sep 17 00:00:00 2001
From: Antonio Barcelos <antonio.barcelos@neo4j.com>
Date: Wed, 7 Apr 2021 17:20:45 +0200
Subject: [PATCH 2/2] Implements bookmarks in the ROUTE message

This important in the case of a creation of a new database to make sure the database is running in all the clusters nodes returned.

This change also skips a test which depends on feature which doesn't exists on the javascript driver.
---
 .../src/bolt/bolt-protocol-v4x0.js            |  1 -
 .../src/bolt/bolt-protocol-v4x3.js            |  8 +++---
 bolt-connection/src/bolt/request-message.js   | 10 ++++---
 .../test/bolt/bolt-protocol-v4x3.test.js      | 26 ++++++++++++++++++-
 .../test/bolt/request-message.test.js         | 15 +++++++----
 testkit-backend/src/skipped-tests.js          |  4 +++
 6 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/bolt-connection/src/bolt/bolt-protocol-v4x0.js b/bolt-connection/src/bolt/bolt-protocol-v4x0.js
index e47daaaa8..a3f7e01b1 100644
--- a/bolt-connection/src/bolt/bolt-protocol-v4x0.js
+++ b/bolt-connection/src/bolt/bolt-protocol-v4x0.js
@@ -149,7 +149,6 @@ export default class BoltProtocol extends BoltProtocolV3 {
     routingContext = {},
     databaseName = null,
     sessionContext = {},
-    initialAddress = null,
     onError,
     onCompleted
   }) {
diff --git a/bolt-connection/src/bolt/bolt-protocol-v4x3.js b/bolt-connection/src/bolt/bolt-protocol-v4x3.js
index ef2cd81f6..8d99f572e 100644
--- a/bolt-connection/src/bolt/bolt-protocol-v4x3.js
+++ b/bolt-connection/src/bolt/bolt-protocol-v4x3.js
@@ -23,6 +23,7 @@ import { RouteObserver } from './stream-observers'
 import { internal } from 'neo4j-driver-core'
 
 const {
+  bookmark: { Bookmark },
   constants: { BOLT_PROTOCOL_V4_3 }
 } = internal
 
@@ -38,6 +39,7 @@ export default class BoltProtocol extends BoltProtocolV42 {
    * @param {object} param.routingContext The routing context used to define the routing table.
    *  Multi-datacenter deployments is one of its use cases
    * @param {string} param.databaseName The database name
+   * @param {Bookmark} params.sessionContext.bookmark The bookmark used for request the routing table
    * @param {function(err: Error)} param.onError
    * @param {function(RawRoutingTable)} param.onCompleted
    * @returns {RouteObserver} the route observer
@@ -45,7 +47,7 @@ export default class BoltProtocol extends BoltProtocolV42 {
   requestRoutingInformation ({
     routingContext = {},
     databaseName = null,
-    initialAddress = null,
+    sessionContext = {},
     onError,
     onCompleted
   }) {
@@ -54,9 +56,9 @@ export default class BoltProtocol extends BoltProtocolV42 {
       onError,
       onCompleted
     })
-
+    const bookmark = sessionContext.bookmark || Bookmark.empty()
     this.write(
-      RequestMessage.route(routingContext, databaseName),
+      RequestMessage.route(routingContext, bookmark.values(), databaseName),
       observer,
       true
     )
diff --git a/bolt-connection/src/bolt/request-message.js b/bolt-connection/src/bolt/request-message.js
index 7f10fef12..39919fcc8 100644
--- a/bolt-connection/src/bolt/request-message.js
+++ b/bolt-connection/src/bolt/request-message.js
@@ -223,14 +223,18 @@ export default class RequestMessage {
    * Generate the ROUTE message, this message is used to fetch the routing table from the server
    *
    * @param {object} routingContext The routing context used to define the routing table. Multi-datacenter deployments is one of its use cases
+   * @param {string[]} bookmarks The list of the bookmark should be used
    * @param {string} databaseName The name of the database to get the routing table for.
    * @return {RequestMessage} the ROUTE message.
    */
-  static route (routingContext = {}, databaseName = null) {
+  static route (routingContext = {}, bookmarks = [], databaseName = null) {
     return new RequestMessage(
       ROUTE,
-      [routingContext, databaseName],
-      () => `ROUTE ${json.stringify(routingContext)} ${databaseName}`
+      [routingContext, bookmarks, databaseName],
+      () =>
+        `ROUTE ${json.stringify(routingContext)} ${json.stringify(
+          bookmarks
+        )} ${databaseName}`
     )
   }
 }
diff --git a/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js b/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js
index 069afc8e3..277b305b4 100644
--- a/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js
+++ b/bolt-connection/test/bolt/bolt-protocol-v4x3.test.js
@@ -49,7 +49,31 @@ describe('#unit BoltProtocolV4x3', () => {
 
     protocol.verifyMessageCount(1)
     expect(protocol.messages[0]).toBeMessage(
-      RequestMessage.route(routingContext, databaseName)
+      RequestMessage.route(routingContext, [], databaseName)
+    )
+    expect(protocol.observers).toEqual([observer])
+    expect(observer).toEqual(jasmine.any(RouteObserver))
+    expect(protocol.flushes).toEqual([true])
+  })
+
+  it('should request routing information sending bookmarks', () => {
+    const recorder = new utils.MessageRecordingConnection()
+    const protocol = new BoltProtocolV4x3(recorder, null, false)
+    utils.spyProtocolWrite(protocol)
+    const routingContext = { someContextParam: 'value' }
+    const listOfBookmarks = ['a', 'b', 'c']
+    const bookmark = new Bookmark(listOfBookmarks)
+    const databaseName = 'name'
+
+    const observer = protocol.requestRoutingInformation({
+      routingContext,
+      databaseName,
+      sessionContext: { bookmark }
+    })
+
+    protocol.verifyMessageCount(1)
+    expect(protocol.messages[0]).toBeMessage(
+      RequestMessage.route(routingContext, listOfBookmarks, databaseName)
     )
     expect(protocol.observers).toEqual([observer])
     expect(observer).toEqual(jasmine.any(RouteObserver))
diff --git a/bolt-connection/test/bolt/request-message.test.js b/bolt-connection/test/bolt/request-message.test.js
index 7dc88be60..c6ae4ebea 100644
--- a/bolt-connection/test/bolt/request-message.test.js
+++ b/bolt-connection/test/bolt/request-message.test.js
@@ -242,14 +242,17 @@ describe('#unit RequestMessage', () => {
   describe('BoltV4.3', () => {
     it('should create ROUTE message', () => {
       const requestContext = { someValue: '1234' }
+      const bookmarks = ['a', 'b']
       const database = 'user_db'
 
-      const message = RequestMessage.route(requestContext, database)
+      const message = RequestMessage.route(requestContext, bookmarks, database)
 
       expect(message.signature).toEqual(0x66)
-      expect(message.fields).toEqual([requestContext, database])
+      expect(message.fields).toEqual([requestContext, bookmarks, database])
       expect(message.toString()).toEqual(
-        `ROUTE ${json.stringify(requestContext)} ${database}`
+        `ROUTE ${json.stringify(requestContext)} ${json.stringify(
+          bookmarks
+        )} ${database}`
       )
     })
 
@@ -257,8 +260,10 @@ describe('#unit RequestMessage', () => {
       const message = RequestMessage.route()
 
       expect(message.signature).toEqual(0x66)
-      expect(message.fields).toEqual([{}, null])
-      expect(message.toString()).toEqual(`ROUTE ${json.stringify({})} ${null}`)
+      expect(message.fields).toEqual([{}, [], null])
+      expect(message.toString()).toEqual(
+        `ROUTE ${json.stringify({})} ${json.stringify([])} ${null}`
+      )
     })
   })
 })
diff --git a/testkit-backend/src/skipped-tests.js b/testkit-backend/src/skipped-tests.js
index 9018a3883..f464168cb 100644
--- a/testkit-backend/src/skipped-tests.js
+++ b/testkit-backend/src/skipped-tests.js
@@ -23,6 +23,10 @@ const skippedTests = [
     'Routing tests are disabled until the fix on the test scenario be merged',
     or(ifStartsWith('stub.routing'), ifStartsWith('stub.retry.TestRetry'))
   ),
+  skip(
+    'The driver no support domain_name_resolver',
+    ifEndsWith('test_should_successfully_acquire_rt_when_router_ip_changes')
+  ),
   skip(
     'Driver is failing trying to update the routing table using the original routing server',
     ifEndsWith(