Change: Pass only storageOptions to Storage constructor

README.md

@@ -91,11 +91,8 @@ var Bluebird = require('bluebird');
 var redefine = require('redefine');
 
 module.exports = redefine.Class({
-  constructor: function (options) {
-    this.options = options;
-    this.options.storageOptions = _.extend({
-      option1: 'defaultValue1'
-    }, this.options.storageOptions)
+  constructor: function ({ option1: 'defaultValue1' } = {}) {
+    this.option1 = option1;
   },
 
   logMigration: function (migrationName) {

src/index.js

@@ -407,7 +407,19 @@ module.exports = class Umzug extends EventEmitter {
       throw new Error('Unable to resolve the storage: ' + this.options.storage + ', ' + e);
     }
 
-    return new Storage(this.options);
+    let storage = new Storage(this.options.storageOptions);
+    if (_.has(storage, 'options.storageOptions')) {
+      console.warn(
+        'Deprecated: Umzug Storage constructor has changed!',
+        'old syntax: new Storage({ storageOptions: { ... } })',
+        'new syntax: new Storage({ ... })',
+        'where ... represents the same storageOptions passed to Umzug constructor.',
+        'For more information: https://github.com/sequelize/umzug/pull/137'
+      );
+      storage = new Storage(this.options);
+    }
+
+    return storage;
   }
 
   /**

src/storages/json.js

@@ -1,7 +1,7 @@
 import _ from 'lodash';
 import Bluebird from 'bluebird';
 import fs from 'fs';
-import path from 'path';
+import _path from 'path';
 
 /**
  * @class JSONStorage
@@ -11,19 +11,11 @@ module.exports = class JSONStorage {
    * Constructs JSON file storage.
    *
    * @param {Object} [options]
-   * @param {Object} [options.storageOptions]
-   * @param {String} [options.storageOptions.path='./umzug.json'] - Path to JSON
-   * file where the log is stored. Defaults './umzug.json' relative to process'
-   * cwd.
-   * @constructs JSONStorage
+   * @param {String} [options.path='./umzug.json'] - Path to JSON file where
+   * the log is stored. Defaults './umzug.json' relative to process' cwd.
    */
-  constructor(options = {}) {
-    this.options = options;
-
-    this.options.storageOptions = {
-      path: path.resolve(process.cwd(), 'umzug.json'),
-      ...this.options.storageOptions || {},
-    };
+  constructor({ path = _path.resolve(process.cwd(), 'umzug.json') } = {}) {
+    this.path = path;
   }
 
   /**
@@ -33,7 +25,7 @@ module.exports = class JSONStorage {
    * @returns {Promise}
    */
   logMigration(migrationName) {
-    var filePath  = this.options.storageOptions.path;
+    var filePath  = this.path;
     var readfile  = Bluebird.promisify(fs.readFile);
     var writefile = Bluebird.promisify(fs.writeFile);
 
@@ -53,7 +45,7 @@ module.exports = class JSONStorage {
    * @returns {Promise}
    */
   unlogMigration(migrationName) {
-    var filePath  = this.options.storageOptions.path;
+    var filePath  = this.path;
     var readfile  = Bluebird.promisify(fs.readFile);
     var writefile = Bluebird.promisify(fs.writeFile);
 
@@ -72,7 +64,7 @@ module.exports = class JSONStorage {
    * @returns {Promise.<String[]>}
    */
   executed() {
-    var filePath = this.options.storageOptions.path;
+    var filePath = this.path;
     var readfile = Bluebird.promisify(fs.readFile);
 
     return readfile(filePath)

src/storages/none.js

@@ -4,14 +4,6 @@ import Bluebird from 'bluebird';
  * @class NoneStorage
  */
 module.exports = class NoneStorage {
-  /**
-   * Constructs none storage.
-   *
-   * @param {Object} [options]
-   * @constructs NoneStorage
-   */
-  constructor(options) {}
-
   /**
    * Does nothing.
    *

src/storages/sequelize.js

@@ -21,75 +21,73 @@ module.exports = class SequelizeStorage {
    * If the table does not exist it will be created automatically.
    *
    * @param {Object} [options]
-   * @param {Object} [options.storageOptions]
-   * @param {Object} [options.storageOptions.sequelize] - configured instance of
-   * Sequelize.
-   * @param {Object} [options.storageOptions.model] - Sequelize model - must
-   * have column name matching "columnName" option.
-   * @param {String} [options.storageOptions.modelName='SequelizeMeta'] - name
-   * of model to create if "model" option is not supplied.
-   * @param {String} [options.storageOptions.tableName=modelName] - name of
-   * table to create if "model" option is not supplied.
-   * @param {String} [options.storageOptions.schema=schema] - name of the schema
-   * to create the table under, defaults to undefined.
-   * @param {String} [options.storageOptions.columnName='name'] - name of table
-   * column holding migration name.
-   * @param {String} [options.storageOptions.columnType=Sequelize.STRING] - type
-   * of the column. For utf8mb4 charsets under InnoDB, you may need to set
-   * this <= 190.
-   * @param {Boolean} [options.storageOptions.timestamps=false] - option to add
-   * timestamps to model table
-   *
-   * @constructs SequelizeStorage
+   * @param {Object} [options.]
+   * @param {Object} [options.sequelize] - configured instance of Sequelize.
+   * @param {Object} [options.model] - Sequelize model - must have column name
+   * matching "columnName" option.
+   * @param {String} [options.modelName='SequelizeMeta'] - name of the model
+   * to create if "model" option is not supplied.
+   * @param {String} [options.tableName=modelName] - name of the table to create
+   * if "model" option is not supplied.
+   * @param {String} [options.schema=schema] - name of the schema to create
+   * the table under, defaults to undefined.
+   * @param {String} [options.columnName='name'] - name of the table column
+   * holding migration name.
+   * @param {String} [options.columnType=Sequelize.STRING] - type of the column.
+   * For utf8mb4 charsets under InnoDB, you may need to set this <= 190.
+   * @param {Boolean} [options.timestamps=false] - option to add timestamps to the model table
    */
-  constructor(options = {}) {
-    this.options = options;
-    this.options.storageOptions = {
-      // note 'sequelize' or 'model' is required
-      modelName: 'SequelizeMeta',
-      // note 'tableName' (optional) also supported
-      columnName: 'name',
-      timestamps: false,
-      ...this.options.storageOptions || {},
-    };
-
-    if (!this.options.storageOptions.model && !this.options.storageOptions.sequelize) {
+  constructor({
+    sequelize,
+    model,
+    modelName = 'SequelizeMeta',
+    tableName,
+    schema,
+    columnName = 'name',
+    columnType,
+    timestamps = false
+  } = {}) {
+    if (!model && !sequelize) {
       throw new Error('One of "sequelize" or "model" storage option is required');
     }
 
-    // initialize model
-    if (!this.options.storageOptions.model) {
-      var sequelize = this.options.storageOptions.sequelize;
-      var modelName = this.options.storageOptions.modelName;
-      var Sequelize = sequelize.constructor;
-      var columnType = this.options.storageOptions.columnType || Sequelize.STRING;
+    this.sequelize = sequelize || model.sequelize;
+
+    const Sequelize = this.sequelize.constructor;
+
+    this.columnType = columnType || Sequelize.STRING;
+    this.columnName = columnName;
+    this.timestamps = timestamps;
+    this.modelName = modelName;
+    this.tableName = tableName;
+    this.schema = schema;
+    this.model = model || this.getModel();
+  }
 
-      if (sequelize.isDefined(modelName)) {
-        this.options.storageOptions.model = sequelize.model(modelName);
-      } else {
-        var attributes = {};
+  getModel() {
+    if (this.sequelize.isDefined(this.modelName)) {
+      return this.sequelize.model(this.modelName);
+    }
 
-        attributes[this.options.storageOptions.columnName] = {
-          type: columnType,
+    return this.sequelize.define(
+      this.modelName,
+      {
+        [this.columnName]: {
+          type: this.columnType,
           allowNull: false,
           unique: true,
           primaryKey: true,
           autoIncrement: false
-        };
-
-        this.options.storageOptions.model = sequelize.define(
-          modelName,
-          attributes,
-          {
-            tableName:  this.options.storageOptions.tableName,
-            schema: this.options.storageOptions.schema,
-            timestamps: this.options.storageOptions.timestamps,
-            charset: 'utf8',
-            collate: 'utf8_unicode_ci'
-          }
-        );
+        },
+      },
+      {
+        tableName:  this.tableName,
+        schema: this.schema,
+        timestamps: this.timestamps,
+        charset: 'utf8',
+        collate: 'utf8_unicode_ci'
       }
-    }
+    );
   }
 
   /**
@@ -105,7 +103,7 @@ module.exports = class SequelizeStorage {
       .sync()
       .then(function(Model) {
         var migration = {};
-        migration[self.options.storageOptions.columnName] = migrationName;
+        migration[self.columnName] = migrationName;
         return Model.create(migration);
       });
   }
@@ -118,14 +116,14 @@ module.exports = class SequelizeStorage {
    */
   unlogMigration(migrationName) {
     var self             = this;
-    var sequelize        = this.options.storageOptions.sequelize;
+    var sequelize        = this.sequelize;
     var sequelizeVersion = !!sequelize.modelManager ? 2 : 1;
 
     return this._model()
       .sync()
       .then(function(Model) {
         var where = {};
-        where[self.options.storageOptions.columnName] = migrationName;
+        where[self.columnName] = migrationName;
 
         if (sequelizeVersion > 1) {
           // This is an ugly hack to find out which function signature we have to use.
@@ -147,11 +145,11 @@ module.exports = class SequelizeStorage {
     return this._model()
       .sync()
       .then(function(Model) {
-        return Model.findAll({ order: [ [ self.options.storageOptions.columnName, 'ASC' ] ] });
+        return Model.findAll({ order: [ [ self.columnName, 'ASC' ] ] });
       })
       .then(function(migrations) {
         return migrations.map(function(migration) {
-          return migration[self.options.storageOptions.columnName];
+          return migration[self.columnName];
         });
       });
   }
@@ -163,6 +161,6 @@ module.exports = class SequelizeStorage {
    * @private
    */
   _model() {
-    return this.options.storageOptions.model;
+    return this.model;
   }
 }

test/storages/json.test.js

@@ -12,12 +12,12 @@ describe('JSON', function () {
   describe('constructor', function () {
     it('stores options', function () {
       var storage = new Storage();
-      expect(storage).to.have.property('options');
+      expect(storage).to.have.property('path');
     });
 
     it('sets the default storage path', function () {
       var storage = new Storage();
-      expect(storage.options.storageOptions.path).to.equal(
+      expect(storage.path).to.equal(
         path.normalize(process.cwd() + '/umzug.json')
       );
     });
@@ -26,9 +26,7 @@ describe('JSON', function () {
   describe('logMigration', function () {
     beforeEach(function () {
       this.path    = __dirname + '/../tmp/umzug.json';
-      this.storage = new Storage({
-        storageOptions: { path: this.path }
-      });
+      this.storage = new Storage({ path: this.path });
       return helper.prepareMigrations(3);
     });
 
@@ -53,9 +51,7 @@ describe('JSON', function () {
   describe('unlogMigration', function () {
     beforeEach(function () {
       this.path    = __dirname + '/../tmp/umzug.json';
-      this.storage = new Storage({
-        storageOptions: { path: this.path }
-      });
+      this.storage = new Storage({ path: this.path });
       return helper.prepareMigrations(3);
     });
 
@@ -88,9 +84,7 @@ describe('JSON', function () {
   describe('executed', function () {
     beforeEach(function () {
       this.path    = __dirname + '/../tmp/umzug.json';
-      this.storage = new Storage({
-        storageOptions: { path: this.path }
-      });
+      this.storage = new Storage({ path: this.path });
       return helper.prepareMigrations(3);
     });