Add Zotero.Notifier.Queue to keep event groups separate, and use for sync

A queue can be created and passed as an option to data layer methods, which
will then queue events on that queue instead of the main internal queue. A
queue or an array of queues can then be passed to Zotero.Notifier.commit() to
commit those events.

Some auxiliary functions don't yet take a queue, so those events will still get
run on DB transaction commit.

Sync data processing now processes notifier events in batches to reduce
repaints, even though individual objects are processed within their own
transactions (so that failures don't roll back other objects' data).

Also remove some unused notifier code

chrome/content/zotero/xpcom/collectionTreeView.js

@@ -2087,7 +2087,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r
 			var parentCollectionID = false;
 		}
 		
-		var unlock = Zotero.Notifier.begin(true);
+		var commitNotifier = Zotero.Notifier.begin();
 		try {
 			for (var i=0; i<data.length; i++) {
 				var file = data[i];

chrome/content/zotero/xpcom/data/collection.js

@@ -326,10 +326,14 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env)
 Zotero.Collection.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) {
 	if (!env.options.skipNotifier) {
 		if (env.isNew) {
-			Zotero.Notifier.queue('add', 'collection', this.id, env.notifierData);
+			Zotero.Notifier.queue(
+				'add', 'collection', this.id, env.notifierData, env.options.notifierQueue
+			);
 		}
 		else {
-			Zotero.Notifier.queue('modify', 'collection', this.id, env.notifierData);
+			Zotero.Notifier.queue(
+				'modify', 'collection', this.id, env.notifierData, env.options.notifierQueue
+			);
 		}
 	}
 	

chrome/content/zotero/xpcom/data/dataObject.js

@@ -1189,7 +1189,8 @@ Zotero.DataObject.prototype._finalizeErase = Zotero.Promise.coroutine(function*
 			'delete',
 			this._objectType,
 			Object.keys(env.notifierData).map(id => parseInt(id)),
-			env.notifierData
+			env.notifierData,
+			env.options.notifierQueue
 		);
 	}
 });

chrome/content/zotero/xpcom/data/feed.js

@@ -271,7 +271,7 @@ Zotero.Feed.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			+ "VALUES (" + Array(params.length).fill('?').join(', ') + ")";
 		yield Zotero.DB.queryAsync(sql, params);
 		
-		Zotero.Notifier.queue('add', 'feed', this.libraryID);
+		Zotero.Notifier.queue('add', 'feed', this.libraryID, env.options.notifierQueue);
 	}
 	else if (changedCols.length) {
 		let sql = "UPDATE feeds SET " + changedCols.map(v => v + '=?').join(', ')
@@ -279,7 +279,7 @@ Zotero.Feed.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		params.push(this.libraryID);
 		yield Zotero.DB.queryAsync(sql, params);
 		
-		Zotero.Notifier.queue('modify', 'feed', this.libraryID);
+		Zotero.Notifier.queue('modify', 'feed', this.libraryID, env.options.notifierQueue);
 	}
 	else {
 		Zotero.debug("Feed data did not change for feed " + this.libraryID, 5);
@@ -305,12 +305,12 @@ Zotero.Feed.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) {
 	
 });
 
