From 3f6ecc00212a652465867cfb795d1e21dea8e91e Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 8 Feb 2018 02:07:44 -0500
Subject: [PATCH] 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.)
---
 chrome/content/zotero/xpcom/db.js       | 10 ++++-----
 chrome/content/zotero/xpcom/notifier.js | 28 ++++++++++++++-----------
 chrome/content/zotero/xpcom/zotero.js   |  6 +++---
 test/tests/notifierTest.js              | 21 +++++++++++++++++++
 4 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js
index 305ac23dc..9450881d4 100644
--- 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
index 42c2473ae..6ffda23b3 100644
--- 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
index 31c5df348..054f26aa8 100644
--- 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
index 1c49f1f4f..c803370ad 100644
--- 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());
+		});
+	});
 });