From bae0de6cd58be3688ad746bdb9b0f42a97d3c125 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Mon, 7 Nov 2016 12:39:49 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20knex-migrator=20v2=20(#7605)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🎨 knex-migrator reset [ci skip] * ✨ add migration example - hooks - 1.0 [ci skip] * 🛠 knex-migrator tarball - remove when released [ci skip] * 🎨 jscs/jshint * 🕵🏻 do not drop the database connection when running tests - please read the comments in the commit * 🔥 remove example migration * 🛠 knex-migrator 0.1.0 * 🛠 knex-migrator 0.1.1 - fix a single test to ensure we catch the error * 🛠 knex-migrator 0.1.2 * 🎨 make tests green - added my keyword: kate-migrations - i will go over all TODO's when removing the old migrations code * 🛠 knex-migrator update * 🛠 knex-migrator 0.2.0 --- .knex-migrator | 4 +- .../data/migrations/hooks/init/after.js | 30 +++++++++++++ .../data/migrations/hooks/init/before.js | 7 +++ .../data/migrations/hooks/init/index.js | 2 + .../data/migrations/hooks/migrate/after.js | 30 +++++++++++++ .../migrations/hooks/migrate/afterEach.js | 5 +++ .../data/migrations/hooks/migrate/before.js | 7 +++ .../migrations/hooks/migrate/beforeEach.js | 5 +++ .../data/migrations/hooks/migrate/index.js | 4 ++ .../data/migrations/init/2-create-fixtures.js | 4 -- .../data/migrations/init/3-insert-settings.js | 20 +-------- core/server/data/schema/backup.js | 44 +++++++++++++++++++ .../integration/model/model_posts_spec.js | 2 +- core/test/unit/migration_spec.js | 8 +++- core/test/unit/server_spec.js | 6 ++- core/test/utils/index.js | 19 +++++--- package.json | 2 +- 17 files changed, 165 insertions(+), 34 deletions(-) create mode 100644 core/server/data/migrations/hooks/init/after.js create mode 100644 core/server/data/migrations/hooks/init/before.js create mode 100644 core/server/data/migrations/hooks/init/index.js create mode 100644 core/server/data/migrations/hooks/migrate/after.js create mode 100644 core/server/data/migrations/hooks/migrate/afterEach.js create mode 100644 core/server/data/migrations/hooks/migrate/before.js create mode 100644 core/server/data/migrations/hooks/migrate/beforeEach.js create mode 100644 core/server/data/migrations/hooks/migrate/index.js create mode 100644 core/server/data/schema/backup.js diff --git a/.knex-migrator b/.knex-migrator index c457fe5de7..a3efe427f0 100644 --- a/.knex-migrator +++ b/.knex-migrator @@ -1,8 +1,10 @@ #!/usr/bin/env node -var config = require('./core/server/config'); +var config = require('./core/server/config'), + versioning = require('./core/server/data/schema/versioning'); module.exports = { + currentVersion: versioning.getNewestDatabaseVersion(), database: config.get('database'), migrationPath: config.get('paths:migrationPath') } diff --git a/core/server/data/migrations/hooks/init/after.js b/core/server/data/migrations/hooks/init/after.js new file mode 100644 index 0000000000..6e18f285ed --- /dev/null +++ b/core/server/data/migrations/hooks/init/after.js @@ -0,0 +1,30 @@ +var errors = require('../../../../errors'), + config = require('../../../../config'), + versioning = require('../../../schema/versioning'), + database = require('../../../db'); + +module.exports = function after(options) { + return versioning.getDatabaseVersion(options) + .catch(function (err) { + if (err instanceof errors.DatabaseVersionError && err.code === 'VERSION_DOES_NOT_EXIST') { + return versioning.setDatabaseVersion(options); + } + + throw err; + }) + .finally(function destroyConnection() { + // do not close database connection in test mode, because all tests are executed one after another + // this check is not nice, but there is only one other solution i can think of: + // forward a custom object to knex-migrator, which get's forwarded to the hooks + if (config.get('env').match(/testing/g)) { + return; + } + + // we need to close the database connection + // the after hook signals the last step of a knex-migrator command + // Example: + // Ghost-CLI calls knexMigrator.init and afterwards it starts Ghost, but Ghost-CLI can't shutdown + // if Ghost keeps a connection alive + return database.knex.destroy(); + }); +}; diff --git a/core/server/data/migrations/hooks/init/before.js b/core/server/data/migrations/hooks/init/before.js new file mode 100644 index 0000000000..006f4287f9 --- /dev/null +++ b/core/server/data/migrations/hooks/init/before.js @@ -0,0 +1,7 @@ +var Promise = require('bluebird'), + models = require('../../../../models'); + +module.exports = function before() { + models.init(); + return Promise.resolve(); +}; diff --git a/core/server/data/migrations/hooks/init/index.js b/core/server/data/migrations/hooks/init/index.js new file mode 100644 index 0000000000..e708882405 --- /dev/null +++ b/core/server/data/migrations/hooks/init/index.js @@ -0,0 +1,2 @@ +exports.after = require('./after'); +exports.before = require('./before'); diff --git a/core/server/data/migrations/hooks/migrate/after.js b/core/server/data/migrations/hooks/migrate/after.js new file mode 100644 index 0000000000..6e18f285ed --- /dev/null +++ b/core/server/data/migrations/hooks/migrate/after.js @@ -0,0 +1,30 @@ +var errors = require('../../../../errors'), + config = require('../../../../config'), + versioning = require('../../../schema/versioning'), + database = require('../../../db'); + +module.exports = function after(options) { + return versioning.getDatabaseVersion(options) + .catch(function (err) { + if (err instanceof errors.DatabaseVersionError && err.code === 'VERSION_DOES_NOT_EXIST') { + return versioning.setDatabaseVersion(options); + } + + throw err; + }) + .finally(function destroyConnection() { + // do not close database connection in test mode, because all tests are executed one after another + // this check is not nice, but there is only one other solution i can think of: + // forward a custom object to knex-migrator, which get's forwarded to the hooks + if (config.get('env').match(/testing/g)) { + return; + } + + // we need to close the database connection + // the after hook signals the last step of a knex-migrator command + // Example: + // Ghost-CLI calls knexMigrator.init and afterwards it starts Ghost, but Ghost-CLI can't shutdown + // if Ghost keeps a connection alive + return database.knex.destroy(); + }); +}; diff --git a/core/server/data/migrations/hooks/migrate/afterEach.js b/core/server/data/migrations/hooks/migrate/afterEach.js new file mode 100644 index 0000000000..385145cabc --- /dev/null +++ b/core/server/data/migrations/hooks/migrate/afterEach.js @@ -0,0 +1,5 @@ +var Promise = require('bluebird'); + +module.exports = function afterEach() { + return Promise.resolve(); +}; diff --git a/core/server/data/migrations/hooks/migrate/before.js b/core/server/data/migrations/hooks/migrate/before.js new file mode 100644 index 0000000000..4093dbdfa9 --- /dev/null +++ b/core/server/data/migrations/hooks/migrate/before.js @@ -0,0 +1,7 @@ +var backup = require('../../../schema/backup'), + models = require('../../../../models'); + +module.exports = function before(options) { + models.init(); + return backup(options); +}; diff --git a/core/server/data/migrations/hooks/migrate/beforeEach.js b/core/server/data/migrations/hooks/migrate/beforeEach.js new file mode 100644 index 0000000000..050b1103fb --- /dev/null +++ b/core/server/data/migrations/hooks/migrate/beforeEach.js @@ -0,0 +1,5 @@ +var Promise = require('bluebird'); + +module.exports = function beforeEach() { + return Promise.resolve(); +}; diff --git a/core/server/data/migrations/hooks/migrate/index.js b/core/server/data/migrations/hooks/migrate/index.js new file mode 100644 index 0000000000..4dea4fe883 --- /dev/null +++ b/core/server/data/migrations/hooks/migrate/index.js @@ -0,0 +1,4 @@ +exports.before = require('./before'); +exports.after = require('./after'); +exports.beforeEach = require('./beforeEach'); +exports.afterEach = require('./afterEach'); diff --git a/core/server/data/migrations/init/2-create-fixtures.js b/core/server/data/migrations/init/2-create-fixtures.js index 3a49a03666..609e1c66b1 100644 --- a/core/server/data/migrations/init/2-create-fixtures.js +++ b/core/server/data/migrations/init/2-create-fixtures.js @@ -1,17 +1,13 @@ var Promise = require('bluebird'), _ = require('lodash'), fixtures = require('../../schema/fixtures'), - models = require('../../../models'), logging = require('../../../logging'); -// @TODO: models.init module.exports = function insertFixtures(options) { var localOptions = _.merge({ context: {internal: true} }, options); - models.init(); - return Promise.mapSeries(fixtures.models, function (model) { logging.info('Model: ' + model.name); return fixtures.utils.addFixturesForModel(model, localOptions); diff --git a/core/server/data/migrations/init/3-insert-settings.js b/core/server/data/migrations/init/3-insert-settings.js index d3a9f5e7b7..ad81ee3497 100644 --- a/core/server/data/migrations/init/3-insert-settings.js +++ b/core/server/data/migrations/init/3-insert-settings.js @@ -1,23 +1,7 @@ var _ = require('lodash'), - models = require('../../../models'), - errors = require('../../../errors'), - versioning = require('../../schema/versioning'); + models = require('../../../models'); -// @TODO: models.init module.exports = function insertSettings(options) { var localOptions = _.merge({context: {internal: true}}, options); - - models.init(); - - return models.Settings.populateDefaults(localOptions) - .then(function () { - return versioning.getDatabaseVersion(localOptions); - }) - .catch(function (err) { - if (err instanceof errors.DatabaseVersionError && err.code === 'VERSION_DOES_NOT_EXIST') { - return versioning.setDatabaseVersion(localOptions); - } - - throw err; - }); + return models.Settings.populateDefaults(localOptions); }; diff --git a/core/server/data/schema/backup.js b/core/server/data/schema/backup.js new file mode 100644 index 0000000000..073906ff39 --- /dev/null +++ b/core/server/data/schema/backup.js @@ -0,0 +1,44 @@ +// # Backup Database +// Provides for backing up the database before making potentially destructive changes +var _ = require('lodash'), + fs = require('fs'), + path = require('path'), + Promise = require('bluebird'), + config = require('../../config'), + exporter = require('../export'), + + writeExportFile, + backup; + +writeExportFile = function writeExportFile(exportResult) { + var filename = path.resolve(config.get('paths').contentPath + '/data/' + exportResult.filename); + + return Promise.promisify(fs.writeFile)(filename, JSON.stringify(exportResult.data)).return(filename); +}; + +/** + * ## Backup + * does an export, and stores this in a local file + * + * @param {{info: logger.info, warn: logger.warn}} [logger] + * @returns {Promise<*>} + */ +backup = function backup(logger) { + // If we get passed a function, use it to output notices, else don't do anything + logger = logger && _.isFunction(logger.info) ? logger : {info: _.noop}; + + logger.info('Creating database backup'); + + var props = { + data: exporter.doExport(), + filename: exporter.fileName() + }; + + return Promise.props(props) + .then(writeExportFile) + .then(function successMessage(filename) { + logger.info('Database backup written to: ' + filename); + }); +}; + +module.exports = backup; diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index 38494c7f9f..8859634797 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -1698,7 +1698,7 @@ describe('Post Model', function () { startTags; // Step 1, fetch a post with its tags, just to see what tags we have - PostModel.findOne({id: postId}, {withRelated: ['tags']}).then(function (results) { + return PostModel.findOne({id: postId}, {withRelated: ['tags']}).then(function (results) { var post = results.toJSON(toJSONOpts); should.exist(results); post.title.should.not.equal('new title'); diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 9cff9b6131..4b51ef19ef 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -51,7 +51,13 @@ describe.skip('DB version integrity', function () { }); }); -describe('Migrations', function () { +/** + * we don't use our logic anymore + * so some tests are failing + * + * @TODO: kate-migrations + */ +describe.skip('Migrations', function () { var loggerStub, resetLogger; before(function () { diff --git a/core/test/unit/server_spec.js b/core/test/unit/server_spec.js index 38c77be5c4..5b7eeb1b43 100644 --- a/core/test/unit/server_spec.js +++ b/core/test/unit/server_spec.js @@ -64,7 +64,11 @@ describe('server bootstrap', function () { .catch(function (err) { migration.populate.calledOnce.should.eql(false); config.get('maintenance').enabled.should.eql(false); - err.code.should.eql('MIGRATION_TABLE_IS_MISSING'); + + // checking the error code is tricky, because it depends on other tests running before + // it's fine just checking the type of the error + // @TODO: kate-migrations (export errors in knex-migrator to be able to check instanceof) + err.errorType.should.eql('DatabaseIsNotOkError'); done(); }); }); diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 42e37f69ce..a19600a021 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -3,11 +3,11 @@ var Promise = require('bluebird'), fs = require('fs-extra'), path = require('path'), Module = require('module'), + debug = require('debug')('ghost:test'), uuid = require('node-uuid'), KnexMigrator = require('knex-migrator'), ghost = require('../../server'), db = require('../../server/data/db'), - migration = require('../../server/data/migration/'), fixtureUtils = require('../../server/data/migration/fixtures/utils'), models = require('../../server/models'), SettingsAPI = require('../../server/api/settings'), @@ -405,9 +405,10 @@ initData = function initData() { return knexMigrator.init(); }; +// we must always try to delete all tables clearData = function clearData() { - // we must always try to delete all tables - return migration.reset(); + debug('Database reset'); + return knexMigrator.reset(); }; toDoList = { @@ -623,12 +624,16 @@ togglePermalinks = function togglePermalinks(request, toggle) { }; teardown = function teardown(done) { + debug('Database reset'); + if (done) { - migration.reset().then(function () { - done(); - }).catch(done); + knexMigrator.reset() + .then(function () { + done(); + }) + .catch(done); } else { - return migration.reset(); + return knexMigrator.reset(); } }; diff --git a/package.json b/package.json index b9e6878acb..f733321719 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "intl-messageformat": "1.3.0", "jsonpath": "0.2.7", "knex": "0.12.5", - "knex-migrator": "0.0.7", + "knex-migrator": "0.2.0", "lodash": "4.16.6", "mobiledoc-html-renderer": "0.3.0", "moment": "2.15.2",