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.
This commit is contained in:
Eric P 2022-09-20 11:34:15 +02:00 committed by GitHub
parent 0e23002414
commit 3f2742a275
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 87 additions and 2 deletions

View File

@ -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)
}

View File

@ -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)