[3.0][Testing] Check that an upgraded database matches a fresh install - #9531
[3.0][Testing] Check that an upgraded database matches a fresh install#9531albertlast wants to merge 20 commits into
Conversation
Provides a reproducible local stack so contributors can work on SMF without installing PHP, Composer or PostgreSQL on the host: - PHP 8.4 on Apache, with every extension other/requirements.md lists as required (mbstring, fileinfo, pgsql, mysqli) or recommended (gd, intl, curl, exif, ftp, xsl, zip). - PostgreSQL 17, with standard_conforming_strings forced on at database level as SMF requires. - Mailpit, so mail() is captured locally and nothing can be sent out. - Adminer, for browsing the database. The entrypoint runs composer install, waits for the database, generates a Settings.php pointed at the db service and drops install.php into place, so a fresh checkout is ready to install on first boot. Everything lives under .docker/ because check-smf-index.php and check-smf-license.php skip dot directories, so the environment cannot break the file integrity checks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
SMF supports MySQL and PostgreSQL, and until now this environment only offered one of them. Both database services now start, and SMF_DB_TYPE decides which one the generated Settings.php points at. It defaults to mysql, since that is what the great majority of installs run on. The two engines keep separate volumes, so a forum can be installed on each and switched between by deleting Settings.php and restarting. Settings.php wins over SMF_DB_TYPE once it exists, and the entrypoint says so rather than silently ignoring the variable. The postgres service is renamed from `db` to say what it is, and keeps `db` as a network alias so Settings.php files written by the previous version still resolve. Engine settings are pinned the same way the postgres side already pinned standard_conforming_strings: utf8mb4 and InnoDB, matching SMF's own table DDL. The collation is deliberately left at the charset default, because SMF sets CHARSET without COLLATE, and forcing one here would diverge from the tables it creates. Also corrects the everyday-use notes: php.ini, the vhost and the entrypoint are copied into the image, so editing them needs a rebuild rather than a restart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PostgreSQL logs every statement that errors together with the SQL that caused it, with no configuration needed, and the log is only on the container stderr. That makes `docker compose logs postgres` the most useful debugging tool in the stack, and nothing said so. MySQL logs server errors only, never the client statement that failed, so the note points out the asymmetry: now that mysql is the default engine, a suspected SQL problem is worth reproducing on postgres. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maintenance::exit() renders the tool's templates, and those are the only place errors are ever shown. On the command line it takes the fallthrough path instead and goes straight to die(), so nothing was reported and the exit status was always 0: a scripted install that died on step three looked exactly like one that had finished. ToolsBase::updateSettingsFile() made the same assumption more directly, calling die() outright when Settings.php could not be written rather than recording the error the way the web path does. Writes the warnings and errors to stderr and exits non-zero when the tool actually failed. A step that merely wants input it was not given sets neither, so pausing part way through is still a success - the installer is meant to be called more than once - and that case now says which step it stopped on instead of nothing at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Two things in the installer only hold when a browser is on the other end,
and both are reached before the forum exists, so neither could be worked
around from outside.
defaultHost() reads $_SERVER['SERVER_NAME'] and ['SERVER_PORT'] whenever
HTTP_HOST is absent. On the command line none of the three is set, so
every run began with an undefined index warning. Falls back to localhost:
the value only seeds the suggested board URL on the form, and a scripted
install passes its own boardurl in.
forumSettings() then built the same suggestion with
substr($self, 0, strrpos($self, '/')). getSelf() is $_SERVER['PHP_SELF'],
which in a request is a rooted path but on the command line is whatever
was typed - usually a bare 'install.php' with no directory in it. strrpos()
returns false, and substr() with a false length is fatal on PHP 8, so the
installer died here on every CLI run.
While in there: an unrecognised database type reported
Lang::getTxt('upgrade_unknown_error'), which is not a string that exists.
The fatal error was therefore blank in the browser too. Names the type
that was rejected and the ones that would have been accepted, which
matters most on the command line where the type is typed by hand rather
than picked from a list of exactly those keys.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
finalize() ends by signing the new administrator in, so the browser that just ran the installer lands on an admin session instead of a login form. It sets a login cookie, then records the session against the user agent that asked for it. None of that has any meaning on the command line. There is no browser to hold the cookie and no user agent to key the session on, so every CLI install ended with four warnings - headers sent after output had already started, a session that could not be started, and an id that could not be regenerated - and then wrote a sessions row built from an undefined HTTP_USER_AGENT. Runs the whole block only when there is a request behind it. The stats that follow it are untouched, so an install still records latestMember, totalMessages and totalTopics either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Two things were wrong with the note the command line prints when a tool stops part way. It indexed the step list to get the number, which counts from zero, while every other line of output uses the step's own id, which counts from one - so it disagreed with the "Step 3: Database Settings" lines immediately above it. It also fired on a successful run. Tools deliberately return false from their last step so the web flow stops and renders its "all done" template, which means reaching that step is success rather than a pause, and a completed install claimed to have stopped at it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The dev environment stopped at a Settings.php and a staged install.php, leaving the actual install to a human clicking through a browser. That is the one step between a fresh clone and a running forum that could not be scripted, and everything that wants to test against a real install has to start by doing it. Adds four scripts under .docker/: install-forum.sh installs a forum, no browser involved use-engine.sh switches which installed forum is live reset.sh empties one engine's database and restages lib.sh shared settings and engine name normalisation The installer is already CLI-native - parseCliArguments() turns --name=value into $_POST and execute() runs every step in one process - so this is two passes rather than 2.1's five curl requests. The second pass carries pop_done, which is the short-circuit past the population report; passing it on the first pass would skip building the schema. --engine both installs MySQL and then PostgreSQL. It has to be sequential: Settings.php pins a single db_type and Db::load() hands back the connection it already made, so only one engine is ever live in a process. Both installs are kept, and use-engine.sh swaps between them by putting the saved Settings.php back - no restart, because the entrypoint only writes one when there is not one already. --pin-secrets fixes auth_secret and image_proxy_secret, which are generated with random_bytes() and stored nowhere but Settings.php. Without it the two installs differ by more than their database and a login cookie does not survive the switch. The cookie name needs no such help: createCookieName() is a crc32 of the database name and prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The README invokes them as .docker/install-forum.sh rather than through bash, which only works with the bit set. Windows checkouts do not carry it, so it has to be recorded in the index. lib.sh is left alone: it is sourced, never run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The installer tells you to delete it and cannot do it itself: the ?delete link it offers is a GET, and command line arguments only ever reach $_POST, so nothing on the CLI path ever gets there. Leaving it behind is not cosmetic. Settings.php redirects every request back into the installer while the file exists, so the forum the script just built is unreachable, and SMF puts a "MAJOR SECURITY RISK: you have not removed install.php" box on every page it shows an administrator - which also lands in front of anything else a test or a person is trying to read on that page. Deleting it is safe for a reinstall because install_one() calls reset.sh first, and reset.sh clears Settings.php and then blocks until the entrypoint has staged a fresh copy. Adds a check in front of the two installer passes to say so out loud when it has not: without one, php reports "Could not open input file: install.php", which reads like a broken script rather than a stack that was never made installable. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Two forums side by side, each with its own administrator, and a password
chosen at install time is a combination that ends in hand written SQL
sooner or later - which is a poor way to answer a question as ordinary as
"is this the password?".
user.sh answers it. list shows the accounts, check says whether SMF would
accept a password and exits 0 or 1 so it can be used in a conditional,
and reset sets a new one. --engine reads the settings use-engine.sh saved
for the other engine, so the forum that is not currently live can be
looked at without switching to it and back.
Two details that stop it being a thin wrapper around an UPDATE:
- The hashing goes through Security::hashPassword() rather than being
written here, so what lands in the table is by construction what
Login2 reads back out. A script that hashes passwords its own way is
a script that eventually disagrees with the forum.
- reset clears passwd_flood too. SMF locks an account out for a while
after enough wrong guesses, and a new password behind a live lockout
behaves exactly like a password that did not take.
check also points out an account that is not activated, which fails to
log in with an entirely correct password.
The password is passed to the container through the environment rather
than in the argument list, which anything able to read the process table
can see. Also completes the file list in the README, which still only
described the image and had none of the scripts in it.
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The installer builds the schema from Sources/Db/Schema/v3_0/ in one go. The upgrader arrives at the same place through a hundred-odd migrations applied to whatever 2.1 left behind. They are meant to converge, and nothing we have checks that they do -- so a column left at the wrong type, an index that was never created, or a primary key quietly dropped goes unnoticed until it is a bug report from someone whose forum upgraded years ago. .docker/schema-tool.php reads the shape of a database -- tables, columns, indexes, sequences, the names in settings -- and compares two of those readings. It talks to the engine directly rather than through SMF, because the database worth looking at is frequently one SMF would refuse to run on. .docker/compare-upgrade.sh drives it: empty, load a 2.1 dump, upgrade, read, reinstall from scratch, read again, report. --baseline takes any 2.1 dump, so this works against a real forum as well as against the synthetic baseline from the 2.1 environment. Two kinds of difference are reported but do not decide the exit code, since a real forum always has some: what is in settings, and the order the columns of a table sit in. And if the upgrade does not reach the end, it stops there and says so rather than comparing anyway -- a half-upgraded database differs from a fresh install in hundreds of places, every one of them the honest consequence of the migrations that never ran. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Checking the reading against pg_dump --schema-only of the same database found two gaps, both of them the kind of thing this is supposed to notice. Three indexes were missing entirely. An index on an expression stores 0 in indkey and has no pg_attribute row, so joining to that table dropped those keys, and an index every one of whose keys is an expression disappeared with them -- idx_member_name_low, idx_real_name_low and idx_birthdate2 on a stock install. pg_get_indexdef() per key renders a plain column and an expression alike, and needs no join at all. The compatibility functions were not read. find_in_set(), instr(), from_unixtime(), the group_concat aggregate and the rest are created at install; a query naming one of them fails outright where it is absent, which makes a missing one a worse problem than a missing index, not a lesser one. The two readings now agree object for object: 72 tables, 113 indexes, 69 primary keys, 41 sequences, 19 functions. All pg_dump still reports that this does not are the public schema and the comment on it. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
|
Checked the reading against Three indexes were missing entirely. An index on an expression stores Three out of 113 on a stock install, and silently: the tool reported no difference where an upgrade had failed to create any of them. The compatibility functions were not read. The two readings agree object for object now:
All Verified by breaking a database on purpose — dropping I checked MySQL the same way, against Worth saying why the tool reads the catalogue rather than diffing two dumps, since that is the obvious alternative. A dump is ordered and formatted for restoring, not for comparing — |
Same check as the last commit, the other way round: against mysqldump --no-data --routines --triggers --events. Column for column and index for index the two agree, 538 and 179, and SMF creates no routines, triggers or events on MySQL, so there is no equivalent of the missing functions. Two things were being read too shallowly, though. EXTRA says that a column is generated and never what from. smf_messages has three STORED columns read out of the edit_history JSON -- modified_time, modified_name and modified_reason -- and a wrong path in one of them gives a column of the right type holding quietly the wrong value. Comparing GENERATION_EXPRESSION is what tells $[0][7] from $[0][9]. ROW_FORMAT was not read at all. It is not decoration on this schema: COMPACT caps an index key at 767 bytes where DYNAMIC allows 3072, so a table left behind in the older format is one where half of SMF's indexes cannot be created at their full width -- and index width is already the second largest group of differences an upgrade produces. AUTO_INCREMENT is still deliberately not read. It measures how much a database has been used, not what shape it is. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
|
Did the same against Two things were being read too shallowly, though. A generated column was compared without its expression. `modified_time` bigint unsigned GENERATED ALWAYS AS (coalesce(json_unquote(json_extract(`edit_history`,'$[0][0]')),0)) STORED
`modified_name` varchar(255) GENERATED ALWAYS AS (coalesce(json_unquote(json_extract(`edit_history`,'$[0][6]')),'')) STORED
`modified_reason` varchar(255) GENERATED ALWAYS AS (coalesce(json_unquote(json_extract(`edit_history`,'$[0][7]')),'')) STOREDA wrong path in one of those produces a column of exactly the right type holding quietly the wrong value, which is about the worst thing this could fail to notice. Comparing
Checked by breaking things on purpose again — moving one JSON path and putting one table back to So both engines have now been checked against their own dump tool, and both found something. That is the more useful outcome than agreement would have been. |
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Description
The installer builds the schema from
Sources/Db/Schema/v3_0/in one go. The upgrader arrives at the same place through a hundred-odd migrations applied to whatever 2.1 left behind. They are meant to converge, and nothing we have checks that they do — so a column left at the wrong type, an index that was never created, or a primary key quietly dropped goes unnoticed until it turns up as a bug report from someone whose forum upgraded two years ago.Two files:
.docker/schema-tool.phpreads the shape of a database — tables, columns, indexes, sequences, the variable names insettings— and compares two of those readings. It talks to the engine directly rather than through SMF, since the database worth looking at is frequently one SMF would refuse to run on..docker/compare-upgrade.shdrives it: empty the database, load a 2.1 dump, upgrade it, read the schema, reinstall from scratch, read that too, report the difference.--baselinetakes any SQL dump of a 2.1 database, so this works against a real forum as well as against the synthetic baseline from #9330.Two kinds of difference are reported but do not decide the exit code, because a real forum always has some: what is in
settings, and the order the columns of a table sit in. And if the upgrade does not reach the end, the script stops there and says so rather than comparing anyway — a half-upgraded database differs from a fresh install in hundreds of places, every one of them the honest consequence of the migrations that never ran, and none of them worth reading. That check is worth more than it sounds: the first run of this produced 315 differences, all of them because the upgrade had stopped a third of the way through.The tool underneath is usable on its own against any two SMF databases on the same engine — two forums you have, or one forum before and after something you are testing.
What it says today
On MySQL, against the 2.1 baseline from #9330, with the
DropTimeOffsetdefects from #9521 patched so the upgrade can finish: 55 differences in the schema, in four groups.Text columns are one size larger than they should be. Around 35 of the 55.
ALTER TABLE … CONVERT TO CHARACTER SET utf8mb4promotes atextcolumn tomediumtextto preserve its byte capacity, so an upgraded forum ends up withmessages.bodyasmediumtextwhere a fresh install hastext, andmail_queue.bodyaslongtextagainstmediumtext. Wider rather than narrower, so nothing breaks, but it is not the schema the code was written against.Index prefix lengths disagree, in both directions.
Indexes 2.1 had that 3.0 does not define are never dropped, so an upgraded forum carries
log_spider_hits.idx_id_spider,log_spider_hits.idx_log_timeand a uniquelog_subscribed.id_subscribethat a fresh install has not got.log_packagesends up with the same column indexed twice, asfilename(191)andidx_filename(15).mentionskeeps defaults the fresh schema drops —id_member,id_mentionedandtimeareDEFAULT 0after an upgrade and have no default at all in a fresh install.Outside the schema, four settings a fresh install writes are missing after an upgrade —
cpu_count,forum_uuid,mostOnlineUpdated,robots_txt_search— and the 2.1-era ones the upgrade means to delete are all still there, which is #9530.On PostgreSQL the script stops before it can compare anything:
AlertsObsoleteusesUPDATE … JOIN, which is MySQL-only. That is #9519, and #9524 is the fix.None of the above is addressed here. This adds the thing that finds it.
Notes for review
Depends on #9344 for
.docker/install-forum.sh, and reads better after #9330, which is where the baseline it was written against comes from. Nothing outside.docker/and one.gitignoreentry changes, so it cannot affect a running forum.Issues References (Fixes|Related|Closes)
Related to #9330, #9344, #9519, #9521, #9530