Add deletion uploading to API syncing [DB reupgrade]

Tags deletions are not currently synced, and maybe don't need to be.

chrome/content/zotero/xpcom/schema.js

@@ -2211,20 +2211,18 @@ Zotero.Schema = new function(){
 				
 				yield Zotero.DB.queryAsync("UPDATE syncDeleteLog SET libraryID=1 WHERE libraryID=0");
 				yield Zotero.DB.queryAsync("ALTER TABLE syncDeleteLog RENAME TO syncDeleteLogOld");
-				yield Zotero.DB.queryAsync("CREATE TABLE syncDeleteLog (\n    syncObjectTypeID INT NOT NULL,\n    libraryID INT NOT NULL,\n    key TEXT NOT NULL,\n    dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n    synced INT NOT NULL DEFAULT 0,\n    UNIQUE (syncObjectTypeID, libraryID, key),\n    FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID),\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)");
+				yield Zotero.DB.queryAsync("CREATE TABLE syncDeleteLog (\n    syncObjectTypeID INT NOT NULL,\n    libraryID INT NOT NULL,\n    key TEXT NOT NULL,\n    dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n    UNIQUE (syncObjectTypeID, libraryID, key),\n    FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID),\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)");
-				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO syncDeleteLog SELECT syncObjectTypeID, libraryID, key, timestamp, 0 FROM syncDeleteLogOld");
+				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO syncDeleteLog SELECT syncObjectTypeID, libraryID, key, timestamp FROM syncDeleteLogOld");
 				yield Zotero.DB.queryAsync("DROP INDEX IF EXISTS syncDeleteLog_timestamp");
-				yield Zotero.DB.queryAsync("CREATE INDEX syncDeleteLog_synced ON syncDeleteLog(synced)");
 				// TODO: Something special for tag deletions?
 				//yield Zotero.DB.queryAsync("DELETE FROM syncDeleteLog WHERE syncObjectTypeID IN (2, 5, 6)");
 				//yield Zotero.DB.queryAsync("DELETE FROM syncObjectTypes WHERE syncObjectTypeID IN (2, 5, 6)");
 				
 				yield Zotero.DB.queryAsync("UPDATE storageDeleteLog SET libraryID=1 WHERE libraryID=0");
 				yield Zotero.DB.queryAsync("ALTER TABLE storageDeleteLog RENAME TO storageDeleteLogOld");
-				yield Zotero.DB.queryAsync("CREATE TABLE storageDeleteLog (\n    libraryID INT NOT NULL,\n    key TEXT NOT NULL,\n    dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n    synced INT NOT NULL DEFAULT 0,\n    PRIMARY KEY (libraryID, key),\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)");
+				yield Zotero.DB.queryAsync("CREATE TABLE storageDeleteLog (\n    libraryID INT NOT NULL,\n    key TEXT NOT NULL,\n    dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n    PRIMARY KEY (libraryID, key),\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)");
-				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO storageDeleteLog SELECT libraryID, key, timestamp, 0 FROM storageDeleteLogOld");
+				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO storageDeleteLog SELECT libraryID, key, timestamp FROM storageDeleteLogOld");
 				yield Zotero.DB.queryAsync("DROP INDEX IF EXISTS storageDeleteLog_timestamp");
-				yield Zotero.DB.queryAsync("CREATE INDEX storageDeleteLog_synced ON storageDeleteLog(synced)");
 				
 				yield Zotero.DB.queryAsync("ALTER TABLE annotations RENAME TO annotationsOld");
 				yield Zotero.DB.queryAsync("CREATE TABLE annotations (\n    annotationID INTEGER PRIMARY KEY,\n    itemID INT NOT NULL,\n    parent TEXT,\n    textNode INT,\n    offset INT,\n    x INT,\n    y INT,\n    cols INT,\n    rows INT,\n    text TEXT,\n    collapsed BOOL,\n    dateModified DATE,\n    FOREIGN KEY (itemID) REFERENCES itemAttachments(itemID) ON DELETE CASCADE\n)");

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

@@ -119,7 +119,7 @@ Zotero.Sync.APIClient.prototype = {
 	
 	
 	/**
-	 * @return {Object|false} - An object with 'libraryVersion' and a 'deleted' array, or
+	 * @return {Object|false} - An object with 'libraryVersion' and a 'deleted' object, or
 	 *     false if 'since' is earlier than the beginning of the delete log
 	 */
 	getDeleted: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, since) {
@@ -277,7 +277,7 @@ Zotero.Sync.APIClient.prototype = {
 	},
 	
 	
-	uploadObjects: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, objectType, method, version, objects) {
+	uploadObjects: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, method, libraryVersion, objectType, objects) {
 		if (method != 'POST' && method != 'PATCH') {
 			throw new Error("Invalid method '" + method + "'");
 		}
@@ -287,7 +287,7 @@ Zotero.Sync.APIClient.prototype = {
 		Zotero.debug("Uploading " + objects.length + " "
 			+ (objects.length == 1 ? objectType : objectTypePlural));
 		
-		Zotero.debug("Sending If-Unmodified-Since-Version: " + version);
+		Zotero.debug("Sending If-Unmodified-Since-Version: " + libraryVersion);
 		
 		var json = JSON.stringify(objects);
 		var params = {
@@ -299,7 +299,7 @@ Zotero.Sync.APIClient.prototype = {
 		
 		var xmlhttp = yield this.makeRequest(method, uri, {
 			headers: {
-				"If-Unmodified-Since-Version": version
+				"If-Unmodified-Since-Version": libraryVersion
 			},
 			body: json,
 			successCodes: [200, 412]
@@ -309,17 +309,57 @@ Zotero.Sync.APIClient.prototype = {
 			Zotero.debug("Server returned 412: " + xmlhttp.responseText, 2);
 			throw new Zotero.HTTP.UnexpectedStatusException(xmlhttp);
 		}
-		var libraryVersion = xmlhttp.getResponseHeader('Last-Modified-Version');
+		libraryVersion = xmlhttp.getResponseHeader('Last-Modified-Version');
 		if (!libraryVersion) {
 			throw new Error("Last-Modified-Version not provided");
 		}
 		return {
-			libraryVersion: libraryVersion,
+			libraryVersion,
 			results: this._parseJSON(xmlhttp.responseText)
 		};
 	}),
 	
 	
+	uploadDeletions: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, libraryVersion, objectType, keys) {
+		var objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
+		
+		Zotero.debug(`Uploading ${keys.length} ${objectType} deletion`
+			+ (keys.length == 1 ? '' : 's'));
+		
+		Zotero.debug("Sending If-Unmodified-Since-Version: " + libraryVersion);
+		
+		var params = {
+			target: objectTypePlural,
+			libraryType: libraryType,
+			libraryTypeID: libraryTypeID
+		};
+		if (objectType == 'tag') {
+			params.tags = keys.join("||");
+		}
+		else {
+			params[objectType + "Key"] = keys.join(",");
+		}
+		var uri = this.buildRequestURI(params);
+		
+		var xmlhttp = yield this.makeRequest("DELETE", uri, {
+			headers: {
+				"If-Unmodified-Since-Version": libraryVersion
+			},
+			successCodes: [204]
+		});
+		// Avoid logging error from Zotero.HTTP.request() in ConcurrentCaller
+		if (xmlhttp.status == 412) {
+			Zotero.debug("Server returned 412: " + xmlhttp.responseText, 2);
+			throw new Zotero.HTTP.UnexpectedStatusException(xmlhttp);
+		}
+		libraryVersion = xmlhttp.getResponseHeader('Last-Modified-Version');
+		if (!libraryVersion) {
+			throw new Error("Last-Modified-Version not provided");
+		}
+		return libraryVersion;
+	}),
+	
+	
 	buildRequestURI: function (params) {
 		var uri = this.baseURL;
 		
@@ -354,6 +394,7 @@ Zotero.Sync.APIClient.prototype = {
 			'itemKey',
 			'collectionKey',
 			'searchKey',
+			'tag',
 			'linkMode',
 			'start',
 			'limit',

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

@@ -67,6 +67,7 @@ Zotero.Sync.Data.Engine = function (options) {
 	this.stopOnError = options.stopOnError;
 	this.requests = [];
 	this.uploadBatchSize = 25;
+	this.uploadDeletionBatchSize = 50;
 	
 	this.failed = false;
 	
@@ -567,21 +568,39 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi
 	
 	var uploadNeeded = false;
 	var objectIDs = {};
+	var objectDeletions = {};
 	
 	// Get unsynced local objects for each object type
 	for (let objectType of Zotero.DataObjectUtilities.getTypesForLibrary(this.libraryID)) {
 		let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
 		
-		let ids = yield Zotero.Sync.Data.Local.getUnsynced(this.libraryID, objectType);
+		// New/modified objects
-		if (!ids.length) {
+		let ids = yield Zotero.Sync.Data.Local.getUnsynced(objectType, this.libraryID);
-			Zotero.debug("No " + objectTypePlural + " to upload in " + this.libraryName);
+		if (ids.length) {
-			continue;
+			Zotero.debug(ids.length + " "
+				+ (ids.length == 1 ? objectType : objectTypePlural)
+				+ " to upload in library " + this.libraryID);
+			objectIDs[objectType] = ids;
+		}
+		else {
+			Zotero.debug("No " + objectTypePlural + " to upload in " + this.libraryName);
+		}
+		
+		// Deleted objects
+		let keys = yield Zotero.Sync.Data.Local.getDeleted(objectType, this.libraryID);
+		if (keys.length) {
+			Zotero.debug(`${keys.length} ${objectType} deletion`
+				+ (keys.length == 1 ? '' : 's')
+				+ ` to upload in ${this.libraryName}`);
+			objectDeletions[objectType] = keys;
+		}
+		else {
+			Zotero.debug(`No ${objectType} deletions to upload in ${this.libraryName}`);
+		}
+		
+		if (ids.length || keys.length) {
+			uploadNeeded = true;
 		}
-		Zotero.debug(ids.length + " "
-			+ (ids.length == 1 ? objectType : objectTypePlural)
-			+ " to upload in library " + this.libraryID);
-		objectIDs[objectType] = ids;
-		uploadNeeded = true;
 	}
 	
 	if (!uploadNeeded) {
@@ -589,192 +608,271 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi
 	}
 	
 	Zotero.debug(JSON.stringify(objectIDs));
-	
 	for (let objectType in objectIDs) {
-		let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
+		libraryVersion = yield this._uploadObjects(
-		let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
+			objectType, objectIDs[objectType], libraryVersion
+		);
+	}
 	
-		let queue = [];
+	Zotero.debug(JSON.stringify(objectDeletions));
-		for (let id of objectIDs[objectType]) {
+	for (let objectType in objectDeletions) {
-			queue.push({
+		libraryVersion = yield this._uploadDeletions(
-				id: id,
+			objectType, objectDeletions[objectType], libraryVersion
-				json: null,
+		);
-				tries: 0,
+	}
-				failed: false
+	
-			});
+	return this.UPLOAD_RESULT_SUCCESS;
+});
+
+
+Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(function* (objectType, ids, libraryVersion) {
+	let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
+	let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
+	
+	let queue = [];
+	for (let id of ids) {
+		queue.push({
+			id: id,
+			json: null,
+			tries: 0,
+			failed: false
+		});
+	}
+	
+	let failureDelayGenerator = null;
+	
+	while (queue.length) {
+		// Get a slice of the queue and generate JSON for objects if necessary
+		let batch = [];
+		let numSkipped = 0;
+		for (let i = 0; i < queue.length && queue.length < this.uploadBatchSize; i++) {
+			let o = queue[i];
+			// Skip requests that failed with 4xx
+			if (o.failed) {
+				numSkipped++;
+				continue;
+			}
+			if (!o.json) {
+				o.json = yield this._getJSONForObject(objectType, o.id);
+			}
+			batch.push(o.json);
 		}
 		
-		let failureDelayGenerator = null;
+		// No more non-failed requests
+		if (!batch.length) {
+			break;
+		}
 		
-		while (queue.length) {
+		// Remove selected and skipped objects from queue
-			// Get a slice of the queue and generate JSON for objects if necessary
+		queue.splice(0, batch.length + numSkipped);
-			let batch = [];
+		
-			for (let i = 0; i < queue.length && queue.length < this.uploadBatchSize; i++) {
+		Zotero.debug("UPLOAD BATCH:");
-				let o = queue[i];
+		Zotero.debug(batch);
-				// Skip requests that failed with 4xx
+		
-				if (o.failed) {
+		let numSuccessful = 0;
+		try {
+			let json = yield this.apiClient.uploadObjects(
+				this.libraryType,
+				this.libraryTypeID,
+				"POST",
+				libraryVersion,
+				objectType,
+				batch
+			);
+			
+			Zotero.debug('======');
+			Zotero.debug(json);
+			
+			libraryVersion = json.libraryVersion;
+			
+			// Mark successful and unchanged objects as synced with new version,
+			// and save uploaded JSON to cache
+			let ids = [];
+			let toSave = [];
+			let toCache = [];
+			for (let state of ['successful', 'unchanged']) {
+				for (let index in json.results[state]) {
+					let current = json.results[state][index];
+					// 'successful' includes objects, not keys
+					let key = state == 'successful' ? current.key : current;
+					
+					if (key != batch[index].key) {
+						throw new Error("Key mismatch (" + key + " != " + batch[index].key + ")");
+					}
+					
+					let obj = yield objectsClass.getByLibraryAndKeyAsync(
+						this.libraryID, key, { noCache: true }
+					)
+					ids.push(obj.id);
+					
+					if (state == 'successful') {
+						// Update local object with saved data if necessary
+						yield obj.loadAllData();
+						obj.fromJSON(current.data);
+						toSave.push(obj);
+						toCache.push(current);
+					}
+					else {
+						let j = yield obj.toJSON();
+						j.version = json.libraryVersion;
+						toCache.push(j);
+					}
+					
+					numSuccessful++;
+					// Remove from batch to mark as successful
+					delete batch[index];
+				}
+			}
+			yield Zotero.Sync.Data.Local.saveCacheObjects(
+				objectType, this.libraryID, toCache
+			);
+			yield Zotero.DB.executeTransaction(function* () {
+				for (let i = 0; i < toSave.length; i++) {
+					yield toSave[i].save();
+				}
+				this.library.libraryVersion = json.libraryVersion;
+				yield this.library.save();
+				objectsClass.updateVersion(ids, json.libraryVersion);
+				objectsClass.updateSynced(ids, true);
+			}.bind(this));
+			
+			// Handle failed objects
+			for (let index in json.results.failed) {
+				let { code, message } = json.results.failed[index];
+				e = new Error(message);
+				e.name = "ZoteroUploadObjectError";
+				e.code = code;
+				Zotero.logError(e);
+				
+				// This shouldn't happen, because the upload request includes a library
+				// version and should prevent an outdated upload before the object version is
+				// checked. If it does, we need to do a full sync.
+				if (e.code == 412) {
+					return this.UPLOAD_RESULT_OBJECT_CONFLICT;
+				}
+				
+				if (this.onError) {
+					this.onError(e);
+				}
+				if (this.stopOnError) {
+					throw new Error(e);
+				}
+				batch[index].tries++;
+				// Mark 400 errors as permanently failed
+				if (e.code >= 400 && e.code < 500) {
+					batch[index].failed = true;
+				}
+				// 500 errors should stay in queue and be retried
+			}
+			
+			// Add failed objects back to end of queue
+			var numFailed = 0;
+			for (let o of batch) {
+				if (o !== undefined) {
+					queue.push(o);
+					// TODO: Clear JSON?
+					numFailed++;
+				}
+			}
+			Zotero.debug("Failed: " + numFailed, 2);
+		}
+		catch (e) {
+			if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
+				if (e.status == 412) {
+					return this.UPLOAD_RESULT_LIBRARY_CONFLICT;
+				}
+				
+				// On 5xx, delay and retry
+				if (e.status >= 500 && e.status <= 600) {
+					if (!failureDelayGenerator) {
+						// Keep trying for up to an hour
+						failureDelayGenerator = Zotero.Utilities.Internal.delayGenerator(
+							Zotero.Sync.Data.failureDelayIntervals, 60 * 60 * 1000
+						);
+					}
+					let keepGoing = yield failureDelayGenerator.next();
+					if (!keepGoing) {
+						Zotero.logError("Failed too many times");
+						throw e;
+					}
 					continue;
 				}
-				if (!o.json) {
-					o.json = yield this._getJSONForObject(objectType, o.id);
-				}
-				batch.push(o.json);
 			}
+			throw e;
+		}
+		// If we didn't make any progress, bail
+		if (!numSuccessful) {
+			throw new Error("Made no progress during upload -- stopping");
+		}
+	}
+	Zotero.debug("Done uploading " + objectTypePlural + " in library " + this.libraryID);
 	
-			// No more non-failed requests
+	return libraryVersion;
-			if (!batch.length) {
+})
-				break;
-			}
 
-			// Remove selected and skipped objects from queue
-			queue.splice(0, batch.length);
 
-			Zotero.debug("UPLOAD BATCH:");
+Zotero.Sync.Data.Engine.prototype._uploadDeletions = Zotero.Promise.coroutine(function* (objectType, keys, libraryVersion) {
-			Zotero.debug(batch);
+	let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
+	let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
 	
-			let numSuccessful = 0;
+	let failureDelayGenerator = null;
-			try {
-				let json = yield this.apiClient.uploadObjects(
-					this.libraryType,
-					this.libraryTypeID,
-					objectType,
-					"POST",
-					libraryVersion,
-					batch
-				);
 	
-				Zotero.debug('======');
+	while (keys.length) {
-				Zotero.debug(json);
+		try {
+			let batch = keys.slice(0, this.uploadDeletionBatchSize);
+			libraryVersion = yield this.apiClient.uploadDeletions(
+				this.libraryType,
+				this.libraryTypeID,
+				libraryVersion,
+				objectType,
+				batch
+			);
+			keys.splice(0, batch.length);
 			
-				libraryVersion = json.libraryVersion;
+			// Update library version
+			this.library.libraryVersion = libraryVersion;
+			yield this.library.saveTx();
 			
-				// Mark successful and unchanged objects as synced with new version,
+			// Remove successful deletions from delete log
-				// and save uploaded JSON to cache
+			yield Zotero.Sync.Data.Local.removeObjectsFromDeleteLog(
-				let ids = [];
+				objectType, this.libraryID, batch
-				let toSave = [];
+			);
-				let toCache = [];
+		}
-				for (let state of ['successful', 'unchanged']) {
+		catch (e) {
-					for (let index in json.results[state]) {
+			if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
-						let current = json.results[state][index];
+				if (e.status == 412) {
-						// 'successful' includes objects, not keys
+					return this.UPLOAD_RESULT_LIBRARY_CONFLICT;
-						let key = state == 'successful' ? current.key : current;
-						
-						if (key != batch[index].key) {
-							throw new Error("Key mismatch (" + key + " != " + batch[index].key + ")");
-						}
-						
-						let obj = yield objectsClass.getByLibraryAndKeyAsync(
-							this.libraryID, key, { noCache: true }
-						)
-						ids.push(obj.id);
-						
-						if (state == 'successful') {
-							// Update local object with saved data if necessary
-							yield obj.loadAllData();
-							obj.fromJSON(current.data);
-							toSave.push(obj);
-							toCache.push(current);
-						}
-						else {
-							let j = yield obj.toJSON();
-							j.version = json.libraryVersion;
-							toCache.push(j);
-						}
-						
-						numSuccessful++;
-						// Remove from batch to mark as successful
-						delete batch[index];
-					}
 				}
-				yield Zotero.Sync.Data.Local.saveCacheObjects(
-					objectType, this.libraryID, toCache
-				);
-				yield Zotero.DB.executeTransaction(function* () {
-					for (let i = 0; i < toSave.length; i++) {
-						yield toSave[i].save();
-					}
-					this.library.libraryVersion = json.libraryVersion;
-					yield this.library.save();
-					objectsClass.updateVersion(ids, json.libraryVersion);
-					objectsClass.updateSynced(ids, true);
-				}.bind(this));
-				
-				// Handle failed objects
-				for (let index in json.results.failed) {
-					let { code, message } = json.results.failed[index];
-					e = new Error(message);
-					e.name = "ZoteroUploadObjectError";
-					e.code = code;
-					Zotero.logError(e);
-					
-					// This shouldn't happen, because the upload request includes a library
-					// version and should prevent an outdated upload before the object version is
-					// checked. If it does, we need to do a full sync.
-					if (e.code == 412) {
-						return this.UPLOAD_RESULT_OBJECT_CONFLICT;
-					}
 				
+				// On 5xx, delay and retry
+				if (e.status >= 500 && e.status <= 600) {
 					if (this.onError) {
 						this.onError(e);
 					}
 					if (this.stopOnError) {
 						throw new Error(e);
 					}
-					batch[index].tries++;
-					// Mark 400 errors as permanently failed
-					if (e.code >= 400 && e.code < 500) {
-						batch[index].failed = true;
-					}
-					// 500 errors should stay in queue and be retried
-				}
 					
-				// Add failed objects back to end of queue
+					if (!failureDelayGenerator) {
-				Zotero.debug("ADDING BACK FAILED");
+						// Keep trying for up to an hour
-				Zotero.debug(batch);
+						failureDelayGenerator = Zotero.Utilities.Internal.delayGenerator(
-				var numFailed = 0;
+							Zotero.Sync.Data.failureDelayIntervals, 60 * 60 * 1000
-				for (let o of batch) {
+						);
-					if (o !== undefined) {
-						queue.push(o);
-						// TODO: Clear JSON?
-						numFailed++;
 					}
+					let keepGoing = yield failureDelayGenerator.next();
+					if (!keepGoing) {
+						Zotero.logError("Failed too many times");
+						throw e;
+					}
+					continue;
 				}
-				Zotero.debug(queue);
-				Zotero.debug("Failed: " + numFailed, 2);
-			}
-			catch (e) {
-				if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
-					if (e.status == 412) {
-						return this.UPLOAD_RESULT_LIBRARY_CONFLICT;
-					}
-					
-					// On 5xx, delay and retry
-					if (e.status >= 500 && e.status <= 600) {
-						if (!failureDelayGenerator) {
-							// Keep trying for up to an hour
-							failureDelayGenerator = Zotero.Utilities.Internal.delayGenerator(
-								Zotero.Sync.Data.failureDelayIntervals, 60 * 60 * 1000
-							);
-						}
-						let keepGoing = yield failureDelayGenerator.next();
-						if (!keepGoing) {
-							Zotero.logError("Failed too many times");
-							throw e;
-						}
-						continue;
-					}
-				}
-				throw e;
-			}
-			// If we didn't make any progress, bail
-			if (!numSuccessful) {
-				throw new Error("Made no progress during upload -- stopping");
 			}
+			throw e;
 		}
-		Zotero.debug("Done uploading " + objectTypePlural + " in library " + this.libraryID);
 	}
+	Zotero.debug(`Done uploading ${objectType} deletions in ${this.libraryName}`);
 	
-	return this.UPLOAD_RESULT_SUCCESS;
+	return libraryVersion;
 });
 
 
@@ -1037,7 +1135,7 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function*
 			let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
 			let toDownload = [];
 			let cacheVersions = yield Zotero.Sync.Data.Local.getLatestCacheObjectVersions(
-				this.libraryID, objectType
+				objectType, this.libraryID
 			);
 			// Queue objects that are out of date or don't exist locally
 			for (let key in results.versions) {
@@ -1082,7 +1180,7 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function*
 			}
 			
 			// Mark synced objects that don't exist remotely as unsynced
-			let syncedKeys = yield Zotero.Sync.Data.Local.getSynced(this.libraryID, objectType);
+			let syncedKeys = yield Zotero.Sync.Data.Local.getSynced(objectType, this.libraryID);
 			let remoteMissing = Zotero.Utilities.arrayDiff(syncedKeys, Object.keys(results.versions));
 			if (remoteMissing.length) {
 				Zotero.debug("Checking remotely missing synced " + objectTypePlural);

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

@@ -37,9 +37,9 @@ Zotero.Sync.EventListeners.ChangeListener = new function () {
 			return;
 		}
 		
-		var syncSQL = "REPLACE INTO syncDeleteLog (syncObjectTypeID, libraryID, key, synced) "
+		var syncSQL = "REPLACE INTO syncDeleteLog (syncObjectTypeID, libraryID, key) "
-			+ "VALUES (?, ?, ?, 0)";
+			+ "VALUES (?, ?, ?)";
-		var storageSQL = "REPLACE INTO storageDeleteLog VALUES (?, ?, 0)";
+		var storageSQL = "REPLACE INTO storageDeleteLog (libraryID, key) VALUES (?, ?)";
 		
 		var storageForLibrary = {};
 		

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

@@ -134,10 +134,11 @@ Zotero.Sync.Data.Local = {
 	
 	
 	/**
+	 * @param {String} objectType
 	 * @param {Integer} libraryID
 	 * @return {Promise<String[]>} - A promise for an array of object keys
 	 */
-	getSynced: function (libraryID, objectType) {
+	getSynced: function (objectType, libraryID) {
 		var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
 		var sql = "SELECT key FROM " + objectsClass.table + " WHERE libraryID=? AND synced=1";
 		return Zotero.DB.columnQueryAsync(sql, [libraryID]);
@@ -145,10 +146,11 @@ Zotero.Sync.Data.Local = {
 	
 	
 	/**
+	 * @param {String} objectType
 	 * @param {Integer} libraryID
 	 * @return {Promise<Integer[]>} - A promise for an array of object ids
 	 */
-	getUnsynced: Zotero.Promise.coroutine(function* (libraryID, objectType) {
+	getUnsynced: Zotero.Promise.coroutine(function* (objectType, libraryID) {
 		var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
 		var sql = "SELECT " + objectsClass.idColumn + " FROM " + objectsClass.table
 			+ " WHERE libraryID=? AND synced=0";
@@ -170,7 +172,7 @@ Zotero.Sync.Data.Local = {
 	 * @return {Promise<Object>} - A promise for an object with object keys as keys and versions
 	 *                             as properties
 	 */
-	getLatestCacheObjectVersions: Zotero.Promise.coroutine(function* (libraryID, objectType) {
+	getLatestCacheObjectVersions: Zotero.Promise.coroutine(function* (objectType, libraryID) {
 		var sql = "SELECT key, version FROM syncCache WHERE libraryID=? AND "
 			+ "syncObjectTypeID IN (SELECT syncObjectTypeID FROM "
 			+ "syncObjectTypes WHERE name=?) ORDER BY version";
@@ -494,10 +496,10 @@ Zotero.Sync.Data.Local = {
 								// Auto-restore some locally deleted objects that have changed remotely
 								case 'collection':
 								case 'search':
-									yield this._removeObjectFromDeleteLog(
+									yield this.removeObjectsFromDeleteLog(
 										objectType,
 										libraryID,
-										objectKey
+										[objectKey]
 									);
 									
 									throw new Error("Unimplemented");
@@ -1014,12 +1016,33 @@ Zotero.Sync.Data.Local = {
 	}),
 	
 	
+	/**
+	 * @return {Promise<String[]>} - Promise for array of keys
+	 */
+	getDeleted: Zotero.Promise.coroutine(function* (objectType, libraryID) {
+		var syncObjectTypeID = Zotero.Sync.Data.Utilities.getSyncObjectTypeID(objectType);
+		var sql = "SELECT key FROM syncDeleteLog WHERE libraryID=? AND syncObjectTypeID=?";
+		return Zotero.DB.columnQueryAsync(sql, [libraryID, syncObjectTypeID]);
+	}),
+	
+	
 	/**
 	 * @return {Promise}
 	 */
-	_removeObjectFromDeleteLog: function (objectType, libraryID, key) {
+	removeObjectsFromDeleteLog: function (objectType, libraryID, keys) {
 		var syncObjectTypeID = Zotero.Sync.Data.Utilities.getSyncObjectTypeID(objectType);
-		var sql = "DELETE FROM syncDeleteLog WHERE libraryID=? AND key=? AND syncObjectTypeID=?";
+		var sql = "DELETE FROM syncDeleteLog WHERE libraryID=? AND syncObjectTypeID=? AND key IN (";
-		return Zotero.DB.queryAsync(sql, [libraryID, key, syncObjectTypeID]);
+		return Zotero.DB.executeTransaction(function* () {
+			return Zotero.Utilities.Internal.forEachChunkAsync(
+				keys,
+				Zotero.DB.MAX_BOUND_PARAMETERS - 2,
+				Zotero.Promise.coroutine(function* (chunk) {
+					var params = [libraryID, syncObjectTypeID].concat(chunk);
+					return Zotero.DB.queryAsync(
+						sql + Array(chunk.length).fill('?').join(',') + ")", params
+					);
+				})
+			);
+		}.bind(this));
 	}
 }

resource/schema/userdata.sql

@@ -330,22 +330,18 @@ CREATE TABLE syncDeleteLog (
     libraryID INT NOT NULL,
     key TEXT NOT NULL,
     dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
-    synced INT NOT NULL DEFAULT 0,
     UNIQUE (syncObjectTypeID, libraryID, key),
     FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID),
     FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE
 );
-CREATE INDEX syncDeleteLog_synced ON syncDeleteLog(synced);
 
 CREATE TABLE storageDeleteLog (
     libraryID INT NOT NULL,
     key TEXT NOT NULL,
     dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
-    synced INT NOT NULL DEFAULT 0,
     PRIMARY KEY (libraryID, key),
     FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE
 );
-CREATE INDEX storageDeleteLog_synced ON storageDeleteLog(synced);
 
 CREATE TABLE annotations (
     annotationID INTEGER PRIMARY KEY,

test/tests/syncEngineTest.js

@@ -310,7 +310,7 @@ describe("Zotero.Sync.Data.Engine", function () {
 			assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion);
 			for (let type of types) {
 				// Make sure objects were set to the correct version and marked as synced
-				assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0);
+				assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(type, libraryID)), 0);
 				let key = objects[type][0].key;
 				let version = objects[type][0].version;
 				assert.equal(version, objectVersions[type][key]);
@@ -391,7 +391,7 @@ describe("Zotero.Sync.Data.Engine", function () {
 			assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion);
 			for (let type of types) {
 				// Make sure objects were set to the correct version and marked as synced
-				assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0);
+				assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(type, libraryID)), 0);
 				let o = objects[type][0];
 				let key = o.key;
 				let version = o.version;
@@ -475,7 +475,7 @@ describe("Zotero.Sync.Data.Engine", function () {
 			assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion);
 			for (let type of types) {
 				// Make sure local objects were updated with new metadata and marked as synced
-				assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0);
+				assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(type, libraryID)), 0);
 				let o = objects[type][0];
 				let key = o.key;
 				let version = o.version;
@@ -490,6 +490,66 @@ describe("Zotero.Sync.Data.Engine", function () {
 			}
 		})
 		
+		it("should upload local deletions", function* () {
+			var { engine, client, caller } = yield setup();
+			var libraryID = Zotero.Libraries.userLibraryID;
+			var lastLibraryVersion = 5;
+			yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion);
+			
+			var types = Zotero.DataObjectUtilities.getTypes();
+			var objects = {};
+			for (let type of types) {
+				let obj1 = yield createDataObject(type);
+				let obj2 = yield createDataObject(type);
+				objects[type] = [obj1.key, obj2.key];
+				yield obj1.eraseTx();
+				yield obj2.eraseTx();
+			}
+			
+			var count = types.length;
+			
+			server.respond(function (req) {
+				if (req.method == "DELETE") {
+					assert.equal(
+						req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion
+					);
+					
+					// TODO: Settings?
+					
+					// Data objects
+					for (let type of types) {
+						let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type);
+						if (req.url.startsWith(baseURL + "users/1/" + typePlural)) {
+							let matches = req.url.match(new RegExp("\\?" + type + "Key=(.+)"));
+							let keys = decodeURIComponent(matches[1]).split(',');
+							assert.sameMembers(keys, objects[type]);
+							req.respond(
+								204,
+								{
+									"Last-Modified-Version": ++lastLibraryVersion
+								}
+							);
+							count--;
+							return;
+						}
+					}
+				}
+			})
+			
+			yield engine.start();
+			
+			assert.equal(count, 0);
+			for (let type of types) {
+				yield assert.eventually.lengthOf(
+					Zotero.Sync.Data.Local.getDeleted(type, libraryID), 0
+				);
+			}
+			assert.equal(
+				Zotero.Libraries.get(libraryID).libraryVersion,
+				lastLibraryVersion
+			);
+		})
+		
 		it("should make only one request if in sync", function* () {
 			yield Zotero.Libraries.setVersion(Zotero.Libraries.userLibraryID, 5);
 			({ engine, client, caller } = yield setup());
@@ -911,10 +971,10 @@ describe("Zotero.Sync.Data.Engine", function () {
 			// Objects 1 should be marked as synced, with versions from the server
 			// Objects 2 should be marked as unsynced
 			for (let type of types) {
-				var synced = yield Zotero.Sync.Data.Local.getSynced(userLibraryID, type);
+				var synced = yield Zotero.Sync.Data.Local.getSynced(type, userLibraryID);
 				assert.deepEqual(synced, [objects[type][0].key]);
 				assert.equal(objects[type][0].version, 10);
-				var unsynced = yield Zotero.Sync.Data.Local.getUnsynced(userLibraryID, type);
+				var unsynced = yield Zotero.Sync.Data.Local.getUnsynced(type, userLibraryID);
 				assert.deepEqual(unsynced, [objects[type][1].id]);
 				
 				assert.equal(versionResults[type].libraryVersion, headers["Last-Modified-Version"]);