From 3dabd63a0aeb5e9ffb138c6e1d11d37a2c87f1f3 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 19 Apr 2016 05:22:16 -0400 Subject: [PATCH] Close #930, [API Syncing] Sync synced settings --- .../zotero/xpcom/sync/syncAPIClient.js | 39 ++++++++++ .../content/zotero/xpcom/sync/syncEngine.js | 76 ++++++++++++++++++- chrome/content/zotero/xpcom/syncedSettings.js | 20 +++++ test/tests/syncEngineTest.js | 42 ++++++++++ 4 files changed, 175 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/sync/syncAPIClient.js b/chrome/content/zotero/xpcom/sync/syncAPIClient.js index 787eaff29..37bb80c91 100644 --- a/chrome/content/zotero/xpcom/sync/syncAPIClient.js +++ b/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 + "'"); diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 84284bbb4..f23082f81 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/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); diff --git a/chrome/content/zotero/xpcom/syncedSettings.js b/chrome/content/zotero/xpcom/syncedSettings.js index 74afbe26a..e80c08b4f 100644 --- a/chrome/content/zotero/xpcom/syncedSettings.js +++ b/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"); diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 2a02e3821..890eaa3bd 100644 --- a/test/tests/syncEngineTest.js +++ b/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) {