From 396352f28151ef868a8088c9496af7caa971abf5 Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 18 Jan 2024 17:31:55 +1100 Subject: backup: Check for backup before deleting repo Reorders the repository restore logic so that we do not remove the repo until we've checked that it has an existing backup. If the repo has no backup, it's skipped from the restore and will be removed later by the caller. The previous order of operations caused issues when performing a full restore that included a dangling repo (a repo created after the backup was taken). When the `Restore` function was executed against this repo, it was removed immediately. Then, the remainder of the restore was skipped since no backup existed for that repo. Since the repo was not marked as restored, the caller of the `Restore` function tried to remove it once again, leading to a "repository not found" error since it had already been erased. The "missing backup" test case for the restore of a specific backup has been updated to expect that the repo exists to align with this logic --- internal/backup/backup.go | 8 ++++---- internal/backup/backup_test.go | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/backup/backup.go b/internal/backup/backup.go index a15f47abe..bc54cbaf0 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -335,10 +335,6 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error { return fmt.Errorf("manager: %w", err) } - if err := repo.Remove(ctx); err != nil { - return fmt.Errorf("manager: %w", err) - } - var backup *Backup if req.BackupID == "" { backup, err = mgr.locator.FindLatest(ctx, req.VanityRepository) @@ -359,6 +355,10 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error { defaultBranch, defaultBranchKnown := git.ReferenceName(backup.HeadReference).Branch() + if err := repo.Remove(ctx); err != nil { + return fmt.Errorf("manager: %w", err) + } + if err := repo.Create(ctx, hash, defaultBranch); err != nil { return fmt.Errorf("manager: %w", err) } diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 81e87a33a..96a06fba3 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -977,6 +977,7 @@ func TestManager_Restore_specific(t *testing.T) { return repo, nil }, expectedErrAs: backup.ErrSkipped, + expectExists: true, }, { desc: "single incremental", -- cgit v1.2.3