diff --git a/chrome/content/zotero/xpcom/data/collections.js b/chrome/content/zotero/xpcom/data/collections.js index 3fa2920b6b..857161b7d4 100644 --- a/chrome/content/zotero/xpcom/data/collections.js +++ b/chrome/content/zotero/xpcom/data/collections.js @@ -179,12 +179,24 @@ Zotero.Collections = function() { let tree = [ids[0]]; let keep = [ids[0]]; let id = ids.shift(); + let seen = new Set([id]); while (true) { let c = Zotero.Collections.get(id); let parentID = c.parentID; if (!parentID) { break; } + // Avoid an infinite loop if collections are incorrectly nested within each other + if (seen.has(parentID)) { + throw new Zotero.Error( + "Incorrectly nested collections", + Zotero.Error.ERROR_INVALID_COLLECTION_NESTING, + { + collectionID: id + } + ); + } + seen.add(parentID); tree.push(parentID); // If parent is in list, remove it let pos = ids.indexOf(parentID); diff --git a/chrome/content/zotero/xpcom/error.js b/chrome/content/zotero/xpcom/error.js index d1305676d2..3c6471eef9 100644 --- a/chrome/content/zotero/xpcom/error.js +++ b/chrome/content/zotero/xpcom/error.js @@ -51,6 +51,7 @@ Zotero.Error.ERROR_ZFS_UPLOAD_QUEUE_LIMIT = 6; Zotero.Error.ERROR_ZFS_FILE_EDITING_DENIED = 7; Zotero.Error.ERROR_INVALID_ITEM_TYPE = 8; Zotero.Error.ERROR_USER_NOT_AVAILABLE = 9; +Zotero.Error.ERROR_INVALID_COLLECTION_NESTING = 10; //Zotero.Error.ERROR_SYNC_EMPTY_RESPONSE_FROM_SERVER = 6; //Zotero.Error.ERROR_SYNC_INVALID_RESPONSE_FROM_SERVER = 7; diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index f0b212862b..decffc7e6d 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -542,7 +542,23 @@ Zotero.Sync.Data.Local = { // Sort descendent collections last if (objectType == 'collection') { - ids = Zotero.Collections.sortByLevel(ids); + try { + ids = Zotero.Collections.sortByLevel(ids); + } + catch (e) { + Zotero.logError(e); + // If collections were incorrectly nested, fix and try again + if (e instanceof Zotero.Error && e.error == Zotero.Error.ERROR_INVALID_COLLECTION_NESTING) { + let c = Zotero.Collections.get(e.collectionID); + Zotero.debug(`Removing parent collection ${c.parentKey} from collection ${c.key}`); + c.parentID = null; + yield c.saveTx(); + return this.getUnsynced(...arguments); + } + else { + throw e; + } + } } return ids; diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js index 60e42fa478..3e95250009 100644 --- a/test/tests/syncLocalTest.js +++ b/test/tests/syncLocalTest.js @@ -466,6 +466,33 @@ describe("Zotero.Sync.Data.Local", function() { }) + describe("#getUnsynced()", function () { + it("should correct incorrectly nested collections", async function () { + var c1 = await createDataObject('collection'); + var c2 = await createDataObject('collection'); + + c1.parentID = c2.id; + await c1.saveTx(); + + await Zotero.DB.queryAsync( + "UPDATE collections SET parentCollectionID=? WHERE collectionID=?", + [ + c1.id, + c2.id + ] + ); + await c2.reload(['primaryData'], true); + + var ids = await Zotero.Sync.Data.Local.getUnsynced('collection', Zotero.Libraries.userLibraryID); + + // One of the items should still be the parent of the other, though which one is undefined + assert.isTrue( + (c1.parentID == c2.id && !c2.parentID) || (c2.parentID == c1.id && !c1.parentID) + ); + }); + }); + + describe("#processObjectsFromJSON()", function () { var types = Zotero.DataObjectUtilities.getTypes();