From c9fbf0152199a7b05f33e7bddd6bab03210458c2 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Wed, 25 May 2016 09:34:46 +0200 Subject: [PATCH] improvement: general fixes - add NODE_LEVEL to print errors while running tests - try/catch while parsing translations file - run setup/teardown as promise or callback - some general error improvements --- core/server/data/migration/index.js | 24 +++++++++--------- core/server/errors/index.js | 4 ++- core/server/i18n.js | 10 +++++++- core/server/permissions/effective.js | 7 +++++ core/test/utils/index.js | 38 +++++++++++++++++++++++----- 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/core/server/data/migration/index.js b/core/server/data/migration/index.js index 96b2e4359a..0bf822bf39 100644 --- a/core/server/data/migration/index.js +++ b/core/server/data/migration/index.js @@ -28,27 +28,27 @@ logger = { init = function init(tablesOnly) { tablesOnly = tablesOnly || false; - // There are 4 possibilities: - // 1. The database exists and is up-to-date - // 2. The database exists but is out of date - // 3. The database exists but the currentVersion setting does not or cannot be understood - // 4. The database has not yet been created + // There are 4 possible cases: + // CASE 1: The database exists and is up-to-date + // CASE 2: The database exists but is out of date + // CASE 3: The database exists but the currentVersion setting does not or cannot be understood + // CASE 4: The database has not yet been created return versioning.getDatabaseVersion().then(function (databaseVersion) { var defaultVersion = versioning.getDefaultDatabaseVersion(); // Update goes first, to allow for FORCE_MIGRATION - // 2. The database exists but is out of date + // CASE 2: The database exists but is out of date if (databaseVersion < defaultVersion || process.env.FORCE_MIGRATION) { // Migrate to latest version logger.info('Database upgrade required from version ' + databaseVersion + ' to ' + defaultVersion); return update(databaseVersion, defaultVersion, logger); - // 1. The database exists and is up-to-date + // CASE 1: The database exists and is up-to-date } else if (databaseVersion === defaultVersion) { logger.info('Up-to-date at version ' + databaseVersion); return; - // 3. The database exists but the currentVersion setting does not or cannot be understood + // CASE 3: The database exists but the currentVersion setting does not or cannot be understood } else { // In this case we don't understand the version because it is too high errors.logErrorAndExit( @@ -58,14 +58,14 @@ init = function init(tablesOnly) { } }, function (err) { if (err && err.message === 'Settings table does not exist') { - // 4. The database has not yet been created + // CASE 4: The database has not yet been created // Bring everything up from initial version. logger.info('Database initialisation required for version ' + versioning.getDefaultDatabaseVersion()); return populate(logger, tablesOnly); } - // 3. The database exists but the currentVersion setting does not or cannot be understood - // In this case the setting was missing or there was some other problem - errors.logErrorAndExit('There is a problem with the database', err.message); + // CASE 3: the database exists but the currentVersion setting does not or cannot be understood + // In this case the setting was missing or there was some other problem + errors.logErrorAndExit(err, 'Problem occurred during migration initialisation!'); }); }; diff --git a/core/server/errors/index.js b/core/server/errors/index.js index 5d6fddd35b..58c2bbb556 100644 --- a/core/server/errors/index.js +++ b/core/server/errors/index.js @@ -155,7 +155,9 @@ errors = { // TODO: Logging framework hookup // Eventually we'll have better logging which will know about envs - if ((process.env.NODE_ENV === 'development' || + // you can use DEBUG=true when running tests and need error stdout + if ((process.env.NODE_LEVEL === 'DEBUG' || + process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'staging' || process.env.NODE_ENV === 'production')) { msgs = [chalk.red(i18n.t('errors.errors.error'), err), '\n']; diff --git a/core/server/i18n.js b/core/server/i18n.js index 942bc6ef51..13268ce4e5 100644 --- a/core/server/i18n.js +++ b/core/server/i18n.js @@ -84,7 +84,15 @@ I18n = { init: function init() { // read file for current locale and keep its content in memory blos = fs.readFileSync(__dirname + '/translations/' + currentLocale + '.json'); - blos = JSON.parse(blos); + + // if translation file is not valid, you will see an error + try { + blos = JSON.parse(blos); + } catch (err) { + blos = undefined; + throw err; + } + if (global.Intl) { // Determine if the built-in `Intl` has the locale data we need. var hasBuiltInLocaleData, diff --git a/core/server/permissions/effective.js b/core/server/permissions/effective.js index c955dda627..06faab4a14 100644 --- a/core/server/permissions/effective.js +++ b/core/server/permissions/effective.js @@ -1,12 +1,19 @@ var _ = require('lodash'), + Promise = require('bluebird'), Models = require('../models'), errors = require('../errors'), + i18n = require('../i18n'), effective; effective = { user: function (id) { return Models.User.findOne({id: id, status: 'all'}, {include: ['permissions', 'roles', 'roles.permissions']}) .then(function (foundUser) { + // CASE: {context: {user: id}} where the id is not in our database + if (!foundUser) { + return Promise.reject(new errors.NotFoundError(i18n.t('errors.models.user.userNotFound'))); + } + var seenPerms = {}, rolePerms = _.map(foundUser.related('roles').models, function (role) { return role.related('permissions').models; diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 5f1b940f96..afa3d2c0a0 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -327,6 +327,11 @@ fixtures = { Owner: 4 }; + // CASE: if empty db will throw SQLITE_MISUSE, hard to debug + if (_.isEmpty(permsToInsert)) { + return Promise.reject(new Error('no permission found:' + obj)); + } + permsToInsert = _.map(permsToInsert, function (perms) { actions.push(perms.action_type); return DataGenerator.forKnex.createBasic(perms); @@ -347,12 +352,18 @@ fixtures = { }); return db.knex('permissions').insert(permsToInsert).then(function () { + if (_.isEmpty(permissionsRoles)) { + return Promise.resolve(); + } + return db.knex('permissions_roles').insert(permissionsRoles); }); }, + insertClients: function insertClients() { return db.knex('clients').insert(DataGenerator.forKnex.clients); }, + insertAccessToken: function insertAccessToken(override) { return db.knex('accesstokens').insert(DataGenerator.forKnex.createToken(override)); } @@ -438,10 +449,15 @@ getFixtureOps = function getFixtureOps(toDos) { // Go through our list of things to do, and add them to an array _.each(toDos, function (value, toDo) { var tmp; + if (toDo !== 'perms:init' && toDo.indexOf('perms:') !== -1) { tmp = toDo.split(':'); fixtureOps.push(toDoList[tmp[0]](tmp[1])); } else { + if (!toDoList[toDo]) { + throw new Error('setup todo does not exist - spell mistake?'); + } + fixtureOps.push(toDoList[toDo]); } }); @@ -470,12 +486,16 @@ setup = function setup() { var self = this, args = arguments; - return function (done) { + return function setup(done) { Models.init(); - return initFixtures.apply(self, args).then(function () { - done(); - }).catch(done); + if (done) { + return initFixtures.apply(self, args).then(function () { + done(); + }).catch(done); + } else { + return initFixtures.apply(self, args); + } }; }; @@ -556,9 +576,13 @@ togglePermalinks = function togglePermalinks(request, toggle) { }; teardown = function teardown(done) { - migration.reset().then(function () { - done(); - }).catch(done); + if (done) { + migration.reset().then(function () { + done(); + }).catch(done); + } else { + return migration.reset(); + } }; module.exports = {