From 02eea9912877d8e08878e997ed3c25039c183070 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 12 Jan 2016 16:52:25 -0500
Subject: [PATCH] Get latest cache object versions in bulk during sync
 downloads

---
 .../content/zotero/xpcom/sync/syncEngine.js   |  8 +--
 chrome/content/zotero/xpcom/sync/syncLocal.js | 35 +++++++---
 test/tests/syncLocalTest.js                   | 64 +++++++++++++++++++
 3 files changed, 94 insertions(+), 13 deletions(-)

diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
index cb8f40088..86017c91a 100644
--- a/chrome/content/zotero/xpcom/sync/syncEngine.js
+++ b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -256,15 +256,15 @@ Zotero.Sync.Data.Engine.prototype._startDownload = Zotero.Promise.coroutine(func
 				+ " modified since last check");
 			
 			let keys = [];
+			let versions = yield Zotero.Sync.Data.Local.getLatestCacheObjectVersions(
+				objectType, this.libraryID, Object.keys(results.versions)
+			);
 			for (let key in results.versions) {
 				// Skip objects that are already up-to-date in the sync cache. Generally all returned
 				// objects should have newer version numbers, but there are some situations, such as
 				// full syncs or interrupted syncs, where we may get versions for objects that are
 				// already up-to-date locally.
-				let version = yield Zotero.Sync.Data.Local.getLatestCacheObjectVersion(
-					objectType, this.libraryID, key
-				);
-				if (version == results.versions[key]) {
+				if (versions[key] == results.versions[key]) {
 					Zotero.debug("Skipping up-to-date " + objectType + " " + this.libraryID + "/" + key);
 					continue;
 				}
diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js
index 9b34fd432..087f3e278 100644
--- a/chrome/content/zotero/xpcom/sync/syncLocal.js
+++ b/chrome/content/zotero/xpcom/sync/syncLocal.js
@@ -247,16 +247,33 @@ 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* (objectType, libraryID) {
-		var sql = "SELECT key, version FROM syncCache WHERE libraryID=? AND "
-			+ "syncObjectTypeID IN (SELECT syncObjectTypeID FROM "
-			+ "syncObjectTypes WHERE name=?) ORDER BY version";
-		var rows = yield Zotero.DB.queryAsync(sql, [libraryID, objectType]);
+	getLatestCacheObjectVersions: Zotero.Promise.coroutine(function* (objectType, libraryID, keys=[]) {
 		var versions = {};
-		for (let i = 0; i < rows.length; i++) {
-			let row = rows[i];
-			versions[row.key] = row.version;
-		}
+		
+		yield Zotero.Utilities.Internal.forEachChunkAsync(
+			keys,
+			Zotero.DB.MAX_BOUND_PARAMETERS - 2,
+			Zotero.Promise.coroutine(function* (chunk) {
+				// The MAX(version) ensures we get the data from the most recent version of the object,
+				// thanks to SQLite 3.7.11 (http://www.sqlite.org/releaselog/3_7_11.html)
+				var sql = "SELECT key, MAX(version) AS version FROM syncCache "
+					+ "WHERE libraryID=? AND "
+					+ "syncObjectTypeID IN (SELECT syncObjectTypeID FROM syncObjectTypes WHERE name=?) ";
+				var params = [libraryID, objectType]
+				if (chunk.length) {
+					sql += "AND key IN (" + chunk.map(key => '?').join(', ') + ") ";
+					params = params.concat(chunk);
+				}
+				sql += "GROUP BY libraryID, key";
+				var rows = yield Zotero.DB.queryAsync(sql, params);
+				
+				for (let i = 0; i < rows.length; i++) {
+					let row = rows[i];
+					versions[row.key] = row.version;
+				}
+			})
+		);
+		
 		return versions;
 	}),
 	
diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js
index 84dc4e4c9..6151cf158 100644
--- a/test/tests/syncLocalTest.js
+++ b/test/tests/syncLocalTest.js
@@ -20,6 +20,70 @@ describe("Zotero.Sync.Data.Local", function() {
 		})
 	})
 	
+	
+	describe("#getLatestCacheObjectVersions", function () {
+		before(function* () {
+			yield Zotero.Sync.Data.Local.saveCacheObjects(
+				'item',
+				Zotero.Libraries.userLibraryID,
+				[
+					{
+						key: 'AAAAAAAA',
+						version: 2,
+						title: "A2"
+					},
+					{
+						key: 'AAAAAAAA',
+						version: 1,
+						title: "A1"
+					},
+					{
+						key: 'BBBBBBBB',
+						version: 1,
+						title: "B1"
+					},
+					{
+						key: 'BBBBBBBB',
+						version: 2,
+						title: "B2"
+					},
+					{
+						key: 'CCCCCCCC',
+						version: 3,
+						title: "C"
+					}
+				]
+			);
+		})
+		
+		it("should return latest version of all objects if no keys passed", function* () {
+			var versions = yield Zotero.Sync.Data.Local.getLatestCacheObjectVersions(
+				'item',
+				Zotero.Libraries.userLibraryID
+			);
+			var keys = Object.keys(versions);
+			assert.lengthOf(keys, 3);
+			assert.sameMembers(keys, ['AAAAAAAA', 'BBBBBBBB', 'CCCCCCCC']);
+			assert.equal(versions.AAAAAAAA, 2);
+			assert.equal(versions.BBBBBBBB, 2);
+			assert.equal(versions.CCCCCCCC, 3);
+		})
+		
+		it("should return latest version of objects with passed keys", function* () {
+			var versions = yield Zotero.Sync.Data.Local.getLatestCacheObjectVersions(
+				'item',
+				Zotero.Libraries.userLibraryID,
+				['AAAAAAAA', 'CCCCCCCC']
+			);
+			var keys = Object.keys(versions);
+			assert.lengthOf(keys, 2);
+			assert.sameMembers(keys, ['AAAAAAAA', 'CCCCCCCC']);
+			assert.equal(versions.AAAAAAAA, 2);
+			assert.equal(versions.CCCCCCCC, 3);
+		})
+	})
+	
+	
 	describe("#processSyncCacheForObjectType()", function () {
 		var types = Zotero.DataObjectUtilities.getTypes();