Merge pull request #1585 from strongloop/fix-lazy-connect

Fix datasource state management

lib/datasource.js

@@ -314,8 +314,6 @@ DataSource.prototype.connect = function(callback) {
 
   // The connect is already in progress
   if (this.connecting) return callback.promise;
-  // Set connecting flag to be true
-  this.connecting = true;
   this.connector.connect(function(err, result) {
     self.connecting = false;
     if (!err) self.connected = true;
@@ -340,6 +338,11 @@ DataSource.prototype.connect = function(callback) {
       if (err) throw err; // It should not happen
     });
   });
+
+  // Set connecting flag to be `true` so that the connector knows there is
+  // a connect in progress. The change of `connecting` should happen immediately
+  // after the connect request is sent
+  this.connecting = true;
   return callback.promise;
 };
 
@@ -416,6 +419,7 @@ DataSource.prototype.setup = function(dsName, settings) {
   // Disconnected by default
   this.connected = false;
   this.connecting = false;
+  this.initialized = false;
 
   this.name = dsName || (typeof this.settings.name === 'string' && this.settings.name);
 
@@ -453,20 +457,38 @@ DataSource.prototype.setup = function(dsName, settings) {
         throw new Error(g.f('Connector is not defined correctly: ' +
           'it should create `{{connector}}` member of dataSource'));
       }
-      this.connected = !err; // Connected now
+      if (!err) {
+        this.initialized = true;
+        this.emit('initialized');
+      }
+      debug('Connector is initialized for dataSource %s', this.name);
+      // If `result` is set to `false` explicitly, the connection will be
+      // lazily established
+      if (!this.settings.lazyConnect) {
+        this.connected = (!err) && (result !== false); // Connected now
+      }
       if (this.connected) {
+        debug('DataSource %s is now connected to %s', this.name, this.connector.name);
         this.emit('connected');
       } else {
         // The connection fails, let's report it and hope it will be recovered in the next call
-        g.error('Connection fails: %s\nIt will be retried for the next request.', err);
+        if (err) {
-        this.emit('error', err);
+          // Reset the connecting to `false`
-        this.connecting = false;
+          this.connecting = false;
+          g.error('Connection fails: %s\nIt will be retried for the next request.', err);
+          this.emit('error', err);
+        } else {
+          // Either lazyConnect or connector initialize() defers the connection
+          debug('DataSource %s will be connected to connector %s', this.name,
+            this.connector.name);
+        }
       }
     }.bind(this);
 
     try {
       if ('function' === typeof connector.initialize) {
         // Call the async initialize method
+        debug('Initializing connector %s', connector.name);
         connector.initialize(this, postInit);
       } else if ('function' === typeof connector) {
         // Use the connector constructor directly
@@ -2464,13 +2486,21 @@ DataSource.prototype.isRelational = function() {
 };
 
 /**
- * Check if the data source is ready.
+ * Check if the data source is connected. If not, the method invocation will be
- * Returns a Boolean value.
+ * deferred and queued.
- * @param {Object} obj  Deferred method call if the connector is not fully connected yet
+ *
- * @param {Object} args  argument passing to method call.
+ *
+ * @param {Object} obj  Receiver for the method call
+ * @param {Object} args  Arguments passing to the method call
+ * @returns - a Boolean value to indicate if the method invocation is deferred.
+ * false: The datasource is already connected
+ * - true: The datasource is yet to be connected
  */
-DataSource.prototype.ready = function(obj, args) {
+DataSource.prototype.queueInvocation = DataSource.prototype.ready =
+function(obj, args) {
   var self = this;
+  debug('Datasource %s: connected=%s connecting=%s', this.name,
+    this.connected, this.connecting);
   if (this.connected) {
     // Connected
     return false;
@@ -2481,6 +2511,7 @@ DataSource.prototype.ready = function(obj, args) {
 
   var onConnected = null, onError = null, timeoutHandle = null;
   onConnected = function() {
+    debug('Datasource %s is now connected - executing method %s', self.name, method.name);
     // Remove the error handler
     self.removeListener('error', onError);
     if (timeoutHandle) {
@@ -2502,6 +2533,7 @@ DataSource.prototype.ready = function(obj, args) {
     }
   };
   onError = function(err) {
+    debug('Datasource %s fails to connect - aborting method %s', self.name, method.name);
     // Remove the connected listener
     self.removeListener('connected', onConnected);
     if (timeoutHandle) {
@@ -2521,6 +2553,9 @@ DataSource.prototype.ready = function(obj, args) {
   // Set up a timeout to cancel the invocation
   var timeout = this.settings.connectionTimeout || 5000;
   timeoutHandle = setTimeout(function() {
+    debug('Datasource %s fails to connect due to timeout - aborting method %s',
+      self.name, method.name);
+    self.connecting = false;
     self.removeListener('error', onError);
     self.removeListener('connected', onConnected);
     var params = [].slice.call(args);
@@ -2531,6 +2566,7 @@ DataSource.prototype.ready = function(obj, args) {
   }, timeout);
 
   if (!this.connecting) {
+    debug('Connecting datasource %s to connector %s', this.name, this.connector.name);
     this.connect();
   }
   return true;

test/datasource.test.js

@@ -155,6 +155,172 @@ describe('DataSource', function() {
     dataSource.connector.should.equal(mockConnector);
   });
 
+  it('should set states correctly with eager connect', function(done) {
+    var mockConnector = {
+      name: 'loopback-connector-mock',
+      initialize: function(ds, cb) {
+        ds.connector = mockConnector;
+        this.connect(cb);
+      },
+
+      connect: function(cb) {
+        process.nextTick(function() {
+          cb(null);
+        });
+      },
+    };
+    var dataSource = new DataSource(mockConnector);
+    // DataSource is instantiated
+    // connected: false, connecting: false, initialized: false
+    dataSource.connected.should.be.false();
+    dataSource.connecting.should.be.false();
+    dataSource.initialized.should.be.false();
+
+    dataSource.on('initialized', function() {
+      // DataSource is initialized with lazyConnect
+      // connected: false, connecting: false, initialized: true
+      dataSource.connected.should.be.false();
+      dataSource.connecting.should.be.false();
+      dataSource.initialized.should.be.true();
+    });
+
+    dataSource.on('connected', function() {
+      // DataSource is now connected
+      // connected: true, connecting: false
+      dataSource.connected.should.be.true();
+      dataSource.connecting.should.be.false();
+    });
+
+    // Call connect() in next tick so that we'll receive initialized event
+    // first
+    process.nextTick(function() {
+      // At this point, the datasource is already connected by
+      // connector's (mockConnector) initialize function
+      dataSource.connect(function() {
+        // DataSource is now connected
+        // connected: true, connecting: false
+        dataSource.connected.should.be.true();
+        dataSource.connecting.should.be.false();
+        done();
+      });
+      // As the datasource is already connected, no connecting will happen
+      // connected: true, connecting: false
+      dataSource.connected.should.be.true();
+      dataSource.connecting.should.be.false();
+    });
+  });
+
+  it('should set states correctly with deferred connect', function(done) {
+    var mockConnector = {
+      name: 'loopback-connector-mock',
+      initialize: function(ds, cb) {
+        ds.connector = mockConnector;
+        // Explicitly call back with false to denote connection is not ready
+        process.nextTick(function() {
+          cb(null, false);
+        });
+      },
+
+      connect: function(cb) {
+        process.nextTick(function() {
+          cb(null);
+        });
+      },
+    };
+    var dataSource = new DataSource(mockConnector);
+    // DataSource is instantiated
+    // connected: false, connecting: false, initialized: false
+    dataSource.connected.should.be.false();
+    dataSource.connecting.should.be.false();
+    dataSource.initialized.should.be.false();
+
+    dataSource.on('initialized', function() {
+      // DataSource is initialized with lazyConnect
+      // connected: false, connecting: false, initialized: true
+      dataSource.connected.should.be.false();
+      dataSource.connecting.should.be.false();
+      dataSource.initialized.should.be.true();
+    });
+
+    dataSource.on('connected', function() {
+      // DataSource is now connected
+      // connected: true, connecting: false
+      dataSource.connected.should.be.true();
+      dataSource.connecting.should.be.false();
+    });
+
+    // Call connect() in next tick so that we'll receive initialized event
+    // first
+    process.nextTick(function() {
+      dataSource.connect(function() {
+        // DataSource is now connected
+        // connected: true, connecting: false
+        dataSource.connected.should.be.true();
+        dataSource.connecting.should.be.false();
+        done();
+      });
+      // As the datasource is not connected, connecting will happen
+      // connected: false, connecting: true
+      dataSource.connected.should.be.false();
+      dataSource.connecting.should.be.true();
+    });
+  });
+
+  it('should set states correctly with lazyConnect = true', function(done) {
+    var mockConnector = {
+      name: 'loopback-connector-mock',
+      initialize: function(ds, cb) {
+        ds.connector = mockConnector;
+        process.nextTick(function() {
+          cb(null);
+        });
+      },
+
+      connect: function(cb) {
+        process.nextTick(function() {
+          cb(null);
+        });
+      },
+    };
+    var dataSource = new DataSource(mockConnector, {lazyConnect: true});
+    // DataSource is instantiated
+    // connected: false, connecting: false, initialized: false
+    dataSource.connected.should.be.false();
+    dataSource.connecting.should.be.false();
+    dataSource.initialized.should.be.false();
+
+    dataSource.on('initialized', function() {
+      // DataSource is initialized with lazyConnect
+      // connected: false, connecting: false, initialized: true
+      dataSource.connected.should.be.false();
+      dataSource.connecting.should.be.false();
+      dataSource.initialized.should.be.true();
+    });
+
+    dataSource.on('connected', function() {
+      // DataSource is now connected
+      // connected: true, connecting: false
+      dataSource.connected.should.be.true();
+      dataSource.connecting.should.be.false();
+    });
+
+    // Call connect() in next tick so that we'll receive initialized event
+    // first
+    process.nextTick(function() {
+      dataSource.connect(function() {
+        // DataSource is now connected
+        // connected: true, connecting: false
+        dataSource.connected.should.be.true();
+        dataSource.connecting.should.be.false();
+        done();
+      });
+      // DataSource is now connecting
+      // connected: false, connecting: true
+      dataSource.connected.should.be.false();
+      dataSource.connecting.should.be.true();
+    });
+  });
+
   describe('deleteModelByName()', () => {
     it('removes the model from ModelBuilder registry', () => {
       const ds = new DataSource('ds', {connector: 'memory'});

types/datasource.d.ts

@@ -76,6 +76,7 @@ export declare class DataSource {
   name: string;
   settings: Options;
 
+  initialized?: boolean;
   connected?: boolean;
   connecting?: boolean;