Restore DB table reconciliation during integrity check

But skip it at startup, even if flagged on, if there are schema update
steps to perform, to avoid creating tables that aren't expected to exist
yet.

Originally added in 5b9e6497a but disabled in c4cc44528 and 7a434df53

chrome/content/zotero/xpcom/retractions.js

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

chrome/content/zotero/xpcom/schema.js

@@ -200,7 +200,10 @@ Zotero.Schema = new function(){
 				
 				// Auto-repair databases flagged for repair or coming from the DB Repair Tool
 				if (integrityCheck) {
-					await this.integrityCheck(true);
+					// If we need to run migration steps, don't reconcile tables, since it might
+					// create tables that aren't expected to exist yet
+					let toVersion = await _getSchemaSQLVersion('userdata');
+					await this.integrityCheck(true, { skipReconcile: userdata < toVersion });
 					options.skipIntegrityCheck = true;
 				}
 				
@@ -1763,7 +1766,14 @@ Zotero.Schema = new function(){
 	};
 	
 	
-	this.integrityCheck = Zotero.Promise.coroutine(function* (fix) {
+	/**
+	 * @param {Boolean} [fix=false]
+	 * @param {Object} [options]
+	 * @param {Boolean} [options.skipReconcile=false] - Don't reconcile the schema to create tables
+	 *     and indexes that should have been created and drop existing ones that should have been
+	 *     deleted
+	 */
+	this.integrityCheck = Zotero.Promise.coroutine(function* (fix, options = {}) {
 		Zotero.debug("Checking database integrity");
 		
 		// Just as a sanity check, make sure combined field tables are populated,
@@ -1787,15 +1797,12 @@ Zotero.Schema = new function(){
 		// 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 = [
-			/*
-			Currently disabled, because it can cause problems with schema update steps that don't
-			expect tables to exist.
-			
-			The test "should repair a missing userdata table" is also disabled.
-			
-			// Create any tables or indexes that are missing and delete any tables or triggers that
-			// still exist but should have been deleted
 			[
+				// Create any tables or indexes that are missing and delete any tables or triggers
+				// that still exist but should have been deleted
+				//
+				// This is skipped for automatic checks, because it can cause problems with schema
+				// update steps that don't expect tables to exist.
 				async function () {
 					var statementsToRun = [];
 					
@@ -1864,10 +1871,12 @@ Zotero.Schema = new function(){
 					for (let statement of statements) {
 						await Zotero.DB.queryAsync(statement);
 					}
+				},
+				{
+					reconcile: true
 				}
 			],
-			*/
+		
-			
 			// Foreign key checks
 			[
 				async function () {
@@ -2037,6 +2046,11 @@ Zotero.Schema = new function(){
 			]
 		];
 		
+		// Remove reconcile steps
+		if (options && options.skipReconcile) {
+			checks = checks.filter(x => !x[2] || !x[2].reconcile);
+		}
+	
 		for (let check of checks) {
 			let errorsFound = false;
 			// SQL statement
@@ -2532,8 +2546,8 @@ Zotero.Schema = new function(){
 		}
 		
 		if (!options.skipIntegrityCheck) {
-			// TEMP: Disabled
+			// Check integrity, but don't create missing tables
-			//yield Zotero.Schema.integrityCheck(true);
+			yield Zotero.Schema.integrityCheck(true, { skipReconcile: true });
 		}
 		
 		Zotero.debug('Updating user data tables from version ' + fromVersion + ' to ' + toVersion);

components/zotero-service.js

@@ -367,6 +367,13 @@ function ZoteroService() {
 							zContext.Zotero.startupErrorHandler();
 						}
 						else if (zContext.Zotero.startupError) {
+							// Try to repair the DB on the next startup, in case it helps resolve
+							// the error
+							try {
+								zContext.Zotero.Schema.setIntegrityCheckRequired(true);
+							}
+							catch (e) {}
+							
 							try {
 								zContext.Zotero.startupError =
 									zContext.Zotero.Utilities.Internal.filterStack(

test/tests/schemaTest.js

@@ -248,9 +248,11 @@ describe("Zotero.Schema", function() {
 			});
 		})
 		
-		it.skip("should repair a missing userdata table", async function () {
+		it("should create missing tables unless 'skipReconcile' is true", async function () {
 			await Zotero.DB.queryAsync("DROP TABLE retractedItems");
 			assert.isFalse(await Zotero.DB.tableExists('retractedItems'));
+			assert.isTrue(await Zotero.Schema.integrityCheck(false, { skipReconcile: true }));
+			
 			assert.isFalse(await Zotero.Schema.integrityCheck());
 			assert.isTrue(await Zotero.Schema.integrityCheck(true));
 			assert.isTrue(await Zotero.DB.tableExists('retractedItems'));