diff --git a/CHANGELOG.md b/CHANGELOG.md index 6423596..bf1ce07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,36 @@ Each heading starts with that ``, once: the app store shows a release t whose heading starts with its `x.y.z` and ignores anything after it, so `## [1.0.0-rc.6]` would be shown under 1.0.0. The git tag of a release candidate goes after the date. +## [1.0.3] - 2026-11-06 + +### Fixed +- Empty folders are backed up and restored (#27), in `occ backup:restore`, `occ backup:restore:full` + and the restore page, where they are shown too. They come back from the first backup made with + this version; older restore points do not contain them. An older version of NG Backup can still + read new restore points, and then only leaves the empty folders out. +- A full disk during a restore no longer looks like a damaged backup. The restore now stops with a + message about the disk, also with `--salvage`, and does not point at `--salvage` any more. The same + goes for other trouble writing to the restore directory, such as permissions. +- A file or folder whose name a snapshot cannot hold (a backslash, a control character, not UTF-8) is + left out and reported as unreadable. Before, a backslash or control character could make the whole + restore point unreadable, and a name that is not UTF-8 made the backup fail. +- A restore that goes on after a stop also removes the temporary files in folders where every file + is already in place, and checks that those folders are inside the restore directory before it + leaves their files alone. +- After a restore that was killed and then ran again, a backup no longer has to wait up to two hours + for the locks of the killed run. +- A restore only removes read-ahead folders of its own (`.ngb-restore-spool-` with 8 hex digits). +- A restore keeps its lock on the location during a long row of files that are already in place. +- A database restore that fails no longer leaves behind tables it created, which could stop the + installation of that app later. When only NG Backup's own settings cannot be put back after the + database was restored, `--salvage` no longer says the database is still the fresh one. +- A connection that breaks while opening a pack on the location (seen with SMB) is retried once and + is handled by `--salvage`, instead of stopping the restore with the error of the storage. +- Without a system cron, several running backups take turns while the page is open. +- The Bin tab shows how far it is while it reads the removed restore points, which can take a while. + It also shows when it cannot read a location, instead of an empty bin, and refreshes when a run + ends. + ## [1.0.2] - 2026-11-06 ### Added diff --git a/PLAN.md b/PLAN.md index 1b0028c..a6c9bc9 100644 --- a/PLAN.md +++ b/PLAN.md @@ -437,7 +437,44 @@ in README "Known limitations": met vertraging is nog niet gemeten); Bin-tabblad; schatting `target:add`; nieuwe screenshots (1–3 vervangen, 4-bin erbij; in `info.xml` bij de release naar tag v1.0.2 laten wijzen en 4-bin toevoegen). Getest: unit (208), psalm, integratie NC35 sqlite/pgsql, S3/SFTP/FTP/SMB, - volledige restore met `--salvage`. Nog te doen: DR-test, release. + volledige restore met `--salvage`. **1.0.2 uitgebracht 2026-11-06** (tag `v1.0.2`, PR #28). +- **Naar 1.0.3, uit de reviews van 1.0.2 (achteraf gedraaid, 2026-11-06):** + 1. Lege mappen (#27). + 2. Een volle schijf onder `--salvage` telt als beschadigde backup en de restore gaat door; zonder + `--salvage` wijst de melding er ten onrechte naar. `fwrite` controleren, schrijffouten stoppen de + restore (`Repository::restoreEntries`; ook `RestoreService::writeFile` nakijken). Beide reviews. + 3. Hervatten: controle op de doelmap vóór het overslaan, en tijdelijke bestanden ook opruimen in + een map waar alles al klopt. + 4. Alleen `.ngb-restore-spool-` opruimen. + 5. Salvage-test: tijdelijke bestanden recursief zoeken; `admin` en `frtest` apart controleren. + 6. Nog nakijken: lease verversen bij overgeslagen bestanden; fout ná de database-commit + (`restoreOwnAppConfig`) en tabellen die na een rollback blijven staan; backups zonder cron + via de pagina (eerlijke beurten, stappen die de webtimeout halen); Bin verversen en fouten + tonen; SMB-uitzondering bij het openen van een pack onder `--salvage`. + Niet doen: bij hervatten de inhoud vergelijken (grootte en mtime is ook het criterium van de backup). + **Stand op branch `1.0.3` (2026-11-06):** punten 1-6 gebouwd. Lege mappen staan in het metarecord + van de snapshot, niet tussen de bestanden: 1.0.2 en ouder lezen zo'n snapshot gewoon en slaan alleen + de lege mappen over. Geen webtimeout-wijziging zonder cron: alleen in de code nagelopen, niet getest + (Nextcloud zet de PHP-tijdslimiet ruim, een gestopte stap wordt overgedaan); wel eerlijke beurten. + Bin leest in delen met teller en balk (besluit 2026-11-06, ook met oog op meerdere backups per + dag). Getest: unit (volle schijf op echte tmpfs), integratie NC35 sqlite/pgsql/mysql, sftp, salvage, + salvage-files; Bin in een echte browser met vertraagde verzoeken (1 van 7 t/m 7). Nog: reviews + vóór de PR, README-regels bij de versiecommit. + **Simulaties (2026-11-06, NC 34.0.4, 2 GB, lokale SFTP, `~/dev-local/ngb-sim/sim.sh`):** normaal + (pgsql), stop en hervat (pgsql → MariaDB, restore met kill -9 gestopt), volle schijf met `--salvage`: + alle drie geslaagd, alle 3.101 gebruikersbestanden en 7 lege mappen gelijk, share, agenda en WebDAV + werken. Vond één fout, opgelost: na een kill bleven vergrendelingen tot 2 uur staan, waardoor de + backup van stap 11 werd geweigerd. `files:scan` "Updated" zijn in de gemeten run alleen mappen + (mtime van mappen wordt niet teruggezet, dus nieuwe mtime en etag; bestanden ongemoeid). "Removed": + o.a. `cache` van een gebruiker, die de backup uitsluit. Waarschijnlijk verklaart dit ook de 50.080 + "updated" op nc01, niet op nc01 zelf vastgesteld. + **Naar 1.1.0 of 1.2.0 (besluit 2026-11-06):** alle mappen krijgen bij een restore hun oorspronkelijke + tijd terug. Hoort zo, vooral bij oudere verzamelingen: de maptijd wordt gebruikt om in te schatten + wanneer een map is aangemaakt. De backup moet dan de tijd van alle mappen vastleggen en de restore + die aan het eind terugzetten; werkt pas vanaf de eerste backup met die versie. Op de roadmap onder 1.1.0. +- **Branches (besluit 2026-11-06):** werk voor een versie op een branch met die naam (`1.0.3`); + `polish` alleen voor kleine cosmetische zaken, per geval bekijken of en wanneer die gemerged + worden. SMB staat uit op sub5 en sub6; sub6 is het testaccount, eerst aanzetten voor een SMB-test. - **1.0.1** (uitgebracht 2026-11-01): alleen de kleine punten (besluit 2026-10-30): 4, 6–13. Punten 1, 2 en 5 gaan naar 1.0.2 (README-roadmap aangepast); punt 3 is een test, ingepland voor de week van 2 november, niet aangekondigd en los van de release. Voor 1.0.2 ook: nieuwe screenshots (het labelveld bij diff --git a/README.md b/README.md index 87b1935..3211c1d 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ installations and hosted Nextcloud. (Choosing SMB as a storage backend is the on see "Storage backend experiences" below — that requirement comes from files_external, not from ng_backup, and applies to any Nextcloud app that writes to an SMB share.) -**Status: 1.0.2, stable.** Disaster recovery has been tested on real installations (see "Tested +**Status: 1.0.3, stable.** Disaster recovery has been tested on real installations (see "Tested disaster recoveries" below). See "Known limitations" below, `PLAN.md` for the design and `docs/HOWTO.md` for a step-by-step guide from setup to a tested restore. Server lost? See `docs/DISASTER-RECOVERY.md`. @@ -169,6 +169,7 @@ Full restores onto a fresh server, following only [the guide](docs/DISASTER-RECO |---|---|---|---| | Production, about 1 TB, 150,000 files (Nextcloud All-in-One) | Hetzner Storage Box (SFTP), Germany → Canada | PostgreSQL → PostgreSQL | 150,622 files restored in about 25 h (13-14 MiB/s), sample identical | | The same, 1.0.2 | Hetzner Storage Box (SMB), Germany → Canada | PostgreSQL → PostgreSQL | 150,755 files restored in about 29 h (7-13 MiB/s), going on after a lost connection; sample identical, empty folders not restored ([#27](https://github.com/dillardblom/ng_backup/issues/27)) | +| 1.0.3, small simulations of 2 GB: as planned, with the restore killed halfway, and with a disk that runs full | Local SFTP server | PostgreSQL → PostgreSQL and MariaDB | all 3,101 user files and all empty folders identical; users, a share and a calendar back | | 138 GB simulation: 20 users, calendars, contacts, mail, shares, team folders with an ACL, versions, trash | OVH S3, Warsaw → Canada | MariaDB → PostgreSQL | all 15,772 files and every database count identical | | The same | OVH S3 | MariaDB → MariaDB | all 15,772 files and every database count identical | | The same | Hetzner Storage Box (SMB), Germany → Canada | MariaDB → SQLite | all 15,772 files and every database count identical (27.6 h, about 1.5 MB/s) | @@ -265,15 +266,12 @@ What is planned, without dates. Ideas and votes are welcome in the [issues](https://github.com/dillardblom/ng_backup/issues); if something you need is listed here, a 👍 on its issue helps decide what comes first. -**1.0.3** -- Empty folders in the backup and the restore - ([#27](https://github.com/dillardblom/ng_backup/issues/27)). - **1.1.0** - Restore without the recovery kit: the backup key from the location itself, opened with one of your passphrases. - A schedule per location, also "every N hours". - Locations backed up one after the other instead of at the same time. +- All folders get their original modification time back on a restore, not only the empty ones. - A note on a restore point that you can change afterwards. - Pinning a restore point, so the retention policy never removes it (for example the one from before an upgrade). @@ -293,12 +291,10 @@ a 👍 on its issue helps decide what comes first. On the backlog; know them before you rely on NG Backup: -- **Empty folders do not come back.** A backup only records files, so a folder without files is - not restored. To be fixed in 1.0.3 - ([#27](https://github.com/dillardblom/ng_backup/issues/27)). -- **A full disk during a restore looks like a damaged backup.** The reason shown is "Size mismatch", - and with `--salvage` the restore goes on and lists the remaining files. The backup itself is fine: - make room and run the restore again. To be fixed in 1.0.3. +- **Empty folders only come back from backups made with 1.0.3 or newer.** Older restore points do + not contain them. +- **Folders with files in them show the time of the restore**, not their original modification time. + The files keep theirs. On the roadmap. - **Only the data directory is backed up.** Object storage as primary storage (`objectstore` in `config.php`) is not supported: a backup refuses to start there. Folders mounted through the External storage app are not in the data directory and are **not included**, without a warning. diff --git a/appinfo/info.xml b/appinfo/info.xml index 090d5a1..6ee5d53 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -45,7 +45,7 @@ welcome: use the "Share your experience" link on the admin page. Developed and sponsored by [Ensembia](https://www.ensembia.com). We can offer (paid) assistance if you need help with restoring a backup after a crash.]]> - 1.0.2 + 1.0.3 agpl Dillard Blom NgBackup @@ -56,10 +56,10 @@ you need help with restoring a backup after a crash.]]> https://github.com/dillardblom/ng_backup https://github.com/dillardblom/ng_backup/issues https://github.com/dillardblom/ng_backup - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.2/screenshots/1-backups.png - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.2/screenshots/2-restore.png - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.2/screenshots/3-settings.png - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.2/screenshots/4-bin.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.3/screenshots/1-backups.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.3/screenshots/2-restore.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.3/screenshots/3-settings.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.3/screenshots/4-bin.png sodium diff --git a/docs/DISASTER-RECOVERY.md b/docs/DISASTER-RECOVERY.md index cd517b6..745f7be 100644 --- a/docs/DISASTER-RECOVERY.md +++ b/docs/DISASTER-RECOVERY.md @@ -123,10 +123,12 @@ without `--install-apps`: occ maintenance:mode --off && occ backup:restore:full offsite --salvage --force - Files that cannot be read are left out and listed at the end; everything else is restored. -- **Check the disk first if the reason is "Size mismatch".** In this version a full disk looks like - damaged data: the message then points at `--salvage`, and with `--salvage` the restore goes on and - lists the remaining files. The backup itself is fine: make room and run the restore again as in - step 8, without `--salvage` unless the backup is damaged as well. To be fixed in 1.0.3. +- A full disk is not damage: the restore stops, also with `--salvage`, and says so. Make room and + run the restore again as in step 8. +- **On version 1.0.2, check the disk first if the reason is "Size mismatch".** There a full disk + looks like damaged data: the message points at `--salvage`, and with `--salvage` the restore goes + on and lists the remaining files. The backup itself is fine. Update NG Backup, make room and run + the restore again as in step 8. - If the database cannot be restored (its dump is damaged, or it does not fit this installation), the database stays as the fresh installation made it, and the data directory and config of the backup go to a folder of their own in the data directory, @@ -149,9 +151,12 @@ The command then ends with exit code 2 instead of 0. The same option exists for - From Nextcloud All-in-One to a regular server: also check the cache settings (`memcache.*`, `redis`), which point at AIO's containers. - Log in, open a few files, a calendar and a contact. -- Empty folders do not come back: a backup only records files (to be fixed in 1.0.3, see - [issue #27](https://github.com/dillardblom/ng_backup/issues/27)). `occ files:scan` then removes - them from the file cache, so it reports them as removed. +- `occ files:scan` can report many folders as "Updated". That is expected: a restore does not yet put + back the modification time of folders with files in them, so they show the time of the restore. + The files themselves keep their time and content. +- Empty folders only come back from backups made with version 1.0.3 or newer. With an older + restore point they are gone: `occ files:scan` then removes them from the file cache, so it reports + them as removed. - Start background jobs again (cron), as in step 2 in reverse. **10. Point your domain at the new server** and check that clients reconnect. diff --git a/lib/Backend/StorageBackend.php b/lib/Backend/StorageBackend.php index 7126f15..ca57e72 100644 --- a/lib/Backend/StorageBackend.php +++ b/lib/Backend/StorageBackend.php @@ -124,7 +124,13 @@ final class StorageBackend implements IBackend, IReadsAhead { // null: not supported or failing for now; false: not opened, which need not mean // missing. Either way the storage's own stream decides. } - $fh = $this->storage->fopen($this->abs($path), 'r'); + try { + $fh = $this->storage->fopen($this->abs($path), 'r'); + } catch (\Exception $e) { + // Some storages throw for what is not a missing file (SMB: a connection that broke). + // As a BackendException, the caller reads once more and --salvage can go on. + throw new BackendException('Cannot read ' . $path . ': ' . $e->getMessage(), 0, $e); + } if ($fh === false) { throw new BackendException('Not found: ' . $path); } diff --git a/lib/Controller/AdminApiController.php b/lib/Controller/AdminApiController.php index 85a2683..37995ec 100644 --- a/lib/Controller/AdminApiController.php +++ b/lib/Controller/AdminApiController.php @@ -82,6 +82,8 @@ final class AdminApiController extends Controller { /** Seconds of backup work one tick() does: well within a normal web request. */ public const TICK_SECONDS = 10.0; + /** Seconds one request for the Bin reads, before the page shows how far it is and asks for more. */ + private const TRASH_PAGE_SECONDS = 5.0; /** * Move running backups forward from the open admin page, when background jobs do not run on a @@ -95,7 +97,10 @@ final class AdminApiController extends Controller { } $deadline = microtime(true) + self::TICK_SECONDS; $stepped = 0; - foreach ($this->runs->findRunning(null, BackupService::KIND_FULL) as $run) { + $running = $this->runs->findRunning(null, BackupService::KIND_FULL); + // Longest wait first: a run that takes all of a tick does not keep another from its turn. + usort($running, static fn ($a, $b) => $a->getUpdatedAt() <=> $b->getUpdatedAt()); + foreach ($running as $run) { if (microtime(true) >= $deadline) { break; } @@ -174,15 +179,18 @@ final class AdminApiController extends Controller { } } - public function trash(int $targetId): JSONResponse { + public function trash(int $targetId, string $after = ''): JSONResponse { try { $delay = $this->keys->deleteDelayDays() * 86400; + $repo = $this->targets->repository($this->targets->get((string)$targetId)); + // In parts, so that the page can show how far it is (a read per removed restore point). + $page = $repo->trashPage($after, microtime(true) + self::TRASH_PAGE_SECONDS); $out = []; - foreach ($this->targets->repository($this->targets->get((string)$targetId))->trash() as $id => $info) { + foreach ($page['items'] as $id => $info) { $out[] = ['id' => $id, 'label' => $info['label'], 'time' => $info['time'], 'removedAt' => $info['forgottenAt'], 'by' => $info['by'], 'deleteAfter' => $info['forgottenAt'] + $delay]; } - return new JSONResponse($out); + return new JSONResponse(['total' => $page['total'], 'items' => $out]); } catch (\Throwable $e) { return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } diff --git a/lib/Db/DbDumper.php b/lib/Db/DbDumper.php index d8402e3..e8eff0c 100644 --- a/lib/Db/DbDumper.php +++ b/lib/Db/DbDumper.php @@ -157,17 +157,27 @@ final class DbDumper { } // Tables the backup knows the structure of are created first (DDL is not transactional // everywhere); tables it does not know are left out, as asked. - foreach ($missing as $table) { - if (isset($manifest['tables'][$table]['schema'])) { - $this->db->createSchemaManager()->createTable(self::tableFromStructure($table, $manifest['tables'][$table]['schema'], $this->tableOptions())); - $report['created'][] = $table; - } else { - $report['missing'][] = $table; + try { + foreach ($missing as $table) { + if (isset($manifest['tables'][$table]['schema'])) { + $report['created'][] = $table; // first: creating it can stop halfway (its indexes) + $this->db->createSchemaManager()->createTable(self::tableFromStructure($table, $manifest['tables'][$table]['schema'], $this->tableOptions())); + } else { + $report['missing'][] = $table; + } } + } catch (\Throwable $e) { + $this->dropCreatedTables($report); + throw $e; } $total = 0; $foreignKeys = []; - $this->db->beginTransaction(); + try { + $this->db->beginTransaction(); + } catch (\Throwable $e) { + $this->dropCreatedTables($report); + throw $e; + } try { $foreignKeys = $this->suspendForeignKeys(); // first: on SQLite it also takes the write lock foreach ($manifest['tables'] as $table => $info) { @@ -220,21 +230,6 @@ final class DbDumper { $this->restoreSequences($manifest['sequences'] ?? [], $report['missing'], array_merge($report['restored'], $report['skipped'])); $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 @@ -242,11 +237,44 @@ final class DbDumper { if (in_array('SET FOREIGN_KEY_CHECKS = 1', $foreignKeys, true)) { $this->db->executeStatement('SET FOREIGN_KEY_CHECKS = 1'); } + $this->dropCreatedTables($report); throw $e; } + // Committed. 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, and the created tables stay. + 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(); + } + } + } return $total; } + /** + * The restore did not go through: a rollback does not undo CREATE TABLE everywhere. Left behind, + * the empty tables of an app would stop that app's installation later ("table already exists"). + * + * @param array{created: list, ...} $report + */ + private function dropCreatedTables(array &$report): void { + foreach ($report['created'] as $table) { + try { + $this->db->createSchemaManager()->dropTable($table); + } catch (\Throwable) { + // the error that brought us here is the one to report; a run again uses the table + } + } + $report['created'] = []; + } + /** * The table prefix of the instance the dump was made on (from its appconfig table), or null * when the dump has none. diff --git a/lib/Job/BackupRun.php b/lib/Job/BackupRun.php index e7cc3f1..4a0c379 100644 --- a/lib/Job/BackupRun.php +++ b/lib/Job/BackupRun.php @@ -55,9 +55,32 @@ final class BackupRun { }); $parent = new ParentCursor($repo, $state['parent'], $state['parentPos']); $lines = []; + $emptyDirs = []; $finishedWalk = true; foreach ($walker->walk($state['after']) as $logical => $path) { + if (!mb_check_encoding($logical, 'UTF-8') || !Repository::isSafePath($logical)) { + // A name a snapshot cannot hold (a backslash, a control character, not UTF-8). One + // such path must not make a backup fail, or the whole snapshot unreadable later. + // The cursor stays where it is: the state is JSON, which cannot hold such a name. A + // next step meets it again, and it is reported once. + self::recordUnreadable($state, $stats, mb_scrub($logical, 'UTF-8')); + continue; + } + if (is_dir($path)) { + $mtime = @filemtime($path); + if ($mtime === false) { + continue; // removed while walking + } + $emptyDirs[] = json_encode([$logical, $mtime], JSON_UNESCAPED_SLASHES | JSON_THROW_ON_ERROR); + $stats['emptyDirs'] = ($stats['emptyDirs'] ?? 0) + 1; + $state['after'] = $logical; + if (microtime(true) >= $deadline) { + $finishedWalk = false; + break; + } + continue; + } $version = Repository::fileVersion($path); if ($version === 'missing') { continue; // deleted while walking @@ -136,6 +159,11 @@ final class BackupRun { $writer->write(implode("\n", $lines) . "\n"); $state['segments'][] = $writer->finish(); } + if ($emptyDirs !== []) { + $writer = $repo->blobWriter($stats); + $writer->write(implode("\n", $emptyDirs) . "\n"); + $state['dirSegments'][] = $writer->finish(); + } $repo->flushPacks(); $state['parentPos'] = $parent->position(); $state['stats'] = $stats; @@ -150,10 +178,18 @@ final class BackupRun { if ($state['phase'] === 'finish') { // Every segment ends with a newline, so they read as one stream: fetched together, // several at a time where the location allows it. + // Empty folders go in the meta record, not between the files: a version that does not + // know them reads the snapshot as before, and only restores no empty folders. + $emptyDirs = []; + foreach (isset($state['dirSegments']) ? $repo->readLines(array_values(array_merge(...$state['dirSegments']))) : [] as $line) { + if ($line !== '') { + $emptyDirs[] = json_decode($line, true, 8, JSON_THROW_ON_ERROR); + } + } $repo->putSnapshot($state['snapshot'], ['time' => $state['started'], 'parent' => $state['parent'], 'label' => $state['label'], 'roots' => array_keys($state['roots']), 'stats' => $state['stats'], - 'warnings' => self::warnings($state)] + $state['meta'], + 'warnings' => self::warnings($state)] + ($emptyDirs === [] ? [] : ['emptyDirs' => $emptyDirs]) + $state['meta'], $repo->readLines(array_values(array_merge(...$state['segments'])))); $state['phase'] = 'commit'; } diff --git a/lib/Job/TreeWalker.php b/lib/Job/TreeWalker.php index ce342b4..25366dc 100644 --- a/lib/Job/TreeWalker.php +++ b/lib/Job/TreeWalker.php @@ -9,7 +9,9 @@ namespace OCA\NgBackup\Job; /** * Walks one or more source roots in a stable depth-first order (entries of a directory sorted - * by name, children right after their directory), yielding "/" for files. + * by name, children right after their directory), yielding "/" for files + * and for directories with nothing in them to walk (the caller tells them apart with is_dir()). + * A root itself without a name is never yielded. * * The same order is used for comparing paths (compare()), so a walk can resume after a cursor * and be merge-joined with the previous snapshot without holding any list in memory. @@ -66,6 +68,7 @@ final class TreeWalker { } $names = array_values(array_diff($names, ['.', '..'])); sort($names, SORT_STRING); + $empty = true; // no file or directory below it that is walked, whatever the cursor skips foreach ($names as $name) { $childLogical = $logical === '' ? $name : $logical . '/' . $name; $childAbs = $abs . '/' . $name; @@ -73,20 +76,26 @@ final class TreeWalker { continue; // never follow symlinks out of the data directory } if (is_dir($childAbs)) { + $empty = $empty && $this->isExcluded($childLogical, true); // Skip a directory entirely if everything in it sorts before the cursor. if ($after !== null && self::compare($childLogical, $after) < 0 && !self::isPrefix($childLogical, $after)) { continue; } yield from $this->walkDir($childAbs, $childLogical, $after); } elseif (is_file($childAbs)) { + $excluded = $this->isExcluded($childLogical, false); + $empty = $empty && $excluded; if ($after !== null && self::compare($childLogical, $after) <= 0) { continue; } - if (!$this->isExcluded($childLogical, false)) { + if (!$excluded) { yield $childLogical => $childAbs; } } } + if ($empty && $logical !== '' && ($after === null || self::compare($logical, $after) > 0)) { + yield $logical => $abs; + } } private function isExcluded(string $logical, bool $isDir): bool { diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 44be374..a631000 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -44,6 +44,8 @@ final class Repository { private ?int $maxBytes = null; private Catalog $catalog; private PackReader $packReader; + /** @var array> meta record of the snapshot read last */ + private array $lastMeta = []; /** verify() lists the pack folders from this many packs on (see presentPacks()); tests lower it. */ public int $listPacksAbove = 256; @@ -257,8 +259,8 @@ final class Repository { // that was being written when the process was killed stays (8 MiB, DR test 2026-11-05) $stale = glob(rtrim($target, '/') . '/.ngb-restore-spool-*', GLOB_ONLYDIR); foreach ($stale === false ? [] : $stale as $dir) { - if (is_link($dir)) { - continue; + if (is_link($dir) || preg_match('/^\.ngb-restore-spool-[0-9a-f]{8}$/', basename($dir)) !== 1) { + continue; // not a spool of ours, whatever it is called } $files = glob($dir . '/*'); foreach ($files === false ? [] : $files as $file) { @@ -313,40 +315,39 @@ final class Repository { if (!self::underPrefix($entry['p'], $prefix)) { continue; } + if ($heartbeat !== null) { + $heartbeat(); // also for a long row of files that are skipped or have no data + } $dest = self::destination($target, $entry['p'], $prefix, $stripPrefix); - if ($keepUnchanged && self::inPlace($dest, $entry)) { - // below the target as written by an earlier run of this restore (checked then) - $count++; - continue; - } - if (!is_dir(dirname($dest))) { - mkdir(dirname($dest), 0700, true); - } - // Defence in depth: the resolved directory must stay below the target (no symlink escapes). - $base = realpath($target); - $dir = realpath(dirname($dest)); - if ($base === false || $dir === false || ($dir !== $base && !str_starts_with($dir, $base . '/')) || is_link($dest)) { - throw new RepositoryException('Refusing to write outside the restore directory: ' . $entry['p']); + $dir = self::directoryInTarget($target, dirname($dest), $entry['p']); + if (is_link($dest)) { + throw new RestoreTargetException('Refusing to write outside the restore directory: ' . $entry['p']); } if (!isset($cleaned[$dir])) { self::removeStaleTemporaries($dir); $cleaned[$dir] = true; } + if ($keepUnchanged && self::inPlace($dest, $entry)) { + $count++; + continue; + } // Write next to the destination and rename after all blobs verified: at most one file // is ever present twice. $tmp = $dest . '.ngb-restore-' . bin2hex(random_bytes(6)); $out = @fopen($tmp, 'xb'); // fails if anything (including a symlink) already exists there if ($out === false) { - throw new RepositoryException('Cannot create a temporary file for ' . $entry['p']); + throw new RestoreTargetException('Cannot create a temporary file for ' . $entry['p'] . ' (disk full, or no permission?)'); } try { foreach ($entry['b'] as $id) { - fwrite($out, $this->restoreBlob($id, $plan)); + self::writeAll($out, $this->restoreBlob($id, $plan), $entry['p']); if ($heartbeat !== null) { $heartbeat(); } } - fclose($out); + if (!@fclose($out)) { + throw new RestoreTargetException('Cannot write ' . $entry['p'] . ' to the restore directory (disk full?)'); + } clearstatcache(true, $tmp); if (filesize($tmp) !== $entry['s']) { throw new RepositoryException('Size mismatch restoring ' . $entry['p']); @@ -363,19 +364,112 @@ final class Repository { } $skipped[$entry['p']] = $e->getMessage(); continue; + } catch (\Throwable $e) { + if (is_resource($out)) { + @fclose($out); + } + @unlink($tmp); + throw $e; } if (!rename($tmp, $dest)) { @unlink($tmp); - throw new RepositoryException('Cannot put the restored file in place: ' . $entry['p']); + throw new RestoreTargetException('Cannot put the restored file in place: ' . $entry['p']); } if (!touch($dest, $entry['m'])) { - throw new RepositoryException('Cannot set the modification time of ' . $entry['p']); + throw new RestoreTargetException('Cannot set the modification time of ' . $entry['p']); } $count++; } + // Last: a file can only be restored into a folder that exists, and the meta record was + // read along with the files, so this asks nothing more of the location. + foreach ($this->emptyDirs($snapshotId) as [$path, $mtime]) { + if (self::underPrefix($path, $prefix)) { + if ($heartbeat !== null) { + $heartbeat(); + } + // The prefix itself, stripped, is the target (for a file it is the file's name there). + $dest = $stripPrefix && $path === rtrim($prefix, '/') ? rtrim($target, '/') : self::destination($target, $path, $prefix, $stripPrefix); + self::restoreEmptyDir($target, $dest, $path, $mtime); + } + } return $count; } + /** + * The folders without anything in them that a snapshot recorded (none before 1.0.3). + * + * @return list path, modification time + */ + public function emptyDirs(string $snapshotId): array { + $meta = $this->lastMeta[$snapshotId] ?? $this->snapshotMeta($snapshotId); + $dirs = []; + foreach (is_array($meta['emptyDirs'] ?? null) ? $meta['emptyDirs'] : [] as $dir) { + // One folder that makes no sense is no reason to give up on the files of a restore. + if (is_array($dir) && is_string($dir[0] ?? null) && self::isSafePath($dir[0]) && is_int($dir[1] ?? null)) { + $dirs[] = [$dir[0], $dir[1]]; + } + } + return $dirs; + } + + private static function restoreEmptyDir(string $target, string $dest, string $path, int $mtime): void { + if (is_link($dest)) { + throw new RestoreTargetException('Refusing to write outside the restore directory: ' . $path); + } + $existed = is_dir($dest); + self::directoryInTarget($target, $dest, $path); + if (!$existed && !@touch($dest, $mtime)) { + throw new RestoreTargetException('Cannot set the modification time of ' . $path); + } + } + + /** + * Make sure a folder exists below the target, without following a link out of it: what exists + * of the path is checked before anything is created. + * + * @return string the folder's real path + */ + private static function directoryInTarget(string $target, string $dir, string $path): string { + if (!is_dir($target) && !@mkdir($target, 0700, true) && !is_dir($target)) { + throw new RestoreTargetException('Cannot create the restore directory ' . $target); + } + $base = realpath($target); + $existing = $dir; + while (!file_exists($existing) && !is_link($existing) && dirname($existing) !== $existing) { + $existing = dirname($existing); + } + $real = realpath($existing); + if ($base === false || $real === false || ($real !== $base && !str_starts_with($real, $base . '/'))) { + throw new RestoreTargetException('Refusing to write outside the restore directory: ' . $path); + } + if ($existing === $dir) { + if (!is_dir($real)) { + throw new RestoreTargetException('Cannot create the folder for ' . $path . ': a file is in the way'); + } + return $real; + } + if (!@mkdir($dir, 0700, true) && !is_dir($dir)) { + throw new RestoreTargetException('Cannot create the folder for ' . $path . ' (disk full, no permission, or a file in the way?)'); + } + return (string)realpath($dir); + } + + /** + * Write all of $data, or stop: a short write leaves a file that looks damaged only by its size. + * A write may take only part of the data and still be fine; one that takes nothing is not. + * + * @param resource $out + */ + public static function writeAll($out, string $data, string $path): void { + while ($data !== '') { + $written = @fwrite($out, $data); + if ($written === false || $written === 0) { + throw new RestoreTargetException('Cannot write ' . $path . ' to the restore directory (disk full?)'); + } + $data = substr($data, $written); + } + } + /** Where restore() writes the snapshot path $path. */ private static function destination(string $target, string $path, string $prefix, bool $stripPrefix): string { $relPath = $path; @@ -722,6 +816,34 @@ final class Repository { return array_keys($doomed); } + /** + * Part of trash(), for a page that shows how far it is: each entry is a read of its own on the + * location. Reads the entries with an id after $after, in order, until $deadline + * (microtime(true)), at least one. The id, not a position: a cleanup that changes the trash + * between two parts then makes no entry come twice or be missed. + * + * @return array{total: int, items: array>} + */ + public function trashPage(string $after, float $deadline): array { + $ids = array_map('strval', array_keys($this->catalog->trash)); + sort($ids, SORT_STRING); + $items = []; + foreach ($ids as $id) { + if (strcmp($id, $after) <= 0) { + continue; + } + try { + $items[$id] = $this->trashInfo($id); + } catch (\Throwable) { + throw new RepositoryException("Trash entry $id listed in the catalog is missing on the location (possible tampering)"); + } + if (microtime(true) >= $deadline) { + break; + } + } + return ['total' => count($ids), 'items' => $items]; + } + private function trashInfo(string $snapshotId): array { return json_decode($this->getObject('trash/info/' . $snapshotId), true, 512, JSON_THROW_ON_ERROR); } @@ -1281,7 +1403,10 @@ final class Repository { $this->cipher->decrypt($fh, $plain, 'snapshot:' . $snapshotId); fclose($fh); rewind($plain); - fgets($plain); // meta line + $meta = json_decode((string)fgets($plain), true); + if (is_array($meta)) { + $this->lastMeta = [$snapshotId => $meta]; // a snapshot never changes + } while (($line = fgets($plain)) !== false) { $entry = json_decode($line, true, 512, JSON_THROW_ON_ERROR); if (!is_array($entry) || !is_string($entry['p'] ?? null) || !self::isSafePath($entry['p']) diff --git a/lib/Repository/RestoreTargetException.php b/lib/Repository/RestoreTargetException.php new file mode 100644 index 0000000..fd5f9e5 --- /dev/null +++ b/lib/Repository/RestoreTargetException.php @@ -0,0 +1,15 @@ +leases->with(PruneService::lockKey($target), LeaseService::EXCLUSIVE, 60, fn () => $this->createRun($target, $label)); } catch (LockedException) { - throw new \RuntimeException('A cleanup or another start for ' . $target->getName() . ' is running; try again later'); + throw new \RuntimeException('A cleanup, a restore or another start for ' . $target->getName() . ' is running, or one was stopped by force (that clears by itself within two hours); try again later'); } } diff --git a/lib/Service/DatabaseAlreadyRestoredException.php b/lib/Service/DatabaseAlreadyRestoredException.php new file mode 100644 index 0000000..9567917 --- /dev/null +++ b/lib/Service/DatabaseAlreadyRestoredException.php @@ -0,0 +1,12 @@ +executeStatement(); } + /** Remove the shared leases on $name, for a caller that knows their holders were stopped. */ + public function dropShared(string $name): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('ngb_leases') + ->where($qb->expr()->eq('name', $qb->createNamedParameter($name))) + ->andWhere($qb->expr()->eq('mode', $qb->createNamedParameter(self::SHARED))); + $qb->executeStatement(); + } + /** Wraps $refresh so calling it often (per file, per row batch) only touches the database every $seconds. */ public static function throttled(callable $refresh, int $seconds = 60): \Closure { $last = time(); diff --git a/lib/Service/RestoreFullService.php b/lib/Service/RestoreFullService.php index 1749aa9..4c2cc68 100644 --- a/lib/Service/RestoreFullService.php +++ b/lib/Service/RestoreFullService.php @@ -151,6 +151,12 @@ final class RestoreFullService { // The database cannot go into this installation; the files can. $check['dbUnreadable'] = 'the database cannot be restored here: ' . $this->describeBlockers($check); } + if ($interrupted) { + // A restore that was killed leaves its leases, for up to two hours: they would keep + // the first backup of the recovered server waiting after this restore completes. + // In maintenance mode, on a partly restored instance, nothing else reads the location. + $this->leases->dropShared(PruneService::lockKey($target)); + } $appConfig->setValueInt(\OCA\NgBackup\AppInfo\Application::APP_ID, self::INTERRUPTED_KEY, time()); $result = $this->restoreInMaintenance($target, $snapshotId, $skipMissingTables, $check, $salvage); $appConfig->deleteKey(\OCA\NgBackup\AppInfo\Application::APP_ID, self::INTERRUPTED_KEY); @@ -175,6 +181,8 @@ final class RestoreFullService { if ($dbError === null) { try { $this->restoreDatabase($repo, $meta, $manifest, $dbReport, $beat, $skipMissingTables); + } catch (DatabaseAlreadyRestoredException $e) { + throw $e; } catch (\Throwable $e) { if (!$salvage) { throw $e; @@ -201,6 +209,8 @@ final class RestoreFullService { try { /** @var array $backupConfig */ $backupConfig = $this->fetchBackupConfig($target, $snapshotId, $salvage); + } catch (\OCA\NgBackup\Repository\RestoreTargetException $e) { + throw $e; // nowhere to put it: not a config.php that cannot be read } catch (\Throwable $e) { if (!$salvage) { throw $e; @@ -259,8 +269,20 @@ final class RestoreFullService { $dumper = new DbDumper($this->db->getInner(), $prefix); try { $dumper->restore($repo, $manifest, $dbReport, $beat, $skipMissingTables); - } finally { + } catch (\Throwable $e) { + try { + $this->restoreOwnAppConfig($ownAppConfig); + } catch (\Throwable) { + // rolled back, so they are still there: the error that brought us here is the one to report + } + throw $e; + } + try { $this->restoreOwnAppConfig($ownAppConfig); + } catch (\Throwable $e) { + // The old server's database is in place by now: not a case for "the database stays as + // the fresh installation made it" under --salvage. A run again restores these rows. + throw new DatabaseAlreadyRestoredException('The database was restored, but NG Backup could not put back its own settings: ' . $e->getMessage(), 0, $e); } } diff --git a/lib/Service/RestoreService.php b/lib/Service/RestoreService.php index 8df6e0a..83d3153 100644 --- a/lib/Service/RestoreService.php +++ b/lib/Service/RestoreService.php @@ -61,7 +61,8 @@ final class RestoreService { $prefix = trim($path, '/'); $prefix = $prefix === '' ? '' : $prefix . '/'; $children = []; - foreach ($this->targets->repository($target)->entries($snapshotId) as $e) { + $repo = $this->targets->repository($target); + foreach ($repo->entries($snapshotId) as $e) { if ($prefix !== '' && !str_starts_with($e['p'], $prefix)) { continue; } @@ -75,6 +76,12 @@ final class RestoreService { } $children[$name] = $c; } + foreach ($repo->emptyDirs($snapshotId) as [$dir]) { // read along with the entries + if ($prefix === '' || str_starts_with($dir, $prefix)) { + $name = explode('/', substr($dir, strlen($prefix)), 2)[0]; + $children[$name] ??= ['name' => $name, 'type' => 'dir', 'size' => 0, 'mtime' => null, 'files' => 0]; + } + } ksort($children, SORT_STRING); return array_values($children); } @@ -123,7 +130,17 @@ final class RestoreService { $entries[substr($e['p'], strlen($source) + 1)] = $e; } } - if ($entries === []) { + // Folders without anything in them, relative to $source ('' when $source itself is one). + /** @var array $emptyDirs modification times; a folder called "2026" is an integer key */ + $emptyDirs = []; + foreach ($single ? [] : $repo->emptyDirs($snapshotId) as [$dir, $mtime]) { + if ($dir === $source) { + $emptyDirs[''] = $mtime; + } elseif (str_starts_with($dir, $source . '/')) { + $emptyDirs[substr($dir, strlen($source) + 1)] = $mtime; + } + } + if ($entries === [] && $emptyDirs === []) { throw new NotFoundException("Nothing at $source in snapshot $snapshotId"); } @@ -134,6 +151,7 @@ final class RestoreService { } elseif ($mode !== self::MODE_MERGE && $mode !== self::MODE_REPLACE) { throw new \InvalidArgumentException("Unknown mode $mode"); } + $destExisted = $parentRel === '' || $userFolder->nodeExists($parentRel); $dest = $this->ensureFolder($userFolder, $parentRel); $names = $single ? [basename($rel) => $entries['']] : $entries; @@ -156,9 +174,21 @@ final class RestoreService { } } + foreach ($emptyDirs as $dir => $mtime) { + $dir = (string)$dir; + $existed = $dir === '' ? $destExisted : $dest->nodeExists($dir); // one that is there keeps its own time + $folder = $this->ensureFolder($dest, $dir); + if (!$existed) { + $folder->touch($mtime); + } + if ($progress !== null) { + $progress($restored, count($names), $dir); // keeps the lease alive too + } + } + $trashed = 0; if ($mode === self::MODE_REPLACE && !$single) { - $trashed = $this->trashExtras($dest, '', array_flip(array_keys($names))); + $trashed = $this->trashExtras($dest, '', array_flip(array_keys($names)) + $emptyDirs); } return ['restored' => $restored, 'trashed' => $trashed, 'target' => $uid . ':' . $userFolder->getRelativePath($dest->getPath())]; } @@ -178,7 +208,7 @@ final class RestoreService { throw new \InvalidArgumentException('Invalid path: ' . $prefix); } if (!is_dir($directory) && !mkdir($directory, 0750, true) && !is_dir($directory)) { - throw new \RuntimeException("Cannot create $directory"); + throw new \OCA\NgBackup\Repository\RestoreTargetException("Cannot create $directory (disk full, or no permission?)"); } return $this->withSharedLock($target, function (callable $refresh) use ($target, $snapshotId, $prefix, $directory, $stripPrefix, $heartbeat, &$skipped, $keepUnchanged): int { $own = LeaseService::throttled($refresh); @@ -262,7 +292,7 @@ final class RestoreService { foreach ($folder->getDirectoryListing() as $node) { $rel = $prefix === '' ? $node->getName() : $prefix . '/' . $node->getName(); if ($node instanceof Folder) { - $hasKept = false; + $hasKept = isset($keep[$rel]); // a folder that was empty in the snapshot foreach (array_keys($keep) as $k) { if (str_starts_with((string)$k, $rel . '/')) { $hasKept = true; diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c138868..d6ad913 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -186,6 +186,7 @@ db]]> db]]> db]]> + db]]> diff --git a/src/api.js b/src/api.js index a81df4e..85b391b 100644 --- a/src/api.js +++ b/src/api.js @@ -21,7 +21,7 @@ export default { addSlot: (passphrase, label) => call('post', '/key/slots', { passphrase, label }), replaceSlot: (slot, passphrase, label) => call('put', `/key/slots/${slot}`, { passphrase, label }), removeSlot: (slot) => call('delete', `/key/slots/${slot}`), - trash: (targetId) => call('get', `/targets/${targetId}/trash`), + trash: (targetId, after = '') => call('get', `/targets/${targetId}/trash`, { after }), untrash: (targetId, snapshotId) => call('post', `/targets/${targetId}/trash/${snapshotId}`), kitUrl: () => url('/key/kit'), confirmKit: (accepted, phrase, code) => call('post', '/key/confirm', { accepted, phrase, code }), diff --git a/src/components/Bin.vue b/src/components/Bin.vue index 9b7bb76..91d1a40 100644 --- a/src/components/Bin.vue +++ b/src/components/Bin.vue @@ -6,8 +6,18 @@ {{ error }} - - {{ t('ng_backup', 'The bin is empty.') }} +
+ + +

{{ t('ng_backup', 'This can take a while: every removed restore point is read from the location.') }}

+
+ {{ t('ng_backup', 'The bin is empty.') }}