mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Fix invalid collection nesting in DB integrity check

Browse source 
It shouldn't be possible to nest two collections inside each other, but
if it happens, fix it in the integrity check.

Also detect it from CollectionTreeView::expandToCollection() (used when
showing the collections containing an item) and crash Zotero with a flag
to run an integrity check after restart. Previously, this would result
in an infinite loop.

This may be the cause of some of the collection disappearances people
have reported. If parentCollectionID never leads to a null, the
collection won't appear anywhere in the tree.

TODO:

- Figure out how this is happening
- Detect and fix it automatically for people it's happened to
This commit is contained in:
Dan Stillman 2020-03-13 18:04:04 -04:00
parent 81739c7a66
commit 0679809735
3 changed files with 132 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
chrome/content/zotero/xpcom/collectionTreeView.js
View file

@ -1039,7 +1039,15 @@ Zotero.CollectionTreeView.prototype.expandToCollection = Zotero.Promise.coroutin
}
var path = [];
var parentID;
var seen = new Set([col.id])
while (parentID = col.parentID) {
// Detect infinite loop due to invalid nesting in DB
if (seen.has(parentID)) {
yield Zotero.Schema.requireIntegrityCheck();
Zotero.crash();
return;
}
seen.add(parentID);
path.unshift(parentID);
col = yield Zotero.Collections.getAsync(parentID);
}

104
chrome/content/zotero/xpcom/schema.js
View file

@ -139,9 +139,7 @@ Zotero.Schema = new function(){
}
// Check if DB is coming from the DB Repair Tool and should be checked
var integrityCheck = await Zotero.DB.valueQueryAsync(
"SELECT value FROM settings WHERE setting='db' AND key='integrityCheck'"
);
var integrityCheck = await this.integrityCheckRequired();
// Check whether bundled global schema file is newer than DB
var bundledGlobalSchema = await _readGlobalSchemaFromFile();
@ -193,12 +191,9 @@ Zotero.Schema = new function(){
await _updateCustomTables();
}
// Auto-repair databases coming from the DB Repair Tool
// Auto-repair databases flagged for repair or coming from the DB Repair Tool
if (integrityCheck) {
await this.integrityCheck(true);
await Zotero.DB.queryAsync(
"DELETE FROM settings WHERE setting='db' AND key='integrityCheck'"
);
}
updated = await _migrateUserDataSchema(userdata, options);
@ -1664,6 +1659,25 @@ Zotero.Schema = new function(){
});
this.integrityCheckRequired = async function () {
return !!await Zotero.DB.valueQueryAsync(
"SELECT value FROM settings WHERE setting='db' AND key='integrityCheck'"
);
};
this.setIntegrityCheckRequired = async function (required) {
var sql;
if (required) {
sql = "REPLACE INTO settings VALUES ('db', 'integrityCheck', 1)";
}
else {
sql = "DELETE FROM settings WHERE setting='db' AND key='integrityCheck'";
}
await Zotero.DB.queryAsync(sql);
};
this.integrityCheck = Zotero.Promise.coroutine(function* (fix) {
Zotero.debug("Checking database integrity");
@ -1787,6 +1801,74 @@ Zotero.Schema = new function(){
let userID = await Zotero.DB.valueQueryAsync("SELECT value FROM settings WHERE setting='account' AND key='userID'");
await Zotero.DB.queryAsync("UPDATE settings SET value=? WHERE setting='account' AND key='userID'", parseInt(userID.trim()));
}
],
// Invalid collections nesting
[
async function () {
let rows = await Zotero.DB.queryAsync(
"SELECT collectionID, parentCollectionID FROM collections"
);
let map = new Map();
let ids = [];
for (let row of rows) {
map.set(row.collectionID, row.parentCollectionID);
ids.push(row.collectionID);
}
for (let id of ids) {
// Keep track of collections we've seen
let seen = new Set([id]);
while (true) {
let parent = map.get(id);
if (!parent) {
break;
}
if (seen.has(parent)) {
Zotero.debug(`Collection ${id} parent ${parent} was already seen`, 2);
return true;
}
seen.add(parent);
id = parent;
}
}
return false;
},
async function () {
let fix = async function () {
let rows = await Zotero.DB.queryAsync(
"SELECT collectionID, parentCollectionID FROM collections"
);
let map = new Map();
let ids = [];
for (let row of rows) {
map.set(row.collectionID, row.parentCollectionID);
ids.push(row.collectionID);
}
for (let id of ids) {
let seen = new Set([id]);
while (true) {
let parent = map.get(id);
if (!parent) {
break;
}
if (seen.has(parent)) {
await Zotero.DB.queryAsync(
"UPDATE collections SET parentCollectionID = NULL "
+ "WHERE collectionID = ?",
id
);
// Restart
return true;
}
seen.add(parent);
id = parent;
}
}
// Done
return false;
};
while (await fix()) {}
}
]
];
@ -1826,12 +1908,20 @@ Zotero.Schema = new function(){
}
catch (e) {
Zotero.logError(e);
// Clear flag on failure, to avoid showing an error on every startup if someone
// doesn't know how to deal with it
await setIntegrityCheckRequired(false);
}
}
return false;
}
// Clear flag on success
if (fix) {
await setIntegrityCheckRequired(false);
}
return true;
});

27
test/tests/schemaTest.js
View file

@ -97,5 +97,32 @@ describe("Zotero.Schema", function() {
yield assert.eventually.isTrue(Zotero.Schema.integrityCheck(true));
yield assert.eventually.isTrue(Zotero.Schema.integrityCheck());
})
it("should repair invalid nesting between two collections", async function () {
var c1 = await createDataObject('collection');
var c2 = await createDataObject('collection', { parentID: c1.id });
await Zotero.DB.queryAsync(
"UPDATE collections SET parentCollectionID=? WHERE collectionID=?",
[c2.id, c1.id]
);
await assert.isFalse(await Zotero.Schema.integrityCheck());
await assert.isTrue(await Zotero.Schema.integrityCheck(true));
await assert.isTrue(await Zotero.Schema.integrityCheck());
});
it("should repair invalid nesting between three collections", async function () {
var c1 = await createDataObject('collection');
var c2 = await createDataObject('collection', { parentID: c1.id });
var c3 = await createDataObject('collection', { parentID: c2.id });
await Zotero.DB.queryAsync(
"UPDATE collections SET parentCollectionID=? WHERE collectionID=?",
[c3.id, c2.id]
);
await assert.isFalse(await Zotero.Schema.integrityCheck());
await assert.isTrue(await Zotero.Schema.integrityCheck(true));
await assert.isTrue(await Zotero.Schema.integrityCheck());
});
})
})