diff --git a/chrome/content/zotero/xpcom/sync/syncAPIClient.js b/chrome/content/zotero/xpcom/sync/syncAPIClient.js index a9c618d58..3c01b44bb 100644 --- a/chrome/content/zotero/xpcom/sync/syncAPIClient.js +++ b/chrome/content/zotero/xpcom/sync/syncAPIClient.js @@ -117,6 +117,10 @@ Zotero.Sync.APIClient.prototype = { }), + /** + * @return {Object|false} - An object with 'libraryVersion' and a 'deleted' array, or + * false if 'since' is earlier than the beginning of the delete log + */ getDeleted: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, since) { var params = { target: "deleted", @@ -125,7 +129,11 @@ Zotero.Sync.APIClient.prototype = { since: since || 0 }; var uri = this._buildRequestURI(params); - var xmlhttp = yield this._makeRequest("GET", uri); + var xmlhttp = yield this._makeRequest("GET", uri, { successCodes: [200, 409] }); + if (xmlhttp.status == 409) { + Zotero.debug(`'since' value '${since}' is earlier than the beginning of the delete log`); + return false; + } var libraryVersion = xmlhttp.getResponseHeader('Last-Modified-Version'); if (!libraryVersion) { throw new Error("Last-Modified-Version not provided"); diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index c99d7fd35..7dc6acc76 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -918,8 +918,8 @@ Zotero.Sync.Data.Engine.prototype._upgradeCheck = Zotero.Promise.coroutine(funct * missing or outdated and not up-to-date in the sync cache, download them. If any local objects * are marked as synced but aren't available remotely, mark them as unsynced for later uploading. * - * (Technically this isn't a _full_ sync on its own, because settings aren't downloaded here and - * objects are only flagged for later upload.) + * (Technically this isn't a full sync on its own, because objects are only flagged for later + * upload.) * * @param {Object[]} [versionResults] - Objects returned from getVersions(), keyed by objectType * @return {Promise} - Promise for the library version after syncing @@ -929,6 +929,7 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function* var gen; var lastLibraryVersion; + var remoteDeleted; loop: while (true) { @@ -1030,14 +1031,19 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function* let syncedKeys = yield Zotero.Sync.Data.Local.getSynced(this.libraryID, objectType); let remoteMissing = Zotero.Utilities.arrayDiff(syncedKeys, Object.keys(results.versions)); if (remoteMissing.length) { - Zotero.debug("Marking remotely missing synced " + objectTypePlural + " as unsynced"); + Zotero.debug("Checking remotely missing synced " + objectTypePlural); Zotero.debug(remoteMissing); - // TODO: Check remote deleted - // If remote delete log doesn't go back far enough, add to recovered items? - // Do we care about local timestamp of last edit? + // Check remotely deleted objects + if (!remoteDeleted) { + let results = yield this.apiClient.getDeleted( + this.libraryType, this.libraryTypeID + ); + remoteDeleted = results.deleted; + } - let ids = []; + let toDelete = []; + let toUpload = []; for (let key of remoteMissing) { let id = objectsClass.getIDFromLibraryAndKey(this.libraryID, key); if (!id) { @@ -1045,11 +1051,25 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function* + " not found to mark as unsynced"); continue; } - ids.push(id); + if (remoteDeleted[objectTypePlural].indexOf(key) != -1) { + toDelete.push(id); + continue; + } + toUpload.push(id); + } + // Delete local objects that were deleted remotely + if (toDelete.length) { + Zotero.debug("Deleting remotely deleted synced " + objectTypePlural); + yield objectsClass.erase(toDelete, { skipDeleteLog: true }); + } + // For remotely missing objects that exist locally, reset version, since old + // version will no longer match remote, and mark for upload + if (toUpload.length) { + Zotero.debug("Marking remotely missing synced " + objectTypePlural + + " as unsynced"); + yield objectsClass.updateVersion(toUpload, 0); + yield objectsClass.updateSynced(toUpload, false); } - // Reset version, since old version will no longer match remote - yield objectsClass.updateVersion(ids, 0); - yield objectsClass.updateSynced(ids, false); } // Process newly cached objects diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index e537fb66f..5662aa6b2 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -387,7 +387,7 @@ Zotero.Sync.Data.Local = { isNewObject = true; // Check if object has been deleted locally - if (yield this._objectInDeleteLog(objectType, libraryID, objectKey)) { + if (yield this.objectInDeleteLog(objectType, libraryID, objectKey)) { switch (objectType) { case 'item': throw new Error("Unimplemented"); @@ -701,7 +701,7 @@ Zotero.Sync.Data.Local = { /** * @return {Promise} */ - _objectInDeleteLog: Zotero.Promise.coroutine(function* (objectType, libraryID, key) { + objectInDeleteLog: Zotero.Promise.coroutine(function* (objectType, libraryID, key) { var syncObjectTypeID = Zotero.Sync.Data.Utilities.getSyncObjectTypeID(objectType); var sql = "SELECT COUNT(*) FROM syncDeleteLog WHERE libraryID=? AND key=? " + "AND syncObjectTypeID=?"; diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 6e5a04d35..468ed2133 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -661,16 +661,16 @@ describe("Zotero.Sync.Data.Engine", function () { assert.isFalse(Zotero.Items.exists(itemID)); // Make sure objects weren't added to sync delete log - assert.isFalse(yield Zotero.Sync.Data.Local._objectInDeleteLog( + assert.isFalse(yield Zotero.Sync.Data.Local.objectInDeleteLog( 'setting', userLibraryID, 'tagColors' )); - assert.isFalse(yield Zotero.Sync.Data.Local._objectInDeleteLog( + assert.isFalse(yield Zotero.Sync.Data.Local.objectInDeleteLog( 'collection', userLibraryID, collectionKey )); - assert.isFalse(yield Zotero.Sync.Data.Local._objectInDeleteLog( + assert.isFalse(yield Zotero.Sync.Data.Local.objectInDeleteLog( 'search', userLibraryID, searchKey )); - assert.isFalse(yield Zotero.Sync.Data.Local._objectInDeleteLog( + assert.isFalse(yield Zotero.Sync.Data.Local.objectInDeleteLog( 'item', userLibraryID, itemKey )); }) @@ -853,7 +853,6 @@ describe("Zotero.Sync.Data.Engine", function () { var userLibraryID = Zotero.Libraries.userLibraryID; ({ engine, client, caller } = yield setup()); - yield Zotero.Items.erase([1, 2], { skipDeleteLog: true }); var types = Zotero.DataObjectUtilities.getTypes(); var objects = {}; var objectJSON = {}; @@ -862,7 +861,7 @@ describe("Zotero.Sync.Data.Engine", function () { } for (let type of types) { - // Create objects with outdated versions, which should be updated + // Create object with outdated version, which should be updated let obj = createUnsavedDataObject(type); obj.synced = true; obj.version = 5; @@ -875,7 +874,7 @@ describe("Zotero.Sync.Data.Engine", function () { name: Zotero.Utilities.randomString() })); - // Create JSON for objects that exist remotely and not locally, + // Create JSON for object that exists remotely and not locally, // which should be downloaded objectJSON[type].push(makeJSONFunctions[type]({ key: Zotero.DataObjectUtilities.generateKey(), @@ -883,13 +882,21 @@ describe("Zotero.Sync.Data.Engine", function () { name: Zotero.Utilities.randomString() })); - // Create objects marked as synced that don't exist remotely, + // Create object marked as synced that doesn't exist remotely, // which should be flagged for upload obj = createUnsavedDataObject(type); obj.synced = true; obj.version = 10; yield obj.saveTx(); objects[type].push(obj); + + // Create object marked as synced that doesn't exist remotely but is in the + // remote delete log, which should be deleted locally + obj = createUnsavedDataObject(type); + obj.synced = true; + obj.version = 10; + yield obj.saveTx(); + objects[type].push(obj); } var headers = { @@ -912,15 +919,17 @@ describe("Zotero.Sync.Data.Engine", function () { } } }); + let deletedJSON = {}; for (let type of types) { - var suffix = type == 'item' ? '&includeTrashed=1' : ''; + let suffix = type == 'item' ? '&includeTrashed=1' : ''; + let plural = Zotero.DataObjectUtilities.getObjectTypePlural(type); var json = {}; json[objectJSON[type][0].key] = objectJSON[type][0].version; json[objectJSON[type][1].key] = objectJSON[type][1].version; setResponse({ method: "GET", - url: "users/1/" + Zotero.DataObjectUtilities.getObjectTypePlural(type) + url: "users/1/" + plural + "?format=versions" + suffix, status: 200, headers: headers, @@ -929,7 +938,7 @@ describe("Zotero.Sync.Data.Engine", function () { setResponse({ method: "GET", - url: "users/1/" + Zotero.DataObjectUtilities.getObjectTypePlural(type) + url: "users/1/" + plural + "?format=json" + "&" + type + "Key=" + objectJSON[type][0].key + "%2C" + objectJSON[type][1].key + suffix, @@ -937,7 +946,16 @@ describe("Zotero.Sync.Data.Engine", function () { headers: headers, json: objectJSON[type] }); + + deletedJSON[plural] = [objects[type][2].key]; } + setResponse({ + method: "GET", + url: "users/1/deleted?since=0", + status: 200, + headers: headers, + json: deletedJSON + }); yield engine._fullSync(); // Check settings @@ -963,6 +981,12 @@ describe("Zotero.Sync.Data.Engine", function () { // JSON objects 2 should be marked as unsynced, with their version reset to 0 assert.equal(objects[type][1].version, 0); assert.isFalse(objects[type][1].synced); + + // JSON objects 3 should be deleted and not in the delete log + assert.isFalse(objectsClass.getByLibraryAndKey(userLibraryID, objects[type][2].key)); + assert.isFalse(yield Zotero.Sync.Data.Local.objectInDeleteLog( + type, userLibraryID, objects[type][2].key + )); } }) })