diff --git a/lib/Command/BackupUser.php b/lib/Command/BackupUser.php index 6f3072f..42fdc8c 100644 --- a/lib/Command/BackupUser.php +++ b/lib/Command/BackupUser.php @@ -51,8 +51,8 @@ HELP); return 1; } try { - $this->installer->ensureEnabled($input, $output); $target = $this->targets->get($input->getArgument('target')); + $this->installer->ensureEnabled($input, $output); $manifest = $this->userBackup->backupUser($target, $user); } catch (\Throwable $e) { $output->writeln('' . $e->getMessage() . ''); diff --git a/lib/Command/KeyImport.php b/lib/Command/KeyImport.php index 6f9dd02..4e03669 100644 --- a/lib/Command/KeyImport.php +++ b/lib/Command/KeyImport.php @@ -58,7 +58,12 @@ HELP); } $file = $input->getOption('passphrase-file'); if ($file !== null) { - $pass = rtrim((string)@file_get_contents($file), "\r\n"); + $raw = @file_get_contents($file); + if ($raw === false) { + $output->writeln('Cannot read the passphrase file'); + return 1; + } + $pass = rtrim($raw, "\r\n"); } else { $helper = $this->getHelper('question'); $q = (new Question('Passphrase of one of the kit\'s slots: '))->setHidden(true)->setHiddenFallback(false); diff --git a/lib/Command/RestoreUser.php b/lib/Command/RestoreUser.php index c2f86ad..6ead952 100644 --- a/lib/Command/RestoreUser.php +++ b/lib/Command/RestoreUser.php @@ -48,9 +48,9 @@ HELP); protected function execute(InputInterface $input, OutputInterface $output): int { try { - $this->installer->ensureEnabled($input, $output); $output->writeln('Note: on restore, user_migration only imports the app settings on its own allowlist (e.g. calendar view and reminder settings); other app settings of this user are skipped. Reason: NG Backup uses user_migration\'s importer; widening the list would mean building a settings importer of our own.'); $target = $this->targets->get($input->getArgument('target')); + $this->installer->ensureEnabled($input, $output); $user = $this->userRestore->restoreUser($target, (string)$input->getArgument('manifest'), (string)$input->getOption('mode')); } catch (\Throwable $e) { $output->writeln('' . $e->getMessage() . ''); diff --git a/lib/Command/Retention.php b/lib/Command/Retention.php index bd44742..7851020 100644 --- a/lib/Command/Retention.php +++ b/lib/Command/Retention.php @@ -33,6 +33,10 @@ final class Retention extends Command { } protected function execute(InputInterface $input, OutputInterface $output): int { + if ($input->getOption('user-last') !== null && !ctype_digit((string)$input->getOption('user-last'))) { + $output->writeln('--user-last must be a whole number'); + return 1; + } $p = $this->prune->policy()->toArray(); $changed = false; foreach (['last', 'daily', 'weekly', 'monthly'] as $k) { @@ -50,13 +54,8 @@ final class Retention extends Command { } } if ($input->getOption('user-last') !== null) { - $userLast = (string)$input->getOption('user-last'); - if (!ctype_digit($userLast)) { - $output->writeln('--user-last must be a whole number'); - return 1; - } try { - $this->prune->setUserExportRetention((int)$userLast); + $this->prune->setUserExportRetention((int)$input->getOption('user-last')); } catch (\InvalidArgumentException $e) { $output->writeln('' . $e->getMessage() . ''); return 1; diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 1721308..51e0999 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -422,9 +422,9 @@ final class Repository { if (!isset($this->catalog->userExports[$manifestPath])) { throw new RepositoryException("$manifestPath is not a recorded user export"); } - $this->backend->delete($manifestPath); unset($this->catalog->userExports[$manifestPath]); $this->catalog->write(); + $this->backend->delete($manifestPath); } /** @return array */ @@ -442,23 +442,26 @@ final class Repository { /** Permanently delete trash entries older than $delaySeconds. @return list purged snapshot ids */ public function purgeTrash(int $delaySeconds): array { - $purged = []; + $doomed = []; foreach ($this->trash() as $id => $info) { if (time() - $info['forgottenAt'] < $delaySeconds) { continue; } + $doomed[$id] = $info; + unset($this->catalog->trash[$id]); + } + if ($doomed === []) { + return []; + } + $this->catalog->write(); + foreach ($doomed as $id => $info) { if ($info['dbInTrash'] !== null) { $this->backend->delete($info['dbInTrash']); } $this->backend->delete('trash/snapshots/' . $id); $this->backend->delete('trash/info/' . $id); - unset($this->catalog->trash[$id]); - $purged[] = $id; } - if ($purged !== []) { - $this->catalog->write(); - } - return $purged; + return array_keys($doomed); } private function trashInfo(string $snapshotId): array { diff --git a/lib/Service/RestoreFullService.php b/lib/Service/RestoreFullService.php index 8fc72b9..fdcf754 100644 --- a/lib/Service/RestoreFullService.php +++ b/lib/Service/RestoreFullService.php @@ -110,6 +110,7 @@ final class RestoreFullService { // directory: once applied, 'secret' may differ from what ng_backup's own Target // options were encrypted with (see restoreFull()'s docblock), and every further // repository access through $this->targets would fail to decrypt them. + /** @var array $backupConfig */ $backupConfig = $this->fetchBackupConfig($target, $snapshotId); $dataDir = rtrim($this->config->getSystemValueString('datadirectory'), '/'); diff --git a/lib/Service/UserBackupService.php b/lib/Service/UserBackupService.php index e4eb877..23aac20 100644 --- a/lib/Service/UserBackupService.php +++ b/lib/Service/UserBackupService.php @@ -39,10 +39,10 @@ final class UserBackupService { $dest = new RepositoryExportDestination($repo, $user->getUID()); $service->export($dest, $user); // A short exclusive lock around the catalog write only: the shared lock above - // allows another concurrent backup (full or per-user) to be reading the same - // catalog generation at the same time, and both writing the next one would let - // one silently overwrite the other's addition. - $this->withCatalogLock($target, fn () => $repo->recordUserExport($dest->manifestPath())); + // allows another concurrent backup (full or per-user) to be writing the catalog at + // the same time. The repository is reopened inside the lock, so the write builds on + // the latest catalog generation instead of the one loaded before the lock was taken. + $this->withCatalogLock($target, fn () => $this->targets->repository($target)->recordUserExport($dest->manifestPath())); return $dest->manifestPath(); }); } diff --git a/lib/Service/UserRestoreService.php b/lib/Service/UserRestoreService.php index 68cf24a..56e61e4 100644 --- a/lib/Service/UserRestoreService.php +++ b/lib/Service/UserRestoreService.php @@ -83,9 +83,9 @@ final class UserRestoreService { $safety = new RepositoryExportDestination($repo, $originalUid); $service->export($safety, $existing); $safetyManifestPath = $safety->manifestPath(); - // Same race as UserBackupService::backupUser(): serialize the catalog write - // itself against any other concurrent backup/restore for this target. - $this->withCatalogLock($target, fn () => $repo->recordUserExport($safetyManifestPath)); + // Same race as UserBackupService::backupUser(): reopen the repository inside the + // catalog lock so the write builds on the latest generation. + $this->withCatalogLock($target, fn () => $this->targets->repository($target)->recordUserExport($safetyManifestPath)); if (!$existing->delete()) { throw new \RuntimeException("Could not delete the existing account $originalUid before restoring; its safety export is at $safetyManifestPath"); } diff --git a/lib/UserMigration/UserMigrationInstaller.php b/lib/UserMigration/UserMigrationInstaller.php index 2c902ab..d352638 100644 --- a/lib/UserMigration/UserMigrationInstaller.php +++ b/lib/UserMigration/UserMigrationInstaller.php @@ -7,7 +7,6 @@ declare(strict_types=1); namespace OCA\NgBackup\UserMigration; -use OCA\NgBackup\AppInfo\Application; use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use Symfony\Component\Console\Input\InputInterface; @@ -43,7 +42,8 @@ final class UserMigrationInstaller { /** Installs (from the app store, if missing) and enables user_migration; throws on failure. */ public function install(): void { $args = $this->codeIsPresent() ? ['app:enable', self::APP] : ['app:install', self::APP]; - $occ = dirname($this->appManager->getAppPath(Application::APP_ID), 2) . '/occ'; + /** @psalm-suppress UndefinedClass */ + $occ = \OC::$SERVERROOT . '/occ'; $command = implode(' ', array_map('escapeshellarg', [PHP_BINARY, $occ, ...$args])) . ' 2>&1'; exec($command, $lines, $code); if ($code !== 0) { @@ -72,9 +72,6 @@ final class UserMigrationInstaller { } $output->writeln('Installing and enabling user_migration (approved by the administrator)...'); $this->install(); - $this->appManager->clearAppsCache(); - if (!$this->isEnabled()) { - throw new \RuntimeException('user_migration was installed; run this command again (check with "occ app:list" if it stays disabled)'); - } + throw new \RuntimeException('user_migration is installed and enabled; run this command again'); } }