Schema integrity check improvements
- Create userdata tables and indexes that are missing - Delete tables and triggers that should no longer exist - Run schema integrity check before user data migration - Run schema integrity check after restart error This is meant to address two problems: 1) Database damage, and subsequent use of the DB Repair Tool, that results in missing tables 2) A small number of cases of schema update steps somehow not being reflected in users' databases despite their having updated userdata numbers, which are set within the same transaction. Until we figure out how that's happening, we should start adding conditional versions of schema update steps to the integrity check. This is currently only running the update check after a restart error, which might not occur for all missed schema update steps, so we might want other triggers for calling setIntegrityCheckRequired().
This commit is contained in:
Dan Stillman
parent
6bcc8af86b
commit
5b9e6497af
5 changed files with 155 additions and 43 deletions
|
|
@ -807,12 +807,15 @@ Zotero.DBConnection.prototype.columnExists = async function (table, column) {
|
|||
};
|
||||
|
||||
|
||||
/**
|
||||
* Parse SQL string and execute transaction with all statements
|
||||
*
|
||||
* @return {Promise}
|
||||
*/
|
||||
Zotero.DBConnection.prototype.executeSQLFile = Zotero.Promise.coroutine(function* (sql) {
|
||||
Zotero.DBConnection.prototype.indexExists = async function (index, db) {
|
||||
await this._getConnectionAsync();
|
||||
var prefix = db ? db + '.' : '';
|
||||
var sql = `SELECT COUNT(*) FROM ${prefix}sqlite_master WHERE type='index' AND name=?`;
|
||||
return !!await this.valueQueryAsync(sql, [index]);
|
||||
};
|
||||
|
||||
|
||||
Zotero.DBConnection.prototype.parseSQLFile = function (sql) {
|
||||
var nonCommentRE = /^[^-]/;
|
||||
var trailingCommentRE = /^(.*?)(?:--.+)?$/;
|
||||
|
||||
|
|
@ -830,13 +833,23 @@ Zotero.DBConnection.prototype.executeSQLFile = Zotero.Promise.coroutine(function
|
|||
var statements = sql.split(";")
|
||||
.map(x => x.replace(/TEMPSEMI/g, ";"));
|
||||
|
||||
return statements;
|
||||
};
|
||||
|
||||
|
||||
/**
|
||||
* Parse SQL string and execute transaction with all statements
|
||||
*
|
||||
* @return {Promise}
|
||||
*/
|
||||
Zotero.DBConnection.prototype.executeSQLFile = async function (sql) {
|
||||
this.requireTransaction();
|
||||
|
||||
var statements = this.parseSQLFile(sql);
|
||||
var statement;
|
||||
while (statement = statements.shift()) {
|
||||
yield this.queryAsync(statement);
|
||||
await this.queryAsync(statement);
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
|
||||
/*
|
||||
|
|
|
|||
|
|
@ -48,14 +48,6 @@ Zotero.Retractions = {
|
|||
return;
|
||||
}
|
||||
|
||||
// TEMP: Until we can figure out why some schema updates aren't going through despite the
|
||||
// version number being incremented, create table here if it's missing
|
||||
await Zotero.DB.queryAsync("CREATE TABLE IF NOT EXISTS retractedItems (\n itemID INTEGER PRIMARY KEY,\n data TEXT,\n FOREIGN KEY (itemID) REFERENCES items(itemID) ON DELETE CASCADE\n);");
|
||||
try {
|
||||
await Zotero.DB.queryAsync("ALTER TABLE retractedItems ADD COLUMN flag INT DEFAULT 0");
|
||||
}
|
||||
catch (e) {}
|
||||
|
||||
// Load mappings of keys (DOI hashes and PMIDs) to items and vice versa and register for
|
||||
// item changes so they can be kept up to date in notify().
|
||||
await this._cacheKeyMappings();
|
||||
|
|
|
|||
|
|
@ -194,6 +194,7 @@ Zotero.Schema = new function(){
|
|||
// Auto-repair databases flagged for repair or coming from the DB Repair Tool
|
||||
if (integrityCheck) {
|
||||
await this.integrityCheck(true);
|
||||
options.skipIntegrityCheck = true;
|
||||
}
|
||||
|
||||
updated = await _migrateUserDataSchema(userdata, options);
|
||||
|
|
@ -1740,27 +1741,10 @@ Zotero.Schema = new function(){
|
|||
// so that we don't try to wipe out all data
|
||||
if (!(yield Zotero.DB.valueQueryAsync("SELECT COUNT(*) FROM fieldsCombined"))
|
||||
|| !(yield Zotero.DB.valueQueryAsync("SELECT COUNT(*) FROM itemTypeFieldsCombined"))) {
|
||||
Zotero.logError("Combined field tables are empty -- skipping integrity check");
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check foreign keys
|
||||
var rows = yield Zotero.DB.queryAsync("PRAGMA foreign_key_check");
|
||||
if (rows.length && !fix) {
|
||||
let suffix1 = rows.length == 1 ? '' : 's';
|
||||
let suffix2 = rows.length == 1 ? 's' : '';
|
||||
Zotero.debug(`Found ${rows.length} row${suffix1} that violate${suffix2} foreign key constraints`, 1);
|
||||
return false;
|
||||
}
|
||||
// If fixing, delete rows that violate FK constraints
|
||||
for (let row of rows) {
|
||||
try {
|
||||
yield Zotero.DB.queryAsync(`DELETE FROM ${row.table} WHERE ROWID=?`, row.rowid);
|
||||
}
|
||||
catch (e) {
|
||||
Zotero.logError(e);
|
||||
}
|
||||
}
|
||||
|
||||
var attachmentID = parseInt(yield Zotero.DB.valueQueryAsync(
|
||||
"SELECT itemTypeID FROM itemTypes WHERE typeName='attachment'"
|
||||
));
|
||||
|
|
@ -1768,14 +1752,104 @@ Zotero.Schema = new function(){
|
|||
"SELECT itemTypeID FROM itemTypes WHERE typeName='note'"
|
||||
));
|
||||
|
||||
|
||||
// Non-foreign key checks
|
||||
//
|
||||
// The first position is for testing and the second is for repairing. Can be either SQL
|
||||
// statements or promise-returning functions. For statements, the repair entry can be either a
|
||||
// string or an array with multiple statements. Functions should avoid assuming any global state
|
||||
// (e.g., loaded data).
|
||||
// statements or promise-returning functions. For statements, the repair entry can be either
|
||||
// a string or an array with multiple statements. Check functions should return false if no
|
||||
// error, and either true or data to pass to the repair function on error. Functions should
|
||||
// avoid assuming any global state (e.g., loaded data).
|
||||
var checks = [
|
||||
// Create any tables or indexes that are missing and delete any tables or triggers that
|
||||
// still exist but should have been deleted
|
||||
[
|
||||
async function () {
|
||||
var statementsToRun = [];
|
||||
|
||||
// Get all existing tables, indexes, and triggers
|
||||
var sql = "SELECT "
|
||||
+ "CASE type "
|
||||
+ "WHEN 'table' THEN 'table:' || tbl_name "
|
||||
+ "WHEN 'index' THEN 'index:' || name "
|
||||
+ "WHEN 'trigger' THEN 'trigger:' || name "
|
||||
+ "END "
|
||||
+ "FROM sqlite_master WHERE type IN ('table', 'index', 'trigger')";
|
||||
var schema = new Set(await Zotero.DB.columnQueryAsync(sql));
|
||||
|
||||
// Check for deleted tables and triggers that still exist
|
||||
var deletedTables = [
|
||||
"transactionSets",
|
||||
"transactions",
|
||||
"transactionLog",
|
||||
];
|
||||
var deletedTriggers = [
|
||||
"insert_date_field",
|
||||
"update_date_field",
|
||||
"fki_itemAttachments",
|
||||
"fku_itemAttachments",
|
||||
"fki_itemNotes",
|
||||
"fku_itemNotes",
|
||||
];
|
||||
for (let table of deletedTables) {
|
||||
if (schema.has('table:' + table)) {
|
||||
statementsToRun.push("DROP TABLE " + table);
|
||||
}
|
||||
}
|
||||
for (let trigger of deletedTriggers) {
|
||||
if (schema.has('trigger:' + trigger)) {
|
||||
statementsToRun.push("DROP TRIGGER " + trigger);
|
||||
}
|
||||
}
|
||||
|
||||
// Check for missing tables and indexes
|
||||
var statements = await Zotero.DB.parseSQLFile(await _getSchemaSQL('userdata'));
|
||||
for (let statement of statements) {
|
||||
var matches = statement.match(/^CREATE TABLE\s+([^\s]+)/);
|
||||
if (matches) {
|
||||
let table = matches[1];
|
||||
if (!schema.has('table:' + table)) {
|
||||
Zotero.debug(`Table ${table} is missing`, 2);
|
||||
statementsToRun.push(statement);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
matches = statement.match(/^CREATE INDEX\s+([^\s]+)/);
|
||||
if (matches) {
|
||||
let index = matches[1];
|
||||
if (!schema.has('index:' + index)) {
|
||||
Zotero.debug(`Index ${index} is missing`, 2);
|
||||
statementsToRun.push(statement);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
return statementsToRun.length ? statementsToRun : false;
|
||||
},
|
||||
async function (statements) {
|
||||
for (let statement of statements) {
|
||||
await Zotero.DB.queryAsync(statement);
|
||||
}
|
||||
}
|
||||
],
|
||||
|
||||
// Foreign key checks
|
||||
[
|
||||
async function () {
|
||||
var rows = await Zotero.DB.queryAsync("PRAGMA foreign_key_check");
|
||||
if (!rows.length) return false;
|
||||
var suffix1 = rows.length == 1 ? '' : 's';
|
||||
var suffix2 = rows.length == 1 ? 's' : '';
|
||||
Zotero.debug(`Found ${rows.length} row${suffix1} that violate${suffix2} foreign key constraints`, 1);
|
||||
return rows;
|
||||
},
|
||||
// If fixing, delete rows that violate FK constraints
|
||||
async function (rows) {
|
||||
for (let row of rows) {
|
||||
await Zotero.DB.queryAsync(`DELETE FROM ${row.table} WHERE ROWID=?`, row.rowid);
|
||||
}
|
||||
}
|
||||
],
|
||||
|
||||
// Can't be a FK with itemTypesCombined
|
||||
[
|
||||
"SELECT COUNT(*) > 0 FROM items WHERE itemTypeID IS NULL",
|
||||
|
|
@ -1957,7 +2031,9 @@ Zotero.Schema = new function(){
|
|||
}
|
||||
// Function
|
||||
else {
|
||||
yield check[1]();
|
||||
// If data was provided by the check function, pass that to the fix function
|
||||
let checkData = typeof errorsFound != 'boolean' ? errorsFound : null;
|
||||
yield check[1](checkData);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
|
@ -2419,6 +2495,10 @@ Zotero.Schema = new function(){
|
|||
return false;
|
||||
}
|
||||
|
||||
if (!options.skipIntegrityCheck) {
|
||||
yield Zotero.Schema.integrityCheck(true);
|
||||
}
|
||||
|
||||
Zotero.debug('Updating user data tables from version ' + fromVersion + ' to ' + toVersion);
|
||||
|
||||
if (options.onBeforeUpdate) {
|
||||
|
|
@ -3029,7 +3109,6 @@ Zotero.Schema = new function(){
|
|||
}
|
||||
}
|
||||
|
||||
// Duplicated in retractions.js::init() due to undiagnosed schema update bug
|
||||
else if (i == 105) {
|
||||
// This was originally in 103 and then 104, but some schema update steps are being
|
||||
// missed for some people, so run again with IF NOT EXISTS until we figure out
|
||||
|
|
|
|||
|
|
@ -356,4 +356,24 @@ describe("Zotero.DB", function() {
|
|||
assert.isFalse(await Zotero.DB.columnExists('foo', 'itemID'));
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe("#indexExists()", function () {
|
||||
it("should return true if an index exists", async function () {
|
||||
assert.isTrue(await Zotero.DB.indexExists('items_synced'));
|
||||
});
|
||||
|
||||
it("should return false if an index doesn't exists", async function () {
|
||||
assert.isFalse(await Zotero.DB.indexExists('foo'));
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe("#parseSQLFile", function () {
|
||||
it("should extract tables and indexes from userdata SQL file", async function () {
|
||||
var sql = Zotero.File.getResource(`resource://zotero/schema/userdata.sql`);
|
||||
var statements = await Zotero.DB.parseSQLFile(sql);
|
||||
assert.isTrue(statements.some(x => x.startsWith('CREATE TABLE items')));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -248,6 +248,14 @@ describe("Zotero.Schema", function() {
|
|||
});
|
||||
})
|
||||
|
||||
it("should repair a missing userdata table", async function () {
|
||||
await Zotero.DB.queryAsync("DROP TABLE retractedItems");
|
||||
assert.isFalse(await Zotero.DB.tableExists('retractedItems'));
|
||||
assert.isFalse(await Zotero.Schema.integrityCheck());
|
||||
assert.isTrue(await Zotero.Schema.integrityCheck(true));
|
||||
assert.isTrue(await Zotero.DB.tableExists('retractedItems'));
|
||||
});
|
||||
|
||||
it("should repair a foreign key violation", function* () {
|
||||
yield assert.eventually.isTrue(Zotero.Schema.integrityCheck());
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue