Refactor MessageTracker into testable module

Author: jsumners

lib/client/client.js

@@ -1,6 +1,8 @@
 'use strict'
 
 const requestQueueFactory = require('./request-queue')
+const messageTrackerFactory = require('./message-tracker')
+const { MAX_MSGID } = require('./constants')
 
 var EventEmitter = require('events').EventEmitter
 var net = require('net')
@@ -49,7 +51,6 @@ var PresenceFilter = filters.PresenceFilter
 var ConnectionError = errors.ConnectionError
 
 var CMP_EXPECT = [errors.LDAP_COMPARE_TRUE, errors.LDAP_COMPARE_FALSE]
-var MAX_MSGID = Math.pow(2, 31) - 1
 
 // node 0.6 got rid of FDs, so make up a client id for logging
 var CLIENT_ID = 0
@@ -88,117 +89,6 @@ function ensureDN (input, strict) {
   }
 }
 
-/**
- * Track message callback by messageID.
- */
-function MessageTracker (opts) {
-  assert.object(opts)
-  assert.string(opts.id)
-  assert.object(opts.parser)
-
-  this.id = opts.id
-  this._msgid = 0
-  this._messages = {}
-  this._abandoned = {}
-  this.parser = opts.parser
-
-  var self = this
-  this.__defineGetter__('pending', function () {
-    return Object.keys(self._messages)
-  })
-}
-
-/**
- * Record a messageID and callback.
- */
-MessageTracker.prototype.track = function track (message, callback) {
-  var msgid = this._nextID()
-  message.messageID = msgid
-  this._messages[msgid] = callback
-  return msgid
-}
-
-/**
- * Fetch callback based on messageID.
- */
-MessageTracker.prototype.fetch = function fetch (msgid) {
-  var msg = this._messages[msgid]
-  if (msg) {
-    this._purgeAbandoned(msgid)
-    return msg
-  }
-  // It's possible that the server has not received the abandon request yet.
-  // While waiting for evidence that the abandon has been received, incoming
-  // messages that match the abandoned msgid will be handled as normal.
-  msg = this._abandoned[msgid]
-  if (msg) {
-    return msg.cb
-  }
-  return null
-}
-
-/**
- * Cease tracking for a given messageID.
- */
-MessageTracker.prototype.remove = function remove (msgid) {
-  if (this._messages[msgid]) {
-    delete this._messages[msgid]
-  } else if (this._abandoned[msgid]) {
-    delete this._abandoned[msgid]
-  }
-}
-
-/**
- * Mark a messageID as abandoned.
- */
-MessageTracker.prototype.abandon = function abandonMsg (msgid) {
-  if (this._messages[msgid]) {
-    // Keep track of "when" the message was abandoned
-    this._abandoned[msgid] = {
-      age: this._msgid,
-      cb: this._messages[msgid]
-    }
-    delete this._messages[msgid]
-  }
-}
-
-/**
- * Purge old items from abandoned list.
- */
-MessageTracker.prototype._purgeAbandoned = function _purgeAbandoned (msgid) {
-  var self = this
-  // Is (comp >= ref) according to sliding window
-  function geWindow (ref, comp) {
-    var max = ref + (MAX_MSGID / 2)
-    var min = ref
-    if (max >= MAX_MSGID) {
-      // Handle roll-over
-      max = max - MAX_MSGID - 1
-      return ((comp <= max) || (comp >= min))
-    } else {
-      return ((comp <= max) && (comp >= min))
-    }
-  }
-
-  Object.keys(this._abandoned).forEach(function (id) {
-    // Abandoned messageIDs can be forgotten if a received messageID is "newer"
-    if (geWindow(self._abandoned[id].age, msgid)) {
-      self._abandoned[id].cb(new errors.AbandonedError(
-        'client request abandoned'))
-      delete self._abandoned[id]
-    }
-  })
-}
-
-/**
- * Allocate the next messageID according to a sliding window.
- */
-MessageTracker.prototype._nextID = function _nextID () {
-  if (++this._msgid >= MAX_MSGID) { this._msgid = 1 }
-
-  return this._msgid
-}
-
 /// --- API
 
 /**
@@ -947,7 +837,7 @@ Client.prototype.connect = function connect () {
 
   // Initialize socket events and LDAP parser.
   function initSocket () {
-    tracker = new MessageTracker({
+    tracker = messageTrackerFactory({
       id: self.url ? self.url.href : self.socketPath,
       parser: new Parser({ log: log })
     })
@@ -1157,11 +1047,8 @@ Client.prototype._onClose = function _onClose (closeError) {
   this.emit('close', closeError)
   // On close we have to walk the outstanding messages and go invoke their
   // callback with an error.
-  tracker.pending.forEach(function (msgid) {
-    var cb = tracker.fetch(msgid)
-    tracker.remove(msgid)
-
-    if (socket.unbindMessageID !== parseInt(msgid, 10)) {
+  tracker.purge(function (msgid, cb) {
+    if (socket.unbindMessageID !== msgid) {
       return cb(new ConnectionError(tracker.id + ' closed'))
     } else {
       // Unbinds will be communicated as a success since we're closed
@@ -1201,7 +1088,7 @@ Client.prototype._updateIdle = function _updateIdle (override) {
   function isIdle (disable) {
     return ((disable !== true) &&
       (self._socket && self.connected) &&
-      (self._tracker.pending.length === 0))
+      (self._tracker.pending === 0))
   }
   if (isIdle(override)) {
     if (!this._idleTimer) {

lib/client/constants.js

@@ -0,0 +1,7 @@
+'use strict'
+
+module.exports = {
+  // https://tools.ietf.org/html/rfc4511#section-4.1.1
+  // Message identifiers are an integer between (0, maxint).
+  MAX_MSGID: Math.pow(2, 31) - 1
+}

lib/client/message-tracker/ge-window.js

@@ -0,0 +1,25 @@
+'use strict'
+
+const { MAX_MSGID } = require('../constants')
+
+/**
+ * Compare a reference id with another id to determine "greater than or equal"
+ * between the two values according to a sliding window.
+ *
+ * @param {integer} ref
+ * @param {integer} comp
+ *
+ * @returns {boolean} `true` if the `comp` value is >= to the `ref` value
+ * within the computed window, otherwise `false`.
+ */
+module.exports = function geWindow (ref, comp) {
+  let max = ref + Math.floor(MAX_MSGID / 2)
+  const min = ref
+  if (max >= MAX_MSGID) {
+    // Handle roll-over
+    max = max - MAX_MSGID - 1
+    return ((comp <= max) || (comp >= min))
+  } else {
+    return ((comp <= max) && (comp >= min))
+  }
+}

