From 976b5c82c6f94e220ac5c913ca7da18c8c25c2bc Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 14 Aug 2016 17:20:36 -0400 Subject: [PATCH] Fix error delaying on second library upload conflict --- .../content/zotero/xpcom/sync/syncEngine.js | 9 ++- test/tests/syncEngineTest.js | 78 +++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index a2ff49c49..00b6bb073 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -149,9 +149,6 @@ Zotero.Sync.Data.Engine.prototype.start = Zotero.Promise.coroutine(function* () // If conflict, start at beginning with downloads case this.UPLOAD_RESULT_LIBRARY_CONFLICT: - downloadResult = yield this._startDownload(); - Zotero.debug("Download result is " + downloadResult, 4); - if (!gen) { var gen = Zotero.Utilities.Internal.delayGenerator( Zotero.Sync.Data.conflictDelayIntervals, 60 * 1000 @@ -160,11 +157,15 @@ Zotero.Sync.Data.Engine.prototype.start = Zotero.Promise.coroutine(function* () // After the first upload version conflict (which is expected after remote changes), // start delaying to give other sync sessions time to complete else { - let keepGoing = yield gen.next(); + let keepGoing = yield gen.next().value; if (!keepGoing) { throw new Error("Could not sync " + this.library.name + " -- too many retries"); } } + + downloadResult = yield this._startDownload(); + Zotero.debug("Download result is " + downloadResult, 4); + break; case this.UPLOAD_RESULT_RESTART: Zotero.debug("Restarting sync for " + this.library.name); diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index d8aad6522..982964196 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -1365,6 +1365,81 @@ describe("Zotero.Sync.Data.Engine", function () { assert.equal(spy.callCount, 3); }); + + it("should delay on second upload conflict", function* () { + var library = Zotero.Libraries.userLibrary; + library.libraryVersion = 5; + yield library.saveTx(); + ({ engine, client, caller } = yield setup()); + + // Try to upload, get 412 + // Download, get new version number + // Try to upload again, get 412 + // Delay + // Download, get new version number + // Upload, get 200 + + var item = yield createDataObject('item'); + + var lastLibraryVersion = 5; + var calls = 0; + var t; + server.respond(function (req) { + if (req.method == "POST") { + calls++; + } + + // On first and second upload attempts, return 412 + if (req.method == "POST" && req.url.startsWith(baseURL + "users/1/items")) { + if (calls == 1 || calls == 2) { + if (calls == 2) { + assert.isAbove(new Date() - t, 50); + } + t = new Date(); + req.respond( + 412, + { + "Last-Modified-Version": ++lastLibraryVersion + }, + "" + ); + } + else { + req.respond( + 200, + { + "Last-Modified-Version": ++lastLibraryVersion + }, + JSON.stringify({ + successful: { + "0": item.toResponseJSON() + }, + unchanged: {}, + failed: {} + }) + ); + } + return; + } + if (req.method == "GET") { + req.respond( + 200, + { + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({}) + ); + return; + } + }); + + Zotero.Sync.Data.conflictDelayIntervals = [50, 70000]; + yield engine.start(); + + assert.equal(calls, 3); + assert.isTrue(item.synced); + assert.equal(library.libraryVersion, lastLibraryVersion); + }); }) describe("#_startDownload()", function () { @@ -1966,6 +2041,9 @@ describe("Zotero.Sync.Data.Engine", function () { Zotero.Sync.Data.conflictDelayIntervals = [50, 70000]; yield engine._startDownload(); + + assert.equal(calls, 2); + assert.equal(library.libraryVersion, lastLibraryVersion); }); });