From 1e6c29766fbf4dc2258f05c3b14bdf3b0b9b1d94 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 1 Nov 2015 03:43:04 -0500 Subject: [PATCH] Add deletion uploading to API syncing [DB reupgrade] Tags deletions are not currently synced, and maybe don't need to be. --- chrome/content/zotero/xpcom/schema.js | 10 +- .../zotero/xpcom/sync/syncAPIClient.js | 53 ++- .../content/zotero/xpcom/sync/syncEngine.js | 444 +++++++++++------- .../zotero/xpcom/sync/syncEventListeners.js | 6 +- chrome/content/zotero/xpcom/sync/syncLocal.js | 39 +- resource/schema/userdata.sql | 4 - test/tests/syncEngineTest.js | 70 ++- 7 files changed, 421 insertions(+), 205 deletions(-) diff --git a/chrome/content/zotero/xpcom/schema.js b/chrome/content/zotero/xpcom/schema.js index 59f39f621..be358897e 100644 --- a/chrome/content/zotero/xpcom/schema.js +++ b/chrome/content/zotero/xpcom/schema.js @@ -2211,20 +2211,18 @@ Zotero.Schema = new function(){ yield Zotero.DB.queryAsync("UPDATE syncDeleteLog SET libraryID=1 WHERE libraryID=0"); yield Zotero.DB.queryAsync("ALTER TABLE syncDeleteLog RENAME TO syncDeleteLogOld"); - yield Zotero.DB.queryAsync("CREATE TABLE syncDeleteLog (\n syncObjectTypeID INT NOT NULL,\n libraryID INT NOT NULL,\n key TEXT NOT NULL,\n dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n synced INT NOT NULL DEFAULT 0,\n UNIQUE (syncObjectTypeID, libraryID, key),\n FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID),\n FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)"); - yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO syncDeleteLog SELECT syncObjectTypeID, libraryID, key, timestamp, 0 FROM syncDeleteLogOld"); + yield Zotero.DB.queryAsync("CREATE TABLE syncDeleteLog (\n syncObjectTypeID INT NOT NULL,\n libraryID INT NOT NULL,\n key TEXT NOT NULL,\n dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n UNIQUE (syncObjectTypeID, libraryID, key),\n FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID),\n FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)"); + yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO syncDeleteLog SELECT syncObjectTypeID, libraryID, key, timestamp FROM syncDeleteLogOld"); yield Zotero.DB.queryAsync("DROP INDEX IF EXISTS syncDeleteLog_timestamp"); - yield Zotero.DB.queryAsync("CREATE INDEX syncDeleteLog_synced ON syncDeleteLog(synced)"); // TODO: Something special for tag deletions? //yield Zotero.DB.queryAsync("DELETE FROM syncDeleteLog WHERE syncObjectTypeID IN (2, 5, 6)"); //yield Zotero.DB.queryAsync("DELETE FROM syncObjectTypes WHERE syncObjectTypeID IN (2, 5, 6)"); yield Zotero.DB.queryAsync("UPDATE storageDeleteLog SET libraryID=1 WHERE libraryID=0"); yield Zotero.DB.queryAsync("ALTER TABLE storageDeleteLog RENAME TO storageDeleteLogOld"); - yield Zotero.DB.queryAsync("CREATE TABLE storageDeleteLog (\n libraryID INT NOT NULL,\n key TEXT NOT NULL,\n dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n synced INT NOT NULL DEFAULT 0,\n PRIMARY KEY (libraryID, key),\n FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)"); - yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO storageDeleteLog SELECT libraryID, key, timestamp, 0 FROM storageDeleteLogOld"); + yield Zotero.DB.queryAsync("CREATE TABLE storageDeleteLog (\n libraryID INT NOT NULL,\n key TEXT NOT NULL,\n dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,\n PRIMARY KEY (libraryID, key),\n FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)"); + yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO storageDeleteLog SELECT libraryID, key, timestamp FROM storageDeleteLogOld"); yield Zotero.DB.queryAsync("DROP INDEX IF EXISTS storageDeleteLog_timestamp"); - yield Zotero.DB.queryAsync("CREATE INDEX storageDeleteLog_synced ON storageDeleteLog(synced)"); yield Zotero.DB.queryAsync("ALTER TABLE annotations RENAME TO annotationsOld"); yield Zotero.DB.queryAsync("CREATE TABLE annotations (\n annotationID INTEGER PRIMARY KEY,\n itemID INT NOT NULL,\n parent TEXT,\n textNode INT,\n offset INT,\n x INT,\n y INT,\n cols INT,\n rows INT,\n text TEXT,\n collapsed BOOL,\n dateModified DATE,\n FOREIGN KEY (itemID) REFERENCES itemAttachments(itemID) ON DELETE CASCADE\n)"); diff --git a/chrome/content/zotero/xpcom/sync/syncAPIClient.js b/chrome/content/zotero/xpcom/sync/syncAPIClient.js index 81d3eaf9a..6e76106b8 100644 --- a/chrome/content/zotero/xpcom/sync/syncAPIClient.js +++ b/chrome/content/zotero/xpcom/sync/syncAPIClient.js @@ -119,7 +119,7 @@ Zotero.Sync.APIClient.prototype = { /** - * @return {Object|false} - An object with 'libraryVersion' and a 'deleted' array, or + * @return {Object|false} - An object with 'libraryVersion' and a 'deleted' object, or * false if 'since' is earlier than the beginning of the delete log */ getDeleted: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, since) { @@ -277,7 +277,7 @@ Zotero.Sync.APIClient.prototype = { }, - uploadObjects: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, objectType, method, version, objects) { + uploadObjects: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, method, libraryVersion, objectType, objects) { if (method != 'POST' && method != 'PATCH') { throw new Error("Invalid method '" + method + "'"); } @@ -287,7 +287,7 @@ Zotero.Sync.APIClient.prototype = { Zotero.debug("Uploading " + objects.length + " " + (objects.length == 1 ? objectType : objectTypePlural)); - Zotero.debug("Sending If-Unmodified-Since-Version: " + version); + Zotero.debug("Sending If-Unmodified-Since-Version: " + libraryVersion); var json = JSON.stringify(objects); var params = { @@ -299,7 +299,7 @@ Zotero.Sync.APIClient.prototype = { var xmlhttp = yield this.makeRequest(method, uri, { headers: { - "If-Unmodified-Since-Version": version + "If-Unmodified-Since-Version": libraryVersion }, body: json, successCodes: [200, 412] @@ -309,17 +309,57 @@ Zotero.Sync.APIClient.prototype = { Zotero.debug("Server returned 412: " + xmlhttp.responseText, 2); throw new Zotero.HTTP.UnexpectedStatusException(xmlhttp); } - var libraryVersion = xmlhttp.getResponseHeader('Last-Modified-Version'); + libraryVersion = xmlhttp.getResponseHeader('Last-Modified-Version'); if (!libraryVersion) { throw new Error("Last-Modified-Version not provided"); } return { - libraryVersion: libraryVersion, + libraryVersion, results: this._parseJSON(xmlhttp.responseText) }; }), + uploadDeletions: Zotero.Promise.coroutine(function* (libraryType, libraryTypeID, libraryVersion, objectType, keys) { + var objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType); + + Zotero.debug(`Uploading ${keys.length} ${objectType} deletion` + + (keys.length == 1 ? '' : 's')); + + Zotero.debug("Sending If-Unmodified-Since-Version: " + libraryVersion); + + var params = { + target: objectTypePlural, + libraryType: libraryType, + libraryTypeID: libraryTypeID + }; + if (objectType == 'tag') { + params.tags = keys.join("||"); + } + else { + params[objectType + "Key"] = keys.join(","); + } + var uri = this.buildRequestURI(params); + + var xmlhttp = yield this.makeRequest("DELETE", uri, { + headers: { + "If-Unmodified-Since-Version": libraryVersion + }, + successCodes: [204] + }); + // 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; + }), + + buildRequestURI: function (params) { var uri = this.baseURL; @@ -354,6 +394,7 @@ Zotero.Sync.APIClient.prototype = { 'itemKey', 'collectionKey', 'searchKey', + 'tag', 'linkMode', 'start', 'limit', diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 66350780c..c8b66ebbd 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -67,6 +67,7 @@ Zotero.Sync.Data.Engine = function (options) { this.stopOnError = options.stopOnError; this.requests = []; this.uploadBatchSize = 25; + this.uploadDeletionBatchSize = 50; this.failed = false; @@ -567,21 +568,39 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi var uploadNeeded = false; var objectIDs = {}; + var objectDeletions = {}; // Get unsynced local objects for each object type for (let objectType of Zotero.DataObjectUtilities.getTypesForLibrary(this.libraryID)) { let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType); - let ids = yield Zotero.Sync.Data.Local.getUnsynced(this.libraryID, objectType); - if (!ids.length) { - Zotero.debug("No " + objectTypePlural + " to upload in " + this.libraryName); - continue; + // New/modified objects + let ids = yield Zotero.Sync.Data.Local.getUnsynced(objectType, this.libraryID); + if (ids.length) { + Zotero.debug(ids.length + " " + + (ids.length == 1 ? objectType : objectTypePlural) + + " to upload in library " + this.libraryID); + objectIDs[objectType] = ids; + } + else { + Zotero.debug("No " + objectTypePlural + " to upload in " + this.libraryName); + } + + // Deleted objects + let keys = yield Zotero.Sync.Data.Local.getDeleted(objectType, this.libraryID); + if (keys.length) { + Zotero.debug(`${keys.length} ${objectType} deletion` + + (keys.length == 1 ? '' : 's') + + ` to upload in ${this.libraryName}`); + objectDeletions[objectType] = keys; + } + else { + Zotero.debug(`No ${objectType} deletions to upload in ${this.libraryName}`); + } + + if (ids.length || keys.length) { + uploadNeeded = true; } - Zotero.debug(ids.length + " " - + (ids.length == 1 ? objectType : objectTypePlural) - + " to upload in library " + this.libraryID); - objectIDs[objectType] = ids; - uploadNeeded = true; } if (!uploadNeeded) { @@ -589,192 +608,271 @@ Zotero.Sync.Data.Engine.prototype._startUpload = Zotero.Promise.coroutine(functi } Zotero.debug(JSON.stringify(objectIDs)); - for (let objectType in objectIDs) { - let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType); - let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); - - let queue = []; - for (let id of objectIDs[objectType]) { - queue.push({ - id: id, - json: null, - tries: 0, - failed: false - }); + libraryVersion = yield this._uploadObjects( + objectType, objectIDs[objectType], libraryVersion + ); + } + + Zotero.debug(JSON.stringify(objectDeletions)); + for (let objectType in objectDeletions) { + libraryVersion = yield this._uploadDeletions( + objectType, objectDeletions[objectType], libraryVersion + ); + } + + return this.UPLOAD_RESULT_SUCCESS; +}); + + +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); + + let queue = []; + for (let id of ids) { + queue.push({ + id: id, + json: null, + tries: 0, + failed: false + }); + } + + let failureDelayGenerator = null; + + while (queue.length) { + // Get a slice of the queue and generate JSON for objects if necessary + let batch = []; + let numSkipped = 0; + for (let i = 0; i < queue.length && queue.length < this.uploadBatchSize; i++) { + let o = queue[i]; + // Skip requests that failed with 4xx + if (o.failed) { + numSkipped++; + continue; + } + if (!o.json) { + o.json = yield this._getJSONForObject(objectType, o.id); + } + batch.push(o.json); } - let failureDelayGenerator = null; + // No more non-failed requests + if (!batch.length) { + break; + } - while (queue.length) { - // Get a slice of the queue and generate JSON for objects if necessary - let batch = []; - for (let i = 0; i < queue.length && queue.length < this.uploadBatchSize; i++) { - let o = queue[i]; - // Skip requests that failed with 4xx - if (o.failed) { + // Remove selected and skipped objects from queue + queue.splice(0, batch.length + numSkipped); + + Zotero.debug("UPLOAD BATCH:"); + Zotero.debug(batch); + + let numSuccessful = 0; + try { + let json = yield this.apiClient.uploadObjects( + this.libraryType, + this.libraryTypeID, + "POST", + libraryVersion, + objectType, + batch + ); + + Zotero.debug('======'); + Zotero.debug(json); + + libraryVersion = json.libraryVersion; + + // Mark successful and unchanged objects as synced with new version, + // and save uploaded JSON to cache + let ids = []; + let toSave = []; + let toCache = []; + for (let state of ['successful', 'unchanged']) { + for (let index in json.results[state]) { + let current = json.results[state][index]; + // 'successful' includes objects, not keys + let key = state == 'successful' ? current.key : current; + + if (key != batch[index].key) { + throw new Error("Key mismatch (" + key + " != " + batch[index].key + ")"); + } + + let obj = yield objectsClass.getByLibraryAndKeyAsync( + this.libraryID, key, { noCache: true } + ) + ids.push(obj.id); + + if (state == 'successful') { + // Update local object with saved data if necessary + yield obj.loadAllData(); + obj.fromJSON(current.data); + toSave.push(obj); + toCache.push(current); + } + else { + let j = yield obj.toJSON(); + j.version = json.libraryVersion; + toCache.push(j); + } + + numSuccessful++; + // Remove from batch to mark as successful + delete batch[index]; + } + } + yield Zotero.Sync.Data.Local.saveCacheObjects( + objectType, this.libraryID, toCache + ); + yield Zotero.DB.executeTransaction(function* () { + for (let i = 0; i < toSave.length; i++) { + yield toSave[i].save(); + } + this.library.libraryVersion = json.libraryVersion; + yield this.library.save(); + objectsClass.updateVersion(ids, json.libraryVersion); + objectsClass.updateSynced(ids, true); + }.bind(this)); + + // Handle failed objects + for (let index in json.results.failed) { + let { code, message } = json.results.failed[index]; + e = new Error(message); + e.name = "ZoteroUploadObjectError"; + e.code = code; + Zotero.logError(e); + + // This shouldn't happen, because the upload request includes a library + // version and should prevent an outdated upload before the object version is + // checked. If it does, we need to do a full sync. + if (e.code == 412) { + return this.UPLOAD_RESULT_OBJECT_CONFLICT; + } + + if (this.onError) { + this.onError(e); + } + if (this.stopOnError) { + throw new Error(e); + } + batch[index].tries++; + // Mark 400 errors as permanently failed + if (e.code >= 400 && e.code < 500) { + batch[index].failed = true; + } + // 500 errors should stay in queue and be retried + } + + // Add failed objects back to end of queue + var numFailed = 0; + for (let o of batch) { + if (o !== undefined) { + queue.push(o); + // TODO: Clear JSON? + numFailed++; + } + } + Zotero.debug("Failed: " + numFailed, 2); + } + catch (e) { + if (e instanceof Zotero.HTTP.UnexpectedStatusException) { + if (e.status == 412) { + return this.UPLOAD_RESULT_LIBRARY_CONFLICT; + } + + // 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; } - if (!o.json) { - o.json = yield this._getJSONForObject(objectType, o.id); - } - batch.push(o.json); } + throw e; + } + // If we didn't make any progress, bail + if (!numSuccessful) { + throw new Error("Made no progress during upload -- stopping"); + } + } + Zotero.debug("Done uploading " + objectTypePlural + " in library " + this.libraryID); + + return libraryVersion; +}) + + +Zotero.Sync.Data.Engine.prototype._uploadDeletions = Zotero.Promise.coroutine(function* (objectType, keys, libraryVersion) { + let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType); + let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); + + let failureDelayGenerator = null; + + while (keys.length) { + try { + let batch = keys.slice(0, this.uploadDeletionBatchSize); + libraryVersion = yield this.apiClient.uploadDeletions( + this.libraryType, + this.libraryTypeID, + libraryVersion, + objectType, + batch + ); + keys.splice(0, batch.length); - // No more non-failed requests - if (!batch.length) { - break; - } + // Update library version + this.library.libraryVersion = libraryVersion; + yield this.library.saveTx(); - // Remove selected and skipped objects from queue - queue.splice(0, batch.length); - - Zotero.debug("UPLOAD BATCH:"); - Zotero.debug(batch); - - let numSuccessful = 0; - try { - let json = yield this.apiClient.uploadObjects( - this.libraryType, - this.libraryTypeID, - objectType, - "POST", - libraryVersion, - batch - ); - - Zotero.debug('======'); - Zotero.debug(json); - - libraryVersion = json.libraryVersion; - - // Mark successful and unchanged objects as synced with new version, - // and save uploaded JSON to cache - let ids = []; - let toSave = []; - let toCache = []; - for (let state of ['successful', 'unchanged']) { - for (let index in json.results[state]) { - let current = json.results[state][index]; - // 'successful' includes objects, not keys - let key = state == 'successful' ? current.key : current; - - if (key != batch[index].key) { - throw new Error("Key mismatch (" + key + " != " + batch[index].key + ")"); - } - - let obj = yield objectsClass.getByLibraryAndKeyAsync( - this.libraryID, key, { noCache: true } - ) - ids.push(obj.id); - - if (state == 'successful') { - // Update local object with saved data if necessary - yield obj.loadAllData(); - obj.fromJSON(current.data); - toSave.push(obj); - toCache.push(current); - } - else { - let j = yield obj.toJSON(); - j.version = json.libraryVersion; - toCache.push(j); - } - - numSuccessful++; - // Remove from batch to mark as successful - delete batch[index]; - } + // Remove successful deletions from delete log + yield Zotero.Sync.Data.Local.removeObjectsFromDeleteLog( + objectType, this.libraryID, batch + ); + } + catch (e) { + if (e instanceof Zotero.HTTP.UnexpectedStatusException) { + if (e.status == 412) { + return this.UPLOAD_RESULT_LIBRARY_CONFLICT; } - yield Zotero.Sync.Data.Local.saveCacheObjects( - objectType, this.libraryID, toCache - ); - yield Zotero.DB.executeTransaction(function* () { - for (let i = 0; i < toSave.length; i++) { - yield toSave[i].save(); - } - this.library.libraryVersion = json.libraryVersion; - yield this.library.save(); - objectsClass.updateVersion(ids, json.libraryVersion); - objectsClass.updateSynced(ids, true); - }.bind(this)); - // Handle failed objects - for (let index in json.results.failed) { - let { code, message } = json.results.failed[index]; - e = new Error(message); - e.name = "ZoteroUploadObjectError"; - e.code = code; - Zotero.logError(e); - - // This shouldn't happen, because the upload request includes a library - // version and should prevent an outdated upload before the object version is - // checked. If it does, we need to do a full sync. - if (e.code == 412) { - return this.UPLOAD_RESULT_OBJECT_CONFLICT; - } - + // On 5xx, delay and retry + if (e.status >= 500 && e.status <= 600) { if (this.onError) { this.onError(e); } if (this.stopOnError) { throw new Error(e); } - batch[index].tries++; - // Mark 400 errors as permanently failed - if (e.code >= 400 && e.code < 500) { - batch[index].failed = true; - } - // 500 errors should stay in queue and be retried - } - - // Add failed objects back to end of queue - Zotero.debug("ADDING BACK FAILED"); - Zotero.debug(batch); - var numFailed = 0; - for (let o of batch) { - if (o !== undefined) { - queue.push(o); - // TODO: Clear JSON? - numFailed++; - } - } - Zotero.debug(queue); - Zotero.debug("Failed: " + numFailed, 2); - } - catch (e) { - if (e instanceof Zotero.HTTP.UnexpectedStatusException) { - if (e.status == 412) { - return this.UPLOAD_RESULT_LIBRARY_CONFLICT; - } - // 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; + 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; - } - // If we didn't make any progress, bail - if (!numSuccessful) { - throw new Error("Made no progress during upload -- stopping"); } + throw e; } - Zotero.debug("Done uploading " + objectTypePlural + " in library " + this.libraryID); } + Zotero.debug(`Done uploading ${objectType} deletions in ${this.libraryName}`); - return this.UPLOAD_RESULT_SUCCESS; + return libraryVersion; }); @@ -1037,7 +1135,7 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function* let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); let toDownload = []; let cacheVersions = yield Zotero.Sync.Data.Local.getLatestCacheObjectVersions( - this.libraryID, objectType + objectType, this.libraryID ); // Queue objects that are out of date or don't exist locally for (let key in results.versions) { @@ -1082,7 +1180,7 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function* } // Mark synced objects that don't exist remotely as unsynced - let syncedKeys = yield Zotero.Sync.Data.Local.getSynced(this.libraryID, objectType); + let syncedKeys = yield Zotero.Sync.Data.Local.getSynced(objectType, this.libraryID); let remoteMissing = Zotero.Utilities.arrayDiff(syncedKeys, Object.keys(results.versions)); if (remoteMissing.length) { Zotero.debug("Checking remotely missing synced " + objectTypePlural); diff --git a/chrome/content/zotero/xpcom/sync/syncEventListeners.js b/chrome/content/zotero/xpcom/sync/syncEventListeners.js index 82f8ecbfb..850595872 100644 --- a/chrome/content/zotero/xpcom/sync/syncEventListeners.js +++ b/chrome/content/zotero/xpcom/sync/syncEventListeners.js @@ -37,9 +37,9 @@ Zotero.Sync.EventListeners.ChangeListener = new function () { return; } - var syncSQL = "REPLACE INTO syncDeleteLog (syncObjectTypeID, libraryID, key, synced) " - + "VALUES (?, ?, ?, 0)"; - var storageSQL = "REPLACE INTO storageDeleteLog VALUES (?, ?, 0)"; + var syncSQL = "REPLACE INTO syncDeleteLog (syncObjectTypeID, libraryID, key) " + + "VALUES (?, ?, ?)"; + var storageSQL = "REPLACE INTO storageDeleteLog (libraryID, key) VALUES (?, ?)"; var storageForLibrary = {}; diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 45a2a6e5f..d052ed3bb 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -134,10 +134,11 @@ Zotero.Sync.Data.Local = { /** + * @param {String} objectType * @param {Integer} libraryID * @return {Promise} - A promise for an array of object keys */ - getSynced: function (libraryID, objectType) { + getSynced: function (objectType, libraryID) { var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); var sql = "SELECT key FROM " + objectsClass.table + " WHERE libraryID=? AND synced=1"; return Zotero.DB.columnQueryAsync(sql, [libraryID]); @@ -145,10 +146,11 @@ Zotero.Sync.Data.Local = { /** + * @param {String} objectType * @param {Integer} libraryID * @return {Promise} - A promise for an array of object ids */ - getUnsynced: Zotero.Promise.coroutine(function* (libraryID, objectType) { + getUnsynced: Zotero.Promise.coroutine(function* (objectType, libraryID) { var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); var sql = "SELECT " + objectsClass.idColumn + " FROM " + objectsClass.table + " WHERE libraryID=? AND synced=0"; @@ -170,7 +172,7 @@ Zotero.Sync.Data.Local = { * @return {Promise} - A promise for an object with object keys as keys and versions * as properties */ - getLatestCacheObjectVersions: Zotero.Promise.coroutine(function* (libraryID, objectType) { + getLatestCacheObjectVersions: Zotero.Promise.coroutine(function* (objectType, libraryID) { var sql = "SELECT key, version FROM syncCache WHERE libraryID=? AND " + "syncObjectTypeID IN (SELECT syncObjectTypeID FROM " + "syncObjectTypes WHERE name=?) ORDER BY version"; @@ -494,10 +496,10 @@ Zotero.Sync.Data.Local = { // Auto-restore some locally deleted objects that have changed remotely case 'collection': case 'search': - yield this._removeObjectFromDeleteLog( + yield this.removeObjectsFromDeleteLog( objectType, libraryID, - objectKey + [objectKey] ); throw new Error("Unimplemented"); @@ -1014,12 +1016,33 @@ Zotero.Sync.Data.Local = { }), + /** + * @return {Promise} - Promise for array of keys + */ + getDeleted: Zotero.Promise.coroutine(function* (objectType, libraryID) { + var syncObjectTypeID = Zotero.Sync.Data.Utilities.getSyncObjectTypeID(objectType); + var sql = "SELECT key FROM syncDeleteLog WHERE libraryID=? AND syncObjectTypeID=?"; + return Zotero.DB.columnQueryAsync(sql, [libraryID, syncObjectTypeID]); + }), + + /** * @return {Promise} */ - _removeObjectFromDeleteLog: function (objectType, libraryID, key) { + removeObjectsFromDeleteLog: function (objectType, libraryID, keys) { var syncObjectTypeID = Zotero.Sync.Data.Utilities.getSyncObjectTypeID(objectType); - var sql = "DELETE FROM syncDeleteLog WHERE libraryID=? AND key=? AND syncObjectTypeID=?"; - return Zotero.DB.queryAsync(sql, [libraryID, key, syncObjectTypeID]); + var sql = "DELETE FROM syncDeleteLog WHERE libraryID=? AND syncObjectTypeID=? AND key IN ("; + return Zotero.DB.executeTransaction(function* () { + return Zotero.Utilities.Internal.forEachChunkAsync( + keys, + Zotero.DB.MAX_BOUND_PARAMETERS - 2, + Zotero.Promise.coroutine(function* (chunk) { + var params = [libraryID, syncObjectTypeID].concat(chunk); + return Zotero.DB.queryAsync( + sql + Array(chunk.length).fill('?').join(',') + ")", params + ); + }) + ); + }.bind(this)); } } diff --git a/resource/schema/userdata.sql b/resource/schema/userdata.sql index 3a47ac67d..6b6ede094 100644 --- a/resource/schema/userdata.sql +++ b/resource/schema/userdata.sql @@ -330,22 +330,18 @@ CREATE TABLE syncDeleteLog ( libraryID INT NOT NULL, key TEXT NOT NULL, dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, - synced INT NOT NULL DEFAULT 0, UNIQUE (syncObjectTypeID, libraryID, key), FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID), FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE ); -CREATE INDEX syncDeleteLog_synced ON syncDeleteLog(synced); CREATE TABLE storageDeleteLog ( libraryID INT NOT NULL, key TEXT NOT NULL, dateDeleted TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, - synced INT NOT NULL DEFAULT 0, PRIMARY KEY (libraryID, key), FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE ); -CREATE INDEX storageDeleteLog_synced ON storageDeleteLog(synced); CREATE TABLE annotations ( annotationID INTEGER PRIMARY KEY, diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 80ac18970..5150fcf87 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -310,7 +310,7 @@ describe("Zotero.Sync.Data.Engine", function () { 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 - assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0); + assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(type, libraryID)), 0); let key = objects[type][0].key; let version = objects[type][0].version; assert.equal(version, objectVersions[type][key]); @@ -391,7 +391,7 @@ describe("Zotero.Sync.Data.Engine", function () { 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 - assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0); + assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(type, libraryID)), 0); let o = objects[type][0]; let key = o.key; let version = o.version; @@ -475,7 +475,7 @@ describe("Zotero.Sync.Data.Engine", function () { assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion); for (let type of types) { // Make sure local objects were updated with new metadata and marked as synced - assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0); + assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(type, libraryID)), 0); let o = objects[type][0]; let key = o.key; let version = o.version; @@ -490,6 +490,66 @@ describe("Zotero.Sync.Data.Engine", function () { } }) + it("should upload local deletions", function* () { + var { engine, client, caller } = yield setup(); + var libraryID = Zotero.Libraries.userLibraryID; + var lastLibraryVersion = 5; + yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion); + + var types = Zotero.DataObjectUtilities.getTypes(); + var objects = {}; + for (let type of types) { + let obj1 = yield createDataObject(type); + let obj2 = yield createDataObject(type); + objects[type] = [obj1.key, obj2.key]; + yield obj1.eraseTx(); + yield obj2.eraseTx(); + } + + var count = types.length; + + server.respond(function (req) { + if (req.method == "DELETE") { + assert.equal( + req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion + ); + + // TODO: Settings? + + // Data objects + for (let type of types) { + let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type); + if (req.url.startsWith(baseURL + "users/1/" + typePlural)) { + let matches = req.url.match(new RegExp("\\?" + type + "Key=(.+)")); + let keys = decodeURIComponent(matches[1]).split(','); + assert.sameMembers(keys, objects[type]); + req.respond( + 204, + { + "Last-Modified-Version": ++lastLibraryVersion + } + ); + count--; + return; + } + } + } + }) + + yield engine.start(); + + assert.equal(count, 0); + for (let type of types) { + yield assert.eventually.lengthOf( + Zotero.Sync.Data.Local.getDeleted(type, libraryID), 0 + ); + } + assert.equal( + Zotero.Libraries.get(libraryID).libraryVersion, + lastLibraryVersion + ); + }) + it("should make only one request if in sync", function* () { yield Zotero.Libraries.setVersion(Zotero.Libraries.userLibraryID, 5); ({ engine, client, caller } = yield setup()); @@ -911,10 +971,10 @@ describe("Zotero.Sync.Data.Engine", function () { // Objects 1 should be marked as synced, with versions from the server // Objects 2 should be marked as unsynced for (let type of types) { - var synced = yield Zotero.Sync.Data.Local.getSynced(userLibraryID, type); + var synced = yield Zotero.Sync.Data.Local.getSynced(type, userLibraryID); assert.deepEqual(synced, [objects[type][0].key]); assert.equal(objects[type][0].version, 10); - var unsynced = yield Zotero.Sync.Data.Local.getUnsynced(userLibraryID, type); + var unsynced = yield Zotero.Sync.Data.Local.getUnsynced(type, userLibraryID); assert.deepEqual(unsynced, [objects[type][1].id]); assert.equal(versionResults[type].libraryVersion, headers["Last-Modified-Version"]);