From f5eb99ad4c4fe765422bb29ff487a1cd42345255 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sat, 6 Nov 2021 03:38:25 -0400
Subject: [PATCH] Upload settings in batches of 250

Closes #2000
---
 .../content/zotero/xpcom/sync/syncEngine.js   | 57 +++++++++--------
 test/tests/syncEngineTest.js                  | 61 +++++++++++++++++++
 2 files changed, 93 insertions(+), 25 deletions(-)

diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
index 97936deaf9..4238acaea2 100644
--- a/chrome/content/zotero/xpcom/sync/syncEngine.js
+++ b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -1102,35 +1102,42 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi
 });
 
 
-Zotero.Sync.Data.Engine.prototype._uploadSettings = Zotero.Promise.coroutine(function* (settings, libraryVersion) {
-	let json = {};
-	for (let key in settings) {
-		json[key] = {
-			value: settings[key]
-		};
-	}
-	libraryVersion = yield this.apiClient.uploadSettings(
-		this.library.libraryType,
-		this.libraryTypeID,
-		libraryVersion,
-		json
-	);
-	yield Zotero.SyncedSettings.markAsSynced(
-		this.libraryID,
+Zotero.Sync.Data.Engine.prototype._uploadSettings = async function (settings, libraryVersion) {
+	const SETTINGS_BATCH_SIZE = 250;
+	await Zotero.Utilities.Internal.forEachChunkAsync(
 		Object.keys(settings),
-		libraryVersion
+		SETTINGS_BATCH_SIZE,
+		async function (keys) {
+			let json = {};
+			for (let key of keys) {
+				json[key] = {
+					value: settings[key]
+				};
+			}
+			libraryVersion = await this.apiClient.uploadSettings(
+				this.library.libraryType,
+				this.libraryTypeID,
+				libraryVersion,
+				json
+			);
+			await Zotero.SyncedSettings.markAsSynced(
+				this.libraryID,
+				keys,
+				libraryVersion
+			);
+			
+			if (this.library.libraryVersion == this.library.storageVersion) {
+				this.library.storageVersion = libraryVersion;
+			}
+			this.library.libraryVersion = libraryVersion;
+			await this.library.saveTx({
+				skipNotifier: true
+			});
+		}.bind(this)
 	);
-	if (this.library.libraryVersion == this.library.storageVersion) {
-		this.library.storageVersion = libraryVersion;
-	}
-	this.library.libraryVersion = libraryVersion;
-	yield this.library.saveTx({
-		skipNotifier: true
-	});
-	
 	Zotero.debug("Done uploading settings in " + this.library.name);
 	return libraryVersion;
-});
+};
 
 
 Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(function* (objectType, ids, libraryVersion) {
diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js
index fb67c4b612..e85c71c9cd 100644
--- a/test/tests/syncEngineTest.js
+++ b/test/tests/syncEngineTest.js
@@ -959,6 +959,67 @@ describe("Zotero.Sync.Data.Engine", function () {
 		});
 		
 		
+		it("should upload settings in batches", async function () {
+			({ engine, client, caller } = await setup());
+			
+			var batchSize = 250;
+			
+			var library = Zotero.Libraries.userLibrary;
+			var libraryID = library.id;
+			var lastLibraryVersion = 5;
+			library.libraryVersion = library.storageVersion = lastLibraryVersion;
+			await library.saveTx();
+			
+			for (let i = 0; i < (batchSize * 2 + 5); i++) {
+				let key = Zotero.DataObjectUtilities.generateKey();
+				let page = Zotero.Utilities.rand(1, 20);
+				await Zotero.SyncedSettings.set(libraryID, `lastPageIndex_u_${key}`, page);
+			}
+			
+			var keys = new Set();
+			
+			var numRequests = 0;
+			server.respond(function (req) {
+				if (req.method == "POST") {
+					numRequests++;
+					assert.equal(
+						req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion
+					);
+					
+					if (req.url == baseURL + "users/1/settings") {
+						let json = JSON.parse(req.requestBody);
+						
+						Zotero.debug(json);
+						if (numRequests == 1 || numRequests == 2) {
+							assert.lengthOf(Object.keys(json), batchSize);
+						}
+						else {
+							assert.lengthOf(Object.keys(json), 5);
+						}
+						
+						for (let key of Object.keys(json)) {
+							keys.add(key);
+						}
+						
+						req.respond(
+							204,
+							{
+								"Last-Modified-Version": ++lastLibraryVersion
+							},
+							""
+						);
+						return;
+					}
+				}
+			})
+			
+			await engine.start();
+			
+			assert.equal(numRequests, 3);
+			assert.equal(keys.size, batchSize * 2 + 5);
+		});
+		
+		
 		it("shouldn't update library storage version after settings upload if storage version was already behind", function* () {
 			({ engine, client, caller } = yield setup());