Mark local collection as unsynced if missing remotely in item request

We should figure out when this happens, but in the meantime, recover from it if it does.
Juris-M · Apr 7, 2017 · d8fed09 · d8fed09
1 parent 172f36d
commit d8fed09
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 25 deletions.
diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -1113,29 +1113,42 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func
 				Zotero.logError("Error for " + objectType + " " + jsonBatch[index].key + " in "
 					+ this.library.name + ":\n\n" + e);
 
-				// If parent item is missing remotely and it isn't in the queue (which shouldn't happen),
-				// mark it as unsynced and add to queue
-				if (e.code == 400 && objectType == 'item' && data && 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;
+				// 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`);
+							}
+							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`);
+							}
+
+							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
@@ -2177,6 +2177,57 @@ describe("Zotero.Sync.Data.Engine", function () {
 		});
 
 
+		it("should mark local collection as unsynced if it doesn't exist when uploading item", 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 collection = createUnsavedDataObject('collection');
+			// Set the collection as synced (though this shouldn't happen)
+			collection.synced = true;
+			yield collection.saveTx();
+			var item = yield createDataObject('item', { collections: [collection.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, item.key);
+					req.respond(
+						200,
+						{
+							"Last-Modified-Version": lastLibraryVersion
+						},
+						JSON.stringify({
+							successful: {},
+							unchanged: {},
+							failed: {
+								0: {
+									code: 409,
+									message: `Collection ${collection.key} doesn't exist`,
+									data: {
+										collection: collection.key
+									}
+								}
+							}
+						})
+					);
+				}
+				called++;
+			});
+
+			var e = yield getPromiseError(engine._startUpload());
+			assert.ok(e);
+			assert.isFalse(collection.synced);
+		});
+
+
 		it("should mark local parent item as unsynced if it doesn't exist when uploading child", function* () {
 			({ engine, client, caller } = yield setup());
 
@@ -2209,8 +2260,8 @@ describe("Zotero.Sync.Data.Engine", function () {
 							unchanged: {},
 							failed: {
 								0: {
-									code: 400,
-									message: `Parent item '${item.key}' doesn't exist`,
+									code: 409,
+									message: `Parent item ${item.key} doesn't exist`,
 									data: {
 										parentItem: item.key
 									}