From 1bd058ed481b879c20161f899f0bedc58d1f1dc5 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 19 Jul 2016 22:12:13 -0400 Subject: [PATCH] Prompt for library data reset on 403 upload error Addresses #1041 for data -- still needed for files --- .../zotero/xpcom/storage/storageUtilities.js | 46 +++++++++- .../content/zotero/xpcom/sync/syncEngine.js | 44 ++++++++-- chrome/content/zotero/xpcom/sync/syncLocal.js | 84 ++----------------- .../zotero/xpcom/sync/syncUtilities.js | 43 ++++++++++ test/tests/syncEngineTest.js | 78 +++++++++++++++++ test/tests/syncLocalTest.js | 26 +++--- 6 files changed, 222 insertions(+), 99 deletions(-) diff --git a/chrome/content/zotero/xpcom/storage/storageUtilities.js b/chrome/content/zotero/xpcom/storage/storageUtilities.js index ea96e50ef9..4e5919f0c4 100644 --- a/chrome/content/zotero/xpcom/storage/storageUtilities.js +++ b/chrome/content/zotero/xpcom/storage/storageUtilities.js @@ -63,5 +63,49 @@ Zotero.Sync.Storage.Utilities = { } } ); - }) + }), + + + /** + * Prompt whether to reset unsynced local files in a library + * + * Keep in sync with Sync.Data.Utilities.showWriteAccessLostPrompt() + * + * @param {Window|null} win + * @param {Zotero.Library} library + * @return {Integer} - 0 to reset, 1 to skip + */ + showFileWriteAccessLostPrompt: function (win, library) { + var libraryType = library.libraryType; + switch (libraryType) { + case 'group': + var msg = Zotero.getString('sync.error.groupFileWriteAccessLost', + [library.name, ZOTERO_CONFIG.DOMAIN_NAME]) + + "\n\n" + + Zotero.getString('sync.error.groupCopyChangedFiles') + var button1Text = Zotero.getString('sync.resetGroupFilesAndSync'); + var button2Text = Zotero.getString('sync.skipGroup'); + break; + + default: + throw new Error("Unsupported library type " + libraryType); + } + + var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] + .getService(Components.interfaces.nsIPromptService); + var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) + + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING) + + ps.BUTTON_DELAY_ENABLE; + + return ps.confirmEx( + win, + Zotero.getString('general.permissionDenied'), + msg, + buttonFlags, + button1Text, + button2Text, + null, + null, {} + ); + } } diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index b753220212..94ef612ebf 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -79,6 +79,8 @@ Zotero.Sync.Data.Engine.prototype.UPLOAD_RESULT_SUCCESS = 1; Zotero.Sync.Data.Engine.prototype.UPLOAD_RESULT_NOTHING_TO_UPLOAD = 2; Zotero.Sync.Data.Engine.prototype.UPLOAD_RESULT_LIBRARY_CONFLICT = 3; Zotero.Sync.Data.Engine.prototype.UPLOAD_RESULT_OBJECT_CONFLICT = 4; +Zotero.Sync.Data.Engine.prototype.UPLOAD_RESULT_RESTART = 5; +Zotero.Sync.Data.Engine.prototype.UPLOAD_RESULT_CANCEL = 6; Zotero.Sync.Data.Engine.prototype.start = Zotero.Promise.coroutine(function* () { Zotero.debug("Starting data sync for " + this.library.name); @@ -163,6 +165,14 @@ Zotero.Sync.Data.Engine.prototype.start = Zotero.Promise.coroutine(function* () throw new Error("Could not sync " + this.library.name + " -- too many retries"); } } + + case this.UPLOAD_RESULT_RESTART: + Zotero.debug("Restarting sync for " + this.library.name); + break; + + case this.UPLOAD_RESULT_CANCEL: + Zotero.debug("Cancelling sync for " + this.library.name); + return; } } @@ -756,10 +766,7 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi } } catch (e) { - if (e instanceof Zotero.HTTP.UnexpectedStatusException && e.status == 412) { - return this.UPLOAD_RESULT_LIBRARY_CONFLICT; - } - throw e; + return this._handleUploadError(e); } // Get unsynced local objects for each object type @@ -824,10 +831,7 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi } } catch (e) { - if (e instanceof Zotero.HTTP.UnexpectedStatusException && e.status == 412) { - return this.UPLOAD_RESULT_LIBRARY_CONFLICT; - } - throw e; + return this._handleUploadError(e); } return this.UPLOAD_RESULT_SUCCESS; @@ -1446,6 +1450,30 @@ Zotero.Sync.Data.Engine.prototype._getOptions = function (additionalOpts = {}) { } +Zotero.Sync.Data.Engine.prototype._handleUploadError = Zotero.Promise.coroutine(function* (e) { + if (e instanceof Zotero.HTTP.UnexpectedStatusException) { + switch (e.status) { + // This should only happen if library permissions were changed between the group check at + // sync start and now, or to people who upgraded from <5.0-beta.r25+66ca2cf with unsynced local + // changes. + case 403: + let index = Zotero.Sync.Data.Utilities.showWriteAccessLostPrompt(null, this.library); + if (index === 0) { + yield Zotero.Sync.Data.Local.resetUnsyncedLibraryData(this.libraryID); + return this.UPLOAD_RESULT_RESTART; + } + if (index == 1) { + return this.UPLOAD_RESULT_CANCEL; + } + throw new Error(`Unexpected index value ${index}`); + + case 412: + return this.UPLOAD_RESULT_LIBRARY_CONFLICT; + } + } + throw e; +}); + Zotero.Sync.Data.Engine.prototype._failedCheck = function () { if (this.stopOnError && this.failed) { Zotero.logError("Stopping on error"); diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index a6dd980c44..5e369a02e7 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -165,11 +165,11 @@ Zotero.Sync.Data.Local = { if (library.editable && !editable && ((yield this._libraryHasUnsyncedData(libraryID)) || (yield this._libraryHasUnsyncedFiles(libraryID)))) { - let index = this._showWriteAccessLostPrompt(win, library); + let index = Zotero.Sync.Data.Utilities.showWriteAccessLostPrompt(win, library); // Reset library if (index == 0) { - yield this._resetUnsyncedLibraryData(libraryID); + yield this.resetUnsyncedLibraryData(libraryID); return true; } @@ -178,11 +178,11 @@ Zotero.Sync.Data.Local = { } if (library.filesEditable && !filesEditable && (yield this._libraryHasUnsyncedFiles(libraryID))) { - let index = this._showFileWriteAccessLostPrompt(win, library); + let index = Zotero.Sync.Storage.Utilities.showFileWriteAccessLostPrompt(win, library); // Reset library files if (index == 0) { - yield this._resetUnsyncedLibraryFiles(libraryID); + yield this.resetUnsyncedLibraryFiles(libraryID); return true; } @@ -222,77 +222,7 @@ Zotero.Sync.Data.Local = { }), - _showWriteAccessLostPrompt: function (win, library) { - var libraryType = library.libraryType; - switch (libraryType) { - case 'group': - var msg = Zotero.getString('sync.error.groupWriteAccessLost', - [library.name, ZOTERO_CONFIG.DOMAIN_NAME]) - + "\n\n" - + Zotero.getString('sync.error.groupCopyChangedItems') - var button1Text = Zotero.getString('sync.resetGroupAndSync'); - var button2Text = Zotero.getString('sync.skipGroup'); - break; - - default: - throw new Error("Unsupported library type " + libraryType); - } - - var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] - .getService(Components.interfaces.nsIPromptService); - var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) - + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING) - + ps.BUTTON_DELAY_ENABLE; - - return ps.confirmEx( - win, - Zotero.getString('general.permissionDenied'), - msg, - buttonFlags, - button1Text, - button2Text, - null, - null, {} - ); - }, - - - _showFileWriteAccessLostPrompt: function (win, library) { - var libraryType = library.libraryType; - switch (libraryType) { - case 'group': - var msg = Zotero.getString('sync.error.groupFileWriteAccessLost', - [library.name, ZOTERO_CONFIG.DOMAIN_NAME]) - + "\n\n" - + Zotero.getString('sync.error.groupCopyChangedFiles') - var button1Text = Zotero.getString('sync.resetGroupFilesAndSync'); - var button2Text = Zotero.getString('sync.skipGroup'); - break; - - default: - throw new Error("Unsupported library type " + libraryType); - } - - var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] - .getService(Components.interfaces.nsIPromptService); - var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) - + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING) - + ps.BUTTON_DELAY_ENABLE; - - return ps.confirmEx( - win, - Zotero.getString('general.permissionDenied'), - msg, - buttonFlags, - button1Text, - button2Text, - null, - null, {} - ); - }, - - - _resetUnsyncedLibraryData: Zotero.Promise.coroutine(function* (libraryID) { + resetUnsyncedLibraryData: Zotero.Promise.coroutine(function* (libraryID) { let settings = yield Zotero.SyncedSettings.getUnsynced(libraryID); if (Object.keys(settings).length) { yield Zotero.Promise.each(Object.keys(settings), function (key) { @@ -338,7 +268,7 @@ Zotero.Sync.Data.Local = { library.libraryVersion = -1; yield library.saveTx(); - yield this._resetUnsyncedLibraryFiles(libraryID); + yield this.resetUnsyncedLibraryFiles(libraryID); }), @@ -347,7 +277,7 @@ Zotero.Sync.Data.Local = { * * _libraryHasUnsyncedFiles(), which checks for updated files, must be called first. */ - _resetUnsyncedLibraryFiles: Zotero.Promise.coroutine(function* (libraryID) { + resetUnsyncedLibraryFiles: Zotero.Promise.coroutine(function* (libraryID) { var itemIDs = yield Zotero.Sync.Storage.Local.getFilesToUpload(libraryID); for (let itemID of itemIDs) { let item = Zotero.Items.get(itemID); diff --git a/chrome/content/zotero/xpcom/sync/syncUtilities.js b/chrome/content/zotero/xpcom/sync/syncUtilities.js index d99a727e96..1e61bc1fb3 100644 --- a/chrome/content/zotero/xpcom/sync/syncUtilities.js +++ b/chrome/content/zotero/xpcom/sync/syncUtilities.js @@ -46,4 +46,47 @@ Zotero.Sync.Data.Utilities = { } return this._syncObjectTypeIDs[objectType]; }, + + + /** + * Prompt whether to reset unsynced local data in a library + * + * Keep in sync with Sync.Storage.Utilities.showFileWriteAccessLostPrompt() + * @param {Window|null} win + * @param {Zotero.Library} library + * @return {Integer} - 0 to reset, 1 to skip + */ + showWriteAccessLostPrompt: function (win, library) { + var libraryType = library.libraryType; + switch (libraryType) { + case 'group': + var msg = Zotero.getString('sync.error.groupWriteAccessLost', + [library.name, ZOTERO_CONFIG.DOMAIN_NAME]) + + "\n\n" + + Zotero.getString('sync.error.groupCopyChangedItems') + var button1Text = Zotero.getString('sync.resetGroupAndSync'); + var button2Text = Zotero.getString('sync.skipGroup'); + break; + + default: + throw new Error("Unsupported library type " + libraryType); + } + + var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] + .getService(Components.interfaces.nsIPromptService); + var buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) + + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING) + + ps.BUTTON_DELAY_ENABLE; + + return ps.confirmEx( + win, + Zotero.getString('general.permissionDenied'), + msg, + buttonFlags, + button1Text, + button2Text, + null, + null, {} + ); + } }; diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 55c692e5a9..bbd21189f7 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -1918,6 +1918,84 @@ describe("Zotero.Sync.Data.Engine", function () { var result = yield engine._startUpload(); assert.equal(result, engine.UPLOAD_RESULT_NOTHING_TO_UPLOAD); }); + + + it("should prompt to reset library on 403 write response and reset on accept", function* () { + var group = yield createGroup({ + libraryVersion: 5 + }); + var libraryID = group.libraryID; + ({ engine, client, caller } = yield setup({ libraryID })); + + var item = createUnsavedDataObject('item'); + item.libraryID = libraryID; + item.setField('title', 'A'); + item.synced = false; + var itemID = yield item.saveTx(); + + var headers = { + "Last-Modified-Version": 5 + }; + setResponse({ + method: "POST", + url: `groups/${group.id}/items`, + status: 403, + headers, + text: "" + }) + + var promise = waitForDialog(function (dialog) { + var text = dialog.document.documentElement.textContent; + assert.include(text, group.name); + }); + + var result = yield engine._startUpload(); + assert.equal(result, engine.UPLOAD_RESULT_RESTART); + + assert.isFalse(Zotero.Items.exists(itemID)); + + // Library version should have been reset to trigger full sync + assert.equal(group.libraryVersion, -1); + }); + + + it("should prompt to reset library on 403 write response and skip on cancel", function* () { + var group = yield createGroup({ + libraryVersion: 5 + }); + var libraryID = group.libraryID; + ({ engine, client, caller } = yield setup({ libraryID })); + + var item = createUnsavedDataObject('item'); + item.libraryID = libraryID; + item.setField('title', 'A'); + item.synced = false; + var itemID = yield item.saveTx(); + + var headers = { + "Last-Modified-Version": 5 + }; + setResponse({ + method: "POST", + url: `groups/${group.id}/items`, + status: 403, + headers, + text: "" + }) + + var promise = waitForDialog(function (dialog) { + var text = dialog.document.documentElement.textContent; + assert.include(text, group.name); + }, "cancel"); + + var result = yield engine._startUpload(); + assert.equal(result, engine.UPLOAD_RESULT_CANCEL); + + assert.isTrue(Zotero.Items.exists(itemID)); + + // Library version shouldn't have changed + assert.equal(group.libraryVersion, 5); + }); }); diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js index 35ba6198e3..e3404961e9 100644 --- a/test/tests/syncLocalTest.js +++ b/test/tests/syncLocalTest.js @@ -109,8 +109,8 @@ describe("Zotero.Sync.Data.Local", function() { }); var mock = sinon.mock(Zotero.Sync.Data.Local); - mock.expects("_resetUnsyncedLibraryData").once().returns(Zotero.Promise.resolve()); - mock.expects("_resetUnsyncedLibraryFiles").never(); + mock.expects("resetUnsyncedLibraryData").once().returns(Zotero.Promise.resolve()); + mock.expects("resetUnsyncedLibraryFiles").never(); assert.isTrue( yield Zotero.Sync.Data.Local.checkLibraryForAccess(null, libraryID, false, false) @@ -129,8 +129,8 @@ describe("Zotero.Sync.Data.Local", function() { }, "cancel"); var mock = sinon.mock(Zotero.Sync.Data.Local); - mock.expects("_resetUnsyncedLibraryData").never(); - mock.expects("_resetUnsyncedLibraryFiles").never(); + mock.expects("resetUnsyncedLibraryData").never(); + mock.expects("resetUnsyncedLibraryFiles").never(); assert.isFalse( yield Zotero.Sync.Data.Local.checkLibraryForAccess(null, libraryID, false, false) @@ -158,8 +158,8 @@ describe("Zotero.Sync.Data.Local", function() { }); var mock = sinon.mock(Zotero.Sync.Data.Local); - mock.expects("_resetUnsyncedLibraryData").never(); - mock.expects("_resetUnsyncedLibraryFiles").once().returns(Zotero.Promise.resolve()); + mock.expects("resetUnsyncedLibraryData").never(); + mock.expects("resetUnsyncedLibraryFiles").once().returns(Zotero.Promise.resolve()); assert.isTrue( yield Zotero.Sync.Data.Local.checkLibraryForAccess(null, libraryID, true, false) @@ -178,8 +178,8 @@ describe("Zotero.Sync.Data.Local", function() { }, "cancel"); var mock = sinon.mock(Zotero.Sync.Data.Local); - mock.expects("_resetUnsyncedLibraryData").never(); - mock.expects("_resetUnsyncedLibraryFiles").never(); + mock.expects("resetUnsyncedLibraryData").never(); + mock.expects("resetUnsyncedLibraryFiles").never(); assert.isFalse( yield Zotero.Sync.Data.Local.checkLibraryForAccess(null, libraryID, true, false) @@ -214,7 +214,7 @@ describe("Zotero.Sync.Data.Local", function() { }); - describe("#_resetUnsyncedLibraryData()", function () { + describe("#resetUnsyncedLibraryData()", function () { it("should revert group and mark for full sync", function* () { var group = yield createGroup({ version: 1, @@ -254,7 +254,7 @@ describe("Zotero.Sync.Data.Local", function() { var deletedItemKey = deletedItem.key; yield deletedItem.eraseTx(); - yield Zotero.Sync.Data.Local._resetUnsyncedLibraryData(libraryID); + yield Zotero.Sync.Data.Local.resetUnsyncedLibraryData(libraryID); assert.isNull(Zotero.SyncedSettings.get(group.libraryID, "testSetting")); @@ -274,7 +274,7 @@ describe("Zotero.Sync.Data.Local", function() { }); - describe("#_resetUnsyncedLibraryFiles", function () { + describe("#resetUnsyncedLibraryFiles", function () { it("should delete unsynced files", function* () { var group = yield createGroup({ version: 1, @@ -288,10 +288,10 @@ describe("Zotero.Sync.Data.Local", function() { attachment1.attachmentSyncedHash = "8caf2ee22919d6725eb0648b98ef6bad"; var attachment2 = yield importFileAttachment('test.pdf', { libraryID }); - // Has to be called before _resetUnsyncedLibraryFiles() + // Has to be called before resetUnsyncedLibraryFiles() assert.isTrue(yield Zotero.Sync.Data.Local._libraryHasUnsyncedFiles(libraryID)); - yield Zotero.Sync.Data.Local._resetUnsyncedLibraryFiles(libraryID); + yield Zotero.Sync.Data.Local.resetUnsyncedLibraryFiles(libraryID); assert.isFalse(yield attachment1.fileExists()); assert.isFalse(yield attachment2.fileExists());