From 6540a9314aeeef3f9d608a769e7c44f2e132b879 Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Fri, 6 Nov 2026 23:38:40 +0100 Subject: [PATCH] A name a snapshot cannot hold gets a notification of its own Reported apart from unreadable files, with example paths and the advice to rename them, instead of advice about permissions. Start of the 1.1.0 plan in PLAN.md. --- CHANGELOG.md | 7 +++++++ PLAN.md | 11 ++++++++++- lib/Command/RunBackup.php | 3 +++ lib/Job/BackupRun.php | 20 ++++++++++++++++++-- lib/Notification/Notifier.php | 4 ++++ lib/Service/AlertService.php | 10 ++++++++++ lib/Service/BackupService.php | 7 ++++++- tests/Unit/Repository/EmptyDirsTest.php | 6 +++++- 8 files changed, 63 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf1ce07..9fdd9ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,13 @@ 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. +## [Unreleased] + +### Changed +- A file or folder that is left out because of its name now gets a notification of its own, in the + log and in `occ backup:run`. It says the item is missing from the backup, gives example paths and + asks to rename it; before, it was reported as unreadable, with advice about permissions. + ## [1.0.3] - 2026-11-06 ### Fixed diff --git a/PLAN.md b/PLAN.md index a6c9bc9..04db100 100644 --- a/PLAN.md +++ b/PLAN.md @@ -472,9 +472,12 @@ in README "Known limitations": 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. + **1.0.3 uitgebracht (2026-11-06)**, ook op nc01 geïnstalleerd. - **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. + Na 1.0.3 gaat het werk naar `1.1.0`. Een spoedfix die niet kan wachten gaat op `1.0.4` vanaf `main`, + krijgt een eigen merge en release; daarna `main` in `1.1.0` mergen, zodat daar met de fix getest wordt. - **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 @@ -569,7 +572,13 @@ in README "Known limitations": - **Polish:** `target:add` toonde bij het eerste indexbestand al "about 642 min left" (S3, 4.568 bestanden, daarna binnen een minuut klaar): een schatting pas na ~5% of na meer dan een paar bestanden. -- **1.1.0:** +- **1.1.0** (geen harde datum: een week was een voorzet, langer mag; de releasefrequentie mag omlaag, + besluit 2026-11-06): + **Betere melding voor namen die een snapshot niet kan bevatten** (backslash, stuurteken, geen UTF-8): + een eigen melding in plaats van "unreadable" met het advies over rechten. Zegt dat deze bestanden of + mappen in de backup ontbreken, geeft voorbeelden van de paden en vraagt ze te hernoemen; de volgende + backup neemt ze mee. Alleen als Nextcloud-melding: de app mailt niet zelf, of meldingen gemaild worden + is de keuze van de beheerder (besluit 2026-11-06). 1. `backup:key:import` zonder kit: de sleutel-slots van de locatie (`config`) lezen, met de kit als reserve als `config` ontbreekt of niet opent. Vergt een andere volgorde van `target:add` en sleutelimport (het gevoeligste pad), daarom niet in rc1. Daarna de diff --git a/lib/Command/RunBackup.php b/lib/Command/RunBackup.php index b5aa06e..f107ea7 100644 --- a/lib/Command/RunBackup.php +++ b/lib/Command/RunBackup.php @@ -79,6 +79,9 @@ HELP); if (($f['unreadable'] ?? 0) > 0) { $output->writeln(sprintf(' warning: %d unreadable files or folders are missing from this backup (see the server log)', $f['unreadable'])); } + if (($f['badNames'] ?? 0) > 0) { + $output->writeln(sprintf(' warning: %d files or folders are missing from this backup because their name is not supported; rename them and the next backup includes them (see the server log)', $f['badNames'])); + } if (($f['changing'] ?? 0) > 0) { $output->writeln(sprintf(' warning: %d files kept changing and were saved as read (see the server log)', $f['changing'])); } diff --git a/lib/Job/BackupRun.php b/lib/Job/BackupRun.php index 4a0c379..ac1ac90 100644 --- a/lib/Job/BackupRun.php +++ b/lib/Job/BackupRun.php @@ -64,7 +64,7 @@ final class BackupRun { // 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')); + self::recordBadName($state, $stats, $logical); continue; } if (is_dir($path)) { @@ -250,10 +250,26 @@ final class BackupRun { $stats['unreadable'] = ($stats['unreadable'] ?? 0) + 1; } - /** @return array{unreadable: list, changing: list} examples for the report (first 50 each) */ + /** + * A file or directory left out because of its name. Remembered like an unreadable one, in a form + * that fits the JSON state and is safe to show: control characters and invalid bytes replaced. + */ + private static function recordBadName(array &$state, array &$stats, string $logical): void { + $shown = (string)preg_replace('/[\x00-\x1f\x7f]/', '?', mb_scrub($logical, 'UTF-8')); + if (isset($state['badNames'][$shown])) { + return; + } + if (count($state['badNames'] ?? []) < 1000) { + $state['badNames'][$shown] = true; + } + $stats['badNames'] = ($stats['badNames'] ?? 0) + 1; + } + + /** @return array{unreadable: list, badNames: list, changing: list} examples for the report (first 50 each) */ public static function warnings(array $state): array { return [ 'unreadable' => array_slice(array_map('strval', array_keys($state['unreadable'] ?? [])), 0, 50), + 'badNames' => array_slice(array_map('strval', array_keys($state['badNames'] ?? [])), 0, 50), 'changing' => array_slice(array_map('strval', array_values($state['changing'] ?? [])), 0, 50), ]; } diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index c1d05ae..87a2f3b 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -46,6 +46,10 @@ final class Notifier implements INotifier { $notification->setParsedSubject($l->t('Backup to %1$s finished with warnings: %2$s unreadable, %3$s changed while being read', [$p['target'], $p['unreadable'], $p['changing']])); $notification->setParsedMessage($l->t('Unreadable files and folders are missing from this backup (check their permissions). Files that kept changing were saved as they were while being read. Examples: %s', [$p['examples'] ?? ''])); break; + case 'run_bad_names': + $notification->setParsedSubject($l->n('Backup to %s finished, but %n item is missing because of its name', 'Backup to %s finished, but %n items are missing because of their names', (int)$p['count'], [$p['target']])); + $notification->setParsedMessage($l->t('Names with a backslash or a control character, or that are not valid UTF-8, are not supported, so these files and folders are not in this backup. Rename them; the next backup includes them. Examples: %s', [$p['examples'] ?? ''])); + break; case 'quota_warning': $notification->setParsedSubject($l->t('Backup location %1$s is at %2$s%% of its limit (%3$s of %4$s)', [$p['target'], $p['percent'], $p['used'], $p['limit']])); $notification->setParsedMessage($l->t('Backups stop when the limit is reached. Raise the limit, or let the retention policy remove older backups.')); diff --git a/lib/Service/AlertService.php b/lib/Service/AlertService.php index ace1dc8..c9bd12e 100644 --- a/lib/Service/AlertService.php +++ b/lib/Service/AlertService.php @@ -61,6 +61,16 @@ final class AlertService { 'examples' => mb_strimwidth(implode(', ', array_merge($examples['unreadable'], $examples['changing'])), 0, 300, '…')]); } + /** + * A backup that finished but left out files or folders because of their name. + * + * @param list $examples + */ + public function runBadNames(Run $run, string $targetName, int $count, array $examples): void { + $this->notifyAdmins('run_bad_names', 'run-' . $run->getId(), ['target' => $targetName, 'count' => (string)$count, + 'examples' => mb_strimwidth(implode(', ', $examples), 0, 300, '…')]); + } + /** * Remember how much is stored on a location (for the settings page) and warn administrators * once a day when it is at 80% of its limit or more. diff --git a/lib/Service/BackupService.php b/lib/Service/BackupService.php index 0d45f55..c106f20 100644 --- a/lib/Service/BackupService.php +++ b/lib/Service/BackupService.php @@ -253,8 +253,13 @@ final class BackupService { try { $this->alerts->storedBytes($target, $repo->storedBytes()); $stats = $state['files']['stats']; + $warnings = BackupRun::warnings($state['files']); + if (($stats['badNames'] ?? 0) > 0) { + $this->logger->warning(sprintf('NG Backup run %d to %s: %d left out because their name is not supported (rename them): %s', + $run->getId(), $target->getName(), $stats['badNames'], implode(', ', $warnings['badNames']))); + $this->alerts->runBadNames($run, $target->getName(), (int)$stats['badNames'], $warnings['badNames']); + } if (($stats['unreadable'] ?? 0) > 0 || ($stats['changing'] ?? 0) > 0) { - $warnings = BackupRun::warnings($state['files']); $this->logger->warning(sprintf('NG Backup run %d to %s: %d unreadable (missing from the snapshot): %s; %d changed while being read: %s', $run->getId(), $target->getName(), $stats['unreadable'] ?? 0, implode(', ', $warnings['unreadable']), $stats['changing'] ?? 0, implode(', ', $warnings['changing']))); diff --git a/tests/Unit/Repository/EmptyDirsTest.php b/tests/Unit/Repository/EmptyDirsTest.php index b318c58..a88b877 100644 --- a/tests/Unit/Repository/EmptyDirsTest.php +++ b/tests/Unit/Repository/EmptyDirsTest.php @@ -82,7 +82,11 @@ class EmptyDirsTest extends TestCase { $state = json_decode(json_encode(BackupRun::step($this->repo(), $state, 0.0), JSON_THROW_ON_ERROR), true, 512, JSON_THROW_ON_ERROR); } $this->assertSame('commit', $state['phase']); - $this->assertSame(4, $state['stats']['unreadable'], 'each reported once, over several steps'); + $this->assertSame(4, $state['stats']['badNames'], 'each reported once, over several steps'); + $this->assertArrayNotHasKey('unreadable', $state['stats'], 'not a problem with permissions'); + $shown = BackupRun::warnings($state)['badNames']; + $this->assertContains('data/alice/files/line?break', $shown, 'no control character in a message'); + $this->assertSame($shown, array_filter($shown, fn (string $name): bool => mb_check_encoding($name, 'UTF-8'))); $this->repo()->recordSnapshot($state['snapshot']); $this->assertSame(['data/alice/files/Hollow/Nested', 'data/skipped/inner'], array_column($this->repo()->emptyDirs($state['snapshot']), 0));