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

🐛 fix delete by author as transaction (#7145)

Browse source 
closes #7137

Deleting the content from the database runs in a transaction. see
https://github.com/TryGhost/Ghost/blob/master/core/server/api/users.js#L390

`destroyByAuthor` is one of the operations we trigger to delete all the conent, see https://github.com/TryGhost/Ghost/blob/master/core/server/models/post.js#L647

The post model has a specific hook for deleting content to delete the relations as well, see https://github.com/TryGhost/Ghost/blob/master/core/server/models/post.js#L122

This hook is part of the transaction. But the `options` are ignored. `(model/*, attr, options*/)` 
We use the `options` to forward the transaction reference, which we need to pass into the bookshelf queries. So `return model.load('tags').call('related', 'tags').call('detach')` does not forward the transaction and that's why it stucks when deleting the content.
This commit is contained in:
Katharina Irrgang 2016-09-19 15:45:36 +02:00 committed by Hannah Wolfe
parent a98e134fe4
commit 9349e99e54
3 changed files with 115 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
core/server/models/base/token.js
View file

@ -36,7 +36,7 @@ Basetoken = ghostBookshelf.Model.extend({
.query('where', 'expires', '<', Date.now())
.fetch(options)
.then(function then(collection) {
collection.invokeThen('destroy', options);
return collection.invokeThen('destroy', options);
});
},
/**
@ -53,7 +53,7 @@ Basetoken = ghostBookshelf.Model.extend({
.query('where', 'user_id', '=', userId)
.fetch(options)
.then(function then(collection) {
collection.invokeThen('destroy', options);
return collection.invokeThen('destroy', options);
});
}

18
core/server/models/post.js
View file

@ -119,11 +119,22 @@ Post = ghostBookshelf.Model.extend({
}
});
this.on('destroying', function (model/*, attr, options*/) {
return model.load('tags').call('related', 'tags').call('detach').then(function then() {
this.on('destroying', function (model, options) {
return model.load('tags', options)
.then(function (response) {
if (!response.related || !response.related('tags') || !response.related('tags').length) {
return;
}
return Promise.mapSeries(response.related('tags').models, function (tag) {
return baseUtils.tagUpdate.detachTagFromPost(model, tag, options)();
});
})
.then(function () {
if (model.previous('status') === 'published') {
model.emitChange('unpublished');
}
model.emitChange('deleted');
});
});
@ -654,7 +665,8 @@ Post = ghostBookshelf.Model.extend({
throw new errors.NotFoundError(i18n.t('errors.models.post.noUserFound'));
}
return postCollection.query('where', 'author_id', '=', authorId)
return postCollection
.query('where', 'author_id', '=', authorId)
.fetch(options)
.call('invokeThen', 'destroy', options)
.catch(function (error) {

92
core/test/integration/api/api_users_spec.js
View file

@ -8,6 +8,7 @@ var testUtils = require('../../utils'),
models = require('../../../server/models'),
UserAPI = require('../../../server/api/users'),
mail = require('../../../server/api/mail'),
db = require('../../../server/data/db'),
context = testUtils.context,
userIdFor = testUtils.users.ids,
@ -21,6 +22,7 @@ describe('Users API', function () {
beforeEach(testUtils.setup(
'users:roles', 'users', 'user:token', 'perms:user', 'perms:role', 'perms:setting', 'perms:init', 'posts'
));
afterEach(testUtils.teardown);
function checkForErrorType(type, done) {
@ -692,6 +694,96 @@ describe('Users API', function () {
});
describe('Destroy', function () {
describe('General Tests', function () {
it('ensure posts get deleted', function (done) {
var postIdsToDelete = [], postIsToKeep = [], options = {};
Promise.mapSeries(testUtils.DataGenerator.forKnex.posts, function (post, i) {
post = _.cloneDeep(post);
if (i % 2) {
post.author_id = userIdFor.editor;
post.status = 'published';
post.tags = testUtils.DataGenerator.forKnex.tags.slice(0, 1);
return models.Post.add(post, _.merge({}, options, context.editor));
} else {
post.author_id = userIdFor.author;
post.status = 'published';
post.tags = testUtils.DataGenerator.forKnex.tags.slice(2, 4);
return models.Post.add(post, _.merge({}, options, context.author));
}
}).then(function () {
return models.Post.findAll(_.merge({}, {
context: context.editor.context,
filter: 'author_id:' + userIdFor.editor,
include: ['tags']
}, options));
}).then(function (posts) {
posts.models.length.should.eql(3);
posts.models[0].relations.tags.length.should.eql(1);
_.each(posts.models, function (post) {
postIdsToDelete.push(post.get('id'));
});
return models.Post.findAll(_.merge({
context: context.author.context,
filter: 'author_id:' + userIdFor.author,
include: ['tags']
}, options));
}).then(function (posts) {
posts.models.length.should.eql(3);
posts.models[0].relations.tags.length.should.eql(2);
_.each(posts.models, function (post) {
postIsToKeep.push(post.get('id'));
});
return Promise.mapSeries(postIdsToDelete, function (id) {
return db.knex('posts_tags').where('post_id', id);
});
}).then(function (result) {
_.flatten(result).length.should.eql(3);
return db.knex('tags');
}).then(function (allTags) {
allTags.length.should.eql(5);
return UserAPI.destroy(_.extend({}, context.owner, _.merge({}, options, {id: userIdFor.editor})));
}).then(function () {
return models.User.findOne(_.merge({}, options, {id: userIdFor.editor}));
}).then(function (user) {
should.not.exist(user);
return models.User.findOne(_.merge({}, options, {id: userIdFor.author}));
}).then(function (user) {
should.exist(user);
return models.Post.findAll(_.merge({}, options, {filter: 'author_id:' + userIdFor.editor}));
}).then(function (posts) {
posts.models.length.should.eql(0);
return models.Post.findAll(_.merge({}, options, {filter: 'author_id:' + userIdFor.author}));
}).then(function (posts) {
posts.models.length.should.eql(3);
return Promise.mapSeries(postIdsToDelete, function (id) {
return db.knex('posts_tags').where('post_id', id);
});
}).then(function (result) {
_.flatten(result).length.should.eql(0);
return Promise.mapSeries(postIsToKeep, function (id) {
return db.knex('posts_tags').where('post_id', id);
});
}).then(function (result) {
_.flatten(result).length.should.eql(6);
return db.knex('tags');
}).then(function (allTags) {
allTags.length.should.eql(5);
done();
}).catch(done);
});
});
describe('Owner', function () {
it('CANNOT destroy self', function (done) {
UserAPI.destroy(_.extend({}, context.owner, {id: userIdFor.owner}))