From 0889ae40dc991992324bee7df8d45c95b356e086 Mon Sep 17 00:00:00 2001
From: Raymond Feng <raymond@strongloop.com>
Date: Mon, 13 Oct 2014 09:47:11 -0700
Subject: [PATCH] Make sure callback happens if a model is not attached to the
 data source

---
 lib/sql.js                            | 36 +++++++++++++-------
 package.json                          |  2 ++
 test/automigrate.test.js              | 48 ++++++++++++++++++++++++++
 test/connectors/test-sql-connector.js | 49 +++++++++++++++++++++++++++
 4 files changed, 123 insertions(+), 12 deletions(-)
 create mode 100644 test/automigrate.test.js
 create mode 100644 test/connectors/test-sql-connector.js

diff --git a/lib/sql.js b/lib/sql.js
index d7dbc2b..29b4617 100644
--- a/lib/sql.js
+++ b/lib/sql.js
@@ -371,22 +371,34 @@ SqlConnector.prototype.automigrate = function (models, cb) {
   }
 
   models = models || Object.keys(self._models);
-  async.each(models, function (model, callback) {
-    if (model in self._models) {
-      self.dropTable(model, function (err) {
+  if (models.length === 0) {
+    return process.nextTick(cb);
+  }
+
+  var invalidModels = models.filter(function(m) {
+      return !(m in self._models);
+    });
+  if (invalidModels.length) {
+    return process.nextTick(function() {
+      cb(new Error('Cannot migrate models not attached to this datasource: ' +
+        invalidModels.join(' ')));
+    });
+  }
+
+  async.each(models, function(model, done) {
+    self.dropTable(model, function(err) {
+      if (err) {
+        // TODO(bajtos) should we abort here and call cb(err)?
+        // The original code in juggler ignored the error completely
+        console.error(err);
+      }
+      self.createTable(model, function(err, result) {
         if (err) {
-          // TODO(bajtos) should we abort here and call cb(err)?
-          // The original code in juggler ignored the error completely
           console.error(err);
         }
-        self.createTable(model, function (err, result) {
-          if (err) {
-            console.error(err);
-          }
-          callback(err, result);
-        });
+        done(err, result);
       });
-    }
+    });
   }, cb);
 };
 
diff --git a/package.json b/package.json
index 30d99eb..a994ace 100644
--- a/package.json
+++ b/package.json
@@ -24,6 +24,8 @@
     "async": "^0.9.0"
   },
   "devDependencies": {
+    "chai": "~1.9.2",
+    "loopback-datasource-juggler": "^2.0.0",
     "mocha": "^1.19.0"
   }
 }
diff --git a/test/automigrate.test.js b/test/automigrate.test.js
new file mode 100644
index 0000000..1f9808f
--- /dev/null
+++ b/test/automigrate.test.js
@@ -0,0 +1,48 @@
+var expect = require('chai').expect;
+var testConnector = require('./connectors/test-sql-connector');
+
+var juggler = require('loopback-datasource-juggler');
+var ds = new juggler.DataSource({
+  connector: testConnector,
+  debug: true
+});
+
+describe('sql connector', function() {
+  beforeEach(function() {
+    ds.connector._tables = {};
+    ds.connector._models = {};
+    ds.createModel('m1', {});
+  });
+
+  it('automigrate all models', function(done) {
+    ds.automigrate(function(err) {
+      expect(ds.connector._tables).have.property('m1');
+      done(err);
+    });
+  });
+
+  it('automigrate one model', function(done) {
+    ds.automigrate('m1', function(err) {
+      expect(ds.connector._tables).have.property('m1');
+      done(err);
+    });
+  });
+
+  it('automigrate one or more models in an array', function(done) {
+    ds.automigrate(['m1'], function(err) {
+      expect(ds.connector._tables).have.property('m1');
+      done(err);
+    });
+  });
+
+  it('automigrate reports errors for models not attached', function(done) {
+    ds.automigrate(['m1', 'm2'], function(err) {
+      expect(err).to.be.an.instanceOf(Error);
+      expect(ds.connector._tables).to.not.have.property('m1');
+      expect(ds.connector._tables).to.not.have.property('m2');
+      done();
+    });
+  });
+
+});
+
diff --git a/test/connectors/test-sql-connector.js b/test/connectors/test-sql-connector.js
new file mode 100644
index 0000000..41e0a24
--- /dev/null
+++ b/test/connectors/test-sql-connector.js
@@ -0,0 +1,49 @@
+/*
+ * A mockup connector that extends SQL connector
+ */
+var util = require('util');
+var SqlConnector = require('../../lib/sql');
+
+exports.initialize = function initializeDataSource(dataSource, callback) {
+  process.nextTick(function() {
+    if(callback) {
+      var connector = new TestConnector();
+      connector.dataSource = dataSource;
+      dataSource.connector = connector;
+      callback(null, connector);
+    }
+  });
+};
+
+function TestConnector() {
+  SqlConnector.apply(this, [].slice.call(arguments));
+  this._tables = {};
+}
+
+util.inherits(TestConnector, SqlConnector);
+
+TestConnector.prototype.dropTable = function(model, cb) {
+  var err;
+  var exists = model in this._tables;
+  if (!exists) {
+    err = new Error('Model doesn\'t exist: ' + model);
+  } else {
+    delete this._tables[model];
+  }
+  process.nextTick(function() {
+    cb(err);
+  });
+};
+
+TestConnector.prototype.createTable = function(model, cb) {
+  var err;
+  var exists = model in this._tables;
+  if (exists) {
+    err = new Error('Model already exists: ' + model);
+  } else {
+    this._tables[model] = model;
+  }
+  process.nextTick(function() {
+    cb(err);
+  });
+};