commit d81e2a5cf0d6612cf9a2fafaa8da48cbd2f7ea9f
parent 85d7c01c8582ba8ea7dad08c9813cb00d1b927e9
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 20 Sep 2017 05:32:44 -0400
Fix sync errors from remote item referencing deleted local collection
Diffstat:
5 files changed, 134 insertions(+), 5 deletions(-)
diff --git a/chrome/content/zotero/bindings/merge.xml b/chrome/content/zotero/bindings/merge.xml
@@ -380,10 +380,15 @@
// Store JSON
this._data = val;
+ // Create a copy of the JSON that we can clean for display, since the remote object
+ // might reference things that don't exist locally
+ var displayJSON = Object.assign({}, val);
+ displayJSON.collections = [];
+
// Create item from JSON for metadata box
var item = new Zotero.Item(val.itemType);
item.libraryID = this.libraryID;
- item.fromJSON(val);
+ item.fromJSON(displayJSON);
objbox.item = item;
]]>
</setter>
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
@@ -3505,7 +3505,7 @@ Zotero.Item.prototype.setCollections = function (collectionIDsOrKeys) {
var id = this.ContainerObjectsClass.getIDFromLibraryAndKey(this.libraryID, val);
if (!id) {
let e = new Error("Collection " + val + " not found for item " + this.libraryKey);
- e.name = "ZoteroObjectNotFoundError";
+ e.name = "ZoteroMissingObjectError";
throw e;
}
return id;
diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -694,9 +694,9 @@ Zotero.Sync.Data.Engine.prototype._downloadObjects = async function (objectType,
let results = await Zotero.Sync.Data.Local.processConflicts(
objectType, this.libraryID, conflicts, this._getOptions()
);
- // Keys can be unprocessed if conflict resolution is cancelled
let keys = results.filter(x => x.processed).map(x => x.key);
- if (!keys.length) {
+ // If all keys are unprocessed and didn't fail from an error, conflict resolution was cancelled
+ if (results.every(x => !x.processed && !x.error)) {
throw new Zotero.Sync.UserCancelledException();
}
await Zotero.Sync.Data.Local.removeObjectsFromSyncQueue(objectType, this.libraryID, keys);
diff --git a/chrome/content/zotero/xpcom/sync/syncExceptions.js b/chrome/content/zotero/xpcom/sync/syncExceptions.js
@@ -30,7 +30,7 @@
* sync completely
*/
Zotero.Sync.UserCancelledException = function (advanceToNextLibrary) {
- this.message = "Sync cancelled";
+ this.message = "User cancelled sync";
this.advanceToNextLibrary = advanceToNextLibrary;
}
diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js
@@ -2102,6 +2102,130 @@ describe("Zotero.Sync.Data.Engine", function () {
assert.isTrue(Zotero.Items.exists(itemID2));
})
+
+ it("should handle new remote item referencing locally deleted collection", async function () {
+ var lastLibraryVersion = 5;
+ var newLibraryVersion = 6;
+ var library = Zotero.Libraries.userLibrary;
+ library.libraryVersion = lastLibraryVersion;
+ await library.saveTx();
+ ({ engine, client, caller } = await setup({
+ stopOnError: false
+ }));
+
+ // Create local deleted collection
+ var collection = await createDataObject('collection');
+ var collectionKey = collection.key;
+ await collection.eraseTx();
+ // Create item JSON
+ var item = await createDataObject('item');
+ var itemResponseJSON = item.toResponseJSON();
+ // Add collection to remote item
+ itemResponseJSON.data.collections = [collectionKey];
+ var itemKey = item.key;
+ await item.eraseTx({
+ skipDeleteLog: true
+ });
+
+ var headers = {
+ "Last-Modified-Version": newLibraryVersion
+ };
+ setDefaultResponses({
+ lastLibraryVersion,
+ libraryVersion: newLibraryVersion
+ });
+ setResponse({
+ method: "GET",
+ url: `users/1/items?format=versions&since=${lastLibraryVersion}&includeTrashed=1`,
+ status: 200,
+ headers,
+ json: {
+ [itemKey]: newLibraryVersion
+ }
+ });
+ setResponse({
+ method: "GET",
+ url: `users/1/items?format=json&itemKey=${itemKey}&includeTrashed=1`,
+ status: 200,
+ headers,
+ json: [itemResponseJSON]
+ });
+
+ await engine._startDownload();
+
+ // Item should be skipped and added to queue, which will allow collection deletion to upload
+ var keys = await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', library.id);
+ assert.sameMembers(keys, [itemKey]);
+ });
+
+
+ it("should handle conflict with remote item referencing deleted local collection", async function () {
+ var lastLibraryVersion = 5;
+ var newLibraryVersion = 6;
+ var library = Zotero.Libraries.userLibrary;
+ library.libraryVersion = lastLibraryVersion;
+ await library.saveTx();
+ ({ engine, client, caller } = await setup({
+ stopOnError: false
+ }));
+
+ // Create local deleted collection and item
+ var collection = await createDataObject('collection');
+ var collectionKey = collection.key;
+ await collection.eraseTx();
+ var item = await createDataObject('item');
+ var itemResponseJSON = item.toResponseJSON();
+ // Add collection to remote item
+ itemResponseJSON.data.collections = [collectionKey];
+ var itemKey = item.key;
+ await item.eraseTx();
+
+ var headers = {
+ "Last-Modified-Version": newLibraryVersion
+ };
+ setDefaultResponses({
+ lastLibraryVersion,
+ libraryVersion: newLibraryVersion
+ });
+ setResponse({
+ method: "GET",
+ url: `users/1/items?format=versions&since=${lastLibraryVersion}&includeTrashed=1`,
+ status: 200,
+ headers,
+ json: {
+ [itemKey]: newLibraryVersion
+ }
+ });
+ setResponse({
+ method: "GET",
+ url: `users/1/items?format=json&itemKey=${itemKey}&includeTrashed=1`,
+ status: 200,
+ headers,
+ json: [itemResponseJSON]
+ });
+
+ // This will trigger a conflict for a remote item that references a collection that doesn't
+ // exist. The collection should be removed for display in the CR window, and then the save
+ // should fail and the item should be added to the queue so that the collection deletion
+ // can upload.
+ waitForWindow('chrome://zotero/content/merge.xul', function (dialog) {
+ var doc = dialog.document;
+ var wizard = doc.documentElement;
+ var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0];
+
+ // 1 (accept remote)
+ //assert.equal(mergeGroup.leftpane.getAttribute('selected'), 'true');
+ mergeGroup.rightpane.click();
+ wizard.getButton('finish').click();
+ });
+ await engine._startDownload();
+
+ // Item should be skipped and added to queue, which will allow collection deletion to upload
+ var keys = await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', library.id);
+ assert.sameMembers(keys, [itemKey]);
+ });
+
+
it("should handle cancellation of conflict resolution window", function* () {
var library = Zotero.Libraries.userLibrary;
library.libraryVersion = 5;
