Delete older versions of processed objects in cache

And recover from "Sync cache had later version than remote" error, which
shouldn't actually happen...

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

@@ -1193,7 +1193,9 @@ Zotero.DataObject.prototype._initErase = Zotero.Promise.method(function (env) {
 
 Zotero.DataObject.prototype._finalizeErase = Zotero.Promise.coroutine(function* (env) {
 	// Delete versions from sync cache
-	yield Zotero.Sync.Data.Local.deleteCacheObject(this.objectType, this._libraryID, this._key);
+	yield Zotero.Sync.Data.Local.deleteCacheObjectVersions(
+		this.objectType, this._libraryID, this._key
+	);
 	
 	Zotero.DB.addCurrentCallback("commit", function () {
 		this.ObjectsClass.unload(env.deletedObjectIDs || this.id);

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

@@ -1054,10 +1054,14 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function*
 				if (cacheVersion == version) {
 					continue;
 				}
+				// This should never happen, but recover if it does
 				if (cacheVersion > version) {
-					throw new Error("Sync cache had later version than remote for "
-						+ objectType + + this.libraryID + "/" + key
-						+ "(" + cacheVersion + " > " + version + ")");
+					Zotero.logError("Sync cache had later version than remote for "
+						+ objectType + " " + this.libraryID + "/" + key
+						+ " (" + cacheVersion + " > " + version + ") -- deleting");
+					yield Zotero.Sync.Data.Local.deleteCacheObjectVersions(
+						objectType, this.libraryID, key, cacheVersion, cacheVersion
+					);
 				}
 				
 				if (obj) {

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

@@ -521,11 +521,18 @@ Zotero.Sync.Data.Local = {
 						let saved = yield this._saveObjectFromJSON(
 							obj, jsonData, options, { skipCache }
 						);
-						// Mark updated attachments for download
-						if (saved && objectType == 'item' && obj.isImportedAttachment()) {
-							yield this._checkAttachmentForDownload(
-								obj, jsonData.mtime, isNewObject
+						if (saved) {
+							// Delete older versions of the item in the cache
+							yield this.deleteCacheObjectVersions(
+								objectType, libraryID, jsonData.key, null, jsonData.version - 1
 							);
+							
+							// Mark updated attachments for download
+							if (objectType == 'item' && obj.isImportedAttachment()) {
+								yield this._checkAttachmentForDownload(
+									obj, jsonData.mtime, isNewObject
+								);
+							}
 						}
 						
 						if (saved) {
@@ -647,11 +654,35 @@ Zotero.Sync.Data.Local = {
 	}),
 	
 	
-	deleteCacheObject: function (objectType, libraryID, key) {
+	/**
+	 * Delete one or more versions of an object from the sync cache
+	 *
+	 * @param {String} objectType
+	 * @param {Integer} libraryID
+	 * @param {String} key
+	 * @param {Integer} [minVersion]
+	 * @param {Integer} [maxVersion]
+	 */
+	deleteCacheObjectVersions: function (objectType, libraryID, key, minVersion, maxVersion) {
 		var sql = "DELETE FROM syncCache WHERE libraryID=? AND key=? "
 			+ "AND syncObjectTypeID IN (SELECT syncObjectTypeID FROM "
-			+ "syncObjectTypes WHERE name=?)";;
-		return Zotero.DB.queryAsync(sql, [libraryID, key, objectType]);
+			+ "syncObjectTypes WHERE name=?)";
+		var params = [libraryID, key, objectType];
+		if (minVersion && minVersion == maxVersion) {
+			sql += " AND version=?";
+			params.push(minVersion);
+		}
+		else {
+			if (minVersion) {
+				sql += " AND version>=?";
+				params.push(minVersion);
+			}
+			if (maxVersion) {
+				sql += " AND version<=?";
+				params.push(maxVersion);
+			}
+		}
+		return Zotero.DB.queryAsync(sql, params);
 	},
 	
 	

test/tests/syncLocalTest.js

@@ -34,7 +34,6 @@ describe("Zotero.Sync.Data.Local", function() {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			
 			var type = 'item';
-			let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
 			let obj = yield createDataObject(type, { version: 5 });
 			let data = yield obj.toJSON();
 			yield Zotero.Sync.Data.Local.saveCacheObjects(
@@ -66,6 +65,43 @@ describe("Zotero.Sync.Data.Local", function() {
 			assert.equal(obj.getField('place'), changedPlace);
 		})
 		
+		it("should delete older versions in sync cache after processing", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			for (let type of types) {
+				let obj = yield createDataObject(type, { version: 5 });
+				let data = yield obj.toJSON();
+				yield Zotero.Sync.Data.Local.saveCacheObjects(
+					type, libraryID, [data]
+				);
+			}
+			
+			for (let type of types) {
+				let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
+				
+				let obj = yield createDataObject(type, { version: 10 });
+				let data = yield obj.toJSON();
+				yield Zotero.Sync.Data.Local.saveCacheObjects(
+					type, libraryID, [data]
+				);
+				yield Zotero.Sync.Data.Local.processSyncCacheForObjectType(
+					libraryID, type, { stopOnError: true }
+				);
+				
+				let localObj = objectsClass.getByLibraryAndKey(libraryID, obj.key);
+				assert.equal(localObj.version, 10);
+				
+				let versions = yield Zotero.Sync.Data.Local.getCacheObjectVersions(
+					type, libraryID, obj.key
+				);
+				assert.sameMembers(
+					versions,
+					[10],
+					"should have only latest version of " + type + " in cache"
+				);
+			}
+		})
+		
 		it("should mark new attachment items for download", function* () {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			Zotero.Sync.Storage.Local.setModeForLibrary(libraryID, 'zfs');
@@ -200,8 +236,6 @@ describe("Zotero.Sync.Data.Local", function() {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			
 			var type = 'item';
-			var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
-			
 			var objects = [];
 			var values = [];
 			var dateAdded = Date.now() - 86400000;
@@ -279,7 +313,6 @@ describe("Zotero.Sync.Data.Local", function() {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			
 			var type = 'item';
-			var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
 			
 			var objects = [];
 			var values = [];