diff --git a/vfs/file.go b/vfs/file.go index 8a6e1fe49..bc213d537 100644 --- a/vfs/file.go +++ b/vfs/file.go @@ -137,29 +137,42 @@ func (f *File) applyPendingRename() { func (f *File) rename(ctx context.Context, destDir *Dir, newName string) error { f.mu.RLock() d := f.d + o := f.o + oldPendingRenameFun := f.pendingRenameFun f.mu.RUnlock() + if features := d.f.Features(); features.Move == nil && features.Copy == nil { err := errors.Errorf("Fs %q can't rename files (no server side Move or Copy)", d.f) fs.Errorf(f.Path(), "Dir.Rename error: %v", err) return err } + newPath := path.Join(destDir.path, newName) + renameCall := func(ctx context.Context) error { - newPath := path.Join(destDir.path, newName) + f.mu.RLock() + o := f.o + f.mu.RUnlock() + if o == nil { + return errors.New("Cannot rename: file object is not available") + } + + // chain rename calls if any + if oldPendingRenameFun != nil { + err := oldPendingRenameFun(ctx) + if err != nil { + return err + } + } + + // do the move of the remote object dstOverwritten, _ := d.f.NewObject(ctx, newPath) - newObject, err := operations.Move(ctx, d.f, dstOverwritten, newPath, f.o) + newObject, err := operations.Move(ctx, d.f, dstOverwritten, newPath, o) if err != nil { fs.Errorf(f.Path(), "File.Rename error: %v", err) return err } - // Rename in the cache too if it exists - if f.d.vfs.Opt.CacheMode >= CacheModeWrites && f.d.vfs.cache.exists(f.Path()) { - if err := f.d.vfs.cache.rename(f.Path(), newPath); err != nil { - fs.Infof(f.Path(), "File.Rename failed in Cache: %v", err) - } - } - // newObject can be nil here for example if --dry-run if newObject == nil { err = errors.New("rename failed: nil object returned") @@ -167,25 +180,32 @@ func (f *File) rename(ctx context.Context, destDir *Dir, newName string) error { return err } // Update the node with the new details - fs.Debugf(f.o, "Updating file with %v %p", newObject, f) + fs.Debugf(o, "Updating file with %v %p", newObject, f) // f.rename(destDir, newObject) f.mu.Lock() f.o = newObject - f.d = destDir - f.leaf = path.Base(newObject.Remote()) f.pendingRenameFun = nil f.mu.Unlock() return nil } - f.mu.RLock() + // Rename in the cache if it exists + if f.d.vfs.Opt.CacheMode != CacheModeOff && f.d.vfs.cache.exists(f.Path()) { + if err := f.d.vfs.cache.rename(f.Path(), newPath); err != nil { + fs.Infof(f.Path(), "File.Rename failed in Cache: %v", err) + } + } + + // rename the file object + f.mu.Lock() + f.d = destDir + f.leaf = newName writing := f._writingInProgress() - f.mu.RUnlock() + f.mu.Unlock() + if writing { - fs.Debugf(f.o, "File is currently open, delaying rename %p", f) + fs.Debugf(o, "File is currently open, delaying rename %p", f) f.mu.Lock() - f.d = destDir - f.leaf = newName f.pendingRenameFun = renameCall f.mu.Unlock() return nil diff --git a/vfs/file_test.go b/vfs/file_test.go index 70848ef63..cf1c1f195 100644 --- a/vfs/file_test.go +++ b/vfs/file_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fstest" "github.com/rclone/rclone/fstest/mockfs" "github.com/rclone/rclone/fstest/mockobject" @@ -13,8 +14,10 @@ import ( "github.com/stretchr/testify/require" ) -func fileCreate(t *testing.T, r *fstest.Run) (*VFS, *File, fstest.Item) { - vfs := New(r.Fremote, nil) +func fileCreate(t *testing.T, r *fstest.Run, mode CacheMode) (*VFS, *File, fstest.Item) { + opt := DefaultOpt + opt.CacheMode = mode + vfs := New(r.Fremote, &opt) file1 := r.WriteObject(context.Background(), "dir/file1", "file1 contents", t1) fstest.CheckItems(t, r.Fremote, file1) @@ -29,7 +32,7 @@ func fileCreate(t *testing.T, r *fstest.Run) (*VFS, *File, fstest.Item) { func TestFileMethods(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() - vfs, file, _ := fileCreate(t, r) + vfs, file, _ := fileCreate(t, r, CacheModeOff) // String assert.Equal(t, "dir/file1", file.String()) @@ -84,7 +87,7 @@ func TestFileSetModTime(t *testing.T) { return } defer r.Finalise() - vfs, file, file1 := fileCreate(t, r) + vfs, file, file1 := fileCreate(t, r, CacheModeOff) err := file.SetModTime(t2) require.NoError(t, err) @@ -97,12 +100,8 @@ func TestFileSetModTime(t *testing.T) { assert.Equal(t, EROFS, err) } -func TestFileOpenRead(t *testing.T) { - r := fstest.NewRun(t) - defer r.Finalise() - _, file, _ := fileCreate(t, r) - - fd, err := file.openRead() +func fileCheckContents(t *testing.T, file *File) { + fd, err := file.Open(os.O_RDONLY) require.NoError(t, err) contents, err := ioutil.ReadAll(fd) @@ -112,6 +111,14 @@ func TestFileOpenRead(t *testing.T) { require.NoError(t, fd.Close()) } +func TestFileOpenRead(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, file, _ := fileCreate(t, r, CacheModeOff) + + fileCheckContents(t, file) +} + func TestFileOpenReadUnknownSize(t *testing.T) { var ( contents = []byte("file contents") @@ -160,7 +167,7 @@ func TestFileOpenReadUnknownSize(t *testing.T) { func TestFileOpenWrite(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() - vfs, file, _ := fileCreate(t, r) + vfs, file, _ := fileCreate(t, r, CacheModeOff) fd, err := file.openWrite(os.O_WRONLY | os.O_TRUNC) require.NoError(t, err) @@ -181,7 +188,7 @@ func TestFileOpenWrite(t *testing.T) { func TestFileRemove(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() - vfs, file, _ := fileCreate(t, r) + vfs, file, _ := fileCreate(t, r, CacheModeOff) err := file.Remove() require.NoError(t, err) @@ -196,7 +203,7 @@ func TestFileRemove(t *testing.T) { func TestFileRemoveAll(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() - vfs, file, _ := fileCreate(t, r) + vfs, file, _ := fileCreate(t, r, CacheModeOff) err := file.RemoveAll() require.NoError(t, err) @@ -211,7 +218,7 @@ func TestFileRemoveAll(t *testing.T) { func TestFileOpen(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() - _, file, _ := fileCreate(t, r) + _, file, _ := fileCreate(t, r, CacheModeOff) fd, err := file.Open(os.O_RDONLY) require.NoError(t, err) @@ -233,3 +240,90 @@ func TestFileOpen(t *testing.T) { fd, err = file.Open(3) assert.Equal(t, EPERM, err) } + +func testFileRename(t *testing.T, mode CacheMode) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, file, item := fileCreate(t, r, mode) + + rootDir, err := vfs.Root() + require.NoError(t, err) + + // check file in cache + if mode != CacheModeOff { + // read contents to get file in cache + fileCheckContents(t, file) + assert.True(t, vfs.cache.exists(item.Path)) + } + + dir := file.Dir() + + // start with "dir/file1" + fstest.CheckItems(t, r.Fremote, item) + + // rename file to "newLeaf" + err = dir.Rename("file1", "newLeaf", rootDir) + require.NoError(t, err) + + item.Path = "newLeaf" + fstest.CheckItems(t, r.Fremote, item) + + // check file in cache + if mode != CacheModeOff { + assert.True(t, vfs.cache.exists(item.Path)) + } + + // check file exists in the vfs layer at its new name + _, err = vfs.Stat("newLeaf") + require.NoError(t, err) + + // rename it back to "dir/file1" + err = rootDir.Rename("newLeaf", "file1", dir) + require.NoError(t, err) + + item.Path = "dir/file1" + fstest.CheckItems(t, r.Fremote, item) + + // check file in cache + if mode != CacheModeOff { + assert.True(t, vfs.cache.exists(item.Path)) + } + + // now try renaming it with the file open + // first open it and write to it but dont close it + fd, err := file.Open(os.O_WRONLY | os.O_TRUNC) + require.NoError(t, err) + newContents := []byte("this is some new contents") + _, err = fd.Write(newContents) + require.NoError(t, err) + + // rename file to "newLeaf" + err = dir.Rename("file1", "newLeaf", rootDir) + require.NoError(t, err) + newItem := fstest.NewItem("newLeaf", string(newContents), item.ModTime) + + // check file has been renamed immediately in the cache + if mode != CacheModeOff { + assert.True(t, vfs.cache.exists("newLeaf")) + } + + // check file exists in the vfs layer at its new name + _, err = vfs.Stat("newLeaf") + require.NoError(t, err) + + // Close the file + require.NoError(t, fd.Close()) + + // Check file has now been renamed on the remote + item.Path = "newLeaf" + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{newItem}, nil, fs.ModTimeNotSupported) +} + +func TestFileRename(t *testing.T) { + t.Run("CacheModeOff", func(t *testing.T) { + testFileRename(t, CacheModeOff) + }) + t.Run("CacheModeFull", func(t *testing.T) { + testFileRename(t, CacheModeFull) + }) +} diff --git a/vfs/read_write_test.go b/vfs/read_write_test.go index 948d8b082..403e61b33 100644 --- a/vfs/read_write_test.go +++ b/vfs/read_write_test.go @@ -698,7 +698,7 @@ func TestRWFileModTimeWithOpenWriters(t *testing.T) { } } -func TestCacheRename(t *testing.T) { +func TestRWCacheRename(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise()