Skip to content

Commit

Permalink
Added more database soft limits (#9225)
Browse files Browse the repository at this point in the history 
refs #8143

Sets soft limits for certain db fields:

- `posts`:
	- `title`: 255 chars (current hard limit: 2,000 chars)
	- `meta_title`: 300 chars (current hard limit: 2,000 chars)
	- `meta_description`: 500 chars (current hard limit: 2,000 chars)
- `users`:
	- `bio`: 200 chars (current hard limit: 65,535 chars)
	- `location`: 150 chars (current hard limit: 65,535 chars)
	- `meta_description`: 500 chars (current hard limit: 2,000 chars)
	- `meta_title`: 300 chars (current hard limit: 2,000 chars)
- `tags`:
	- `description`: 500 chars (current hard limit: 65,535 chars)
	- `meta_title`: 300 chars (current hard limit: 2,000 chars)
	- `meta_description`: 500 chars (current hard limit: 2,000 chars)

- same error message for isLength validator as for hard limits (more improvements are comming with #6050)
- added more tests for importer
- added dynamic translation key handling for validators errors (isLength is only supported atm)
  • Loading branch information
@aileen @kirrg001
aileen authored and kirrg001 committed Nov 9, 2017
1 parent f22a278 commit a35c0c2
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 36 deletions.
20 changes: 10 additions & 10 deletions core/server/data/schema/schema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module.exports = {
posts: {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
uuid: {type: 'string', maxlength: 36, nullable: false, validations: {isUUID: true}},
title: {type: 'string', maxlength: 2000, nullable: false},
title: {type: 'string', maxlength: 2000, nullable: false, validations: {isLength: {max: 255}}},
slug: {type: 'string', maxlength: 191, nullable: false, unique: true},
mobiledoc: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},
html: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},
Expand All @@ -14,8 +14,8 @@ module.exports = {
status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'draft'},
locale: {type: 'string', maxlength: 6, nullable: true},
visibility: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}},
meta_title: {type: 'string', maxlength: 2000, nullable: true},
meta_description: {type: 'string', maxlength: 2000, nullable: true},
meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}},
meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}},
author_id: {type: 'string', maxlength: 24, nullable: false},
created_at: {type: 'dateTime', nullable: false},
created_by: {type: 'string', maxlength: 24, nullable: false},
Expand Down Expand Up @@ -44,17 +44,17 @@ module.exports = {
email: {type: 'string', maxlength: 191, nullable: false, unique: true, validations: {isEmail: true}},
profile_image: {type: 'string', maxlength: 2000, nullable: true},
cover_image: {type: 'string', maxlength: 2000, nullable: true},
bio: {type: 'text', maxlength: 65535, nullable: true},
bio: {type: 'text', maxlength: 65535, nullable: true, validations: {isLength: {max: 200}}},
website: {type: 'string', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}},
location: {type: 'text', maxlength: 65535, nullable: true},
location: {type: 'text', maxlength: 65535, nullable: true, validations: {isLength: {max: 150}}},
facebook: {type: 'string', maxlength: 2000, nullable: true},
twitter: {type: 'string', maxlength: 2000, nullable: true},
accessibility: {type: 'text', maxlength: 65535, nullable: true},
status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'active'},
locale: {type: 'string', maxlength: 6, nullable: true},
visibility: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}},
meta_title: {type: 'string', maxlength: 2000, nullable: true},
meta_description: {type: 'string', maxlength: 2000, nullable: true},
meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}},
meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}},
tour: {type: 'text', maxlength: 65535, nullable: true},
last_seen: {type: 'dateTime', nullable: true},
created_at: {type: 'dateTime', nullable: false},
Expand Down Expand Up @@ -116,12 +116,12 @@ module.exports = {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
name: {type: 'string', maxlength: 191, nullable: false, validations: {matches: /^([^,]|$)/}},
slug: {type: 'string', maxlength: 191, nullable: false, unique: true},
description: {type: 'text', maxlength: 65535, nullable: true},
description: {type: 'text', maxlength: 65535, nullable: true, validations: {isLength: {max: 500}}},
feature_image: {type: 'string', maxlength: 2000, nullable: true},
parent_id: {type: 'string', nullable: true},
visibility: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'public', validations: {isIn: [['public', 'internal']]}},
meta_title: {type: 'string', maxlength: 2000, nullable: true},
meta_description: {type: 'string', maxlength: 2000, nullable: true},
meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}},
meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}},
created_at: {type: 'dateTime', nullable: false},
created_by: {type: 'string', maxlength: 24, nullable: false},
updated_at: {type: 'dateTime', nullable: true},
Expand Down
59 changes: 37 additions & 22 deletions core/server/data/validation/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ validateSchema = function validateSchema(tableName, model) {

// check validations objects
if (schema[tableName][columnKey].hasOwnProperty('validations')) {
validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations));
validationErrors = validationErrors.concat(validate(strVal, columnKey, schema[tableName][columnKey].validations, tableName));
}

