diff --git a/CHANGELOG.md b/CHANGELOG.md index 40fb760fd..e99426e9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,34 @@ ones are marked like "v1.0.0-fork". the same way `SqlFileParser` does since #241. This is what failed the Windows PHPUnit jobs. Verified end-to-end against MeCab 0.996 and covered by tests that assert LF, CRLF and CR-only output tokenize identically. +* **Restoring a backup wiped the database and reported success** (#249): on a + Windows host, "Restore from Backup" emptied the install and restored nothing, + while the page still said "Database restored". `Restore::restoreFile` split + the dump into statements on `';' . PHP_EOL` — `";\r\n"` on Windows — but LWT + always writes `";\n"`, so the split never matched, every statement piled up + in the accumulator and the statement list came back empty. The restore then + dropped every table and replayed nothing. This is the same defect fixed in + `SqlFileParser` for #241; the restore path was missed. Line endings are now + normalized before splitting, so a dump restores identically regardless of + which OS wrote or reads it. Three further hardening changes make the failure + non-destructive if it ever recurs: a dump that parses to zero statements is + refused *before* any table is dropped, the restore's own report (query and + record counts) is shown verbatim instead of a flat "Database restored", and a + final statement not followed by a newline is no longer discarded. +* **Restore left the database in pieces when foreign keys were enabled** + (#249): a dump's `CREATE TABLE` statements come from `SHOW CREATE TABLE`, so + they carry their `FOREIGN KEY` clauses, but they are replayed in backup-table + order rather than dependency order (`feed_links` references `news_feeds` yet + is created before it). With foreign-key checks on, every such statement + failed with errno 1215 — on a 12-table backup, 11 of 12 `CREATE TABLE`s were + lost. Checks are now suspended for the replay and enforced again once every + table exists. +* **Choosing a restore file did nothing** (#249): the file field stayed empty + and the "Restore from Backup" button stayed disabled, because the inline + `@change` handler used optional chaining + (`$event.target.files[0]?.name || ''`), which `@alpinejs/csp` cannot parse — + it threw "CSP Parser Error: Unexpected token" on every selection. The handler + moved into the `backupManager` component as `selectFile()`. ### Changed diff --git a/src/Modules/Admin/Application/AdminFacade.php b/src/Modules/Admin/Application/AdminFacade.php index 9df9125ba..06ac84723 100644 --- a/src/Modules/Admin/Application/AdminFacade.php +++ b/src/Modules/Admin/Application/AdminFacade.php @@ -190,7 +190,7 @@ public function saveAndClearSession(string $key, string $value): void * @param array{name: string, type: string, tmp_name: string, error: int, size: int}|null $fileData * Validated file data from InputValidator::getUploadedFile() * - * @return array{success: bool, error: ?string} + * @return array{success: bool, error: ?string, message?: string} */ public function restoreFromUpload(?array $fileData): array { diff --git a/src/Modules/Admin/Application/UseCases/Backup/RestoreFromUpload.php b/src/Modules/Admin/Application/UseCases/Backup/RestoreFromUpload.php index b374e2b08..3a602d9bf 100644 --- a/src/Modules/Admin/Application/UseCases/Backup/RestoreFromUpload.php +++ b/src/Modules/Admin/Application/UseCases/Backup/RestoreFromUpload.php @@ -55,7 +55,10 @@ public function __construct(BackupRepositoryInterface $repository) * @param array{name: string, type: string, tmp_name: string, error: int, size: int}|null $fileData * Validated file data from InputValidator::getUploadedFile() * - * @return array{success: bool, error: ?string} + * @return array{success: bool, error: ?string, message?: string} + * On success, `message` carries the restore's own report (query + * and record counts), which the caller should show verbatim + * rather than substituting a generic "restored" string. */ public function execute(?array $fileData): array { @@ -97,6 +100,6 @@ public function execute(?array $fileData): array if (str_starts_with($message, 'Error:')) { return ['success' => false, 'error' => $message]; } - return ['success' => true, 'error' => null]; + return ['success' => true, 'error' => null, 'message' => $message]; } } diff --git a/src/Modules/Admin/Http/AdminController.php b/src/Modules/Admin/Http/AdminController.php index efafb3a98..b68d0bce3 100644 --- a/src/Modules/Admin/Http/AdminController.php +++ b/src/Modules/Admin/Http/AdminController.php @@ -131,7 +131,13 @@ public function backup(array $params): void $result = $this->adminFacade->restoreFromUpload( InputValidator::getUploadedFile('thefile') ); - $message = $result['success'] ? 'Database restored' : ($result['error'] ?? 'Restore failed'); + // Show the restore's own report rather than a flat "Database + // restored": its query/record counts are the only signal an admin + // has that the dump actually replayed. A restore that parsed zero + // statements used to look identical to a real one here (#249). + $message = $result['success'] + ? ($result['message'] ?? 'Database restored') + : ($result['error'] ?? 'Restore failed'); } elseif ($this->hasParam('backup')) { $this->adminFacade->downloadBackup(); // downloadBackup exits, so we never reach here diff --git a/src/Modules/Admin/Views/backup.php b/src/Modules/Admin/Views/backup.php index 9a7c14661..17891d7ba 100644 --- a/src/Modules/Admin/Views/backup.php +++ b/src/Modules/Admin/Views/backup.php @@ -192,7 +192,7 @@