lib/client/message-tracker/id-generator.js

@@ -0,0 +1,23 @@
+'use strict'
+
+const { MAX_MSGID } = require('../constants')
+
+/**
+ * Returns a function that generates message identifiers. According to RFC 4511
+ * the identifers should be `(0, MAX_MSGID)`. The returned function handles
+ * this and wraps around when the maximum has been reached.
+ *
+ * @param {integer} [start=0] Starting number in the identifier sequence.
+ *
+ * @returns {function} This function accepts no parameters and returns an
+ * increasing sequence identifier each invocation until it reaches the maximum
+ * identifier. At this point the sequence starts over.
+ */
+module.exports = function idGeneratorFactory (start = 0) {
+  let currentID = start
+  return function nextID () {
+    const nextID = currentID + 1
+    currentID = (nextID >= MAX_MSGID) ? 1 : nextID
+    return currentID
+  }
+}

lib/client/message-tracker/index.js

@@ -0,0 +1,151 @@
+'use strict'
+
+const idGeneratorFactory = require('./id-generator')
+const purgeAbandoned = require('./purge-abandoned')
+
+/**
+ * Returns a message tracker object that keeps track of which message
+ * identifiers correspond to which message handlers. Also handles keeping track
+ * of abandoned messages.
+ *
+ * @param {object} options
+ * @param {string} options.id An identifier for the tracker.
+ * @param {object} options.parser An object that will be used to parse messages.
+ *
+ * @returns {MessageTracker}
+ */
+module.exports = function messageTrackerFactory (options) {
+  if (Object.prototype.toString.call(options) !== '[object Object]') {
+    throw Error('options object is required')
+  }
+  if (!options.id || typeof options.id !== 'string') {
+    throw Error('options.id string is required')
+  }
+  if (!options.parser || Object.prototype.toString.call(options.parser) !== '[object Object]') {
+    throw Error('options.parser object is required')
+  }
+
+  let currentID = 0
+  const nextID = idGeneratorFactory()
+  const messages = new Map()
+  const abandoned = new Map()
+
+  /**
+   * @typedef {object} MessageTracker
+   * @property {string} id The identifier of the tracker as supplied via the options.
+   * @property {object} parser The parser object given by the the options.
+   */
+  const tracker = {
+    id: options.id,
+    parser: options.parser
+  }
+
+  /**
+   * Count of messages awaiting response.
+   *
+   * @alias pending
+   * @memberof! MessageTracker#
+   */
+  Object.defineProperty(tracker, 'pending', {
+    get () {
+      return messages.size
+    }
+  })
+
+  /**
+   * Move a specific message to the abanded track.
+   *
+   * @param {integer} msgID The identifier for the message to move.
+   *
+   * @memberof MessageTracker
+   * @method abandon
+   */
+  tracker.abandon = function abandonMessage (msgID) {
+    if (messages.has(msgID) === false) return false
+    abandoned.set(msgID, {
+      age: currentID,
+      cb: messages.get(msgID)
+    })
+    return messages.delete(msgID)
+  }
+
+  /**
+   * Retrieves the message handler for a message. Removes abandoned messages
+   * that have been given time to be resolved.
+   *
+   * @param {integer} msgID The identifier for the message to get the handler for.
+   *
+   * @memberof MessageTracker
+   * @method fetch
+   */
+  tracker.fetch = function fetchMessage (msgID) {
+    const messageCB = messages.get(msgID)
+    if (messageCB) {
+      purgeAbandoned(msgID, abandoned)
+      return messageCB
+    }
+
+    // We sent an abandon request but the server either wasn't able to process
+    // it or has not received it yet. Therefore, we received a response for the
+    // abandoned message. So we must return the abandoned message's callback
+    // to be processed normally.
+    const abandonedMsg = abandoned.get(msgID)
+    if (abandonedMsg) {
+      return abandonedMsg.cb
+    }
+
+    return null
+  }
+
+  /**
+   * Removes all message tracks, cleans up the abandoned track, and invokes
+   * a callback for each message purged.
+   *
+   * @param {function} cb A function with the signature `(msgID, handler)`.
+   *
+   * @memberof MessageTracker
+   * @method purge
+   */
+  tracker.purge = function purgeMessages (cb) {
+    messages.forEach((val, key) => {
+      purgeAbandoned(key, abandoned)
+      tracker.remove(key)
+      cb(key, val)
+    })
+  }
+
+  /**
+   * Removes a message from all tracking.
+   *
+   * @param {integer} msgID The identifier for the message to remove from tracking.
+   *
+   * @memberof MessageTracker
+   * @method remove
+   */
+  tracker.remove = function removeMessage (msgID) {
+    if (messages.delete(msgID) === false) {
+      abandoned.delete(msgID)
+    }
+  }
+
+  /**
+   * Add a message handler to be tracked.
+   *
+   * @param {object} message The message object to be tracked. This object will
+   * have a new property added to it: `messageID`.
+   * @param {function} callback The handler for the message.
+   *
+   * @memberof MessageTracker
+   * @method track
+   */
+  tracker.track = function trackMessage (message, callback) {
+    currentID = nextID()
+    // This side effect is not ideal but the client doesn't attach the tracker
+    // to itself until after the `.connect` method has fired. If this can be
+    // refactored later, then we can possibly get rid of this side effect.
+    message.messageID = currentID
+    messages.set(currentID, callback)
+  }
+
+  return tracker
+}