diff --git a/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po index b74a64b124..94dbd9fdfa 100644 --- a/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po @@ -8801,6 +8801,14 @@ msgstr "" msgid "Schema shape" msgstr "Drucker-Update fehlgeschlagen!" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "Drucker-Update fehlgeschlagen!" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "Drucker-Update fehlgeschlagen!" + msgid "Schema sweep" msgstr "" @@ -12429,7 +12437,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po index 8923084bb9..97e7c8193b 100644 --- a/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po @@ -8810,6 +8810,14 @@ msgstr "" msgid "Schema shape" msgstr "Printer update failed!" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "Printer update failed!" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "Printer update failed!" + msgid "Schema sweep" msgstr "" @@ -12432,7 +12440,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po index 28a4d5d595..3004b0dbde 100644 --- a/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po @@ -8932,6 +8932,14 @@ msgstr "" msgid "Schema shape" msgstr "actualización de la impresora ha fallado!" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "actualización de la impresora ha fallado!" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "actualización de la impresora ha fallado!" + msgid "Schema sweep" msgstr "" @@ -12590,7 +12598,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po index 480dc826f6..0a6cccc83f 100644 --- a/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po @@ -8802,6 +8802,14 @@ msgstr "" msgid "Schema shape" msgstr "Drucker-Update fehlgeschlagen!" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "Drucker-Update fehlgeschlagen!" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "Drucker-Update fehlgeschlagen!" + msgid "Schema sweep" msgstr "" @@ -12430,7 +12438,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po index 62e9d4bfc1..4833726552 100644 --- a/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po @@ -8794,6 +8794,14 @@ msgstr "" msgid "Schema shape" msgstr "mise à jour de l'imprimante a échoué!" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "mise à jour de l'imprimante a échoué!" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "mise à jour de l'imprimante a échoué!" + msgid "Schema sweep" msgstr "" @@ -12417,7 +12425,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po index 69f2907309..5479711769 100644 --- a/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po @@ -8581,6 +8581,14 @@ msgstr "" msgid "Schema shape" msgstr "Aggiornamento della stampante non riuscito" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "Aggiornamento della stampante non riuscito" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "Aggiornamento della stampante non riuscito" + msgid "Schema sweep" msgstr "" @@ -12101,7 +12109,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po index b54f37c10a..cb8aef816b 100644 --- a/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po @@ -7572,6 +7572,14 @@ msgstr "スキーマの初期レコード投入" msgid "Schema shape" msgstr "スキーマ構造" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "スキーマ構造" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "スキーマ構造" + msgid "Schema sweep" msgstr "スキーマの検査と整理" @@ -10736,7 +10744,8 @@ msgstr "上記の CA 組み込み設定から自動判定" msgid "derived from the public CA setting above" msgstr "上記の公開 CA 設定から自動判定" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +#, fuzzy +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "マニフェストとの差異。変更は行われていません。差異を報告しただけで、修復はしていません。" msgid "directive in php.ini" diff --git a/packages/web/management/languages/messages.pot b/packages/web/management/languages/messages.pot index f35452e0e4..c6d4992ec8 100644 --- a/packages/web/management/languages/messages.pot +++ b/packages/web/management/languages/messages.pot @@ -7548,6 +7548,12 @@ msgstr "" msgid "Schema shape" msgstr "" +msgid "Schema shape repair refused" +msgstr "" + +msgid "Schema shape repaired" +msgstr "" + msgid "Schema sweep" msgstr "" @@ -10710,7 +10716,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po index 5ea0dcc03d..0dbd94af62 100644 --- a/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po @@ -8797,6 +8797,14 @@ msgstr "" msgid "Schema shape" msgstr "atualização da impressora falhou!" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "atualização da impressora falhou!" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "atualização da impressora falhou!" + msgid "Schema sweep" msgstr "" @@ -12420,7 +12428,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po index a71a2a89c5..89dc1bb296 100644 --- a/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po @@ -8797,6 +8797,14 @@ msgstr "" msgid "Schema shape" msgstr "打印机更新失败！" +#, fuzzy +msgid "Schema shape repair refused" +msgstr "打印机更新失败！" + +#, fuzzy +msgid "Schema shape repaired" +msgstr "打印机更新失败！" + msgid "Schema sweep" msgstr "" @@ -12420,7 +12428,7 @@ msgstr "" msgid "derived from the public CA setting above" msgstr "" -msgid "difference(s) from the manifest. Nothing was changed -- these are reported, not repaired." +msgid "difference(s) from the manifest. These are reported, not repaired: only a change that cannot lose data is repaired." msgstr "" msgid "directive in php.ini" diff --git a/packages/web/src/Db/SchemaReconciler.php b/packages/web/src/Db/SchemaReconciler.php index 7951aa7cff..c81db69635 100644 --- a/packages/web/src/Db/SchemaReconciler.php +++ b/packages/web/src/Db/SchemaReconciler.php @@ -664,6 +664,10 @@ public static function reconcile($manifest = null, $map = null) error_log(sprintf('%s: %s', _('Schema reconcile'), $sql)); } } + // Lossless type repair, before the foreign keys: a child column + // widened here can be what lets its constraint land below. + self::repairShape($manifest); + // Pass 4 -- foreign keys, after every table and column exists. // // Re-snapshots inside applyConstraints(): pass 1 may have created @@ -677,7 +681,7 @@ public static function reconcile($manifest = null, $map = null) // way forward from the browser. Logged and reported instead. self::applyConstraints(null, $map); - // Reports, never repairs -- GH-1542. The four passes above all ask + // Reports what repairShape() above left -- GH-1542. The four passes above all ask // "is this thing PRESENT?" and create it when it is not. Nothing // asks whether a column or an index that IS present has the right // SHAPE, so drift is invisible: a column whose type no longer @@ -686,13 +690,12 @@ public static function reconcile($manifest = null, $map = null) // any reason is never put back, taking its guarantee with it // silently. // - // Repairing either is a bigger decision than this pass is allowed to - // make -- MODIFY COLUMN on a populated column can truncate, and - // ADD UNIQUE on a table that has since acquired duplicates fails - // outright and needs a sweep in front of it, in the shape ADR 0031 - // decision 8 gives the constraint groups. So this turns an invisible - // condition into a readable one and settles how often it actually - // happens before anything starts issuing ALTERs. + // repairShape() fixes only a column change that cannot lose data. + // Everything else stays report-only: MODIFY COLUMN to a narrower or + // different type can truncate, and ADD UNIQUE on a table that has + // since acquired duplicates fails outright and needs a sweep in front + // of it, in the shape ADR 0031 decision 8 gives the constraint + // groups. self::reportShapeDrift($manifest); if (count($errors ?: [])) { @@ -740,8 +743,9 @@ public static function reportShapeDrift($manifest = null) _('Schema shape'), count($drift), _( - 'difference(s) from the manifest. Nothing was' - . ' changed -- these are reported, not repaired.' + 'difference(s) from the manifest. These are' + . ' reported, not repaired: only a change that' + . ' cannot lose data is repaired.' ) ) ); @@ -749,6 +753,198 @@ public static function reportShapeDrift($manifest = null) return $drift; } + /** + * Applies the column repairs planWidenings() says cannot lose data. + * + * GH-2000. A 1.5.x server carries a schema count that lands on a + * different 1.6 step, so the updater skips the steps below it. plan() + * puts back what those steps CREATED, but a step that changed the type of + * an existing column is lost: users.uName stayed varchar(40) NOT NULL on + * an upgraded server where a fresh install has varchar(255). Before this, + * each such case needed its own schema step, found by hand. + * + * Never fails the update. A repair the server refuses -- a wider column + * in an index that would pass the key length limit, a column a foreign + * key pins -- is logged, and reportShapeDrift() still reports the column. + * + * @param array|null $manifest Expected structure; defaults to the + * shipped one. + * + * @return array the statements that were applied + */ + public static function repairShape($manifest = null) + { + if (null === $manifest) { + $manifest = self::manifest(); + } + $drift = self::shapeDrift($manifest); + if (!count($drift)) { + return []; + } + // A column's own charset, collation and AUTO_INCREMENT, so the MODIFY + // keeps them. The manifest declares the first two per table and the + // third only in the CREATE statement, and MODIFY replaces the whole + // column definition: without them it converts the column to the + // table charset, which can lose characters, and drops AUTO_INCREMENT, + // which makes every later INSERT that omits the id fail with 1364. + $res = self::$DB->query( + sprintf( + 'SELECT `TABLE_NAME`, `COLUMN_NAME`, `CHARACTER_SET_NAME`,' + . ' `COLLATION_NAME`, `EXTRA` FROM `information_schema`.`COLUMNS`' + . ' WHERE `TABLE_SCHEMA` = %s', + self::$DB->escape(self::$DB->dbName()) + ) + ); + if (false !== $res->error) { + return []; + } + $charsets = []; + foreach ((array)$res->fetch(\PDO::FETCH_ASSOC, 'fetch_all')->get() as $row) { + $charsets[strtolower((string)$row['TABLE_NAME'])] + [strtolower((string)$row['COLUMN_NAME'])] = [ + (string)($row['CHARACTER_SET_NAME'] ?? ''), + (string)($row['COLLATION_NAME'] ?? ''), + false !== stripos((string)($row['EXTRA'] ?? ''), 'auto_increment'), + ]; + } + $applied = []; + foreach (self::planWidenings($drift, $manifest, $charsets) as $sql) { + if (false === self::$DB->query($sql)->error) { + $applied[] = $sql; + error_log(sprintf('%s: %s', _('Schema shape repaired'), $sql)); + continue; + } + error_log( + sprintf( + '%s: %s: %s', + _('Schema shape repair refused'), + self::$DB->error, + $sql + ) + ); + } + return $applied; + } + + /** + * The MODIFY statements for column drift that cannot lose data. + * + * Pure. A column qualifies only when every part of its change is lossless: + * + * - the type is the same, or a longer varchar/char/varbinary/binary, or a + * wider integer of the same signedness. An integer's display width, + * int(11) against int, is not a change at all. + * - nullability stays, or goes from NOT NULL to NULL. + * + * Anything else -- a narrower type, a different type, NULL to NOT NULL, + * an enum to tinyint -- stays report-only. Those can truncate a value or + * refuse a row, and they need a schema step that knows the data. + * + * @param array $drift shapeDrift() output + * @param array $manifest expected structure + * @param array $charsets [table][column] => [charset, collation, + * auto_increment], live + * + * @return array SQL statements + */ + public static function planWidenings(array $drift, $manifest, array $charsets = []) + { + $out = []; + foreach ($drift as $d) { + if ('column' !== ($d['kind'] ?? '')) { + continue; + } + $decl = (string)($manifest['tables'][$d['table']]['columns'][$d['name']] ?? ''); + if ('' === $decl) { + continue; + } + $want = self::_declShape($decl); + $haveNull = (bool)preg_match('/ NULL$/', $d['actual']) + && !preg_match('/ NOT NULL$/', $d['actual']); + $haveType = (string)preg_replace('/ (NOT )?NULL$/', '', $d['actual']); + if (!$want['null'] && $haveNull) { + continue; + } + if (!self::_typeWidens($haveType, $want['type'])) { + continue; + } + if (self::_sameInteger($haveType, $want['type']) + && $want['null'] === $haveNull + ) { + // Only the display width differs. Nothing to change. + continue; + } + // Keep the column's own charset: put it right after the type, + // where MODIFY expects it. + $decl = trim($decl); + $typeLen = strlen((string)preg_split( + '/\s+(?:NOT\s+NULL|NULL|DEFAULT|AUTO_INCREMENT|COMMENT|' + . 'CHARACTER\s+SET|COLLATE|ON\s+UPDATE|GENERATED)\b/i', + $decl + )[0]); + $cs = $charsets[strtolower((string)$d['table'])][strtolower((string)$d['name'])] ?? ['', '', false]; + $charset = '' !== $cs[0] && '' !== $cs[1] + ? sprintf(' CHARACTER SET %s COLLATE %s', $cs[0], $cs[1]) + : ''; + $rest = substr($decl, $typeLen); + if (!empty($cs[2]) && false === stripos($rest, 'AUTO_INCREMENT')) { + $rest .= ' AUTO_INCREMENT'; + } + $out[] = sprintf( + 'ALTER TABLE `%s` MODIFY `%s` %s%s%s', + $d['table'], + $d['name'], + substr($decl, 0, $typeLen), + $charset, + $rest + ); + } + return $out; + } + + /** + * Whether every value of type $have fits in type $want unchanged. + * + * @param string $have live COLUMN_TYPE, lowercase + * @param string $want manifest type, lowercase + * + * @return bool + */ + private static function _typeWidens($have, $want) + { + if ($have === $want) { + return true; + } + $len = '/^(varchar|char|varbinary|binary)\(([0-9]+)\)$/'; + if (preg_match($len, $have, $h) && preg_match($len, $want, $w)) { + return $h[1] === $w[1] && (int)$w[2] >= (int)$h[2]; + } + $ranks = ['tinyint' => 1, 'smallint' => 2, 'mediumint' => 3, 'int' => 4, 'bigint' => 5]; + $int = '/^(tinyint|smallint|mediumint|int|bigint)(?:\([0-9]+\))?( unsigned)?( zerofill)?$/'; + if (preg_match($int, $have, $h) && preg_match($int, $want, $w)) { + return ($h[2] ?? '') === ($w[2] ?? '') + && ($h[3] ?? '') === ($w[3] ?? '') + && $ranks[$w[1]] >= $ranks[$h[1]]; + } + return false; + } + + /** + * Whether two integer types differ only in display width. + * + * @param string $have live COLUMN_TYPE, lowercase + * @param string $want manifest type, lowercase + * + * @return bool + */ + private static function _sameInteger($have, $want) + { + $strip = static function ($t) { + return (string)preg_replace('/^(tinyint|smallint|mediumint|int|bigint)\([0-9]+\)/', '$1', $t); + }; + return $strip($have) === $strip($want); + } + /** * Columns and UNIQUE indexes whose live shape differs from the manifest. * diff --git a/tests/schema-reconciler-lossless-repair.test.php b/tests/schema-reconciler-lossless-repair.test.php new file mode 100644 index 0000000000..52ce5c89eb --- /dev/null +++ b/tests/schema-reconciler-lossless-repair.test.php @@ -0,0 +1,109 @@ + ['users' => ['columns' => [ + 'uName' => 'varchar(255) DEFAULT NULL', + 'uCode' => 'varchar(10) NOT NULL', + 'uFlag' => 'tinyint(1) NOT NULL DEFAULT 0', + 'uId' => 'int(11) NOT NULL', + 'uBig' => 'bigint(20) NOT NULL', + 'uSigned' => 'int(11) NOT NULL', + 'uChar' => 'char(8) NOT NULL', + 'uText' => 'longtext NOT NULL', +]]]]; +$charsets = ['users' => ['uname' => ['utf8mb3', 'utf8mb3_general_ci']]]; + +$col = static function ($name, $expected, $actual) { + return ['table' => 'users', 'kind' => 'column', 'name' => $name, + 'expected' => $expected, 'actual' => $actual]; +}; +$plan = static function (array $drift) use ($manifest, $charsets) { + return SchemaReconciler::planWidenings($drift, $manifest, $charsets); +}; + +// --- the GH-2000 case: longer and nullable --------------------------------- +$sql = $plan([$col('uName', 'varchar(255) NULL', 'varchar(40) NOT NULL')]); +$t->check('varchar(40) NOT NULL -> varchar(255) NULL is repaired', count($sql) === 1); +$t->check( + 'the MODIFY keeps the column charset and the manifest default', + ($sql[0] ?? '') === 'ALTER TABLE `users` MODIFY `uName` varchar(255)' + . ' CHARACTER SET utf8mb3 COLLATE utf8mb3_general_ci DEFAULT NULL' +); + +// --- lossless, each on its own ------------------------------------------------ +$t->check('a longer char is repaired', count($plan([$col('uChar', 'char(8) NOT NULL', 'char(4) NOT NULL')])) === 1); +$t->check('a wider integer is repaired', count($plan([$col('uBig', 'bigint(20) NOT NULL', 'int(11) NOT NULL')])) === 1); +$t->check('NOT NULL -> NULL alone is repaired', count($plan([$col('uName', 'varchar(255) NULL', 'varchar(255) NOT NULL')])) === 1); + +// --- could lose data: left alone -------------------------------------------- +$t->check('a shorter varchar is not repaired', $plan([$col('uCode', 'varchar(10) NOT NULL', 'varchar(40) NOT NULL')]) === []); +$t->check('NULL -> NOT NULL is not repaired', $plan([$col('uCode', 'varchar(10) NOT NULL', 'varchar(10) NULL')]) === []); +$t->check('longer but NULL -> NOT NULL is not repaired', $plan([$col('uCode', 'varchar(10) NOT NULL', 'varchar(5) NULL')]) === []); +$t->check('enum -> tinyint is not repaired', $plan([$col('uFlag', 'tinyint(1) NOT NULL', "enum('0','1') NOT NULL")]) === []); +$t->check('a narrower integer is not repaired', $plan([$col('uId', 'int(11) NOT NULL', 'bigint(20) NOT NULL')]) === []); +$t->check('unsigned -> signed is not repaired', $plan([$col('uSigned', 'int(11) NOT NULL', 'int(10) unsigned NOT NULL')]) === []); +$t->check('varchar -> longtext is not repaired', $plan([$col('uText', 'longtext NOT NULL', 'varchar(40) NOT NULL')]) === []); +$t->check('char -> varchar is not repaired', $plan([$col('uChar', 'char(8) NOT NULL', 'varchar(4) NOT NULL')]) === []); + +// --- no change at all --------------------------------------------------------- +$t->check('int(11) against int (MySQL 8) is not a change', $plan([$col('uId', 'int(11) NOT NULL', 'int NOT NULL')]) === []); +$t->check('a UNIQUE index finding is not a column repair', $plan([ + ['table' => 'users', 'kind' => 'unique', 'name' => 'name', 'expected' => 'UNIQUE(uname)', 'actual' => 'absent'], +]) === []); +$t->check('a column the manifest does not declare is skipped', $plan([$col('uGone', 'int(11) NOT NULL', 'int(4) NOT NULL')]) === []); + +// --- AUTO_INCREMENT survives the MODIFY ------------------------------------- +// The manifest's column declaration does not carry AUTO_INCREMENT; only its +// CREATE does. Widening modules.id (mediumint -> int) from the declaration +// alone dropped it, and step 419's INSERT then failed with 1364. +$sql = SchemaReconciler::planWidenings( + [$col('uId', 'int(11) NOT NULL', 'mediumint(9) NOT NULL')], + $manifest, + ['users' => ['uid' => ['', '', true]]] +); +$t->check( + 'an AUTO_INCREMENT column keeps AUTO_INCREMENT', + ($sql[0] ?? '') === 'ALTER TABLE `users` MODIFY `uId` int(11) NOT NULL AUTO_INCREMENT' +); +$sql = SchemaReconciler::planWidenings( + [$col('uId', 'int(11) NOT NULL', 'mediumint(9) NOT NULL')], + $manifest, + ['users' => ['uid' => ['', '', false]]] +); +$t->check('a plain column does not gain AUTO_INCREMENT', ($sql[0] ?? '') === 'ALTER TABLE `users` MODIFY `uId` int(11) NOT NULL'); + +// --- without a charset reading, no charset clause --------------------------- +$sql = SchemaReconciler::planWidenings([$col('uChar', 'char(8) NOT NULL', 'char(4) NOT NULL')], $manifest, []); +$t->check('no live charset: the manifest declaration alone', ($sql[0] ?? '') === 'ALTER TABLE `users` MODIFY `uChar` char(8) NOT NULL'); + +// --- reconcile() runs the repair before the foreign keys ---------------------- +$src = file_get_contents(dirname(__DIR__) . '/packages/web/src/Db/SchemaReconciler.php'); +$t->check( + 'reconcile() calls repairShape() before applyConstraints() and reportShapeDrift()', + (bool)preg_match('/self::repairShape\(\$manifest\);.*self::applyConstraints\(null, \$map\);.*self::reportShapeDrift\(\$manifest\);/s', $src) +); + +$t->finish(); diff --git a/tests/upgrade-from-15-ci.test.sh b/tests/upgrade-from-15-ci.test.sh new file mode 100644 index 0000000000..dd64e7012f --- /dev/null +++ b/tests/upgrade-from-15-ci.test.sh @@ -0,0 +1,262 @@ +#!/bin/bash +# +# Upgrade a REAL 1.5 database to this tree, through the real updater. +# +# tests/upgrade-from-15-ci.test.sh +# +# WHY THIS EXISTS (GH-2000). schemaVersion.vValue is a COUNT of applied +# $this->schema elements, and 1.5 and 1.6 fill the same positions with +# different migrations. So a 1.5 server's count lands on a different 1.6 +# step every time 1.5 adds one, and the updater skips every 1.6 step below +# it. 1.5.10.2518 arrived at 288: 1.6's CREATE TABLE fileDeleteQueue (287) +# was skipped and the first upgrade failed, and step 279's widening of +# users.uName was skipped and the column kept 1.5's 40 characters. +# +# Nothing else in CI could see that. tests/upgrade-rehearsal-ci.test.sh +# builds its starting database from THIS tree's steps [0, N), which is a 1.6 +# database at N, never a 1.5 one. Only 1.5's own step array produces a 1.5 +# database, and 1.5's steps call 1.5's classes, so they have to run inside a +# 1.5 tree. +# +# WHAT IT DOES, both halves the way bin/installfog.sh does them: +# +# 1. Takes packages/web and the installer's functions.sh from FROM_REF +# (default origin/stable) with `git archive`. +# 2. Renders each tree's config from ITS OWN installer template -- the +# `echo "/dev/null || true +} +trap cleanup EXIT + +if ! git -C "$REPO" rev-parse --verify -q "$FROM_REF^{commit}" >/dev/null; then + fail "$FROM_REF is not a commit here. Fetch it first (git fetch origin ${FROM_REF#origin/})." +fi +echo "upgrade from 1.5: $FROM_REF ($(git -C "$REPO" rev-parse --short "$FROM_REF")) -> this tree" + +mkdir -p "$WORK/src15" "$WORK/var/log" +git -C "$REPO" archive "$FROM_REF" packages/web lib/common/functions.sh | tar -x -C "$WORK/src15" \ + || fail "could not extract $FROM_REF" +cp -r "$REPO/packages/web" "$WORK/web16" || fail "could not copy this tree" +web15="$WORK/src15/packages/web" +web16="$WORK/web16" + +# Run a PHP snippet against the server with the credential in the +# environment, never on a command line. +sql() { + UP15_HOST="$HOST" UP15_USER="$DBUSER" UP15_PASS="$DBPASS" php -r ' + $pdo = new PDO("mysql:host=" . getenv("UP15_HOST"), getenv("UP15_USER"), getenv("UP15_PASS")); + $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + $r = $pdo->query($argv[1]); + if ($r && $r->columnCount()) { foreach ($r->fetchAll(PDO::FETCH_NUM) as $row) { echo implode("\t", $row), "\n"; } } + ' "$1" +} +sql "DROP DATABASE IF EXISTS \`$DB\`" || fail "cannot reach the database server at $HOST" + +# Render the installer's own config template from $1 (a functions.sh) into $2. +# The template is one bash double-quoted string, `echo " path`. +# It is evaluated with the variables an install would have set, and with the +# redirect replaced by our target. +render_config() { + local functions=$1 target=$2 end start body + end=$(grep -n -E '^\}" > "\$\{webdirdest\}/(lib/fog|commons)/config\.class\.php"$' "$functions" | head -1 | cut -d: -f1) + [[ -n $end ]] || fail "no config template end marker in $functions" + start=$(head -n "$end" "$functions" | grep -n -E '^ echo "<\?php$' | tail -1 | cut -d: -f1) + [[ -n $start ]] || fail "no config template start marker in $functions" + body="$(sed -n "${start},$((end - 1))p" "$functions")"$'\n}"' + grep -q 'class Config' <<<"$body" || fail "the block found in $functions is not the config template" + # 1.5's installer names these one way and 1.6's another; both are set. + local snmysqlhost="$HOST" mysqldbname="$DB" snmysqluser="$DBUSER" + local DB_host="$HOST" DB_name="$DB" DB_user="$DBUSER" + local phpescsnmysqlpass="${DBPASS//\\/\\\\}" + phpescsnmysqlpass="${phpescsnmysqlpass//\'/\\\'}" + local installToken="$TOKEN" confighostip=127.0.0.1 snapindir=/opt/fog/snapins + local interface=lo NET_interface=lo username=fogproject SVC_user=fogproject + local password="" SVC_password="" webdirdest="${target%/*/*}" + local storageLocation=/images STORAGE_image_share_path=/images + local storageLocationCapture=/images/dev storageDefaultCidr=127.0.0.0/8 + local webroot=/fog/ WEB_root=/fog/ tftpdirdst=/tftpboot + mkdir -p "$(dirname "$target")" + # set +u: a template variable added later must render empty, as it would + # for an install that never set it, not abort the gate. + set +u + eval "$body" > "$target" || fail "could not render $target" + set -u + php -l "$target" >/dev/null || fail "rendered $target is not valid PHP" +} + +TOKEN="$(head -c 24 /dev/urandom | od -An -tx1 | tr -d ' \n')" +render_config "$WORK/src15/lib/common/functions.sh" "$web15/lib/fog/config.class.php" +render_config "$REPO/lib/common/functions.sh" "$web16/commons/config.class.php" +printf " "$web16/commons/fogpaths.php" + +# The web server. Apache with mod_php, as a real install runs it, and not +# `php -S` or php-cgi: 1.5 reads REQUEST_METHOD and QUERY_STRING through +# filter_input(INPUT_SERVER), which both of those leave NULL, so 1.5 never sees +# the POST and redirects it. 7.4 is the supported floor; mysqli and gettext are +# what both trees' extension checks require. +CT="${UP15_CONTAINER:-docker}" +command -v "$CT" >/dev/null || fail "no $CT to run the web server (set UP15_CONTAINER)" +IMAGE=fog-upgrade-from-15:php74 +mkdir -p "$WORK/image" +printf 'FROM docker.io/library/php:7.4-apache\nRUN docker-php-ext-install pdo_mysql mysqli gettext >/dev/null\n' \ + > "$WORK/image/Containerfile" +"$CT" build -q -t "$IMAGE" -f "$WORK/image/Containerfile" "$WORK/image" >/dev/null \ + || fail "could not build the web server image" +# Apache runs as www-data, and the 1.6 updater writes its error log into the +# tree and its own log under FOG_BASE_DIR. +chmod -R a+rwX "$WORK" + +# Serve $1 at /fog/ and POST the schema deploy, the way the installer does. +# Prints the HTTP status. Host networking, so the database address in the +# config means the same thing inside the container as it does here. +deploy() { + local root=$1 log=$2 port name code i + port=$(php -r '$s = stream_socket_server("tcp://127.0.0.1:0"); echo explode(":", stream_socket_get_name($s, false))[1];') + name="fog-upgrade-from-15-$$-$port" + "$CT" run -d --rm --name "$name" --network host \ + -v "$root:/var/www/html/fog:Z" -v "$WORK/var:$WORK/var:Z" "$IMAGE" \ + sh -c "sed -i 's/^Listen 80\$/Listen $port/' /etc/apache2/ports.conf && exec apache2-foreground" \ + >/dev/null || fail "could not start the web server" + for i in $(seq 1 60); do + curl -s -o /dev/null "http://127.0.0.1:$port/" && break + sleep 0.5 + done + code=$(curl -s -o "$log.body" -w '%{http_code}' -X POST \ + -H "X-Fog-Install-Token: $TOKEN" -d 'schemaupdate=1' \ + "http://127.0.0.1:$port/fog/management/index.php?node=schema") + "$CT" logs "$name" >"$log" 2>&1 + "$CT" stop -t 2 "$name" >/dev/null 2>&1 + echo "$code" +} + +version() { sql "SELECT vValue FROM \`$DB\`.schemaVersion" 2>/dev/null | head -1; } +want15=$(awk -F'[(,)]' "/define\\('FOG_SCHEMA'/{gsub(/ /,\"\",\$3); print \$3; exit}" "$web15/lib/fog/system.class.php") +want16=$(awk -F'[(,)]' "/define\\('FOG_SCHEMA'/{gsub(/ /,\"\",\$3); print \$3; exit}" "$web16/src/Base/System.php") + +# --- 1. a fresh 1.5 install ---------------------------------------------- +code=$(deploy "$web15" "$WORK/deploy15.log") +got=$(version) +if [[ $code != 2* || -z $got ]]; then + sed 's/^/ /' "$WORK/deploy15.log" "$WORK/deploy15.log.body" 2>/dev/null | tail -20 + fail "the 1.5 schema deploy answered HTTP $code and left schema '${got:-none}'" +fi +echo " ok 1.5 install: schema $got (FOG_SCHEMA $want15)" + +# --- 2. the upgrade -------------------------------------------------------- +code=$(deploy "$web16" "$WORK/deploy16.log") +got=$(version) +# The updater writes this log as the web server's user with mode 0200, so it +# is read back through the container. +errlog() { + "$CT" run --rm -v "$web16:/fog:Z" "$IMAGE" \ + sh -c 'cat /fog/fog_schema_update_error.log 2>/dev/null' | sed 's/^/ /' | tail -40 +} +if [[ $code != 2* ]]; then + sed 's/^/ /' "$WORK/deploy16.log.body" 2>/dev/null | tail -20 + echo + errlog + fail "the upgrade answered HTTP $code and left schema ${got:-none}" +fi +if [[ -e $web16/fog_schema_update_error.log ]]; then + errlog + fail "the upgrade answered HTTP $code but wrote fog_schema_update_error.log" +fi +[[ $got == "$want16" ]] || fail "the upgrade landed on schema $got, not FOG_SCHEMA $want16" +echo " ok upgrade: HTTP $code, schema $got, no update error" + +# --- 3. the structure matches a fresh 1.6 install --------------------------- +cp -r "$REPO/packages/web" "$WORK/webshape" +printf " "$WORK/webshape/commons/fogpaths.php" +R() { REHEARSAL_DB="$DB" php "$REPO/bin/upgrade-rehearsal.php" "$WORK/webshape" "$@" 2>&1 | grep -v '^PHP Warning'; } +R makeconfig --host="$HOST" --user="$DBUSER" --pass="$DBPASS" >/dev/null || fail "makeconfig did not write a lab config" +shape="$(R shape --db="$DB")" +n=$(sed -n 's/.*differences from the manifest: \([0-9]*\).*/\1/p' <<<"$shape") +[[ -n $n ]] || { echo "$shape" | sed 's/^/ /'; fail "the shape check did not run"; } +if [[ $n -ne 0 ]]; then + echo "$shape" | sed 's/^/ /' + fail "$n column(s) or UNIQUE index(es) differ from a fresh install" +fi +echo " ok structure matches commons/schema-expected.php" + +# --- 4. every AUTO_INCREMENT a fresh install has ------------------------------ +# shapeDrift() compares a column's type and nullability, not AUTO_INCREMENT, +# and a MODIFY that leaves it out drops it silently. The next INSERT that omits +# the id then fails with 1364. The manifest's CREATE statements declare it. +lost=$(UP15_HOST="$HOST" UP15_USER="$DBUSER" UP15_PASS="$DBPASS" UP15_DB="$DB" \ + php -r ' + $m = include $argv[1]; + $pdo = new PDO("mysql:host=" . getenv("UP15_HOST"), getenv("UP15_USER"), getenv("UP15_PASS")); + $live = []; + $q = $pdo->prepare("SELECT LOWER(TABLE_NAME), LOWER(COLUMN_NAME) FROM information_schema.COLUMNS" + . " WHERE TABLE_SCHEMA = ? AND EXTRA LIKE \"%auto_increment%\""); + $q->execute([getenv("UP15_DB")]); + foreach ($q->fetchAll(PDO::FETCH_NUM) as $r) { $live[$r[0] . "." . $r[1]] = true; } + $tables = $pdo->prepare("SELECT LOWER(TABLE_NAME) FROM information_schema.TABLES WHERE TABLE_SCHEMA = ?"); + $tables->execute([getenv("UP15_DB")]); + $have = array_flip($tables->fetchAll(PDO::FETCH_COLUMN)); + foreach ((array)$m["tables"] as $t => $spec) { + if (!isset($have[strtolower($t)])) { continue; } + if (preg_match("/[(,]\\s*`([^`]+)` [^,]*AUTO_INCREMENT/i", (string)($spec["create"] ?? ""), $c) + && !isset($live[strtolower($t) . "." . strtolower($c[1])])) { + echo "$t.$c[1]\n"; + } + }' "$REPO/packages/web/commons/schema-expected.php") +if [[ -n $lost ]]; then + sed 's/^/ /' <<<"$lost" + fail "column(s) that are AUTO_INCREMENT on a fresh install are not after the upgrade" +fi +echo " ok every AUTO_INCREMENT column a fresh install has" +echo +echo " upgrade from $FROM_REF: pass" +exit 0