Fix out-of-memory error syncing collections nested inside each other

It shouldn't be possible for collections to be nested this way, if it
happens, it shouldn't result in an infinite loop.

This removes one of the parent assignments at sync time.
This commit is contained in:
Dan Stillman 2019-11-26 15:30:43 -07:00
parent 47d8a54600
commit 8c7677a009
4 changed files with 57 additions and 1 deletions

View file

@ -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);

View file

@ -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;

View file

@ -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;

View file

@ -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();