Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add row-level locking options #1810

Merged
merged 3 commits into from
Apr 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/bookshelf.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,11 @@ function Bookshelf(knex) {
* resolve when the transaction is committed, or fail if the transaction is
* rolled back.
*
* When fetching inside a transaction it's possible to specify a row-level
* lock by passing the wanted lock type in the `lock` option to
* {@linkcode Model#fetch fetch}. Available options are `forUpdate` and
* `forShare`.
*
* var Promise = require('bluebird');
*
* Bookshelf.transaction(function(t) {
Expand Down
10 changes: 9 additions & 1 deletion src/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,12 @@ const BookshelfCollection = CollectionBase.extend({
* Model.NotFoundError NotFoundError} if no result is found.
* @param {(string|string[])} [options.columns='*']
* Limit the number of columns fetched.
* @param {Transaction} options.transacting
* @param {Transaction} [options.transacting]
* Optionally run the query in a transaction.
* @param {string} [options.lock]
* Type of row-level lock to use. Valid options are `forShare` and
* `forUpdate`. This only works in conjunction with the `transacting`
* option, and requires a database that supports it.
*
* @throws {Model.NotFoundError}
* @returns {Promise<Model|null>}
Expand All @@ -240,6 +244,10 @@ const BookshelfCollection = CollectionBase.extend({
* @param {string|string[]} relations The relation, or relations, to be loaded.
* @param {Object=} options Hash of options.
* @param {Transaction=} options.transacting
* @param {string=} options.lock
* Type of row-level lock to use. Valid options are `forShare` and
* `forUpdate`. This only works in conjunction with the `transacting`
* option, and requires a database that supports it.
*
* @returns {Promise<Collection>} A promise resolving to this {@link
* Collection collection}
Expand Down
8 changes: 8 additions & 0 deletions src/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,10 @@ const BookshelfModel = ModelBase.extend({
* Specify columns to be retrieved.
* @param {Transaction} [options.transacting]
* Optionally run the query in a transaction.
* @param {string} [options.lock]
* Type of row-level lock to use. Valid options are `forShare` and
* `forUpdate`. This only works in conjunction with the `transacting`
* option, and requires a database that supports it.
* @param {string|Object|mixed[]} [options.withRelated]
* Relations to be retrieved with `Model` instance. Either one or more
* relation names or objects mapping relation names to query callbacks.
Expand Down Expand Up @@ -859,6 +863,10 @@ const BookshelfModel = ModelBase.extend({
* @param {Object=} options Hash of options.
* @param {Transaction=} options.transacting
* Optionally run the query in a transaction.
* @param {string=} options.lock
* Type of row-level lock to use. Valid options are `forShare` and
* `forUpdate`. This only works in conjunction with the `transacting`
* option, and requires a database that supports it.
* @returns {Promise<Model>} A promise resolving to this {@link Model model}
*/
load: Promise.method(function(relations, options) {
Expand Down
6 changes: 5 additions & 1 deletion src/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import _ from 'lodash';
import Promise from './base/promise';

const supportsReturning = (client) => _.includes(['postgresql', 'postgres', 'pg', 'oracle', 'mssql'], client)
const validLocks = ['forShare', 'forUpdate']

// Sync is the dispatcher for any database queries,
// taking the "syncing" `model` or `collection` being queried, along with
Expand All @@ -16,7 +17,10 @@ const Sync = function(syncing, options) {
this.syncing = syncing.resetQuery();
this.options = options;
if (options.debug) this.query.debug();
if (options.transacting) this.query.transacting(options.transacting);
if (options.transacting) {
this.query.transacting(options.transacting);
if (validLocks.indexOf(options.lock) > -1) this.query[options.lock]();
}
if (options.withSchema) this.query.withSchema(options.withSchema);
};

Expand Down
50 changes: 50 additions & 0 deletions test/integration/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,56 @@ module.exports = function(bookshelf) {
expect(author.get('name')).to.eql('Ryan Coogler');
})
})

it('locks the table when called with the forUpdate option during a transaction', function() {
var newAuthorId;

return new Models.Author().save({first_name: 'foo', site_id: 1}).then(function(author) {
newAuthorId = author.id;

return Promise.all([
bookshelf.transaction(function(t) {
return new Models.Author({id: author.id}).fetch({transacting: t, lock: 'forUpdate'}).then(function() {
return Promise.delay(100)
}).then(function() {
return new Models.Author({id: author.id}).fetch({transacting: t})
}).then(function(author) {
expect(author.get('first_name')).to.equal('foo')
})
}),
Promise.delay(25).then(function() {
return new Models.Author({id: author.id}).save({first_name: 'changed'})
})
])
}).then(function() {
return new Models.Author({id: newAuthorId}).destroy()
})
})

it('locks the table when called with the forShare option during a transaction', function() {
var newAuthorId;

return new Models.Author().save({first_name: 'foo', site_id: 1}).then(function(author) {
newAuthorId = author.id;

return Promise.all([
bookshelf.transaction(function(t) {
return new Models.Author({id: author.id}).fetch({transacting: t, lock: 'forShare'}).then(function() {
return Promise.delay(100)
}).then(function() {
return new Models.Author({id: author.id}).fetch({transacting: t})
}).then(function(author) {
expect(author.get('first_name')).to.equal('foo')
})
}),
Promise.delay(25).then(function() {
return new Models.Author({id: author.id}).save({first_name: 'changed'})
})
])
}).then(function() {
return new Models.Author({id: newAuthorId}).destroy()
})
})
});

describe('fetchAll', function() {
Expand Down
28 changes: 28 additions & 0 deletions test/unit/sql/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,34 @@ module.exports = function() {
setSchema.should.have.been.calledWith(testSchema);
})

it('accepts a lock option option if called with a transaction', function() {
var setLock = sinon.spy();
var mockModel = {
query: function() {
return {forUpdate: setLock, transacting: function() {}}
},
resetQuery: function() {}
}

new Sync(mockModel, {lock: 'forUpdate', transacting: 'something'});

setLock.should.have.been.called;
})

it('ignores the lock option if called without a transaction', function() {
var setLock = sinon.spy();
var mockModel = {
query: function() {
return {forUpdate: setLock, transacting: function() {}}
},
resetQuery: function() {}
}

new Sync(mockModel, {lock: 'forUpdate'});

setLock.should.not.have.been.called;
})

describe('prefixFields', function() {
it('should prefix all keys of the passed in object with the tablename', function() {
var sync = new Sync(stubModel());
Expand Down