✨ use migration runner for init db (#7502)

refs #7489

* 🎨  protect error when creating owner
* 🎨  reset migration table
- temporary solution, see TODO's
* 🎨  use sephiroth in bootUp script
- do not populate the database
- ask sephiroth for database state
- do seeding manually (this will be removed in next seeding PR)
* 🎨  rewrite createTableIfNotExists because it causes error when running twice
- see knex issue
- hasTable and createTable
- indexes can cause trouble when calling them twice
* 🎨  tests: populate db in test env
- when forking db
- when starting ghost()
- this basically affects only the functional tests
* 🎨  server spec test adaption
- we now throw an error when database is not populated, instead of populating the database
* 🎨   migration spec adaption
- reset database now deletes migration table
- we will move the reset script into sephiroth and then we make it pretty
* 🎨  error creation adaption in bootUp
* 🎨  fixes
- sephiroth error handling
- fix tests

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);
+                });
         }
     });
 };

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;
     });
 };
 

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
+            }));
         });
 };

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) {

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();

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();
 

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 () {

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 () {

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');

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 () {

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 () {

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 () {

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 () {

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 () {

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 () {

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;
 

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;
 

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 () {

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) {

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) {

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);

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();
                 });
         });
 

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();

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,