commit 3f6ecc00212a652465867cfb795d1e21dea8e91e
parent 80cfd609ea3ecfda9fa8024abe3ff6edc63f202c
Author: Dan Stillman <dstillman@zotero.org>
Date: Thu, 8 Feb 2018 02:07:44 -0500
Fix "Can't queue event outside of a transaction"
If a transaction took over 30 seconds and another transaction timed out
waiting for it, the second transaction would reset the notifier queue,
but if the first transaction then tried to queue an event, it would fail
with this error and roll back. (It would be nice to figure out why
transactions are taking over 30 seconds, though.)
Diffstat:
4 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js
@@ -485,7 +485,7 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func
// Run begin callbacks
for (var i=0; i<this._callbacks.begin.length; i++) {
if (this._callbacks.begin[i]) {
- this._callbacks.begin[i]();
+ this._callbacks.begin[i](id);
}
}
var conn = this._getConnection(options) || (yield this._getConnectionAsync(options));
@@ -516,13 +516,13 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func
// Run temporary commit callbacks
var f;
while (f = this._callbacks.current.commit.shift()) {
- yield Zotero.Promise.resolve(f());
+ yield Zotero.Promise.resolve(f(id));
}
// Run commit callbacks
for (var i=0; i<this._callbacks.commit.length; i++) {
if (this._callbacks.commit[i]) {
- yield this._callbacks.commit[i]();
+ yield this._callbacks.commit[i](id);
}
}
@@ -549,13 +549,13 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func
// Run temporary commit callbacks
var f;
while (f = this._callbacks.current.rollback.shift()) {
- yield Zotero.Promise.resolve(f());
+ yield Zotero.Promise.resolve(f(id));
}
// Run rollback callbacks
for (var i=0; i<this._callbacks.rollback.length; i++) {
if (this._callbacks.rollback[i]) {
- yield Zotero.Promise.resolve(this._callbacks.rollback[i]());
+ yield Zotero.Promise.resolve(this._callbacks.rollback[i](id));
}
}
diff --git a/chrome/content/zotero/xpcom/notifier.js b/chrome/content/zotero/xpcom/notifier.js
@@ -32,7 +32,7 @@ Zotero.Notifier = new function(){
'collection-item', 'item-tag', 'tag', 'setting', 'group', 'trash',
'bucket', 'relation', 'feed', 'feedItem', 'sync', 'api-key'
];
- var _inTransaction;
+ var _transactionID = false;
var _queue = {};
@@ -106,7 +106,7 @@ Zotero.Notifier = new function(){
* - New events and types should be added to the order arrays in commit()
**/
this.trigger = Zotero.Promise.coroutine(function* (event, type, ids, extraData, force) {
- if (_inTransaction && !force) {
+ if (_transactionID && !force) {
return this.queue(event, type, ids, extraData);
}
@@ -173,7 +173,7 @@ Zotero.Notifier = new function(){
queue = queue._queue;
}
else {
- if (!_inTransaction) {
+ if (!_transactionID) {
throw new Error("Can't queue event outside of a transaction");
}
queue = _queue;
@@ -278,11 +278,11 @@ Zotero.Notifier = new function(){
*
* Note: Be sure the matching commit() gets called (e.g. in a finally{...} block) or
* notifications will break until Firefox is restarted or commit(true)/reset() is called manually
+ *
+ * @param {String} [transactionID]
*/
- this.begin = function () {
- if (!_inTransaction) {
- _inTransaction = true;
- }
+ this.begin = function (transactionID = true) {
+ _transactionID = transactionID;
}
@@ -291,8 +291,9 @@ Zotero.Notifier = new function(){
*
* @param {Zotero.Notifier.Queue|Zotero.Notifier.Queue[]} [queues] - One or more queues to use
* instead of the internal queue
+ * @param {String} [transactionID]
*/
- this.commit = Zotero.Promise.coroutine(function* (queues) {
+ this.commit = Zotero.Promise.coroutine(function* (queues, transactionID = true) {
if (queues) {
if (!Array.isArray(queues)) {
queues = [queues];
@@ -308,7 +309,7 @@ Zotero.Notifier = new function(){
}
}
}
- else if (!_inTransaction) {
+ else if (!_transactionID) {
throw new Error("Can't commit outside of transaction");
}
else {
@@ -375,7 +376,7 @@ Zotero.Notifier = new function(){
}
if (!queues) {
- this.reset();
+ this.reset(transactionID);
}
if (totals) {
@@ -407,10 +408,13 @@ Zotero.Notifier = new function(){
/*
* Reset the event queue
*/
- this.reset = function () {
+ this.reset = function (transactionID = true) {
+ if (transactionID != _transactionID) {
+ return;
+ }
//Zotero.debug("Resetting notifier event queue");
_queue = {};
- _inTransaction = false;
+ _transactionID = false;
}
}
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
@@ -618,9 +618,9 @@ Services.scriptloader.loadSubScript("resource://zotero/polyfill.js");
Zotero.HTTP.triggerProxyAuth();
// Add notifier queue callbacks to the DB layer
- Zotero.DB.addCallback('begin', function () { return Zotero.Notifier.begin(); });
- Zotero.DB.addCallback('commit', function () { return Zotero.Notifier.commit(); });
- Zotero.DB.addCallback('rollback', function () { return Zotero.Notifier.reset(); });
+ Zotero.DB.addCallback('begin', id => Zotero.Notifier.begin(id));
+ Zotero.DB.addCallback('commit', id => Zotero.Notifier.commit(null, id));
+ Zotero.DB.addCallback('rollback', id => Zotero.Notifier.reset(id));
try {
// Require >=2.1b3 database to ensure proper locking
diff --git a/test/tests/notifierTest.js b/test/tests/notifierTest.js
@@ -57,4 +57,25 @@ describe("Zotero.Notifier", function () {
Zotero.Notifier.unregisterObserver(id);
});
});
+
+ describe("#queue", function () {
+ it("should handle notification after DB timeout from another transaction", async function () {
+ var promise1 = Zotero.DB.executeTransaction(async function () {
+ var item = createUnsavedDataObject('item');
+ await item.save();
+
+ await Zotero.Promise.delay(2000);
+
+ Zotero.Notifier.queue('refresh', 'item', item.id);
+ }.bind(this));
+
+ var promise2 = Zotero.DB.executeTransaction(async function () {
+ var item = createUnsavedDataObject('item');
+ await item.save();
+ }.bind(this), { waitTimeout: 1000 });
+
+ await promise1;
+ assert.isTrue(promise2.isRejected());
+ });
+ });
});