-Zotero.Feed.prototype._finalizeErase = Zotero.Promise.coroutine(function* (){
+Zotero.Feed.prototype._finalizeErase = Zotero.Promise.coroutine(function* (env) {
 	let notifierData = {};
 	notifierData[this.libraryID] = {
 		libraryID: this.libraryID
 	};
-	Zotero.Notifier.trigger('delete', 'feed', this.id, notifierData);
+	Zotero.Notifier.queue('delete', 'feed', this.id, notifierData, env.options.notifierQueue);
 	Zotero.Feeds.unregister(this.libraryID);
 
 	let syncedFeeds = Zotero.SyncedSettings.get(Zotero.Libraries.userLibraryID, 'feeds') || {};
@@ -395,7 +395,7 @@ Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () {
 	}
 	let deferred = Zotero.Promise.defer();
 	this._updating = deferred.promise;
-	Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
+	yield Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
 	this._set('_feedLastCheckError', null);
 	
 	try {
@@ -491,7 +491,7 @@ Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () {
 	yield this.updateUnreadCount();
 	deferred.resolve();
 	this._updating = false;
-	Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
+	yield Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
 });
 
 Zotero.Feed.prototype.updateFeed = Zotero.Promise.coroutine(function* () {
@@ -511,6 +511,6 @@ Zotero.Feed.prototype.updateUnreadCount = Zotero.Promise.coroutine(function* ()
 	
 	if (newCount != this._feedUnreadCount) {
 		this._feedUnreadCount = newCount;
-		Zotero.Notifier.trigger('unreadCountUpdated', 'feed', this.id);
+		yield Zotero.Notifier.trigger('unreadCountUpdated', 'feed', this.id);
 	}
 });

chrome/content/zotero/xpcom/data/item.js

@@ -1269,7 +1269,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		
 		if (!env.options.skipNotifier) {
-			Zotero.Notifier.queue('add', 'item', itemID, env.notifierData);
+			Zotero.Notifier.queue('add', 'item', itemID, env.notifierData, env.options.notifierQueue);
 		}
 	}
 	else {
@@ -1278,7 +1278,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		yield Zotero.DB.queryAsync(sql, env.sqlValues);
 		
 		if (!env.options.skipNotifier) {
-			Zotero.Notifier.queue('modify', 'item', itemID, env.notifierData);
+			Zotero.Notifier.queue('modify', 'item', itemID, env.notifierData, env.options.notifierQueue);
 		}
 	}
 	
@@ -1387,7 +1387,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			
 			let newParentItemNotifierData = {};
 			//newParentItemNotifierData[newParentItem.id] = {};
-			Zotero.Notifier.queue('modify', 'item', parentItemID, newParentItemNotifierData);
+			Zotero.Notifier.queue(
+				'modify', 'item', parentItemID, newParentItemNotifierData, env.options.notifierQueue
+			);
 			
 			switch (Zotero.ItemTypes.getName(itemTypeID)) {
 				case 'note':
@@ -1411,7 +1413,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 				
 				let newParentItemNotifierData = {};
 				//newParentItemNotifierData[newParentItem.id] = {};
-				Zotero.Notifier.queue('modify', 'item', parentItemID, newParentItemNotifierData);
+				Zotero.Notifier.queue(
+					'modify',
+					'item',
+					parentItemID,
+					newParentItemNotifierData,
+					env.options.notifierQueue
+				);
 			}
 			
 			let oldParentKey = this._previousData.parentKey;
@@ -1421,7 +1429,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 				if (oldParentItemID) {
 					let oldParentItemNotifierData = {};
 					//oldParentItemNotifierData[oldParentItemID] = {};
-					Zotero.Notifier.queue('modify', 'item', oldParentItemID, oldParentItemNotifierData);
+					Zotero.Notifier.queue(
+						'modify',
+						'item',
+						oldParentItemID,
+						oldParentItemNotifierData,
+						env.options.notifierQueue
+					);
 				}
 				else {
 					Zotero.debug("Old source item " + oldParentKey
@@ -1445,7 +1459,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 						Zotero.Notifier.queue(
 							'remove',
 							'collection-item',
-							changedCollections[i] + '-' + this.id
+							changedCollections[i] + '-' + this.id,
+							{},
+							env.options.notifierQueue
 						);
 					}
 					let parentOptions = {
@@ -1511,9 +1527,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		yield Zotero.DB.queryAsync(sql, itemID);
 		
 		// Refresh trash
-		Zotero.Notifier.queue('refresh', 'trash', this.libraryID);
+		Zotero.Notifier.queue('refresh', 'trash', this.libraryID, {}, env.options.notifierQueue);
 		if (this._deleted) {
-			Zotero.Notifier.queue('trash', 'item', this.id);
+			Zotero.Notifier.queue('trash', 'item', this.id, {}, env.options.notifierQueue);
 		}
 		
 		if (parentItemID) {
@@ -1633,7 +1649,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 				tag: tag.tag,
 				type: tagType
 			};
-			Zotero.Notifier.queue('add', 'item-tag', this.id + '-' + tagID, notifierData);
+			Zotero.Notifier.queue(
+				'add', 'item-tag', this.id + '-' + tagID, notifierData, env.options.notifierQueue
+			);
 		}
 		
 		if (toRemove.length) {
@@ -1647,7 +1665,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 					libraryID: this.libraryID,
 					tag: tag.tag
 				};
-				Zotero.Notifier.queue('remove', 'item-tag', this.id + '-' + tagID, notifierData);
+				Zotero.Notifier.queue(
+					'remove', 'item-tag', this.id + '-' + tagID, notifierData, env.options.notifierQueue
+				);
 			}
 			Zotero.Prefs.set('purge.tags', true);
 		}
@@ -1680,7 +1700,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 					+ "(collectionID, itemID, orderIndex) VALUES (?, ?, ?)";
 				yield Zotero.DB.queryAsync(sql, [collectionID, this.id, orderIndex]);
 				
-				Zotero.Notifier.queue('add', 'collection-item', collectionID + '-' + this.id);
+				Zotero.Notifier.queue(
+					'add',
+					'collection-item',
+					collectionID + '-' + this.id,
+					{},
+					env.options.notifierQueue
+				);
 			}
 			
 			// Add this item to any loaded collections' cached item lists after commit
@@ -1700,7 +1726,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			for (let i=0; i<toRemove.length; i++) {
 				let collectionID = toRemove[i];
 				
-				Zotero.Notifier.queue('remove', 'collection-item', collectionID + '-' + this.id);
+				Zotero.Notifier.queue(
+					'remove',
+					'collection-item',
+					collectionID + '-' + this.id,
+					{},
+					env.options.notifierQueue
+				);
 			}
 			
 			// Remove this item from any loaded collections' cached item lists after commit

chrome/content/zotero/xpcom/notifier.js

@@ -27,21 +27,13 @@
 
 Zotero.Notifier = new function(){
 	var _observers = {};
-	var _disabled = false;
 	var _types = [
 		'collection', 'search', 'share', 'share-items', 'item', 'file',
 		'collection-item', 'item-tag', 'tag', 'setting', 'group', 'trash', 'publications',
 		'bucket', 'relation', 'feed', 'feedItem'
 	];
 	var _inTransaction;
-	var _locked = false;
+	var _queue = {};
-	var _queue = [];
-	
-	this.begin = begin;
-	this.reset = reset;
-	this.disable = disable;
-	this.enable = enable;
-	this.isEnabled = isEnabled;
 	
 	
 	this.registerObserver = function (ref, types, id, priority) {
@@ -74,7 +66,6 @@ Zotero.Notifier = new function(){
 		if (priority) {
 			msg += " with priority " + priority;
 		}
-		Zotero.debug(msg);
 		_observers[hash] = {
 			ref: ref,
 			types: types,
@@ -112,11 +103,6 @@ Zotero.Notifier = new function(){
 			return this.queue(event, type, ids, extraData);
 		}
 		
-		if (_disabled){
-			Zotero.debug("Notifications are disabled");
-			return false;
-		}
-		
 		if (_types && _types.indexOf(type) == -1) {
 			throw new Error("Invalid type '" + type + "'");
 		}
@@ -163,12 +149,7 @@ Zotero.Notifier = new function(){
 	 *
 	 * @throws If a notifier transaction isn't currently open
 	 */
-	this.queue = function (event, type, ids, extraData) {
+	this.queue = function (event, type, ids, extraData, queue) {
-		if (_disabled){
-			Zotero.debug("Notifications are disabled");
-			return false;
-		}
-		
 		if (_types && _types.indexOf(type) == -1) {
 			throw new Error("Invalid type '" + type + "'");
 		}
@@ -176,51 +157,62 @@ Zotero.Notifier = new function(){
 		ids = Zotero.flattenArguments(ids);
 		
 		if (Zotero.Debug.enabled) {
-			_logTrigger(event, type, ids, extraData, true);
+			_logTrigger(event, type, ids, extraData, true, queue ? queue.id : null);
 		}
 		
+		// Use a queue if one is provided, or else use main queue
+		if (queue) {
+			queue.size++;
+			queue = queue._queue;
+		}
+		else {
 			if (!_inTransaction) {
 				throw new Error("Can't queue event outside of a transaction");
 			}
+			queue = _queue;
+		}
 		
+		_mergeEvent(queue, event, type, ids, extraData);
+	}
+	
+	
+	function _mergeEvent(queue, event, type, ids, extraData) {
 		// Merge with existing queue
-		if (!_queue[type]) {
+		if (!queue[type]) {
-			_queue[type] = [];
+			queue[type] = [];
 		}
-		if (!_queue[type][event]) {
+		if (!queue[type][event]) {
-			_queue[type][event] = {};
+			queue[type][event] = {};
 		}
-		if (!_queue[type][event].ids) {
+		if (!queue[type][event].ids) {
-			_queue[type][event].ids = [];
+			queue[type][event].ids = [];
-			_queue[type][event].data = {};
+			queue[type][event].data = {};
 		}
 		
 		// Merge ids
-		_queue[type][event].ids = _queue[type][event].ids.concat(ids);
+		queue[type][event].ids = queue[type][event].ids.concat(ids);
 		
 		// Merge extraData keys
 		if (extraData) {
 			// If just a single id, extra data can be keyed by id or passed directly
 			if (ids.length == 1) {
 				let id = ids[0];
-				_queue[type][event].data[id] = extraData[id] ? extraData[id] : extraData;
+				queue[type][event].data[id] = extraData[id] ? extraData[id] : extraData;
 			}
 			// For multiple ids, check for data keyed by the id
 			else {
 				for (let i = 0; i < ids.length; i++) {
 					let id = ids[i];
 					if (extraData[id]) {
-						_queue[type][event].data[id] = extraData[id];
+						queue[type][event].data[id] = extraData[id];
+					}
 				}
 			}
 		}
 	}
 	
-		return true;
-	}
 	
-	
+	function _logTrigger(event, type, ids, extraData, queueing, queueID) {
-	function _logTrigger(event, type, ids, extraData, queueing) {
 		Zotero.debug("Notifier.trigger("
 			+ "'" + event + "', "
 			+ "'" + type + "', "
@@ -228,7 +220,7 @@ Zotero.Notifier = new function(){
 			+ (extraData ? ", " + JSON.stringify(extraData) : "")
 			+ ")"
 			+ (queueing
-				? " queued "
+				? " " + (queueID ? "added to queue " + queueID : "queued") + " "
 				: " called "
 			+ "[observers: " + _countObserversForType(type) + "]")
 		);
@@ -274,66 +266,46 @@ Zotero.Notifier = new function(){
 	}
 	
 	
-	this.untrigger = function (event, type, ids) {
+	/**
-		if (!_inTransaction) {
-			throw ("Zotero.Notifier.untrigger() called with no active event queue")
-		}
-		
-		ids = Zotero.flattenArguments(ids);
-		
-		for each(var id in ids) {
-			var index = _queue[type][event].ids.indexOf(id);
-			if (index == -1) {
-				Zotero.debug(event + '-' + type + ' id ' + id +
-					' not found in queue in Zotero.Notifier.untrigger()');
-				continue;
-			}
-			_queue[type][event].ids.splice(index, 1);
-			delete _queue[type][event].data[id];
-		}
-	}
-	
-	
-	/*
 	 * Begin queueing event notifications (i.e. don't notify the observers)
 	 *
-	 * _lock_ will prevent subsequent commits from running the queue until commit() is called
-	 * with the _unlock_ being true
-	 *
 	 * 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
 	 */
-	function begin(lock) {
+	this.begin = function () {
-		if (lock && !_locked) {
+		if (!_inTransaction) {
-			_locked = true;
-			var unlock = true;
-		}
-		else {
-			var unlock = false;
-		}
-		
-		if (_inTransaction) {
-			//Zotero.debug("Notifier queue already open", 4);
-		}
-		else {
-			//Zotero.debug("Beginning notifier event queue");
 			_inTransaction = true;
 		}
-		
-		return unlock;
 	}
 	
 	
-	/*
+	/**
 	 * Send notifications for ids in the event queue
 	 *
-	 * If the queue is locked, notifications will only run if _unlock_ is true
+	 * @param {Zotero.Notifier.Queue|Zotero.Notifier.Queue[]} [queues] - One or more queues to use
+	 *     instead of the internal queue
 	 */
-	this.commit = Zotero.Promise.coroutine(function* (unlock) {
+	this.commit = Zotero.Promise.coroutine(function* (queues) {
-		// If there's a lock on the event queue and _unlock_ isn't given, don't commit
+		if (queues) {
-		if ((unlock == undefined && _locked) || (unlock != undefined && !unlock)) {
+			if (!Array.isArray(queues)) {
-			//Zotero.debug("Keeping Notifier event queue open", 4);
+				queues = [queues];
-			return;
+			}
+			
+			var queue = {};
+			for (let q of queues) {
+				q = q._queue;
+				for (let type in q) {
+					for (let event in q[type]) {
+						_mergeEvent(queue, event, type, q[type][event].ids, q[type][event].data);
+					}
+				}
+			}
+		}
+		else if (!_inTransaction) {
+			throw new Error("Can't commit outside of transaction");
+		}
+		else {
+			var queue = _queue;
 		}
 		
 		var runQueue = [];
@@ -352,7 +324,7 @@ Zotero.Notifier = new function(){
 		var typeOrder = ['collection', 'search', 'item', 'collection-item', 'item-tag', 'tag'];
 		var eventOrder = ['add', 'modify', 'remove', 'move', 'delete', 'trash'];
 		
-		var queueTypes = Object.keys(_queue);
+		var queueTypes = Object.keys(queue);
 		queueTypes.sort(getSorter(typeOrder));
 		
 		var totals = '';
@@ -361,18 +333,18 @@ Zotero.Notifier = new function(){
 				runQueue[type] = [];
 			}
 			
-			let typeEvents = Object.keys(_queue[type]);
+			let typeEvents = Object.keys(queue[type]);
 			typeEvents.sort(getSorter(eventOrder));
 			
 			for (let event of typeEvents) {
 				runQueue[type][event] = {
 					ids: [],
-					data: _queue[type][event].data
+					data: queue[type][event].data
 				};
 				
 				// Remove redundant ids
-				for (var i=0; i<_queue[type][event].ids.length; i++) {
+				for (let i = 0; i < queue[type][event].ids.length; i++) {
-					var id = _queue[type][event].ids[i];
+					let id = queue[type][event].ids[i];
 					
 					// Don't send modify on nonexistent items or tags
 					if (event == 'modify') {
@@ -395,13 +367,21 @@ Zotero.Notifier = new function(){
 			}
 		}
 		
-		reset();
+		if (!queues) {
+			this.reset();
+		}
 		
 		if (totals) {
+			if (queues) {
+				Zotero.debug("Committing notifier event queues" + totals
+					+ " [queues: " + queues.map(q => q.id).join(", ") + "]");
+			}
+			else {
 				Zotero.debug("Committing notifier event queue" + totals);
+			}
 			
-			for (var type in runQueue) {
+			for (let type in runQueue) {
-				for (var event in runQueue[type]) {
+				for (let event in runQueue[type]) {
 					if (runQueue[type][event].ids.length || event == 'refresh') {
 						yield this.trigger(
 							event,
@@ -420,41 +400,17 @@ Zotero.Notifier = new function(){
 	/*
 	 * Reset the event queue
 	 */
-	function reset() {
+	this.reset = function () {
 		//Zotero.debug("Resetting notifier event queue");
-		_locked = false;
+		_queue = {};
-		_queue = [];
 		_inTransaction = false;
 	}
-	
-	
-	// 
-	// These should rarely be used now that we have event queuing
-	//
-	
-	/*
-	 * Disables Notifier notifications
-	 *
-	 * Returns false if the Notifier was already disabled, true otherwise
-	 */
-	function disable() {
-		if (_disabled) {
-			Zotero.debug('Notifier notifications are already disabled');
-			return false;
-		}
-		Zotero.debug('Disabling Notifier notifications'); 
-		_disabled = true;
-		return true;
 }
 
 
-	function enable() {
+Zotero.Notifier.Queue = function () {
-		Zotero.debug('Enabling Notifier notifications');
+	this.id = Zotero.Utilities.randomString();
-		_disabled = false; 
+	Zotero.debug("Creating notifier queue " + this.id);
-	}
+	this._queue = {};
-	
+	this.size = 0;
-	
+};
-	function isEnabled() {
-		return !_disabled;
-	}
-}

chrome/content/zotero/xpcom/search.js

@@ -210,10 +210,10 @@ Zotero.Search.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env)
 		// Update library searches status
 		yield Zotero.Libraries.get(this.libraryID).updateSearches();
 		
-		Zotero.Notifier.queue('add', 'search', this.id, env.notifierData);
+		Zotero.Notifier.queue('add', 'search', this.id, env.notifierData, env.options.notifierQueue);
 	}
 	else if (!env.options.skipNotifier) {
-		Zotero.Notifier.queue('modify', 'search', this.id, env.notifierData);
+		Zotero.Notifier.queue('modify', 'search', this.id, env.notifierData, env.options.notifierQueue);
 	}
 	
 	if (env.isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) {

chrome/content/zotero/xpcom/sync/syncLocal.js

@@ -433,17 +433,15 @@ Zotero.Sync.Data.Local = {
 		}
 		
 		var batchSize = 10;
-		var batchCounter = 0;
+		var notifierQueues = [];
 		try {
 			for (let i = 0; i < json.length; i++) {
 				// Batch notifier updates
-				if (batchCounter == 0) {
+				if (notifierQueues.length == batchSize) {
-					Zotero.Notifier.begin();
+					yield Zotero.Notifier.commit(notifierQueues);
-				}
+					notifierQueues = [];
-				else if (batchCounter == batchSize || i == json.length - 1) {
-					Zotero.Notifier.commit();
-					Zotero.Notifier.begin();
 				}
+				let notifierQueue = new Zotero.Notifier.Queue;
 				
 				let jsonObject = json[i];
 				let jsonData = jsonObject.data;
@@ -454,6 +452,7 @@ Zotero.Sync.Data.Local = {
 				saveOptions.isNewObject = false;
 				saveOptions.skipCache = false;
 				saveOptions.storageDetailsChanged = false;
+				saveOptions.notifierQueue = notifierQueue;
 				
 				Zotero.debug(`Processing ${objectType} ${libraryID}/${objectKey}`);
 				Zotero.debug(jsonObject);
@@ -542,14 +541,14 @@ Zotero.Sync.Data.Local = {
 										obj,
 										jsonObject,
 										{
-											skipData: true
+											skipData: true,
+											notifierQueue
 										}
 									);
 									results.push(saveResults);
 									if (!saveResults.processed) {
 										throw saveResults.error;
 									}
-									batchCounter++;
 									return;
 								}
 								
@@ -643,8 +642,11 @@ Zotero.Sync.Data.Local = {
 						if (!saveResults.processed) {
 							throw saveResults.error;
 						}
-						batchCounter++;
 					}.bind(this));
+					
+					if (notifierQueue.size) {
+						notifierQueues.push(notifierQueue);
+					}
 				}
 				catch (e) {
 					// Display nicer debug line for known errors
@@ -668,19 +670,18 @@ Zotero.Sync.Data.Local = {
 						options.onError(e);
 					}
 					
-					if (options.stopOnError) {
+					if (options.stopOnError || e.fatal) {
 						throw e;
 					}
 				}
 			}
 		}
-		catch (e) {
-			Zotero.Notifier.reset();
-			throw e;
-		}
 		finally {
-			Zotero.Notifier.commit();
+			if (notifierQueues.length) {
+				yield Zotero.Notifier.commit(notifierQueues);
 			}
+		}
+		
 		
 		//
 		// Conflict resolution
@@ -704,17 +705,15 @@ Zotero.Sync.Data.Local = {
 				Zotero.debug("Processing resolved conflicts");
 				
 				let batchSize = 50;
-				let batchCounter = 0;
+				let notifierQueues = [];
 				try {
 					for (let i = 0; i < mergeData.length; i++) {
 						// Batch notifier updates
-						if (batchCounter == 0) {
+						if (notifierQueues.length == batchSize) {
-							Zotero.Notifier.begin();
+							yield Zotero.Notifier.commit(notifierQueues);
-						}
+							notifierQueues = [];
-						else if (batchCounter == batchSize || i == json.length - 1) {
-							Zotero.Notifier.commit();
-							Zotero.Notifier.begin();
 						}
+						let notifierQueue = new Zotero.Notifier.Queue;
 						
 						let json = mergeData[i];
 						
@@ -722,6 +721,7 @@ Zotero.Sync.Data.Local = {
 						Object.assign(saveOptions, options);
 						// Tell _saveObjectFromJSON to save as unsynced
 						saveOptions.saveAsChanged = true;
+						saveOptions.notifierQueue = notifierQueue;
 						
 						// Errors have to be thrown in order to roll back the transaction, so catch
 						// those here and continue
@@ -735,7 +735,9 @@ Zotero.Sync.Data.Local = {
 									// Delete local object
 									if (json.deleted) {
 										try {
-											yield obj.erase();
+											yield obj.erase({
+												notifierQueue
+											});
 										}
 										catch (e) {
 											results.push({
@@ -784,6 +786,10 @@ Zotero.Sync.Data.Local = {
 								}
 
 							}.bind(this));
+							
+							if (notifierQueue.size) {
+								notifierQueues.push(notifierQueue);
+							}
 						}
 						catch (e) {
 							Zotero.logError(e);
@@ -798,11 +804,10 @@ Zotero.Sync.Data.Local = {
 						}
 					}
 				}
-				catch (e) {
-					Zotero.Notifier.reset();
-				}
 				finally {
-					Zotero.Notifier.commit();
+					if (notifierQueues.length) {
+						yield Zotero.Notifier.commit(notifierQueues);
+					}
 				}
 			}
 		}
@@ -1001,6 +1006,7 @@ Zotero.Sync.Data.Local = {
 				skipDateModifiedUpdate: true,
 				skipSelect: true,
 				skipCache: options.skipCache || false,
+				notifierQueue: options.notifierQueue,
 				// Errors are logged elsewhere, so skip in DataObject.save()
 				errorHandler: function (e) {
 					return;

test/tests/notifierTest.js

@@ -0,0 +1,60 @@
+"use strict";
+
+describe("Zotero.Notifier", function () {
+	describe("#trigger()", function () {
+		it("should trigger add events before modify events", function* () {
+			var deferred = Zotero.Promise.defer();
+			var events = [];
+			var observer = {
+				notify: (action, type, ids) => {
+					events.push(action);
+					if (events.length == 2) {
+						deferred.resolve();
+					}
+				}
+			};
+			var id = Zotero.Notifier.registerObserver(observer, null, 'test_trigger');
+			
+			yield Zotero.DB.executeTransaction(function* () {
+				var item = new Zotero.Item('book');
+				item.setField('title', 'A');
+				yield item.save();
+				item.setField('title', 'B');
+				yield item.save();
+				
+				Zotero.Notifier.queue('unknown', 'item', item.id);
+			});
+			
+			assert.isTrue(deferred.promise.isResolved());
+			assert.lengthOf(events, 3);
+			assert.equal(events[0], 'add');
+			assert.equal(events[1], 'modify');
+			assert.equal(events[2], 'unknown');
+			
+			Zotero.Notifier.unregisterObserver(id);
+		});
+		
+		it("should add events to passed queue", function* () {
+			var collection = yield createDataObject('collection');
+			
+			var deferred = Zotero.Promise.defer();
+			var observer = {
+				notify: () => deferred.resolve()
+			};
+			var id = Zotero.Notifier.registerObserver(observer, null, 'test_trigger');
+			
+			var queue = new Zotero.Notifier.Queue;
+			var item = createUnsavedDataObject('item');
+			item.setCollections([collection.id]);
+			yield item.saveTx({
+				notifierQueue: queue
+			});
+			assert.isTrue(deferred.promise.isPending());
+			assert.equal(queue.size, 2);
+			yield Zotero.Notifier.commit(queue);
+			assert.isTrue(deferred.promise.isResolved());
+			
+			Zotero.Notifier.unregisterObserver(id);
+		});
+	});
+});