From b2e18344d9b19768525a043adbbe133f534a756f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 28 May 2017 07:15:20 +0200 Subject: [PATCH] chore: mark private variables as such and remove obsolete ones --- changelog.md | 3 ++ index.js | 76 ++++++++++++++++++++--------------- lib/connect.js | 32 ++++++++------- lib/individualCommands.js | 36 ++++++++--------- lib/multi.js | 9 ++--- lib/offlineCommand.js | 4 +- lib/pubsub.js | 30 +++++++------- lib/readyHandler.js | 24 +++++------ lib/reconnect.js | 6 +-- lib/replyHandler.js | 12 +++--- lib/utils.js | 2 +- lib/writeCommands.js | 8 ++-- test/auth.spec.js | 9 ++--- test/commands/client.spec.js | 24 +++++------ test/commands/monitor.spec.js | 12 +++--- test/connection.spec.js | 21 +++++----- test/helper.js | 2 +- test/node_redis.spec.js | 24 +++++------ test/pubsub.spec.js | 2 +- test/return_buffers.spec.js | 2 +- test/tls.spec.js | 6 +-- 21 files changed, 179 insertions(+), 165 deletions(-) diff --git a/changelog.md b/changelog.md index d9e1fa8b34..d87157fd7e 100644 --- a/changelog.md +++ b/changelog.md @@ -17,6 +17,9 @@ It will not restore the support for old Node.js versions, the return value of connectTimeout behavior. It will also only partially restore snake_case support and maybe more. +Bugfixes +- Fixed auth in batch not saving the password + Features - Native promise support - Auto pipelining diff --git a/index.js b/index.js index d966e97b7e..3218bd7121 100644 --- a/index.js +++ b/index.js @@ -59,9 +59,16 @@ class RedisClient extends EventEmitter { this.address = `${cnxOptions.host}:${cnxOptions.port}` } - this.connectionOptions = cnxOptions - this.connectionId = RedisClient.connectionId++ + // Public Variables this.connected = false + this.shouldBuffer = false + this.commandQueue = new Queue() // Holds sent commands to de-pipeline them + this.offlineQueue = new Queue() // Holds commands issued but not able to be sent + this.serverInfo = {} + + // Private Variables + this._connectionOptions = cnxOptions + this.connectionId = RedisClient.connectionId++ if (options.socketKeepalive === undefined) { options.socketKeepalive = true } @@ -72,6 +79,14 @@ class RedisClient extends EventEmitter { } options.returnBuffers = !!options.returnBuffers options.detectBuffers = !!options.detectBuffers + if (typeof options.enableOfflineQueue !== 'boolean') { + if (options.enableOfflineQueue !== undefined) { + throw new TypeError('enableOfflineQueue must be a boolean') + } + options.enableOfflineQueue = true + } + // Only used as timeout until redis has to be connected to redis until throwing an connection error + options.connectTimeout = +options.connectTimeout || 60000 // 60 * 1000 ms // Override the detectBuffers setting if returnBuffers is active and print a warning if (options.returnBuffers && options.detectBuffers) { process.nextTick( @@ -81,31 +96,27 @@ class RedisClient extends EventEmitter { ) options.detectBuffers = false } - this.shouldBuffer = false - this.commandQueue = new Queue() // Holds sent commands to de-pipeline them - this.offlineQueue = new Queue() // Holds commands issued but not able to be sent this._pipelineQueue = new Queue() // Holds all pipelined commands - // Only used as timeout until redis has to be connected to redis until throwing an connection error - this.connectTimeout = +options.connectTimeout || 60000 // 60 * 1000 ms - this.enableOfflineQueue = options.enableOfflineQueue !== false - this.pubSubMode = 0 - this.subscriptionSet = {} - this.monitoring = false + this._pubSubMode = 0 + this._subscriptionSet = {} + this._monitoring = false this.messageBuffers = false - this.closing = false - this.serverInfo = {} - this.authPass = options.authPass || options.password + this._closing = false + if (options.authPass) { + if (options.password) { + throw new TypeError('The "password" and "authPass" option may not both be set at the same time.') + } + options.password = options.authPass + } this.selectedDb = options.db // Save the selected db here, used when reconnecting - this.oldState = null this._strCache = '' this._pipeline = false - this.subCommandsLeft = 0 - this.renameCommands = options.renameCommands || {} + this._subCommandsLeft = 0 this.timesConnected = 0 this.buffers = options.returnBuffers || options.detectBuffers - this.options = options + this._options = options this._multi = false - this.reply = 'ON' // Returning replies is the default + this._reply = 'ON' // Returning replies is the default this.retryStrategy = options.retryStrategy || function (options) { if (options.attempt > 100) { return @@ -113,8 +124,8 @@ class RedisClient extends EventEmitter { // reconnect after return Math.min(options.attempt * 100, 3000) } - this.retryStrategyProvided = !!options.retryStrategy - this.subscribeChannels = [] + this._retryStrategyProvided = !!options.retryStrategy + this._subscribeChannels = [] utils.setReconnectDefaults(this) // Init parser and connect connect(this) @@ -128,6 +139,7 @@ class RedisClient extends EventEmitter { // Do not call internalSendCommand directly, if you are not absolutely certain it handles everything properly // e.g. monitor / info does not work with internalSendCommand only + // TODO: Move this function out of the client as a private function internalSendCommand (commandObj) { if (this.ready === false || this._stream.writable === false) { // Handle offline commands right away @@ -143,16 +155,16 @@ class RedisClient extends EventEmitter { // Handle `CLIENT REPLY ON|OFF|SKIP` // This has to be checked after callOnWrite /* istanbul ignore else: TODO: Remove this as soon as we test Redis 3.2 on travis */ - if (this.reply === 'ON') { + if (this._reply === 'ON') { this.commandQueue.push(commandObj) } else { // Do not expect a reply // Does this work in combination with the pub sub mode? utils.replyInOrder(this, commandObj.callback, null, undefined, this.commandQueue) - if (this.reply === 'SKIP') { - this.reply = 'SKIP_ONE_MORE' - } else if (this.reply === 'SKIP_ONE_MORE') { - this.reply = 'ON' + if (this._reply === 'SKIP') { + this._reply = 'SKIP_ONE_MORE' + } else if (this._reply === 'SKIP_ONE_MORE') { + this._reply = 'ON' } } return commandObj.promise @@ -202,7 +214,7 @@ class RedisClient extends EventEmitter { this._stream.on('error', noop) this.connected = false this.ready = false - this.closing = true + this._closing = true return this._stream.destroySoon() } @@ -212,9 +224,7 @@ class RedisClient extends EventEmitter { this._stream.unref() } else { debug('Not connected yet, will unref later') - this.once('connect', function () { - this.unref() - }) + this.once('connect', () => this._stream.unref()) } } @@ -224,7 +234,7 @@ class RedisClient extends EventEmitter { callback = options options = null } - const existingOptions = utils.clone(this.options) + const existingOptions = utils.clone(this._options) options = utils.clone(options) for (const elem in options) { if (options.hasOwnProperty(elem)) { @@ -234,11 +244,11 @@ class RedisClient extends EventEmitter { const client = new RedisClient(existingOptions) client.selectedDb = this.selectedDb if (typeof callback === 'function') { - const errorListener = function (err) { + const errorListener = (err) => { callback(err) client.end(true) } - const readyListener = function () { + const readyListener = () => { callback(null, client) client.removeAllListeners(errorListener) } diff --git a/lib/connect.js b/lib/connect.js index 9bfbe84834..039bbb4f0c 100644 --- a/lib/connect.js +++ b/lib/connect.js @@ -16,7 +16,7 @@ var reconnect = function (client, why, err) { } function onStreamError (client, err) { - if (client.closing) { + if (client._closing) { return } @@ -26,7 +26,7 @@ function onStreamError (client, err) { client.ready = false // Only emit the error if the retryStrategy option is not set - if (client.retryStrategyProvided === false) { + if (client._retryStrategyProvided === false) { client.emit('error', err) } // 'error' events get turned into exceptions if they aren't listened for. If the user handled this error @@ -61,7 +61,7 @@ function createParser (client) { setImmediate(() => client.emit('error', err)) }, returnBuffers: client.buffers || client.messageBuffers, - stringNumbers: client.options.stringNumbers || false + stringNumbers: client._options.stringNumbers || false }) } @@ -78,12 +78,12 @@ function connect (client) { const parser = createParser(client) client._replyParser = parser - if (client.options.stream) { + if (client._options.stream) { // Only add the listeners once in case of a reconnect try (that won't work) if (client._stream) { return } - client._stream = client.options.stream + client._stream = client._options.stream } else { // On a reconnect destroy the former stream and retry if (client._stream) { @@ -92,25 +92,25 @@ function connect (client) { } /* istanbul ignore if: travis does not work with stunnel atm. Therefore the tls tests are skipped on travis */ - if (client.options.tls) { - client._stream = tls.connect(client.connectionOptions) + if (client._options.tls) { + client._stream = tls.connect(client._connectionOptions) } else { - client._stream = net.createConnection(client.connectionOptions) + client._stream = net.createConnection(client._connectionOptions) } } const stream = client._stream - if (client.options.connectTimeout) { + if (client._options.connectTimeout) { // TODO: Investigate why this is not properly triggered - stream.setTimeout(client.connectTimeout, () => { + stream.setTimeout(client._options.connectTimeout, () => { // Note: This is only tested if a internet connection is established reconnect(client, 'timeout') }) } /* istanbul ignore next: travis does not work with stunnel atm. Therefore the tls tests are skipped on travis */ - const connectEvent = client.options.tls ? 'secureConnect' : 'connect' + const connectEvent = client._options.tls ? 'secureConnect' : 'connect' stream.once(connectEvent, () => { stream.removeAllListeners('timeout') client.timesConnected++ @@ -142,11 +142,13 @@ function connect (client) { stream.setNoDelay() - // Fire the command before redis is connected to be sure it's the first fired command - if (client.authPass !== undefined) { + // Fire the command before redis is connected to be sure it's the first fired command. + // TODO: Consider calling the ready check before Redis is connected as well. + // That could improve the ready performance. Measure the rough time difference! + if (client._options.password !== undefined) { client.ready = true - client.auth(client.authPass).catch((err) => { - client.closing = true + client.auth(client._options.password).catch((err) => { + client._closing = true process.nextTick(() => { client.emit('error', err) client.end(true) diff --git a/lib/individualCommands.js b/lib/individualCommands.js index 7b9f8835ef..c1cffc331d 100644 --- a/lib/individualCommands.js +++ b/lib/individualCommands.js @@ -46,7 +46,7 @@ RedisClient.prototype.monitor = function monitor () { const callOnWrite = () => { // Activating monitor mode has to happen before Redis returned the callback. The monitor result is returned first. // Therefore we expect the command to be properly processed. If this is not the case, it's not an issue either. - this.monitoring = true + this._monitoring = true } return this.internalSendCommand(new Command('monitor', [], callOnWrite)) } @@ -54,16 +54,16 @@ RedisClient.prototype.monitor = function monitor () { // Only works with batch, not in a transaction Multi.prototype.monitor = function monitor () { // Use a individual command, as this is a special case that does not has to be checked for any other command - if (this.exec !== this.execTransaction) { + if (this._type !== 'multi') { const callOnWrite = () => { - this._client.monitoring = true + this._client._monitoring = true } this._queue.push(new Command('monitor', [], callOnWrite)) return this } // Set multi monitoring to indicate the exec that it should abort // Remove this "hack" as soon as Redis might fix this - this.monitoring = true + this._monitoring = true return this } @@ -91,7 +91,7 @@ RedisClient.prototype.quit = function quit () { // this.ready = this.offlineQueue.length === 0; const backpressureIndicator = this.internalSendCommand(new Command('quit', [], null, quitCallback(this))) // Calling quit should always end the connection, no matter if there's a connection or not - this.closing = true + this._closing = true this.ready = false return backpressureIndicator } @@ -100,7 +100,7 @@ RedisClient.prototype.quit = function quit () { Multi.prototype.quit = function quit () { const callOnWrite = () => { // If called in a multi context, we expect redis is available - this._client.closing = true + this._client._closing = true this._client.ready = false } this._queue.push(new Command('quit', [], null, quitCallback(this._client), callOnWrite)) @@ -198,7 +198,7 @@ RedisClient.prototype.auth = function auth (pass) { debug('Sending auth to %s id %s', this.address, this.connectionId) // Stash auth for connect and reconnect. - this.authPass = pass + this._options.password = pass const ready = this.ready this.ready = ready || this.offlineQueue.length === 0 const tmp = this.internalSendCommand(new Command('auth', [pass], null, authCallback(this, pass))) @@ -211,7 +211,7 @@ Multi.prototype.auth = function auth (pass) { debug('Sending auth to %s id %s', this.address, this.connectionId) // Stash auth for connect and reconnect. - this.authPass = pass + this._client._options.password = pass this._queue.push(new Command('auth', [pass], null, authCallback(this._client))) return this } @@ -229,7 +229,7 @@ RedisClient.prototype.client = function client () { const replyOnOff = arr[1].toString().toUpperCase() if (replyOnOff === 'ON' || replyOnOff === 'OFF' || replyOnOff === 'SKIP') { callOnWrite = () => { - this.reply = replyOnOff + this._reply = replyOnOff } } } @@ -249,7 +249,7 @@ Multi.prototype.client = function client () { const replyOnOff = arr[1].toString().toUpperCase() if (replyOnOff === 'ON' || replyOnOff === 'OFF' || replyOnOff === 'SKIP') { callOnWrite = () => { - this._client.reply = replyOnOff + this._client._reply = replyOnOff } } } @@ -264,7 +264,7 @@ RedisClient.prototype.subscribe = function subscribe () { arr[i] = arguments[i] } const callOnWrite = () => { - this.pubSubMode = this.pubSubMode || this.commandQueue.length + 1 + this._pubSubMode = this._pubSubMode || this.commandQueue.length + 1 } return this.internalSendCommand(new Command('subscribe', arr, callOnWrite)) } @@ -276,7 +276,7 @@ Multi.prototype.subscribe = function subscribe () { arr[i] = arguments[i] } const callOnWrite = () => { - this._client.pubSubMode = this._client.pubSubMode || this._client.commandQueue.length + 1 + this._client._pubSubMode = this._client._pubSubMode || this._client.commandQueue.length + 1 } this._queue.push(new Command('subscribe', arr, callOnWrite)) return this @@ -290,7 +290,7 @@ RedisClient.prototype.unsubscribe = function unsubscribe () { } const callOnWrite = () => { // Pub sub has to be activated even if not in pub sub mode, as the return value is manipulated in the callback - this.pubSubMode = this.pubSubMode || this.commandQueue.length + 1 + this._pubSubMode = this._pubSubMode || this.commandQueue.length + 1 } return this.internalSendCommand(new Command('unsubscribe', arr, callOnWrite)) } @@ -303,7 +303,7 @@ Multi.prototype.unsubscribe = function unsubscribe () { } const callOnWrite = () => { // Pub sub has to be activated even if not in pub sub mode, as the return value is manipulated in the callback - this._client.pubSubMode = this._client.pubSubMode || this._client.commandQueue.length + 1 + this._client._pubSubMode = this._client._pubSubMode || this._client.commandQueue.length + 1 } this._queue.push(new Command('unsubscribe', arr, callOnWrite)) return this @@ -316,7 +316,7 @@ RedisClient.prototype.psubscribe = function psubscribe () { arr[i] = arguments[i] } const callOnWrite = () => { - this.pubSubMode = this.pubSubMode || this.commandQueue.length + 1 + this._pubSubMode = this._pubSubMode || this.commandQueue.length + 1 } return this.internalSendCommand(new Command('psubscribe', arr, callOnWrite)) } @@ -328,7 +328,7 @@ Multi.prototype.psubscribe = function psubscribe () { arr[i] = arguments[i] } const callOnWrite = () => { - this._client.pubSubMode = this._client.pubSubMode || this._client.commandQueue.length + 1 + this._client._pubSubMode = this._client._pubSubMode || this._client.commandQueue.length + 1 } this._queue.push(new Command('psubscribe', arr, callOnWrite)) return this @@ -342,7 +342,7 @@ RedisClient.prototype.punsubscribe = function punsubscribe () { } const callOnWrite = () => { // Pub sub has to be activated even if not in pub sub mode, as the return value is manipulated in the callback - this.pubSubMode = this.pubSubMode || this.commandQueue.length + 1 + this._pubSubMode = this._pubSubMode || this.commandQueue.length + 1 } return this.internalSendCommand(new Command('punsubscribe', arr, callOnWrite)) } @@ -355,7 +355,7 @@ Multi.prototype.punsubscribe = function punsubscribe () { } const callOnWrite = () => { // Pub sub has to be activated even if not in pub sub mode, as the return value is manipulated in the callback - this._client.pubSubMode = this._client.pubSubMode || this._client.commandQueue.length + 1 + this._client._pubSubMode = this._client._pubSubMode || this._client.commandQueue.length + 1 } this._queue.push(new Command('punsubscribe', arr, callOnWrite)) return this diff --git a/lib/multi.js b/lib/multi.js index 27aa21932d..016662e003 100644 --- a/lib/multi.js +++ b/lib/multi.js @@ -22,7 +22,7 @@ function pipelineTransactionCommand (multi, command, index) { if (err) { tmp(err) err.position = index - multi.errors.push(err) + multi._errors.push(err) return } tmp(null, reply) @@ -73,7 +73,7 @@ function multiCallback (multi, replies) { function execTransaction (multi) { const client = multi._client const queue = multi._queue - if (multi.monitoring || client.monitoring) { + if (multi._monitoring || client._monitoring) { const err = new RangeError( 'Using transaction with a client that is in monitor mode does not work due to faulty return values of Redis.' ) @@ -87,9 +87,8 @@ function execTransaction (multi) { }) } const len = queue.length - multi.errors = [] + multi._errors = [] client._multi = true - multi.wantsBuffers = new Array(len) // Silently ignore this error. We'll receive the error for the exec as well const promises = [client.internalSendCommand(new Command('multi', [])).catch(() => {})] // Drain queue, callback will catch 'QUEUED' or error @@ -100,7 +99,7 @@ function execTransaction (multi) { const main = client.internalSendCommand(new Command('exec', [])) return Promise.all(promises).then(() => main.then((replies) => multiCallback(multi, replies)).catch((err) => { - err.errors = multi.errors + err.errors = multi._errors return Promise.reject(err) })) } diff --git a/lib/offlineCommand.js b/lib/offlineCommand.js index 55d44bdf33..ec5821efb1 100644 --- a/lib/offlineCommand.js +++ b/lib/offlineCommand.js @@ -6,8 +6,8 @@ const utils = require('./utils') function offlineCommand (client, command) { const commandName = command.command.toUpperCase() - if (client.closing || !client.enableOfflineQueue) { - const msg = client.closing === true + if (client._closing || client._options.enableOfflineQueue === false) { + const msg = client._closing === true ? 'The connection is already closed.' : client._stream.writable === true ? 'The connection is not yet established and the offline queue is deactivated.' diff --git a/lib/pubsub.js b/lib/pubsub.js index 2b4d3ee74b..f4b593302d 100644 --- a/lib/pubsub.js +++ b/lib/pubsub.js @@ -12,7 +12,7 @@ function subscribeUnsubscribe (client, reply, type) { // Subscribe commands take an optional callback and also emit an event, but only the Last_ response is included in the callback // The pub sub commands return each argument in a separate return value and have to be handled that way const commandObj = client.commandQueue.get(0) - const buffer = client.options.returnBuffers || client.options.detectBuffers && commandObj.bufferArgs + const buffer = client._options.returnBuffers || client._options.detectBuffers && commandObj.bufferArgs const channel = (buffer || reply[1] === null) ? reply[1] : reply[1].toString() const count = +reply[2] // Return the channel counter as number no matter if `stringNumbers` is activated or not debug(type, channel) @@ -20,38 +20,38 @@ function subscribeUnsubscribe (client, reply, type) { // Emit first, then return the callback if (channel !== null) { // Do not emit or "unsubscribe" something if there was no channel to unsubscribe from if (type === 'subscribe' || type === 'psubscribe') { - client.subscriptionSet[`${type}_${channel}`] = channel + client._subscriptionSet[`${type}_${channel}`] = channel } else { const innerType = type === 'unsubscribe' ? 'subscribe' : 'psubscribe' // Make types consistent - delete client.subscriptionSet[`${innerType}_${channel}`] + delete client._subscriptionSet[`${innerType}_${channel}`] } client.emit(type, channel, count) - client.subscribeChannels.push(channel) + client._subscribeChannels.push(channel) } - if (commandObj.argsLength === 1 || client.subCommandsLeft === 1 || commandObj.argsLength === 0 && (count === 0 || channel === null)) { + if (commandObj.argsLength === 1 || client._subCommandsLeft === 1 || commandObj.argsLength === 0 && (count === 0 || channel === null)) { if (count === 0) { // Unsubscribed from all channels var runningCommand var i = 1 - client.pubSubMode = 0 // Deactivating pub sub mode + client._pubSubMode = 0 // Deactivating pub sub mode // This should be a rare case and therefore handling it this way should be good performance wise for the general case for (runningCommand = client.commandQueue.get(i); runningCommand !== undefined; runningCommand = client.commandQueue.get(i)) { if (SUBSCRIBE_COMMANDS[runningCommand.command]) { - client.pubSubMode = i // Entering pub sub mode again + client._pubSubMode = i // Entering pub sub mode again break } i++ } } client.commandQueue.shift() - commandObj.callback(null, [count, client.subscribeChannels]) - client.subscribeChannels = [] - client.subCommandsLeft = 0 + commandObj.callback(null, [count, client._subscribeChannels]) + client._subscribeChannels = [] + client._subCommandsLeft = 0 } else { - if (client.subCommandsLeft !== 0) { - client.subCommandsLeft-- + if (client._subCommandsLeft !== 0) { + client._subCommandsLeft-- } else { - client.subCommandsLeft = commandObj.argsLength ? commandObj.argsLength - 1 : count + client._subCommandsLeft = commandObj.argsLength ? commandObj.argsLength - 1 : count } } } @@ -59,14 +59,14 @@ function subscribeUnsubscribe (client, reply, type) { function returnPubSub (client, reply) { const type = reply[0].toString() if (type === 'message') { // Channel, message - if (!client.options.returnBuffers || client.messageBuffers) { // Backwards compatible. Refactor this in v.3 to always return a string on the normal emitter + if (!client._options.returnBuffers || client.messageBuffers) { // Backwards compatible. Refactor this in v.3 to always return a string on the normal emitter client.emit('message', reply[1].toString(), reply[2].toString()) client.emit('messageBuffer', reply[1], reply[2]) } else { client.emit('message', reply[1], reply[2]) } } else if (type === 'pmessage') { // Pattern, channel, message - if (!client.options.returnBuffers || client.messageBuffers) { // Backwards compatible. Refactor this in v.3 to always return a string on the normal emitter + if (!client._options.returnBuffers || client.messageBuffers) { // Backwards compatible. Refactor this in v.3 to always return a string on the normal emitter client.emit('pmessage', reply[1].toString(), reply[2].toString(), reply[3].toString()) client.emit('pmessageBuffer', reply[1], reply[2], reply[3]) } else { diff --git a/lib/readyHandler.js b/lib/readyHandler.js index e641e9bdd2..a3b3af2dce 100644 --- a/lib/readyHandler.js +++ b/lib/readyHandler.js @@ -11,14 +11,14 @@ function onConnect (client) { // fast properties. If that's not the case, make them fast properties // again! client.connected = true - client._stream.setKeepAlive(client.options.socketKeepalive) + client._stream.setKeepAlive(client._options.socketKeepalive) client._stream.setTimeout(0) // TODO: Deprecate the connect event. client.emit('connect') utils.setReconnectDefaults(client) - if (client.options.noReadyCheck) { + if (client._options.noReadyCheck) { readyHandler(client) } else { readyCheck(client) @@ -53,21 +53,21 @@ function readyHandler (client) { if (client.selectedDb !== undefined) { client.internalSendCommand(new Command('select', [client.selectedDb])).catch((err) => { - if (!client.closing) { + if (!client._closing) { // TODO: These internal things should be wrapped in a // special error that describes what is happening process.nextTick(client.emit, 'error', err) } }) } - if (client.monitoring) { // Monitor has to be fired before pub sub commands + if (client._monitoring) { // Monitor has to be fired before pub sub commands client.internalSendCommand(new Command('monitor', [])).catch((err) => { - if (!client.closing) { + if (!client._closing) { process.nextTick(client.emit, 'error', err) } }) } - const callbackCount = Object.keys(client.subscriptionSet).length + const callbackCount = Object.keys(client._subscriptionSet).length // TODO: Replace the disableResubscribing by a individual function that may be called // Add HOOKS!!! // Replace the disableResubscribing by: @@ -77,14 +77,14 @@ function readyHandler (client) { // subscriptions: true, // // individual: function noop () {} // } - if (!client.options.disableResubscribing && callbackCount) { + if (!client._options.disableResubscribing && callbackCount) { debug('Sending pub/sub commands') - for (const key in client.subscriptionSet) { - if (client.subscriptionSet.hasOwnProperty(key)) { + for (const key in client._subscriptionSet) { + if (client._subscriptionSet.hasOwnProperty(key)) { const command = key.slice(0, key.indexOf('_')) - const args = client.subscriptionSet[key] + const args = client._subscriptionSet[key] client[command]([args]).catch((err) => { - if (!client.closing) { + if (!client._closing) { process.nextTick(client.emit, 'error', err) } }) @@ -133,7 +133,7 @@ function readyCheck (client) { debug('Redis server still loading, trying again in %s', retryTime) setTimeout((client) => readyCheck(client), retryTime, client) }).catch((err) => { - if (client.closing) { + if (client._closing) { return } diff --git a/lib/reconnect.js b/lib/reconnect.js index a9fc647bd2..0e69a23b76 100644 --- a/lib/reconnect.js +++ b/lib/reconnect.js @@ -47,7 +47,7 @@ function reconnect (client, why, error) { debug('Redis connection is gone from %s event.', why) client.connected = false client.ready = false - client.pubSubMode = 0 + client._pubSubMode = 0 client.emit('end') @@ -63,7 +63,7 @@ function reconnect (client, why, error) { } // If client is a requested shutdown, then don't retry - if (client.closing) { + if (client._closing) { debug('Connection ended by quit / end command, not retrying.') flushAndError(client, 'Stream connection ended and command aborted.', 'NR_CLOSED', { error @@ -94,7 +94,7 @@ function reconnect (client, why, error) { } // Retry commands after a reconnect instead of throwing an error. Use this with caution - if (client.options.retryUnfulfilledCommands) { + if (client._options.retryUnfulfilledCommands) { client.offlineQueue.unshift.apply(client.offlineQueue, client.commandQueue.toArray()) client.commandQueue.clear() // TODO: If only the pipelineQueue contains the error we could improve the situation. diff --git a/lib/replyHandler.js b/lib/replyHandler.js index 631f529743..f7bac3fa43 100644 --- a/lib/replyHandler.js +++ b/lib/replyHandler.js @@ -15,8 +15,8 @@ function onError (client, err) { } // Count down pub sub mode if in entering modus - if (client.pubSubMode > 1) { - client.pubSubMode-- + if (client._pubSubMode > 1) { + client._pubSubMode-- } const match = err.message.match(utils.errCode) @@ -40,7 +40,7 @@ function onResult (client, reply) { // If in monitor mode, all normal commands are still working and we only want to emit the streamlined commands // As this is not the average use case and monitor is expensive anyway, let's change the code here, to improve // the average performance of all other commands in case of no monitor mode - if (client.monitoring) { + if (client._monitoring) { var replyStr if (client.buffers && Buffer.isBuffer(reply)) { replyStr = reply.toString() @@ -58,10 +58,10 @@ function onResult (client, reply) { return } } - if (client.pubSubMode === 0) { + if (client._pubSubMode === 0) { normalReply(client, reply) - } else if (client.pubSubMode !== 1) { - client.pubSubMode-- + } else if (client._pubSubMode !== 1) { + client._pubSubMode-- normalReply(client, reply) } else if (!(reply instanceof Array) || reply.length <= 2) { // Only PING and QUIT are allowed in this context besides the pub sub commands diff --git a/lib/utils.js b/lib/utils.js index 5fea2a008a..41d5ab55c0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -139,7 +139,7 @@ function warn (client, msg) { * @returns {string|number|null|Buffer|any[]|object} */ function handleReply (client, reply, command) { - if (client.options.detectBuffers === true && command.bufferArgs === false) { // client.messageBuffers + if (client._options.detectBuffers === true && command.bufferArgs === false) { // client.messageBuffers reply = replyToStrings(reply) } diff --git a/lib/writeCommands.js b/lib/writeCommands.js index 1cc9f2e380..6eab6c3c17 100644 --- a/lib/writeCommands.js +++ b/lib/writeCommands.js @@ -128,8 +128,8 @@ function returnErr (client, command) { function normalizeAndWrite (client, command) { const args = command.args const origName = command.command - const renameCommands = client.renameCommands - const name = renameCommands[origName] !== undefined + const renameCommands = client._options.renameCommands + const name = renameCommands !== undefined && renameCommands[origName] !== undefined ? renameCommands[origName] : origName @@ -142,12 +142,12 @@ function normalizeAndWrite (client, command) { return returnErr(client, command) } - if (typeof client.options.prefix === 'string') { + if (typeof client._options.prefix === 'string') { const prefixKeys = Commands.getKeyIndexes(origName, copy) prefixKeys.forEach((i) => { // Attention it would be to expensive to detect if the input is non utf8 Buffer // In that case the prefix *might* destroys user information - copy[i] = client.options.prefix + copy[i] + copy[i] = client._options.prefix + copy[i] }) } diff --git a/test/auth.spec.js b/test/auth.spec.js index 838536b599..6c78bafb68 100644 --- a/test/auth.spec.js +++ b/test/auth.spec.js @@ -72,9 +72,8 @@ if (process.platform !== 'win32') { if (helper.redisProcess().spawnFailed()) this.skip() client = redis.createClient(`redis://${config.HOST[ip]}:${config.PORT}?db=2&password=${auth}`) - assert.strictEqual(client.options.db, '2') - assert.strictEqual(client.options.password, auth) - assert.strictEqual(client.authPass, auth) + assert.strictEqual(client._options.db, '2') + assert.strictEqual(client._options.password, auth) client.on('ready', () => { const promises = [] // Set a key so the used database is returned in the info command @@ -209,7 +208,7 @@ if (process.platform !== 'win32') { client = redis.createClient.apply(null, args) client.set('foo', 'bar') client.subscribe('somechannel', 'another channel').then(() => { - assert.strictEqual(client.pubSubMode, 1) + assert.strictEqual(client._pubSubMode, 1) client.once('ready', () => { client.get('foo').catch((err) => { assert(/ERR only \(P\)SUBSCRIBE \/ \(P\)UNSUBSCRIBE/.test(err.message)) @@ -219,7 +218,7 @@ if (process.platform !== 'win32') { }) client.once('ready', () => { // Coherent behavior with all other offline commands fires commands before emitting but does not wait till they return - assert.strictEqual(client.pubSubMode, 2) + assert.strictEqual(client._pubSubMode, 2) client.ping().then(() => { // Make sure all commands were properly processed already client._stream.destroy() }) diff --git a/test/commands/client.spec.js b/test/commands/client.spec.js index 0accb0155c..5ad65c36c4 100644 --- a/test/commands/client.spec.js +++ b/test/commands/client.spec.js @@ -46,27 +46,27 @@ describe('The \'client\' method', () => { describe('as normal command', () => { it('on', function () { helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') const promises = [client.client('reply', 'on').then(helper.isString('OK'))] - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') promises.push(client.set('foo', 'bar')) return Promise.all(promises) }) it('off', function () { helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') const promises = [client.client(Buffer.from('REPLY'), 'OFF').then(helper.isUndefined())] - assert.strictEqual(client.reply, 'OFF') + assert.strictEqual(client._reply, 'OFF') promises.push(client.set('foo', 'bar').then(helper.isUndefined())) return Promise.all(promises) }) it('skip', function () { helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') const promises = [client.client('REPLY', Buffer.from('SKIP')).then(helper.isUndefined())] - assert.strictEqual(client.reply, 'SKIP_ONE_MORE') + assert.strictEqual(client._reply, 'SKIP_ONE_MORE') promises.push(client.set('foo', 'bar').then(helper.isUndefined())) promises.push(client.get('foo').then(helper.isString('bar'))) return Promise.all(promises) @@ -77,9 +77,9 @@ describe('The \'client\' method', () => { it('on', function () { helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) const batch = client.batch() - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') batch.client('reply', 'on') - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') batch.set('foo', 'bar') return batch.exec().then(helper.isDeepEqual(['OK', 'OK'])) }) @@ -87,20 +87,20 @@ describe('The \'client\' method', () => { it('off', function () { helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) const batch = client.batch() - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') batch.set('hello', 'world') batch.client(Buffer.from('REPLY'), Buffer.from('OFF')) batch.get('hello') batch.get('hello') return batch.exec().then((res) => { - assert.strictEqual(client.reply, 'OFF') + assert.strictEqual(client._reply, 'OFF') assert.deepStrictEqual(res, ['OK', undefined, undefined, undefined]) }) }) it('skip', function () { helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') return client.batch() .set('hello', 'world') .client('REPLY', 'SKIP') @@ -108,7 +108,7 @@ describe('The \'client\' method', () => { .get('foo') .exec() .then((res) => { - assert.strictEqual(client.reply, 'ON') + assert.strictEqual(client._reply, 'ON') assert.deepStrictEqual(res, ['OK', undefined, undefined, 'bar']) }) }) diff --git a/test/commands/monitor.spec.js b/test/commands/monitor.spec.js index 1ef792f127..4a9ddf5d0d 100644 --- a/test/commands/monitor.spec.js +++ b/test/commands/monitor.spec.js @@ -62,7 +62,7 @@ describe('The \'monitor\' method', () => { }) monitorClient.on('monitor', (time, args, rawOutput) => { - assert.strictEqual(monitorClient.monitoring, true) + assert.strictEqual(monitorClient._monitoring, true) assert.deepStrictEqual(args, responses.shift()) assert(utils.monitorRegex.test(rawOutput), rawOutput) if (responses.length === 0) { @@ -81,7 +81,7 @@ describe('The \'monitor\' method', () => { }) monitorClient.monitor().then((res) => { - assert.strictEqual(monitorClient.monitoring, true) + assert.strictEqual(monitorClient._monitoring, true) assert.strictEqual(res.inspect(), Buffer.from('OK').inspect()) monitorClient.mget('hello', Buffer.from('world')) }) @@ -100,7 +100,7 @@ describe('The \'monitor\' method', () => { client.monitor().then(helper.isString('OK')) client.mget('hello', 'world') client.on('monitor', (time, args, rawOutput) => { - assert.strictEqual(client.monitoring, true) + assert.strictEqual(client._monitoring, true) assert(utils.monitorRegex.test(rawOutput), rawOutput) assert.deepStrictEqual(args, ['mget', 'hello', 'world']) if (called) { @@ -120,7 +120,7 @@ describe('The \'monitor\' method', () => { multi.mget('hello', 'world') multi.exec().then(helper.isDeepEqual(['OK', [null, null]])) client.on('monitor', (time, args, rawOutput) => { - assert.strictEqual(client.monitoring, true) + assert.strictEqual(client._monitoring, true) assert(utils.monitorRegex.test(rawOutput), rawOutput) assert.deepStrictEqual(args, ['mget', 'hello', 'world']) if (called) { @@ -141,12 +141,12 @@ describe('The \'monitor\' method', () => { client._stream.destroy() const end = helper.callFuncAfter(done, 2) client.on('monitor', (time, args, rawOutput) => { - assert.strictEqual(client.monitoring, true) + assert.strictEqual(client._monitoring, true) end() }) client.on('reconnecting', () => { client.get('foo').then((res) => { - assert.strictEqual(client.monitoring, true) + assert.strictEqual(client._monitoring, true) end() }) }) diff --git a/test/connection.spec.js b/test/connection.spec.js index 6ef925bedf..e61ad89360 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -23,8 +23,9 @@ describe('connection tests', () => { // Therefore this is not officially supported! const socket = new net.Socket() client = new redis.RedisClient({ - prefix: 'test' - }, socket) + prefix: 'test', + stream: socket + }) assert.strictEqual(client._stream, socket) assert.strictEqual(client._stream.listeners('error').length, 1) assert.strictEqual(client.address, '"Private stream"') @@ -160,7 +161,7 @@ describe('connection tests', () => { retryStrategy () {} } client = redis.createClient(options) - assert.strictEqual(client.connectionOptions.family, ip === 'IPv6' ? 6 : 4) + assert.strictEqual(client._connectionOptions.family, ip === 'IPv6' ? 6 : 4) assert.strictEqual(Object.keys(options).length, 4) const end = helper.callFuncAfter(done, 2) @@ -246,7 +247,7 @@ describe('connection tests', () => { }) process.nextTick(() => assert.strictEqual(client._stream.listeners('timeout').length, 1)) assert.strictEqual(client.address, '10.255.255.1:6379') - assert.strictEqual(client.connectionOptions.family, 4) + assert.strictEqual(client._connectionOptions.family, 4) client.on('reconnecting', () => { throw new Error('No reconnect, since no connection was ever established') @@ -274,7 +275,7 @@ describe('connection tests', () => { host: '2001:db8::ff00:42:8329' // auto detect ip v6 }) assert.strictEqual(client.address, '2001:db8::ff00:42:8329:6379') - assert.strictEqual(client.connectionOptions.family, 6) + assert.strictEqual(client._connectionOptions.family, 6) process.nextTick(() => { assert.strictEqual(client._stream.listeners('timeout').length, 0) done() @@ -337,7 +338,7 @@ describe('connection tests', () => { it('connects with a port only', (done) => { client = redis.createClient(6379) - assert.strictEqual(client.connectionOptions.family, 4) + assert.strictEqual(client._connectionOptions.family, 4) client.on('error', done) client.once('ready', () => { @@ -433,7 +434,7 @@ describe('connection tests', () => { client = redis.createClient('//127.0.0.1', { connectTimeout: 1000 }) - assert.strictEqual(client.options.connectTimeout, 1000) + assert.strictEqual(client._options.connectTimeout, 1000) client.on('ready', done) }) @@ -441,10 +442,10 @@ describe('connection tests', () => { client = redis.createClient({ url: `http://foo:porkchopsandwiches@${config.HOST[ip]}/3` }) - assert.strictEqual(client.authPass, 'porkchopsandwiches') + assert.strictEqual(client._options.password, 'porkchopsandwiches') assert.strictEqual(+client.selectedDb, 3) - assert(!client.options.port) - assert.strictEqual(client.options.host, config.HOST[ip]) + assert(!client._options.port) + assert.strictEqual(client._options.host, config.HOST[ip]) client.on('ready', done) }) diff --git a/test/helper.js b/test/helper.js index 01cc678e1c..a7ab58cdc2 100644 --- a/test/helper.js +++ b/test/helper.js @@ -201,7 +201,7 @@ module.exports = { }, killConnection (client) { // Change the connection option to a non existing one and destroy the stream - client.connectionOptions = { + client._connectionOptions = { port: 65535, host: '127.0.0.1', family: 4 diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index 8b809eb0e5..36e19be5de 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -77,9 +77,9 @@ describe('The nodeRedis client', () => { assert.strictEqual(client2.selectedDb, 2) assert(client.connected) assert(!client2.connected) - for (const elem in client.options) { - if (client.options.hasOwnProperty(elem)) { - assert.strictEqual(client2.options[elem], client.options[elem]) + for (const elem in client._options) { + if (client._options.hasOwnProperty(elem)) { + assert.strictEqual(client2._options[elem], client._options[elem]) } } client2.on('error', (err) => { @@ -101,13 +101,13 @@ describe('The nodeRedis client', () => { }) assert(client.connected) assert(!client2.connected) - assert.strictEqual(client.options.noReadyCheck, undefined) - assert.strictEqual(client2.options.noReadyCheck, true) - assert.notDeepEqual(client.options, client2.options) - for (const elem in client.options) { - if (client.options.hasOwnProperty(elem)) { + assert.strictEqual(client._options.noReadyCheck, undefined) + assert.strictEqual(client2._options.noReadyCheck, true) + assert.notDeepEqual(client._options, client2._options) + for (const elem in client._options) { + if (client._options.hasOwnProperty(elem)) { if (elem !== 'noReadyCheck') { - assert.strictEqual(client2.options[elem], client.options[elem]) + assert.strictEqual(client2._options[elem], client._options[elem]) } } } @@ -331,17 +331,17 @@ describe('The nodeRedis client', () => { it('reconnects properly when monitoring', (done) => { client.on('reconnecting', function onRecon (params) { client.on('ready', function onReady () { - assert.strictEqual(client.monitoring, true, 'monitoring after reconnect') + assert.strictEqual(client._monitoring, true, 'monitoring after reconnect') client.removeListener('ready', onReady) client.removeListener('reconnecting', onRecon) done() }) }) - assert.strictEqual(client.monitoring, false, 'monitoring off at start') + assert.strictEqual(client._monitoring, false, 'monitoring off at start') client.set('recon 1', 'one') client.monitor().then((res) => { - assert.strictEqual(client.monitoring, true, 'monitoring on after monitor()') + assert.strictEqual(client._monitoring, true, 'monitoring on after monitor()') client.set('recon 2', 'two').then((res) => { // Do not do this in normal programs. This is to simulate the server closing on us. // For orderly shutdown in normal programs, do client.quit() diff --git a/test/pubsub.spec.js b/test/pubsub.spec.js index f96f959d8e..40e99bed3e 100644 --- a/test/pubsub.spec.js +++ b/test/pubsub.spec.js @@ -77,7 +77,7 @@ describe('publish/subscribe', () => { assert.strictEqual(typeof count, 'number') assert.strictEqual(--i, count) if (count === 0) { - assert.deepStrictEqual(sub.subscriptionSet, {}) + assert.deepStrictEqual(sub._subscriptionSet, {}) end() } }) diff --git a/test/return_buffers.spec.js b/test/return_buffers.spec.js index 5b140f32c0..708308bbac 100644 --- a/test/return_buffers.spec.js +++ b/test/return_buffers.spec.js @@ -20,7 +20,7 @@ describe('returnBuffers', () => { let i = 1 if (args[2].detectBuffers) { // Test if detectBuffer option was deactivated - assert.strictEqual(client.options.detectBuffers, false) + assert.strictEqual(client._options.detectBuffers, false) args[2].detectBuffers = false i++ } diff --git a/test/tls.spec.js b/test/tls.spec.js index f4f202bf96..575916e173 100644 --- a/test/tls.spec.js +++ b/test/tls.spec.js @@ -76,7 +76,7 @@ describe('TLS connection tests', () => { assert.strictEqual(client.emittedEnd, true) assert.strictEqual(client.connected, false) assert.strictEqual(client.ready, false) - assert.strictEqual(client.closing, true) + assert.strictEqual(client._closing, true) assert.strictEqual(time, connectTimeout) done() }) @@ -97,8 +97,8 @@ describe('TLS connection tests', () => { }) // verify connection is using TCP, not UNIX socket - assert.strictEqual(client.connectionOptions.host, 'localhost') - assert.strictEqual(client.connectionOptions.port, tlsPort) + assert.strictEqual(client._connectionOptions.host, 'localhost') + assert.strictEqual(client._connectionOptions.port, tlsPort) assert.strictEqual(client.address, `localhost:${tlsPort}`) assert(client._stream.encrypted)