From e58b86fc48af39d3fdcea41fddcb78e4c8d80d73 Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Sat, 7 Nov 2026 00:04:00 +0100 Subject: [PATCH] A schedule per location, and locations backed up one after the other Off, once a day or every N hours from a time, each in its own time zone. Existing locations keep the time that held for all of them. A backup that has to wait shows as queued, and its restore point has the time it really began. --- CHANGELOG.md | 7 ++ PLAN.md | 4 + README.md | 1 + appinfo/info.xml | 1 + appinfo/routes.php | 2 +- docs/HOWTO.md | 4 +- lib/BackgroundJob/BackupJob.php | 26 +++---- lib/Command/Status.php | 5 +- lib/Command/TargetList.php | 3 +- lib/Command/TargetSchedule.php | 50 ++++++++++++ lib/Controller/AdminApiController.php | 28 +++---- lib/Db/Target.php | 10 +++ .../Version000008Date20261107010000.php | 49 ++++++++++++ lib/Service/AlertService.php | 5 +- lib/Service/BackupService.php | 16 ++++ lib/Service/LocationSchedule.php | 78 +++++++++++++++++++ lib/Service/Schedule.php | 33 ++------ lib/Service/TargetService.php | 19 +++++ psalm-baseline.xml | 8 ++ src/api.js | 2 +- src/components/Backups.vue | 30 ++----- src/components/LocationScheduleRow.vue | 77 ++++++++++++++++++ tests/Unit/Service/LocationScheduleTest.php | 74 ++++++++++++++++++ tests/Unit/Service/ScheduleTest.php | 11 --- tests/integration/roundtrip.sh | 38 +++++++++ 25 files changed, 487 insertions(+), 94 deletions(-) create mode 100644 lib/Command/TargetSchedule.php create mode 100644 lib/Migration/Version000008Date20261107010000.php create mode 100644 lib/Service/LocationSchedule.php create mode 100644 src/components/LocationScheduleRow.vue create mode 100644 tests/Unit/Service/LocationScheduleTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e72c7c0..024943c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,15 @@ shown under 1.0.0. The git tag of a release candidate goes after the date. - A note on a restore point that you can change afterwards, on the restore page or with `occ backup:note`. Pins and notes are stored encrypted on the location, so a new server sees them. An older version of NG Backup can still read the location, but drops them when it writes to it. +- A schedule per location: off, once a day, or every N hours from a given time, on the page under + Backups or with `occ backup:target:schedule`. After the update every location keeps the time that + held for all of them; a new location starts with the schedule of the first one. ### Changed +- Locations are backed up one after the other: a backup that has to wait shows as queued. A restore + point has the time its backup really began, not the time it was queued. +- Choosing a time no longer starts a backup right away when that time has already passed today. +- A location without a schedule no longer gets the notification that it has had no recent backup. - 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. diff --git a/PLAN.md b/PLAN.md index 04db100..9823465 100644 --- a/PLAN.md +++ b/PLAN.md @@ -579,6 +579,10 @@ in README "Known limitations": 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). + **Stand op branch `1.1.0` (2026-11-07):** klaar: betere melding, punten 4, 5, 6 en 8. Pin en notitie staan + in de catalogus, met een kopie in de database voor de lijsten. Een moment van vóór het instellen van + een schema start niets. Geen schema is een keuze: dan ook geen melding "geen recente backup". Nog niet + getest: de update vanaf 1.0.3, de pagina in een browser, de tekst van de nieuwe melding in beeld. 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/README.md b/README.md index 402984e..b8b136c 100644 --- a/README.md +++ b/README.md @@ -329,6 +329,7 @@ On the backlog; know them before you rely on NG Backup: | `backup:key:init`, `backup:key:kit`, `backup:key:confirm` | create the key, recovery kit, confirmation | | `backup:key:slots`, `backup:key:slot:add/replace/remove` | passphrase slots | | `backup:target:backends/add/edit/list/remove/append-only` | backup locations | +| `backup:target:schedule` | when a location is backed up: off, daily, or every N hours | | `backup:run`, `backup:status`, `backup:list` | back up, status (runs of the last 7 days; `--all` for all), restore points | | `backup:cancel ` | stop a running backup | | `backup:browse`, `backup:restore` | browse and restore | diff --git a/appinfo/info.xml b/appinfo/info.xml index 1f15e3e..a20fdd1 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -90,6 +90,7 @@ you need help with restoring a backup after a crash.]]> OCA\NgBackup\Command\TargetAppendOnly OCA\NgBackup\Command\TargetTrustCurrent OCA\NgBackup\Command\TargetLimit + OCA\NgBackup\Command\TargetSchedule OCA\NgBackup\Command\Trash OCA\NgBackup\Command\TrashRestore OCA\NgBackup\Command\KeySlots diff --git a/appinfo/routes.php b/appinfo/routes.php index 22d510c..467974d 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -25,7 +25,7 @@ return [ ['name' => 'AdminApi#setTargetLimit', 'url' => '/api/targets/{id}/limit', 'verb' => 'PUT'], ['name' => 'AdminApi#startBackup', 'url' => '/api/backups', 'verb' => 'POST'], ['name' => 'AdminApi#tick', 'url' => '/api/tick', 'verb' => 'POST'], - ['name' => 'AdminApi#setSchedule', 'url' => '/api/schedule', 'verb' => 'PUT'], + ['name' => 'AdminApi#setTargetSchedule', 'url' => '/api/targets/{id}/schedule', 'verb' => 'PUT'], ['name' => 'AdminApi#snapshots', 'url' => '/api/targets/{targetId}/snapshots', 'verb' => 'GET'], ['name' => 'AdminApi#setSnapshotNote', 'url' => '/api/targets/{targetId}/snapshots/{snapshotId}/note', 'verb' => 'PUT'], ['name' => 'AdminApi#setSnapshotPinned', 'url' => '/api/targets/{targetId}/snapshots/{snapshotId}/pin', 'verb' => 'PUT'], diff --git a/docs/HOWTO.md b/docs/HOWTO.md index 552e724..e640f95 100644 --- a/docs/HOWTO.md +++ b/docs/HOWTO.md @@ -79,7 +79,9 @@ occ backup:list Later runs upload only changed data. An unchanged run uploads (almost) nothing; the output shows the number of uploaded bytes, so check that value when you test incremental behaviour. -Schedule `occ backup:run` from system cron (or use the Administration settings, if you prefer). +Set when each location is backed up in the Administration settings, or with +`occ backup:target:schedule offsite 02:00` (once a day), `02:00/6` (every 6 hours from 02:00) or `off`. +Locations are backed up one after the other. ## 4. Check the backup regularly diff --git a/lib/BackgroundJob/BackupJob.php b/lib/BackgroundJob/BackupJob.php index 76dd5fc..99fa12d 100644 --- a/lib/BackgroundJob/BackupJob.php +++ b/lib/BackgroundJob/BackupJob.php @@ -43,17 +43,16 @@ final class BackupJob extends TimedJob { $this->setTimeSensitivity(IJob::TIME_SENSITIVE); } - private const MAX_DAILY_ATTEMPTS = 3; - protected function run($argument): void { $budget = PHP_SAPI === 'cli' ? 240.0 : 20.0; $deadline = microtime(true) + $budget; $this->startScheduled(); - foreach ($this->runs->findRunning(null, BackupService::KIND_FULL) as $run) { + // One after the other: runs that read the same disks at the same time are not faster together. + foreach ($this->backups->runsInTurn() as $run) { while (microtime(true) < $deadline && $run->getStatus() === 'running') { $next = $this->backups->step($run, min($deadline, microtime(true) + $budget)); if ($next === $run) { - break; // step() hands back the same object when another process holds the run's lease + break 2; // step() hands back the same object when another process holds the run's lease } $run = $next; } @@ -102,21 +101,18 @@ final class BackupJob extends TimedJob { $this->appConfig->setValueString(Application::APP_ID, 'last_prune_week', $due); } - /** Daily schedule: "HH:MM" in the schedule's time zone (see Schedule); empty = no schedule. */ + /** Start the backups the schedules of the locations ask for; they run in turn (see LocationSchedule). */ private function startScheduled(): void { - $todayAt = $this->schedule->todayAt(); - if ($todayAt === null || time() < $todayAt) { - return; - } + $now = time(); foreach ($this->targets->list() as $t) { - $last = $this->runs->lastFinished($t->getId(), BackupService::KIND_FULL); - if ($this->runs->findRunning($t->getId(), BackupService::KIND_FULL) !== [] || ($last !== null && $last->getStartedAt() >= $todayAt)) { + $moment = \OCA\NgBackup\Service\LocationSchedule::lastMoment((string)$t->getSchedule(), $this->schedule->zoneOr($t->getScheduleZone()), $now); + if ($moment === null || $this->runs->findRunning($t->getId(), BackupService::KIND_FULL) !== []) { continue; } - // After a failure: retry at most MAX_DAILY_ATTEMPTS times a day, an hour apart, instead - // of starting (and notifying about) a new full run every five minutes all day. - $today = $this->runs->startedSince($t->getId(), BackupService::KIND_FULL, $todayAt); - if (count($today) >= self::MAX_DAILY_ATTEMPTS || ($today !== [] && time() - (int)$today[0]->getFinishedAt() < 3600)) { + $attempts = $this->runs->startedSince($t->getId(), BackupService::KIND_FULL, $moment); + if (!\OCA\NgBackup\Service\LocationSchedule::isDue($moment, (int)$t->getScheduleSince(), + $this->runs->lastFinished($t->getId(), BackupService::KIND_FULL)?->getStartedAt(), + count($attempts), ($attempts[0] ?? null)?->getFinishedAt(), $now)) { continue; } try { diff --git a/lib/Command/Status.php b/lib/Command/Status.php index 65a1260..9f36c5b 100644 --- a/lib/Command/Status.php +++ b/lib/Command/Status.php @@ -31,6 +31,7 @@ final class Status extends Command { private VerifyService $verify, private Schedule $schedule, private CronHealth $cron, + private \OCA\NgBackup\Service\BackupService $backups, ) { parent::__construct(); } @@ -61,9 +62,11 @@ final class Status extends Command { $rows = []; $all = (bool)$input->getOption('all'); $shown = $this->runs->findShown($all ? null : time() - self::RECENT_DAYS * 86400); + $queued = array_map(static fn (\OCA\NgBackup\Db\Run $r): int => $r->getId(), array_slice($this->backups->runsInTurn(), 1)); foreach ($shown as $r) { $s = json_decode($r->getStats() ?? '{}', true)['files'] ?? []; - $rows[] = [$r->getId(), $names[$r->getTargetId()] ?? '#' . $r->getTargetId(), $r->getKind(), $r->getStatus() === 'running' ? self::runningStatus($r) : $r->getStatus(), + $rows[] = [$r->getId(), $names[$r->getTargetId()] ?? '#' . $r->getTargetId(), $r->getKind(), + in_array($r->getId(), $queued, true) ? 'queued' : ($r->getStatus() === 'running' ? self::runningStatus($r) : $r->getStatus()), Format::time($r->getStartedAt(), $zone), $r->getFinishedAt() ? Format::duration($r->getFinishedAt() - $r->getStartedAt()) : '', $s['files'] ?? '', isset($s['bytes']) ? self::size((int)$s['bytes']) : '', isset($s['uploaded']) ? self::size((int)$s['uploaded']) : '', mb_strimwidth((string)$r->getError(), 0, 50, '…')]; diff --git a/lib/Command/TargetList.php b/lib/Command/TargetList.php index 058b10c..0e5e068 100644 --- a/lib/Command/TargetList.php +++ b/lib/Command/TargetList.php @@ -31,7 +31,8 @@ final class TargetList extends Base { $rows = []; foreach ($this->targets->list() as $t) { $row = ['id' => $t->getId(), 'name' => $t->getName(), 'backend' => $t->getBackend(), 'path' => $t->getBasePath(), - 'repository' => $t->getRepositoryId(), 'added' => date('Y-m-d H:i', $t->getCreatedAt())]; + 'repository' => $t->getRepositoryId(), 'added' => date('Y-m-d H:i', $t->getCreatedAt()), + 'schedule' => (string)$t->getSchedule() !== '' ? $t->getSchedule() : 'off']; if ($input->getOption('test')) { try { $repo = $this->targets->repository($t); diff --git a/lib/Command/TargetSchedule.php b/lib/Command/TargetSchedule.php new file mode 100644 index 0000000..39233c2 --- /dev/null +++ b/lib/Command/TargetSchedule.php @@ -0,0 +1,50 @@ +setName('backup:target:schedule') + ->setDescription('Show or set when a location is backed up') + ->addArgument('target', InputArgument::REQUIRED, 'Location name or id') + ->addArgument('schedule', InputArgument::OPTIONAL, '"off", HH:MM for once a day, or HH:MM/N for every N hours from HH:MM; omit to show the current one') + ->addOption('timezone', null, InputOption::VALUE_REQUIRED, 'Time zone of the time, e.g. Europe/Amsterdam'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $t = $this->targets->get($input->getArgument('target')); + $schedule = $input->getArgument('schedule'); + $zone = $input->getOption('timezone') ?? $this->schedule->zoneOr($t->getScheduleZone()); + if ($schedule !== null) { + try { + $t = $this->targets->setSchedule($t, $schedule, (string)$zone); + } catch (\InvalidArgumentException $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; + } + } + $current = (string)$t->getSchedule(); + $output->writeln($t->getName() . ': ' . ($current === '' ? 'off' : $current . ' (' . $zone . ')')); + return 0; + } +} diff --git a/lib/Controller/AdminApiController.php b/lib/Controller/AdminApiController.php index a27e594..eddcd45 100644 --- a/lib/Controller/AdminApiController.php +++ b/lib/Controller/AdminApiController.php @@ -59,10 +59,12 @@ final class AdminApiController extends Controller { foreach ($this->targets->list() as $t) { $targets[] = ['id' => $t->getId(), 'name' => $t->getName(), 'backend' => $t->getBackend(), 'path' => $t->getBasePath(), 'repositoryId' => $t->getRepositoryId(), 'createdAt' => $t->getCreatedAt(), 'appendOnly' => (bool)$t->getAppendOnly(), - 'maxBytes' => $t->getMaxBytes(), 'storedBytes' => $this->alerts->lastStoredBytes($t)]; + 'maxBytes' => $t->getMaxBytes(), 'storedBytes' => $this->alerts->lastStoredBytes($t), + 'schedule' => (string)$t->getSchedule(), 'scheduleZone' => $this->schedule->zoneOr($t->getScheduleZone())]; } $shown = $this->runs->findShown($allRuns ? null : time() - \OCA\NgBackup\Command\Status::RECENT_DAYS * 86400); - $runs = array_map(fn (Run $r) => $this->runJson($r), $shown); + $queued = array_flip(array_map(static fn (Run $r): int => $r->getId(), array_slice($this->backups->runsInTurn(), 1))); + $runs = array_map(fn (Run $r) => $this->runJson($r) + ['queued' => isset($queued[$r->getId()])], $shown); return new JSONResponse([ 'key' => ['initialized' => $this->keys->isInitialized(), 'fingerprint' => $this->keys->fingerprint(), 'confirmation' => $this->keys->recoveryKitConfirmation(), 'everConfirmed' => $this->keys->isInitialized() && $this->keys->everConfirmed(), @@ -74,8 +76,6 @@ final class AdminApiController extends Controller { 'runs' => $runs, 'olderRuns' => $allRuns ? 0 : $this->runs->countAll() - count($shown), 'recentDays' => \OCA\NgBackup\Command\Status::RECENT_DAYS, - 'schedule' => $this->schedule->get()['time'], - 'scheduleTimezone' => $this->schedule->get()['timezone'], 'cron' => $this->cron->check(), ]); } @@ -97,16 +97,16 @@ final class AdminApiController extends Controller { } $deadline = microtime(true) + self::TICK_SECONDS; $stepped = 0; - $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) { + foreach ($this->backups->runsInTurn() as $run) { if (microtime(true) >= $deadline) { break; } // step() returns at once when the background job (or another tab) holds the run. - $this->backups->step($run, $deadline); + $next = $this->backups->step($run, $deadline); $stepped++; + if ($next->getStatus() === Run::RUNNING) { + break; // not finished: the next one waits for its turn + } } return new JSONResponse(['stepped' => $stepped]); } @@ -278,14 +278,14 @@ final class AdminApiController extends Controller { return new JSONResponse($this->runJson($run)); } - /** @param string $timezone the administrator's zone (the browser's); empty: keep the current one */ - public function setSchedule(string $time, string $timezone = ''): JSONResponse { + /** @param string $timezone the administrator's zone (the browser's); empty: the default one */ + public function setTargetSchedule(int $id, string $schedule, string $timezone = ''): JSONResponse { try { - $this->schedule->set($time, $timezone !== '' ? $timezone : $this->schedule->get()['timezone']); - } catch (\InvalidArgumentException $e) { + $t = $this->targets->setSchedule($this->targets->get((string)$id), $schedule, $timezone !== '' ? $timezone : $this->schedule->get()['timezone']); + } catch (\Throwable $e) { return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } - return new JSONResponse(['schedule' => $time, 'scheduleTimezone' => $this->schedule->get()['timezone']]); + return new JSONResponse(['schedule' => $t->getSchedule(), 'scheduleZone' => $t->getScheduleZone()]); } public function snapshots(int $targetId): JSONResponse { diff --git a/lib/Db/Target.php b/lib/Db/Target.php index 0930a62..a4d924c 100644 --- a/lib/Db/Target.php +++ b/lib/Db/Target.php @@ -28,6 +28,12 @@ use OCP\AppFramework\Db\Entity; * @method void setMaxBytes(?int $v) * @method int getCreatedAt() * @method void setCreatedAt(int $v) + * @method string|null getSchedule() + * @method void setSchedule(?string $v) + * @method string|null getScheduleZone() + * @method void setScheduleZone(?string $v) + * @method int|null getScheduleSince() + * @method void setScheduleSince(?int $v) */ final class Target extends Entity { protected string $name = ''; @@ -39,8 +45,12 @@ final class Target extends Entity { protected int $createdAt = 0; protected ?bool $appendOnly = null; protected ?int $maxBytes = null; + protected ?string $schedule = null; + protected ?string $scheduleZone = null; + protected ?int $scheduleSince = null; public function __construct() { + $this->addType('scheduleSince', 'integer'); $this->addType('createdAt', 'integer'); $this->addType('appendOnly', 'boolean'); $this->addType('maxBytes', 'integer'); diff --git a/lib/Migration/Version000008Date20261107010000.php b/lib/Migration/Version000008Date20261107010000.php new file mode 100644 index 0000000..ea4ed14 --- /dev/null +++ b/lib/Migration/Version000008Date20261107010000.php @@ -0,0 +1,49 @@ +getTable('ngb_targets'); + if ($t->hasColumn('schedule')) { + return null; + } + $t->addColumn('schedule', Types::STRING, ['notnull' => false, 'length' => 16]); + $t->addColumn('schedule_zone', Types::STRING, ['notnull' => false, 'length' => 64]); + $t->addColumn('schedule_since', Types::BIGINT, ['notnull' => false, 'unsigned' => true]); + return $schema; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + ['time' => $time, 'timezone' => $zone] = $this->schedule->get(); + $qb = $this->db->getQueryBuilder(); + $qb->update('ngb_targets') + ->set('schedule', $qb->createNamedParameter(Schedule::validTime($time) ? LocationSchedule::normalize($time) : '')) + ->set('schedule_zone', $qb->createNamedParameter($zone)) + ->where($qb->expr()->isNull('schedule')); + $qb->executeStatement(); + } +} diff --git a/lib/Service/AlertService.php b/lib/Service/AlertService.php index c9bd12e..5f53271 100644 --- a/lib/Service/AlertService.php +++ b/lib/Service/AlertService.php @@ -103,7 +103,10 @@ final class AlertService { /** At most one notification per location per day. */ public function checkStale(Target $target): void { - $last = $this->runs->lastFinished($target->getId(), BackupService::KIND_FULL); + if ((string)$target->getSchedule() === '') { + return; // no schedule is a choice, as for a location that is only connected now and then + } + $last =$this->runs->lastFinished($target->getId(), BackupService::KIND_FULL); $lastTime = $last?->getFinishedAt() ?? $target->getCreatedAt(); if (time() - $lastTime < self::STALE_AFTER || $this->runs->findRunning($target->getId(), BackupService::KIND_FULL) !== []) { return; diff --git a/lib/Service/BackupService.php b/lib/Service/BackupService.php index c106f20..534a6cd 100644 --- a/lib/Service/BackupService.php +++ b/lib/Service/BackupService.php @@ -58,6 +58,18 @@ final class BackupService { $this->openedWith = $opened; } + /** + * The running backups in the order they get their turn: first come, first served. The first one + * runs, the others are queued; only one at a time reads the disks. + * + * @return list + */ + public function runsInTurn(): array { + $running = $this->runs->findRunning(null, self::KIND_FULL); + usort($running, static fn (Run $a, Run $b): int => [$a->getStartedAt(), $a->getId()] <=> [$b->getStartedAt(), $b->getId()]); + return $running; + } + public function start(Target $target, string $label = ''): Run { $label = trim($label); if (mb_strlen($label) > self::LABEL_MAX) { @@ -210,6 +222,10 @@ final class BackupService { $state = json_decode($run->getState(), true, 512, JSON_THROW_ON_ERROR); if ($run->getPhase() === 'db') { + if (!isset($state['dump'])) { + // It may have been queued for hours: the restore point has the time of its data. + $state['files']['started'] = gmdate('c'); + } $dumper = new DbDumper($this->db->getInner(), $this->config->getSystemValueString('dbtableprefix', 'oc_')); $dbStats = $state['dbStats'] ?? []; // With system cron the whole dump usually fits in one step (one transaction, fully diff --git a/lib/Service/LocationSchedule.php b/lib/Service/LocationSchedule.php new file mode 100644 index 0000000..5726f75 --- /dev/null +++ b/lib/Service/LocationSchedule.php @@ -0,0 +1,78 @@ + 24) { + throw new \InvalidArgumentException('Every 1 to 24 hours'); + } + return $hours === 24 ? $m[1] : $m[1] . '/' . $hours; + } + + /** + * The most recent scheduled moment at or before $now; null without a schedule. + * + * @param non-empty-string $zone + */ + public static function lastMoment(string $schedule, string $zone, int $now): ?int { + if ($schedule === '') { + return null; + } + $tz = new \DateTimeZone($zone); + $today = (new \DateTimeImmutable('@' . $now))->setTimezone($tz); + $moments = array_merge(self::moments($schedule, $today->modify('-1 day'), $tz), self::moments($schedule, $today, $tz)); + $last = null; + foreach ($moments as $moment) { + if ($moment <= $now && ($last === null || $moment > $last)) { + $last = $moment; + } + } + return $last; + } + + /** @return list the moments of one day */ + private static function moments(string $schedule, \DateTimeImmutable $day, \DateTimeZone $tz): array { + [$time, $every] = array_pad(explode('/', $schedule), 2, '24'); + [$hour, $minute] = array_map('intval', explode(':', $time)); + $moments = []; + for ($h = $hour; $h < 24; $h += (int)$every) { + $moments[] = (new \DateTimeImmutable($day->format('Y-m-d') . sprintf(' %02d:%02d', $h, $minute), $tz))->getTimestamp(); + } + return $moments; + } + + /** + * Whether a backup has to start for $moment. One that was made since then is enough, also when + * several moments went by during a long backup. A moment from before the schedule was set does + * not count: choosing a time must not start a backup right away. + */ + public static function isDue(int $moment, int $since, ?int $lastDoneStarted, int $attempts, ?int $lastAttemptFinished, int $now): bool { + if ($moment < $since || ($lastDoneStarted !== null && $lastDoneStarted >= $moment) || $attempts >= self::MAX_ATTEMPTS) { + return false; + } + return $attempts === 0 || $now - (int)$lastAttemptFinished >= self::RETRY_AFTER; + } +} diff --git a/lib/Service/Schedule.php b/lib/Service/Schedule.php index 326c537..58f80e8 100644 --- a/lib/Service/Schedule.php +++ b/lib/Service/Schedule.php @@ -13,10 +13,11 @@ use OCP\IConfig; use OCP\IGroupManager; /** - * The daily backup time: "HH:MM" in a time zone. Nextcloud runs PHP in UTC, so a time without a - * zone meant UTC (issue #19: 03:00 ran at 05:00 in Berlin). A time saved before there was a zone - * is converted once to the zone of the server or of an administrator, at today's offset: it then - * shows as it effectively ran (02:30 UTC becomes 04:30 Europe/Berlin in summer). + * The time zone times are shown in, and the daily time from before each location had its own + * schedule (see LocationSchedule). Nextcloud runs PHP in UTC, so a time without a zone meant UTC + * (issue #19: 03:00 ran at 05:00 in Berlin). A time saved before there was a zone is converted once + * to the zone of the server or of an administrator, at today's offset: it then shows as it + * effectively ran (02:30 UTC becomes 04:30 Europe/Berlin in summer). */ final class Schedule { public const TIME_KEY = 'schedule_time'; @@ -44,27 +45,9 @@ final class Schedule { return ['time' => $time, 'timezone' => $zone]; } - /** @throws \InvalidArgumentException for a malformed time or an unknown zone */ - public function set(string $time, string $zone): void { - if ($time !== '' && !self::validTime($time)) { - throw new \InvalidArgumentException('Use HH:MM or empty'); - } - if (!self::validZone($zone)) { - throw new \InvalidArgumentException('Unknown time zone: ' . $zone); - } - $this->appConfig->setValueString(Application::APP_ID, self::ZONE_KEY, $zone); - $this->appConfig->setValueString(Application::APP_ID, self::TIME_KEY, $time); - } - - /** Today's scheduled moment as a Unix time, in the schedule's zone; null without a schedule. */ - public function todayAt(?int $now = null): ?int { - ['time' => $time, 'timezone' => $zone] = $this->get(); - if ($time === '' || !self::validTime($time)) { - return null; - } - $tz = new \DateTimeZone($zone); - $day = (new \DateTimeImmutable('@' . ($now ?? time())))->setTimezone($tz)->format('Y-m-d'); - return (new \DateTimeImmutable($day . ' ' . $time, $tz))->getTimestamp(); + /** @return non-empty-string the zone of a location's schedule, or else this one */ + public function zoneOr(?string $zone): string { + return $zone !== null && self::validZone($zone) ? $zone : $this->get()['timezone']; } /** diff --git a/lib/Service/TargetService.php b/lib/Service/TargetService.php index b30308a..80ddc18 100644 --- a/lib/Service/TargetService.php +++ b/lib/Service/TargetService.php @@ -78,6 +78,11 @@ final class TargetService { $target->setBasePath(trim($basePath, '/')); $target->setRepositoryId($repo->id()); $target->setCreatedAt(time()); + // Like the location before it: a backup scheme is usually one scheme. + $first = $this->mapper->findAll()[0] ?? null; + $target->setSchedule($first?->getSchedule() ?? ''); + $target->setScheduleZone($first?->getScheduleZone()); + $target->setScheduleSince(time()); $target = $this->mapper->insert($target); if (!$created) { // Disaster recovery: occ backup:list reads this local cache, not the repository @@ -268,6 +273,20 @@ final class TargetService { } /** Size limit for the data stored on this location (null = none). */ + /** + * @param string $schedule see LocationSchedule + * @throws \InvalidArgumentException + */ + public function setSchedule(Target $target, string $schedule, string $zone): Target { + if (!Schedule::validZone($zone)) { + throw new \InvalidArgumentException('Unknown time zone: ' . $zone); + } + $target->setSchedule(LocationSchedule::normalize($schedule)); + $target->setScheduleZone($zone); + $target->setScheduleSince(time()); + return $this->mapper->update($target); + } + public function setMaxBytes(Target $target, ?int $maxBytes): Target { $target->setMaxBytes($maxBytes); return $this->mapper->update($target); diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c8bf83b..cbdc470 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -300,6 +300,14 @@ + + + + + + + + diff --git a/src/api.js b/src/api.js index 63dabbd..1000eca 100644 --- a/src/api.js +++ b/src/api.js @@ -34,7 +34,7 @@ export default { setTargetLimit: (id, maxGb) => call('put', `/targets/${id}/limit`, { maxGb }), startBackup: (targetId, label) => call('post', '/backups', { targetId, label }), tick: () => call('post', '/tick'), - setSchedule: (time, timezone) => call('put', '/schedule', { time, timezone }), + setTargetSchedule: (id, schedule, timezone) => call('put', `/targets/${id}/schedule`, { schedule, timezone }), snapshots: (targetId) => call('get', `/targets/${targetId}/snapshots`), setSnapshotNote: (targetId, snapshotId, note) => call('put', `/targets/${targetId}/snapshots/${snapshotId}/note`, { note }), setSnapshotPinned: (targetId, snapshotId, pinned) => call('put', `/targets/${targetId}/snapshots/${snapshotId}/pin`, { pinned }), diff --git a/src/components/Backups.vue b/src/components/Backups.vue index 8a2feea..e1a8310 100644 --- a/src/components/Backups.vue +++ b/src/components/Backups.vue @@ -5,10 +5,7 @@ diff --git a/tests/Unit/Service/LocationScheduleTest.php b/tests/Unit/Service/LocationScheduleTest.php new file mode 100644 index 0000000..7a994a0 --- /dev/null +++ b/tests/Unit/Service/LocationScheduleTest.php @@ -0,0 +1,74 @@ +assertSame('', S::normalize('off')); + $this->assertSame('', S::normalize(' ')); + $this->assertSame('02:30', S::normalize('02:30')); + $this->assertSame('02:30', S::normalize('02:30/24')); + $this->assertSame('00:00/6', S::normalize('00:00/6')); + foreach (['2:30', '24:00', '02:30/0', '02:30/25', 'daily', '02:30/x'] as $bad) { + try { + S::normalize($bad); + $this->fail("$bad accepted"); + } catch (\InvalidArgumentException) { + $this->addToAssertionCount(1); + } + } + } + + public function testOffHasNoMoment(): void { + $this->assertNull(S::lastMoment('', self::BERLIN, self::at('2026-11-10 12:00 UTC'))); + } + + public function testDailyIsInTheZoneOfTheLocation(): void { + $this->assertSame(self::at('2026-11-10 02:00 UTC'), S::lastMoment('03:00', self::BERLIN, self::at('2026-11-10 12:00 UTC'))); + $this->assertSame(self::at('2026-11-09 02:00 UTC'), S::lastMoment('03:00', self::BERLIN, self::at('2026-11-10 01:00 UTC')), 'not yet today'); + } + + public function testEveryNHoursStartsOverTheNextDay(): void { + $zone = 'UTC'; + $this->assertSame(self::at('2026-11-10 12:00 UTC'), S::lastMoment('02:00/5', $zone, self::at('2026-11-10 13:00 UTC'))); + $this->assertSame(self::at('2026-11-09 22:00 UTC'), S::lastMoment('02:00/5', $zone, self::at('2026-11-10 01:59 UTC')), '02, 07, 12, 17, 22, then 02 again'); + $this->assertSame(self::at('2026-11-10 02:00 UTC'), S::lastMoment('02:00/5', $zone, self::at('2026-11-10 02:00 UTC')), 'at the moment itself'); + } + + public function testAClockChangeInTheNightStillGivesOneMoment(): void { + $this->assertSame(self::at('2026-03-29 01:30 UTC'), S::lastMoment('02:30', self::BERLIN, self::at('2026-03-29 12:00 UTC')), '02:30 does not exist that night'); + $this->assertNotNull(S::lastMoment('02:30', self::BERLIN, self::at('2026-10-25 12:00 UTC'))); + } + + public function testChoosingATimeDoesNotStartABackup(): void { + $moment = self::at('2026-11-10 02:00 UTC'); + $this->assertFalse(S::isDue($moment, $moment + 1, null, 0, null, $moment + 3600)); + $this->assertTrue(S::isDue($moment, $moment - 1, null, 0, null, $moment + 3600)); + $this->assertTrue(S::isDue($moment, 0, null, 0, null, $moment), 'a location from before schedules per location'); + } + + public function testABackupSinceTheMomentIsEnough(): void { + $moment = 1000000; + $this->assertFalse(S::isDue($moment, 0, $moment, 1, $moment + 10, $moment + 20)); + $this->assertTrue(S::isDue($moment, 0, $moment - 1, 0, null, $moment + 20), 'it started before this moment: one more'); + } + + public function testAfterAFailureOnlyAFewAttemptsAnHourApart(): void { + $moment = 1000000; + $finished = $moment + 60; + $this->assertFalse(S::isDue($moment, 0, null, 1, $finished, $finished + S::RETRY_AFTER - 1)); + $this->assertTrue(S::isDue($moment, 0, null, 1, $finished, $finished + S::RETRY_AFTER)); + $this->assertFalse(S::isDue($moment, 0, null, S::MAX_ATTEMPTS, $finished, $finished + 86400)); + } +} diff --git a/tests/Unit/Service/ScheduleTest.php b/tests/Unit/Service/ScheduleTest.php index db8458d..0d82f7e 100644 --- a/tests/Unit/Service/ScheduleTest.php +++ b/tests/Unit/Service/ScheduleTest.php @@ -57,15 +57,4 @@ class ScheduleTest extends TestCase { $this->assertSame(['time' => '02:30', 'timezone' => 'UTC'], $this->schedule()->get()); } - public function testTodayAtIsTheMomentInTheSchedulesZone(): void { - $this->values = [Schedule::TIME_KEY => '03:00', Schedule::ZONE_KEY => 'Europe/Berlin']; - $at = $this->schedule()->todayAt(strtotime('2026-10-22 12:00 UTC')); - $this->assertSame(strtotime('2026-10-22 01:00 UTC'), $at, '03:00 in Berlin (summer time) is 01:00 UTC'); - } - - public function testAnUnknownZoneOrABadTimeIsRefused(): void { - $s = $this->schedule(); - $this->expectException(\InvalidArgumentException::class); - $s->set('03:00', 'Mars/Olympus'); - } } diff --git a/tests/integration/roundtrip.sh b/tests/integration/roundtrip.sh index 3738e85..9e0644d 100755 --- a/tests/integration/roundtrip.sh +++ b/tests/integration/roundtrip.sh @@ -101,6 +101,24 @@ ok "without system cron: warning in backup:status and the API, backup finished b # The Bin reads in parts, as the page does (what is in the parts: CatalogTest) api "$API/targets/$TID/trash" | php -r '$p = json_decode(stream_get_contents(STDIN), true); exit(is_int($p["total"] ?? null) && is_array($p["items"] ?? null) && count($p["items"]) <= $p["total"] ? 0 : 1);' || fail "the Bin API does not give a total and items" ok "the Bin API reads in parts, with a total" +# Locations are backed up one after the other: the second one started is queued until the first is done. +for L in 2 3; do + $X sh -c "mkdir -p /tmp/ngb-rt-target$L" + occ backup:target:add ci$L local -a null::null -o datadir=/tmp/ngb-rt-target$L --path=repo >/dev/null || fail "target add ci$L" +done +IDS=$(api "$API/status" | php -r 'foreach (json_decode(stream_get_contents(STDIN), true)["targets"] as $t) { if ($t["name"] === "ci2" || $t["name"] === "ci3") { echo $t["id"], " "; } }') +for ID in $IDS; do api -X POST -d "{\"targetId\": $ID, \"label\": \"turn\"}" "$API/backups" >/dev/null || fail "start a backup to location $ID"; done +occ backup:status | grep "queued" >/dev/null || fail "the second backup is not queued" +QUEUED=$(api "$API/status" | php -r 'echo count(array_filter(json_decode(stream_get_contents(STDIN), true)["runs"], fn ($r) => $r["queued"]));') +[ "$QUEUED" = 1 ] || fail "$QUEUED backups queued instead of 1" +for i in $(seq 1 60); do + api -X POST "$API/tick" >/dev/null || fail "tick" + RUNNING=$(api "$API/status" | php -r 'echo count(array_filter(json_decode(stream_get_contents(STDIN), true)["runs"], fn ($r) => $r["status"] === "running"));') + [ "$RUNNING" != 0 ] || break +done +[ "$RUNNING" = 0 ] || fail "the queued backups did not finish" +occ backup:list ci3 | grep " turn " >/dev/null || fail "the queued backup made no snapshot" +ok "locations in turn: the second backup queued, and done after the first" occ background:cron >/dev/null $X php -f /var/www/html/cron.php >/dev/null 2>&1 || true occ backup:status | grep "system cron" >/dev/null && fail "backup:status still warns after cron ran" @@ -109,6 +127,26 @@ api "$API/status" | php -r 'exit(json_decode(stream_get_contents(STDIN), true)[" ok "with a system cron that ran: no warning in backup:status or the API" occ user:delete "$ADM" >/dev/null +# A schedule per location. Choosing a time starts nothing; the moment itself does, and only there. +JOB=$(occ background-job:list --class='OCA\NgBackup\BackgroundJob\BackupJob' --output=json | php -r 'echo json_decode(stream_get_contents(STDIN), true)[0]["id"];') +job() { occ background-job:execute "$JOB" --force-execute >/dev/null; } +occ backup:target:schedule ci off >/dev/null +occ backup:target:schedule ci3 off >/dev/null +occ backup:target:schedule ci2 00:00 --timezone=UTC >/dev/null || fail "set a schedule" +occ backup:target:schedule ci2 | grep "00:00 (UTC)" >/dev/null || fail "schedule not shown" +job +[ "$(occ backup:list ci2 | grep -c ' scheduled ')" = 0 ] || fail "choosing a time started a backup" +occ backup:target:schedule ci2 "$($X date -u -d '+1 minute' +%H:%M)/6" --timezone=UTC >/dev/null || fail "set every 6 hours" +sleep 65 +job +occ backup:list ci2 | grep " scheduled " >/dev/null || fail "the scheduled moment started no backup" +[ "$(occ backup:list ci3 | grep -c ' scheduled ')" = 0 ] || fail "a location that is off was backed up" +job +[ "$(occ backup:list ci2 | grep -c ' scheduled ')" = 1 ] || fail "one moment started more than one backup" +occ backup:target:schedule ci2 nonsense >/dev/null 2>&1 && fail "a bad schedule was accepted" +for L in 2 3; do occ backup:target:remove ci$L >/dev/null || fail "target remove ci$L"; done +ok "schedule per location: nothing on choosing a time, one backup at the moment, none when off" + # Checksum audit: shallow (index presence) and deep (download, decrypt, verify every blob). occ backup:verify ci "$SNAP" | grep "^OK$" >/dev/null || fail "shallow verify" occ backup:verify ci "$SNAP" --deep | grep "^OK$" >/dev/null || fail "deep verify"