From 038b9bc2b6a3d5acbc358768b866200b3d123d1c Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 01:15:07 +0100 Subject: [PATCH 01/12] Use dedicated ServerAddress class for holding address information --- src/v1/driver.js | 20 +- src/v1/index.js | 5 +- src/v1/internal/browser/browser-channel.js | 23 +- src/v1/internal/channel-config.js | 6 +- src/v1/internal/connection-error-handler.js | 8 +- src/v1/internal/connection-providers.js | 30 +- src/v1/internal/connection.js | 23 +- src/v1/internal/http/http-driver.js | 8 +- src/v1/internal/node/node-channel.js | 20 +- .../internal/node/node-host-name-resolver.js | 23 +- src/v1/internal/pool.js | 65 +- ...olver.js => configured-custom-resolver.js} | 15 +- src/v1/internal/routing-table.js | 10 +- src/v1/internal/routing-util.js | 7 +- src/v1/internal/server-address.js | 64 ++ src/v1/routing-driver.js | 39 +- test/internal/browser/browser-channel.test.js | 21 +- test/internal/channel-config.test.js | 10 +- .../internal/connection-error-handler.test.js | 21 +- test/internal/connection-providers.test.js | 743 +++++++++--------- test/internal/connection.test.js | 17 +- .../node/node-host-name-resolver.test.js | 35 +- .../node/routing.driver.boltkit.test.js | 42 +- test/internal/pool.test.js | 261 +++--- test/internal/rediscovery.test.js | 15 +- test/internal/routing-table.test.js | 116 +-- test/internal/routing-util.test.js | 7 +- test/v1/session.test.js | 5 +- 28 files changed, 894 insertions(+), 765 deletions(-) rename src/v1/internal/resolver/{configured-host-name-resolver.js => configured-custom-resolver.js} (76%) create mode 100644 src/v1/internal/server-address.js diff --git a/src/v1/driver.js b/src/v1/driver.js index f667b55ca..bf4ddcb17 100644 --- a/src/v1/driver.js +++ b/src/v1/driver.js @@ -62,17 +62,17 @@ class Driver { /** * You should not be calling this directly, instead use {@link driver}. * @constructor - * @param {string} hostPort + * @param {ServerAddress} address * @param {string} userAgent * @param {object} authToken * @param {object} config * @protected */ - constructor(hostPort, userAgent, authToken = {}, config = {}) { + constructor(address, userAgent, authToken = {}, config = {}) { sanitizeConfig(config); this._id = idGenerator++; - this._hostPort = hostPort; + this._address = address; this._userAgent = userAgent; this._openConnections = {}; this._authToken = authToken; @@ -102,7 +102,7 @@ class Driver { * @protected */ _afterConstruction() { - this._log.info(`Direct driver ${this._id} created for server address ${this._hostPort}`); + this._log.info(`Direct driver ${this._id} created for server address ${this._address}`); } /** @@ -133,9 +133,9 @@ class Driver { * @return {Promise} promise resolved with a new connection or rejected when failed to connect. * @access private */ - _createConnection(hostPort, release) { - const connection = Connection.create(hostPort, this._config, this._createConnectionErrorHandler(), this._log); - connection._release = () => release(hostPort, connection); + _createConnection(address, release) { + const connection = Connection.create(address, this._config, this._createConnectionErrorHandler(), this._log); + connection._release = () => release(address, connection); this._openConnections[connection.id] = connection; return connection.connect(this._userAgent, this._authToken) @@ -206,8 +206,8 @@ class Driver { } // Extension point - _createConnectionProvider(hostPort, connectionPool, driverOnErrorCallback) { - return new DirectConnectionProvider(hostPort, connectionPool, driverOnErrorCallback); + _createConnectionProvider(address, connectionPool, driverOnErrorCallback) { + return new DirectConnectionProvider(address, connectionPool, driverOnErrorCallback); } // Extension point @@ -218,7 +218,7 @@ class Driver { _getOrCreateConnectionProvider() { if (!this._connectionProvider) { const driverOnErrorCallback = this._driverOnErrorCallback.bind(this); - this._connectionProvider = this._createConnectionProvider(this._hostPort, this._pool, driverOnErrorCallback); + this._connectionProvider = this._createConnectionProvider(this._address, this._pool, driverOnErrorCallback); } return this._connectionProvider; } diff --git a/src/v1/index.js b/src/v1/index.js index 91a1608a4..5d73b17df 100644 --- a/src/v1/index.js +++ b/src/v1/index.js @@ -31,6 +31,7 @@ import urlUtil from './internal/url-util'; import HttpDriver from './internal/http/http-driver'; import {isPoint, Point} from './spatial-types'; import {Date, DateTime, Duration, isDate, isDateTime, isDuration, isLocalDateTime, isLocalTime, isTime, LocalDateTime, LocalTime, Time} from './temporal-types'; +import ServerAddress from './internal/server-address'; /** * @property {function(username: string, password: string, realm: ?string)} basic the function to create a @@ -227,12 +228,12 @@ function driver(url, authToken, config = {}) { assertString(url, 'Bolt URL'); const parsedUrl = urlUtil.parseDatabaseUrl(url); if (parsedUrl.scheme === 'bolt+routing') { - return new RoutingDriver(parsedUrl.hostAndPort, parsedUrl.query, USER_AGENT, authToken, config); + return new RoutingDriver(ServerAddress.fromUrl(parsedUrl.hostAndPort), parsedUrl.query, USER_AGENT, authToken, config); } else if (parsedUrl.scheme === 'bolt') { if (!isEmptyObjectOrNull(parsedUrl.query)) { throw new Error(`Parameters are not supported with scheme 'bolt'. Given URL: '${url}'`); } - return new Driver(parsedUrl.hostAndPort, USER_AGENT, authToken, config); + return new Driver(ServerAddress.fromUrl(parsedUrl.hostAndPort), USER_AGENT, authToken, config); } else if (parsedUrl.scheme === 'http' || parsedUrl.scheme === 'https') { return new HttpDriver(parsedUrl, USER_AGENT, authToken, config); } else { diff --git a/src/v1/internal/browser/browser-channel.js b/src/v1/internal/browser/browser-channel.js index 0046680a7..10ee4ff7c 100644 --- a/src/v1/internal/browser/browser-channel.js +++ b/src/v1/internal/browser/browser-channel.js @@ -46,7 +46,7 @@ export default class WebSocketChannel { return; } - this._ws = createWebSocket(scheme, config.url); + this._ws = createWebSocket(scheme, config.address); this._ws.binaryType = "arraybuffer"; let self = this; @@ -169,13 +169,13 @@ export default class WebSocketChannel { } } -function createWebSocket(scheme, parsedUrl) { - const url = scheme + '://' + parsedUrl.hostAndPort; +function createWebSocket(scheme, address) { + const url = scheme + '://' + address.asHostPort(); try { return new WebSocket(url); } catch (error) { - if (isIPv6AddressIssueOnWindows(error, parsedUrl)) { + if (isIPv6AddressIssueOnWindows(error, address)) { // WebSocket in IE and Edge browsers on Windows do not support regular IPv6 address syntax because they contain ':'. // It's an invalid character for UNC (https://en.wikipedia.org/wiki/IPv6_address#Literal_IPv6_addresses_in_UNC_path_names) @@ -190,7 +190,7 @@ function createWebSocket(scheme, parsedUrl) { // Creation of WebSocket with unconverted address results in SyntaxError without message or stacktrace. // That is why here we "catch" SyntaxError and rewrite IPv6 address if needed. - const windowsFriendlyUrl = asWindowsFriendlyIPv6Address(scheme, parsedUrl); + const windowsFriendlyUrl = asWindowsFriendlyIPv6Address(scheme, address); return new WebSocket(windowsFriendlyUrl); } else { throw error; @@ -198,18 +198,17 @@ function createWebSocket(scheme, parsedUrl) { } } -function isIPv6AddressIssueOnWindows(error, parsedUrl) { - return error.name === 'SyntaxError' && isIPv6Address(parsedUrl); +function isIPv6AddressIssueOnWindows(error, address) { + return error.name === 'SyntaxError' && isIPv6Address(address.asHostPort()); } -function isIPv6Address(parsedUrl) { - const hostAndPort = parsedUrl.hostAndPort; +function isIPv6Address(hostAndPort) { return hostAndPort.charAt(0) === '[' && hostAndPort.indexOf(']') !== -1; } -function asWindowsFriendlyIPv6Address(scheme, parsedUrl) { +function asWindowsFriendlyIPv6Address(scheme, address) { // replace all ':' with '-' - const hostWithoutColons = parsedUrl.host.replace(new RegExp(':', 'g'), '-'); + const hostWithoutColons = address.host().replace(new RegExp(':', 'g'), '-'); // replace '%' with 's' for link-local IPv6 address like 'fe80::1%lo0' const hostWithoutPercent = hostWithoutColons.replace('%', 's'); @@ -217,7 +216,7 @@ function asWindowsFriendlyIPv6Address(scheme, parsedUrl) { // append magic '.ipv6-literal.net' suffix const ipv6Host = hostWithoutPercent + '.ipv6-literal.net'; - return `${scheme}://${ipv6Host}:${parsedUrl.port}`; + return `${scheme}://${ipv6Host}:${address.port()}`; } /** diff --git a/src/v1/internal/channel-config.js b/src/v1/internal/channel-config.js index 50f2f2178..bfd9cf522 100644 --- a/src/v1/internal/channel-config.js +++ b/src/v1/internal/channel-config.js @@ -31,12 +31,12 @@ export default class ChannelConfig { /** * @constructor - * @param {Url} url the URL for the channel to connect to. + * @param {ServerAddress} address the address for the channel to connect to. * @param {object} driverConfig the driver config provided by the user when driver is created. * @param {string} connectionErrorCode the default error code to use on connection errors. */ - constructor(url, driverConfig, connectionErrorCode) { - this.url = url; + constructor(address, driverConfig, connectionErrorCode) { + this.address = address; this.encrypted = extractEncrypted(driverConfig); this.trust = extractTrust(driverConfig); this.trustedCertificates = extractTrustedCertificates(driverConfig); diff --git a/src/v1/internal/connection-error-handler.js b/src/v1/internal/connection-error-handler.js index 9666f48a6..3c1def379 100644 --- a/src/v1/internal/connection-error-handler.js +++ b/src/v1/internal/connection-error-handler.js @@ -38,15 +38,15 @@ export default class ConnectionErrorHandler { /** * Handle and transform the error. * @param {Neo4jError} error the original error. - * @param {string} hostPort the host and port of the connection where the error happened. + * @param {ServerAddress} address the address of the connection where the error happened. * @return {Neo4jError} new error that should be propagated to the user. */ - handleAndTransformError(error, hostPort) { + handleAndTransformError(error, address) { if (isAvailabilityError(error)) { - return this._handleUnavailability(error, hostPort); + return this._handleUnavailability(error, address); } if (isFailureToWrite(error)) { - return this._handleWriteFailure(error, hostPort); + return this._handleWriteFailure(error, address); } return error; } diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index e88b488d3..0af727cd3 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -23,6 +23,8 @@ import Session from '../session'; import RoutingTable from './routing-table'; import Rediscovery from './rediscovery'; import RoutingUtil from './routing-util'; +import { HostNameResolver } from './node'; +import { flatMap } from 'lodash/collection'; const UNAUTHORIZED_ERROR_CODE = 'Neo.ClientError.Security.Unauthorized'; @@ -45,38 +47,40 @@ class ConnectionProvider { export class DirectConnectionProvider extends ConnectionProvider { - constructor(hostPort, connectionPool, driverOnErrorCallback) { + constructor(address, connectionPool, driverOnErrorCallback) { super(); - this._hostPort = hostPort; + this._address = address; this._connectionPool = connectionPool; this._driverOnErrorCallback = driverOnErrorCallback; } acquireConnection(mode) { - const connectionPromise = this._connectionPool.acquire(this._hostPort); + const connectionPromise = this._connectionPool.acquire(this._address); return this._withAdditionalOnErrorCallback(connectionPromise, this._driverOnErrorCallback); } } export class LoadBalancer extends ConnectionProvider { - constructor(hostPort, routingContext, connectionPool, loadBalancingStrategy, hostNameResolver, driverOnErrorCallback, log) { + constructor(address, routingContext, connectionPool, loadBalancingStrategy, hostNameResolver, driverOnErrorCallback, log) { super(); - this._seedRouter = hostPort; + this._seedRouter = address; this._routingTable = new RoutingTable([this._seedRouter]); this._rediscovery = new Rediscovery(new RoutingUtil(routingContext)); this._connectionPool = connectionPool; this._driverOnErrorCallback = driverOnErrorCallback; this._loadBalancingStrategy = loadBalancingStrategy; this._hostNameResolver = hostNameResolver; + this._dnsResolver = new HostNameResolver(); this._log = log; this._useSeedRouter = false; } acquireConnection(accessMode) { + let that = this; const connectionPromise = this._freshRoutingTable(accessMode).then(routingTable => { if (accessMode === READ) { - const address = this._loadBalancingStrategy.selectReader(routingTable.readers); + const address = that._loadBalancingStrategy.selectReader(routingTable.readers); return this._acquireConnectionToServer(address, 'read'); } else if (accessMode === WRITE) { const address = this._loadBalancingStrategy.selectWriter(routingTable.writers); @@ -173,7 +177,7 @@ export class LoadBalancer extends ConnectionProvider { } _fetchRoutingTableUsingSeedRouter(seenRouters, seedRouter) { - const resolvedAddresses = this._hostNameResolver.resolve(seedRouter); + const resolvedAddresses = this._resolveSeedRouter(seedRouter); return resolvedAddresses.then(resolvedRouterAddresses => { // filter out all addresses that we've already tried const newAddresses = resolvedRouterAddresses.filter(address => seenRouters.indexOf(address) < 0); @@ -181,6 +185,18 @@ export class LoadBalancer extends ConnectionProvider { }); } + _resolveSeedRouter(seedRouter) { + const customResolution = this._hostNameResolver.resolve(seedRouter); + const dnsResolutions = customResolution.then(resolvedAddresses => { + return Promise.all(resolvedAddresses.map(address => { + return this._dnsResolver.resolve(address); + })); + }); + return dnsResolutions.then(results => { + return [].concat.apply([], results); + }); + } + _fetchRoutingTable(routerAddresses, routingTable) { return routerAddresses.reduce((refreshedTablePromise, currentRouter, currentIndex) => { return refreshedTablePromise.then(newRoutingTable => { diff --git a/src/v1/internal/connection.js b/src/v1/internal/connection.js index e756de5ee..d24c4a5f2 100644 --- a/src/v1/internal/connection.js +++ b/src/v1/internal/connection.js @@ -50,14 +50,14 @@ export default class Connection { * @constructor * @param {Channel} channel - channel with a 'write' function and a 'onmessage' callback property. * @param {ConnectionErrorHandler} errorHandler the error handler. - * @param {string} hostPort - the hostname and port to connect to. + * @param {ServerAddress} address - the server address to connect to. * @param {Logger} log - the configured logger. * @param {boolean} disableLosslessIntegers if this connection should convert all received integers to native JS numbers. */ - constructor(channel, errorHandler, hostPort, log, disableLosslessIntegers = false) { + constructor(channel, errorHandler, address, log, disableLosslessIntegers = false) { this.id = idGenerator++; - this.hostPort = hostPort; - this.server = {address: hostPort}; + this.address = address; + this.server = { address: address.asHostPort() }; this.creationTimestamp = Date.now(); this._errorHandler = errorHandler; this._disableLosslessIntegers = disableLosslessIntegers; @@ -81,22 +81,21 @@ export default class Connection { this._isBroken = false; if (this._log.isDebugEnabled()) { - this._log.debug(`${this} created towards ${hostPort}`); + this._log.debug(`${this} created towards ${address}`); } } /** * Crete new connection to the provided address. Returned connection is not connected. - * @param {string} url - the Bolt endpoint to connect to. + * @param {ServerAddress} address - the Bolt endpoint to connect to. * @param {object} config - this driver configuration. * @param {ConnectionErrorHandler} errorHandler - the error handler for connection errors. * @param {Logger} log - configured logger. * @return {Connection} - new connection. */ - static create(url, config, errorHandler, log) { - const parsedAddress = urlUtil.parseDatabaseUrl(url); - const channelConfig = new ChannelConfig(parsedAddress, config, errorHandler.errorCode()); - return new Connection(new Channel(channelConfig), errorHandler, parsedAddress.hostAndPort, log, config.disableLosslessIntegers); + static create(address, config, errorHandler, log) { + const channelConfig = new ChannelConfig(address, config, errorHandler.errorCode()); + return new Connection(new Channel(channelConfig), errorHandler, address, log, config.disableLosslessIntegers); } /** @@ -217,7 +216,7 @@ export default class Connection { */ _handleFatalError(error) { this._isBroken = true; - this._error = this._errorHandler.handleAndTransformError(error, this.hostPort); + this._error = this._errorHandler.handleAndTransformError(error, this.address); if (this._log.isErrorEnabled()) { this._log.error(`${this} experienced a fatal error ${JSON.stringify(this._error)}`); @@ -267,7 +266,7 @@ export default class Connection { } try { const error = newError(payload.message, payload.code); - this._currentFailure = this._errorHandler.handleAndTransformError(error, this.hostPort); + this._currentFailure = this._errorHandler.handleAndTransformError(error, this.address); this._currentObserver.onError( this._currentFailure ); } finally { this._updateCurrentObserver(); diff --git a/src/v1/internal/http/http-driver.js b/src/v1/internal/http/http-driver.js index bd4ea8cc1..434cd7795 100644 --- a/src/v1/internal/http/http-driver.js +++ b/src/v1/internal/http/http-driver.js @@ -20,16 +20,18 @@ import Driver from '../../driver'; import HttpSession from './http-session'; import HttpSessionTracker from './http-session-tracker'; +import ServerAddress from '../server-address'; export default class HttpDriver extends Driver { - constructor(hostPort, userAgent, token, config) { - super(hostPort, userAgent, token, config); + constructor(url, userAgent, token, config) { + super(ServerAddress.fromUrl(url.hostAndPort), userAgent, token, config); + this._url = url; this._sessionTracker = new HttpSessionTracker(); } session() { - return new HttpSession(this._hostPort, this._authToken, this._config, this._sessionTracker); + return new HttpSession(this._url, this._authToken, this._config, this._sessionTracker); } close() { diff --git a/src/v1/internal/node/node-channel.js b/src/v1/internal/node/node-channel.js index 0ff817588..3b5c7360b 100644 --- a/src/v1/internal/node/node-channel.js +++ b/src/v1/internal/node/node-channel.js @@ -125,8 +125,8 @@ const TrustStrategy = { return; } - const tlsOpts = newTlsOptions(config.url.host, config.trustedCertificates.map((f) => fs.readFileSync(f))); - const socket = tls.connect(config.url.port, config.url.host, tlsOpts, function () { + const tlsOpts = newTlsOptions(config.address.host(), config.trustedCertificates.map((f) => fs.readFileSync(f))); + const socket = tls.connect(config.address.port(), config.address.resolvedHost(), tlsOpts, function () { if (!socket.authorized) { onFailure(newError("Server certificate is not trusted. If you trust the database you are connecting to, add" + " the signing certificate, or the server certificate, to the list of certificates trusted by this driver" + @@ -142,8 +142,8 @@ const TrustStrategy = { return configureSocket(socket); }, TRUST_SYSTEM_CA_SIGNED_CERTIFICATES : function( config, onSuccess, onFailure ) { - const tlsOpts = newTlsOptions(config.url.host); - const socket = tls.connect(config.url.port, config.url.host, tlsOpts, function () { + const tlsOpts = newTlsOptions(config.address.host()); + const socket = tls.connect(config.address.port(), config.address.resolvedHost(), tlsOpts, function () { if (!socket.authorized) { onFailure(newError("Server certificate is not trusted. If you trust the database you are connecting to, use " + "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES and add" + @@ -166,8 +166,8 @@ const TrustStrategy = { console.warn('`TRUST_ON_FIRST_USE` has been deprecated as option and will be removed in a future version of ' + "the driver. Please use `TRUST_ALL_CERTIFICATES` instead."); - const tlsOpts = newTlsOptions(config.url.host); - const socket = tls.connect(config.url.port, config.url.host, tlsOpts, function () { + const tlsOpts = newTlsOptions(config.address.host()); + const socket = tls.connect(config.address.port(), config.address.resolvedHost(), tlsOpts, function () { const serverCert = socket.getPeerCertificate(/*raw=*/true); if( !serverCert.raw ) { @@ -184,7 +184,7 @@ const TrustStrategy = { const serverFingerprint = crypto.createHash('sha512').update(serverCert.raw).digest('hex'); const knownHostsPath = config.knownHostsPath || path.join(userHome(), ".neo4j", "known_hosts"); - const serverId = config.url.hostAndPort; + const serverId = config.address.asHostPort(); loadFingerprint(serverId, knownHostsPath, (knownFingerprint) => { if( knownFingerprint === serverFingerprint ) { @@ -216,8 +216,8 @@ const TrustStrategy = { }, TRUST_ALL_CERTIFICATES: function (config, onSuccess, onFailure) { - const tlsOpts = newTlsOptions(config.url.host); - const socket = tls.connect(config.url.port, config.url.host, tlsOpts, function () { + const tlsOpts = newTlsOptions(config.address.host()); + const socket = tls.connect(config.address.port(), config.address.resolvedHost(), tlsOpts, function () { const certificate = socket.getPeerCertificate(); if (isEmptyObjectOrNull(certificate)) { onFailure(newError("Secure connection was successful but server did not return any valid " + @@ -244,7 +244,7 @@ const TrustStrategy = { function connect( config, onSuccess, onFailure=(()=>null) ) { const trustStrategy = trustStrategyName(config); if (!isEncrypted(config)) { - const socket = net.connect(config.url.port, config.url.host, onSuccess); + const socket = net.connect(config.address.port(), config.address.resolvedHost(), onSuccess); socket.on('error', onFailure); return configureSocket(socket); } else if (TrustStrategy[trustStrategy]) { diff --git a/src/v1/internal/node/node-host-name-resolver.js b/src/v1/internal/node/node-host-name-resolver.js index 3cac4b668..3eb4e4e89 100644 --- a/src/v1/internal/node/node-host-name-resolver.js +++ b/src/v1/internal/node/node-host-name-resolver.js @@ -23,29 +23,16 @@ import nodeDns from 'dns'; export default class NodeHostNameResolver extends BaseHostNameResolver { - resolve(seedRouter) { - const parsedAddress = urlUtil.parseDatabaseUrl(seedRouter); - + resolve(address) { return new Promise((resolve) => { - nodeDns.lookup(parsedAddress.host, {all: true}, (error, addresses) => { + nodeDns.lookup(address.host(), { all: true }, (error, resolvedTo) => { if (error) { - resolve(this._resolveToItself(seedRouter)); + resolve([address]); } else { - const addressesWithPorts = addresses.map(address => addressWithPort(address, parsedAddress.port)); - resolve(addressesWithPorts); + const resolvedAddresses = resolvedTo.map(a => address.resolveWith(a.address)); + resolve(resolvedAddresses); } }); }); } } - -function addressWithPort(addressObject, port) { - const address = addressObject.address; - const addressFamily = addressObject.family; - - if (!port) { - return address; - } - - return addressFamily === 6 ? urlUtil.formatIPv6Address(address, port) : urlUtil.formatIPv4Address(address, port); -} diff --git a/src/v1/internal/pool.js b/src/v1/internal/pool.js index 967773d00..1523a28af 100644 --- a/src/v1/internal/pool.js +++ b/src/v1/internal/pool.js @@ -51,11 +51,13 @@ class Pool { /** * Acquire and idle resource fom the pool or create a new one. - * @param {string} key the resource key. + * @param {ServerAddress} address the address for which we're acquiring. * @return {object} resource that is ready to use. */ - acquire(key) { - return this._acquire(key).then(resource => { + acquire(address) { + return this._acquire(address).then(resource => { + const key = address.asKey(); + if (resource) { resourceAcquired(key, this._activeResourceCounts); if (this._log.isDebugEnabled()) { @@ -99,44 +101,40 @@ class Pool { } /** - * Destroy all idle resources for the given key. - * @param {string} key the resource key to purge. + * Destroy all idle resources for the given address. + * @param {ServerAddress} address the address of the server to purge its pool. */ - purge(key) { - const pool = this._pools[key] || []; - while (pool.length) { - const resource = pool.pop(); - this._destroy(resource) - } - delete this._pools[key] + purge(address) { + this._purgeKey(address.asKey()) } /** * Destroy all idle resources in this pool. */ purgeAll() { - Object.keys(this._pools).forEach(key => this.purge(key)); + Object.keys(this._pools).forEach(key => this._purgeKey(key)); } /** - * Check if this pool contains resources for the given key. - * @param {string} key the resource key to check. + * Check if this pool contains resources for the given address. + * @param {ServerAddress} address the address of the server to check. * @return {boolean} `true` when pool contains entries for the given key, false otherwise. */ - has(key) { - return (key in this._pools); + has(address) { + return (address.asKey() in this._pools); } /** * Get count of active (checked out of the pool) resources for the given key. - * @param {string} key the resource key to check. + * @param {ServerAddress} address the address of the server to check. * @return {number} count of resources acquired by clients. */ - activeResourceCount(key) { - return this._activeResourceCounts[key] || 0; + activeResourceCount(address) { + return this._activeResourceCounts[address.asKey()] || 0; } - _acquire(key) { + _acquire(address) { + const key = address.asKey(); let pool = this._pools[key]; if (!pool) { pool = []; @@ -153,15 +151,16 @@ class Pool { } } - if (this._maxSize && this.activeResourceCount(key) >= this._maxSize) { + if (this._maxSize && this.activeResourceCount(address) >= this._maxSize) { return Promise.resolve(null); } // there exist no idle valid resources, create a new one for acquisition - return this._create(key, this._release); + return this._create(address, this._release); } - _release(key, resource) { + _release(address, resource) { + const key = address.asKey(); const pool = this._pools[key]; if (pool) { @@ -186,16 +185,26 @@ class Pool { } resourceReleased(key, this._activeResourceCounts); - this._processPendingAcquireRequests(key); + this._processPendingAcquireRequests(address); + } + + _purgeKey(key) { + const pool = this._pools[key] || []; + while (pool.length) { + const resource = pool.pop(); + this._destroy(resource); + } + delete this._pools[key]; } - _processPendingAcquireRequests(key) { + _processPendingAcquireRequests(address) { + const key = address.asKey(); const requests = this._acquireRequests[key]; if (requests) { const pendingRequest = requests.shift(); // pop a pending acquire request if (pendingRequest) { - this._acquire(key) + this._acquire(address) .catch(error => { // failed to acquire/create a new connection to resolve the pending acquire request // propagate the error by failing the pending request @@ -209,7 +218,7 @@ class Pool { if (pendingRequest.isCompleted()) { // request has been completed, most likely failed by a timeout // return the acquired resource back to the pool - this._release(key, resource); + this._release(address, resource); } else { // request is still pending and can be resolved with the newly acquired resource resourceAcquired(key, this._activeResourceCounts); // increment the active counter diff --git a/src/v1/internal/resolver/configured-host-name-resolver.js b/src/v1/internal/resolver/configured-custom-resolver.js similarity index 76% rename from src/v1/internal/resolver/configured-host-name-resolver.js rename to src/v1/internal/resolver/configured-custom-resolver.js index 5b6747259..7d5555d9c 100644 --- a/src/v1/internal/resolver/configured-host-name-resolver.js +++ b/src/v1/internal/resolver/configured-custom-resolver.js @@ -16,24 +16,25 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import ServerAddress from '../server-address'; -import BaseHostNameResolver from './base-host-name-resolver'; - -export default class ConfiguredHostNameResolver extends BaseHostNameResolver { +function resolveToSelf(address) { + return Promise.resolve([address]); +} +export default class ConfiguredCustomResolver { constructor(resolverFunction) { - super(); - this._resolverFunction = resolverFunction; + this._resolverFunction = resolverFunction ? resolverFunction : resolveToSelf; } resolve(seedRouter) { - return new Promise(resolve => resolve(this._resolverFunction(seedRouter))) + return new Promise(resolve => resolve(this._resolverFunction(seedRouter.asHostPort()))) .then(resolved => { if (!Array.isArray(resolved)) { throw new TypeError(`Configured resolver function should either return an array of addresses or a Promise resolved with an array of addresses.` + `Each address is ':'. Got: ${resolved}`); } - return resolved; + return resolved.map(r => ServerAddress.fromUrl(r)); }); } } diff --git a/src/v1/internal/routing-table.js b/src/v1/internal/routing-table.js index 318db07c8..84971d9ec 100644 --- a/src/v1/internal/routing-table.js +++ b/src/v1/internal/routing-table.js @@ -48,10 +48,12 @@ export default class RoutingTable { } serversDiff(otherRoutingTable) { - const oldServers = new Set(this._allServers()); + const oldServers = this._allServers(); const newServers = otherRoutingTable._allServers(); - newServers.forEach(newServer => oldServers.delete(newServer)); - return Array.from(oldServers); + const diffTable = {}; + oldServers.forEach(oldServer => diffTable[oldServer.asKey()] = oldServer); + newServers.forEach(newServer => delete diffTable[newServer.asKey()]); + return Object.values(diffTable); } /** @@ -87,5 +89,5 @@ export default class RoutingTable { * @return {Array} new filtered array. */ function removeFromArray(array, element) { - return array.filter(item => item !== element); + return array.filter(item => item.asKey() !== element.asKey()); } diff --git a/src/v1/internal/routing-util.js b/src/v1/internal/routing-util.js index 9194379de..59f7b18a3 100644 --- a/src/v1/internal/routing-util.js +++ b/src/v1/internal/routing-util.js @@ -23,6 +23,7 @@ import {ServerVersion, VERSION_3_2_0} from './server-version'; import Bookmark from './bookmark'; import TxConfig from './tx-config'; import {ACCESS_MODE_WRITE} from "./constants"; +import ServerAddress from './server-address'; const CALL_GET_SERVERS = 'CALL dbms.cluster.routing.getServers'; const CALL_GET_ROUTING_TABLE = 'CALL dbms.cluster.routing.getRoutingTable($context)'; @@ -88,11 +89,11 @@ export default class RoutingUtil { const addresses = server['addresses']; if (role === 'ROUTE') { - routers = parseArray(addresses); + routers = parseArray(addresses).map(address => ServerAddress.fromUrl(address)); } else if (role === 'WRITE') { - writers = parseArray(addresses); + writers = parseArray(addresses).map(address => ServerAddress.fromUrl(address)); } else if (role === 'READ') { - readers = parseArray(addresses); + readers = parseArray(addresses).map(address => ServerAddress.fromUrl(address)); } else { throw newError('Unknown server role "' + role + '"', PROTOCOL_ERROR); } diff --git a/src/v1/internal/server-address.js b/src/v1/internal/server-address.js new file mode 100644 index 000000000..13b509650 --- /dev/null +++ b/src/v1/internal/server-address.js @@ -0,0 +1,64 @@ +/** + * Copyright (c) 2002-2019 "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 { assertNumber, assertString } from './util'; +import urlUtil from './url-util'; + +export default class ServerAddress { + + constructor(host, resolved, port, hostPort) { + this._host = assertString(host, 'host'); + this._resolved = resolved ? assertString(resolved, 'resolved') : null; + this._port = assertNumber(port, 'port'); + this._hostPort = hostPort; + this._stringValue = resolved ? `${hostPort}[${resolved}]` : `${hostPort}`; + } + + host() { + return this._host; + } + + resolvedHost() { + return this._resolved ? this._resolved : this._host; + } + + port() { + return this._port; + } + + resolveWith(resolved) { + return new ServerAddress(this._host, resolved, this._port, this._hostPort); + } + + asHostPort() { + return this._hostPort; + } + + asKey() { + return this._hostPort; + } + + toString() { + return this._stringValue; + } + + static fromUrl(url) { + const urlParsed = urlUtil.parseDatabaseUrl(url); + return new ServerAddress(urlParsed.host, null, urlParsed.port, urlParsed.hostAndPort); + } +} diff --git a/src/v1/routing-driver.js b/src/v1/routing-driver.js index 8147fd887..265ba5d39 100644 --- a/src/v1/routing-driver.js +++ b/src/v1/routing-driver.js @@ -23,8 +23,7 @@ import {LoadBalancer} from './internal/connection-providers'; import LeastConnectedLoadBalancingStrategy, {LEAST_CONNECTED_STRATEGY_NAME} from './internal/least-connected-load-balancing-strategy'; import RoundRobinLoadBalancingStrategy, {ROUND_ROBIN_STRATEGY_NAME} from './internal/round-robin-load-balancing-strategy'; import ConnectionErrorHandler from './internal/connection-error-handler'; -import ConfiguredHostNameResolver from './internal/resolver/configured-host-name-resolver'; -import {HostNameResolver} from './internal/node'; +import ConfiguredCustomResolver from './internal/resolver/configured-custom-resolver'; /** * A driver that supports routing in a causal cluster. @@ -32,39 +31,39 @@ import {HostNameResolver} from './internal/node'; */ class RoutingDriver extends Driver { - constructor(hostPort, routingContext, userAgent, token = {}, config = {}) { - super(hostPort, userAgent, token, validateConfig(config)); + constructor(address, routingContext, userAgent, token = {}, config = {}) { + super(address, userAgent, token, validateConfig(config)); this._routingContext = routingContext; } _afterConstruction() { - this._log.info(`Routing driver ${this._id} created for server address ${this._hostPort}`); + this._log.info(`Routing driver ${this._id} created for server address ${this._address}`); } - _createConnectionProvider(hostPort, connectionPool, driverOnErrorCallback) { + _createConnectionProvider(address, connectionPool, driverOnErrorCallback) { const loadBalancingStrategy = RoutingDriver._createLoadBalancingStrategy(this._config, connectionPool); const resolver = createHostNameResolver(this._config); - return new LoadBalancer(hostPort, this._routingContext, connectionPool, loadBalancingStrategy, resolver, driverOnErrorCallback, this._log); + return new LoadBalancer(address, this._routingContext, connectionPool, loadBalancingStrategy, resolver, driverOnErrorCallback, this._log); } _createConnectionErrorHandler() { // connection errors mean SERVICE_UNAVAILABLE for direct driver but for routing driver they should only // result in SESSION_EXPIRED because there might still exist other servers capable of serving the request return new ConnectionErrorHandler(SESSION_EXPIRED, - (error, hostPort) => this._handleUnavailability(error, hostPort), - (error, hostPort) => this._handleWriteFailure(error, hostPort)); + (error, address) => this._handleUnavailability(error, address), + (error, address) => this._handleWriteFailure(error, address)); } - _handleUnavailability(error, hostPort) { - this._log.warn(`Routing driver ${this._id} will forget ${hostPort} because of an error ${error.code} '${error.message}'`); - this._connectionProvider.forget(hostPort); + _handleUnavailability(error, address) { + this._log.warn(`Routing driver ${this._id} will forget ${address} because of an error ${error.code} '${error.message}'`); + this._connectionProvider.forget(address); return error; } - _handleWriteFailure(error, hostPort) { - this._log.warn(`Routing driver ${this._id} will forget writer ${hostPort} because of an error ${error.code} '${error.message}'`); - this._connectionProvider.forgetWriter(hostPort); - return newError('No longer possible to write to server at ' + hostPort, SESSION_EXPIRED); + _handleWriteFailure(error, address) { + this._log.warn(`Routing driver ${this._id} will forget writer ${address} because of an error ${error.code} '${error.message}'`); + this._connectionProvider.forgetWriter(address); + return newError('No longer possible to write to server at ' + address, SESSION_EXPIRED); } /** @@ -88,13 +87,11 @@ class RoutingDriver extends Driver { /** * @private - * @returns {HostNameResolver} new resolver. + * @returns {ConfiguredCustomResolver} new custom resolver that wraps the passed-in resolver function. + * If resolved function is not specified, it defaults to an identity resolver. */ function createHostNameResolver(config) { - if (config.resolver) { - return new ConfiguredHostNameResolver(config.resolver); - } - return new HostNameResolver(); + return new ConfiguredCustomResolver(config.resolver); } /** diff --git a/test/internal/browser/browser-channel.test.js b/test/internal/browser/browser-channel.test.js index 78e40f770..a1168706f 100644 --- a/test/internal/browser/browser-channel.test.js +++ b/test/internal/browser/browser-channel.test.js @@ -23,6 +23,7 @@ import urlUtil from '../../../src/v1/internal/url-util'; import {Neo4jError, SERVICE_UNAVAILABLE} from '../../../src/v1/error'; import {setTimeoutMock} from '../timers-util'; import {ENCRYPTION_OFF, ENCRYPTION_ON} from '../../../src/v1/internal/util'; +import ServerAddress from '../../../src/v1/internal/server-address'; describe('WebSocketChannel', () => { @@ -71,9 +72,9 @@ describe('WebSocketChannel', () => { }; }; - const url = urlUtil.parseDatabaseUrl('bolt://localhost:7687'); + const address = ServerAddress.fromUrl('bolt://localhost:8989'); const driverConfig = {connectionTimeout: 4242}; - const channelConfig = new ChannelConfig(url, driverConfig, SERVICE_UNAVAILABLE); + const channelConfig = new ChannelConfig(address, driverConfig, SERVICE_UNAVAILABLE); webSocketChannel = new WebSocketChannel(channelConfig); @@ -125,9 +126,9 @@ describe('WebSocketChannel', () => { }; }; - const url = urlUtil.parseDatabaseUrl('bolt://localhost:8989'); + const address = ServerAddress.fromUrl('bolt://localhost:8989'); const driverConfig = {encrypted: true, trust: 'TRUST_ON_FIRST_USE'}; - const channelConfig = new ChannelConfig(url, driverConfig, SERVICE_UNAVAILABLE); + const channelConfig = new ChannelConfig(address, driverConfig, SERVICE_UNAVAILABLE); const channel = new WebSocketChannel(channelConfig, protocolSupplier); @@ -158,10 +159,10 @@ describe('WebSocketChannel', () => { }; }; - const url = urlUtil.parseDatabaseUrl(boltAddress); + const address = ServerAddress.fromUrl(boltAddress); // disable connection timeout, so that WebSocketChannel does not set any timeouts const driverConfig = {connectionTimeout: 0}; - const channelConfig = new ChannelConfig(url, driverConfig, SERVICE_UNAVAILABLE); + const channelConfig = new ChannelConfig(address, driverConfig, SERVICE_UNAVAILABLE); webSocketChannel = new WebSocketChannel(channelConfig); @@ -180,8 +181,8 @@ describe('WebSocketChannel', () => { }; }; - const url = urlUtil.parseDatabaseUrl('bolt://localhost:8989'); - const channelConfig = new ChannelConfig(url, driverConfig, SERVICE_UNAVAILABLE); + const address = ServerAddress.fromUrl('bolt://localhost:8989'); + const channelConfig = new ChannelConfig(address, driverConfig, SERVICE_UNAVAILABLE); const channel = new WebSocketChannel(channelConfig, protocolSupplier); expect(channel._ws.url).toEqual(expectedScheme + '://localhost:8989'); @@ -201,8 +202,8 @@ describe('WebSocketChannel', () => { const warnMessages = []; console.warn = message => warnMessages.push(message); - const url = urlUtil.parseDatabaseUrl('bolt://localhost:8989'); - const config = new ChannelConfig(url, {encrypted: encrypted}, SERVICE_UNAVAILABLE); + const address = ServerAddress.fromUrl('bolt://localhost:8989'); + const config = new ChannelConfig(address, {encrypted: encrypted}, SERVICE_UNAVAILABLE); const protocolSupplier = () => scheme + ':'; const channel = new WebSocketChannel(config, protocolSupplier); diff --git a/test/internal/channel-config.test.js b/test/internal/channel-config.test.js index d3684c951..c80f0f26c 100644 --- a/test/internal/channel-config.test.js +++ b/test/internal/channel-config.test.js @@ -21,17 +21,17 @@ import ChannelConfig from '../../src/v1/internal/channel-config'; import urlUtil from '../../src/v1/internal/url-util'; import {SERVICE_UNAVAILABLE} from '../../src/v1/error'; import {ENCRYPTION_OFF, ENCRYPTION_ON} from '../../src/v1/internal/util'; +import ServerAddress from '../../src/v1/internal/server-address'; describe('ChannelConfig', () => { it('should respect given Url', () => { - const url = urlUtil.parseDatabaseUrl('bolt://neo4j.com:4242'); + const address = ServerAddress.fromUrl('bolt://neo4j.com:4242'); - const config = new ChannelConfig(url, {}, ''); + const config = new ChannelConfig(address, {}, ''); - expect(config.url.scheme).toEqual('bolt'); - expect(config.url.host).toEqual('neo4j.com'); - expect(config.url.port).toEqual(4242); + expect(config.address.host()).toEqual('neo4j.com'); + expect(config.address.port()).toEqual(4242); }); it('should respect given encrypted conf', () => { diff --git a/test/internal/connection-error-handler.test.js b/test/internal/connection-error-handler.test.js index fbe236226..557e6e570 100644 --- a/test/internal/connection-error-handler.test.js +++ b/test/internal/connection-error-handler.test.js @@ -19,6 +19,7 @@ import ConnectionErrorHandler from '../../src/v1/internal/connection-error-handler'; import {newError, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../../src/v1/error'; +import ServerAddress from '../../src/v1/internal/server-address'; describe('ConnectionErrorHandler', () => { @@ -30,11 +31,11 @@ describe('ConnectionErrorHandler', () => { it('should handle and transform availability errors', () => { const errors = []; - const hostPorts = []; + const addresses = []; const transformedError = newError('Message', 'Code'); - const handler = new ConnectionErrorHandler(SERVICE_UNAVAILABLE, (error, hostPort) => { + const handler = new ConnectionErrorHandler(SERVICE_UNAVAILABLE, (error, address) => { errors.push(error); - hostPorts.push(hostPort); + addresses.push(address); return transformedError; }); @@ -43,21 +44,21 @@ describe('ConnectionErrorHandler', () => { const error3 = newError('C', 'Neo.TransientError.General.DatabaseUnavailable'); [error1, error2, error3].forEach((error, idx) => { - const newTransformedError = handler.handleAndTransformError(error, 'localhost:' + idx); + const newTransformedError = handler.handleAndTransformError(error, ServerAddress.fromUrl('localhost:' + idx)); expect(newTransformedError).toEqual(transformedError); }); expect(errors).toEqual([error1, error2, error3]); - expect(hostPorts).toEqual(['localhost:0', 'localhost:1', 'localhost:2']); + expect(addresses).toEqual([ServerAddress.fromUrl('localhost:0'), ServerAddress.fromUrl('localhost:1'), ServerAddress.fromUrl('localhost:2')]); }); it('should handle and transform failure to write errors', () => { const errors = []; - const hostPorts = []; + const addresses = []; const transformedError = newError('Message', 'Code'); - const handler = new ConnectionErrorHandler(SERVICE_UNAVAILABLE, null, (error, hostPort) => { + const handler = new ConnectionErrorHandler(SERVICE_UNAVAILABLE, null, (error, address) => { errors.push(error); - hostPorts.push(hostPort); + addresses.push(address); return transformedError; }); @@ -65,12 +66,12 @@ describe('ConnectionErrorHandler', () => { const error2 = newError('B', 'Neo.ClientError.General.ForbiddenOnReadOnlyDatabase'); [error1, error2].forEach((error, idx) => { - const newTransformedError = handler.handleAndTransformError(error, 'localhost:' + idx); + const newTransformedError = handler.handleAndTransformError(error, ServerAddress.fromUrl('localhost:' + idx)); expect(newTransformedError).toEqual(transformedError); }); expect(errors).toEqual([error1, error2]); - expect(hostPorts).toEqual(['localhost:0', 'localhost:1']); + expect(addresses).toEqual([ServerAddress.fromUrl('localhost:0'), ServerAddress.fromUrl('localhost:1')]); }); }); diff --git a/test/internal/connection-providers.test.js b/test/internal/connection-providers.test.js index ced161517..d8f6270f5 100644 --- a/test/internal/connection-providers.test.js +++ b/test/internal/connection-providers.test.js @@ -26,6 +26,7 @@ import Pool from '../../src/v1/internal/pool'; import LeastConnectedLoadBalancingStrategy from '../../src/v1/internal/least-connected-load-balancing-strategy'; import Logger from '../../src/v1/internal/logger'; import SimpleHostNameResolver from '../../src/v1/internal/browser/browser-host-name-resolver'; +import ServerAddress from '../../src/v1/internal/server-address'; const NO_OP_DRIVER_CALLBACK = () => { }; @@ -33,14 +34,15 @@ const NO_OP_DRIVER_CALLBACK = () => { describe('DirectConnectionProvider', () => { it('acquires connection from the pool', done => { + const address = ServerAddress.fromUrl('localhost:123'); const pool = newPool(); - const connectionProvider = newDirectConnectionProvider('localhost:123', pool); + const connectionProvider = newDirectConnectionProvider(address, pool); connectionProvider.acquireConnection(READ).then(connection => { expect(connection).toBeDefined(); - expect(connection.address).toEqual('localhost:123'); + expect(connection.address).toEqual(address); expect(connection.release).toBeDefined(); - expect(pool.has('localhost:123')).toBeTruthy(); + expect(pool.has(address)).toBeTruthy(); done(); }); @@ -49,101 +51,130 @@ describe('DirectConnectionProvider', () => { }); describe('LoadBalancer', () => { + const server0 = ServerAddress.fromUrl('server0'); + const server1 = ServerAddress.fromUrl('server1'); + const server2 = ServerAddress.fromUrl('server2'); + const server3 = ServerAddress.fromUrl('server3'); + const server4 = ServerAddress.fromUrl('server4'); + const server5 = ServerAddress.fromUrl('server5'); + const server6 = ServerAddress.fromUrl('server6'); + const server7 = ServerAddress.fromUrl('server7'); + const server42 = ServerAddress.fromUrl('server42'); + + const server01 = ServerAddress.fromUrl('server01'); + const server02 = ServerAddress.fromUrl('server02'); + const server03 = ServerAddress.fromUrl('server03'); + + const serverA = ServerAddress.fromUrl('serverA'); + const serverB = ServerAddress.fromUrl('serverB'); + const serverC = ServerAddress.fromUrl('serverC'); + const serverD = ServerAddress.fromUrl('serverD'); + const serverE = ServerAddress.fromUrl('serverE'); + const serverF = ServerAddress.fromUrl('serverF'); + const serverG = ServerAddress.fromUrl('serverG'); + + const serverAA = ServerAddress.fromUrl('serverAA'); + const serverBB = ServerAddress.fromUrl('serverBB'); + const serverCC = ServerAddress.fromUrl('serverCC'); + const serverDD = ServerAddress.fromUrl('serverDD'); + const serverEE = ServerAddress.fromUrl('serverEE'); + + const serverABC = ServerAddress.fromUrl('serverABC'); it('can forget address', () => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-2'], - ['server-2', 'server-4'] + [server1, server2], + [server3, server2], + [server2, server4] ); - loadBalancer.forget('server-2'); + loadBalancer.forget(server2); expectRoutingTable(loadBalancer, - ['server-1', 'server-2'], - ['server-3'], - ['server-4'] + [server1, server2], + [server3], + [server4] ); }); it('can not forget unknown address', () => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'] + [server1, server2], + [server3, server4], + [server5, server6] ); - loadBalancer.forget('server-42'); + loadBalancer.forget(server42); expectRoutingTable(loadBalancer, - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'] + [server1, server2], + [server3, server4], + [server5, server6] ); }); it('purges connections when address is forgotten', () => { const pool = newPool(); - pool.acquire('server-1'); - pool.acquire('server-3'); - pool.acquire('server-5'); - expectPoolToContain(pool, ['server-1', 'server-3', 'server-5']); + pool.acquire(server1); + pool.acquire(server3); + pool.acquire(server5); + expectPoolToContain(pool, [server1, server3, server5]); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-2'], - ['server-2', 'server-4'], + [server1, server2], + [server3, server2], + [server2, server4], pool ); - loadBalancer.forget('server-1'); - loadBalancer.forget('server-5'); + loadBalancer.forget(server1); + loadBalancer.forget(server5); - expectPoolToContain(pool, ['server-3']); - expectPoolToNotContain(pool, ['server-1', 'server-5']); + expectPoolToContain(pool, [server3]); + expectPoolToNotContain(pool, [server1, server5]); }); it('can forget writer address', () => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-2'], - ['server-2', 'server-4'] + [server1, server2], + [server3, server2], + [server2, server4] ); - loadBalancer.forgetWriter('server-2'); + loadBalancer.forgetWriter(server2); expectRoutingTable(loadBalancer, - ['server-1', 'server-2'], - ['server-3', 'server-2'], - ['server-4'] + [server1, server2], + [server3, server2], + [server4] ); }); it('can not forget unknown writer address', () => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'] + [server1, server2], + [server3, server4], + [server5, server6] ); - loadBalancer.forgetWriter('server-42'); + loadBalancer.forgetWriter(server42); expectRoutingTable(loadBalancer, - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'] + [server1, server2], + [server3, server4], + [server5, server6] ); }); it('initializes routing table with the given router', () => { const connectionPool = newPool(); const loadBalancingStrategy = new LeastConnectedLoadBalancingStrategy(connectionPool); - const loadBalancer = new LoadBalancer('server-ABC', {}, connectionPool, loadBalancingStrategy, new SimpleHostNameResolver(), + const loadBalancer = new LoadBalancer(serverABC, {}, connectionPool, loadBalancingStrategy, new SimpleHostNameResolver(), NO_OP_DRIVER_CALLBACK, Logger.noOp()); expectRoutingTable(loadBalancer, - ['server-ABC'], + [serverABC], [], [] ); @@ -152,19 +183,19 @@ describe('LoadBalancer', () => { it('acquires read connection with up-to-date routing table', done => { const pool = newPool(); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool ); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-3'); - expect(pool.has('server-3')).toBeTruthy(); + expect(connection.address).toEqual(server3); + expect(pool.has(server3)).toBeTruthy(); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-4'); - expect(pool.has('server-4')).toBeTruthy(); + expect(connection.address).toEqual(server4); + expect(pool.has(server4)).toBeTruthy(); done(); }); @@ -174,19 +205,19 @@ describe('LoadBalancer', () => { it('acquires write connection with up-to-date routing table', done => { const pool = newPool(); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool ); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-5'); - expect(pool.has('server-5')).toBeTruthy(); + expect(connection.address).toEqual(server5); + expect(pool.has(server5)).toBeTruthy(); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-6'); - expect(pool.has('server-6')).toBeTruthy(); + expect(connection.address).toEqual(server6); + expect(pool.has(server6)).toBeTruthy(); done(); }); @@ -195,9 +226,9 @@ describe('LoadBalancer', () => { it('throws for illegal access mode', done => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'] + [server1, server2], + [server3, server4], + [server5, server6] ); loadBalancer.acquireConnection('WRONG').catch(error => { @@ -209,26 +240,26 @@ describe('LoadBalancer', () => { it('refreshes stale routing table to get read connection', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table - {'server-1': updatedRoutingTable} + { 'server1:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-C'); - expect(pool.has('server-C')).toBeTruthy(); + expect(connection.address).toEqual(serverC); + expect(pool.has(serverC)).toBeTruthy(); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-D'); - expect(pool.has('server-D')).toBeTruthy(); + expect(connection.address).toEqual(serverD); + expect(pool.has(serverD)).toBeTruthy(); done(); }); @@ -238,26 +269,26 @@ describe('LoadBalancer', () => { it('refreshes stale routing table to get write connection', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table - {'server-1': updatedRoutingTable} + { 'server1:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-E'); - expect(pool.has('server-E')).toBeTruthy(); + expect(connection.address).toEqual(serverE); + expect(pool.has(serverE)).toBeTruthy(); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-F'); - expect(pool.has('server-F')).toBeTruthy(); + expect(connection.address).toEqual(serverF); + expect(pool.has(serverF)).toBeTruthy(); done(); }); @@ -267,29 +298,29 @@ describe('LoadBalancer', () => { it('refreshes stale routing table to get read connection when one router fails', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': updatedRoutingTable, + 'server1:7687': null, // returns no routing table + 'server2:7687': updatedRoutingTable, } ); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-C'); - expect(pool.has('server-C')).toBeTruthy(); + expect(connection.address).toEqual(serverC); + expect(pool.has(serverC)).toBeTruthy(); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-D'); - expect(pool.has('server-D')).toBeTruthy(); + expect(connection.address).toEqual(serverD); + expect(pool.has(serverD)).toBeTruthy(); done(); }); @@ -299,29 +330,29 @@ describe('LoadBalancer', () => { it('refreshes stale routing table to get write connection when one router fails', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': updatedRoutingTable, + 'server1:7687': null, // returns no routing table + 'server2:7687': updatedRoutingTable, } ); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-E'); - expect(pool.has('server-E')).toBeTruthy(); + expect(connection.address).toEqual(serverE); + expect(pool.has(serverE)).toBeTruthy(); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-F'); - expect(pool.has('server-F')).toBeTruthy(); + expect(connection.address).toEqual(serverF); + expect(pool.has(serverF)).toBeTruthy(); done(); }); @@ -331,29 +362,29 @@ describe('LoadBalancer', () => { it('refreshes routing table without readers to get read connection', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], + [server1, server2], [], // no readers - ['server-3', 'server-4'], + [server3, server4], pool, Integer.MAX_VALUE, { - 'server-1': null, // returns no routing table - 'server-2': updatedRoutingTable, + 'server1:7687': null, // returns no routing table + 'server2:7687': updatedRoutingTable, } ); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-C'); - expect(pool.has('server-C')).toBeTruthy(); + expect(connection.address).toEqual(serverC); + expect(pool.has(serverC)).toBeTruthy(); loadBalancer.acquireConnection(READ).then(connection => { - expect(connection.address).toEqual('server-D'); - expect(pool.has('server-D')).toBeTruthy(); + expect(connection.address).toEqual(serverD); + expect(pool.has(serverD)).toBeTruthy(); done(); }); @@ -363,29 +394,29 @@ describe('LoadBalancer', () => { it('refreshes routing table without writers to get write connection', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], + [server1, server2], + [server3, server4], [], // no writers pool, int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': updatedRoutingTable, + 'server1:7687': null, // returns no routing table + 'server2:7687': updatedRoutingTable, } ); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-E'); - expect(pool.has('server-E')).toBeTruthy(); + expect(connection.address).toEqual(serverE); + expect(pool.has(serverE)).toBeTruthy(); loadBalancer.acquireConnection(WRITE).then(connection => { - expect(connection.address).toEqual('server-F'); - expect(pool.has('server-F')).toBeTruthy(); + expect(connection.address).toEqual(serverF); + expect(pool.has(serverF)).toBeTruthy(); done(); }); @@ -394,14 +425,14 @@ describe('LoadBalancer', () => { it('throws when all routers return nothing while getting read connection', done => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], newPool(), int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null // returns no routing table + 'server1:7687': null, // returns no routing table + 'server2:7687': null // returns no routing table } ); @@ -413,14 +444,14 @@ describe('LoadBalancer', () => { it('throws when all routers return nothing while getting write connection', done => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], newPool(), int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null // returns no routing table + 'server1:7687': null, // returns no routing table + 'server2:7687': null // returns no routing table } ); @@ -432,19 +463,19 @@ describe('LoadBalancer', () => { it('throws when all routers return routing tables without readers while getting read connection', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], + [serverA, serverB], [], // no readers - table can't satisfy connection requirement - ['server-C', 'server-D'] + [serverC, serverD] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], newPool(), int(0), // expired routing table { - 'server-1': updatedRoutingTable, - 'server-2': updatedRoutingTable + 'server1:7687': updatedRoutingTable, + 'server2:7687': updatedRoutingTable } ); @@ -456,19 +487,19 @@ describe('LoadBalancer', () => { it('throws when all routers return routing tables without writers while getting write connection', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], + [serverA, serverB], + [serverC, serverD], [] // no writers - table can't satisfy connection requirement ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], newPool(), int(0), // expired routing table { - 'server-1': updatedRoutingTable, - 'server-2': updatedRoutingTable + 'server1:7687': updatedRoutingTable, + 'server2:7687': updatedRoutingTable } ); @@ -481,8 +512,8 @@ describe('LoadBalancer', () => { it('throws when stale routing table without routers while getting read connection', done => { const loadBalancer = newLoadBalancer( [], // no routers - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server3, server4], + [server5, server6], newPool(), int(0) // expired routing table ); @@ -496,8 +527,8 @@ describe('LoadBalancer', () => { it('throws when stale routing table without routers while getting write connection', done => { const loadBalancer = newLoadBalancer( [], // no routers - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server3, server4], + [server5, server6], newPool(), int(0) // expired routing table ); @@ -511,37 +542,37 @@ describe('LoadBalancer', () => { it('updates routing table after refresh', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table { - 'server-1': updatedRoutingTable + 'server1:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(READ).then(() => { expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); - expectPoolToNotContain(pool, ['server-1', 'server-2', 'server-3', 'server-4', 'server-5', 'server-6']); + expectPoolToNotContain(pool, [server1, server2, server3, server4, server5, server6]); done(); }); }); it('forgets all routers when they fail while acquiring read connection', done => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2', 'server-3'], - ['server-4', 'server-5'], - ['server-6', 'server-7'], + [server1, server2, server3], + [server4, server5], + [server6, server7], newPool(), int(0) // expired routing table ); @@ -550,8 +581,8 @@ describe('LoadBalancer', () => { expect(error.code).toEqual(SERVICE_UNAVAILABLE); expectRoutingTable(loadBalancer, [], - ['server-4', 'server-5'], - ['server-6', 'server-7'] + [server4, server5], + [server6, server7] ); done(); }); @@ -559,9 +590,9 @@ describe('LoadBalancer', () => { it('forgets all routers when they fail while acquiring write connection', done => { const loadBalancer = newLoadBalancer( - ['server-1', 'server-2', 'server-3'], - ['server-4', 'server-5'], - ['server-6', 'server-7'], + [server1, server2, server3], + [server4, server5], + [server6, server7], newPool(), int(0) // expired routing table ); @@ -570,8 +601,8 @@ describe('LoadBalancer', () => { expect(error.code).toEqual(SERVICE_UNAVAILABLE); expectRoutingTable(loadBalancer, [], - ['server-4', 'server-5'], - ['server-6', 'server-7'] + [server4, server5], + [server6, server7] ); done(); }); @@ -579,35 +610,35 @@ describe('LoadBalancer', () => { it('uses seed router address when all existing routers fail', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B', 'server-C'], - ['server-D', 'server-E'], - ['server-F', 'server-G'] + [serverA, serverB, serverC], + [serverD, serverE], + [serverF, serverG] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-0'], // seed router address resolves just to itself - ['server-1', 'server-2', 'server-3'], - ['server-4', 'server-5'], - ['server-6', 'server-7'], + server0, [server0], // seed router address resolves just to itself + [server1, server2, server3], + [server4, server5], + [server6, server7], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null, // returns no routing table - 'server-3': null, // returns no routing table - 'server-0': updatedRoutingTable + 'server1:7687': null, // returns no routing table + 'server2:7687': null, // returns no routing table + 'server3:7687': null, // returns no routing table + 'server0:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(READ).then(connection1 => { - expect(connection1.address).toEqual('server-D'); + expect(connection1.address).toEqual(serverD); loadBalancer.acquireConnection(WRITE).then(connection2 => { - expect(connection2.address).toEqual('server-F'); + expect(connection2.address).toEqual(serverF); expectRoutingTable(loadBalancer, - ['server-A', 'server-B', 'server-C'], - ['server-D', 'server-E'], - ['server-F', 'server-G'] + [serverA, serverB, serverC], + [serverD, serverE], + [serverF, serverG] ); done(); }); @@ -616,35 +647,35 @@ describe('LoadBalancer', () => { it('uses resolved seed router address when all existing routers fail', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-01'], // seed router address resolves to a different one - ['server-1', 'server-2', 'server-3'], - ['server-4', 'server-5'], - ['server-6', 'server-7'], + server0, [server01], // seed router address resolves to a different one + [server1, server2, server3], + [server4, server5], + [server6, server7], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null, // returns no routing table - 'server-3': null, // returns no routing table - 'server-01': updatedRoutingTable + 'server1:7687': null, // returns no routing table + 'server2:7687': null, // returns no routing table + 'server3:7687': null, // returns no routing table + 'server01:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(WRITE).then(connection1 => { - expect(connection1.address).toEqual('server-E'); + expect(connection1.address).toEqual(serverE); loadBalancer.acquireConnection(READ).then(connection2 => { - expect(connection2.address).toEqual('server-C'); + expect(connection2.address).toEqual(serverC); expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); done(); }); @@ -653,35 +684,35 @@ describe('LoadBalancer', () => { it('uses resolved seed router address that returns correct routing table when all existing routers fail', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C'], - ['server-D', 'server-E'] + [serverA, serverB], + [serverC], + [serverD, serverE] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-01', 'server-02', 'server-03'], // seed router address resolves to 3 different addresses - ['server-1'], - ['server-2'], - ['server-3'], + server0, [server01, server02, server03], // seed router address resolves to 3 different addresses + [server1], + [server2], + [server3], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-01': null, // returns no routing table - 'server-02': null, // returns no routing table - 'server-03': updatedRoutingTable + 'server1:7687': null, // returns no routing table + 'server01:7687': null, // returns no routing table + 'server02:7687': null, // returns no routing table + 'server03:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(WRITE).then(connection1 => { - expect(connection1.address).toEqual('server-D'); + expect(connection1.address).toEqual(serverD); loadBalancer.acquireConnection(WRITE).then(connection2 => { - expect(connection2.address).toEqual('server-E'); + expect(connection2.address).toEqual(serverE); expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C'], - ['server-D', 'server-E'] + [serverA, serverB], + [serverC], + [serverD, serverE] ); done(); }); @@ -690,16 +721,16 @@ describe('LoadBalancer', () => { it('fails when both existing routers and seed router fail to return a routing table', done => { const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-0'], // seed router address resolves just to itself - ['server-1', 'server-2', 'server-3'], - ['server-4', 'server-5'], - ['server-6'], + server0, [server0], // seed router address resolves just to itself + [server1, server2, server3], + [server4, server5], + [server6], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null, // returns no routing table - 'server-3': null, // returns no routing table - 'server-0': null // returns no routing table + 'server1:7687': null, // returns no routing table + 'server2:7687': null, // returns no routing table + 'server3:7687': null, // returns no routing table + 'server0:7687': null // returns no routing table } ); @@ -708,8 +739,8 @@ describe('LoadBalancer', () => { expectRoutingTable(loadBalancer, [], // all routers were forgotten because they failed - ['server-4', 'server-5'], - ['server-6'], + [server4, server5], + [server6], ); loadBalancer.acquireConnection(WRITE).catch(error => { @@ -717,8 +748,8 @@ describe('LoadBalancer', () => { expectRoutingTable(loadBalancer, [], // all routers were forgotten because they failed - ['server-4', 'server-5'], - ['server-6'], + [server4, server5], + [server6], ); done(); @@ -728,15 +759,15 @@ describe('LoadBalancer', () => { it('fails when both existing routers and resolved seed router fail to return a routing table', done => { const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-01'], // seed router address resolves to a different one - ['server-1', 'server-2'], - ['server-3'], - ['server-4'], + server0, [server01], // seed router address resolves to a different one + [server1, server2], + [server3], + [server4], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null, // returns no routing table - 'server-01': null // returns no routing table + 'server1:7687': null, // returns no routing table + 'server2:7687': null, // returns no routing table + 'server01:7687': null // returns no routing table } ); @@ -745,8 +776,8 @@ describe('LoadBalancer', () => { expectRoutingTable(loadBalancer, [], // all routers were forgotten because they failed - ['server-3'], - ['server-4'], + [server3], + [server4], ); loadBalancer.acquireConnection(READ).catch(error => { @@ -754,8 +785,8 @@ describe('LoadBalancer', () => { expectRoutingTable(loadBalancer, [], // all routers were forgotten because they failed - ['server-3'], - ['server-4'], + [server3], + [server4], ); done(); @@ -765,17 +796,17 @@ describe('LoadBalancer', () => { it('fails when both existing routers and all resolved seed routers fail to return a routing table', done => { const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-02', 'server-01'], // seed router address resolves to 2 different addresses - ['server-1', 'server-2', 'server-3'], - ['server-4'], - ['server-5'], + server0, [server02, server01], // seed router address resolves to 2 different addresses + [server1, server2, server3], + [server4], + [server5], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-2': null, // returns no routing table - 'server-3': null, // returns no routing table - 'server-01': null, // returns no routing table - 'server-02': null // returns no routing table + 'server1:7687': null, // returns no routing table + 'server2:7687': null, // returns no routing table + 'server3:7687': null, // returns no routing table + 'server01:7687': null, // returns no routing table + 'server02:7687': null // returns no routing table } ); @@ -784,8 +815,8 @@ describe('LoadBalancer', () => { expectRoutingTable(loadBalancer, [], // all known seed servers failed to return routing tables and were forgotten - ['server-4'], - ['server-5'], + [server4], + [server5], ); loadBalancer.acquireConnection(WRITE).catch(error => { @@ -793,8 +824,8 @@ describe('LoadBalancer', () => { expectRoutingTable(loadBalancer, [], // all known seed servers failed to return routing tables and were forgotten - ['server-4'], - ['server-5'], + [server4], + [server5], ); done(); @@ -804,32 +835,32 @@ describe('LoadBalancer', () => { it('uses seed router when no existing routers', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C'], - ['server-D'] + [serverA, serverB], + [serverC], + [serverD] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-0'], // seed router address resolves just to itself + server0, [server0], // seed router address resolves just to itself [], // no routers in the known routing table - ['server-1', 'server-2'], - ['server-3'], + [server1, server2], + [server3], Integer.MAX_VALUE, // not expired { - 'server-0': updatedRoutingTable + 'server0:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(WRITE).then(connection1 => { - expect(connection1.address).toEqual('server-D'); + expect(connection1.address).toEqual(serverD); loadBalancer.acquireConnection(READ).then(connection2 => { - expect(connection2.address).toEqual('server-C'); + expect(connection2.address).toEqual(serverC); expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C'], - ['server-D'] + [serverA, serverB], + [serverC], + [serverD] ); done(); }); @@ -838,32 +869,32 @@ describe('LoadBalancer', () => { it('uses resolved seed router when no existing routers', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-F', 'server-E'] + [serverA, serverB], + [serverC, serverD], + [serverF, serverE] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-01'], // seed router address resolves to a different one + server0, [server01], // seed router address resolves to a different one [], // no routers in the known routing table - ['server-1', 'server-2'], - ['server-3', 'server-4'], + [server1, server2], + [server3, server4], Integer.MAX_VALUE, // not expired { - 'server-01': updatedRoutingTable + 'server01:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(READ).then(connection1 => { - expect(connection1.address).toEqual('server-C'); + expect(connection1.address).toEqual(serverC); loadBalancer.acquireConnection(WRITE).then(connection2 => { - expect(connection2.address).toEqual('server-F'); + expect(connection2.address).toEqual(serverF); expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-F', 'server-E'] + [serverA, serverB], + [serverC, serverD], + [serverF, serverE] ); done(); }); @@ -872,34 +903,34 @@ describe('LoadBalancer', () => { it('uses resolved seed router that returns routing table when no existing routers exist', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B', 'server-C'], - ['server-D', 'server-E'], - ['server-F'] + [serverA, serverB, serverC], + [serverD, serverE], + [serverF] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-02', 'server-01', 'server-03'], // seed router address resolves to 3 different addresses + server0, [server02, server01, server03], // seed router address resolves to 3 different addresses [], // no routers in the known routing table - ['server-1'], - ['server-2', 'server-3'], + [server1], + [server2, server3], Integer.MAX_VALUE, // not expired { - 'server-01': null, // returns no routing table - 'server-02': null, // returns no routing table - 'server-03': updatedRoutingTable + 'server01:7687': null, // returns no routing table + 'server02:7687': null, // returns no routing table + 'server03:7687': updatedRoutingTable } ); loadBalancer.acquireConnection(WRITE).then(connection1 => { - expect(connection1.address).toEqual('server-F'); + expect(connection1.address).toEqual(serverF); loadBalancer.acquireConnection(READ).then(connection2 => { - expect(connection2.address).toEqual('server-D'); + expect(connection2.address).toEqual(serverD); expectRoutingTable(loadBalancer, - ['server-A', 'server-B', 'server-C'], - ['server-D', 'server-E'], - ['server-F'] + [serverA, serverB, serverC], + [serverD, serverE], + [serverF] ); done(); }); @@ -908,44 +939,44 @@ describe('LoadBalancer', () => { it('ignores already probed routers after seed router resolution', done => { const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-1', 'server-01', 'server-2', 'server-02'], // seed router address resolves to 4 different addresses - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + server0, [server1, server01, server2, server02], // seed router address resolves to 4 different addresses + [server1, server2], + [server3, server4], + [server5, server6], int(0), // expired routing table { - 'server-1': null, // returns no routing table - 'server-01': null, // returns no routing table - 'server-2': null, // returns no routing table - 'server-02': updatedRoutingTable + 'server1:7687': null, // returns no routing table + 'server01:7687': null, // returns no routing table + 'server2:7687': null, // returns no routing table + 'server02:7687': updatedRoutingTable } ); const usedRouterArrays = []; setupLoadBalancerToRememberRouters(loadBalancer, usedRouterArrays); loadBalancer.acquireConnection(READ).then(connection1 => { - expect(connection1.address).toEqual('server-C'); + expect(connection1.address).toEqual(serverC); loadBalancer.acquireConnection(WRITE).then(connection2 => { - expect(connection2.address).toEqual('server-E'); + expect(connection2.address).toEqual(serverE); // two sets of routers probed: - // 1) existing routers 'server-1' & 'server-2' - // 2) resolved routers 'server-01' & 'server-02' + // 1) existing routers server1 & server2 + // 2) resolved routers server01 & server02 expect(usedRouterArrays.length).toEqual(2); - expect(usedRouterArrays[0]).toEqual(['server-1', 'server-2']); - expect(usedRouterArrays[1]).toEqual(['server-01', 'server-02']); + expect(usedRouterArrays[0]).toEqual([server1, server2]); + expect(usedRouterArrays[1]).toEqual([server01, server02]); expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C', 'server-D'], - ['server-E', 'server-F'] + [serverA, serverB], + [serverC, serverD], + [serverE, serverF] ); done(); }); @@ -955,18 +986,18 @@ describe('LoadBalancer', () => { it('throws session expired when refreshed routing table has no readers', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], + [serverA, serverB], [], // no readers - ['server-C', 'server-D'] + [serverC, serverD] ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table { - 'server-1': updatedRoutingTable, + 'server1:7687': updatedRoutingTable, } ); @@ -979,18 +1010,18 @@ describe('LoadBalancer', () => { it('throws session expired when refreshed routing table has no writers', done => { const pool = newPool(); const updatedRoutingTable = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], + [serverA, serverB], + [serverC, serverD], [] // no writers ); const loadBalancer = newLoadBalancer( - ['server-1', 'server-2'], - ['server-3', 'server-4'], - ['server-5', 'server-6'], + [server1, server2], + [server3, server4], + [server5, server6], pool, int(0), // expired routing table { - 'server-1': updatedRoutingTable, + 'server1:7687': updatedRoutingTable, } ); @@ -1002,50 +1033,50 @@ describe('LoadBalancer', () => { it('should use resolved seed router after accepting table with no writers', done => { const routingTable1 = newRoutingTable( - ['server-A', 'server-B'], - ['server-C', 'server-D'], + [serverA, serverB], + [serverC, serverD], [] // no writers ); const routingTable2 = newRoutingTable( - ['server-AA', 'server-BB'], - ['server-CC', 'server-DD'], - ['server-EE'] + [serverAA, serverBB], + [serverCC, serverDD], + [serverEE] ); const loadBalancer = newLoadBalancerWithSeedRouter( - 'server-0', ['server-02', 'server-01'], // seed router address resolves to 2 different addresses - ['server-1'], - ['server-2', 'server-3'], - ['server-4', 'server-5'], + server0, [server02, server01], // seed router address resolves to 2 different addresses + [server1], + [server2, server3], + [server4, server5], int(0), // expired routing table { - 'server-1': routingTable1, - 'server-A': routingTable1, - 'server-B': routingTable1, - 'server-01': null, // returns no routing table - 'server-02': routingTable2 + 'server1:7687': routingTable1, + 'serverA:7687': routingTable1, + 'serverB:7687': routingTable1, + 'server01:7687': null, // returns no routing table + 'server02:7687': routingTable2 } ); loadBalancer.acquireConnection(READ).then(connection1 => { - expect(connection1.address).toEqual('server-C'); + expect(connection1.address).toEqual(serverC); loadBalancer.acquireConnection(READ).then(connection2 => { - expect(connection2.address).toEqual('server-D'); + expect(connection2.address).toEqual(serverD); expectRoutingTable(loadBalancer, - ['server-A', 'server-B'], - ['server-C', 'server-D'], + [serverA, serverB], + [serverC, serverD], [] ); loadBalancer.acquireConnection(WRITE).then(connection3 => { - expect(connection3.address).toEqual('server-EE'); + expect(connection3.address).toEqual(serverEE); expectRoutingTable(loadBalancer, - ['server-AA', 'server-BB'], - ['server-CC', 'server-DD'], - ['server-EE'] + [serverAA, serverBB], + [serverCC, serverDD], + [serverEE] ); done(); @@ -1064,7 +1095,7 @@ function newLoadBalancer(routers, readers, writers, pool = null, expirationTime = Integer.MAX_VALUE, routerToRoutingTable = {}) { - const seedRouter = 'server-non-existing-seed-router'; + const seedRouter = ServerAddress.fromUrl('server-non-existing-seed-router'); return newLoadBalancerWithSeedRouter(seedRouter, [seedRouter], routers, readers, writers, expirationTime, routerToRoutingTable, pool); } @@ -1134,7 +1165,7 @@ class FakeRediscovery { } lookupRoutingTableOnRouter(ignored, router) { - return this._routerToRoutingTable[router]; + return this._routerToRoutingTable[router.asKey()]; } } diff --git a/test/internal/connection.test.js b/test/internal/connection.test.js index 570b63a6d..1fbbce15d 100644 --- a/test/internal/connection.test.js +++ b/test/internal/connection.test.js @@ -33,6 +33,7 @@ import testUtils from '../internal/test-utils'; import Bookmark from '../../src/v1/internal/bookmark'; import TxConfig from '../../src/v1/internal/tx-config'; import {WRITE} from "../../src/v1/driver"; +import ServerAddress from '../../src/v1/internal/server-address'; const ILLEGAL_MESSAGE = {signature: 42, fields: []}; const SUCCESS_MESSAGE = {signature: 0x70, fields: [{}]}; @@ -105,7 +106,7 @@ describe('Connection', () => { it('should write protocol handshake', () => { const channel = new DummyChannel(); - connection = new Connection(channel, new ConnectionErrorHandler(SERVICE_UNAVAILABLE), 'localhost:7687', Logger.noOp()); + connection = new Connection(channel, new ConnectionErrorHandler(SERVICE_UNAVAILABLE), ServerAddress.fromUrl('localhost:7687'), Logger.noOp()); connection._negotiateProtocol(); @@ -136,7 +137,7 @@ describe('Connection', () => { it('should convert failure messages to errors', done => { const channel = new DummyChannel(); - connection = new Connection(channel, new ConnectionErrorHandler(SERVICE_UNAVAILABLE), 'localhost:7687', Logger.noOp()); + connection = new Connection(channel, new ConnectionErrorHandler(SERVICE_UNAVAILABLE), ServerAddress.fromUrl('localhost:7687'), Logger.noOp()); connection._negotiateProtocol(); @@ -324,22 +325,22 @@ describe('Connection', () => { it('should handle and transform fatal errors', done => { const errors = []; - const hostPorts = []; + const addresses = []; const transformedError = newError('Message', 'Code'); - const errorHandler = new ConnectionErrorHandler(SERVICE_UNAVAILABLE, (error, hostPort) => { + const errorHandler = new ConnectionErrorHandler(SERVICE_UNAVAILABLE, (error, address) => { errors.push(error); - hostPorts.push(hostPort); + addresses.push(address); return transformedError; }); - connection = Connection.create('bolt://localhost', {}, errorHandler, Logger.noOp()); + connection = Connection.create(ServerAddress.fromUrl('bolt://localhost'), {}, errorHandler, Logger.noOp()); connection._queueObserver({ onError: error => { expect(error).toEqual(transformedError); expect(errors.length).toEqual(1); expect(errors[0].code).toEqual(SERVICE_UNAVAILABLE); - expect(hostPorts).toEqual([connection.hostPort]); + expect(addresses).toEqual([connection.address]); done(); } }); @@ -462,7 +463,7 @@ describe('Connection', () => { * @return {Connection} */ function createConnection(url, config, errorCode = null) { - return Connection.create(url, config || {}, new ConnectionErrorHandler(errorCode || SERVICE_UNAVAILABLE), Logger.noOp()); + return Connection.create(ServerAddress.fromUrl(url), config || {}, new ConnectionErrorHandler(errorCode || SERVICE_UNAVAILABLE), Logger.noOp()); } function recordWrittenMessages(connection, messages) { diff --git a/test/internal/node/node-host-name-resolver.test.js b/test/internal/node/node-host-name-resolver.test.js index 0f3d5760d..94c26f9f3 100644 --- a/test/internal/node/node-host-name-resolver.test.js +++ b/test/internal/node/node-host-name-resolver.test.js @@ -19,6 +19,7 @@ import NodeHostNameResolver from '../../../src/v1/internal/node/node-host-name-resolver'; import urlUtil from '../../../src/v1/internal/url-util'; +import ServerAddress from '../../../src/v1/internal/server-address'; describe('NodeHostNameResolver', () => { @@ -35,7 +36,7 @@ describe('NodeHostNameResolver', () => { }); it('should resolve address', done => { - const seedRouter = 'neo4j.com'; + const seedRouter = ServerAddress.fromUrl('neo4j.com'); const resolver = new NodeHostNameResolver(); resolver.resolve(seedRouter).then(addresses => { @@ -43,10 +44,9 @@ describe('NodeHostNameResolver', () => { addresses.forEach(address => { expectToBeDefined(address); - const parsedUrl = urlUtil.parseDatabaseUrl(address); - expect(parsedUrl.scheme).toBeNull(); - expectToBeDefined(parsedUrl.host); - expect(parsedUrl.port).toEqual(7687); // default port should be appended + expect(address.host()).toEqual('neo4j.com'); + expect(address.resolvedHost()).not.toEqual('neo4j.com'); + expect(address.port()).toEqual(7687); // default port should be appended }); done(); @@ -54,7 +54,7 @@ describe('NodeHostNameResolver', () => { }); it('should resolve address with port', done => { - const seedRouter = 'neo4j.com:7474'; + const seedRouter = ServerAddress.fromUrl('neo4j.com:7474'); const resolver = new NodeHostNameResolver(); resolver.resolve(seedRouter).then(addresses => { @@ -62,10 +62,9 @@ describe('NodeHostNameResolver', () => { addresses.forEach(address => { expectToBeDefined(address); - const parsedUrl = urlUtil.parseDatabaseUrl(address); - expect(parsedUrl.scheme).toBeNull(); - expectToBeDefined(parsedUrl.host); - expect(parsedUrl.port).toEqual(7474); + expect(address.host()).toEqual('neo4j.com'); + expect(address.resolvedHost()).not.toEqual('neo4j.com'); + expect(address.port()).toEqual(7474); // default port should be appended }); done(); @@ -73,25 +72,27 @@ describe('NodeHostNameResolver', () => { }); it('should resolve IPv4 address to itself', done => { - const addressToResolve = '127.0.0.1'; + const addressToResolve = ServerAddress.fromUrl('127.0.0.1'); const expectedResolvedAddress = '127.0.0.1:7687'; // includes default port testIpAddressResolution(addressToResolve, expectedResolvedAddress, done); }); it('should resolve IPv4 address with port to itself', done => { - const address = '127.0.0.1:7474'; - testIpAddressResolution(address, address, done); + const address = ServerAddress.fromUrl('127.0.0.1:7474'); + const expectedResolvedAddress = '127.0.0.1:7474'; // includes default port + testIpAddressResolution(address, expectedResolvedAddress, done); }); it('should resolve IPv6 address to itself', done => { - const addressToResolve = '[2001:4860:4860::8888]'; + const addressToResolve = ServerAddress.fromUrl('[2001:4860:4860::8888]'); const expectedResolvedAddress = '[2001:4860:4860::8888]:7687'; // includes default port testIpAddressResolution(addressToResolve, expectedResolvedAddress, done); }); it('should resolve IPv6 address with port to itself', done => { - const address = '[2001:4860:4860::8888]:7474'; - testIpAddressResolution(address, address, done); + const address = ServerAddress.fromUrl('[2001:4860:4860::8888]:7474'); + const expectedResolvedAddress = '[2001:4860:4860::8888]:7474'; + testIpAddressResolution(address, expectedResolvedAddress, done); }); }); @@ -100,7 +101,7 @@ function testIpAddressResolution(address, expectedResolvedAddress, done) { resolver.resolve(address).then(addresses => { expect(addresses.length).toEqual(1); - expect(addresses[0]).toEqual(expectedResolvedAddress); + expect(addresses[0].asHostPort()).toEqual(expectedResolvedAddress); done(); }); } diff --git a/test/internal/node/routing.driver.boltkit.test.js b/test/internal/node/routing.driver.boltkit.test.js index 358ff1833..b5040cfa2 100644 --- a/test/internal/node/routing.driver.boltkit.test.js +++ b/test/internal/node/routing.driver.boltkit.test.js @@ -23,6 +23,7 @@ import boltStub from '../bolt-stub'; import RoutingTable from '../../../src/v1/internal/routing-table'; import {SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../../../src/v1/error'; import lolex from 'lolex'; +import ServerAddress from '../../../src/v1/internal/server-address'; describe('routing driver with stub server', () => { @@ -84,7 +85,7 @@ describe('routing driver with stub server', () => { expect(hasAddressInConnectionPool(driver, '127.0.0.1:9001')).toBeTruthy(); assertHasReaders(driver, ['127.0.0.1:9001', '[::1]:9001']); assertHasWriters(driver, ['[2001:db8:a0b:12f0::1]:9002', '[3731:54:65fe:2::a7]:9003']); - assertHasRouters(driver, ['[ff02::1]:9001', '[684D:1111:222:3333:4444:5555:6:77]:9002', '[::1]:9003']); + assertHasRouters(driver, ['[ff02::1]:9001', '[684d:1111:222:3333:4444:5555:6:77]:9002', '[::1]:9003']); driver.close(); server.exit(code => { @@ -2322,31 +2323,31 @@ describe('routing driver with stub server', () => { } function hasAddressInConnectionPool(driver, address) { - return getConnectionPool(driver).has(address); + return getConnectionPool(driver).has(ServerAddress.fromUrl(address)); } function hasRouterInRoutingTable(driver, expectedRouter) { - return getRoutingTable(driver).routers.indexOf(expectedRouter) > -1; + return getRoutingTable(driver).routers.indexOf(ServerAddress.fromUrl(expectedRouter)) > -1; } function hasReaderInRoutingTable(driver, expectedReader) { - return getRoutingTable(driver).readers.indexOf(expectedReader) > -1; + return getRoutingTable(driver).readers.indexOf(ServerAddress.fromUrl(expectedReader)) > -1; } function hasWriterInRoutingTable(driver, expectedWriter) { - return getRoutingTable(driver).writers.indexOf(expectedWriter) > -1; + return getRoutingTable(driver).writers.indexOf(ServerAddress.fromUrl(expectedWriter)) > -1; } function assertHasRouters(driver, expectedRouters) { - expect(getRoutingTable(driver).routers).toEqual(expectedRouters); + expect(getRoutingTable(driver).routers.map(s => s.asHostPort())).toEqual(expectedRouters); } function assertHasReaders(driver, expectedReaders) { - expect(getRoutingTable(driver).readers).toEqual(expectedReaders); + expect(getRoutingTable(driver).readers.map(s => s.asHostPort())).toEqual(expectedReaders); } function assertHasWriters(driver, expectedWriters) { - expect(getRoutingTable(driver).writers).toEqual(expectedWriters); + expect(getRoutingTable(driver).writers.map(s => s.asHostPort())).toEqual(expectedWriters); } function setUpMemorizingRoutingTable(driver) { @@ -2357,7 +2358,12 @@ describe('routing driver with stub server', () => { function setupFakeHostNameResolution(driver, seedRouter, resolvedAddresses) { const connectionProvider = driver._getOrCreateConnectionProvider(); - connectionProvider._hostNameResolver = new FakeHostNameResolver(seedRouter, resolvedAddresses); + connectionProvider._hostNameResolver._resolverFunction = function (address) { + if (address === seedRouter) { + return Promise.resolve(resolvedAddresses); + } + return Promise.reject(new Error('Unexpected seed router address ' + address)); + }; } function getConnectionPool(driver) { @@ -2464,23 +2470,7 @@ describe('routing driver with stub server', () => { } assertForgotRouters(expectedRouters) { - expect(this._forgottenRouters).toEqual(expectedRouters); - } - } - - class FakeHostNameResolver { - - constructor(seedRouter, resolvedAddresses) { - this._seedRouter = seedRouter; - this._resolvedAddresses = resolvedAddresses; - } - - resolve(seedRouter) { - if (seedRouter === this._seedRouter) { - return Promise.resolve(this._resolvedAddresses); - } - return Promise.reject(new Error('Unexpected seed router address ' + seedRouter)); + expect(this._forgottenRouters.map(s => s.asHostPort())).toEqual(expectedRouters); } } - }); diff --git a/test/internal/pool.test.js b/test/internal/pool.test.js index 5537daeb1..bbe5714d3 100644 --- a/test/internal/pool.test.js +++ b/test/internal/pool.test.js @@ -19,18 +19,19 @@ import Pool from '../../src/v1/internal/pool'; import PoolConfig from '../../src/v1/internal/pool-config'; +import ServerAddress from '../../src/v1/internal/server-address'; describe('Pool', () => { it('allocates if pool is empty', (done) => { // Given let counter = 0; - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release))); + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release))); // When - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); // Then Promise.all([ p0, p1 ]).then(values => { @@ -48,15 +49,15 @@ describe('Pool', () => { it('pools if resources are returned', (done) => { // Given a pool that allocates let counter = 0; - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release))); + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release))); // When - const p0 = pool.acquire(key).then(r0 => { + const p0 = pool.acquire(address).then(r0 => { r0.close(); return r0; }); - const p1 = p0.then(r0 => pool.acquire(key)); + const p1 = p0.then(r0 => pool.acquire(address)); // Then Promise.all([ p0, p1 ]).then(values => { @@ -74,16 +75,16 @@ describe('Pool', () => { it('handles multiple keys', (done) => { // Given a pool that allocates let counter = 0; - const key1 = 'bolt://localhost:7687'; - const key2 = 'bolt://localhost:7688'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release))); + const address1 = ServerAddress.fromUrl('bolt://localhost:7687'); + const address2 = ServerAddress.fromUrl('bolt://localhost:7688'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release))); // When - const p0 = pool.acquire(key1); - const p1 = pool.acquire(key2); + const p0 = pool.acquire(address1); + const p1 = pool.acquire(address2); const p01 = Promise.all([ p0, p1 ]).then(values => values[0].close()); - const p2 = p01.then(() => pool.acquire(key1)); - const p3 = p01.then(() => pool.acquire(key2)); + const p2 = p01.then(() => pool.acquire(address1)); + const p3 = p01.then(() => pool.acquire(address2)); // Then Promise.all([ p0, p1, p2, p3 ]).then(values => { @@ -108,9 +109,9 @@ describe('Pool', () => { // Given a pool that allocates let counter = 0; let destroyed = []; - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => { destroyed.push(resource); }, @@ -119,8 +120,8 @@ describe('Pool', () => { ); // When - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); // Then Promise.all([ p0, p1 ]).then(values => { @@ -142,9 +143,9 @@ describe('Pool', () => { it('purges keys', (done) => { // Given a pool that allocates let counter = 0; - const key1 = 'bolt://localhost:7687'; - const key2 = 'bolt://localhost:7688'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release)), + const address1 = ServerAddress.fromUrl('bolt://localhost:7687'); + const address2 = ServerAddress.fromUrl('bolt://localhost:7688'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release)), res => { res.destroyed = true; return true; @@ -152,22 +153,22 @@ describe('Pool', () => { ); // When - const p0 = pool.acquire(key1); - const p1 = pool.acquire(key2); + const p0 = pool.acquire(address1); + const p1 = pool.acquire(address2); const p01 = Promise.all([ p0, p1 ]).then(values => { values.forEach(v => v.close()); - expect(pool.has(key1)).toBeTruthy(); - expect(pool.has(key2)).toBeTruthy(); + expect(pool.has(address1)).toBeTruthy(); + expect(pool.has(address2)).toBeTruthy(); - pool.purge(key1); + pool.purge(address1); - expect(pool.has(key1)).toBeFalsy(); - expect(pool.has(key2)).toBeTruthy(); + expect(pool.has(address1)).toBeFalsy(); + expect(pool.has(address2)).toBeTruthy(); }); - const p2 = p01.then(() => pool.acquire(key1)); - const p3 = p01.then(() => pool.acquire(key2)); + const p2 = p01.then(() => pool.acquire(address1)); + const p3 = p01.then(() => pool.acquire(address2)); // Then Promise.all([ p0, p1, p2, p3 ]).then(values => { @@ -188,25 +189,25 @@ describe('Pool', () => { it('destroys resource when key was purged', (done) => { let counter = 0; - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release)), + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release)), res => { res.destroyed = true; return true; } ); - const p0 = pool.acquire(key); + const p0 = pool.acquire(address); p0.then(r0 => { - expect(pool.has(key)).toBeTruthy(); + expect(pool.has(address)).toBeTruthy(); expect(r0.id).toEqual(0); - pool.purge(key); - expect(pool.has(key)).toBeFalsy(); + pool.purge(address); + expect(pool.has(address)).toBeFalsy(); expect(r0.destroyed).toBeFalsy(); r0.close(); - expect(pool.has(key)).toBeFalsy(); + expect(pool.has(address)).toBeFalsy(); expect(r0.destroyed).toBeTruthy(); done(); @@ -216,11 +217,11 @@ describe('Pool', () => { it('purges all keys', (done) => { let counter = 0; - const key1 = 'bolt://localhost:7687'; - const key2 = 'bolt://localhost:7688'; - const key3 = 'bolt://localhost:7689'; + const address1 = ServerAddress.fromUrl('bolt://localhost:7687'); + const address2 = ServerAddress.fromUrl('bolt://localhost:7688'); + const address3 = ServerAddress.fromUrl('bolt://localhost:7689'); - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release)), + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release)), res => { res.destroyed = true; return true; @@ -228,12 +229,12 @@ describe('Pool', () => { ); const acquiredResources = [ - pool.acquire(key1), - pool.acquire(key2), - pool.acquire(key3), - pool.acquire(key1), - pool.acquire(key2), - pool.acquire(key3) + pool.acquire(address1), + pool.acquire(address2), + pool.acquire(address3), + pool.acquire(address1), + pool.acquire(address2), + pool.acquire(address3) ]; Promise.all(acquiredResources).then(values => { @@ -250,8 +251,8 @@ describe('Pool', () => { it('skips broken connections during acquire', (done) => { let validated = false; let counter = 0; - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, counter++, release)), + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release)), res => { res.destroyed = true; return true; @@ -265,11 +266,11 @@ describe('Pool', () => { } ); - const p0 = pool.acquire(key); + const p0 = pool.acquire(address); const p1 = p0.then(r0 => { r0.close(); - return pool.acquire(key); + return pool.acquire(address); }); Promise.all([ p0, p1 ]).then(values => { @@ -283,64 +284,64 @@ describe('Pool', () => { }); it('reports presence of the key', (done) => { - const existingKey = 'bolt://localhost:7687'; - const absentKey = 'bolt://localhost:7688'; + const existingAddress = ServerAddress.fromUrl('bolt://localhost:7687'); + const absentAddress = ServerAddress.fromUrl('bolt://localhost:7688'); - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, 42, release))); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, 42, release))); - const p0 = pool.acquire(existingKey); - const p1 = pool.acquire(existingKey); + const p0 = pool.acquire(existingAddress); + const p1 = pool.acquire(existingAddress); Promise.all([ p0, p1 ]).then(() => { - expect(pool.has(existingKey)).toBeTruthy(); - expect(pool.has(absentKey)).toBeFalsy(); + expect(pool.has(existingAddress)).toBeTruthy(); + expect(pool.has(absentAddress)).toBeFalsy(); done(); }); }); it('reports zero active resources when empty', () => { - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, 42, release))); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, 42, release))); - expect(pool.activeResourceCount('bolt://localhost:1')).toEqual(0); - expect(pool.activeResourceCount('bolt://localhost:2')).toEqual(0); - expect(pool.activeResourceCount('bolt://localhost:3')).toEqual(0); + expect(pool.activeResourceCount(ServerAddress.fromUrl('bolt://localhost:1'))).toEqual(0); + expect(pool.activeResourceCount(ServerAddress.fromUrl('bolt://localhost:2'))).toEqual(0); + expect(pool.activeResourceCount(ServerAddress.fromUrl('bolt://localhost:3'))).toEqual(0); }); it('reports active resources', (done) => { - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, 42, release))); + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, 42, release))); - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); - const p2 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); + const p2 = pool.acquire(address); Promise.all([ p0, p1, p2 ]).then(values => { values.forEach(v => expect(v).toBeDefined()); - expect(pool.activeResourceCount(key)).toEqual(3); + expect(pool.activeResourceCount(address)).toEqual(3); done(); }); }); it('reports active resources when they are acquired', (done) => { - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, 42, release))); + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, 42, release))); // three new resources are created and returned to the pool - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); - const p2 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); + const p2 = pool.acquire(address); const p012 = Promise.all([ p0, p1, p2 ]).then(values => { values.forEach(v => v.close()); return values; }); // three idle resources are acquired from the pool - const p3 = p012.then(() => pool.acquire(key)); - const p4 = p012.then(() => pool.acquire(key)); - const p5 = p012.then(() => pool.acquire(key)); + const p3 = p012.then(() => pool.acquire(address)); + const p4 = p012.then(() => pool.acquire(address)); + const p5 = p012.then(() => pool.acquire(address)); const pAll = Promise.all([ p012, p3, p4, p5 ]).then(values => { const r0 = values[0][0]; @@ -351,43 +352,43 @@ describe('Pool', () => { expect(values).toContain(r1); expect(values).toContain(r2); - expect(pool.activeResourceCount(key)).toEqual(3); + expect(pool.activeResourceCount(address)).toEqual(3); done(); }); }); it('does not report resources that are returned to the pool', (done) => { - const key = 'bolt://localhost:7687'; - const pool = new Pool((url, release) => Promise.resolve(new Resource(url, 42, release))); + const address = ServerAddress.fromUrl('bolt://localhost:7687'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, 42, release))); - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); - const p2 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); + const p2 = pool.acquire(address); const p012 = Promise.all([ p0, p1, p2 ]).then(values => { const r0 = values[0]; const r1 = values[1]; const r2 = values[2]; - expect(pool.activeResourceCount(key)).toEqual(3); + expect(pool.activeResourceCount(address)).toEqual(3); r0.close(); - expect(pool.activeResourceCount(key)).toEqual(2); + expect(pool.activeResourceCount(address)).toEqual(2); r1.close(); - expect(pool.activeResourceCount(key)).toEqual(1); + expect(pool.activeResourceCount(address)).toEqual(1); r2.close(); - expect(pool.activeResourceCount(key)).toEqual(0); + expect(pool.activeResourceCount(address)).toEqual(0); return values; }); - const p3 = p012.then(() => pool.acquire(key)).then(r3 => { - expect(pool.activeResourceCount(key)).toEqual(1); + const p3 = p012.then(() => pool.acquire(address)).then(r3 => { + expect(pool.activeResourceCount(address)).toEqual(1); r3.close(); - expect(pool.activeResourceCount(key)).toEqual(0); + expect(pool.activeResourceCount(address)).toEqual(0); done(); }); @@ -396,16 +397,16 @@ describe('Pool', () => { it('should wait for a returned connection when max pool size is reached', done => { let counter = 0; - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => {}, resource => true, new PoolConfig(2, 5000) ); - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); const p01 = Promise.all([ p0, p1 ]).then(values => { const r0 = values[0]; const r1 = values[1]; @@ -413,14 +414,14 @@ describe('Pool', () => { expect(r0.id).toEqual(0); expect(r1.id).toEqual(1); - const p2 = pool.acquire(key).then(r2 => { + const p2 = pool.acquire(address).then(r2 => { expect(r2).toBe(r1); done(); }); setTimeout(() => { - expectNumberOfAcquisitionRequests(pool, key, 1); + expectNumberOfAcquisitionRequests(pool, address, 1); r1.close(); }, 1000); }); @@ -429,16 +430,16 @@ describe('Pool', () => { it('should time out when max pool size is reached', done => { let counter = 0; - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => {}, resource => true, new PoolConfig(2, 1000) ); - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); const p01 = Promise.all([ p0, p1 ]).then(values => { const r0 = values[0]; const r1 = values[1]; @@ -446,9 +447,9 @@ describe('Pool', () => { expect(r0.id).toEqual(0); expect(r1.id).toEqual(1); - pool.acquire(key).catch(error => { + pool.acquire(address).catch(error => { expect(error.message).toContain('timed out'); - expectNumberOfAcquisitionRequests(pool, key, 0); + expectNumberOfAcquisitionRequests(pool, address, 0); done(); }); }); @@ -457,15 +458,15 @@ describe('Pool', () => { it('should not time out if max pool size is not set', done => { let counter = 0; - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => {}, resource => true ); - const p0 = pool.acquire(key); - const p1 = pool.acquire(key); + const p0 = pool.acquire(address); + const p1 = pool.acquire(address); const p01 = Promise.all([ p0, p1 ]).then(values => { const r0 = values[0]; const r1 = values[1]; @@ -473,7 +474,7 @@ describe('Pool', () => { expect(r0.id).toEqual(0); expect(r1.id).toEqual(1); - pool.acquire(key).then(r2 => { + pool.acquire(address).then(r2 => { expect(r2.id).toEqual(2); expectNoPendingAcquisitionRequests(pool); done(); @@ -485,19 +486,19 @@ describe('Pool', () => { const acquisitionTimeout = 1000; let counter = 0; - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => { }, () => true, new PoolConfig(2, acquisitionTimeout) ); - pool.acquire(key).then(resource1 => { + pool.acquire(address).then(resource1 => { expect(resource1.id).toEqual(0); - pool.acquire(key).then(resource2 => { + pool.acquire(address).then(resource2 => { expect(resource2.id).toEqual(1); // try to release both resources around the time acquisition fails with timeout @@ -508,7 +509,7 @@ describe('Pool', () => { resource2.close(); }, acquisitionTimeout); - pool.acquire(key).then(someResource => { + pool.acquire(address).then(someResource => { expect(someResource).toBeDefined(); expect(someResource).not.toBeNull(); expectNoPendingAcquisitionRequests(pool); @@ -524,32 +525,32 @@ describe('Pool', () => { }); it('should resolve pending acquisition request when single invalid resource returned', done => { - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const acquisitionTimeout = 1000; let counter = 0; const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => { }, resourceValidOnlyOnceValidationFunction, new PoolConfig(1, acquisitionTimeout) ); - pool.acquire(key).then(resource1 => { + pool.acquire(address).then(resource1 => { expect(resource1.id).toEqual(0); - expect(pool.activeResourceCount(key)).toEqual(1); + expect(pool.activeResourceCount(address)).toEqual(1); // release the resource before the acquisition timeout, it should be treated as invalid setTimeout(() => { - expectNumberOfAcquisitionRequests(pool, key, 1); + expectNumberOfAcquisitionRequests(pool, address, 1); resource1.close(); }, acquisitionTimeout / 2); - pool.acquire(key).then(resource2 => { + pool.acquire(address).then(resource2 => { expect(resource2.id).toEqual(1); expectNoPendingAcquisitionRequests(pool); - expect(pool.activeResourceCount(key)).toEqual(1); + expect(pool.activeResourceCount(address)).toEqual(1); done(); }).catch(error => { done.fail(error); @@ -558,37 +559,37 @@ describe('Pool', () => { }); it('should work fine when invalid resources released and acquisition attempt pending', done => { - const key = 'bolt://localhost:7687'; + const address = ServerAddress.fromUrl('bolt://localhost:7687'); const acquisitionTimeout = 1000; let counter = 0; const pool = new Pool( - (url, release) => Promise.resolve(new Resource(url, counter++, release)), + (server, release) => Promise.resolve(new Resource(server, counter++, release)), resource => { }, resourceValidOnlyOnceValidationFunction, new PoolConfig(2, acquisitionTimeout) ); - pool.acquire(key).then(resource1 => { + pool.acquire(address).then(resource1 => { expect(resource1.id).toEqual(0); - expect(pool.activeResourceCount(key)).toEqual(1); + expect(pool.activeResourceCount(address)).toEqual(1); - pool.acquire(key).then(resource2 => { + pool.acquire(address).then(resource2 => { expect(resource2.id).toEqual(1); - expect(pool.activeResourceCount(key)).toEqual(2); + expect(pool.activeResourceCount(address)).toEqual(2); // release both resources before the acquisition timeout, they should be treated as invalid setTimeout(() => { - expectNumberOfAcquisitionRequests(pool, key, 1); + expectNumberOfAcquisitionRequests(pool, address, 1); resource1.close(); resource2.close(); }, acquisitionTimeout / 2); - pool.acquire(key).then(resource3 => { + pool.acquire(address).then(resource3 => { expect(resource3.id).toEqual(2); expectNoPendingAcquisitionRequests(pool); - expect(pool.activeResourceCount(key)).toEqual(1); + expect(pool.activeResourceCount(address)).toEqual(1); done(); }).catch(error => { done.fail(error); @@ -609,8 +610,8 @@ function expectNoPendingAcquisitionRequests(pool) { }); } -function expectNumberOfAcquisitionRequests(pool, key, expectedNumber) { - expect(pool._acquireRequests[key].length).toEqual(expectedNumber); +function expectNumberOfAcquisitionRequests(pool, address, expectedNumber) { + expect(pool._acquireRequests[address.asKey()].length).toEqual(expectedNumber); } function resourceValidOnlyOnceValidationFunction(resource) { diff --git a/test/internal/rediscovery.test.js b/test/internal/rediscovery.test.js index b1d71fa26..0c6ce74c9 100644 --- a/test/internal/rediscovery.test.js +++ b/test/internal/rediscovery.test.js @@ -23,6 +23,7 @@ import {newError, PROTOCOL_ERROR} from '../../src/v1/error'; import Record from '../../src/v1/record'; import {int} from '../../src/v1/integer'; import RoutingTable from '../../src/v1/internal/routing-table'; +import ServerAddress from '../../src/v1/internal/server-address'; const ROUTER_ADDRESS = 'bolt+routing://test.router.com'; @@ -149,15 +150,15 @@ describe('rediscovery', () => { }); it('should return valid routing table with 1 router, 1 reader and 1 writer', done => { - testValidRoutingTable(['router1'], ['reader1'], ['writer1'], int(42), done); + testValidRoutingTable(['router1:7687'], ['reader1:7687'], ['writer1:7687'], int(42), done); }); it('should return valid routing table with 2 routers, 2 readers and 2 writers', done => { - testValidRoutingTable(['router1', 'router2'], ['reader1', 'reader2'], ['writer1', 'writer2'], int(Date.now()), done); + testValidRoutingTable(['router1:7687', 'router2:7687'], ['reader1:7687', 'reader2:7687'], ['writer1:7687', 'writer2:7687'], int(Date.now()), done); }); it('should return valid routing table with 1 router, 3 readers and 1 writer', done => { - testValidRoutingTable(['router1'], ['reader1', 'reader2', 'reader3'], ['writer1'], int(12345), done); + testValidRoutingTable(['router1:7687'], ['reader1:7687', 'reader2:7687', 'reader3:7687'], ['writer1:7687'], int(12345), done); }); function testValidRoutingTable(routerAddresses, readerAddresses, writerAddresses, expires, done) { @@ -166,9 +167,9 @@ describe('rediscovery', () => { parseTtl: () => expires, parseServers: () => { return { - routers: routerAddresses, - readers: readerAddresses, - writers: writerAddresses + routers: routerAddresses.map(a => ServerAddress.fromUrl(a)), + readers: readerAddresses.map(a => ServerAddress.fromUrl(a)), + writers: writerAddresses.map(a => ServerAddress.fromUrl(a)) }; } }); @@ -181,7 +182,7 @@ describe('rediscovery', () => { const allServers = routingTable.serversDiff(new RoutingTable()).sort(); const allExpectedServers = [...routerAddresses, ...readerAddresses, ...writerAddresses].sort(); - expect(allServers).toEqual(allExpectedServers); + expect(allServers.map(s => s.asHostPort())).toEqual(allExpectedServers); done(); }); diff --git a/test/internal/routing-table.test.js b/test/internal/routing-table.test.js index 11673d54b..ab6f8d344 100644 --- a/test/internal/routing-table.test.js +++ b/test/internal/routing-table.test.js @@ -19,113 +19,126 @@ import RoutingTable from '../../src/v1/internal/routing-table'; import {int} from '../../src/v1/integer'; import {READ, WRITE} from '../../src/v1/driver'; +import ServerAddress from '../../src/v1/internal/server-address'; describe('routing-table', () => { + const server1 = ServerAddress.fromUrl('server1'); + const server2 = ServerAddress.fromUrl('server2'); + const server3 = ServerAddress.fromUrl('server3'); + const server4 = ServerAddress.fromUrl('server4'); + const server5 = ServerAddress.fromUrl('server5'); + const server6 = ServerAddress.fromUrl('server6'); + const server7 = ServerAddress.fromUrl('server7'); + const server11 = ServerAddress.fromUrl('server11'); + const server22 = ServerAddress.fromUrl('server22'); + const server33 = ServerAddress.fromUrl('server33'); + const server44 = ServerAddress.fromUrl('server44'); + const server42 = ServerAddress.fromUrl('server42'); it('should not be stale when has routers, readers, writers and future expiration date', () => { - const table = createTable([1, 2], [3, 4], [5, 6], notExpired()); + const table = createTable([server1, server2], [server3, server4], [server5, server6], notExpired()); expect(table.isStaleFor(READ)).toBeFalsy(); expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should be stale when expiration date in the past', () => { - const table = createTable([1, 2], [1, 2], [1, 2], expired()); + const table = createTable([server1, server2], [server1, server2], [server1, server2], expired()); expect(table.isStaleFor(READ)).toBeTruthy(); expect(table.isStaleFor(WRITE)).toBeTruthy(); }); it('should not be stale when has single router', () => { - const table = createTable([1], [2, 3], [4, 5], notExpired()); + const table = createTable([server1], [server2, server3], [server4, server5], notExpired()); expect(table.isStaleFor(READ)).toBeFalsy(); expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should be stale for reads but not writes when no readers', () => { - const table = createTable([1, 2], [], [3, 4], notExpired()); + const table = createTable([server1, server2], [], [server3, server4], notExpired()); expect(table.isStaleFor(READ)).toBeTruthy(); expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should be stale for writes but not reads when no writers', () => { - const table = createTable([1, 2], [3, 4], [], notExpired()); + const table = createTable([server1, server2], [server3, server4], [], notExpired()); expect(table.isStaleFor(READ)).toBeFalsy(); expect(table.isStaleFor(WRITE)).toBeTruthy(); }); it('should not be stale with single reader', () => { - const table = createTable([1, 2], [3], [4, 5], notExpired()); + const table = createTable([server1, server2], [server3], [server4, server5], notExpired()); expect(table.isStaleFor(READ)).toBeFalsy(); expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should not be stale with single writer', () => { - const table = createTable([1, 2], [3, 4], [5], notExpired()); + const table = createTable([server1, server2], [server3, server4], [server5], notExpired()); expect(table.isStaleFor(READ)).toBeFalsy(); expect(table.isStaleFor(WRITE)).toBeFalsy(); }); it('should forget reader, writer but not router', () => { - const table = createTable([1, 2], [1, 2], [1, 2], notExpired()); + const table = createTable([server1, server2], [server1, server2], [server1, server2], notExpired()); - table.forget(1); + table.forget(server1); - expect(table.routers).toEqual([1, 2]); - expect(table.readers).toEqual([2]); - expect(table.writers).toEqual([2]); + expect(table.routers).toEqual([server1, server2]); + expect(table.readers).toEqual([server2]); + expect(table.writers).toEqual([server2]); }); it('should forget single reader', () => { - const table = createTable([1, 2], [42], [1, 2, 3], notExpired()); + const table = createTable([server1, server2], [server42], [server1, server2, server3], notExpired()); - table.forget(42); + table.forget(server42); - expect(table.routers).toEqual([1, 2]); + expect(table.routers).toEqual([server1, server2]); expect(table.readers).toEqual([]); - expect(table.writers).toEqual([1, 2, 3]); + expect(table.writers).toEqual([server1, server2, server3]); }); it('should forget single writer', () => { - const table = createTable([1, 2], [3, 4, 5], [42], notExpired()); + const table = createTable([server1, server2], [server3, server4, server5], [server42], notExpired()); - table.forget(42); + table.forget(server42); - expect(table.routers).toEqual([1, 2]); - expect(table.readers).toEqual([3, 4, 5]); + expect(table.routers).toEqual([server1, server2]); + expect(table.readers).toEqual([server3, server4, server5]); expect(table.writers).toEqual([]); }); it('should forget router', () => { - const table = createTable([1, 2], [1, 3], [4, 1], notExpired()); + const table = createTable([server1, server2], [server1, server3], [server4, server1], notExpired()); - table.forgetRouter(1); + table.forgetRouter(server1); - expect(table.routers).toEqual([2]); - expect(table.readers).toEqual([1, 3]); - expect(table.writers).toEqual([4, 1]); + expect(table.routers).toEqual([server2]); + expect(table.readers).toEqual([server1, server3]); + expect(table.writers).toEqual([server4, server1]); }); it('should forget writer', () => { - const table = createTable([1, 2, 3], [2, 1, 5], [5, 1], notExpired()); + const table = createTable([server1, server2, server3], [server2, server1, server5], [server5, server1], notExpired()); - table.forgetWriter(1); + table.forgetWriter(server1); - expect(table.routers).toEqual([1, 2, 3]); - expect(table.readers).toEqual([2, 1, 5]); - expect(table.writers).toEqual([5]); + expect(table.routers).toEqual([server1, server2, server3]); + expect(table.readers).toEqual([server2, server1, server5]); + expect(table.writers).toEqual([server5]); }); it('should return all servers in diff when other table is empty', () => { - const oldTable = createTable([1, 2], [3, 4], [5, 6], notExpired()); + const oldTable = createTable([server1, server2], [server3, server4], [server5, server6], notExpired()); const newTable = createTable([], [], [], notExpired()); const servers = oldTable.serversDiff(newTable); - expect(servers).toEqual([1, 2, 3, 4, 5, 6]); + expect(servers).toEqual([server1, server2, server3, server4, server5, server6]); }); it('should no servers in diff when this table is empty', () => { const oldTable = createTable([], [], [], notExpired()); - const newTable = createTable([1, 2], [3, 4], [5, 6], notExpired()); + const newTable = createTable([server1, server2], [server3, server4], [server5, server6], notExpired()); const servers = oldTable.serversDiff(newTable); @@ -133,47 +146,56 @@ describe('routing-table', () => { }); it('should include different routers in servers diff', () => { - const oldTable = createTable([1, 7, 2, 42], [3, 4], [5, 6], notExpired()); - const newTable = createTable([1, 2], [3, 4], [5, 6], notExpired()); + const oldTable = createTable([server1, server7, server2, server42], [server3, server4], [server5, server6], notExpired()); + const newTable = createTable([server1, server2], [server3, server4], [server5, server6], notExpired()); const servers = oldTable.serversDiff(newTable); - expect(servers).toEqual([7, 42]); + expect(servers).toEqual([server7, server42]); }); it('should include different readers in servers diff', () => { - const oldTable = createTable([1, 2], [3, 7, 4, 42], [5, 6], notExpired()); - const newTable = createTable([1, 2], [3, 4], [5, 6], notExpired()); + const oldTable = createTable([server1, server2], [server3, server7, server4, server42], [server5, server6], notExpired()); + const newTable = createTable([server1, server2], [server3, server4], [server5, server6], notExpired()); const servers = oldTable.serversDiff(newTable); - expect(servers).toEqual([7, 42]); + expect(servers).toEqual([server7, server42]); }); it('should include different writers in servers diff', () => { - const oldTable = createTable([1, 2], [3, 4], [5, 7, 6, 42], notExpired()); - const newTable = createTable([1, 2], [3, 4], [5, 6], notExpired()); + const oldTable = createTable([server1, server2], [server3, server4], [server5, server7, server6, server42], notExpired()); + const newTable = createTable([server1, server2], [server3, server4], [server5, server6], notExpired()); const servers = oldTable.serversDiff(newTable); - expect(servers).toEqual([7, 42]); + expect(servers).toEqual([server7, server42]); }); it('should include different servers in diff', () => { - const oldTable = createTable([1, 2, 11], [22, 3, 33, 4], [5, 44, 6], notExpired()); - const newTable = createTable([1], [2, 3, 4, 6], [5], notExpired()); + const oldTable = createTable([server1, server2, server11], [server22, server3, server33, server4], [server5, server44, server6], notExpired()); + const newTable = createTable([server1], [server2, server3, server4, server6], [server5], notExpired()); const servers = oldTable.serversDiff(newTable); - expect(servers).toEqual([11, 22, 33, 44]); + expect(servers).toEqual([server11, server22, server33, server44]); + }); + + it('should include different servers in diff with logical equality', () => { + const oldTable = createTable([server1, server11], [server2, server22], [server3, server33], notExpired()); + const newTable = createTable([ServerAddress.fromUrl(server1.asHostPort())], [ServerAddress.fromUrl(server2.asHostPort())], [ServerAddress.fromUrl(server3.asHostPort())], notExpired()); + + const servers = oldTable.serversDiff(newTable); + + expect(servers).toEqual([server11, server22, server33]); }); it('should have correct toString', () => { const originalDateNow = Date.now; try { Date.now = () => 4242; - const table = createTable([1, 2], [3, 4], [5, 6], 42); - expect(table.toString()).toEqual('RoutingTable[expirationTime=42, currentTime=4242, routers=[1,2], readers=[3,4], writers=[5,6]]'); + const table = createTable([server1, server2], [server3, server4], [server5, server6], 42); + expect(table.toString()).toEqual('RoutingTable[expirationTime=42, currentTime=4242, routers=[server1:7687,server2:7687], readers=[server3:7687,server4:7687], writers=[server5:7687,server6:7687]]'); } finally { Date.now = originalDateNow; } diff --git a/test/internal/routing-util.test.js b/test/internal/routing-util.test.js index 02f14cbd6..7b33f082f 100644 --- a/test/internal/routing-util.test.js +++ b/test/internal/routing-util.test.js @@ -23,6 +23,7 @@ import Integer, {int} from '../../src/v1/integer'; import {newError, PROTOCOL_ERROR, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../../src/v1/error'; import lolex from 'lolex'; import FakeConnection from './fake-connection'; +import ServerAddress from '../../src/v1/internal/server-address'; const ROUTER_ADDRESS = 'test.router.com:4242'; @@ -257,9 +258,9 @@ describe('RoutingUtil', () => { const {routers, readers, writers} = parseServers(record); - expect(routers).toEqual(routerAddresses); - expect(readers).toEqual(readerAddresses); - expect(writers).toEqual(writerAddresses); + expect(routers).toEqual(routerAddresses.map(r => ServerAddress.fromUrl(r))); + expect(readers).toEqual(readerAddresses.map(r => ServerAddress.fromUrl(r))); + expect(writers).toEqual(writerAddresses.map(w => ServerAddress.fromUrl(w))); } function callRoutingProcedure(session, routingContext) { diff --git a/test/v1/session.test.js b/test/v1/session.test.js index d2473afa4..0706a5936 100644 --- a/test/v1/session.test.js +++ b/test/v1/session.test.js @@ -29,6 +29,7 @@ import {ServerVersion, VERSION_3_1_0} from '../../src/v1/internal/server-version import {isString} from '../../src/v1/internal/util'; import testUtils from '../internal/test-utils'; import {newError, PROTOCOL_ERROR, SESSION_EXPIRED} from '../../src/v1/error'; +import ServerAddress from '../../src/v1/internal/server-address'; describe('session', () => { @@ -1133,7 +1134,7 @@ describe('session', () => { function idleConnectionCount(driver) { const connectionProvider = driver._connectionProvider; - const address = connectionProvider._hostPort; + const address = connectionProvider._address.asKey(); const connectionPool = connectionProvider._connectionPool; const idleConnections = connectionPool._pools[address]; return idleConnections.length; @@ -1181,7 +1182,7 @@ describe('session', () => { function numberOfAcquiredConnectionsFromPool() { const pool = driver._pool; - return pool.activeResourceCount('localhost:7687'); + return pool.activeResourceCount(ServerAddress.fromUrl('localhost:7687')); } function testConnectionTimeout(encrypted, done) { From a2c01e8621d8d8f20d6bab5a558a9a70da87d925 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 10:23:36 +0100 Subject: [PATCH 02/12] Add tests for server address implementation --- src/v1/internal/server-address.js | 2 +- test/internal/server-address.test.js | 122 +++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 test/internal/server-address.test.js diff --git a/src/v1/internal/server-address.js b/src/v1/internal/server-address.js index 13b509650..f02905afb 100644 --- a/src/v1/internal/server-address.js +++ b/src/v1/internal/server-address.js @@ -26,7 +26,7 @@ export default class ServerAddress { this._resolved = resolved ? assertString(resolved, 'resolved') : null; this._port = assertNumber(port, 'port'); this._hostPort = hostPort; - this._stringValue = resolved ? `${hostPort}[${resolved}]` : `${hostPort}`; + this._stringValue = resolved ? `${hostPort}(${resolved})` : `${hostPort}`; } host() { diff --git a/test/internal/server-address.test.js b/test/internal/server-address.test.js new file mode 100644 index 000000000..050eb0853 --- /dev/null +++ b/test/internal/server-address.test.js @@ -0,0 +1,122 @@ +/** + * Copyright (c) 2002-2019 "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 ServerAddress from '../../src/v1/internal/server-address'; + +describe('ServerAddress', () => { + + it('should construct with correct values', () => { + verifyAddress(ServerAddress.fromUrl('host.some.domain:8687'), { + host: 'host.some.domain', + port: 8687, + hostAndPort: 'host.some.domain:8687', + key: 'host.some.domain:8687', + toString: 'host.some.domain:8687' + }); + + verifyAddress(ServerAddress.fromUrl('http://host.some.domain:8687'), { + host: 'host.some.domain', + port: 8687, + hostAndPort: 'host.some.domain:8687', + key: 'host.some.domain:8687', + toString: 'host.some.domain:8687' + }); + + verifyAddress(ServerAddress.fromUrl('host2.some.domain'), { + host: 'host2.some.domain', + port: 7687, + hostAndPort: 'host2.some.domain:7687', + key: 'host2.some.domain:7687', + toString: 'host2.some.domain:7687' + }); + + verifyAddress(ServerAddress.fromUrl('https://host2.some.domain'), { + host: 'host2.some.domain', + port: 7473, + hostAndPort: 'host2.some.domain:7473', + key: 'host2.some.domain:7473', + toString: 'host2.some.domain:7473' + }); + + verifyAddress(ServerAddress.fromUrl('10.10.192.0'), { + host: '10.10.192.0', + port: 7687, + hostAndPort: '10.10.192.0:7687', + key: '10.10.192.0:7687', + toString: '10.10.192.0:7687' + }); + + verifyAddress(ServerAddress.fromUrl('[1afc:0:a33:85a3::ff2f]:8889'), { + host: '1afc:0:a33:85a3::ff2f', + port: 8889, + hostAndPort: '[1afc:0:a33:85a3::ff2f]:8889', + key: '[1afc:0:a33:85a3::ff2f]:8889', + toString: '[1afc:0:a33:85a3::ff2f]:8889' + }); + + }); + + it('should return correct values when resolved', () => { + const address = ServerAddress.fromUrl('host.some.domain:8787'); + const resolved1 = address.resolveWith('172.0.0.1'); + const resolved2 = address.resolveWith('172.0.1.1'); + + verifyAddress(resolved1, { + host: 'host.some.domain', + port: 8787, + hostAndPort: 'host.some.domain:8787', + key: 'host.some.domain:8787', + toString: 'host.some.domain:8787(172.0.0.1)', + resolvedHost: '172.0.0.1' + }); + + verifyAddress(resolved2, { + host: 'host.some.domain', + port: 8787, + hostAndPort: 'host.some.domain:8787', + key: 'host.some.domain:8787', + toString: 'host.some.domain:8787(172.0.1.1)', + resolvedHost: '172.0.1.1' + }); + }); + + it('should not lose host info if resolved', () => { + const address = ServerAddress.fromUrl('host.some.domain:8787'); + const resolved1 = address.resolveWith('192.168.0.1'); + const resolved2 = resolved1.resolveWith('192.168.100.1'); + + verifyAddress(resolved2, { + host: 'host.some.domain', + port: 8787, + hostAndPort: 'host.some.domain:8787', + key: 'host.some.domain:8787', + toString: 'host.some.domain:8787(192.168.100.1)', + resolvedHost: '192.168.100.1' + }); + }); +}); + +function verifyAddress(address, { host, port, hostAndPort, key, toString, resolvedHost = null } = {}) { + expect(address.host()).toEqual(host); + expect(address.port()).toEqual(port); + expect(address.asHostPort()).toEqual(hostAndPort); + expect(address.asKey()).toEqual(key); + expect(address.toString()).toEqual(toString); + expect(address.resolvedHost()).toEqual(resolvedHost ? resolvedHost : host); +} From 34c58188f7ea6c0637002002d7618b2a44790f4d Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 10:28:30 +0100 Subject: [PATCH 03/12] Remove unused import --- src/v1/internal/connection-providers.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index 0af727cd3..3740a3a11 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -24,7 +24,6 @@ import RoutingTable from './routing-table'; import Rediscovery from './rediscovery'; import RoutingUtil from './routing-util'; import { HostNameResolver } from './node'; -import { flatMap } from 'lodash/collection'; const UNAUTHORIZED_ERROR_CODE = 'Neo.ClientError.Security.Unauthorized'; From e293d441081000198399d9206995e908cd966a5d Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 13:42:08 +0100 Subject: [PATCH 04/12] Default to seed router on construction --- src/v1/internal/connection-providers.js | 2 +- src/v1/internal/pool.js | 9 +++++---- test/internal/bolt-stub.js | 7 +++++++ test/internal/node/routing.driver.boltkit.test.js | 8 ++++---- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index 3740a3a11..7932ef9ba 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -72,7 +72,7 @@ export class LoadBalancer extends ConnectionProvider { this._hostNameResolver = hostNameResolver; this._dnsResolver = new HostNameResolver(); this._log = log; - this._useSeedRouter = false; + this._useSeedRouter = true; } acquireConnection(accessMode) { diff --git a/src/v1/internal/pool.js b/src/v1/internal/pool.js index 1523a28af..e3855e112 100644 --- a/src/v1/internal/pool.js +++ b/src/v1/internal/pool.js @@ -61,7 +61,7 @@ class Pool { if (resource) { resourceAcquired(key, this._activeResourceCounts); if (this._log.isDebugEnabled()) { - this._log.debug(`${resource} acquired from the pool`); + this._log.debug(`${resource} acquired from the pool ${key}`); } return resource; } @@ -94,7 +94,7 @@ class Pool { } }, this._acquisitionTimeout); - request = new PendingRequest(resolve, reject, timeoutId, this._log); + request = new PendingRequest(key, resolve, reject, timeoutId, this._log); allRequests[key].push(request); }); }); @@ -261,7 +261,8 @@ function resourceReleased(key, activeResourceCounts) { class PendingRequest { - constructor(resolve, reject, timeoutId, log) { + constructor(key, resolve, reject, timeoutId, log) { + this._key = key; this._resolve = resolve; this._reject = reject; this._timeoutId = timeoutId; @@ -281,7 +282,7 @@ class PendingRequest { clearTimeout(this._timeoutId); if (this._log.isDebugEnabled()) { - this._log.debug(`${resource} acquired from the pool`); + this._log.debug(`${resource} acquired from the pool ${this._key}`); } this._resolve(resource); } diff --git a/test/internal/bolt-stub.js b/test/internal/bolt-stub.js index 8fee9673a..e5916e60e 100644 --- a/test/internal/bolt-stub.js +++ b/test/internal/bolt-stub.js @@ -112,8 +112,15 @@ class StubServer { } function newDriver(url, config = {}) { + // left here for debugging purposes + const logging = { + level: 'debug', + logger: (level, msg) => console.log(`${level}: ${msg}`) + }; // boltstub currently does not support encryption, create driver with encryption turned off const newConfig = Object.assign({encrypted: 'ENCRYPTION_OFF'}, config); + // use for logging enabled + // const newConfig = Object.assign({encrypted: 'ENCRYPTION_OFF', logging}, config); return neo4j.driver(url, sharedNeo4j.authToken, newConfig); } diff --git a/test/internal/node/routing.driver.boltkit.test.js b/test/internal/node/routing.driver.boltkit.test.js index b5040cfa2..fc62e0b65 100644 --- a/test/internal/node/routing.driver.boltkit.test.js +++ b/test/internal/node/routing.driver.boltkit.test.js @@ -1522,7 +1522,7 @@ describe('routing driver with stub server', () => { return; } - const router1 = boltStub.start('./test/resources/boltstub/acquire_endpoints.script', 9010); + const router1 = boltStub.start('./test/resources/boltstub/acquire_endpoints_and_exit.script', 9011); // start new router on a different port to emulate host name resolution // this router uses different script that contains itself as reader const router2 = boltStub.start('./test/resources/boltstub/rediscover_using_initial_router.script', 9009); @@ -1957,13 +1957,13 @@ describe('routing driver with stub server', () => { boltStub.run(() => { const resolverFunction = address => { - if (address === '127.0.0.1:9001') { - return ['127.0.0.1:9010', '127.0.0.1:9011', '127.0.0.1:9042']; + if (address === '127.0.0.1:9000') { + return ['127.0.0.1:9010', '127.0.0.1:9001', '127.0.0.1:9042']; } throw new Error(`Unexpected address ${address}`); }; - const driver = boltStub.newDriver('bolt+routing://127.0.0.1:9001', {resolver: resolverFunction}); + const driver = boltStub.newDriver('bolt+routing://127.0.0.1:9000', { resolver: resolverFunction }); const session = driver.session(READ); // run a query that should trigger discovery against 9001 and then read from it From 3e1c947f5a2f248fef10df1690df67d76d29543b Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 14:45:50 +0100 Subject: [PATCH 05/12] Fix test failures --- src/v1/internal/routing-util.js | 2 +- test/internal/connection-providers.test.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/v1/internal/routing-util.js b/src/v1/internal/routing-util.js index 59f7b18a3..42e58efd1 100644 --- a/src/v1/internal/routing-util.js +++ b/src/v1/internal/routing-util.js @@ -50,7 +50,7 @@ export default class RoutingUtil { if (error.code === PROCEDURE_NOT_FOUND_CODE) { // throw when getServers procedure not found because this is clearly a configuration issue throw newError( - `Server at ${routerAddress} can't perform routing. Make sure you are connecting to a causal cluster`, + `Server at ${routerAddress.asHostPort()} can't perform routing. Make sure you are connecting to a causal cluster`, SERVICE_UNAVAILABLE); } else { // return nothing when failed to connect because code higher in the callstack is still able to retry with a diff --git a/test/internal/connection-providers.test.js b/test/internal/connection-providers.test.js index d8f6270f5..b6d1971fc 100644 --- a/test/internal/connection-providers.test.js +++ b/test/internal/connection-providers.test.js @@ -957,6 +957,9 @@ describe('LoadBalancer', () => { 'server02:7687': updatedRoutingTable } ); + // override default use of seed router + loadBalancer._useSeedRouter = false; + const usedRouterArrays = []; setupLoadBalancerToRememberRouters(loadBalancer, usedRouterArrays); @@ -1057,6 +1060,8 @@ describe('LoadBalancer', () => { 'server02:7687': routingTable2 } ); + // override default use of seed router + loadBalancer._useSeedRouter = false; loadBalancer.acquireConnection(READ).then(connection1 => { expect(connection1.address).toEqual(serverC); From 27cd5c1fc2e1ae0bfd711280a4b677bd5227038e Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 16:20:08 +0100 Subject: [PATCH 06/12] Clean-up connections failed on acquire --- src/v1/driver.js | 2 ++ test/v1/driver.test.js | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/v1/driver.js b/src/v1/driver.js index bf4ddcb17..7ead03a28 100644 --- a/src/v1/driver.js +++ b/src/v1/driver.js @@ -144,6 +144,8 @@ class Driver { // notify Driver.onError callback about connection initialization errors this.onError(error); } + // let's destroy this connection + this._destroyConnection(connection); // propagate the error because connection failed to connect / initialize throw error; }); diff --git a/test/v1/driver.test.js b/test/v1/driver.test.js index a01ae9042..5f507f3e9 100644 --- a/test/v1/driver.test.js +++ b/test/v1/driver.test.js @@ -79,6 +79,20 @@ describe('driver', () => { startNewTransaction(driver); }, 10000); + it('should destroy failed connections', done => { + // Given + driver = neo4j.driver('bolt://local-host', sharedNeo4j.authToken); + + const session = driver.session(); + + session.run('RETURN 1').catch(err => { + expect(driver._openConnections).toEqual({}); + + done(); + }); + }, 10000); + + it('should fail with correct error message when connecting to port 80', done => { if (testUtils.isClient()) { // good error message is not available in browser From c2bd2fae7befb7a74a7223126c66bb426cfeea7c Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 16:20:42 +0100 Subject: [PATCH 07/12] Add a specific test for pool resource counters after purge --- test/internal/pool.test.js | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/internal/pool.test.js b/test/internal/pool.test.js index bbe5714d3..36963ab50 100644 --- a/test/internal/pool.test.js +++ b/test/internal/pool.test.js @@ -187,6 +187,54 @@ describe('Pool', () => { }); }); + it('clears out resource counters even after purge', (done) => { + // Given a pool that allocates + let counter = 0; + const address1 = ServerAddress.fromUrl('bolt://localhost:7687'); + const address2 = ServerAddress.fromUrl('bolt://localhost:7688'); + const pool = new Pool((server, release) => Promise.resolve(new Resource(server, counter++, release)), + res => { + res.destroyed = true; + return true; + } + ); + + // When + const p00 = pool.acquire(address1); + const p01 = pool.acquire(address1); + const p10 = pool.acquire(address2); + const p11 = pool.acquire(address2); + const p12 = pool.acquire(address2); + const pall = Promise.all([p00, p01, p10, p11, p12]).then(values => { + expect(pool.activeResourceCount(address1)).toEqual(2); + expect(pool.activeResourceCount(address2)).toEqual(3); + + expect(pool.has(address1)).toBeTruthy(); + expect(pool.has(address2)).toBeTruthy(); + + values[0].close(); + + expect(pool.activeResourceCount(address1)).toEqual(1); + + pool.purge(address1); + + expect(pool.activeResourceCount(address1)).toEqual(1); + + values[1].close(); + + expect(pool.activeResourceCount(address1)).toEqual(0); + expect(pool.activeResourceCount(address2)).toEqual(3); + + expect(values[0].destroyed).toBeTruthy(); + expect(values[1].destroyed).toBeTruthy(); + + expect(pool.has(address1)).toBeFalsy(); + expect(pool.has(address2)).toBeTruthy(); + + done(); + }); + }); + it('destroys resource when key was purged', (done) => { let counter = 0; const address = ServerAddress.fromUrl('bolt://localhost:7687'); From 8ceca26f032f629168f8ca6974fabb0c6b4e1653 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 16:20:52 +0100 Subject: [PATCH 08/12] Fix test failure --- test/internal/routing-util.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/internal/routing-util.test.js b/test/internal/routing-util.test.js index 7b33f082f..46c8cefd5 100644 --- a/test/internal/routing-util.test.js +++ b/test/internal/routing-util.test.js @@ -25,7 +25,7 @@ import lolex from 'lolex'; import FakeConnection from './fake-connection'; import ServerAddress from '../../src/v1/internal/server-address'; -const ROUTER_ADDRESS = 'test.router.com:4242'; +const ROUTER_ADDRESS = ServerAddress.fromUrl('test.router.com:4242'); describe('RoutingUtil', () => { From 650f99eed59a6dfec2263ffb11d9039ebe014768 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 16:48:59 +0100 Subject: [PATCH 09/12] Remove redundant `that` identifier --- src/v1/internal/connection-providers.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index 7932ef9ba..3327796cd 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -76,10 +76,9 @@ export class LoadBalancer extends ConnectionProvider { } acquireConnection(accessMode) { - let that = this; const connectionPromise = this._freshRoutingTable(accessMode).then(routingTable => { if (accessMode === READ) { - const address = that._loadBalancingStrategy.selectReader(routingTable.readers); + const address = this._loadBalancingStrategy.selectReader(routingTable.readers); return this._acquireConnectionToServer(address, 'read'); } else if (accessMode === WRITE) { const address = this._loadBalancingStrategy.selectWriter(routingTable.writers); From 748be194321e57e01225a0dec3b135fa36b6deee Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 17:14:06 +0100 Subject: [PATCH 10/12] Update neo4j branch version to be -e 3.5 --- test/internal/shared-neo4j.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/internal/shared-neo4j.js b/test/internal/shared-neo4j.js index 7c5cfe355..17ec0b972 100644 --- a/test/internal/shared-neo4j.js +++ b/test/internal/shared-neo4j.js @@ -118,7 +118,7 @@ const additionalConfig = { }; const neoCtrlVersionParam = '-e'; -const defaultNeo4jVersion = '3.4.6'; +const defaultNeo4jVersion = '3.5'; const defaultNeoCtrlArgs = `${neoCtrlVersionParam} ${defaultNeo4jVersion}`; function neo4jCertPath(dir) { From 0396124e25c5a8d3f93fd7dc86dfa219df3dd4c4 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 20:02:29 +0100 Subject: [PATCH 11/12] Skip test on versions later than 4.0 --- src/v1/internal/server-version.js | 2 ++ test/v1/driver.test.js | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/v1/internal/server-version.js b/src/v1/internal/server-version.js index f9c61bb8f..c168d035d 100644 --- a/src/v1/internal/server-version.js +++ b/src/v1/internal/server-version.js @@ -113,6 +113,7 @@ const VERSION_3_1_0 = new ServerVersion(3, 1, 0); const VERSION_3_2_0 = new ServerVersion(3, 2, 0); const VERSION_3_4_0 = new ServerVersion(3, 4, 0); const VERSION_3_5_0 = new ServerVersion(3, 5, 0); +const VERSION_4_0_0 = new ServerVersion(4, 0, 0); const maxVer = Number.MAX_SAFE_INTEGER; const VERSION_IN_DEV = new ServerVersion(maxVer, maxVer, maxVer); @@ -122,6 +123,7 @@ export { VERSION_3_2_0, VERSION_3_4_0, VERSION_3_5_0, + VERSION_4_0_0, VERSION_IN_DEV }; diff --git a/test/v1/driver.test.js b/test/v1/driver.test.js index 5f507f3e9..250ab978b 100644 --- a/test/v1/driver.test.js +++ b/test/v1/driver.test.js @@ -22,7 +22,7 @@ import sharedNeo4j from '../internal/shared-neo4j'; import FakeConnection from '../internal/fake-connection'; import lolex from 'lolex'; import {DEFAULT_ACQUISITION_TIMEOUT, DEFAULT_MAX_SIZE} from '../../src/v1/internal/pool-config'; -import {ServerVersion, VERSION_3_1_0} from '../../src/v1/internal/server-version'; +import { ServerVersion, VERSION_3_1_0, VERSION_4_0_0 } from '../../src/v1/internal/server-version'; import testUtils from '../internal/test-utils'; describe('driver', () => { @@ -201,6 +201,10 @@ describe('driver', () => { }); it('should fail nicely when connecting with routing to standalone server', done => { + if (serverVersion.compareTo(VERSION_4_0_0) >= 0) { + done(); + } + // Given driver = neo4j.driver("bolt+routing://localhost", sharedNeo4j.authToken); From 34051df1ebd858f06a7e8e77f52d9ba61a28101b Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 30 Apr 2019 20:40:37 +0100 Subject: [PATCH 12/12] Don't continue execution when test is ignored --- test/v1/driver.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/v1/driver.test.js b/test/v1/driver.test.js index 250ab978b..d15f4e7f0 100644 --- a/test/v1/driver.test.js +++ b/test/v1/driver.test.js @@ -203,6 +203,7 @@ describe('driver', () => { it('should fail nicely when connecting with routing to standalone server', done => { if (serverVersion.compareTo(VERSION_4_0_0) >= 0) { done(); + return; } // Given @@ -412,6 +413,7 @@ describe('driver', () => { if (serverVersion.compareTo(VERSION_3_1_0) < 0) { // IPv6 listen address only supported starting from neo4j 3.1, so let's ignore the rest done(); + return; } driver = neo4j.driver(url, sharedNeo4j.authToken);