www

commit fe88530120f6834ca7e8babe4e154d69cb1967ca
parent 2db41b0adc136df9b026a0890dd31409ec91505d
Author: Dan Stillman <dstillman@zotero.org>
Date:   Fri, 28 Apr 2017 03:18:50 -0400

Recover from a remotely missing parent collection

Diffstat:
M
chrome/content/zotero/xpcom/sync/syncEngine.js
 | 
69
+++++++++++++++++++++++++++++++++++++++++----------------------------
M
test/tests/syncEngineTest.js
 | 
52
++++++++++++++++++++++++++++++++++++++++++++++++++++

2 files changed, 93 insertions(+), 28 deletions(-)
diff --git 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
@@ -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());