0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00

Extended the API object validation

refs #9548

- do not forward `tag.parent` to the model layer
  - the model layer should only know `tag.parent_id`
  - and the API should only expose `tag.parent` (this is an API feature)
  - currently Ghost has a mixture of using `toJSON` and the API validation layer for this
  - we just continue with this for now (no time to fix this)
- disallow sending nested-nested relations
  - unsupported
  - see comment for more information
  - this can cause problems with calling `hasChanged` on relations
- add unit tests
This commit is contained in:
kirrg001 2018-04-05 15:59:52 +02:00 committed by Katharina Irrgang
parent 2f8dc97286
commit bda76acba6
4 changed files with 187 additions and 1 deletions

View file

@ -305,6 +305,16 @@ utils = {
* ### Check Object
* Check an object passed to the API is in the correct format
*
* @TODO:
* The weird thing about this function is..
* - that the API converts properties back to model notation
* - post.author -> post.author_id
* - and the model layer implementation of `toJSON` knows about these transformations as well
* - post.author_id -> post.author
* - this must live in one place
* - API IN <-> API OUT
* - this should be unrelated to the model layer
*
* @param {Object} object
* @param {String} docName
* @returns {Promise(Object)} resolves to the original object if it checks out
@ -352,6 +362,51 @@ utils = {
message: common.i18n.t('errors.api.utils.invalidStructure', {key: 'posts[*].authors'})
}));
}
/**
* CASE: we don't support updating nested-nested relations e.g. `post.authors[*].roles` yet.
*
* Bookshelf-relations supports this feature, BUT bookshelf's `hasChanged` fn will currently
* clash with this, because `hasChanged` won't be able to tell if relations have changed or not.
* It would always return `changed.roles = [....]`. It would always throw a model event that relations
* were updated, which is not true.
*
* Bookshelf-relations can tell us if a relation has changed, it knows that.
* But the connection between our model layer, Bookshelf's `hasChanged` fn and Bookshelf-relations
* is not present. As long as we don't support this case, we have to ignore this.
*/
if (object.posts[0].authors && object.posts[0].authors.length) {
_.each(object.posts[0].authors, (author, index) => {
if (author.hasOwnProperty('roles')) {
delete object.posts[0].authors[index].roles;
}
if (author.hasOwnProperty('permissions')) {
delete object.posts[0].authors[index].permissions;
}
});
}
}
/**
* Model notation is: `tag.parent_id`.
* The API notation is `tag.parent`.
*
* See @TODO on the fn description. This information lives in two places. Not nice.
*/
if (object.posts[0].hasOwnProperty('tags')) {
if (_.isArray(object.posts[0].tags) && object.posts[0].tags.length) {
_.each(object.posts[0].tags, (tag, index) => {
if (tag.hasOwnProperty('parent')) {
object.posts[0].tags[index].parent_id = tag.parent;
delete object.posts[0].tags[index].parent;
}
if (tag.hasOwnProperty('posts')) {
delete object.posts[0].tags[index].posts;
}
});
}
}
}

View file

@ -1835,6 +1835,10 @@ describe('Post Model', function () {
newJSON.tags[0].created_at = moment().add(2, 'days').format('YYYY-MM-DD HH:mm:ss');
newJSON.tags[0].updated_at = moment().add(2, 'days').format('YYYY-MM-DD HH:mm:ss');
// NOTE: this is currently only removed in the API layer
newJSON.tags[0].parent_id = newJSON.tags[0].parent;
delete newJSON.tags[0].parent;
// Edit the post
return Promise.delay(1000)
.then(function () {

View file

@ -351,6 +351,16 @@ describe('API Utils', function () {
}).catch(done);
});
it('passed through a simple, correct object', function (done) {
var object = {test: [{id: 1}]};
apiUtils.checkObject(_.cloneDeep(object), 'test').then(function (data) {
should.exist(data);
data.should.have.ownProperty('test');
object.should.eql(data);
done();
}).catch(done);
});
it('[DEPRECATED] should do author_id to author conversion for posts', function (done) {
var object = {posts: [{id: 1, author: 4}]};
apiUtils.checkObject(_.cloneDeep(object), 'posts').then(function (data) {
@ -425,7 +435,76 @@ describe('API Utils', function () {
}).catch(done);
});
describe('invalid post.authors structure', function () {
describe('post.tags structure', function () {
it('post.tags is empty', function (done) {
var object = {
posts: [{
id: 1,
tags: []
}]
};
apiUtils.checkObject(_.cloneDeep(object), 'posts')
.then(function (object) {
object.posts[0].tags.length.should.eql(0);
done();
})
.catch(done);
});
it('post.tags contains `parent`', function (done) {
var object = {
posts: [{
id: 1,
tags: [{id: 'objectid', parent: null}]
}]
};
apiUtils.checkObject(_.cloneDeep(object), 'posts')
.then(function (object) {
object.posts[0].tags[0].hasOwnProperty('parent').should.be.false();
object.posts[0].tags[0].hasOwnProperty('parent_id').should.be.true();
done();
})
.catch(done);
});
it('post.tags contains `parent_id`', function (done) {
var object = {
posts: [{
id: 1,
tags: [{id: 'objectid', parent_id: null}]
}]
};
apiUtils.checkObject(_.cloneDeep(object), 'posts')
.then(function (object) {
object.posts[0].tags[0].hasOwnProperty('parent').should.be.false();
object.posts[0].tags[0].hasOwnProperty('parent_id').should.be.true();
done();
})
.catch(done);
});
it('post.tags contains no parent', function (done) {
var object = {
posts: [{
id: 1,
tags: [{id: 'objectid'}]
}]
};
apiUtils.checkObject(_.cloneDeep(object), 'posts')
.then(function (object) {
object.posts[0].tags[0].hasOwnProperty('parent').should.be.false();
object.posts[0].tags[0].hasOwnProperty('parent_id').should.be.false();
done();
})
.catch(done);
});
});
describe('post.authors structure', function () {
it('post.authors is not present', function (done) {
var object = {posts: [{id: 1}]};
@ -488,6 +567,23 @@ describe('API Utils', function () {
done();
});
});
it('post.authors contains nested relations', function (done) {
var object = {
posts: [{
id: 1,
authors: [{id: 'objectid', name: 'Kate', roles: [{id: 'something'}], permissions: []}]
}]
};
apiUtils.checkObject(_.cloneDeep(object), 'posts')
.then(function (object) {
should.not.exist(object.posts[0].authors[0].roles);
should.not.exist(object.posts[0].authors[0].permissions);
done();
})
.catch(done);
});
});
});

View file

@ -309,6 +309,37 @@ describe('Unit: models/user', function () {
});
});
describe('Fetch', function () {
let knexMock;
before(function () {
models.init();
});
after(function () {
sandbox.restore();
});
before(function () {
knexMock = new testUtils.mocks.knex();
knexMock.mock();
});
after(function () {
knexMock.unmock();
});
it('ensure data type', function () {
return models.User.findOne({slug: 'joe-bloggs'}, testUtils.context.internal)
.then((user) => {
user.get('updated_by').should.be.a.String();
user.get('created_by').should.be.a.String();
user.get('created_at').should.be.a.Date();
user.get('updated_at').should.be.a.Date();
});
});
});
describe('Edit', function () {
let knexMock;