From e2e83a0f7bfa03bd18ca0cafb7db9c3903291470 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Thu, 6 Oct 2016 15:50:55 +0200 Subject: [PATCH] Migration: New database versioning (#7499) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #7489 - new database versioning scheme which is based upon the Ghost version, and so easier to reason about - massive refactor of all the version related code Summary of changes: * ✨ new error: DatabaseNotSeeded * 🎨 change versioning module - versioning is based on Ghost Version * 🎨 change bootUp file - add big picture description - version error get's trigger from versioning module * 🎨 default setting for database version is null - very important change: this is caused by the big picture - see bootUp description - the database version get's set by the seed script later - db version is by default null - 1. population happens (we ensure that this has finished, by checking if each table exists) - 2. seeds happening (we ensure that seeds happend if database version is set to X.X) * 🎨 temporary change for population logic - set database version after population happens - ensure population of default settings happend before - both: get's removed in next iteration * 🎨 adapt tests && mark TODO's * 🎨 err instance checking --- core/server/data/migration/populate.js | 12 +++ core/server/data/schema/bootup.js | 34 ++++--- core/server/data/schema/default-settings.json | 2 +- core/server/data/schema/versioning.js | 98 ++++++++++++------- core/server/errors.js | 6 ++ core/server/translations/en.json | 1 + core/test/integration/api/api_db_spec.js | 2 - .../integration/model/model_posts_spec.js | 12 +++ .../integration/model/model_users_spec.js | 12 +++ core/test/unit/migration_spec.js | 12 ++- core/test/unit/server_spec.js | 39 -------- core/test/unit/versioning_spec.js | 58 +++++++---- core/test/utils/index.js | 4 + 13 files changed, 179 insertions(+), 113 deletions(-) diff --git a/core/server/data/migration/populate.js b/core/server/data/migration/populate.js index 461846b373..3b1daaec6b 100644 --- a/core/server/data/migration/populate.js +++ b/core/server/data/migration/populate.js @@ -3,9 +3,11 @@ var Promise = require('bluebird'), _ = require('lodash'), commands = require('../schema').commands, + versioning = require('../schema').versioning, fixtures = require('./fixtures'), db = require('../../data/db'), logging = require('../../logging'), + models = require('../../models'), errors = require('../../errors'), schema = require('../schema').tables, schemaTables = Object.keys(schema), @@ -40,6 +42,16 @@ populate = function populate(options) { return Promise.mapSeries(schemaTables, function createTable(table) { logger.info('Creating table: ' + table); return commands.createTable(table, transaction); + }).then(function () { + // @TODO: + // - key: migrations-kate + // - move to seed + return models.Settings.populateDefaults(_.merge({}, {transacting: transaction}, modelOptions)); + }).then(function () { + // @TODO: + // - key: migrations-kate + // - move to seed + return versioning.setDatabaseVersion(transaction); }).then(function populateFixtures() { if (tablesOnly) { return; diff --git a/core/server/data/schema/bootup.js b/core/server/data/schema/bootup.js index e17b3deff9..b12ca9f194 100644 --- a/core/server/data/schema/bootup.js +++ b/core/server/data/schema/bootup.js @@ -4,20 +4,30 @@ var Promise = require('bluebird'), errors = require('./../../errors'); module.exports = function bootUp() { + /** + * @TODO: + * - 1. call is check if tables are populated + * - 2. call is check if db is seeded + * + * These are the calls Ghost will make to find out if the db is in OK state! + * These check's will have nothing to do with the migration module! + * Ghost will not touch the migration module at all. + * + * Example code: + * models.Settings.findOne({key: 'databasePopulated'}) + * If not, throw error and tell user what to do (ghost db-init)! + * + * versioning.getDatabaseVersion() - not sure about that yet. + * This will read the database version of the settings table! + * If not, throw error and tell user what to do (ghost db-seed)! + * + * @TODO: + * - remove return populate() -> belongs to db init + * - key: migrations-kate + */ return versioning .getDatabaseVersion() - .then(function successHandler(result) { - if (!/^alpha/.test(result)) { - // This database was not created with Ghost alpha, and is not compatible - throw new errors.DatabaseVersionError({ - message: 'Your database version is not compatible with Ghost 1.0.0 Alpha (master branch)', - context: 'Want to keep your DB? Use Ghost < 1.0.0 or the "stable" branch. Otherwise please delete your DB and restart Ghost', - help: 'More information on the Ghost 1.0.0 Alpha at https://support.ghost.org/v1-0-alpha' - }); - } - }, - // We don't use .catch here, as it would catch the error from the successHandler - function errorHandler(err) { + .catch(function errorHandler(err) { if (err instanceof errors.DatabaseNotPopulatedError) { return populate(); } diff --git a/core/server/data/schema/default-settings.json b/core/server/data/schema/default-settings.json index 736576e4bd..73617084ea 100644 --- a/core/server/data/schema/default-settings.json +++ b/core/server/data/schema/default-settings.json @@ -1,7 +1,7 @@ { "core": { "databaseVersion": { - "defaultValue": "alpha.1" + "defaultValue": null }, "dbHash": { "defaultValue": null diff --git a/core/server/data/schema/versioning.js b/core/server/data/schema/versioning.js index 484867859a..351ee5733e 100644 --- a/core/server/data/schema/versioning.js +++ b/core/server/data/schema/versioning.js @@ -1,60 +1,82 @@ -var path = require('path'), - Promise = require('bluebird'), - db = require('../db'), - errors = require('../../errors'), - i18n = require('../../i18n'), - defaultSettings = require('./default-settings'), +var path = require('path'), + Promise = require('bluebird'), + db = require('../db'), + errors = require('../../errors'), + config = require('../../config'), + i18n = require('../../i18n'); - defaultDatabaseVersion; - -// Newest Database Version -// The migration version number according to the hardcoded default settings -// This is the version the database should be at or migrated to -function getNewestDatabaseVersion() { - if (!defaultDatabaseVersion) { - // This be the current version according to the software - defaultDatabaseVersion = defaultSettings.core.databaseVersion.defaultValue; +/** + * Database version has always two digits + * Database version is Ghost Version X.X + * + * @TODO: remove alpha text! + * @TODO: extend database validation + */ +function validateDatabaseVersion(version) { + if (version === null) { + throw new errors.DatabaseNotSeededError({ + message: i18n.t('errors.data.versioning.index.databaseNotSeeded') + }); } - return defaultDatabaseVersion; + if (!version.match(/\d\.\d/gi)) { + throw new errors.DatabaseVersionError({ + message: 'Your database version is not compatible with Ghost 1.0.0 Alpha (master branch)', + context: 'Want to keep your DB? Use Ghost < 1.0.0 or the "stable" branch. Otherwise please delete your DB and restart Ghost', + help: 'More information on the Ghost 1.0.0 Alpha at https://support.ghost.org/v1-0-alpha' + }); + } + + return version; } -// Database Current Version -// The migration version number according to the database -// This is what the database is currently at and may need to be updated +/** + * If the database version is null, the database was never seeded. + * The seed migration script will set your database to current Ghost Version. + */ function getDatabaseVersion() { return db.knex.schema.hasTable('settings').then(function (exists) { - // Check for the current version from the settings table - if (exists) { - // Temporary code to deal with old databases with currentVersion settings - return db.knex('settings') - .where('key', 'databaseVersion') - .first('value') - .then(function (version) { - return version.value; - }); + if (!exists) { + return Promise.reject(new errors.DatabaseNotPopulatedError({ + message: i18n.t('errors.data.versioning.index.databaseNotPopulated') + })); } - return Promise.reject(new errors.DatabaseNotPopulatedError({message: i18n.t('errors.data.versioning.index.databaseNotPopulated')})); + return db.knex('settings') + .where('key', 'databaseVersion') + .first('value') + .then(function (version) { + return validateDatabaseVersion(version.value); + }); }); } -function setDatabaseVersion(transaction, version) { +function getNewestDatabaseVersion() { + return config.get('ghostVersion').slice(0, 3); +} + +/** + * Database version cannot set from outside. + * If this function get called, we set the database version to your current Ghost version. + */ +function setDatabaseVersion(transaction) { return (transaction || db.knex)('settings') .where('key', 'databaseVersion') - .update({value: version || defaultDatabaseVersion}); -} - -function pad(num, width) { - return Array(Math.max(width - String(num).length + 1, 0)).join(0) + num; + .update({ + value: getNewestDatabaseVersion() + }); } +/** + * return the versions which need migration + * when on 1.1 and we update to 1.4, we expect [1.2, 1.3, 1.4] + */ function getMigrationVersions(fromVersion, toVersion) { var versions = [], i; - for (i = parseInt(fromVersion, 10); i <= toVersion; i += 1) { - versions.push(pad(i, 3)); + for (i = (fromVersion * 10) + 1; i <= toVersion * 10; i += 1) { + versions.push((i / 10).toString()); } return versions; @@ -92,7 +114,7 @@ function getUpdateFixturesTasks(version, logger) { } module.exports = { - canMigrateFromVersion: '003', + canMigrateFromVersion: '1.0', getNewestDatabaseVersion: getNewestDatabaseVersion, getDatabaseVersion: getDatabaseVersion, setDatabaseVersion: setDatabaseVersion, diff --git a/core/server/errors.js b/core/server/errors.js index 583a35c45e..7800b60156 100644 --- a/core/server/errors.js +++ b/core/server/errors.js @@ -89,6 +89,12 @@ var errors = { errorType: 'DatabaseNotPopulatedError' }, options)); }, + DatabaseNotSeededError: function DatabaseNotSeededError(options) { + GhostError.call(this, _.merge({ + statusCode: 500, + errorType: 'DatabaseNotSeededError' + }, options)); + }, UnauthorizedError: function UnauthorizedError(options) { GhostError.call(this, _.merge({ statusCode: 401, diff --git a/core/server/translations/en.json b/core/server/translations/en.json index 4bf4a1b710..9245bb0b23 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -440,6 +440,7 @@ "index": { "dbVersionNotRecognized": "Database version is not recognized", "databaseNotPopulated": "Database is not populated.", + "databaseNotSeeded": "Database is not seeded.", "cannotMigrate": { "error": "Unable to upgrade from version 0.4.2 or earlier.", "context": "Please upgrade to 0.7.1 first." diff --git a/core/test/integration/api/api_db_spec.js b/core/test/integration/api/api_db_spec.js index 9d37dbf8e6..89f3ef2149 100644 --- a/core/test/integration/api/api_db_spec.js +++ b/core/test/integration/api/api_db_spec.js @@ -1,8 +1,6 @@ var testUtils = require('../../utils'), should = require('should'), _ = require('lodash'), - - // Stuff we are testing dbAPI = require('../../../server/api/db'), ModelTag = require('../../../server/models/tag'), ModelPost = require('../../../server/models/post'); diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index 948caac0eb..38494c7f9f 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -10,6 +10,7 @@ var testUtils = require('../../utils'), ghostBookshelf = require('../../../server/models/base'), PostModel = require('../../../server/models/post').Post, TagModel = require('../../../server/models/tag').Tag, + models = require('../../../server/models'), events = require('../../../server/events'), errors = require('../../../server/errors'), DataGenerator = testUtils.DataGenerator, @@ -26,6 +27,17 @@ describe('Post Model', function () { beforeEach(function () { eventSpy = sandbox.spy(events, 'emit'); + + /** + * @TODO: + * - key: migrations-kate + * - this is not pretty + * - eventSpy get's now more events then expected + * - because on migrations.populate we trigger populateDefaults + * - how to solve? eventSpy must be local and not global? + */ + models.init(); + sandbox.stub(models.Settings, 'populateDefaults').returns(Promise.resolve()); }); afterEach(function () { diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index 41076965aa..6bf92686cb 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -10,6 +10,7 @@ var testUtils = require('../../utils'), gravatar = require('../../../server/utils/gravatar'), UserModel = require('../../../server/models/user').User, RoleModel = require('../../../server/models/role').Role, + models = require('../../../server/models'), events = require('../../../server/events'), context = testUtils.context.admin, sandbox = sinon.sandbox.create(); @@ -29,6 +30,17 @@ describe('User Model', function run() { beforeEach(function () { eventSpy = sandbox.spy(events, 'emit'); + + /** + * @TODO: + * - key: migrations-kate + * - this is not pretty + * - eventSpy get's now more events then expected + * - because on migrations.populate we trigger populateDefaults + * - how to solve? eventSpy must be local and not global? + */ + models.init(); + sandbox.stub(models.Settings, 'populateDefaults').returns(Promise.resolve()); }); describe('Registration', function runRegistration() { diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index c4f8104a51..7bc90d9239 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -161,7 +161,7 @@ describe('Migrations', function () { }); describe('Populate', function () { - var createStub, fixturesStub; + var createStub, fixturesStub, setDatabaseVersionStub, populateDefaultsStub; beforeEach(function () { fixturesStub = sandbox.stub(fixtures, 'populate').returns(new Promise.resolve()); @@ -169,10 +169,14 @@ describe('Migrations', function () { it('should create all tables, and populate fixtures', function (done) { createStub = sandbox.stub(schema.commands, 'createTable').returns(new Promise.resolve()); + setDatabaseVersionStub = sandbox.stub(schema.versioning, 'setDatabaseVersion').returns(new Promise.resolve()); + populateDefaultsStub = sandbox.stub(models.Settings, 'populateDefaults').returns(new Promise.resolve()); populate().then(function (result) { should.not.exist(result); + populateDefaultsStub.called.should.be.true(); + setDatabaseVersionStub.called.should.be.true(); createStub.called.should.be.true(); createStub.callCount.should.be.eql(schemaTables.length); createStub.firstCall.calledWith(schemaTables[0]).should.be.true(); @@ -224,7 +228,7 @@ describe('Migrations', function () { }); it('should throw error if versions are too old', function () { - var response = update.isDatabaseOutOfDate({fromVersion: '000', toVersion: '002'}); + var response = update.isDatabaseOutOfDate({fromVersion: '0.8', toVersion: '1.0'}); updateDatabaseSchemaStub.calledOnce.should.be.false(); (response.error instanceof errors.DatabaseVersionError).should.eql(true); }); @@ -232,7 +236,7 @@ describe('Migrations', function () { it('should just return if versions are the same', function () { var migrateToDatabaseVersionStub = sandbox.stub().returns(new Promise.resolve()), migrateToDatabaseVersionReset = update.__set__('migrateToDatabaseVersion', migrateToDatabaseVersionStub), - response = update.isDatabaseOutOfDate({fromVersion: '004', toVersion: '004'}); + response = update.isDatabaseOutOfDate({fromVersion: '1.0', toVersion: '1.0'}); response.migrate.should.eql(false); versionsSpy.calledOnce.should.be.false(); @@ -241,7 +245,7 @@ describe('Migrations', function () { }); it('should throw an error if the database version is higher than the default', function () { - var response = update.isDatabaseOutOfDate({fromVersion: '010', toVersion: '004'}); + var response = update.isDatabaseOutOfDate({fromVersion: '1.3', toVersion: '1.2'}); updateDatabaseSchemaStub.calledOnce.should.be.false(); (response.error instanceof errors.DatabaseVersionError).should.eql(true); }); diff --git a/core/test/unit/server_spec.js b/core/test/unit/server_spec.js index 3ab516c11e..639c128cd0 100644 --- a/core/test/unit/server_spec.js +++ b/core/test/unit/server_spec.js @@ -125,44 +125,5 @@ describe('server bootstrap', function () { done(err); }); }); - - // @TODO remove these temporary tests ;) - it('TEMP: database does exist: expect alpha error', function (done) { - sandbox.stub(migration.update, 'isDatabaseOutOfDate').returns({migrate:false}); - sandbox.spy(migration.update, 'execute'); - - sandbox.stub(versioning, 'getDatabaseVersion', function () { - return Promise.resolve('006'); - }); - - bootstrap() - .then(function () { - done('This should not be called'); - }) - .catch(function (err) { - err.errorType.should.eql('DatabaseVersionError'); - err.message.should.eql('Your database version is not compatible with Ghost 1.0.0 Alpha (master branch)'); - done(); - }); - }); - - it('TEMP: database does exist: expect alpha error', function (done) { - sandbox.stub(migration.update, 'isDatabaseOutOfDate').returns({migrate:true}); - sandbox.stub(migration.update, 'execute').returns(Promise.resolve()); - - sandbox.stub(versioning, 'getDatabaseVersion', function () { - return Promise.resolve('006'); - }); - - bootstrap() - .then(function () { - done('This should not be called'); - }) - .catch(function (err) { - err.errorType.should.eql('DatabaseVersionError'); - err.message.should.eql('Your database version is not compatible with Ghost 1.0.0 Alpha (master branch)'); - done(); - }); - }); }); }); diff --git a/core/test/unit/versioning_spec.js b/core/test/unit/versioning_spec.js index deb989668b..e2f4564d5f 100644 --- a/core/test/unit/versioning_spec.js +++ b/core/test/unit/versioning_spec.js @@ -1,13 +1,13 @@ -var should = require('should'), - sinon = require('sinon'), +var should = require('should'), + sinon = require('sinon'), Promise = require('bluebird'), // Stuff we are testing versioning = require('../../server/data/schema').versioning, - db = require('../../server/data/db'), - errors = require('../../server/errors'), + db = require('../../server/data/db'), + errors = require('../../server/errors'), - sandbox = sinon.sandbox.create(); + sandbox = sinon.sandbox.create(); describe('Versioning', function () { afterEach(function () { @@ -17,25 +17,26 @@ describe('Versioning', function () { describe('getMigrationVersions', function () { it('should output a single item if the from and to versions are the same', function () { should.exist(versioning.getMigrationVersions); - versioning.getMigrationVersions('003', '003').should.eql(['003']); - versioning.getMigrationVersions('004', '004').should.eql(['004']); + versioning.getMigrationVersions('1.0', '1.0').should.eql([]); + versioning.getMigrationVersions('1.2', '1.2').should.eql([]); }); it('should output an empty array if the toVersion is higher than the fromVersion', function () { - versioning.getMigrationVersions('003', '002').should.eql([]); + versioning.getMigrationVersions('1.2', '1.1').should.eql([]); }); it('should output all the versions between two versions', function () { - versioning.getMigrationVersions('003', '004').should.eql(['003', '004']); - versioning.getMigrationVersions('003', '005').should.eql(['003', '004', '005']); - versioning.getMigrationVersions('003', '006').should.eql(['003', '004', '005', '006']); - versioning.getMigrationVersions('010', '011').should.eql(['010', '011']); + versioning.getMigrationVersions('1.0', '1.1').should.eql(['1.1']); + versioning.getMigrationVersions('1.0', '1.2').should.eql(['1.1', '1.2']); + versioning.getMigrationVersions('1.2', '1.5').should.eql(['1.3', '1.4', '1.5']); + versioning.getMigrationVersions('2.1', '2.2').should.eql(['2.2']); }); }); describe('getNewestDatabaseVersion', function () { it('should return the correct version', function () { - var currentVersion = require('../../server/data/schema').defaultSettings.core.databaseVersion.defaultValue; + var currentVersion = '1.0'; + // This function has an internal cache, so we call it twice. // First, to check that it fetches the correct version from default-settings.json. versioning.getNewestDatabaseVersion().should.eql(currentVersion); @@ -59,7 +60,11 @@ describe('Versioning', function () { }; // this MUST use sinon, not sandbox, see sinonjs/sinon#781 - knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); + knexStub = sinon.stub(db, 'knex', { + get: function () { + return knexMock; + } + }); }); afterEach(function () { @@ -89,12 +94,12 @@ describe('Versioning', function () { it('should lookup & return version, if settings table exists', function (done) { // Setup knexMock.schema.hasTable.returns(new Promise.resolve(true)); - queryMock.first.returns(new Promise.resolve({value: '001'})); + queryMock.first.returns(new Promise.resolve({value: '1.0'})); // Execute versioning.getDatabaseVersion().then(function (version) { should.exist(version); - version.should.eql('001'); + version.should.eql('1.0'); knexStub.get.calledTwice.should.be.true(); knexMock.schema.hasTable.calledOnce.should.be.true(); @@ -155,6 +160,21 @@ describe('Versioning', function () { done(); }).catch(done); }); + + it('database does exist: expect alpha error', function (done) { + knexMock.schema.hasTable.returns(new Promise.resolve(true)); + queryMock.first.returns(new Promise.resolve({value: '008'})); + + versioning.getDatabaseVersion() + .then(function () { + done('Should throw an error if version is not a number'); + }) + .catch(function (err) { + (err instanceof errors.DatabaseVersionError).should.eql(true); + err.message.should.eql('Your database version is not compatible with Ghost 1.0.0 Alpha (master branch)'); + done(); + }); + }); }); describe('setDatabaseVersion', function () { @@ -169,7 +189,11 @@ describe('Versioning', function () { knexMock = sandbox.stub().returns(queryMock); // this MUST use sinon, not sandbox, see sinonjs/sinon#781 - knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); + knexStub = sinon.stub(db, 'knex', { + get: function () { + return knexMock; + } + }); }); afterEach(function () { diff --git a/core/test/utils/index.js b/core/test/utils/index.js index cd8179c40e..bc3f7bee5f 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -457,6 +457,10 @@ toDoList = { * * `perms:obj` - initialise permissions for a particular object type * * `users:roles` - create a full suite of users, one per role * @param {Object} toDos + * + * @TODO: + * - key: migrations-kate + * - call migration-runner */ getFixtureOps = function getFixtureOps(toDos) { // default = default fixtures, if it isn't present, init with tables only