Skip to content

Commit

Permalink
✨ use migration runner for init db (#7502)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kirrg001 authored and ErisDS committed Oct 11, 2016
1 parent 49191c9 commit 9fad7f1
Show file tree
Hide file tree
Showing 24 changed files with 144 additions and 97 deletions.
13 changes: 11 additions & 2 deletions core/server/data/migration/fixtures/populate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
});
};
Expand Down
19 changes: 15 additions & 4 deletions core/server/data/migration/reset.js
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;
});
};

Expand Down
68 changes: 37 additions & 31 deletions core/server/data/schema/bootup.js
Original file line number Diff line number Diff line change
@@ -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
}));
});
};
29 changes: 20 additions & 9 deletions core/server/data/schema/commands.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/module/module_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions core/test/functional/routes/admin_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/authentication_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/db_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
3 changes: 1 addition & 2 deletions core/test/functional/routes/api/error_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
var supertest = require('supertest'),
should = require('should'),
testUtils = require('../../../utils'),

ghost = require('../../../../../core'),
ghost = testUtils.startGhost,
request;

require('should-http');
Expand Down
3 changes: 1 addition & 2 deletions core/test/functional/routes/api/notifications_spec.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down
4 changes: 1 addition & 3 deletions core/test/functional/routes/api/posts_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
4 changes: 1 addition & 3 deletions core/test/functional/routes/api/public_api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
4 changes: 1 addition & 3 deletions core/test/functional/routes/api/settings_spec.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down
4 changes: 1 addition & 3 deletions core/test/functional/routes/api/slugs_spec.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down
4 changes: 1 addition & 3 deletions core/test/functional/routes/api/tags_spec.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/themes_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/upload_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 1 addition & 3 deletions core/test/functional/routes/api/users_spec.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down
3 changes: 1 addition & 2 deletions core/test/functional/routes/channel_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/frontend_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions core/test/unit/migration_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -137,23 +137,23 @@ 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) {
should.exist(result);
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);
Expand Down
Loading

0 comments on commit 9fad7f1

Please sign in to comment.