Skip to content

Commit 85d9a8b

Browse files
lutovichlutovich
committed
Fix routing driver boltkit test
And add early validation of the session mode parameter.
1 parent 2e0fc1e commit 85d9a8b

File tree

5 files changed

+68
-36
lines changed

5 files changed

+68
-36
lines changed

‎src/v1/driver.js‎

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

20-
import Session from './session';
21-
import Pool from './internal/pool';
22-
import Integer from './integer';
20+
import Session from "./session";
21+
import Pool from "./internal/pool";
2322
import {connect} from "./internal/connector";
24-
import StreamObserver from './internal/stream-observer';
23+
import StreamObserver from "./internal/stream-observer";
2524
import {newError, SERVICE_UNAVAILABLE} from "./error";
2625

2726
const READ = 'READ', WRITE = 'WRITE';
@@ -111,7 +110,8 @@ class Driver {
111110
* @return {Session} new session.
112111
*/
113112
session(mode) {
114-
let connectionPromise = this._acquireConnection(mode);
113+
const sessionMode = Driver._validateSessionMode(mode);
114+
const connectionPromise = this._acquireConnection(sessionMode);
115115
connectionPromise.catch((err) => {
116116
if (this.onError && err.code === SERVICE_UNAVAILABLE) {
117117
this.onError(err);
@@ -145,6 +145,14 @@ class Driver {
145145
});
146146
}
147147

148+
static _validateSessionMode(rawMode) {
149+
const mode = rawMode || WRITE;
150+
if (mode !== READ && mode !== WRITE) {
151+
throw newError('Illegal session mode ' + mode);
152+
}
153+
return mode;
154+
}
155+
148156
//Extension point
149157
_acquireConnection(mode) {
150158
return Promise.resolve(this._pool.acquire(this._url));

‎src/v1/index.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
import {int, isInt, inSafeRange, toNumber, toString} from './integer';
2121
import {Node, Relationship, UnboundRelationship, PathSegment, Path} from './graph-types'
22-
import {Neo4jError, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from './error';
22+
import {Neo4jError, SERVICE_UNAVAILABLE, SESSION_EXPIRED, PROTOCOL_ERROR} from './error';
2323
import Result from './result';
2424
import ResultSummary from './result-summary';
2525
import Record from './record';
@@ -138,7 +138,8 @@ const session = {
138138
};
139139
const error = {
140140
SERVICE_UNAVAILABLE,
141-
SESSION_EXPIRED
141+
SESSION_EXPIRED,
142+
PROTOCOL_ERROR
142143
};
143144
const integer = {
144145
toNumber,

‎src/v1/routing-driver.js‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,25 +117,27 @@ class RoutingDriver extends Driver {
117117

118118
return refreshedTablePromise.then(newRoutingTable => {
119119
if (newRoutingTable) {
120-
// correct routing table was fetched, we need to clean all connections that are not in the new table and
121-
// return the new table
120+
// valid routing table fetched, close old connections to servers not present in the new routing table
122121
const staleServers = currentRoutingTable.serversDiff(newRoutingTable);
123122
staleServers.forEach(server => this._pool.purge);
123+
124+
// make this driver instance aware of the new table
125+
this._routingTable = newRoutingTable;
126+
124127
return newRoutingTable
125128
}
126129
throw newError('Could not perform discovery. No routing servers available.', SERVICE_UNAVAILABLE);
127130
});
128131
}
129132

130133
_acquireConnection(mode) {
131-
const connectionMode = mode || WRITE;
132134
return this._refreshedRoutingTable().then(routingTable => {
133-
if (connectionMode === READ) {
135+
if (mode === READ) {
134136
return this._acquireConnectionToServer(routingTable.readers, "read");
135-
} else if (connectionMode === WRITE) {
137+
} else if (mode === WRITE) {
136138
return this._acquireConnectionToServer(routingTable.writers, "write");
137139
} else {
138-
return Promise.reject(connectionMode + " is not a valid option"); // todo: better message
140+
throw newError('Illegal session mode ' + mode);
139141
}
140142
});
141143
}
@@ -149,8 +151,8 @@ class RoutingDriver extends Driver {
149151
}
150152

151153
_forget(url) {
154+
this._routingTable.forget(url);
152155
this._pool.purge(url);
153-
this._routingTable.remove(url);
154156
}
155157

156158
static _validateConfig(config) {

‎test/v1/routing.driver.boltkit.it.js‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919

2020
var neo4j = require("../../lib/v1");
2121
var boltkit = require('./boltkit');
22-
describe('routing driver ', function () {
22+
23+
describe('routing driver', function () {
2324
var originalTimeout;
2425

2526
beforeAll(function(){
@@ -49,9 +50,9 @@ describe('routing driver ', function () {
4950
session.close();
5051
// Then
5152
expect(driver._pool.has('127.0.0.1:9001')).toBeTruthy();
52-
expect(driver._clusterView.routers.toArray()).toEqual(["127.0.0.1:9001", "127.0.0.1:9002", "127.0.0.1:9003"]);
53-
expect(driver._clusterView.readers.toArray()).toEqual(["127.0.0.1:9002", "127.0.0.1:9003"]);
54-
expect(driver._clusterView.writers.toArray()).toEqual(["127.0.0.1:9001"]);
53+
expect(driver._routingTable.routers.toArray()).toEqual(["127.0.0.1:9001", "127.0.0.1:9002", "127.0.0.1:9003"]);
54+
expect(driver._routingTable.readers.toArray()).toEqual(["127.0.0.1:9002", "127.0.0.1:9003"]);
55+
expect(driver._routingTable.writers.toArray()).toEqual(["127.0.0.1:9001"]);
5556

5657
driver.close();
5758
server.exit(function (code) {
@@ -78,9 +79,9 @@ describe('routing driver ', function () {
7879
session.run("MATCH (n) RETURN n.name").then(function () {
7980

8081
// Then
81-
expect(driver._clusterView.routers.toArray()).toEqual(["127.0.0.1:9004", "127.0.0.1:9002", "127.0.0.1:9003"]);
82-
expect(driver._clusterView.readers.toArray()).toEqual(["127.0.0.1:9005", "127.0.0.1:9003"]);
83-
expect(driver._clusterView.writers.toArray()).toEqual(["127.0.0.1:9001"]);
82+
expect(driver._routingTable.routers.toArray()).toEqual(["127.0.0.1:9004", "127.0.0.1:9002", "127.0.0.1:9003"]);
83+
expect(driver._routingTable.readers.toArray()).toEqual(["127.0.0.1:9005", "127.0.0.1:9003"]);
84+
expect(driver._routingTable.writers.toArray()).toEqual(["127.0.0.1:9001"]);
8485

8586
driver.close();
8687
server.exit(function (code) {
@@ -108,9 +109,9 @@ describe('routing driver ', function () {
108109
onCompleted: function () {
109110

110111
// Then
111-
expect(driver._clusterView.routers.toArray()).toEqual(["127.0.0.1:9004", "127.0.0.1:9002", "127.0.0.1:9003"]);
112-
expect(driver._clusterView.readers.toArray()).toEqual(["127.0.0.1:9005", "127.0.0.1:9003"]);
113-
expect(driver._clusterView.writers.toArray()).toEqual(["127.0.0.1:9001"]);
112+
expect(driver._routingTable.routers.toArray()).toEqual(["127.0.0.1:9004", "127.0.0.1:9002", "127.0.0.1:9003"]);
113+
expect(driver._routingTable.readers.toArray()).toEqual(["127.0.0.1:9005", "127.0.0.1:9003"]);
114+
expect(driver._routingTable.writers.toArray()).toEqual(["127.0.0.1:9001"]);
114115

115116
driver.close();
116117
server.exit(function (code) {
@@ -137,7 +138,7 @@ describe('routing driver ', function () {
137138
// When
138139
var session = driver.session(neo4j.READ);
139140
session.run("MATCH (n) RETURN n.name").catch(function (err) {
140-
expect(err.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
141+
expect(err.code).toEqual(neo4j.error.PROTOCOL_ERROR);
141142

142143
session.close();
143144
driver.close();
@@ -418,9 +419,9 @@ describe('routing driver ', function () {
418419
session.run("MATCH (n) RETURN n.name").then(function () {
419420

420421
// Then
421-
expect(driver._clusterView.routers.toArray()).toEqual(['127.0.0.1:9001', '127.0.0.1:9002', '127.0.0.1:9003']);
422-
expect(driver._clusterView.readers.toArray()).toEqual(['127.0.0.1:9005', '127.0.0.1:9006']);
423-
expect(driver._clusterView.writers.toArray()).toEqual(['127.0.0.1:9007', '127.0.0.1:9008']);
422+
expect(driver._routingTable.routers.toArray()).toEqual(['127.0.0.1:9001', '127.0.0.1:9002', '127.0.0.1:9003']);
423+
expect(driver._routingTable.readers.toArray()).toEqual(['127.0.0.1:9005', '127.0.0.1:9006']);
424+
expect(driver._routingTable.writers.toArray()).toEqual(['127.0.0.1:9007', '127.0.0.1:9008']);
424425
driver.close();
425426
seedServer.exit(function (code1) {
426427
readServer.exit(function (code2) {
@@ -452,9 +453,9 @@ describe('routing driver ', function () {
452453
// Then
453454
expect(driver._pool.has('127.0.0.1:9001')).toBeTruthy();
454455
expect(driver._pool.has('127.0.0.1:9005')).toBeFalsy();
455-
expect(driver._clusterView.routers.toArray()).toEqual(['127.0.0.1:9001', '127.0.0.1:9002', '127.0.0.1:9003']);
456-
expect(driver._clusterView.readers.toArray()).toEqual(['127.0.0.1:9006']);
457-
expect(driver._clusterView.writers.toArray()).toEqual(['127.0.0.1:9007', '127.0.0.1:9008']);
456+
expect(driver._routingTable.routers.toArray()).toEqual(['127.0.0.1:9001', '127.0.0.1:9002', '127.0.0.1:9003']);
457+
expect(driver._routingTable.readers.toArray()).toEqual(['127.0.0.1:9006']);
458+
expect(driver._routingTable.writers.toArray()).toEqual(['127.0.0.1:9007', '127.0.0.1:9008']);
458459
driver.close();
459460
seedServer.exit(function (code1) {
460461
readServer.exit(function (code2) {
@@ -485,9 +486,9 @@ describe('routing driver ', function () {
485486
// Then
486487
expect(driver._pool.has('127.0.0.1:9001')).toBeTruthy();
487488
expect(driver._pool.has('127.0.0.1:9005')).toBeFalsy();
488-
expect(driver._clusterView.routers.toArray()).toEqual(['127.0.0.1:9001', '127.0.0.1:9002', '127.0.0.1:9003']);
489-
expect(driver._clusterView.readers.toArray()).toEqual(['127.0.0.1:9006']);
490-
expect(driver._clusterView.writers.toArray()).toEqual(['127.0.0.1:9007', '127.0.0.1:9008']);
489+
expect(driver._routingTable.routers.toArray()).toEqual(['127.0.0.1:9001', '127.0.0.1:9002', '127.0.0.1:9003']);
490+
expect(driver._routingTable.readers.toArray()).toEqual(['127.0.0.1:9006']);
491+
expect(driver._routingTable.writers.toArray()).toEqual(['127.0.0.1:9007', '127.0.0.1:9008']);
491492
driver.close();
492493
seedServer.exit(function (code) {
493494
expect(code).toEqual(0);
@@ -568,7 +569,7 @@ describe('routing driver ', function () {
568569
var session = driver.session();
569570
session.run("CREATE ()").catch(function (err) {
570571
//the server at 9007 should have been removed
571-
expect(driver._clusterView.writers.toArray()).toEqual(['127.0.0.1:9008']);
572+
expect(driver._routingTable.writers.toArray()).toEqual(['127.0.0.1:9008']);
572573
expect(err.code).toEqual(neo4j.error.SESSION_EXPIRED);
573574
session.close();
574575
driver.close();
@@ -602,7 +603,7 @@ describe('routing driver ', function () {
602603

603604
tx.commit().catch(function (err) {
604605
//the server at 9007 should have been removed
605-
expect(driver._clusterView.writers.toArray()).toEqual(['127.0.0.1:9008']);
606+
expect(driver._routingTable.writers.toArray()).toEqual(['127.0.0.1:9008']);
606607
expect(err.code).toEqual(neo4j.error.SESSION_EXPIRED);
607608
session.close();
608609
driver.close();

‎test/v1/session.test.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,26 @@ describe('session', function () {
375375
expect(() => session.beginTransaction([])).toThrowError(TypeError);
376376
expect(() => session.beginTransaction(['bookmark'])).toThrowError(TypeError);
377377
});
378+
379+
it('should allow creation of a ' + neo4j.session.READ + ' session', done => {
380+
const readSession = driver.session(neo4j.session.READ);
381+
readSession.run('RETURN 1').then(() => {
382+
readSession.close();
383+
done();
384+
});
385+
});
386+
387+
it('should allow creation of a ' + neo4j.session.WRITE + ' session', done => {
388+
const writeSession = driver.session(neo4j.session.WRITE);
389+
writeSession.run('CREATE ()').then(() => {
390+
writeSession.close();
391+
done();
392+
});
393+
});
394+
395+
it('should fail for illegal session mode', () => {
396+
expect(() => driver.session('ILLEGAL_MODE')).toThrow();
397+
});
378398
});
379399

380400

0 commit comments

Comments
 (0)