From 81deb88263f364dd875348947923a5cff9da1c32 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Fri, 11 Mar 2016 22:46:05 +0000 Subject: [PATCH] Improve `getDatabaseVersion` & versioning tests refs #6301 - `currentVersion` was leftover from before the first public release of Ghost! - simplified the code for `getDatabaseVersion` - improved & made consistent how errors are handled in `getDatabaseVersion` - migration error handling updated to reflect the changes in `getDatabaseVersion` - added tests for both `getDatabaseVersion` and `setDatabaseVersion` --- core/server/data/migration/index.js | 2 +- core/server/data/schema/versioning.js | 29 ++--- core/test/integration/api/api_db_spec.js | 2 +- core/test/unit/versioning_spec.js | 151 ++++++++++++++++++++++- 4 files changed, 161 insertions(+), 23 deletions(-) diff --git a/core/server/data/migration/index.js b/core/server/data/migration/index.js index 7a781ce66a..dbf1a3d783 100644 --- a/core/server/data/migration/index.js +++ b/core/server/data/migration/index.js @@ -110,7 +110,7 @@ init = function (tablesOnly) { ); } }, function (err) { - if (err.message || err === 'Settings table does not exist') { + if (err && err.message === 'Settings table does not exist') { // 4. The database has not yet been created // Bring everything up from initial version. logInfo('Database initialisation required for version ' + versioning.getDefaultDatabaseVersion()); diff --git a/core/server/data/schema/versioning.js b/core/server/data/schema/versioning.js index b14843c679..d7283fa04d 100644 --- a/core/server/data/schema/versioning.js +++ b/core/server/data/schema/versioning.js @@ -1,10 +1,8 @@ -var _ = require('lodash'), - db = require('../db'), +var db = require('../db'), errors = require('../../errors'), i18n = require('../../i18n'), defaultSettings = require('./default-settings'), - initialVersion = '000', defaultDatabaseVersion; // Default Database Version @@ -29,25 +27,20 @@ function getDatabaseVersion() { // Temporary code to deal with old databases with currentVersion settings return db.knex('settings') .where('key', 'databaseVersion') - .orWhere('key', 'currentVersion') - .select('value') - .then(function (versions) { - var databaseVersion = _.reduce(versions, function (memo, version) { - if (isNaN(version.value)) { - errors.throwError(i18n.t('errors.data.versioning.index.dbVersionNotRecognized')); - } - return parseInt(version.value, 10) > parseInt(memo, 10) ? version.value : memo; - }, initialVersion); - - if (!databaseVersion || databaseVersion.length === 0) { - // we didn't get a response we understood, assume initialVersion - databaseVersion = initialVersion; + .first('value') + .then(function (version) { + if (!version || isNaN(version.value)) { + return errors.rejectError(new Error( + i18n.t('errors.data.versioning.index.dbVersionNotRecognized') + )); } - return databaseVersion; + return version.value; }); } - throw new Error(i18n.t('errors.data.versioning.index.settingsTableDoesNotExist')); + return errors.rejectError(new Error( + i18n.t('errors.data.versioning.index.settingsTableDoesNotExist') + )); }); } diff --git a/core/test/integration/api/api_db_spec.js b/core/test/integration/api/api_db_spec.js index 3478994cf0..5faa92b8ab 100644 --- a/core/test/integration/api/api_db_spec.js +++ b/core/test/integration/api/api_db_spec.js @@ -12,7 +12,7 @@ describe('DB API', function () { // Keep the DB clean before(testUtils.teardown); afterEach(testUtils.teardown); - beforeEach(testUtils.setup('users:roles', 'posts', 'perms:db', 'perms:init')); + beforeEach(testUtils.setup('users:roles', 'settings', 'posts', 'perms:db', 'perms:init')); should.exist(dbAPI); diff --git a/core/test/unit/versioning_spec.js b/core/test/unit/versioning_spec.js index c435401f78..f97d2aeb8f 100644 --- a/core/test/unit/versioning_spec.js +++ b/core/test/unit/versioning_spec.js @@ -1,9 +1,11 @@ -/*globals describe, it, afterEach */ -var should = require('should'), - sinon = require('sinon'), +/*globals describe, it, afterEach, beforeEach */ +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'), sandbox = sinon.sandbox.create(); @@ -43,6 +45,149 @@ describe('Versioning', function () { }); }); + describe('getDatabaseVersion', function () { + var errorSpy, knexStub, knexMock, queryMock; + + beforeEach(function () { + errorSpy = sandbox.spy(errors, 'rejectError'); + + queryMock = { + where: sandbox.stub().returnsThis(), + first: sandbox.stub() + }; + + knexMock = sandbox.stub().returns(queryMock); + knexMock.schema = { + hasTable: sandbox.stub() + }; + + // this MUST use sinon, not sandbox, see sinonjs/sinon#781 + knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); + }); + + afterEach(function () { + knexStub.restore(); + }); + + it('should throw error if settings table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.returns(new Promise.resolve(false)); + + // Execute + versioning.getDatabaseVersion().then(function () { + done('Should throw an error if the settings table does not exist'); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Settings table does not exist'); + errorSpy.calledOnce.should.be.true(); + + knexStub.get.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('settings').should.be.true(); + queryMock.where.called.should.be.false(); + queryMock.first.called.should.be.false(); + done(); + }).catch(done); + }); + + 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'})); + + // Execute + versioning.getDatabaseVersion().then(function (version) { + should.exist(version); + version.should.eql('001'); + errorSpy.called.should.be.false(); + + knexStub.get.calledTwice.should.be.true(); + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('settings').should.be.true(); + queryMock.where.called.should.be.true(); + queryMock.first.called.should.be.true(); + + done(); + }).catch(done); + }); + + it('should throw error if version does not exist', function (done) { + // Setup + knexMock.schema.hasTable.returns(new Promise.resolve(true)); + queryMock.first.returns(new Promise.resolve()); + + // Execute + versioning.getDatabaseVersion().then(function () { + done('Should throw an error if version does not exist'); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Database version is not recognized'); + errorSpy.calledOnce.should.be.true(); + + knexStub.get.calledTwice.should.be.true(); + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('settings').should.be.true(); + queryMock.where.called.should.be.true(); + queryMock.first.called.should.be.true(); + + done(); + }).catch(done); + }); + + it('should throw error if version is not a number', function (done) { + // Setup + knexMock.schema.hasTable.returns(new Promise.resolve(true)); + queryMock.first.returns(new Promise.resolve('Eyjafjallajökull')); + + // Execute + versioning.getDatabaseVersion().then(function () { + done('Should throw an error if version is not a number'); + }).catch(function (err) { + should.exist(err); + err.message.should.eql('Database version is not recognized'); + errorSpy.calledOnce.should.be.true(); + + knexStub.get.calledTwice.should.be.true(); + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('settings').should.be.true(); + queryMock.where.called.should.be.true(); + queryMock.first.called.should.be.true(); + + done(); + }).catch(done); + }); + }); + + describe('setDatabaseVersion', function () { + var knexStub, knexMock, queryMock; + + beforeEach(function () { + queryMock = { + where: sandbox.stub().returnsThis(), + update: sandbox.stub().returns(new Promise.resolve()) + }; + + knexMock = sandbox.stub().returns(queryMock); + + // this MUST use sinon, not sandbox, see sinonjs/sinon#781 + knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); + }); + + afterEach(function () { + knexStub.restore(); + }); + + it('should try to update the databaseVersion', function (done) { + versioning.setDatabaseVersion().then(function () { + knexStub.get.calledOnce.should.be.true(); + queryMock.where.called.should.be.true(); + queryMock.update.called.should.be.true(); + + done(); + }).catch(done); + }); + }); + describe('showCannotMigrateError', function () { it('should output a detailed error message', function () { var errorStub = sandbox.stub(errors, 'logAndRejectError');