// check type
Expand Down Expand Up @@ -243,7 +243,7 @@ validateSettings = function validateSettings(defaultSettings, model) {
matchingDefault = defaultSettings[values.key];

if (matchingDefault && matchingDefault.validations) {
validationErrors = validationErrors.concat(validate(values.value, values.key, matchingDefault.validations));
validationErrors = validationErrors.concat(validate(values.value, values.key, matchingDefault.validations, 'settings'));
}

if (validationErrors.length !== 0) {
Expand All @@ -253,25 +253,29 @@ validateSettings = function validateSettings(defaultSettings, model) {
return Promise.resolve();
};

// Validate default settings using the validator module.
// Each validation's key is a method name and its value is an array of options
//
// eg:
// validations: { isURL: true, isLength: [20, 40] }
//
// will validate that a setting's length is a URL between 20 and 40 chars.
//
// If you pass a boolean as the value, it will specify the "good" result. By default
// the "good" result is assumed to be true.
//
// eg:
// validations: { isNull: false } // means the "good" result would
// // fail the `isNull` check, so
// // not null.
//
// available validators: https://github.com/chriso/validator.js#validators
validate = function validate(value, key, validations) {
var validationErrors = [];
/**
* Validate keys using the validator module.
* Each validation's key is a method name and its value is an array of options
* eg:
* validations: { isURL: true, isLength: [20, 40] }
* will validate that a values's length is a URL between 20 and 40 chars.
*
* If you pass a boolean as the value, it will specify the "good" result. By default
* the "good" result is assumed to be true.
* eg:
* validations: { isNull: false } // means the "good" result would
* // fail the `isNull` check, so
* // not null.
*
* available validators: https://github.com/chriso/validator.js#validators
* @param {String} value the value to validate.
* @param {String} key the db column key of the value to validate.
* @param {Object} validations the validations object as described above.
* @param {String} tableName (optional) the db table of the value to validate, used for error message.
* @return {Array} returns an Array including the found validation errors (empty if none found);
*/
validate = function validate(value, key, validations, tableName) {
var validationErrors = [], translation;
value = _.toString(value);

_.each(validations, function each(validationOptions, validationName) {
Expand All @@ -288,8 +292,19 @@ validate = function validate(value, key, validations) {

// equivalent of validator.isSomething(option1, option2)
if (validator[validationName].apply(validator, validationOptions) !== goodResult) {
// CASE: You can define specific translations for validators e.g. isLength
if (i18n.doesTranslationKeyExist('notices.data.validation.index.validationFailedTypes.' + validationName)) {
translation = i18n.t('notices.data.validation.index.validationFailedTypes.' + validationName, _.merge({
validationName: validationName,
key: key,
tableName: tableName
}, validationOptions[1]));
} else {
translation = i18n.t('notices.data.validation.index.validationFailed', {validationName: validationName, key: key});
}

validationErrors.push(new errors.ValidationError({
message: i18n.t('notices.data.validation.index.validationFailed', {validationName: validationName, key: key})
message: translation
}));
}

Expand Down
5 changes: 5 additions & 0 deletions core/server/i18n.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ I18n = {
return matchingString;
},

doesTranslationKeyExist: function doesTranslationKeyExist(msgPath) {
var translation = I18n.findString(msgPath);
return translation !== blos.errors.errors.anErrorOccurred;
},

/**
* Setup i18n support:
* - Load proper language file in to memory
Expand Down
5 changes: 4 additions & 1 deletion core/server/translations/en.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,10 @@
"valueExceedsMaxLength": "Value in [{tableName}.{columnKey}] exceeds maximum length of {maxlength} characters.",
"valueIsNotInteger": "Value in [{tableName}.{columnKey}] is not an integer.",
"themeCannotBeActivated": "{themeName} cannot be activated because it is not currently installed.",
"validationFailed": "Validation ({validationName}) failed for {key}"
"validationFailed": "Validation ({validationName}) failed for {key}",
"validationFailedTypes": {
"isLength": "Value in [{tableName}.{key}] exceeds maximum length of {max} characters."
}
}
}
}
Expand Down
86 changes: 83 additions & 3 deletions core/test/integration/data/importer/importers/data_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -572,14 +572,94 @@ describe('Import', function () {
testUtils.fixtures.loadExportFixture('export-001', {lts:true}).then(function (exported) {
exportData = exported;

// change title to 1001 characters
exportData.data.posts[0].title = new Array(2002).join('a');
// change title to 300 characters (soft limit is 255)
exportData.data.posts[0].title = new Array(300).join('a');
exportData.data.posts[0].tags = 'Tag';
return dataImporter.doImport(exportData);
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}).catch(function (error) {
error[0].message.should.eql('Value in [posts.title] exceeds maximum length of 2000 characters.');
error[0].message.should.eql('Value in [posts.title] exceeds maximum length of 255 characters.');
error[0].errorType.should.eql('ValidationError');

Promise.all([
knex('users').select(),
knex('posts').select(),
knex('tags').select()
]).then(function (importedData) {
should.exist(importedData);

importedData.length.should.equal(3, 'Did not get data successfully');

var users = importedData[0],
posts = importedData[1],
tags = importedData[2];

// we always have 1 user, the default user we added
users.length.should.equal(1, 'There should only be one user');

// Nothing should have been imported
posts.length.should.equal(0, 'Wrong number of posts');
tags.length.should.equal(0, 'no new tags');

done();
});
}).catch(done);
});

it('doesn\'t import a tag when meta title too long', function (done) {
var exportData;

testUtils.fixtures.loadExportFixture('export-001', {lts:true}).then(function (exported) {
exportData = exported;

// change meta_title to 305 characters (soft limit is 300)
exportData.data.tags[0].meta_title = new Array(305).join('a');
return dataImporter.doImport(exportData);
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}).catch(function (error) {
error[0].message.should.eql('Value in [tags.meta_title] exceeds maximum length of 300 characters.');
error[0].errorType.should.eql('ValidationError');

Promise.all([
knex('users').select(),
knex('posts').select(),
knex('tags').select()
]).then(function (importedData) {
should.exist(importedData);

importedData.length.should.equal(3, 'Did not get data successfully');

var users = importedData[0],
posts = importedData[1],
tags = importedData[2];

// we always have 1 user, the default user we added
users.length.should.equal(1, 'There should only be one user');

// Nothing should have been imported
posts.length.should.equal(0, 'Wrong number of posts');
tags.length.should.equal(0, 'no new tags');

done();
});
}).catch(done);
});

it('doesn\'t import a user user when bio too long', function (done) {
var exportData;

testUtils.fixtures.loadExportFixture('export-001', {lts:true}).then(function (exported) {
exportData = exported;

// change bio to 300 characters (soft limit is 200)
exportData.data.users[0].bio = new Array(300).join('a');
return dataImporter.doImport(exportData);
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}).catch(function (error) {
error[0].message.should.eql('Value in [users.bio] exceeds maximum length of 200 characters.');
error[0].errorType.should.eql('ValidationError');

Promise.all([
Expand Down

0 comments on commit a35c0c2

Please sign in to comment.