diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b7fb3e..e8df09c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,3 +106,5 @@ jobs: run: tests/integration/full-restore-roundtrip.sh 35-apache - name: Full restore round trip on PostgreSQL, with Mail (foreign keys) and a disabled app run: tests/integration/full-restore-roundtrip.sh 35-apache pgsql + - name: Full restore round trip on MariaDB, with Mail (foreign keys) and a disabled app + run: tests/integration/full-restore-roundtrip.sh 35-apache mysql diff --git a/lib/Command/RestoreFull.php b/lib/Command/RestoreFull.php index 777582d..1516391 100644 --- a/lib/Command/RestoreFull.php +++ b/lib/Command/RestoreFull.php @@ -105,6 +105,9 @@ HELP); $output->writeln(sprintf('Created %d tables that did not exist here (apps that are missing or were removed): %s', count($result['dbReport']['created']), implode(', ', $result['dbReport']['created']))); } + foreach ($result['dbReport']['foreignKeyErrors'] ?? [] as $error) { + $output->writeln('Could not add a foreign key to a created table (the data is restored): ' . $error . ''); + } if ($result['dbReport']['missing'] !== []) { $output->writeln('Left out (structure unknown): ' . implode(', ', $result['dbReport']['missing']) . ''); } diff --git a/lib/Db/DbDumper.php b/lib/Db/DbDumper.php index 0f27e32..bc5f981 100644 --- a/lib/Db/DbDumper.php +++ b/lib/Db/DbDumper.php @@ -214,6 +214,21 @@ final class DbDumper { $this->restoreSequences($manifest['sequences'] ?? []); $this->resumeForeignKeys($foreignKeys); $this->db->commit(); + // Foreign keys of the tables created above, now their rows are in. Not inside the + // transaction: MySQL commits on DDL. A failure is reported, not fatal: the data is + // restored either way. + foreach ($report['created'] as $table) { + $structure = $manifest['tables'][$table]['schema'] ?? []; + foreach ($structure['foreignKeys'] ?? [] as $fk) { + try { + $this->db->createSchemaManager()->createForeignKey( + new \Doctrine\DBAL\Schema\ForeignKeyConstraint($fk['columns'], $fk['table'], $fk['foreign'], $fk['name'], $fk['options'] ?? []), + $table); + } catch (\Throwable $e) { + $report['foreignKeyErrors'][] = $table . '.' . $fk['name'] . ': ' . $e->getMessage(); + } + } + } } catch (\Throwable $e) { $this->db->rollBack(); // PostgreSQL rolls the dropped foreign keys back with the transaction; MySQL's session @@ -268,10 +283,18 @@ final class DbDumper { $indexes = []; foreach ($table->getIndexes() as $i) { if (!$i->isPrimary()) { - $indexes[] = ['name' => $i->getName(), 'columns' => array_values($i->getColumns()), 'unique' => $i->isUnique()]; + // options holds the prefix lengths MySQL needs for indexes on long text columns + $indexes[] = ['name' => $i->getName(), 'columns' => array_values($i->getColumns()), 'unique' => $i->isUnique(), + 'flags' => array_values($i->getFlags()), 'options' => $i->getOptions()]; } } - return ['columns' => $columns, 'pk' => array_values($table->getPrimaryKey()?->getColumns() ?? []), 'indexes' => $indexes]; + $foreignKeys = []; + foreach ($table->getForeignKeys() as $fk) { + $foreignKeys[] = ['name' => $fk->getName(), 'columns' => array_values($fk->getLocalColumns()), + 'table' => $fk->getForeignTableName(), 'foreign' => array_values($fk->getForeignColumns()), 'options' => $fk->getOptions()]; + } + return ['columns' => $columns, 'pk' => array_values($table->getPrimaryKey()?->getColumns() ?? []), 'indexes' => $indexes, + 'foreignKeys' => $foreignKeys]; } private static function tableFromStructure(string $name, array $structure): \Doctrine\DBAL\Schema\Table { @@ -287,7 +310,9 @@ final class DbDumper { $table->setPrimaryKey($structure['pk']); } foreach ($structure['indexes'] as $i) { - $i['unique'] ? $table->addUniqueIndex($i['columns'], $i['name']) : $table->addIndex($i['columns'], $i['name']); + $i['unique'] + ? $table->addUniqueIndex($i['columns'], $i['name'], $i['options'] ?? []) + : $table->addIndex($i['columns'], $i['name'], $i['flags'] ?? [], $i['options'] ?? []); } return $table; } @@ -316,8 +341,30 @@ final class DbDumper { return $restore; } if ($platform instanceof \Doctrine\DBAL\Platforms\AbstractMySQLPlatform) { + // MySQL does not check rows again when FOREIGN_KEY_CHECKS goes back to 1, and its DDL + // would commit the transaction halfway. So every foreign key is checked explicitly, + // with a query that finds rows pointing at nothing (resumeForeignKeys()). + $rows = $this->db->fetchAllAssociative( + 'SELECT TABLE_NAME AS tbl, CONSTRAINT_NAME AS name, COLUMN_NAME AS col, REFERENCED_TABLE_NAME AS rtbl, REFERENCED_COLUMN_NAME AS rcol + FROM information_schema.KEY_COLUMN_USAGE + WHERE TABLE_SCHEMA = DATABASE() AND REFERENCED_TABLE_NAME IS NOT NULL + ORDER BY TABLE_NAME, CONSTRAINT_NAME, ORDINAL_POSITION'); + $keys = []; + foreach ($rows as $r) { + if (str_starts_with((string)$r['tbl'], $this->prefix)) { + $keys[$r['tbl'] . "\0" . $r['name']][] = $r; + } + } + $q = fn (string $i): string => $this->db->quoteIdentifier($i); + $checks = ['SET FOREIGN_KEY_CHECKS = 1']; + foreach ($keys as $parts) { + $join = implode(' AND ', array_map(fn (array $r): string => 'c.' . $q($r['col']) . ' = p.' . $q($r['rcol']), $parts)); + $set = implode(' AND ', array_map(fn (array $r): string => 'c.' . $q($r['col']) . ' IS NOT NULL', $parts)); + $checks[] = sprintf('SELECT COUNT(*) FROM %s c LEFT JOIN %s p ON %s WHERE %s AND p.%s IS NULL /* %s */', + $q($parts[0]['tbl']), $q($parts[0]['rtbl']), $join, $set, $q($parts[0]['rcol']), $parts[0]['name']); + } $this->db->executeStatement('SET FOREIGN_KEY_CHECKS = 0'); - return ['SET FOREIGN_KEY_CHECKS = 1']; + return $checks; } if ($platform instanceof \Doctrine\DBAL\Platforms\SqlitePlatform) { $this->db->executeStatement('PRAGMA defer_foreign_keys = ON'); // checked at commit @@ -325,11 +372,22 @@ final class DbDumper { return []; } - /** Add the foreign keys back; a row that breaks one fails the restore here, before it commits. */ + /** + * Check the foreign keys again; a row that breaks one fails the restore here, before it commits. + * Statements add them back (PostgreSQL) or switch the checks on (MySQL); queries count the rows + * that point at nothing (MySQL) and must find none. + */ private function resumeForeignKeys(array $statements): void { foreach ($statements as $sql) { try { - $this->db->executeStatement($sql); + if (str_starts_with($sql, 'SELECT ')) { + $orphans = (int)$this->db->fetchOne($sql); + if ($orphans > 0) { + throw new \RuntimeException("$orphans rows point at nothing"); + } + } else { + $this->db->executeStatement($sql); + } } catch (\Throwable $e) { throw new \RuntimeException('The restored data breaks a foreign key (' . $sql . '): ' . $e->getMessage(), 0, $e); } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 546574d..b25f599 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -189,6 +189,8 @@ db]]> db]]> db]]> + db]]> + db]]> @@ -197,6 +199,7 @@ + diff --git a/tests/integration/full-restore-roundtrip.sh b/tests/integration/full-restore-roundtrip.sh index c29f6d0..e9c3657 100755 --- a/tests/integration/full-restore-roundtrip.sh +++ b/tests/integration/full-restore-roundtrip.sh @@ -2,9 +2,9 @@ # Disaster recovery: occ backup:key:import + backup:target:add + backup:restore:full on a # SECOND, fresh Nextcloud instance that never saw the first one, sharing only the backup # location (a local directory, bind-mounted into both containers) -- see PLAN.md 3.7. -# Usage: full-restore-roundtrip.sh [sqlite|pgsql] +# Usage: full-restore-roundtrip.sh [sqlite|pgsql|mysql] # e.g. full-restore-roundtrip.sh 35-apache pgsql -# With pgsql the origin also has what broke real disaster recoveries: the Mail app with a synced +# With pgsql or mysql the origin also has what broke real disaster recoveries: the Mail app with a synced # account (foreign keys between its tables) and Deck installed and then switched off (tables of # an app that is not enabled). The restore instance has neither; --install-apps adds Mail. set -euo pipefail @@ -35,6 +35,11 @@ db_env() { -e POSTGRES_PASSWORD=nextcloud postgres:16-alpine >/dev/null for i in $(seq 1 30); do docker exec "ngb-frt-db-$1" pg_isready -q 2>/dev/null && break; sleep 2; done echo "-e POSTGRES_HOST=ngb-frt-db-$1 -e POSTGRES_DB=nextcloud -e POSTGRES_USER=nextcloud -e POSTGRES_PASSWORD=nextcloud" + elif [ "$DB" = mysql ]; then + docker run -d --name "ngb-frt-db-$1" --network "$NET" -e MARIADB_DATABASE=nextcloud -e MARIADB_USER=nextcloud \ + -e MARIADB_PASSWORD=nextcloud -e MARIADB_ROOT_PASSWORD=nextcloud mariadb:11 >/dev/null + for i in $(seq 1 60); do docker exec "ngb-frt-db-$1" mariadb-admin ping -unextcloud -pnextcloud --silent >/dev/null 2>&1 && break; sleep 2; done + echo "-e MYSQL_HOST=ngb-frt-db-$1 -e MYSQL_DATABASE=nextcloud -e MYSQL_USER=nextcloud -e MYSQL_PASSWORD=nextcloud" else echo "-e SQLITE_DATABASE=nextcloud" fi @@ -64,7 +69,12 @@ docker exec -u www-data -e OC_PASS="$NC_PASS" ngb-frt-origin php occ user:add -- docker exec ngb-frt-origin sh -c "mkdir -p /var/www/html/data/frtest/files/Docs && head -c 200000 /dev/urandom > /var/www/html/data/frtest/files/Docs/report.bin && echo 'hello disaster recovery' > /var/www/html/data/frtest/files/Docs/note.txt && chown -R www-data:www-data /var/www/html/data/frtest" run_on ngb-frt-origin files:scan frtest >/dev/null -if [ "$DB" = pgsql ]; then +# SQL on an instance's own database (pgsql/mysql only) +sql() { + if [ "$DB" = pgsql ]; then docker exec "ngb-frt-db-$1" psql -U nextcloud -d nextcloud -Atc "$2" + else docker exec "ngb-frt-db-$1" mariadb -unextcloud -pnextcloud -N -B nextcloud -e "$2"; fi +} +if [ "$DB" != sqlite ]; then # Mail with a synced account: oc_mail_accounts points at oc_mail_mailboxes (foreign keys), # while the restore loads tables in name order, accounts before mailboxes. docker run -d --name ngb-frt-imap --network "$NET" -e MAILNAME=mail.domain.tld -e MAIL_ADDRESS=user@domain.tld \ @@ -75,7 +85,7 @@ if [ "$DB" = pgsql ]; then run_on ngb-frt-origin mail:account:create frtest Test user@domain.tld ngb-frt-imap 993 ssl user@domain.tld mypassword \ ngb-frt-imap 25 tls user@domain.tld mypassword >/dev/null || fail "mail account" run_on ngb-frt-origin mail:account:sync -f 1 >/dev/null || fail "mail sync" - [ "$(docker exec ngb-frt-db-origin psql -U nextcloud -d nextcloud -Atc 'select count(*) from oc_mail_accounts where drafts_mailbox_id is not null')" = 1 ] \ + [ "$(sql origin 'select count(*) from oc_mail_accounts where drafts_mailbox_id is not null')" = 1 ] \ || fail "the mail account does not point at its mailboxes" # Deck installed and then switched off: its tables stay in the database and in the backup run_on ngb-frt-origin app:install deck >/dev/null || fail "install deck on the origin" @@ -124,11 +134,10 @@ docker exec ngb-frt-restore test -f /var/www/html/data/frtest/files/Docs/note.tx run_on ngb-frt-restore files:scan frtest >/dev/null || fail "files:scan fails after restore" ok "database, files and ng_backup's own state are all intact and usable after the restore" -if [ "$DB" = pgsql ]; then - Q() { docker exec ngb-frt-db-restore psql -U nextcloud -d nextcloud -Atc "$1"; } - [ "$(Q "select count(*) from pg_tables where tablename = 'oc_deck_boards'")" = 1 ] || fail "the tables of the disabled app were not created" - [ "$(Q "select count(*) from oc_mail_accounts where drafts_mailbox_id is not null")" = 1 ] || fail "the mail account was not restored" - [ "$(Q "select count(*) from pg_constraint where contype = 'f' and conrelid::regclass::text like 'oc_mail_%'")" -gt 0 ] \ +if [ "$DB" != sqlite ]; then + [ "$(sql restore "select count(*) from information_schema.tables where table_name = 'oc_deck_boards'")" = 1 ] || fail "the tables of the disabled app were not created" + [ "$(sql restore "select count(*) from oc_mail_accounts where drafts_mailbox_id is not null")" = 1 ] || fail "the mail account was not restored" + [ "$(sql restore "select count(*) from information_schema.table_constraints where constraint_type = 'FOREIGN KEY' and table_name like 'oc_mail_%'")" -gt 0 ] \ || fail "the foreign keys of Mail are missing after the restore" printf '%s\n' "$OUT" | grep -q "Created .* tables" || fail "the restore did not report the tables it created" ok "tables of a disabled app created, Mail restored with its foreign keys"