Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-06 22:40:14 -05:00
Code Issues Activity

Moved schema validator into the schema module

Browse source 
- This is a really specific piece of code related to validating models against our internal schema.js format
- This doesn't make sense without a schema.js file
- It does depend on the internal validator and validate tools - but those are used elsewhere too, and can reasonably be moved out of the codebase
- I don't see schema.js moving out of the codebase any time soon. We can move the validator but it would be a class that requires schema via DI
- For now my focus is on getting the data/validation tooling separated and making clear sense
- Improving data/schema can come later :)
This commit is contained in:
Hannah Wolfe 2021-06-15 11:45:37 +01:00
parent 68d60a1834
commit d3cc85c920
No known key found for this signature in database
GPG key ID: 9F8C7532D0A6BA55
7 changed files with 18 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
core/server/data/schema/index.js
View file

@ -2,3 +2,4 @@ module.exports.tables = require('./schema');
module.exports.checks = require('./checks');
module.exports.commands = require('./commands');
module.exports.defaultSettings = require('./default-settings');
module.exports.validate = require('./validator');

5
core/server/data/validation/schema.js → core/server/data/schema/validator.js
View file

@ -3,10 +3,9 @@ const Promise = require('bluebird');
const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const {validator, validate} = require('../validation');
const schema = require('../schema').tables;
const validator = require('./validator');
const validate = require('./validate');
const schema = require('./schema');
const messages = {
valueCannotBeBlank: 'Value in [{tableName}.{columnKey}] cannot be blank.',

3
core/server/data/validation/index.js
View file

@ -3,6 +3,5 @@ module.exports = {
validator: require('./validator'),
// These two things are dependent on validator, not related
validatePassword: require('./password'),
validateSchema: require('./schema')
validatePassword: require('./password')
};

3
core/server/models/base/index.js
View file

@ -21,7 +21,6 @@ const errors = require('@tryghost/errors');
const security = require('@tryghost/security');
const schema = require('../../data/schema');
const urlUtils = require('../../../shared/url-utils');
const validation = require('../../data/validation');
const bulkOperations = require('./bulk-operations');
const plugins = require('../plugins');
const tpl = require('@tryghost/tpl');
@ -312,7 +311,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
*/
onValidate: function onValidate(model, columns, options) {
this.setEmptyValuesToNull();
return validation.validateSchema(this.tableName, this, options);
return schema.validate(this.tableName, this, options);
},
onFetched() {},

19
test/unit/data/validation/schema_spec.js → test/unit/data/schema/validator_spec.js
View file

@ -3,7 +3,8 @@ const _ = require('lodash');
const ObjectId = require('bson-objectid');
const testUtils = require('../../../utils');
const models = require('../../../../core/server/models');
const validation = require('../../../../core/server/data/validation');
const validateSchema = require('../../../../core/server/data/schema/validator');
describe('Validate Schema', function () {
before(function () {
@ -13,7 +14,7 @@ describe('Validate Schema', function () {
describe('models.add', function () {
it('blank model', function () {
// NOTE: Fields with `defaultTo` are getting ignored. This is handled on the DB level.
return validation.validateSchema('posts', models.Post.forge(), {method: 'insert'})
return validateSchema('posts', models.Post.forge(), {method: 'insert'})
.then(function () {
throw new Error('Expected ValidationError.');
})
@ -41,7 +42,7 @@ describe('Validate Schema', function () {
slug: 'test'
}));
return validation.validateSchema('posts', postModel, {method: 'insert'})
return validateSchema('posts', postModel, {method: 'insert'})
.then(function () {
throw new Error('Expected ValidationError.');
})
@ -56,7 +57,7 @@ describe('Validate Schema', function () {
});
it('should pass', function () {
return validation.validateSchema(
return validateSchema(
'posts',
models.Post.forge(testUtils.DataGenerator.forKnex.createPost({slug: 'title'})),
{method: 'insert'}
@ -67,7 +68,7 @@ describe('Validate Schema', function () {
const post = models.Post.forge(testUtils.DataGenerator.forKnex.createPost({slug: 'test', featured: 0}));
post.get('featured').should.eql(0);
return validation.validateSchema('posts', post, {method: 'insert'})
return validateSchema('posts', post, {method: 'insert'})
.then(function () {
post.get('featured').should.eql(false);
});
@ -77,7 +78,7 @@ describe('Validate Schema', function () {
const post = models.Post.forge(testUtils.DataGenerator.forKnex.createPost({slug: 'test', featured: true}));
post.get('featured').should.eql(true);
return validation.validateSchema('posts', post, {method: 'insert'})
return validateSchema('posts', post, {method: 'insert'})
.then(function () {
post.get('featured').should.eql(true);
});
@ -92,7 +93,7 @@ describe('Validate Schema', function () {
}));
// NOTE: Fields with `defaultTo` are getting ignored. This is handled on the DB level.
return validation.validateSchema('webhooks', webhook, {method: 'insert'})
return validateSchema('webhooks', webhook, {method: 'insert'})
.then(function () {
throw new Error('Expected ValidationError.');
})
@ -114,7 +115,7 @@ describe('Validate Schema', function () {
postModel.changed = {uuid: postModel.get('uuid')};
return validation.validateSchema('posts', postModel)
return validateSchema('posts', postModel)
.then(function () {
throw new Error('Expected ValidationError.');
})
@ -133,7 +134,7 @@ describe('Validate Schema', function () {
postModel.changed = {created_at: postModel.get('updated_at')};
return validation.validateSchema('posts', postModel)
return validateSchema('posts', postModel)
.then(function () {
throw new Error('Expected ValidationError.');
})

3
test/unit/data/validation/index_spec.js
View file

@ -8,12 +8,11 @@ describe('Validation', function () {
should.exist(validation);
validation.should.have.properties(
['validate', 'validator', 'validateSchema', 'validatePassword']
['validate', 'validator', 'validatePassword']
);
validation.validate.should.be.a.Function();
validation.validatePassword.should.be.a.Function();
validation.validateSchema.should.be.a.Function();
validation.validator.should.have.properties(['empty', 'notContains', 'isTimezone', 'isEmptyOrURL', 'isSlug']);
});

4
test/unit/models/user_spec.js
View file

@ -4,7 +4,7 @@ const Promise = require('bluebird');
const errors = require('@tryghost/errors');
const models = require('../../../core/server/models');
const permissions = require('../../../core/server/services/permissions');
const validation = require('../../../core/server/data/validation');
const schema = require('../../../core/server/data/schema');
const security = require('@tryghost/security');
const testUtils = require('../../utils');
@ -86,7 +86,7 @@ describe('Unit: models/user', function () {
sinon.stub(security.password, 'compare').resolves(true);
// NOTE: Add a user with a broken field to ensure we only validate changed fields on login
sinon.stub(validation, 'validateSchema').resolves();
sinon.stub(schema, 'validate').resolves();
const user = models.User.forge(testUtils.DataGenerator.forKnex.createUser({
status: 'warn-1',