diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 5f78979aff..2f5b603784 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -1118,37 +1118,50 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func // If an item's dependency is missing remotely and it isn't in the queue (which // shouldn't happen), mark it as unsynced if (e.code == 400 || e.code == 409) { - if (objectType == 'item' && data) { - if (data.collection) { - let collection = Zotero.Collections.getByLibraryAndKey(this.libraryID, data.collection); - if (!collection) { - throw new Error(`Item ${this.libraryID}/${jsonBatch[index].key} ` - + `references collection ${data.collection}, which doesn't exist`); + if (data) { + if (objectType == 'collection' && e.code == 409) { + if (data.collection) { + let collection = Zotero.Collections.getByLibraryAndKey(this.libraryID, data.collection); + if (!collection) { + throw new Error(`Collection ${this.libraryID}/${jsonBatch[index].key} ` + + `references parent collection ${data.collection}, which doesn't exist`); + } + Zotero.logError(`Marking collection ${data.collection} as unsynced`); + yield Zotero.Sync.Data.Local.markObjectAsUnsynced(collection); } - Zotero.logError(`Marking collection ${data.collection} as unsynced`); - yield Zotero.Sync.Data.Local.markObjectAsUnsynced(collection); } - else if (data.parentItem) { - let parentItem = Zotero.Items.getByLibraryAndKey(this.libraryID, data.parentItem); - if (!parentItem) { - throw new Error(`Item ${this.libraryID}/${jsonBatch[index].key} references parent ` - + `item ${data.parentItem}, which doesn't exist`); + else if (objectType == 'item') { + if (data.collection) { + let collection = Zotero.Collections.getByLibraryAndKey(this.libraryID, data.collection); + if (!collection) { + throw new Error(`Item ${this.libraryID}/${jsonBatch[index].key} ` + + `references collection ${data.collection}, which doesn't exist`); + } + Zotero.logError(`Marking collection ${data.collection} as unsynced`); + yield Zotero.Sync.Data.Local.markObjectAsUnsynced(collection); } - - let id = parentItem.id; - // If parent item isn't already in queue, mark it as unsynced and add it - if (!queue.find(o => o.id == id) && !batch.find(o => o.id == id)) { - yield Zotero.Sync.Data.Local.markObjectAsUnsynced(parentItem); - Zotero.logError(`Adding parent item ${data.parentItem} to upload queue`); - queue.push({ - id, - json: null, - tries: 0, - failed: false - }); - // Pretend that we were successful so syncing continues - numSuccessful++; - continue; + else if (data.parentItem) { + let parentItem = Zotero.Items.getByLibraryAndKey(this.libraryID, data.parentItem); + if (!parentItem) { + throw new Error(`Item ${this.libraryID}/${jsonBatch[index].key} references parent ` + + `item ${data.parentItem}, which doesn't exist`); + } + + let id = parentItem.id; + // If parent item isn't already in queue, mark it as unsynced and add it + if (!queue.find(o => o.id == id) && !batch.find(o => o.id == id)) { + yield Zotero.Sync.Data.Local.markObjectAsUnsynced(parentItem); + Zotero.logError(`Adding parent item ${data.parentItem} to upload queue`); + queue.push({ + id, + json: null, + tries: 0, + failed: false + }); + // Pretend that we were successful so syncing continues + numSuccessful++; + continue; + } } } } diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 76407cf4ba..3f6a889912 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -2177,6 +2177,58 @@ describe("Zotero.Sync.Data.Engine", function () { }); + // Note: This shouldn't be necessary, since collections are sorted top-down before uploading + it("should mark local collection as unsynced if it doesn't exist when uploading collection", function* () { + ({ engine, client, caller } = yield setup()); + + var library = Zotero.Libraries.userLibrary; + var libraryID = library.id; + var lastLibraryVersion = 5; + library.libraryVersion = lastLibraryVersion; + yield library.saveTx(); + + var collection1 = createUnsavedDataObject('collection'); + // Set the collection as synced (though this shouldn't happen) + collection1.synced = true; + yield collection1.saveTx(); + var collection2 = yield createDataObject('collection', { collections: [collection1.id] }); + + var called = 0; + server.respond(function (req) { + let requestJSON = JSON.parse(req.requestBody); + + if (called == 0) { + assert.lengthOf(requestJSON, 1); + assert.equal(requestJSON[0].key, collection2.key); + req.respond( + 200, + { + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({ + successful: {}, + unchanged: {}, + failed: { + 0: { + code: 409, + message: `Parent collection ${collection1.key} doesn't exist`, + data: { + collection: collection1.key + } + } + } + }) + ); + } + called++; + }); + + var e = yield getPromiseError(engine._startUpload()); + assert.ok(e); + assert.isFalse(collection1.synced); + }); + + it("should mark local collection as unsynced if it doesn't exist when uploading item", function* () { ({ engine, client, caller } = yield setup());