Skip to content

Commit 14b91ee

Browse files
committed
Fix HELLO message RoutingContext
The routing context was missing information about the initial routing server. This information is important for the server side routing.
1 parent dd1beb3 commit 14b91ee

File tree

10 files changed

+156
-20
lines changed

10 files changed

+156
-20
lines changed

bolt-connection/src/bolt/bolt-protocol-v4x0.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export default class BoltProtocol extends BoltProtocolV3 {
156156
const resultObserver = this.run(
157157
CALL_GET_ROUTING_TABLE_MULTI_DB,
158158
{
159-
[CONTEXT]: { ...routingContext, address: initialAddress },
159+
[CONTEXT]: routingContext,
160160
[DATABASE]: databaseName
161161
},
162162
{ ...sessionContext, txConfig: TxConfig.empty() }

bolt-connection/src/bolt/bolt-protocol-v4x3.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,7 @@ export default class BoltProtocol extends BoltProtocolV42 {
5656
})
5757

5858
this.write(
59-
RequestMessage.route(
60-
{ ...routingContext, address: initialAddress },
61-
databaseName
62-
),
59+
RequestMessage.route(routingContext, databaseName),
6360
observer,
6461
true
6562
)

bolt-connection/src/bolt/request-message.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ export default class RequestMessage {
108108
*/
109109
static hello (userAgent, authToken, routing = null) {
110110
const metadata = Object.assign({ user_agent: userAgent }, authToken)
111-
if (routing != null) {
111+
if (routing) {
112112
metadata.routing = routing
113113
}
114114
return new RequestMessage(

bolt-connection/src/connection-provider/connection-provider-routing.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,13 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
6565
this._config,
6666
this._createConnectionErrorHandler(),
6767
this._log,
68-
routingContext || {}
68+
this._routingContext
6969
)
7070
})
7171

72+
this._routingContext = { ...routingContext, address: address.toString() }
7273
this._seedRouter = address
73-
this._rediscovery = new Rediscovery(routingContext, address.toString())
74+
this._rediscovery = new Rediscovery(this._routingContext)
7475
this._loadBalancingStrategy = new LeastConnectedLoadBalancingStrategy(
7576
this._connectionPool
7677
)

bolt-connection/src/rediscovery/rediscovery.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,9 @@ export default class Rediscovery {
2828
/**
2929
* @constructor
3030
* @param {object} routingContext
31-
* @param {string} initialAddress
3231
*/
33-
constructor (routingContext, initialAddress) {
32+
constructor (routingContext) {
3433
this._routingContext = routingContext
35-
this._initialAddress = initialAddress
3634
}
3735

3836
/**
@@ -66,7 +64,6 @@ export default class Rediscovery {
6664
return new Promise((resolve, reject) => {
6765
connection.protocol().requestRoutingInformation({
6866
routingContext: this._routingContext,
69-
initialAddress: this._initialAddress,
7067
databaseName: database,
7168
sessionContext: {
7269
bookmark: session._lastBookmark,

bolt-connection/test/bolt/bolt-protocol-v4x0.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ describe('#unit BoltProtocolV4x0', () => {
146146
expect(protocol._run[0]).toEqual([
147147
'CALL dbms.routing.getRoutingTable($context, $database)',
148148
{
149-
context: { ...routingContext, address: initialAddress },
149+
context: routingContext,
150150
database: databaseName
151151
},
152152
{ ...sessionContext, txConfig: TxConfig.empty() }

bolt-connection/test/bolt/bolt-protocol-v4x3.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ describe('#unit BoltProtocolV4x3', () => {
4949

5050
protocol.verifyMessageCount(1)
5151
expect(protocol.messages[0]).toBeMessage(
52-
RequestMessage.route({ ...routingContext, address: null }, databaseName)
52+
RequestMessage.route(routingContext, databaseName)
5353
)
5454
expect(protocol.observers).toEqual([observer])
5555
expect(observer).toEqual(jasmine.any(RouteObserver))
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/**
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
import Connection from '../src/connection/connection'
21+
22+
/**
23+
* This class is like a mock of {@link Connection} that tracks invocations count.
24+
* It tries to maintain same "interface" as {@link Connection}.
25+
* It could be replaced with a proper mock by a library like testdouble.
26+
* At the time of writing such libraries require {@link Proxy} support but browser tests execute in
27+
* PhantomJS which does not support proxies.
28+
*/
29+
export default class FakeConnection extends Connection {
30+
constructor () {
31+
super(null)
32+
33+
this._open = true
34+
this._id = 0
35+
this._databaseId = null
36+
this._requestRoutingInformationMock = null
37+
this.creationTimestamp = Date.now()
38+
39+
this.resetInvoked = 0
40+
this.releaseInvoked = 0
41+
this.seenQueries = []
42+
this.seenParameters = []
43+
this.seenProtocolOptions = []
44+
this._server = {}
45+
this.protocolVersion = undefined
46+
this.protocolErrorsHandled = 0
47+
this.seenProtocolErrors = []
48+
this.seenRequestRoutingInformation = []
49+
}
50+
51+
get id () {
52+
return this._id
53+
}
54+
55+
get databaseId () {
56+
return this._databaseId
57+
}
58+
59+
set databaseId (value) {
60+
this._databaseId = value
61+
}
62+
63+
get server () {
64+
return this._server
65+
}
66+
67+
get version () {
68+
return this._server.version
69+
}
70+
71+
set version (value) {
72+
this._server.version = value
73+
}
74+
75+
protocol () {
76+
// return fake protocol object that simply records seen queries and parameters
77+
return {
78+
run: (query, parameters, protocolOptions) => {
79+
this.seenQueries.push(query)
80+
this.seenParameters.push(parameters)
81+
this.seenProtocolOptions.push(protocolOptions)
82+
},
83+
requestRoutingInformation: params => {
84+
this.seenRequestRoutingInformation.push(params)
85+
if (this._requestRoutingInformationMock) {
86+
this._requestRoutingInformationMock(params)
87+
}
88+
},
89+
version: this.protocolVersion
90+
}
91+
}
92+
93+
resetAndFlush () {
94+
this.resetInvoked++
95+
return Promise.resolve()
96+
}
97+
98+
_release () {
99+
this.releaseInvoked++
100+
return Promise.resolve()
101+
}
102+
103+
isOpen () {
104+
return this._open
105+
}
106+
107+
isNeverReleased () {
108+
return this.isReleasedTimes(0)
109+
}
110+
111+
isReleasedOnce () {
112+
return this.isReleasedTimes(1)
113+
}
114+
115+
isReleasedTimes (times) {
116+
return this.resetInvoked === times && this.releaseInvoked === times
117+
}
118+
119+
_handleProtocolError (message) {
120+
this.protocolErrorsHandled++
121+
this.seenProtocolErrors.push(message)
122+
}
123+
124+
withProtocolVersion (version) {
125+
this.protocolVersion = version
126+
return this
127+
}
128+
129+
withCreationTimestamp (value) {
130+
this.creationTimestamp = value
131+
return this
132+
}
133+
134+
withRequestRoutingInformationMock (requestRoutingInformationMock) {
135+
this._requestRoutingInformationMock = requestRoutingInformationMock
136+
return this
137+
}
138+
139+
closed () {
140+
this._open = false
141+
return this
142+
}
143+
}

test/internal/rediscovery.test.js renamed to bolt-connection/test/rediscovery/rediscovery.test.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
* limitations under the License.
1818
*/
1919

20-
import { RawRoutingTable } from '../../bolt-connection/lib/bolt'
21-
import Rediscovery from '../../bolt-connection/lib/rediscovery'
22-
import RoutingTable from '../../bolt-connection/lib/rediscovery/routing-table'
23-
import FakeConnection from './fake-connection'
20+
import { RawRoutingTable } from '../../src/bolt'
21+
import Rediscovery from '../../src/rediscovery'
22+
import RoutingTable from '../../src/rediscovery/routing-table'
23+
import FakeConnection from '../fake-connection'
2424
import lolex from 'lolex'
2525
import { newError, error, int, internal } from 'neo4j-driver-core'
2626

@@ -106,7 +106,6 @@ describe('#unit Rediscovery', () => {
106106
const requestParams = connection.seenRequestRoutingInformation[0]
107107
expect(requestParams.routingContext).toEqual(routingContext)
108108
expect(requestParams.databaseName).toEqual(database)
109-
expect(requestParams.initialAddress).toEqual(initialAddress)
110109
expect(requestParams.sessionContext).toEqual({
111110
bookmark: session._lastBookmark,
112111
mode: session._mode,

test/internal/fake-connection.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919

2020
import Connection from '../../bolt-connection/lib/connection/connection'
21-
import { textChangeRangeIsUnchanged } from 'typescript'
2221
import {
2322
ServerVersion,
2423
VERSION_3_4_0,

0 commit comments

Comments
 (0)