refactor: introduce makeStateMachine, reuse for all fsms

This patch ensures that we use the same method for building state
machines across all modern (read: unified topology) classes.

NODE-1517

Author: mbroadst

lib/core/connection/pool.js

@@ -20,12 +20,20 @@ const Buffer = require('safe-buffer').Buffer;
 const connect = require('./connect');
 const updateSessionFromResponse = require('../sessions').updateSessionFromResponse;
 const eachAsync = require('../utils').eachAsync;
-
-var DISCONNECTED = 'disconnected';
-var CONNECTING = 'connecting';
-var CONNECTED = 'connected';
-var DESTROYING = 'destroying';
-var DESTROYED = 'destroyed';
+const makeStateMachine = require('../utils').makeStateMachine;
+
+const DISCONNECTED = 'disconnected';
+const CONNECTING = 'connecting';
+const CONNECTED = 'connected';
+const DESTROYING = 'destroying';
+const DESTROYED = 'destroyed';
+const stateTransition = makeStateMachine({
+  [DISCONNECTED]: [CONNECTING, DESTROYING, DISCONNECTED],
+  [CONNECTING]: [CONNECTING, DESTROYING, CONNECTED, DISCONNECTED],
+  [CONNECTED]: [CONNECTED, DISCONNECTED, DESTROYING],
+  [DESTROYING]: [DESTROYING, DESTROYED],
+  [DESTROYED]: [DESTROYED]
+});
 
 const CONNECTION_EVENTS = new Set([
   'error',
@@ -80,6 +88,10 @@ var Pool = function(topology, options) {
   // Store topology for later use
   this.topology = topology;
 
+  this.s = {
+    state: DISCONNECTED
+  };
+
   // Add the options
   this.options = Object.assign(
     {
@@ -138,8 +150,6 @@ var Pool = function(topology, options) {
 
   // Logger instance
   this.logger = Logger('Pool', options);
-  // Pool state
-  this.state = DISCONNECTED;
   // Connections
   this.availableConnections = [];
   this.inUseConnections = [];
@@ -208,6 +218,13 @@ Object.defineProperty(Pool.prototype, 'socketTimeout', {
   }
 });
 
+Object.defineProperty(Pool.prototype, 'state', {
+  enumerable: true,
+  get: function() {
+    return this.s.state;
+  }
+});
+
 // clears all pool state
 function resetPoolState(pool) {
   pool.inUseConnections = [];
@@ -220,33 +237,6 @@ function resetPoolState(pool) {
   pool.reconnectId = null;
 }
 
-function stateTransition(self, newState) {
-  var legalTransitions = {
-    disconnected: [CONNECTING, DESTROYING, DISCONNECTED],
-    connecting: [CONNECTING, DESTROYING, CONNECTED, DISCONNECTED],
-    connected: [CONNECTED, DISCONNECTED, DESTROYING],
-    destroying: [DESTROYING, DESTROYED],
-    destroyed: [DESTROYED]
-  };
-
-  // Get current state
-  var legalStates = legalTransitions[self.state];
-  if (legalStates && legalStates.indexOf(newState) !== -1) {
-    self.emit('stateChanged', self.state, newState);
-    self.state = newState;
-  } else {
-    self.logger.error(
-      f(
-        'Pool with id [%s] failed attempted illegal state transition from [%s] to [%s] only following state allowed [%s]',
-        self.id,
-        self.state,
-        newState,
-        legalStates
-      )
-    );
-  }
-}
-
 function connectionFailureHandler(pool, event, err, conn) {
   if (conn) {
     if (conn._connectionFailHandled) return;

lib/core/sdam/server.js

@@ -15,6 +15,7 @@ const MongoNetworkError = require('../error').MongoNetworkError;
 const collationNotSupported = require('../utils').collationNotSupported;
 const debugOptions = require('../connection/utils').debugOptions;
 const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError;
+const makeStateMachine = require('../utils').makeStateMachine;
 
 // Used for filtering out fields for logging
 const DEBUG_FIELDS = [
@@ -44,10 +45,16 @@ const DEBUG_FIELDS = [
   'servername'
 ];
 
-const STATE_DISCONNECTING = 'disconnecting';
-const STATE_DISCONNECTED = 'disconnected';
+const STATE_CLOSING = 'closing';
+const STATE_CLOSED = 'closed';
 const STATE_CONNECTING = 'connecting';
 const STATE_CONNECTED = 'connected';
+const stateTransition = makeStateMachine({
+  [STATE_CLOSED]: [STATE_CLOSED, STATE_CONNECTING],
+  [STATE_CONNECTING]: [STATE_CONNECTING, STATE_CLOSING, STATE_CONNECTED, STATE_CLOSED],
+  [STATE_CONNECTED]: [STATE_CONNECTED, STATE_CLOSING, STATE_CLOSED],
+  [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED]
+});
 
 /**
  *
@@ -100,7 +107,7 @@ class Server extends EventEmitter {
       // the connection pool
       pool: null,
       // the server state
-      state: STATE_DISCONNECTED,
+      state: STATE_CLOSED,
       credentials: options.credentials,
       topology
     };
@@ -161,7 +168,7 @@ class Server extends EventEmitter {
     // relay all command monitoring events
     relayEvents(this.s.pool, this, ['commandStarted', 'commandSucceeded', 'commandFailed']);
 
-    this.s.state = STATE_CONNECTING;
+    stateTransition(this, STATE_CONNECTING);
 
     // If auth settings have been provided, use them
     if (options.auth) {
@@ -180,11 +187,19 @@ class Server extends EventEmitter {
   destroy(options, callback) {
     if (typeof options === 'function') (callback = options), (options = {});
     options = Object.assign({}, { force: false }, options);
+    if (this.s.state === STATE_CLOSED) {
+      if (typeof callback === 'function') {
+        callback();
+      }
+
+      return;
+    }
+
+    stateTransition(this, STATE_CLOSING);
 
-    this.s.state = STATE_DISCONNECTING;
     const done = err => {
+      stateTransition(this, STATE_CLOSED);
       this.emit('closed');
-      this.s.state = STATE_DISCONNECTED;
       if (typeof callback === 'function') {
         callback(err, null);
       }
@@ -482,12 +497,12 @@ function connectEventHandler(server) {
       );
     }
 
-    // emit an event indicating that our description has changed
-    server.emit('descriptionReceived', new ServerDescription(server.description.address, ismaster));
-
     // we are connected and handshaked (guaranteed by the pool)
-    server.s.state = STATE_CONNECTED;
+    stateTransition(server, STATE_CONNECTED);
     server.emit('connect', server);
+
+    // emit an event indicating that our description has changed
+    server.emit('descriptionReceived', new ServerDescription(server.description.address, ismaster));
   };
 }
 
@@ -503,7 +518,7 @@ function errorEventHandler(server) {
 
 function parseErrorEventHandler(server) {
   return function(err) {
-    server.s.state = STATE_DISCONNECTED;
+    stateTransition(this, STATE_CLOSED);
     server.emit('error', new MongoParseError(err));
   };
 }

lib/core/sdam/topology.js

@@ -25,6 +25,7 @@ const MongoError = require('../error').MongoError;
 const resolveClusterTime = require('../topologies/shared').resolveClusterTime;
 const SrvPoller = require('./srv_polling').SrvPoller;
 const getMMAPError = require('../topologies/shared').getMMAPError;
+const makeStateMachine = require('../utils').makeStateMachine;
 
 // Global state
 let globalTopologyCounter = 0;
@@ -63,28 +64,12 @@ const STATE_CLOSING = 'closing';
 const STATE_CLOSED = 'closed';
 const STATE_CONNECTING = 'connecting';
 const STATE_CONNECTED = 'connected';
-
-function stateTransition(topology, newState) {
-  const legalTransitions = {
-    [STATE_CLOSED]: [STATE_CLOSED, STATE_CONNECTING],
-    [STATE_CONNECTING]: [STATE_CONNECTING, STATE_CLOSING, STATE_CONNECTED, STATE_CLOSED],
-    [STATE_CONNECTED]: [STATE_CONNECTED, STATE_CLOSING, STATE_CLOSED],
-    [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED]
-  };
-
-  const legalStates = legalTransitions[topology.s.state];
-  if (legalStates && legalStates.indexOf(newState) < 0) {
-    console.log('throwing an error');
-    throw new MongoError(
-      `illegal state transition from [${
-        topology.s.state
-      }] => [${newState}], allowed: [${legalStates}]`
-    );
-  }
-
-  topology.emit('stateChanged', topology.s.state, newState);
-  topology.s.state = newState;
-}
+const stateTransition = makeStateMachine({
+  [STATE_CLOSED]: [STATE_CLOSED, STATE_CONNECTING],
+  [STATE_CONNECTING]: [STATE_CONNECTING, STATE_CLOSING, STATE_CONNECTED, STATE_CLOSED],
+  [STATE_CONNECTED]: [STATE_CONNECTED, STATE_CLOSING, STATE_CLOSED],
+  [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED]
+});
 
 /**
  * A container of server instances representing a connection to a MongoDB topology.

lib/core/utils.js

@@ -177,6 +177,22 @@ function tagsStrictEqual(tags, tags2) {
   return tagsKeys.length === tags2Keys.length && tagsKeys.every(key => tags2[key] === tags[key]);
 }
 
+function makeStateMachine(stateTable) {
+  return function stateTransition(target, newState) {
+    const legalStates = stateTable[target.s.state];
+    if (legalStates && legalStates.indexOf(newState) < 0) {
+      throw new TypeError(
+        `illegal state transition from [${
+          target.s.state
+        }] => [${newState}], allowed: [${legalStates}]`
+      );
+    }
+
+    target.emit('stateChanged', target.s.state, newState);
+    target.s.state = newState;
+  };
+}
+
 module.exports = {
   uuidV4,
   calculateDurationInMs,
@@ -189,5 +205,6 @@ module.exports = {
   eachAsync,
   isUnifiedTopology,
   arrayStrictEqual,
-  tagsStrictEqual
+  tagsStrictEqual,
+  makeStateMachine
 };