diff options
author | Eric P <eric@kastelo.net> | 2022-09-20 11:34:15 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-20 11:34:15 +0200 |
commit | 3f2742a275b6cf629456f73f5b2b6feb95f3947c (patch) | |
tree | ebcbbfd1460e698623ef8588841ff886926c895e | |
parent | 0e23002414daa1b2bdb5bd56ff312156f2ac75a7 (diff) |
lib/versioner: Fix error in Trashcan restore (fixes: #7965) (#8549)
The restore function of Trash Can ran a rename at the end regardless of whether there was anything to rename. In this case, when the file-to-be-restored did not exist in the destination folder, this resulted in an error. I added a simple check, keeping track of whether the file existed prior to restoring it in the destination folder and depending on this value it will now return nil after the restoration to prevent the renaming function to kick off. Added a test for this specific edge-case as well.
-rw-r--r-- | lib/versioner/trashcan.go | 17 | ||||
-rw-r--r-- | lib/versioner/trashcan_test.go | 72 |
2 files changed, 87 insertions, 2 deletions
diff --git a/lib/versioner/trashcan.go b/lib/versioner/trashcan.go index e300de92a6..2b59eb9da0 100644 --- a/lib/versioner/trashcan.go +++ b/lib/versioner/trashcan.go @@ -113,6 +113,10 @@ func (t *trashcan) Restore(filepath string, versionTime time.Time) error { // tag but when the restoration is finished, we rename it (untag it). This is only important if when restoring A, // there already exists a file at the same location + // If we restore a deleted file, there won't be a conflict and archiving won't happen thus there won't be anything + // in the archive to rename afterwards. Log whether the file exists prior to restoring. + _, dstPathErr := t.folderFs.Lstat(filepath) + taggedName := "" tagger := func(name, tag string) string { // We also abuse the fact that tagger gets called twice, once for tagging the restoration version, which @@ -126,10 +130,19 @@ func (t *trashcan) Restore(filepath string, versionTime time.Time) error { return name } - err := restoreFile(t.copyRangeMethod, t.versionsFs, t.folderFs, filepath, versionTime, tagger) - if taggedName == "" { + if err := restoreFile(t.copyRangeMethod, t.versionsFs, t.folderFs, filepath, versionTime, tagger); taggedName == "" { return err } + // If a deleted file was restored, even though the RenameOrCopy method is robust, check if the file exists and + // skip the renaming function if this is the case. + if fs.IsNotExist(dstPathErr) { + if _, err := t.folderFs.Lstat(filepath); err != nil { + return err + } + + return nil + } + return t.versionsFs.Rename(taggedName, filepath) } diff --git a/lib/versioner/trashcan_test.go b/lib/versioner/trashcan_test.go index 348d2ac417..2405f3176a 100644 --- a/lib/versioner/trashcan_test.go +++ b/lib/versioner/trashcan_test.go @@ -96,6 +96,78 @@ func TestTrashcanArchiveRestoreSwitcharoo(t *testing.T) { } } +func TestTrashcanRestoreDeletedFile(t *testing.T) { + // This tests that the Trash Can restore function works correctly when the file + // to be restored was deleted/nonexistent in the folder where the file/folder is + // going to be restored in. (Issue: #7965) + + tmpDir1 := t.TempDir() + + tmpDir2 := t.TempDir() + + cfg := config.FolderConfiguration{ + FilesystemType: fs.FilesystemTypeBasic, + Path: tmpDir1, + Versioning: config.VersioningConfiguration{ + FSType: fs.FilesystemTypeBasic, + FSPath: tmpDir2, + }, + } + + folderFs := cfg.Filesystem(nil) + + versionsFs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmpDir2) + + versioner := newTrashcan(cfg) + + writeFile(t, folderFs, "file", "Some content") + + if err := versioner.Archive("file"); err != nil { + t.Fatal(err) + } + + // Shouldn't be in the default folder anymore, thus "deleted" + if _, err := folderFs.Stat("file"); !fs.IsNotExist(err) { + t.Fatal(err) + } + + // It should, however, be in the archive + if _, err := versionsFs.Lstat("file"); fs.IsNotExist(err) { + t.Fatal(err) + } + + versions, err := versioner.GetVersions() + if err != nil { + t.Fatal(err) + } + + fileVersions := versions["file"] + if len(fileVersions) != 1 { + t.Fatalf("unexpected number of versions: %d != 1", len(fileVersions)) + } + + fileVersion := fileVersions[0] + + if !fileVersion.ModTime.Equal(fileVersion.VersionTime) { + t.Error("time mismatch") + } + + // Restore the file from the archive. + if err := versioner.Restore("file", fileVersion.VersionTime); err != nil { + t.Fatal(err) + } + + // The file should be correctly restored + if content := readFile(t, folderFs, "file"); content != "Some content" { + t.Errorf("expected A got %s", content) + } + + // It should no longer be in the archive + if _, err := versionsFs.Lstat("file"); !fs.IsNotExist(err) { + t.Fatal(err) + } +} + func readFile(t *testing.T, filesystem fs.Filesystem, name string) string { t.Helper() fd, err := filesystem.Open(name) |