diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 14043fd48..fc97b4b65 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -794,6 +794,7 @@ Zotero.Sync.Data.Local = { // Skip objects with unmet dependencies if (objectType == 'item' || objectType == 'collection') { + // Missing parent collection or item let parentProp = 'parent' + objectType[0].toUpperCase() + objectType.substr(1); let parentKey = jsonData[parentProp]; if (parentKey) { @@ -815,17 +816,37 @@ Zotero.Sync.Data.Local = { } } - /*if (objectType == 'item') { - for (let j = 0; j < jsonData.collections.length; i++) { - let parentKey = jsonData.collections[j]; - let parentCollection = Zotero.Collections.getByLibraryAndKey( - libraryID, parentKey, { noCache: true } - ); - if (!parentCollection) { - // ??? + // Missing collection -- this could happen if the collection was deleted + // locally and an item in it was modified remotely + if (objectType == 'item' && jsonData.collections) { + let error; + for (let key of jsonData.collections) { + let collection = Zotero.Collections.getByLibraryAndKey(libraryID, key); + if (!collection) { + error = new Error(`Collection ${libraryID}/${key} not found ` + + `-- skipping item`); + error.name = "ZoteroMissingObjectError"; + Zotero.debug(error.message); + results.push({ + key: objectKey, + processed: false, + error, + retry: false + }); + + // If the collection is in the delete log, the deletion will upload + // after downloads are done. Otherwise, we somehow missed + // downloading it and should add it to the queue to try again. + if (!(yield this.getDateDeleted('collection', libraryID, key))) { + yield this.addObjectsToSyncQueue('collection', libraryID, [key]); + } + break; } } - }*/ + if (error) { + continue; + } + } } // Errors have to be thrown in order to roll back the transaction, so catch those here diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 770712daa..e8ee8036e 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -2112,23 +2112,13 @@ describe("Zotero.Sync.Data.Engine", function () { var library = Zotero.Libraries.userLibrary; library.libraryVersion = lastLibraryVersion; await library.saveTx(); - ({ engine, client, caller } = await setup({ - stopOnError: false - })); + ({ engine, client, caller } = await setup()); // 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 itemKey = "AAAAAAAA"; var headers = { "Last-Modified-Version": newLibraryVersion @@ -2151,7 +2141,14 @@ describe("Zotero.Sync.Data.Engine", function () { url: `users/1/items?format=json&itemKey=${itemKey}&includeTrashed=1`, status: 200, headers, - json: [itemResponseJSON] + json: [ + makeItemJSON({ + key: itemKey, + version: newLibraryVersion, + itemType: "book", + collections: [collectionKey] + }) + ] }); await engine._startDownload(); @@ -2159,6 +2156,67 @@ describe("Zotero.Sync.Data.Engine", function () { // 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]); + + // Collection should not be in sync queue + assert.lengthOf( + await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('collection', library.id), 0 + ); + }); + + + it("should handle new remote item referencing locally missing 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()); + + var collectionKey = 'AAAAAAAA'; + var itemKey = 'BBBBBBBB' + + 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: [ + makeItemJSON({ + key: itemKey, + version: newLibraryVersion, + itemType: "book", + collections: [collectionKey] + }) + ] + }); + + await engine._startDownload(); + + // Item should be skipped and added to queue + var keys = await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', library.id); + assert.sameMembers(keys, [itemKey]); + + // Collection should be in queue + assert.sameMembers( + await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('collection', library.id), + [collectionKey] + ); }); @@ -2168,9 +2226,7 @@ describe("Zotero.Sync.Data.Engine", function () { var library = Zotero.Libraries.userLibrary; library.libraryVersion = lastLibraryVersion; await library.saveTx(); - ({ engine, client, caller } = await setup({ - stopOnError: false - })); + ({ engine, client, caller } = await setup()); // Create local deleted collection and item var collection = await createDataObject('collection'); @@ -2207,20 +2263,6 @@ describe("Zotero.Sync.Data.Engine", function () { 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