diff --git a/packages/web/src/Pages/SchemaUpdaterPage.php b/packages/web/src/Pages/SchemaUpdaterPage.php index f8d2fec880..3541f3a33c 100644 --- a/packages/web/src/Pages/SchemaUpdaterPage.php +++ b/packages/web/src/Pages/SchemaUpdaterPage.php @@ -402,6 +402,9 @@ public function indexPost() if (in_array($err, $skiperrs)) { continue; } + if (self::_repairAndRetry($update, $err, $skiperrs)) { + continue; + } $errors[] = sprintf( "%s: %s\n", _('Update ID'), @@ -579,6 +582,48 @@ public function indexPost() } $this->jsonSend($code, $msg); } + /** + * Repair the structure and run the statement once more after a 1146. + * + * 1146 means the step names a table this database never got. Steps are + * positions, not identities, and 1.5.x fills the same positions with + * different migrations: 1.5.10.2518 arrives at 288, so 1.6's + * `CREATE TABLE fileDeleteQueue` (`// 287`) is skipped and `// 290` + * ALTERs a table that is not there. The reconcile after the loop creates + * it, but only once the update has already failed, so the installer + * printed "Failed!" and a second run was needed. This runs that repair at + * the point of failure instead. + * + * Any other error is left to the caller. So is a repair that does not + * succeed, and a retry that still fails: one repair, one retry. + * + * @param string $update the statement that failed + * @param int|string $err its MySQL error number + * @param array $skiperrs the codes the updater tolerates + * @param callable|null $repair SchemaReconciler::reconcile() unless given + * + * @return bool true when the statement now stands + */ + private static function _repairAndRetry( + $update, + $err, + array $skiperrs, + $repair = null + ) { + if (1146 != $err) { + return false; + } + if (true !== call_user_func($repair ?: [SchemaReconciler::class, 'reconcile'])) { + // Leave the statement's own error behind for the caller to + // report, not whatever the repair ran last. + self::$DB->query($update); + return false; + } + if (false === self::$DB->query($update)->error) { + return true; + } + return in_array(self::$DB->errorCode, $skiperrs); + } /** * Drop NO_ZERO_DATE and NO_ZERO_IN_DATE from this session's sql_mode. * diff --git a/tests/schema-updater-repairs-missing-table.test.php b/tests/schema-updater-repairs-missing-table.test.php new file mode 100644 index 0000000000..b0e45491f4 --- /dev/null +++ b/tests/schema-updater-repairs-missing-table.test.php @@ -0,0 +1,118 @@ +setAccessible(true); + +$skiperrs = [1050, 1054, 1060, 1061, 1062, 1091]; +$alter = 'ALTER TABLE `fileDeleteQueue` ADD `fdqPathType` VARCHAR(255) NOT NULL'; + +/** + * Counters the closures below write into. An object rather than by-reference + * ints, so each check reads what the code under test actually did. + */ +$seen = new \stdClass(); + +/** + * Answer $alter with $codes in turn (0 = success) and everything else with + * success. Resets the counters. + */ +$serve = static function (array $codes) use ($db, $alter, $seen) { + $seen->runs = 0; + $seen->repairs = 0; + $db->responder = static function ($sql) use ($db, $alter, &$codes, $seen) { + $code = 0; + if ($sql === $alter) { + $seen->runs++; + $code = $codes ? array_shift($codes) : 0; + } + $db->error = $code ? "error $code" : false; + $db->errorCode = $code ?: false; + return []; + }; +}; + +$repairOk = static function () use ($seen) { + $seen->repairs++; + return true; +}; + +// --- 1146, the repair creates the table, the retry succeeds ----------------- +$serve([0]); +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairOk); +$t->check('1146: reported as handled', $ok === true); +$t->check('1146: the repair ran once', $seen->repairs === 1); +$t->check('1146: the statement ran again', $seen->runs === 1); + +// --- 1146, the retry hits a tolerated code (the repair added the column) ---- +$serve([1060]); +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairOk); +$t->check('1146 then 1060: handled like any tolerated duplicate', $ok === true); + +// --- 1146, the retry still fails -------------------------------------------- +$serve([1146]); +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairOk); +$t->check('1146 twice: left to the caller', $ok === false); +$t->check('1146 twice: retried once, not looped', $seen->runs === 1 && $seen->repairs === 1); +$t->check('1146 twice: the caller sees the statement\'s own error', $db->errorCode === 1146); + +// --- 1146, the repair itself fails ------------------------------------------ +$serve([1146]); +$repairFails = static function () use ($seen) { + $seen->repairs++; + return 'reconcile failed'; +}; +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairFails); +$t->check('failed repair: left to the caller', $ok === false); +$t->check('failed repair: the caller sees the statement\'s own error', $db->errorCode === 1146); + +// --- any other error is not this method's business -------------------------- +foreach ([1064, 1452, 1005] as $code) { + $serve([]); + $ok = $retry->invoke(null, $alter, $code, $skiperrs, $repairOk); + $t->check("$code: left to the caller", $ok === false); + $t->check("$code: no repair, no retry", $seen->repairs === 0 && $seen->runs === 0); +} + +// --- the production call site ------------------------------------------------ +$src = file_get_contents(dirname(__DIR__) . '/packages/web/src/Pages/SchemaUpdaterPage.php'); +$t->check( + 'update() consults _repairAndRetry() after the tolerated codes', + (bool)preg_match( + '/in_array\(\$err, \$skiperrs\)\)\s*\{\s*continue;\s*\}\s*if \(self::_repairAndRetry\(\$update, \$err, \$skiperrs\)\)\s*\{\s*continue;\s*\}/', + $src + ) +); + +$t->finish();