Fix unnecessary sync looping after downloading items

An extra sync loop would be performed for every object downloaded, so a
download to an empty database could result in a huge number of
unnecessary loops. This was a regression from 52932b6eb, which started
queuing auto-syncs while a sync was in progress. The fix here is to skip
auto-sync for all objects saved from a sync download.

There are two new mechanisms involved:

- Event-level notifier options that get passed to passed to notify() at
  the top level of extraData rather than being included with every
  object (e.g., because `skipAutoSync` should apply to an entire save
  transaction)
- The ability to pass event-level notifier options when initializing
  a Zotero.Notifier.Queue, such as the one used for sync downloads

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

@@ -918,13 +918,19 @@ Zotero.DataObject.prototype.save = Zotero.Promise.coroutine(function* (options =
 		}
 		
 		env.notifierData = {};
-		// Pass along any 'notifierData' values
+		// Pass along any 'notifierData' values, which become 'extraData' in notifier events
 		if (env.options.notifierData) {
 			Object.assign(env.notifierData, env.options.notifierData);
 		}
 		if (env.options.skipSelect) {
 			env.notifierData.skipSelect = true;
 		}
+		// Pass along event-level notifier options, which become top-level extraData properties
+		for (let option of Zotero.Notifier.EVENT_LEVEL_OPTIONS) {
+			if (env.options[option]) {
+				env.notifierData[option] = true;
+			}
+		}
 		if (!env.isNew) {
 			env.changed = this._previousData;
 		}

chrome/content/zotero/xpcom/notifier.js

@@ -26,6 +26,9 @@
 "use strict";
 
 Zotero.Notifier = new function(){
+	// Options that apply to an entire event, not a specific object
+	this.EVENT_LEVEL_OPTIONS = ['autoSyncDelay', 'skipAutoSync'];
+	
 	var _observers = {};
 	var _types = [
 		'collection', 'search', 'share', 'share-items', 'item', 'file',
@@ -201,6 +204,14 @@ Zotero.Notifier = new function(){
 		
 		// Merge extraData keys
 		if (extraData) {
+			// Set event-level options as top-level properties in extraData
+			for (let option of Zotero.Notifier.EVENT_LEVEL_OPTIONS) {
+				if (extraData[option]) {
+					queue[type][event].data[option] = true;
+					delete extraData[option];
+				}
+			}
+			
 			// If just a single id, extra data can be keyed by id or passed directly
 			if (ids.length == 1) {
 				let id = ids[0];
@@ -300,11 +311,21 @@ Zotero.Notifier = new function(){
 			}
 			
 			var queue = {};
-			for (let q of queues) {
-				q = q._queue;
+			for (let { _queue: q, options } of queues) {
 				for (let type in q) {
 					for (let event in q[type]) {
-						_mergeEvent(queue, event, type, q[type][event].ids, q[type][event].data);
+						let extraData = Object.assign({}, q[type][event].data);
+						// Add options from queue as top-level options in extraData
+						if (options) {
+							for (let option in options) {
+								if (!this.EVENT_LEVEL_OPTIONS.includes(option)) {
+									throw new Error(`Queue option '${option} must be an event-level option`);
+								}
+								extraData[option] = options[option];
+							}
+						}
+						
+						_mergeEvent(queue, event, type, q[type][event].ids, extraData);
 					}
 				}
 			}
@@ -312,6 +333,7 @@ Zotero.Notifier = new function(){
 		else if (!_transactionID) {
 			throw new Error("Can't commit outside of transaction");
 		}
+		// Use default queue
 		else {
 			var queue = _queue;
 		}
@@ -419,9 +441,10 @@ Zotero.Notifier = new function(){
 }
 
 
-Zotero.Notifier.Queue = function () {
+Zotero.Notifier.Queue = function (options = {}) {
 	this.id = Zotero.Utilities.randomString();
 	Zotero.debug("Creating notifier queue " + this.id);
 	this._queue = {};
 	this.size = 0;
+	this.options = options;
 };

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

@@ -131,6 +131,19 @@ Zotero.Sync.EventListeners.AutoSyncListener = {
 			return;
 		}
 		
+		var autoSyncDelay = 0;
+		if (extraData) {
+			// Some events (e.g., writes from the server) skip auto-syncing altogether
+			if (extraData.skipAutoSync) {
+				return;
+			}
+			
+			// Use a different timeout if specified (e.g., for note editing)
+			if (extraData.autoSyncDelay) {
+				autoSyncDelay = extraData.autoSyncDelay;
+			}
+		}
+		
 		// Determine affected libraries so only those can be synced
 		let libraries = [];
 		var fileLibraries = new Set();
@@ -175,13 +188,7 @@ Zotero.Sync.EventListeners.AutoSyncListener = {
 			return;
 		}
 		
-		var autoSyncDelay = 0;
 		if (type == 'item') {
-			// Use a different timeout if specified (e.g., for note editing)
-			if (extraData[ids[0]] && extraData[ids[0]].autoSyncDelay) {
-				autoSyncDelay = Math.max(autoSyncDelay, extraData[ids[0]].autoSyncDelay);
-			}
-			
 			// Check whether file syncing or full-text syncing are necessary
 			if (event == 'add' || event == 'modify' || event == 'index') {
 				for (let id of ids) {

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

@@ -807,7 +807,9 @@ Zotero.Sync.Data.Local = {
 						batchSize = options.getNotifierBatchSize()
 					}
 				}
-				let notifierQueue = new Zotero.Notifier.Queue;
+				let notifierQueue = new Zotero.Notifier.Queue({
+					skipAutoSync: true
+				});
 				
 				let jsonObject = json[i];
 				let jsonData = jsonObject.data;

test/tests/notifierTest.js

@@ -78,4 +78,38 @@ describe("Zotero.Notifier", function () {
 			assert.isTrue(promise2.isRejected());
 		});
 	});
+	
+	describe("Queue", function () {
+		describe("#commit()", function () {
+			it("should add options from queue to extraData", async function () {
+				var called = false;
+				var data;
+				
+				var notifierID = Zotero.Notifier.registerObserver({
+					notify: (event, type, ids, extraData) => {
+						called = true;
+						data = extraData;
+					}
+				});
+				
+				var notifierQueue = new Zotero.Notifier.Queue({
+					skipAutoSync: true
+				});
+				
+				var item = createUnsavedDataObject('item');
+				await item.saveTx({
+					notifierQueue
+				});
+				
+				assert.isFalse(called);
+				
+				await Zotero.Notifier.commit(notifierQueue);
+				
+				assert.isTrue(called);
+				assert.propertyVal(data, 'skipAutoSync', true);
+				
+				Zotero.Notifier.unregisterObserver(notifierID);
+			});
+		});
+	});
 });

test/tests/syncLocalTest.js

@@ -515,6 +515,35 @@ describe("Zotero.Sync.Data.Local", function() {
 			});
 		})
 		
+		it("shouldn't trigger an auto-sync", async function () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			var item = createUnsavedDataObject('item');
+			let data = item.toJSON();
+			data.key = Zotero.DataObjectUtilities.generateKey();
+			data.version = 10;
+			let json = {
+				key: data.key,
+				version: 10,
+				data
+			};
+			
+			// Make sure the pref in question is still disabled by default during tests
+			assert.isFalse(Zotero.Prefs.get('sync.autoSync'));
+			Zotero.Prefs.set('sync.autoSync', true);
+			var stub = sinon.stub(Zotero.Sync.Runner, 'setSyncTimeout');
+			
+			await Zotero.Sync.Data.Local.processObjectsFromJSON(
+				'item', libraryID, [json], { stopOnError: true }
+			);
+			
+			// setSyncTimeout() shouldn't have been called at all
+			assert.isFalse(stub.called);
+			
+			stub.restore();
+			Zotero.Prefs.set('sync.autoSync', false);
+		});
+		
 		it("should update local version number and mark as synced if remote version is identical", function* () {
 			var libraryID = Zotero.Libraries.userLibraryID;