Close #930, [API Syncing] Sync synced settings

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

@@ -282,6 +282,45 @@ Zotero.Sync.APIClient.prototype = {
 	},
 	
 	
+	uploadSettings: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, libraryVersion, settings) {
+		var method = "POST";
+		var objectType = "setting";
+		var objectTypePlural = "settings";
+		var numSettings = Object.keys(settings).length;
+		
+		Zotero.debug(`Uploading ${numSettings} ${numSettings == 1 ? objectType : objectTypePlural}`);
+		
+		Zotero.debug("Sending If-Unmodified-Since-Version: " + libraryVersion);
+		
+		var json = JSON.stringify(settings);
+		var params = {
+			target: objectTypePlural,
+			libraryType: libraryType,
+			libraryTypeID: libraryTypeID
+		};
+		var uri = this.buildRequestURI(params);
+		
+		var xmlhttp = yield this.makeRequest(method, uri, {
+			headers: {
+				"Content-Type": "application/json",
+				"If-Unmodified-Since-Version": libraryVersion
+			},
+			body: json,
+			successCodes: [204, 412]
+		});
+		// 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;
+	}),
+	
+	
 	uploadObjects: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, method, libraryVersion, objectType, objects) {
 		if (method != 'POST' && method != 'PATCH') {
 			throw new Error("Invalid method '" + method + "'");

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

@@ -401,7 +401,6 @@ Zotero.Sync.Data.Engine.prototype._downloadSettings = Zotero.Promise.coroutine(f
 	var numObjects = Object.keys(results.settings).length;
 	if (numObjects) {
 		Zotero.debug(numObjects + " settings modified since last check");
-		// Settings we process immediately rather than caching
 		for (let setting in results.settings) {
 			yield Zotero.SyncedSettings.set(
 				this.libraryID,
@@ -650,10 +649,29 @@ Zotero.Sync.Data.Engine.prototype._downloadObjects = Zotero.Promise.coroutine(fu
 Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(function* () {
 	var libraryVersion = this.library.libraryVersion;
 	
+	var settingsUploaded = false;
 	var uploadNeeded = false;
 	var objectIDs = {};
 	var objectDeletions = {};
 	
+	// Upload synced settings
+	try {
+		let settings = yield Zotero.SyncedSettings.getUnsynced(this.libraryID);
+		if (Object.keys(settings).length) {
+			libraryVersion = yield this._uploadSettings(settings, libraryVersion);
+			settingsUploaded = true;
+		}
+		else {
+			Zotero.debug("No settings to upload in " + this.library.name);
+		}
+	}
+	catch (e) {
+		if (e instanceof Zotero.HTTP.UnexpectedStatusException && e.status == 412) {
+			return this.UPLOAD_RESULT_LIBRARY_CONFLICT;
+		}
+		throw e;
+	}
+	
 	// Get unsynced local objects for each object type
 	for (let objectType of Zotero.DataObjectUtilities.getTypesForLibrary(this.libraryID)) {
 		let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
@@ -688,7 +706,7 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi
 	}
 	
 	if (!uploadNeeded) {
-		return this.UPLOAD_RESULT_NOTHING_TO_UPLOAD;
+		return settingsUploaded ? this.UPLOAD_RESULT_SUCCESS : this.UPLOAD_RESULT_NOTHING_TO_UPLOAD;
 	}
 	
 	try {
@@ -717,6 +735,60 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi
 });
 
 
+Zotero.Sync.Data.Engine.prototype._uploadSettings = Zotero.Promise.coroutine(function* (settings, libraryVersion) {
+	while (true) {
+		try {
+			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,
+				Object.keys(settings),
+				libraryVersion
+			);
+			break;
+		}
+		catch (e) {
+			if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
+				if (e.status == 412) {
+					throw e;
+				}
+				
+				// 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;
+		}
+	}
+	
+	Zotero.debug("Done uploading settings in " + this.library.name);
+	
+	return libraryVersion;
+});
+
+
 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);

chrome/content/zotero/xpcom/syncedSettings.js

@@ -118,6 +118,26 @@ Zotero.SyncedSettings = (function () {
 			};
 		},
 		
+		getUnsynced: Zotero.Promise.coroutine(function* (libraryID) {
+			var sql = "SELECT setting, value FROM syncedSettings WHERE synced=0 AND libraryID=?";
+			var rows = yield Zotero.DB.queryAsync(sql, libraryID);
+			var obj = {};
+			rows.forEach(row => obj[row.setting] = JSON.parse(row.value));
+			return obj;
+		}),
+		
+		markAsSynced: Zotero.Promise.coroutine(function* (libraryID, settings, version) {
+				Zotero.debug(settings);
+			var sql = "UPDATE syncedSettings SET synced=1, version=? WHERE libraryID=? AND setting IN "
+				+ "(" + settings.map(x => '?').join(', ') + ")";
+			yield Zotero.DB.queryAsync(sql, [version, libraryID].concat(settings));
+			for (let key of settings) {
+				let setting = _cache[libraryID][key];
+				setting.synced = true;
+				setting.version = version;
+			}
+		}),
+		
 		set: Zotero.Promise.coroutine(function* (libraryID, setting, value, version = 0, synced) {
 			if (typeof value == undefined) {
 				throw new Error("Value not provided");

test/tests/syncEngineTest.js

@@ -289,6 +289,9 @@ describe("Zotero.Sync.Data.Engine", function () {
 			var lastLibraryVersion = 5;
 			yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion);
 			
+			yield Zotero.SyncedSettings.set(libraryID, "testSetting1", { foo: "bar" });
+			yield Zotero.SyncedSettings.set(libraryID, "testSetting2", { bar: "foo" });
+			
 			var types = Zotero.DataObjectUtilities.getTypes();
 			var objects = {};
 			var objectResponseJSON = {};
@@ -305,6 +308,26 @@ describe("Zotero.Sync.Data.Engine", function () {
 						req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion
 					);
 					
+					// Both settings should be uploaded
+					if (req.url == baseURL + "users/1/settings") {
+						let json = JSON.parse(req.requestBody);
+						assert.lengthOf(Object.keys(json), 2);
+						assert.property(json, "testSetting1");
+						assert.property(json, "testSetting2");
+						assert.property(json.testSetting1, "value");
+						assert.property(json.testSetting2, "value");
+						assert.propertyVal(json.testSetting1.value, "foo", "bar");
+						assert.propertyVal(json.testSetting2.value, "bar", "foo");
+						req.respond(
+							204,
+							{
+								"Last-Modified-Version": ++lastLibraryVersion
+							},
+							""
+						);
+						return;
+					}
+					
 					for (let type of types) {
 						let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type);
 						if (req.url == baseURL + "users/1/" + typePlural) {
@@ -344,6 +367,8 @@ describe("Zotero.Sync.Data.Engine", function () {
 			
 			yield engine.start();
 			
+			yield Zotero.SyncedSettings.set(libraryID, "testSetting2", { bar: "bar" });
+			
 			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
@@ -368,6 +393,23 @@ describe("Zotero.Sync.Data.Engine", function () {
 						req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion
 					);
 					
+					// Modified setting should be uploaded
+					if (req.url == baseURL + "users/1/settings") {
+						let json = JSON.parse(req.requestBody);
+						assert.lengthOf(Object.keys(json), 1);
+						assert.property(json, "testSetting2");
+						assert.property(json.testSetting2, "value");
+						assert.propertyVal(json.testSetting2.value, "bar", "bar");
+						req.respond(
+							204,
+							{
+								"Last-Modified-Version": ++lastLibraryVersion
+							},
+							""
+						);
+						return;
+					}
+					
 					for (let type of types) {
 						let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type);
 						if (req.url == baseURL + "users/1/" + typePlural) {