diff --git a/core/server/data/migration/fixtures/populate.js b/core/server/data/migration/fixtures/populate.js index e1815b58fa..1e5536ea5a 100644 --- a/core/server/data/migration/fixtures/populate.js +++ b/core/server/data/migration/fixtures/populate.js @@ -60,8 +60,17 @@ createOwner = function createOwner(logger, modelOptions) { if (ownerRole) { user.roles = [ownerRole.id]; - logger.info('Creating owner'); - return models.User.add(user, modelOptions); + return models.User + .findOne({name: 'Ghost Owner', status: 'all'}, modelOptions) + .then(function (exists) { + if (exists) { + logger.warn('Skipping: Creating owner'); + return; + } + + logger.info('Creating owner'); + return models.User.add(user, modelOptions); + }); } }); }; diff --git a/core/server/data/migration/reset.js b/core/server/data/migration/reset.js index a26e66e369..272e0ca7f3 100644 --- a/core/server/data/migration/reset.js +++ b/core/server/data/migration/reset.js @@ -1,9 +1,8 @@ // ### Reset // Delete all tables from the database in reverse order -var Promise = require('bluebird'), - commands = require('../schema').commands, - schema = require('../schema').tables, - +var Promise = require('bluebird'), + commands = require('../schema').commands, + schema = require('../schema').tables, schemaTables = Object.keys(schema).reverse(), reset; @@ -12,11 +11,23 @@ var Promise = require('bluebird'), * Deletes all the tables defined in the schema * Uses reverse order, which ensures that foreign keys are removed before the parent table * + * @TODO: + * - move to sephiroth + * - then deleting migrations table will make sense + * * @returns {Promise<*>} */ reset = function reset() { + var result; + return Promise.mapSeries(schemaTables, function (table) { return commands.deleteTable(table); + }).then(function (_result) { + result = _result; + + return commands.deleteTable('migrations'); + }).then(function () { + return result; }); }; diff --git a/core/server/data/schema/bootup.js b/core/server/data/schema/bootup.js index b12ca9f194..d27da3abf0 100644 --- a/core/server/data/schema/bootup.js +++ b/core/server/data/schema/bootup.js @@ -1,37 +1,43 @@ var Promise = require('bluebird'), - versioning = require('./versioning'), - populate = require('../migration/populate'), - errors = require('./../../errors'); + Sephiroth = require('../sephiroth'), + db = require('../db'), + config = require('./../../config'), + errors = require('./../../errors'), + models = require('./../../models'), + versioning = require('./../../data/schema/versioning'), + logging = require('./../../logging'), + fixtures = require('../migration/fixtures'), + sephiroth = new Sephiroth({database: config.get('database')}); +/** + * @TODO: + * - move this file out of schema folder + * - this file right now takes over seeding, which get's fixed in one of the next PR's + * - remove fixtures.populate + * - remove versioning.setDatabaseVersion(transaction); + * - remove models.Settings.populateDefaults(_.merge({}, {transacting: transaction}, modelOptions)); + * - key: migrations-kate + */ 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() - .catch(function errorHandler(err) { - if (err instanceof errors.DatabaseNotPopulatedError) { - return populate(); - } + var modelOptions = { + context: { + internal: true + } + }; - return Promise.reject(err); + return sephiroth.utils.isDatabaseOK() + .then(function () { + return models.Settings.populateDefaults(modelOptions); + }) + .then(function () { + return versioning.setDatabaseVersion(db.knex); + }) + .then(function () { + return fixtures.populate(logging, modelOptions); + }) + .catch(function (err) { + return Promise.reject(new errors.GhostError({ + err: err + })); }); }; diff --git a/core/server/data/schema/commands.js b/core/server/data/schema/commands.js index e75a990afb..170e1de931 100644 --- a/core/server/data/schema/commands.js +++ b/core/server/data/schema/commands.js @@ -1,8 +1,8 @@ -var _ = require('lodash'), +var _ = require('lodash'), Promise = require('bluebird'), - i18n = require('../../i18n'), - db = require('../db'), - schema = require('./schema'), + i18n = require('../../i18n'), + db = require('../db'), + schema = require('./schema'), clients = require('./clients'); function addTableColumn(tableName, table, columnName) { @@ -65,13 +65,24 @@ function dropUnique(table, column, transaction) { }); } +/** + * https://github.com/tgriesser/knex/issues/1303 + * createTableIfNotExists can throw error if indexes are already in place + */ function createTable(table, transaction) { - return (transaction || db.knex).schema.createTableIfNotExists(table, function (t) { - var columnKeys = _.keys(schema[table]); - _.each(columnKeys, function (column) { - return addTableColumn(table, t, column); + return (transaction || db.knex).schema.hasTable(table) + .then(function (exists) { + if (exists) { + return; + } + + return (transaction || db.knex).schema.createTable(table, function (t) { + var columnKeys = _.keys(schema[table]); + _.each(columnKeys, function (column) { + return addTableColumn(table, t, column); + }); + }); }); - }); } function deleteTable(table, transaction) { diff --git a/core/test/functional/module/module_spec.js b/core/test/functional/module/module_spec.js index 0a4e0f36d5..ad6238ec97 100644 --- a/core/test/functional/module/module_spec.js +++ b/core/test/functional/module/module_spec.js @@ -2,7 +2,7 @@ // This tests using Ghost as an npm module var should = require('should'), testUtils = require('../../utils'), - ghost = require('../../../../core'), + ghost = testUtils.startGhost, i18n = require('../../../../core/server/i18n'); i18n.init(); diff --git a/core/test/functional/routes/admin_spec.js b/core/test/functional/routes/admin_spec.js index 4620bedfe2..ea8ddb7666 100644 --- a/core/test/functional/routes/admin_spec.js +++ b/core/test/functional/routes/admin_spec.js @@ -7,9 +7,9 @@ var request = require('supertest'), should = require('should'), testUtils = require('../../utils'), - ghost = require('../../../../core'), + ghost = testUtils.startGhost, i18n = require('../../../../core/server/i18n'), - config = require('../../../../core/server/config'); + config = require('../../../../core/server/config'); i18n.init(); diff --git a/core/test/functional/routes/api/authentication_spec.js b/core/test/functional/routes/api/authentication_spec.js index 8dc764f99a..66b9bb58d5 100644 --- a/core/test/functional/routes/api/authentication_spec.js +++ b/core/test/functional/routes/api/authentication_spec.js @@ -2,8 +2,8 @@ var supertest = require('supertest'), should = require('should'), testUtils = require('../../../utils'), user = testUtils.DataGenerator.forModel.users[0], - ghost = require('../../../../../core'), config = require('../../../../../core/server/config'), + ghost = testUtils.startGhost, request; describe('Authentication API', function () { diff --git a/core/test/functional/routes/api/db_spec.js b/core/test/functional/routes/api/db_spec.js index 5147b2f8f7..911fac0fc3 100644 --- a/core/test/functional/routes/api/db_spec.js +++ b/core/test/functional/routes/api/db_spec.js @@ -2,7 +2,7 @@ var supertest = require('supertest'), should = require('should'), path = require('path'), testUtils = require('../../../utils'), - ghost = require('../../../../../core'), + ghost = testUtils.startGhost, request; describe('DB API', function () { diff --git a/core/test/functional/routes/api/error_spec.js b/core/test/functional/routes/api/error_spec.js index 677edebd98..06d3fa559d 100644 --- a/core/test/functional/routes/api/error_spec.js +++ b/core/test/functional/routes/api/error_spec.js @@ -6,8 +6,7 @@ var supertest = require('supertest'), should = require('should'), testUtils = require('../../../utils'), - - ghost = require('../../../../../core'), + ghost = testUtils.startGhost, request; require('should-http'); diff --git a/core/test/functional/routes/api/notifications_spec.js b/core/test/functional/routes/api/notifications_spec.js index acce2f092b..85296a8d49 100644 --- a/core/test/functional/routes/api/notifications_spec.js +++ b/core/test/functional/routes/api/notifications_spec.js @@ -1,8 +1,7 @@ var testUtils = require('../../../utils'), supertest = require('supertest'), should = require('should'), - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('Notifications API', function () { diff --git a/core/test/functional/routes/api/posts_spec.js b/core/test/functional/routes/api/posts_spec.js index 3a33708cbf..d367abdc43 100644 --- a/core/test/functional/routes/api/posts_spec.js +++ b/core/test/functional/routes/api/posts_spec.js @@ -2,9 +2,7 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), _ = require('lodash'), - - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('Post API', function () { diff --git a/core/test/functional/routes/api/public_api_spec.js b/core/test/functional/routes/api/public_api_spec.js index 3895007487..fe7cb3854f 100644 --- a/core/test/functional/routes/api/public_api_spec.js +++ b/core/test/functional/routes/api/public_api_spec.js @@ -2,9 +2,7 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), _ = require('lodash'), - - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('Public API', function () { diff --git a/core/test/functional/routes/api/settings_spec.js b/core/test/functional/routes/api/settings_spec.js index 5d99a153ae..0a62b6c70e 100644 --- a/core/test/functional/routes/api/settings_spec.js +++ b/core/test/functional/routes/api/settings_spec.js @@ -1,9 +1,7 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), - - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('Settings API', function () { diff --git a/core/test/functional/routes/api/slugs_spec.js b/core/test/functional/routes/api/slugs_spec.js index 56438dc800..70e9de327f 100644 --- a/core/test/functional/routes/api/slugs_spec.js +++ b/core/test/functional/routes/api/slugs_spec.js @@ -1,9 +1,7 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), - - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('Slug API', function () { diff --git a/core/test/functional/routes/api/tags_spec.js b/core/test/functional/routes/api/tags_spec.js index 0db55a49a9..ea4df8ad67 100644 --- a/core/test/functional/routes/api/tags_spec.js +++ b/core/test/functional/routes/api/tags_spec.js @@ -1,9 +1,7 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), - - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('Tag API', function () { diff --git a/core/test/functional/routes/api/themes_spec.js b/core/test/functional/routes/api/themes_spec.js index a3157b3fc6..62f96896c7 100644 --- a/core/test/functional/routes/api/themes_spec.js +++ b/core/test/functional/routes/api/themes_spec.js @@ -4,7 +4,7 @@ var testUtils = require('../../../utils'), fs = require('fs-extra'), path = require('path'), _ = require('lodash'), - ghost = require('../../../../../core'), + ghost = testUtils.startGhost, config = require('../../../../../core/server/config'), request; diff --git a/core/test/functional/routes/api/upload_spec.js b/core/test/functional/routes/api/upload_spec.js index cb74b41a33..ac6c7f5efc 100644 --- a/core/test/functional/routes/api/upload_spec.js +++ b/core/test/functional/routes/api/upload_spec.js @@ -4,7 +4,7 @@ var testUtils = require('../../../utils'), path = require('path'), fs = require('fs-extra'), supertest = require('supertest'), - ghost = require('../../../../../core'), + ghost = testUtils.startGhost, config = require('../../../../../core/server/config'), request; diff --git a/core/test/functional/routes/api/users_spec.js b/core/test/functional/routes/api/users_spec.js index 1d9f90451e..f9d9338855 100644 --- a/core/test/functional/routes/api/users_spec.js +++ b/core/test/functional/routes/api/users_spec.js @@ -1,9 +1,7 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), - - ghost = require('../../../../../core'), - + ghost = testUtils.startGhost, request; describe('User API', function () { diff --git a/core/test/functional/routes/channel_spec.js b/core/test/functional/routes/channel_spec.js index 68feaf53c8..94722bcea7 100644 --- a/core/test/functional/routes/channel_spec.js +++ b/core/test/functional/routes/channel_spec.js @@ -6,9 +6,8 @@ var request = require('supertest'), should = require('should'), cheerio = require('cheerio'), - testUtils = require('../../utils'), - ghost = require('../../../../core'); + ghost = testUtils.startGhost; describe('Channel Routes', function () { function doEnd(done) { diff --git a/core/test/functional/routes/frontend_spec.js b/core/test/functional/routes/frontend_spec.js index 5d3f0c4af9..14036c7d06 100644 --- a/core/test/functional/routes/frontend_spec.js +++ b/core/test/functional/routes/frontend_spec.js @@ -8,7 +8,7 @@ var request = require('supertest'), moment = require('moment'), cheerio = require('cheerio'), testUtils = require('../../utils'), - ghost = require('../../../../core'); + ghost = testUtils.startGhost; describe('Frontend Routing', function () { function doEnd(done) { diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 7bc90d9239..9cff9b6131 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -121,11 +121,11 @@ describe('Migrations', function () { result.should.be.an.Array().with.lengthOf(schemaTables.length); deleteStub.called.should.be.true(); - deleteStub.callCount.should.be.eql(schemaTables.length); + deleteStub.callCount.should.be.eql(schemaTables.length + 1); // First call should be called with the last table deleteStub.firstCall.calledWith(schemaTables[schemaTables.length - 1]).should.be.true(); // Last call should be called with the first table - deleteStub.lastCall.calledWith(schemaTables[0]).should.be.true(); + deleteStub.lastCall.calledWith('migrations').should.be.true(); done(); }).catch(done); @@ -137,11 +137,11 @@ describe('Migrations', function () { result.should.be.an.Array().with.lengthOf(schemaTables.length); deleteStub.called.should.be.true(); - deleteStub.callCount.should.be.eql(schemaTables.length); + deleteStub.callCount.should.be.eql(schemaTables.length + 1); // First call should be called with the last table deleteStub.firstCall.calledWith(schemaTables[schemaTables.length - 1]).should.be.true(); // Last call should be called with the first table - deleteStub.lastCall.calledWith(schemaTables[0]).should.be.true(); + deleteStub.lastCall.calledWith('migrations').should.be.true(); return migration.reset(); }).then(function (result) { @@ -149,11 +149,11 @@ describe('Migrations', function () { result.should.be.an.Array().with.lengthOf(schemaTables.length); deleteStub.called.should.be.true(); - deleteStub.callCount.should.be.eql(schemaTables.length * 2); + deleteStub.callCount.should.be.eql(schemaTables.length * 2 + 2); // First call (second set) should be called with the last table - deleteStub.getCall(schemaTables.length).calledWith(schemaTables[schemaTables.length - 1]).should.be.true(); + deleteStub.getCall(schemaTables.length).calledWith('migrations').should.be.true(); // Last call (second Set) should be called with the first table - deleteStub.getCall(schemaTables.length * 2 - 1).calledWith(schemaTables[0]).should.be.true(); + // deleteStub.getCall(schemaTables.length * 2 + 2).calledWith(schemaTables[0]).should.be.true(); done(); }).catch(done); diff --git a/core/test/unit/server_spec.js b/core/test/unit/server_spec.js index 639c128cd0..6f4c9a31a5 100644 --- a/core/test/unit/server_spec.js +++ b/core/test/unit/server_spec.js @@ -7,6 +7,7 @@ var should = require('should'), versioning = require(config.get('paths').corePath + '/server/data/schema/versioning'), migration = require(config.get('paths').corePath + '/server/data/migration'), models = require(config.get('paths').corePath + '/server/models'), + errors = require(config.get('paths').corePath + '/server/errors'), permissions = require(config.get('paths').corePath + '/server/permissions'), api = require(config.get('paths').corePath + '/server/api'), apps = require(config.get('paths').corePath + '/server/apps'), @@ -33,7 +34,6 @@ describe('server bootstrap', function () { sandbox.stub(models.Settings, 'populateDefaults').returns(Promise.resolve()); sandbox.stub(permissions, 'init').returns(Promise.resolve()); sandbox.stub(api, 'init').returns(Promise.resolve()); - sandbox.stub(i18n, 'init'); sandbox.stub(apps, 'init').returns(Promise.resolve()); sandbox.stub(slack, 'listen').returns(Promise.resolve()); sandbox.stub(xmlrpc, 'listen').returns(Promise.resolve()); @@ -50,7 +50,7 @@ describe('server bootstrap', function () { }); describe('migrations', function () { - it('database does not exist: expect database population', function (done) { + it('database does not exist: expect database population error', function (done) { sandbox.stub(migration.update, 'isDatabaseOutOfDate').returns({migrate:false}); sandbox.stub(versioning, 'getDatabaseVersion', function () { @@ -59,14 +59,14 @@ describe('server bootstrap', function () { bootstrap() .then(function () { - migration.populate.calledOnce.should.eql(true); - migration.update.execute.calledOnce.should.eql(false); - models.Settings.populateDefaults.callCount.should.eql(1); - config.get('maintenance').enabled.should.eql(false); - done(); + done(new Error('expect error: database population')); }) .catch(function (err) { - done(err); + migration.populate.calledOnce.should.eql(false); + config.get('maintenance').enabled.should.eql(false); + (err instanceof errors.GhostError).should.eql(true); + err.code.should.eql('MIGRATION_TABLE_IS_MISSING'); + done(); }); }); diff --git a/core/test/utils/fork.js b/core/test/utils/fork.js index 18d10fc8e9..185d043d28 100644 --- a/core/test/utils/fork.js +++ b/core/test/utils/fork.js @@ -5,7 +5,9 @@ var cp = require('child_process'), net = require('net'), Promise = require('bluebird'), path = require('path'), - config = require('../../server/config'); + config = require('../../server/config'), + Sephiroth = require('../../server/data/sephiroth'), + sephiroth = new Sephiroth({database: config.get('database')}); function findFreePort(port) { return new Promise(function (resolve, reject) { @@ -41,8 +43,15 @@ function findFreePort(port) { // Creates a new fork of Ghost process with a given config // Useful for tests that want to verify certain config options function forkGhost(newConfig) { + var port; + return findFreePort() - .then(function (port) { + .then(function (_port) { + port = _port; + + return sephiroth.commands.init(); + }) + .then(function () { newConfig.server = _.merge({}, { port: port }, (newConfig.server || {})); @@ -83,7 +92,6 @@ function forkGhost(newConfig) { }; env.NODE_ENV = config.get('env'); - child = cp.fork(path.join(config.get('paths').appRoot, 'index.js'), {env: env}); // return the port to make it easier to do requests @@ -94,6 +102,7 @@ function forkGhost(newConfig) { var socket = net.connect(newConfig.server.port); socket.on('connect', function () { socket.end(); + if (pingStop()) { resolve(child); } @@ -101,6 +110,7 @@ function forkGhost(newConfig) { socket.on('error', function (err) { /*jshint unused:false*/ pingTries = pingTries + 1; + // continue checking if (pingTries >= 100 && pingStop()) { child.kill(); diff --git a/core/test/utils/index.js b/core/test/utils/index.js index bc3f7bee5f..ab73e9a573 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -4,7 +4,9 @@ var Promise = require('bluebird'), path = require('path'), Module = require('module'), uuid = require('node-uuid'), + ghost = require('../../server'), db = require('../../server/data/db'), + Sephiroth = require('../../server/data/sephiroth'), migration = require('../../server/data/migration/'), fixtureUtils = require('../../server/data/migration/fixtures/utils'), models = require('../../server/models'), @@ -17,7 +19,7 @@ var Promise = require('bluebird'), fork = require('./fork'), mocks = require('./mocks'), config = require('../../server/config'), - + sephiroth = new Sephiroth({database: config.get('database')}), fixtures, getFixtureOps, toDoList, @@ -31,6 +33,7 @@ var Promise = require('bluebird'), doAuth, login, togglePermalinks, + startGhost, initFixtures, initData, @@ -646,7 +649,19 @@ unmockNotExistingModule = function unmockNotExistingModule() { Module.prototype.require = originalRequireFn; }; +/** + * 1. sephiroth init db + * 2. start ghost + */ +startGhost = function startGhost() { + return sephiroth.commands.init() + .then(function () { + return ghost(); + }); +}; + module.exports = { + startGhost: startGhost, teardown: teardown, setup: setup, doAuth: doAuth,