Ghost/core/server/models/plugins/collision.js

94 lines
4.1 KiB
JavaScript
Raw Normal View History

✨ post update collision detection (#8328) (#8362) closes #5599 If two users edit the same post, it can happen that they override each others content or post settings. With this change this won't happen anymore. ✨ Update collision for posts - add a new bookshelf plugin to detect these changes - use the `changed` object of bookshelf -> we don't have to create our own diff - compare client and server updated_at field - run editing posts in a transaction (see comments in code base) 🙀 update collision for tags - `updateTags` for adding posts on `onCreated` - happens after the post was inserted --> it's "okay" to attach the tags afterwards on insert --> there is no need to add collision for inserting data --> it's very hard to move the updateTags call to `onCreating`, because the `updateTags` function queries the database to look up the affected post - `updateTags` while editing posts on `onSaving` - all operations run in a transactions and are rolled back if something get's rejected - Post model edit: if we push a transaction from outside, take this one ✨ introduce options.forUpdate - if two queries happening in a transaction we have to signalise knex/mysql that we select for an update - otherwise the following case happens: >> you fetch posts for an update >> a user requests comes in and updates the post (e.g. sets title to "X") >> you update the fetched posts, title would get overriden to the old one use options.forUpdate and protect internal post updates: model listeners - use a transaction for listener updates - signalise forUpdate - write a complex test use options.forUpdate and protect internal post updates: scheduling - publish endpoint runs in a transaction - add complex test - @TODO: right now scheduling api uses posts api, therefor we had to extend the options for api's >> allowed to pass transactions through it >> but these are only allowed if defined from outside {opts: [...]} >> so i think this is fine and not dirty >> will wait for opinions >> alternatively we have to re-write the scheduling endpoint to use the models directly
2017-04-19 16:53:23 +03:00
var moment = require('moment-timezone'),
Promise = require('bluebird'),
_ = require('lodash'),
common = require('../../lib/common');
✨ post update collision detection (#8328) (#8362) closes #5599 If two users edit the same post, it can happen that they override each others content or post settings. With this change this won't happen anymore. ✨ Update collision for posts - add a new bookshelf plugin to detect these changes - use the `changed` object of bookshelf -> we don't have to create our own diff - compare client and server updated_at field - run editing posts in a transaction (see comments in code base) 🙀 update collision for tags - `updateTags` for adding posts on `onCreated` - happens after the post was inserted --> it's "okay" to attach the tags afterwards on insert --> there is no need to add collision for inserting data --> it's very hard to move the updateTags call to `onCreating`, because the `updateTags` function queries the database to look up the affected post - `updateTags` while editing posts on `onSaving` - all operations run in a transactions and are rolled back if something get's rejected - Post model edit: if we push a transaction from outside, take this one ✨ introduce options.forUpdate - if two queries happening in a transaction we have to signalise knex/mysql that we select for an update - otherwise the following case happens: >> you fetch posts for an update >> a user requests comes in and updates the post (e.g. sets title to "X") >> you update the fetched posts, title would get overriden to the old one use options.forUpdate and protect internal post updates: model listeners - use a transaction for listener updates - signalise forUpdate - write a complex test use options.forUpdate and protect internal post updates: scheduling - publish endpoint runs in a transaction - add complex test - @TODO: right now scheduling api uses posts api, therefor we had to extend the options for api's >> allowed to pass transactions through it >> but these are only allowed if defined from outside {opts: [...]} >> so i think this is fine and not dirty >> will wait for opinions >> alternatively we have to re-write the scheduling endpoint to use the models directly
2017-04-19 16:53:23 +03:00
module.exports = function (Bookshelf) {
var ParentModel = Bookshelf.Model,
Model;
Model = Bookshelf.Model.extend({
/**
* Update collision protection.
*
* IMPORTANT NOTES:
* The `sync` method is called for any query e.g. update, add, delete, fetch
*
* We had the option to override Bookshelf's `save` method, but hooking into the `sync` method gives us
* the ability to access the `changed` object. Bookshelf already knows which attributes has changed.
*
* Bookshelf's timestamp function can't be overridden, as it's synchronous, there is no way to return an Error.
*
* If we want to enable the collision plugin for other tables, the queries might need to run in a transaction.
* This depends on if we fetch the model before editing. Imagine two concurrent requests come in, both would fetch
* the same current database values and both would succeed to update and override each other.
*/
sync: function timestamp(options) {
var parentSync = ParentModel.prototype.sync.apply(this, arguments),
originalUpdateSync = parentSync.update,
self = this;
// CASE: only enabled for posts table
if (this.tableName !== 'posts' ||
!self.serverData ||
((options.method !== 'update' && options.method !== 'patch') || !options.method)
) {
return parentSync;
}
/**
* Only hook into the update sync
*
* IMPORTANT NOTES:
* Even if the client sends a different `id` property, it get's ignored by bookshelf.
* Because you can't change the `id` of an existing post.
*
* HTML is always auto generated, ignore.
*/
parentSync.update = function update() {
var changed = _.omit(self.changed, [
'created_at', 'updated_at', 'author_id', 'id',
'published_by', 'updated_by', 'html', 'plaintext'
]),
clientUpdatedAt = moment(self.clientData.updated_at || self.serverData.updated_at || new Date()),
serverUpdatedAt = moment(self.serverData.updated_at || clientUpdatedAt);
✨ post update collision detection (#8328) (#8362) closes #5599 If two users edit the same post, it can happen that they override each others content or post settings. With this change this won't happen anymore. ✨ Update collision for posts - add a new bookshelf plugin to detect these changes - use the `changed` object of bookshelf -> we don't have to create our own diff - compare client and server updated_at field - run editing posts in a transaction (see comments in code base) 🙀 update collision for tags - `updateTags` for adding posts on `onCreated` - happens after the post was inserted --> it's "okay" to attach the tags afterwards on insert --> there is no need to add collision for inserting data --> it's very hard to move the updateTags call to `onCreating`, because the `updateTags` function queries the database to look up the affected post - `updateTags` while editing posts on `onSaving` - all operations run in a transactions and are rolled back if something get's rejected - Post model edit: if we push a transaction from outside, take this one ✨ introduce options.forUpdate - if two queries happening in a transaction we have to signalise knex/mysql that we select for an update - otherwise the following case happens: >> you fetch posts for an update >> a user requests comes in and updates the post (e.g. sets title to "X") >> you update the fetched posts, title would get overriden to the old one use options.forUpdate and protect internal post updates: model listeners - use a transaction for listener updates - signalise forUpdate - write a complex test use options.forUpdate and protect internal post updates: scheduling - publish endpoint runs in a transaction - add complex test - @TODO: right now scheduling api uses posts api, therefor we had to extend the options for api's >> allowed to pass transactions through it >> but these are only allowed if defined from outside {opts: [...]} >> so i think this is fine and not dirty >> will wait for opinions >> alternatively we have to re-write the scheduling endpoint to use the models directly
2017-04-19 16:53:23 +03:00
if (Object.keys(changed).length) {
if (clientUpdatedAt.diff(serverUpdatedAt) !== 0) {
// @TODO: Remove later. We want to figure out how many people experience the error in which context.
return Promise.reject(new common.errors.UpdateCollisionError({
✨ post update collision detection (#8328) (#8362) closes #5599 If two users edit the same post, it can happen that they override each others content or post settings. With this change this won't happen anymore. ✨ Update collision for posts - add a new bookshelf plugin to detect these changes - use the `changed` object of bookshelf -> we don't have to create our own diff - compare client and server updated_at field - run editing posts in a transaction (see comments in code base) 🙀 update collision for tags - `updateTags` for adding posts on `onCreated` - happens after the post was inserted --> it's "okay" to attach the tags afterwards on insert --> there is no need to add collision for inserting data --> it's very hard to move the updateTags call to `onCreating`, because the `updateTags` function queries the database to look up the affected post - `updateTags` while editing posts on `onSaving` - all operations run in a transactions and are rolled back if something get's rejected - Post model edit: if we push a transaction from outside, take this one ✨ introduce options.forUpdate - if two queries happening in a transaction we have to signalise knex/mysql that we select for an update - otherwise the following case happens: >> you fetch posts for an update >> a user requests comes in and updates the post (e.g. sets title to "X") >> you update the fetched posts, title would get overriden to the old one use options.forUpdate and protect internal post updates: model listeners - use a transaction for listener updates - signalise forUpdate - write a complex test use options.forUpdate and protect internal post updates: scheduling - publish endpoint runs in a transaction - add complex test - @TODO: right now scheduling api uses posts api, therefor we had to extend the options for api's >> allowed to pass transactions through it >> but these are only allowed if defined from outside {opts: [...]} >> so i think this is fine and not dirty >> will wait for opinions >> alternatively we have to re-write the scheduling endpoint to use the models directly
2017-04-19 16:53:23 +03:00
message: 'Saving failed! Someone else is editing this post.',
code: 'UPDATE_COLLISION',
level: 'critical',
context: JSON.stringify({
clientUpdatedAt: self.clientData.updated_at,
serverUpdatedAt: self.serverData.updated_at,
changed: changed
})
✨ post update collision detection (#8328) (#8362) closes #5599 If two users edit the same post, it can happen that they override each others content or post settings. With this change this won't happen anymore. ✨ Update collision for posts - add a new bookshelf plugin to detect these changes - use the `changed` object of bookshelf -> we don't have to create our own diff - compare client and server updated_at field - run editing posts in a transaction (see comments in code base) 🙀 update collision for tags - `updateTags` for adding posts on `onCreated` - happens after the post was inserted --> it's "okay" to attach the tags afterwards on insert --> there is no need to add collision for inserting data --> it's very hard to move the updateTags call to `onCreating`, because the `updateTags` function queries the database to look up the affected post - `updateTags` while editing posts on `onSaving` - all operations run in a transactions and are rolled back if something get's rejected - Post model edit: if we push a transaction from outside, take this one ✨ introduce options.forUpdate - if two queries happening in a transaction we have to signalise knex/mysql that we select for an update - otherwise the following case happens: >> you fetch posts for an update >> a user requests comes in and updates the post (e.g. sets title to "X") >> you update the fetched posts, title would get overriden to the old one use options.forUpdate and protect internal post updates: model listeners - use a transaction for listener updates - signalise forUpdate - write a complex test use options.forUpdate and protect internal post updates: scheduling - publish endpoint runs in a transaction - add complex test - @TODO: right now scheduling api uses posts api, therefor we had to extend the options for api's >> allowed to pass transactions through it >> but these are only allowed if defined from outside {opts: [...]} >> so i think this is fine and not dirty >> will wait for opinions >> alternatively we have to re-write the scheduling endpoint to use the models directly
2017-04-19 16:53:23 +03:00
}));
}
}
return originalUpdateSync.apply(this, arguments);
};
return parentSync;
},
/**
* We have to remember current server data and client data.
* The `sync` method has no access to it.
* `updated_at` is already set to "Date.now" when the overridden `sync.update` is called.
* See https://github.com/tgriesser/bookshelf/blob/79c526870e618748caf94e7476a0bc796ee090a6/src/model.js#L955
*/
save: function save(data) {
this.clientData = _.cloneDeep(data) || {};
this.serverData = _.cloneDeep(this.attributes);
return ParentModel.prototype.save.apply(this, arguments);
}
});
Bookshelf.Model = Model;
};