From 48d6e4a72141aae592c493f9a840249fb1b1f0dc Mon Sep 17 00:00:00 2001 From: Tamlyn Rhodes Date: Tue, 17 Apr 2018 15:15:45 +0100 Subject: [PATCH] feat: support passing array of migration options --- README.md | 9 ++- src/index.js | 113 ++++++++++++++++++------------ src/migration.js | 12 ++-- test/Umzug/constructor.test.js | 7 ++ test/Umzug/execute.test.js | 4 +- test/Umzug/findMigrations.test.js | 33 +++++++++ test/fixtures/index.js | 10 +-- 7 files changed, 125 insertions(+), 63 deletions(-) create mode 100644 test/Umzug/findMigrations.test.js diff --git a/README.md b/README.md index 5b8e02c8..e7607d27 100644 --- a/README.md +++ b/README.md @@ -327,7 +327,8 @@ It is possible to configure *umzug* instance by passing an object to the constru // The name of the negative method in migrations. downName: 'down', - // (advanced) you can pass an array of Migration instances instead of the options below + // This can be an object or an array of objects. Pass an array if you want to + // load from different paths, or have a different resolver for different file types migrations: { // The params that gets passed to the migrations. // Might be an array or a synchronous function which returns an array. @@ -352,7 +353,11 @@ It is possible to configure *umzug* instance by passing an object to the constru customResolver: function (sqlPath) { return { up: () => sequelize.query(require('fs').readFileSync(sqlPath, 'utf8')) } } - } + }, + + // (advanced) an array of pre-configured Migration instances. + // If this is set, `options.migrations` is ignored. + migrationInstances: [new Migration('custom-migration.js'), {...}] } ``` diff --git a/src/index.js b/src/index.js index b2601d63..18f780e4 100644 --- a/src/index.js +++ b/src/index.js @@ -30,7 +30,7 @@ module.exports = class Umzug extends EventEmitter { * @param {Object} [options.storageOptions] - The options for the storage. * Check the available storages for further details. * @param {Object|Array} [options.migrations] - options for loading migration - * files, or (advanced) an array of Migration instances + * files, or an array of options * @param {Array} [options.migrations.params] - The params that gets passed to * the migrations. Might be an array or a synchronous function which returns * an array. @@ -45,6 +45,9 @@ module.exports = class Umzug extends EventEmitter { * function that specifies how to get a migration object from a path. This * should return an object of the form { up: Function, down: Function }. * Without this defined, a regular javascript import will be performed. + * @param {Object|Array} [options.migrationInstances] - Array of `Migration` + * instances. This is an advanced use case that bypasses Umzug's normal migration + * loading logic. * @constructs Umzug */ constructor (options = {}) { @@ -63,17 +66,24 @@ module.exports = class Umzug extends EventEmitter { throw new Error('The logging-option should be either a function or false'); } - if (!Array.isArray(this.options.migrations)) { - this.options.migrations = { - params: [], - path: path.resolve(process.cwd(), 'migrations'), - pattern: /^\d+[\w-]+\.js$/, - traverseDirectories: false, - wrap: fun => fun, - ...this.options.migrations, - }; + if (Array.isArray(this.options.migrations)) { + if (this.options.migrations[0] instanceof Migration) { + this.options.migrationInstances = this.options.migrations; + this.options.migrations = [{}]; + } + } else { + this.options.migrations = [this.options.migrations]; } + this.options.migrations = this.options.migrations.map(migrationOptions => ({ + params: [], + path: path.resolve(process.cwd(), 'migrations'), + pattern: /^\d+[\w-]+\.js$/, + traverseDirectories: false, + wrap: fun => fun, + ...migrationOptions, + })); + this.storage = this._initStorage(); } @@ -119,7 +129,7 @@ module.exports = class Umzug extends EventEmitter { .tap(function (executed) { if (!executed || (options.method === 'down')) { let fun = (migration[options.method] || Bluebird.resolve); - let params = self.options.migrations.params; + let params = migration.options.params; if (typeof params === 'function') { params = params(); @@ -444,40 +454,20 @@ module.exports = class Umzug extends EventEmitter { * @returns {Promise.} * @private */ - _findMigrations (migrationPath) { - if (Array.isArray(this.options.migrations)) { - return Bluebird.resolve(this.options.migrations); - } - let isRoot = !migrationPath; - if (isRoot) { - migrationPath = this.options.migrations.path; + _findMigrations () { + if (this.options.migrationInstances) { + return Bluebird.resolve(this.options.migrationInstances); } + return Bluebird - .promisify(fs.readdir)(migrationPath) - .bind(this) - .map(function (file) { - let filePath = path.resolve(migrationPath, file); - if (this.options.migrations.traverseDirectories) { - if (fs.lstatSync(filePath).isDirectory()) { - return this._findMigrations(filePath) - .then(function (migrations) { - return migrations; - }); - } - } - if (this.options.migrations.pattern.test(file)) { - return new Migration(filePath, this.options); - } - this.log('File: ' + file + ' does not match pattern: ' + this.options.migrations.pattern); - return file; - }) - .reduce(function (a, b) { return a.concat(b); }, []) // flatten the result to an array - .filter(function (file) { - return file instanceof Migration; // only care about Migration - }) - .then(function (migrations) { - if (isRoot) { // only sort if its root - return migrations.sort(function (a, b) { + .map( + this.options.migrations, + migrationOptions => this._loadMigrationGroup(migrationOptions) + ) + .then(migrationGroups => + migrationGroups + .reduce((flattened, group) => flattened.concat(group)) + .sort(function (a, b) { if (a.file > b.file) { return 1; } else if (a.file < b.file) { @@ -485,10 +475,43 @@ module.exports = class Umzug extends EventEmitter { } else { return 0; } + }) + ); + } + + /** + * Load a set of migration files from a given folder with a given config + * + * @param {Object} migrationOptions - options for this group of migrations + * @returns {Promise.} + * @private + */ + _loadMigrationGroup (migrationOptions) { + const migrationPath = migrationOptions.path; + return Bluebird + .promisify(fs.readdir)(migrationPath) + .map(file => { + let filePath = path.resolve(migrationPath, file); + if (migrationOptions.traverseDirectories) { + if (fs.lstatSync(filePath).isDirectory()) { + return this._loadMigrationGroup({ + ...migrationOptions, + path: filePath, + }); + } + } + if (migrationOptions.pattern.test(file)) { + return new Migration(filePath, { + ...migrationOptions, + upName: this.options.upName, + downName: this.options.downName, }); } - return migrations; - }); + this.log('File: ' + file + ' does not match pattern: ' + migrationOptions.pattern); + return file; + }) + .reduce((a, b) => a.concat(b), []) // flatten the result to an array + .filter(file => file instanceof Migration); // only care about Migration } /** diff --git a/src/migration.js b/src/migration.js index 2dc14df1..359c900f 100644 --- a/src/migration.js +++ b/src/migration.js @@ -22,10 +22,8 @@ module.exports = class Migration { * module. * @param {String} options.downName - Name of the method `down` in migration * module. - * @param {Object} options.migrations - * @param {Migration~wrap} options.migrations.wrap - Wrapper function for - * migration methods. - * @param {Migration~customResolver} [options.migrations.customResolver] - A + * @param {function} options.wrap - Wrapper function for migration methods. + * @param {function} options.customResolver - A * function that specifies how to get a migration object from a path. This * should return an object of the form { up: Function, down: Function }. * Without this defined, a regular javascript import will be performed. @@ -46,8 +44,8 @@ module.exports = class Migration { * @returns {Promise.} Required migration module */ migration () { - if (typeof this.options.migrations.customResolver === 'function') { - return this.options.migrations.customResolver(this.path); + if (typeof this.options.customResolver === 'function') { + return this.options.customResolver(this.path); } if (this.path.match(/\.coffee$/)) { // 2.x compiler registration @@ -110,7 +108,7 @@ module.exports = class Migration { fun = migration.default[method] || migration[method]; } if (!fun) throw new Error('Could not find migration method: ' + method); - const wrappedFun = this.options.migrations.wrap(fun); + const wrappedFun = this.options.wrap(fun); await wrappedFun.apply(migration, args); } diff --git a/test/Umzug/constructor.test.js b/test/Umzug/constructor.test.js index b19a52fa..cddb0420 100644 --- a/test/Umzug/constructor.test.js +++ b/test/Umzug/constructor.test.js @@ -55,4 +55,11 @@ describe('constructor', function () { umzug.log(); expect(spy.called).to.be.true; }); + + it('converts migration options object to array', () => { + const umzug = new Umzug({ migrations: { traverseDirectories: true } }); + expect(umzug.options.migrations).to.be.an.array; + expect(umzug.options.migrations).to.have.length(1); + expect(umzug.options.migrations[0].traverseDirectories).to.be.true; + }); }); diff --git a/test/Umzug/execute.test.js b/test/Umzug/execute.test.js index d3f9a27c..595e8957 100644 --- a/test/Umzug/execute.test.js +++ b/test/Umzug/execute.test.js @@ -93,7 +93,7 @@ describe('execute', function () { }); it('calls the migration with the specified params', function () { - this.umzug.options.migrations.params = [1, 2, 3]; + this.umzug.options.migrations[0].params = [1, 2, 3]; return this.migrate('up').then(() => { expect(this.upStub.getCall(0).args).to.eql([1, 2, 3]); @@ -101,7 +101,7 @@ describe('execute', function () { }); it('calls the migration with the result of the passed function', function () { - this.umzug.options.migrations.params = () => { + this.umzug.options.migrations[0].params = () => { return [1, 2, 3]; }; diff --git a/test/Umzug/findMigrations.test.js b/test/Umzug/findMigrations.test.js new file mode 100644 index 00000000..71ffc113 --- /dev/null +++ b/test/Umzug/findMigrations.test.js @@ -0,0 +1,33 @@ +import { expect } from 'chai'; +import helper from '../helper'; +import Umzug from '../../src/index'; +import { join } from 'path'; + +describe('_findMigrations', function () { + beforeEach(function () { + helper.clearTmp(); + return helper.prepareMigrations(4, { + directories: [['one'], ['one'], ['two', '1'], ['two', '2']], + }); + }); + + it('loads files with each config', async function () { + const umzug = new Umzug({ + migrations: [ + { path: join(__dirname, '..', 'tmp', 'one') }, + { + path: join(__dirname, '..', 'tmp', 'two'), + pattern: /3-migration/, + traverseDirectories: true, + }, + ], + }); + const migrations = await umzug._findMigrations(); + expect(migrations).to.have.length(3); + expect(migrations.map(m => m.file)).to.eql([ + '1-migration.js', + '2-migration.js', + '3-migration.js', + ]); + }); +}); diff --git a/test/fixtures/index.js b/test/fixtures/index.js index 21649285..be4fe80e 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -57,22 +57,18 @@ describe('custom resolver', () => { await this.verifyTables(); }); - it('an array of migrations created manually can be passed in', async function () { + it('an array of migrations instances created manually can be passed in', async function () { const umzug = new Umzug({ migrations: [ new Migration(require.resolve('./javascript/1.users'), { upName: 'up', downName: 'down', - migrations: { - wrap: fn => () => fn(this.sequelize.getQueryInterface(), this.sequelize.constructor), - }, + wrap: fn => () => fn(this.sequelize.getQueryInterface(), this.sequelize.constructor), }), new Migration(require.resolve('./javascript/2.things'), { upName: 'up', downName: 'down', - migrations: { - wrap: fn => () => fn(this.sequelize.getQueryInterface(), this.sequelize.constructor), - }, + wrap: fn => () => fn(this.sequelize.getQueryInterface(), this.sequelize.constructor), }), ], storage: 'sequelize',