Improve importer error messaging

closes #3274

- Ensure that validation errors are always handled by moving them into the
  importer
- Ensure that db errors are handled consistently across sqlite and mysql
- Change the errors to be output in a table, with a short failure notification
- Add tests for 003 importing bad files
This commit is contained in:
Hannah Wolfe 2014-07-28 22:41:45 +01:00
parent c094facb28
commit 0ffc5e6d47
14 changed files with 524 additions and 67 deletions

View File

@ -1,5 +1,6 @@
var DebugController = Ember.Controller.extend(Ember.Evented, {
uploadButtonText: 'Import',
importErrors: '',
actions: {
onUpload: function (file) {
@ -7,6 +8,7 @@ var DebugController = Ember.Controller.extend(Ember.Evented, {
formData = new FormData();
this.set('uploadButtonText', 'Importing');
this.notifications.closePassive();
formData.append('importfile', file);
@ -20,7 +22,10 @@ var DebugController = Ember.Controller.extend(Ember.Evented, {
}).then(function () {
self.notifications.showSuccess('Import successful.');
}).catch(function (response) {
self.notifications.showAPIError(response);
if (response && response.jqXHR && response.jqXHR.responseJSON && response.jqXHR.responseJSON.errors) {
self.set('importErrors', response.jqXHR.responseJSON.errors);
}
self.notifications.showError('Import Failed');
}).finally(function () {
self.set('uploadButtonText', 'Import');
self.trigger('reset');

View File

@ -0,0 +1,7 @@
{{#if importErrors}}
<table class="table">
{{#each importErrors}}
<tr><td>{{message}}</td></tr>
{{/each}}
</table>
{{/if}}

View File

@ -28,6 +28,7 @@
<fieldset>
<div class="form-group">
<label>Import</label>
{{partial "import-errors"}}
{{gh-file-upload id="importfile" uploadButtonText=uploadButtonText}}
<p>Import from another Ghost installation. If you import a user, this will replace the current user & log you out.</p>
</div>

View File

@ -52,6 +52,8 @@ var Notifications = Ember.ArrayProxy.extend({
if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.error) {
this.showError(resp.jqXHR.responseJSON.error, delayed);
} else if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.errors) {
this.showErrors(resp.jqXHR.responseJSON.errors, delayed);
} else {
this.showError(defaultErrorText, delayed);
}

View File

@ -8,7 +8,6 @@ var dataExport = require('../data/export'),
nodefn = require('when/node'),
_ = require('lodash'),
path = require('path'),
validation = require('../data/validation'),
errors = require('../../server/errors'),
canThis = require('../permissions').canThis,
api = {},
@ -80,20 +79,19 @@ db = {
return when.reject(new errors.UnsupportedMediaTypeError('Please select a .json file to import.'));
}
}).then(function () {
return api.settings.read({key: 'databaseVersion', context: { internal: true }}).then(function (response) {
return api.settings.read(
{key: 'databaseVersion', context: { internal: true }}
).then(function (response) {
var setting = response.settings[0];
return when(setting.value);
}, function () {
return when('002');
});
}).then(function (version) {
databaseVersion = version;
// Read the file contents
return nodefn.call(fs.readFile, filepath);
}).then(function (fileContents) {
var importData,
error = '';
var importData;
// Parse the json data
try {
@ -104,26 +102,16 @@ db = {
importData = importData.db[0];
}
} catch (e) {
errors.logError(e, "API DB import content", "check that the import file is valid JSON.");
return when.reject(new errors.BadRequest("Failed to parse the import JSON file"));
errors.logError(e, 'API DB import content', 'check that the import file is valid JSON.');
return when.reject(new errors.BadRequest('Failed to parse the import JSON file'));
}
if (!importData.meta || !importData.meta.version) {
return when.reject(new errors.ValidationError("Import data does not specify version", 'meta.version'));
return when.reject(
new errors.ValidationError('Import data does not specify version', 'meta.version')
);
}
_.each(_.keys(importData.data), function (tableName) {
_.each(importData.data[tableName], function (importValues) {
validation.validateSchema(tableName, importValues).catch(function (err) {
error += error !== '' ? '<br>' : '';
error += err.message;
});
});
});
if (error !== '') {
return when.reject(new Error(error));
}
// Import for the current version
return dataImport(databaseVersion, importData);

View File

@ -37,7 +37,7 @@ Importer000.prototype.canImport = function (data) {
return when.resolve(this.importFrom[data.meta.version]);
}
return when.reject("Unsupported version of data: " + data.meta.version);
return when.reject('Unsupported version of data: ' + data.meta.version);
};
@ -92,7 +92,9 @@ function importTags(ops, tableData, transaction) {
if (!_tag) {
return models.Tag.add(tag, _.extend(internal, {transacting: transaction}))
// add pass-through error handling so that bluebird doesn't think we've dropped it
.otherwise(function (error) { return when.reject(error); });
.catch(function (error) {
return when.reject({raw: error, model: 'tag', data: tag});
});
}
return when.resolve(_tag);
}));
@ -104,7 +106,9 @@ function importPosts(ops, tableData, transaction) {
_.each(tableData, function (post) {
ops.push(models.Post.add(post, _.extend(internal, {transacting: transaction, importing: true}))
// add pass-through error handling so that bluebird doesn't think we've dropped it
.otherwise(function (error) { return when.reject(error); }));
.catch(function (error) {
return when.reject({raw: error, model: 'post', data: post});
}));
});
}
@ -114,7 +118,9 @@ function importUsers(ops, tableData, transaction) {
tableData[0].id = 1;
ops.push(models.User.edit(tableData[0], _.extend(internal, {id: 1, transacting: transaction}))
// add pass-through error handling so that bluebird doesn't think we've dropped it
.otherwise(function (error) { return when.reject(error); }));
.catch(function (error) {
return when.reject({raw: error, model: 'user', data: tableData[0]});
}));
}
function importSettings(ops, tableData, transaction) {
@ -135,7 +141,9 @@ function importSettings(ops, tableData, transaction) {
ops.push(models.Settings.edit(tableData, _.extend(internal, {transacting: transaction}))
// add pass-through error handling so that bluebird doesn't think we've dropped it
.otherwise(function (error) { return when.reject(error); }));
.catch(function (error) {
return when.reject({raw: error, model: 'setting', data: tableData});
}));
}
function importApps(ops, tableData, transaction) {
@ -146,7 +154,9 @@ function importApps(ops, tableData, transaction) {
if (!_app) {
return models.App.add(app, _.extend(internal, {transacting: transaction}))
// add pass-through error handling so that bluebird doesn't think we've dropped it
.otherwise(function (error) { return when.reject(error); });
.catch(function (error) {
return when.reject({raw: error, model: 'app', data: app});
});
}
return when.resolve(_app);
}));
@ -171,7 +181,7 @@ function importApps(ops, tableData, transaction) {
// appSetting.app_id = _app.id;
// return models.AppSetting.add(appSetting, {transacting: transaction})
// // add pass-through error handling so that bluebird doesn't think we've dropped it
// .otherwise(function (error) { return when.reject(error); });
// .catch(function (error) { return when.reject(error); });
// }
// // Gracefully ignore missing apps
// return when.resolve(_app);
@ -249,8 +259,8 @@ Importer000.prototype.basicImport = function (data) {
}).then(function () {
//TODO: could return statistics of imported items
return when.resolve();
}, function (error) {
return when.reject(error);
}, function (errors) {
return when.reject(errors);
});
};

View File

@ -1,17 +1,103 @@
var when = require('when');
var when = require('when'),
_ = require('lodash'),
validation = require('../validation'),
errors = require('../../errors'),
validate,
handleErrors,
cleanError;
cleanError = function cleanError(error) {
var temp,
message,
offendingProperty,
value;
if (error.raw.message.toLowerCase().indexOf('unique') !== -1) {
// This is a unique constraint failure
if (error.raw.message.indexOf('ER_DUP_ENTRY') !== -1) {
temp = error.raw.message.split('\'');
if (temp.length === 5) {
value = temp[1];
temp = temp[3].split('_');
offendingProperty = temp.length === 3 ? temp[0] + '.' + temp[1] : error.model;
}
} else if (error.raw.message.indexOf('SQLITE_CONSTRAINT') !== -1) {
temp = error.raw.message.split('failed: ');
offendingProperty = temp.length === 2 ? temp[1] : error.model;
temp = offendingProperty.split('.');
value = temp.length === 2 ? error.data[temp[1]] : 'unknown';
}
message = 'Duplicate entry found. Multiple values of "' + value + '" found for ' + offendingProperty + '.';
}
offendingProperty = offendingProperty || error.model;
value = value || 'unknown';
message = message || error.raw.message;
return new errors.DataImportError(message, offendingProperty, value);
};
handleErrors = function handleErrors(errorList) {
var processedErrors = [];
_.each(errorList, function (error) {
if (!error.raw) {
// These are validation errors
processedErrors.push(error);
} else if (_.isArray(error.raw)) {
processedErrors = processedErrors.concat(error.raw);
} else {
processedErrors.push(cleanError(error));
}
});
return when.reject(processedErrors);
};
validate = function validate(data) {
var validateOps = [];
_.each(_.keys(data.data), function (tableName) {
_.each(data.data[tableName], function (importValues) {
validateOps.push(validation.validateSchema(tableName, importValues));
});
});
return when.settle(validateOps).then(function (descriptors) {
var errorList = [];
_.each(descriptors, function (d) {
if (d.state === 'rejected') {
errorList = errorList.concat(d.reason);
}
});
if (!_.isEmpty(errorList)) {
return when.reject(errorList);
}
return when.resolve();
});
};
module.exports = function (version, data) {
var importer;
try {
importer = require('./' + version);
} catch (ignore) {
// Zero effs given
}
return validate(data).then(function () {
try {
importer = require('./' + version);
} catch (ignore) {
// Zero effs given
}
if (!importer) {
return when.reject("No importer found");
}
if (!importer) {
return when.reject('No importer found');
}
return importer.importData(data);
return importer.importData(data);
}).catch(function (result) {
return handleErrors(result);
});
};

View File

View File

@ -0,0 +1,17 @@
// # Data import error
// Custom error class with status code and type prefilled.
function DataImportError(message, offendingProperty, value) {
this.message = message;
this.stack = new Error().stack;
this.code = 500;
this.type = this.name;
this.property = offendingProperty || undefined;
this.value = value || undefined;
}
DataImportError.prototype = Object.create(Error.prototype);
DataImportError.prototype.name = 'DataImportError';
module.exports = DataImportError;

View File

@ -14,6 +14,7 @@ var _ = require('lodash'),
ValidationError = require('./validationerror'),
UnsupportedMediaTypeError = require('./unsupportedmediaerror'),
EmailError = require('./emailerror'),
DataImportError = require('./dataimporterror'),
errors,
// Paths for views
@ -315,3 +316,4 @@ module.exports.ValidationError = ValidationError;
module.exports.RequestEntityTooLargeError = RequestEntityTooLargeError;
module.exports.UnsupportedMediaTypeError = UnsupportedMediaTypeError;
module.exports.EmailError = EmailError;
module.exports.DataImportError = DataImportError;

View File

@ -96,7 +96,7 @@ Settings = ghostBookshelf.Model.extend({
// Accept an array of models as input
if (item.toJSON) { item = item.toJSON(); }
if (!(_.isString(item.key) && item.key.length > 0)) {
return when.reject(new errors.ValidationError('Setting key cannot be empty.'));
return when.reject(new errors.ValidationError('Value in [settings.key] cannot be blank.'));
}
item = self.filterData(item);

View File

@ -300,7 +300,7 @@ describe('Import', function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}, function (error) {
error[0].message.should.eql('Setting key cannot be empty.');
error[0].message.should.eql('Value in [settings.key] cannot be blank.');
error[0].type.should.eql('ValidationError');
when.all([
@ -478,7 +478,7 @@ describe('Import', function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}, function (error) {
error[0].message.should.eql('Setting key cannot be empty.');
error[0].message.should.eql('Value in [settings.key] cannot be blank.');
error[0].type.should.eql('ValidationError');
when.all([
@ -519,34 +519,49 @@ describe('Import', function () {
should.exist(Importer003);
it('safely imports data from 003', function (done) {
it('handles validation errors nicely', function (done) {
var exportData;
testUtils.fixtures.loadExportFixture('export-003').then(function (exported) {
testUtils.fixtures.loadExportFixture('broken-validation').then(function (exported) {
exportData = exported;
return importer('003', exportData);
}).then(function () {
// Grab the data from tables
return when.all([
knex('apps').select(),
knex('app_settings').select()
]);
}).then(function (importedData) {
should.exist(importedData);
importedData.length.should.equal(2, 'Did not get data successfully');
var apps = importedData[0];
// app_settings = importedData[1];
// test apps
apps.length.should.equal(exportData.data.apps.length, 'imported apps');
// test app settings
// app_settings.length.should.equal(exportData.data.app_settings.length, 'imported app settings');
}).catch(function (response) {
response.length.should.equal(5);
response[0].type.should.equal('ValidationError');
response[0].message.should.eql('Value in [posts.title] cannot be blank.');
response[1].type.should.equal('ValidationError');
response[1].message.should.eql('Value in [posts.slug] cannot be blank.');
response[2].type.should.equal('ValidationError');
response[2].message.should.eql('Value in [settings.key] cannot be blank.');
response[3].type.should.equal('ValidationError');
response[3].message.should.eql('Value in [tags.slug] cannot be blank.');
response[4].type.should.equal('ValidationError');
response[4].message.should.eql('Value in [tags.name] cannot be blank.');
done();
}).catch(done);
});
});
it('handles database errors nicely', function (done) {
var exportData;
testUtils.fixtures.loadExportFixture('broken-db-errors').then(function (exported) {
exportData = exported;
return importer('003', exportData);
}).catch(function (response) {
response.length.should.equal(3);
response[0].type.should.equal('DataImportError');
response[0].message.should.eql(
'Duplicate entry found. Multiple values of "tagging-things" found for tags.slug.'
);
response[1].type.should.equal('DataImportError');
response[1].message.should.eql(
'Duplicate entry found. Multiple values of "tagging-things" found for tags.slug.'
);
response[2].type.should.equal('DataImportError');
response[2].message.should.eql(
'Duplicate entry found. Multiple values of "test-ghost-post" found for posts.slug.'
);
done();
});
});
});
});

View File

@ -0,0 +1,156 @@
{
"meta": {
"exported_on": 1388318311015,
"version": "003"
},
"data": {
"posts": [
{
"id": 1,
"title": "Test Ghost Post",
"slug": "test-ghost-post",
"markdown": "You're live! Nice.",
"html": "<p>You're live! Nice.</p>",
"image": null,
"featured": 0,
"page": 0,
"status": "published",
"language": "en_US",
"meta_title": null,
"meta_description": null,
"author_id": 1,
"created_at": 1388318310782,
"created_by": 1,
"updated_at": 1388318310782,
"updated_by": 1,
"published_at": 1388318310783,
"published_by": 1
},
{
"id": 2,
"title": "Test Ghost Post 2",
"slug": "test-ghost-post",
"markdown": "You're live! Nice.",
"html": "<p>You're live! Nice.</p>",
"image": null,
"featured": 0,
"page": 0,
"status": "published",
"language": "en_US",
"meta_title": null,
"meta_description": null,
"author_id": 1,
"created_at": 1388318310782,
"created_by": 1,
"updated_at": 1388318310782,
"updated_by": 1,
"published_at": 1388318310783,
"published_by": 1
}
],
"users": [
{
"id": 1,
"uuid": "e5188224-4742-4c32-a2d6-e9c5c5d4c123",
"name": "Josephine Bloggs",
"slug": "josephine-blogs",
"password": "$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKABC",
"email": "josephinebloggs@example.com",
"image": null,
"cover": null,
"bio": "A blogger",
"website": null,
"location": null,
"accessibility": null,
"status": "active",
"language": "en_US",
"meta_title": null,
"meta_description": null,
"last_login": null,
"created_at": 1388319501897,
"created_by": 1,
"updated_at": null,
"updated_by": null
}
],
"settings": [
{
"id": 1,
"key": "title",
"value": "",
"type": "blog",
"created_at": 1388318310830,
"created_by": 1,
"updated_at": 1388318310830,
"updated_by": 1
},
{
"id": 2,
"key": "title",
"value": "Title",
"type": "blog",
"created_at": 1388318310830,
"created_by": 1,
"updated_at": 1388318310830,
"updated_by": 1
}
],
"tags": [
{
"id": 1,
"name": "Tagging Things",
"slug": "tagging-things",
"description": null,
"parent_id": null,
"meta_title": null,
"meta_description": null,
"created_at": 1388318310790,
"created_by": 1,
"updated_at": 1388318310790,
"updated_by": 1
},
{
"id": 2,
"name": "Tagging Things",
"slug": "tagging-things",
"description": null,
"parent_id": null,
"meta_title": null,
"meta_description": null,
"created_at": 1388318310790,
"created_by": 1,
"updated_at": 1388318310790,
"updated_by": 1
},
{
"id": 3,
"name": "Tag Things",
"slug": "tagging-things",
"description": null,
"parent_id": null,
"meta_title": null,
"meta_description": null,
"created_at": 1388318310790,
"created_by": 1,
"updated_at": 1388318310790,
"updated_by": 1
}
],
"posts_tags": [
{
"id": 1,
"post_id": 1,
"tag_id": 1
},
{
"id": 2,
"post_id": 2,
"tag_id": 2
},{
"id": 3,
"post_id": 3,
"tag_id": 3
}
]
}
}

View File

@ -0,0 +1,168 @@
{
"meta": {
"exported_on": 1388318311015,
"version": "003"
},
"data": {
"posts": [
{
"id": 1,
"title": "Welcome to Ghost",
"slug": "welcome-to-ghost",
"markdown": "You're live! Nice.",
"html": "<p>You're live! Nice.</p>"
},
{
"id": 2,
"title": "",
"slug": "no-title-post",
"markdown": "I am a post.",
"html": "<p>I am a post.</p>",
"image": null,
"featured": 0,
"page": 0,
"status": "published",
"language": "en_US",
"meta_title": null,
"meta_description": null,
"author_id": 1,
"created_at": 1388318310782,
"created_by": 1,
"updated_at": 1388318310782,
"updated_by": 1,
"published_at": 1388318310783,
"published_by": 1
},
{
"id": 3,
"title": "I don't have a slug",
"slug": "",
"markdown": "I don't have a slug.",
"html": "<p>I don't have a slug.</p>",
"image": null,
"featured": 0,
"page": 0,
"status": "published",
"language": "en_US",
"meta_title": null,
"meta_description": null,
"author_id": 1,
"created_at": 1388318310782,
"created_by": 1,
"updated_at": 1388318310782,
"updated_by": 1,
"published_at": 1388318310783,
"published_by": 1
}
],
"users": [
{
"id": 1,
"uuid": "e5188224-4742-4c32-a2d6-e9c5c5d4c123",
"name": "Josephine Bloggs",
"slug": "josephine-blogs",
"password": "$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKABC",
"email": "josephinebloggs@example.com",
"image": null,
"cover": null,
"bio": "A blogger",
"website": null,
"location": null,
"accessibility": null,
"status": "active",
"language": "en_US",
"meta_title": null,
"meta_description": null,
"last_login": null,
"created_at": 1388319501897,
"created_by": 1,
"updated_at": null,
"updated_by": null
}
],
"settings": [
{
"id": 3,
"uuid": "c356fbde-0bc5-4fe1-9309-2510291aa34d",
"key": "title",
"value": "",
"type": "blog",
"created_at": 1388318310830,
"created_by": 1,
"updated_at": 1388318310830,
"updated_by": 1
},
{
"id": 4,
"uuid": "858dc11f-8f9e-4011-99ee-d94c48d5a2ce",
"key": "",
"value": "Just a blogging platform.",
"type": "blog",
"created_at": 1388318310830,
"created_by": 1,
"updated_at": 1388318310830,
"updated_by": 1
},
{
"id": 5,
"uuid": "37ca5ae7-bca6-4dd5-8021-4ef6c6dcb097",
"key": "email",
"value": "blah",
"type": "blog",
"created_at": 1388318310830,
"created_by": 1,
"updated_at": 1388318310830,
"updated_by": 1
}
],
"tags": [
{
"id": 1,
"name": "Getting Started",
"slug": "getting-started"
},
{
"id": 2,
"name": "Test Empty Slug",
"slug": "",
"description": null,
"parent_id": null,
"meta_title": null,
"meta_description": null,
"created_at": 1388318310790,
"created_by": 1,
"updated_at": 1388318310790,
"updated_by": 1
},
{
"id": 3,
"name": "",
"slug": "no-title-tag",
"description": null,
"parent_id": null,
"meta_title": null,
"meta_description": null,
"created_at": 1388318310790,
"created_by": 1,
"updated_at": 1388318310790,
"updated_by": 1
}
],
"posts_tags": [
{
"id": 1,
"post_id": 1,
"tag_id": 1
},
{
"id": 2,
"post_id": 2,
"tag_id": 2
},{
"id": 3,
"post_id": 3,
"tag_id": 3
}
]
}
}