From e18122e88b3e432ddc4fe83f11ce4451b325063d Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sun, 29 Oct 2017 21:14:05 +0000 Subject: [PATCH] vfs: add tests and subsequent fixes * Tests for VFS layer * Small fixes found during testing * Fix Close, Flush and Release behaviour for ReadFileHandle and WriteFileHandle * Fix nil object bugs on File --- cmd/cmount/fs.go | 12 +- cmd/mount/file.go | 2 +- cmd/mount/write.go | 18 +- cmd/mountlib/mounttest/fs.go | 6 +- cmd/mountlib/mounttest/write_unix.go | 4 +- vfs/createinfo_test.go | 29 ++ vfs/dir.go | 10 +- vfs/dir_handle_test.go | 112 +++++++ vfs/dir_test.go | 455 +++++++++++++++++++++++++++ vfs/errors_test.go | 13 + vfs/file.go | 19 +- vfs/file_test.go | 185 +++++++++++ vfs/read.go | 45 ++- vfs/read_test.go | 238 ++++++++++++++ vfs/vfs.go | 5 +- vfs/vfs_test.go | 252 +++++++++++++++ vfs/write.go | 30 +- vfs/write_test.go | 160 ++++++++++ 18 files changed, 1536 insertions(+), 59 deletions(-) create mode 100644 vfs/createinfo_test.go create mode 100644 vfs/dir_handle_test.go create mode 100644 vfs/dir_test.go create mode 100644 vfs/errors_test.go create mode 100644 vfs/file_test.go create mode 100644 vfs/read_test.go create mode 100644 vfs/vfs_test.go create mode 100644 vfs/write_test.go diff --git a/cmd/cmount/fs.go b/cmd/cmount/fs.go index bcf1c6e11..63b6227e1 100644 --- a/cmd/cmount/fs.go +++ b/cmd/cmount/fs.go @@ -5,6 +5,7 @@ package cmount import ( + "io" "os" "path" "runtime" @@ -397,13 +398,10 @@ func (fsys *FS) Read(path string, buff []byte, ofst int64, fh uint64) (n int) { if errc != 0 { return errc } - rfh, ok := handle.(*vfs.ReadFileHandle) - if !ok { - // Can only read from read file handle - return -fuse.EIO - } - n, err := rfh.ReadAt(buff, ofst) - if err != nil { + n, err := handle.ReadAt(buff, ofst) + if err == io.EOF { + err = nil + } else if err != nil { return translateError(err) } return n diff --git a/cmd/mount/file.go b/cmd/mount/file.go index 35a4f76ca..ecb7e1df3 100644 --- a/cmd/mount/file.go +++ b/cmd/mount/file.go @@ -64,7 +64,7 @@ func (f *File) Open(ctx context.Context, req *fuse.OpenRequest, resp *fuse.OpenR // fuse flags are based off syscall flags as are os flags, so // should be compatible - handle, err := f.File.Open(int(resp.Flags)) + handle, err := f.File.Open(int(req.Flags)) if err != nil { return nil, translateError(err) } diff --git a/cmd/mount/write.go b/cmd/mount/write.go index f1a648bfa..42f17c8b8 100644 --- a/cmd/mount/write.go +++ b/cmd/mount/write.go @@ -4,6 +4,7 @@ package mount import ( "errors" + "io" "bazil.org/fuse" fusefs "bazil.org/fuse/fs" @@ -20,7 +21,22 @@ type WriteFileHandle struct { } // Check interface satisfied -var _ fusefs.Handle = (*WriteFileHandle)(nil) +var _ fusefs.HandleReader = (*FileHandle)(nil) + +// Read from the file handle +func (fh *FileHandle) Read(ctx context.Context, req *fuse.ReadRequest, resp *fuse.ReadResponse) (err error) { + var n int + defer fs.Trace(fh, "len=%d, offset=%d", req.Size, req.Offset)("read=%d, err=%v", &n, &err) + data := make([]byte, req.Size) + n, err = fh.Handle.ReadAt(data, req.Offset) + if err == io.EOF { + err = nil + } else if err != nil { + return translateError(err) + } + resp.Data = data[:n] + return nil +} // Check interface satisfied var _ fusefs.HandleWriter = (*WriteFileHandle)(nil) diff --git a/cmd/mountlib/mounttest/fs.go b/cmd/mountlib/mounttest/fs.go index 6f700f33c..60b43707d 100644 --- a/cmd/mountlib/mounttest/fs.go +++ b/cmd/mountlib/mounttest/fs.go @@ -207,10 +207,10 @@ func (r *Run) readLocal(t *testing.T, dir dirMap, filepath string) { if fi.IsDir() { dir[name+"/"] = struct{}{} r.readLocal(t, dir, name) - assert.Equal(t, run.vfs.Opt.DirPerms, fi.Mode().Perm()) + assert.Equal(t, run.vfs.Opt.DirPerms&os.ModePerm, fi.Mode().Perm()) } else { dir[fmt.Sprintf("%s %d", name, fi.Size())] = struct{}{} - assert.Equal(t, run.vfs.Opt.FilePerms, fi.Mode().Perm()) + assert.Equal(t, run.vfs.Opt.FilePerms&os.ModePerm, fi.Mode().Perm()) } } } @@ -292,5 +292,5 @@ func TestRoot(t *testing.T) { fi, err := os.Lstat(run.mountPath) require.NoError(t, err) assert.True(t, fi.IsDir()) - assert.Equal(t, fi.Mode().Perm(), run.vfs.Opt.DirPerms) + assert.Equal(t, run.vfs.Opt.DirPerms&os.ModePerm, fi.Mode().Perm()) } diff --git a/cmd/mountlib/mounttest/write_unix.go b/cmd/mountlib/mounttest/write_unix.go index abb42b74a..d2b6a5337 100644 --- a/cmd/mountlib/mounttest/write_unix.go +++ b/cmd/mountlib/mounttest/write_unix.go @@ -42,9 +42,9 @@ func TestWriteFileDoubleClose(t *testing.T) { _, err = syscall.Write(fd2, buf) assert.Error(t, err, "input/output error") - // close the dup - should produce an error + // close the dup - should not produce an error err = syscall.Close(fd2) - assert.Error(t, err, "input/output error") + assert.NoError(t, err) run.rm(t, "testdoubleclose") } diff --git a/vfs/createinfo_test.go b/vfs/createinfo_test.go new file mode 100644 index 000000000..240015164 --- /dev/null +++ b/vfs/createinfo_test.go @@ -0,0 +1,29 @@ +package vfs + +import ( + "testing" + "time" + + "github.com/ncw/rclone/fs" + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" +) + +func TestCreateInfo(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + + remote := "file/to/be/created" + ci := newCreateInfo(r.Fremote, remote) + + // Test methods + assert.Equal(t, r.Fremote, ci.Fs()) + assert.Equal(t, remote, ci.String()) + assert.Equal(t, remote, ci.Remote()) + _, err := ci.Hash(fs.HashMD5) + assert.Equal(t, fs.ErrHashUnsupported, err) + assert.WithinDuration(t, time.Now(), ci.ModTime(), time.Second) + assert.Equal(t, int64(0), ci.Size()) + assert.Equal(t, true, ci.Storable()) + +} diff --git a/vfs/dir.go b/vfs/dir.go index 6072821f4..8fffb0b42 100644 --- a/vfs/dir.go +++ b/vfs/dir.go @@ -58,7 +58,7 @@ func (d *Dir) IsDir() bool { // Mode bits of the directory - satisfies Node interface func (d *Dir) Mode() (mode os.FileMode) { - return os.ModeDir | 0777 + return d.vfs.Opt.DirPerms } // Name (base) of the directory - satisfies Node interface @@ -109,10 +109,10 @@ func (d *Dir) ForgetPath(relativePath string) { }) } -// walk runs a function on all directories whose path matches -// the given absolute one. It will be called on a directory's -// children first. It will not apply the function to parent -// nodes, regardless of the given path. +// walk runs a function on all cached directories whose path matches +// the given absolute one. It will be called on a directory's children +// first. It will not apply the function to parent nodes, regardless +// of the given path. func (d *Dir) walk(absPath string, fun func(*Dir)) { d.mu.Lock() defer d.mu.Unlock() diff --git a/vfs/dir_handle_test.go b/vfs/dir_handle_test.go new file mode 100644 index 000000000..4bbe635e8 --- /dev/null +++ b/vfs/dir_handle_test.go @@ -0,0 +1,112 @@ +package vfs + +import ( + "io" + "os" + "testing" + + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDirHandleMethods(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, dir, _ := dirCreate(t, r) + + h, err := dir.Open(os.O_RDONLY) + require.NoError(t, err) + fh, ok := h.(*DirHandle) + assert.True(t, ok) + + // String + assert.Equal(t, "dir/ (r)", fh.String()) + assert.Equal(t, "", (*DirHandle)(nil).String()) + assert.Equal(t, "", newDirHandle(nil).String()) + + // Stat + fi, err := fh.Stat() + require.NoError(t, err) + assert.Equal(t, dir, fi) + + // Node + assert.Equal(t, dir, fh.Node()) + + // Close + require.NoError(t, h.Close()) + assert.Equal(t, []os.FileInfo(nil), fh.fis) +} + +func TestDirHandleReaddir(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("dir/file1", "file1 contents", t1) + file2 := r.WriteObject("dir/file2", "file2- contents", t2) + file3 := r.WriteObject("dir/subdir/file3", "file3-- contents", t3) + fstest.CheckItems(t, r.Fremote, file1, file2, file3) + + node, err := vfs.Stat("dir") + require.NoError(t, err) + dir := node.(*Dir) + + // Read in one chunk + fh, err := dir.Open(os.O_RDONLY) + require.NoError(t, err) + + fis, err := fh.Readdir(-1) + require.NoError(t, err) + require.Equal(t, 3, len(fis)) + assert.Equal(t, "file1", fis[0].Name()) + assert.Equal(t, "file2", fis[1].Name()) + assert.Equal(t, "subdir", fis[2].Name()) + assert.False(t, fis[0].IsDir()) + assert.False(t, fis[1].IsDir()) + assert.True(t, fis[2].IsDir()) + + require.NoError(t, fh.Close()) + + // Read in multiple chunks + fh, err = dir.Open(os.O_RDONLY) + require.NoError(t, err) + + fis, err = fh.Readdir(2) + require.NoError(t, err) + require.Equal(t, 2, len(fis)) + assert.Equal(t, "file1", fis[0].Name()) + assert.Equal(t, "file2", fis[1].Name()) + assert.False(t, fis[0].IsDir()) + assert.False(t, fis[1].IsDir()) + + fis, err = fh.Readdir(2) + require.NoError(t, err) + require.Equal(t, 1, len(fis)) + assert.Equal(t, "subdir", fis[0].Name()) + assert.True(t, fis[0].IsDir()) + + fis, err = fh.Readdir(2) + assert.Equal(t, io.EOF, err) + require.Equal(t, 0, len(fis)) + + require.NoError(t, fh.Close()) + +} + +func TestDirHandleReaddirnames(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, dir, _ := dirCreate(t, r) + + fh, err := dir.Open(os.O_RDONLY) + require.NoError(t, err) + + // Smoke test only since heavy lifting done in Readdir + fis, err := fh.Readdirnames(-1) + require.NoError(t, err) + require.Equal(t, 1, len(fis)) + assert.Equal(t, "file1", fis[0]) + + require.NoError(t, fh.Close()) +} diff --git a/vfs/dir_test.go b/vfs/dir_test.go new file mode 100644 index 000000000..5f3cf0016 --- /dev/null +++ b/vfs/dir_test.go @@ -0,0 +1,455 @@ +package vfs + +import ( + "fmt" + "os" + "sort" + "testing" + "time" + + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func dirCreate(t *testing.T, r *fstest.Run) (*VFS, *Dir, fstest.Item) { + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("dir/file1", "file1 contents", t1) + fstest.CheckItems(t, r.Fremote, file1) + + node, err := vfs.Stat("dir") + require.NoError(t, err) + require.True(t, node.IsDir()) + + return vfs, node.(*Dir), file1 +} + +func TestDirMethods(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, _ := dirCreate(t, r) + + // String + assert.Equal(t, "dir/", dir.String()) + assert.Equal(t, "", (*Dir)(nil).String()) + + // IsDir + assert.Equal(t, true, dir.IsDir()) + + // IsFile + assert.Equal(t, false, dir.IsFile()) + + // Mode + assert.Equal(t, vfs.Opt.DirPerms, dir.Mode()) + + // Name + assert.Equal(t, "dir", dir.Name()) + + // Sys + assert.Equal(t, nil, dir.Sys()) + + // Inode + assert.NotEqual(t, uint64(0), dir.Inode()) + + // Node + assert.Equal(t, dir, dir.Node()) + + // ModTime + assert.WithinDuration(t, t1, dir.ModTime(), 100*365*24*60*60*time.Second) + + // Size + assert.Equal(t, int64(0), dir.Size()) + + // Fsync + assert.NoError(t, dir.Fsync()) + + // DirEntry + assert.Equal(t, dir.entry, dir.DirEntry()) + + // VFS + assert.Equal(t, vfs, dir.VFS()) +} + +func TestDirForgetAll(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, file1 := dirCreate(t, r) + + // Make sure / and dir are in cache + _, err := vfs.Stat(file1.Path) + require.NoError(t, err) + + root, err := vfs.Root() + require.NoError(t, err) + + assert.Equal(t, 1, len(root.items)) + assert.Equal(t, 1, len(dir.items)) + + dir.ForgetAll() + assert.Equal(t, 1, len(root.items)) + assert.Equal(t, 0, len(dir.items)) + + root.ForgetAll() + assert.Equal(t, 0, len(root.items)) + assert.Equal(t, 0, len(dir.items)) +} + +func TestDirForgetPath(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, file1 := dirCreate(t, r) + + // Make sure / and dir are in cache + _, err := vfs.Stat(file1.Path) + require.NoError(t, err) + + root, err := vfs.Root() + require.NoError(t, err) + + assert.Equal(t, 1, len(root.items)) + assert.Equal(t, 1, len(dir.items)) + + root.ForgetPath("dir") + assert.Equal(t, 1, len(root.items)) + assert.Equal(t, 0, len(dir.items)) + + root.ForgetPath("not/in/cache") + assert.Equal(t, 1, len(root.items)) + assert.Equal(t, 0, len(dir.items)) +} + +func TestDirWalk(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, _, file1 := dirCreate(t, r) + + file2 := r.WriteObject("fil/a/b/c", "super long file", t1) + fstest.CheckItems(t, r.Fremote, file1, file2) + + root, err := vfs.Root() + require.NoError(t, err) + + // Forget the cache since we put another object in + root.ForgetAll() + + // Read the directories in + _, err = vfs.Stat("dir") + require.NoError(t, err) + _, err = vfs.Stat("fil/a/b") + require.NoError(t, err) + fil, err := vfs.Stat("fil") + require.NoError(t, err) + + var result []string + fn := func(d *Dir) { + result = append(result, d.path) + } + + result = nil + root.walk("", fn) + sort.Strings(result) // sort as there is a map traversal involved + assert.Equal(t, []string{"", "dir", "fil", "fil/a", "fil/a/b"}, result) + + result = nil + root.walk("dir", fn) + assert.Equal(t, []string{"dir"}, result) + + result = nil + root.walk("not found", fn) + assert.Equal(t, []string(nil), result) + + result = nil + root.walk("fil", fn) + assert.Equal(t, []string{"fil/a/b", "fil/a", "fil"}, result) + + result = nil + fil.(*Dir).walk("fil", fn) + assert.Equal(t, []string{"fil/a/b", "fil/a", "fil"}, result) + + result = nil + root.walk("fil/a", fn) + assert.Equal(t, []string{"fil/a/b", "fil/a"}, result) + + result = nil + fil.(*Dir).walk("fil/a", fn) + assert.Equal(t, []string{"fil/a/b", "fil/a"}, result) + + result = nil + root.walk("fil/a", fn) + assert.Equal(t, []string{"fil/a/b", "fil/a"}, result) + + result = nil + root.walk("fil/a/b", fn) + assert.Equal(t, []string{"fil/a/b"}, result) +} + +func TestDirSetModTime(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, _ := dirCreate(t, r) + + err := dir.SetModTime(t1) + require.NoError(t, err) + assert.WithinDuration(t, t1, dir.ModTime(), time.Second) + + err = dir.SetModTime(t2) + require.NoError(t, err) + assert.WithinDuration(t, t2, dir.ModTime(), time.Second) + + vfs.Opt.ReadOnly = true + err = dir.SetModTime(t2) + assert.Equal(t, EROFS, err) +} + +func TestDirStat(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, dir, _ := dirCreate(t, r) + + node, err := dir.Stat("file1") + require.NoError(t, err) + _, ok := node.(*File) + assert.True(t, ok) + assert.Equal(t, int64(14), node.Size()) + assert.Equal(t, "file1", node.Name()) + + node, err = dir.Stat("not found") + assert.Equal(t, ENOENT, err) +} + +// This lists dir and checks the listing is as expected +func checkListing(t *testing.T, dir *Dir, want []string) { + var got []string + nodes, err := dir.ReadDirAll() + require.NoError(t, err) + for _, node := range nodes { + got = append(got, fmt.Sprintf("%s,%d,%v", node.Name(), node.Size(), node.IsDir())) + } + assert.Equal(t, want, got) +} + +func TestDirReadDirAll(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("dir/file1", "file1 contents", t1) + file2 := r.WriteObject("dir/file2", "file2- contents", t2) + file3 := r.WriteObject("dir/subdir/file3", "file3-- contents", t3) + fstest.CheckItems(t, r.Fremote, file1, file2, file3) + + node, err := vfs.Stat("dir") + require.NoError(t, err) + dir := node.(*Dir) + + checkListing(t, dir, []string{"file1,14,false", "file2,15,false", "subdir,0,true"}) + + node, err = vfs.Stat("") + require.NoError(t, err) + dir = node.(*Dir) + + checkListing(t, dir, []string{"dir,0,true"}) + + node, err = vfs.Stat("dir/subdir") + require.NoError(t, err) + dir = node.(*Dir) + + checkListing(t, dir, []string{"file3,16,false"}) +} + +func TestDirOpen(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, dir, _ := dirCreate(t, r) + + fd, err := dir.Open(os.O_RDONLY) + require.NoError(t, err) + _, ok := fd.(*DirHandle) + assert.True(t, ok) + require.NoError(t, fd.Close()) + + fd, err = dir.Open(os.O_WRONLY) + assert.Equal(t, EPERM, err) +} + +func TestDirCreate(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, _ := dirCreate(t, r) + + file, fd, err := dir.Create("potato") + require.NoError(t, err) + assert.Equal(t, int64(0), file.Size()) + + // FIXME Note that this fails with the current implementation + // until the file has been opened. + + // file2, err := vfs.Stat("dir/potato") + // require.NoError(t, err) + // assert.Equal(t, file, file2) + + n, err := fd.Write([]byte("hello")) + require.NoError(t, err) + assert.Equal(t, 5, n) + + require.NoError(t, fd.Close()) + + file2, err := vfs.Stat("dir/potato") + require.NoError(t, err) + assert.Equal(t, int64(5), file2.Size()) + + vfs.Opt.ReadOnly = true + _, _, err = dir.Create("sausage") + assert.Equal(t, EROFS, err) +} + +func TestDirMkdir(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, file1 := dirCreate(t, r) + + _, err := dir.Mkdir("file1") + assert.Error(t, err) + + sub, err := dir.Mkdir("sub") + assert.NoError(t, err) + + // check the vfs + checkListing(t, dir, []string{"file1,14,false", "sub,0,true"}) + checkListing(t, sub, []string(nil)) + + // check the underlying r.Fremote + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{"dir", "dir/sub"}, r.Fremote.Precision()) + + vfs.Opt.ReadOnly = true + _, err = dir.Mkdir("sausage") + assert.Equal(t, EROFS, err) +} + +func TestDirRemove(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, _ := dirCreate(t, r) + + // check directory is there + node, err := vfs.Stat("dir") + require.NoError(t, err) + assert.True(t, node.IsDir()) + + err = dir.Remove() + assert.Equal(t, ENOTEMPTY, err) + + // Delete the sub file + node, err = vfs.Stat("dir/file1") + require.NoError(t, err) + err = node.Remove() + require.NoError(t, err) + + // Remove the now empty directory + err = dir.Remove() + require.NoError(t, err) + + // check directory is not there + node, err = vfs.Stat("dir") + assert.Equal(t, ENOENT, err) + + // check the vfs + root, err := vfs.Root() + require.NoError(t, err) + checkListing(t, root, []string(nil)) + + // check the underlying r.Fremote + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{}, []string{}, r.Fremote.Precision()) + + // read only check + vfs.Opt.ReadOnly = true + err = dir.Remove() + assert.Equal(t, EROFS, err) +} + +func TestDirRemoveAll(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, _ := dirCreate(t, r) + + // Remove the directory and contents + err := dir.RemoveAll() + require.NoError(t, err) + + // check the vfs + root, err := vfs.Root() + require.NoError(t, err) + checkListing(t, root, []string(nil)) + + // check the underlying r.Fremote + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{}, []string{}, r.Fremote.Precision()) + + // read only check + vfs.Opt.ReadOnly = true + err = dir.RemoveAll() + assert.Equal(t, EROFS, err) +} + +func TestDirRemoveName(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, _ := dirCreate(t, r) + + err := dir.RemoveName("file1") + require.NoError(t, err) + checkListing(t, dir, []string(nil)) + root, err := vfs.Root() + require.NoError(t, err) + checkListing(t, root, []string{"dir,0,true"}) + + // check the underlying r.Fremote + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{}, []string{"dir"}, r.Fremote.Precision()) + + // read only check + vfs.Opt.ReadOnly = true + err = dir.RemoveName("potato") + assert.Equal(t, EROFS, err) +} + +func TestDirRename(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, dir, file1 := dirCreate(t, r) + + root, err := vfs.Root() + require.NoError(t, err) + + err = dir.Rename("not found", "tuba", dir) + assert.Equal(t, ENOENT, err) + + // Rename a directory + err = root.Rename("dir", "dir2", root) + assert.NoError(t, err) + checkListing(t, root, []string{"dir2,0,true"}) + checkListing(t, dir, []string{"file1,14,false"}) + + // check the underlying r.Fremote + file1.Path = "dir2/file1" + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{"dir2"}, r.Fremote.Precision()) + + // refetch dir + node, err := vfs.Stat("dir2") + assert.NoError(t, err) + dir = node.(*Dir) + + // Rename a file + err = dir.Rename("file1", "file2", root) + assert.NoError(t, err) + checkListing(t, root, []string{"dir2,0,true", "file2,14,false"}) + checkListing(t, dir, []string(nil)) + + // check the underlying r.Fremote + file1.Path = "file2" + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{"dir2"}, r.Fremote.Precision()) + + // read only check + vfs.Opt.ReadOnly = true + err = dir.Rename("potato", "tuba", dir) + assert.Equal(t, EROFS, err) +} diff --git a/vfs/errors_test.go b/vfs/errors_test.go new file mode 100644 index 000000000..7865fb6f3 --- /dev/null +++ b/vfs/errors_test.go @@ -0,0 +1,13 @@ +package vfs + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestErrorError(t *testing.T) { + assert.Equal(t, "Success", OK.Error()) + assert.Equal(t, "Function not implemented", ENOSYS.Error()) + assert.Equal(t, "Low level error 99", Error(99).Error()) +} diff --git a/vfs/file.go b/vfs/file.go index 2e26b3b6e..f32dcd0cf 100644 --- a/vfs/file.go +++ b/vfs/file.go @@ -53,12 +53,12 @@ func (f *File) IsDir() bool { // Mode bits of the file or directory - satisfies Node interface func (f *File) Mode() (mode os.FileMode) { - return 0666 + return f.d.vfs.Opt.FilePerms } // Name (base) of the directory - satisfies Node interface func (f *File) Name() (name string) { - return path.Base(f.o.Remote()) + return f.leaf } // Sys returns underlying data source (can be nil) - satisfies Node interface @@ -76,11 +76,12 @@ func (f *File) Node() Node { return f } -// rename should be called to update f.o and f.d after a rename +// rename should be called to update the internals after a rename func (f *File) rename(d *Dir, o fs.Object) { f.mu.Lock() f.o = o f.d = d + f.leaf = path.Base(o.Remote()) f.mu.Unlock() } @@ -257,10 +258,12 @@ func (f *File) Remove() error { if f.d.vfs.Opt.ReadOnly { return EROFS } - err := f.o.Remove() - if err != nil { - fs.Errorf(f, "File.Remove file error: %v", err) - return err + if f.o != nil { + err := f.o.Remove() + if err != nil { + fs.Errorf(f, "File.Remove file error: %v", err) + return err + } } // Remove the item from the directory listing f.d.delObject(f.Name()) @@ -272,7 +275,7 @@ func (f *File) RemoveAll() error { return f.Remove() } -// DirEntry returns the underlying fs.DirEntry +// DirEntry returns the underlying fs.DirEntry - may be nil func (f *File) DirEntry() (entry fs.DirEntry) { return f.o } diff --git a/vfs/file_test.go b/vfs/file_test.go new file mode 100644 index 000000000..87ced3655 --- /dev/null +++ b/vfs/file_test.go @@ -0,0 +1,185 @@ +package vfs + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func fileCreate(t *testing.T, r *fstest.Run) (*VFS, *File, fstest.Item) { + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("dir/file1", "file1 contents", t1) + fstest.CheckItems(t, r.Fremote, file1) + + node, err := vfs.Stat("dir/file1") + require.NoError(t, err) + require.True(t, node.IsFile()) + + return vfs, node.(*File), file1 +} + +func TestFileMethods(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, file, _ := fileCreate(t, r) + + // String + assert.Equal(t, "dir/file1", file.String()) + assert.Equal(t, "", (*File)(nil).String()) + + // IsDir + assert.Equal(t, false, file.IsDir()) + + // IsFile + assert.Equal(t, true, file.IsFile()) + + // Mode + assert.Equal(t, vfs.Opt.FilePerms, file.Mode()) + + // Name + assert.Equal(t, "file1", file.Name()) + + // Sys + assert.Equal(t, nil, file.Sys()) + + // Inode + assert.NotEqual(t, uint64(0), file.Inode()) + + // Node + assert.Equal(t, file, file.Node()) + + // ModTime + assert.WithinDuration(t, t1, file.ModTime(), r.Fremote.Precision()) + + // Size + assert.Equal(t, int64(14), file.Size()) + + // Fsync + assert.NoError(t, file.Fsync()) + + // DirEntry + assert.Equal(t, file.o, file.DirEntry()) + + // Dir + assert.Equal(t, file.d, file.Dir()) + + // VFS + assert.Equal(t, vfs, file.VFS()) +} + +func TestFileSetModTime(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, file, file1 := fileCreate(t, r) + + err := file.SetModTime(t2) + require.NoError(t, err) + + file1.ModTime = t2 + fstest.CheckItems(t, r.Fremote, file1) + + vfs.Opt.ReadOnly = true + err = file.SetModTime(t2) + 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() + require.NoError(t, err) + + contents, err := ioutil.ReadAll(fd) + require.NoError(t, err) + assert.Equal(t, "file1 contents", string(contents)) + + require.NoError(t, fd.Close()) +} + +func TestFileOpenWrite(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, file, _ := fileCreate(t, r) + + fd, err := file.OpenWrite() + require.NoError(t, err) + + newContents := []byte("this is some new contents") + n, err := fd.Write(newContents) + require.NoError(t, err) + assert.Equal(t, len(newContents), n) + require.NoError(t, fd.Close()) + + assert.Equal(t, int64(25), file.Size()) + + vfs.Opt.ReadOnly = true + _, err = file.OpenWrite() + assert.Equal(t, EROFS, err) +} + +func TestFileRemove(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, file, _ := fileCreate(t, r) + + err := file.Remove() + require.NoError(t, err) + + fstest.CheckItems(t, r.Fremote) + + vfs.Opt.ReadOnly = true + err = file.Remove() + assert.Equal(t, EROFS, err) +} + +func TestFileRemoveAll(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, file, _ := fileCreate(t, r) + + err := file.RemoveAll() + require.NoError(t, err) + + fstest.CheckItems(t, r.Fremote) + + vfs.Opt.ReadOnly = true + err = file.RemoveAll() + assert.Equal(t, EROFS, err) +} + +func TestFileOpen(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, file, _ := fileCreate(t, r) + + fd, err := file.Open(os.O_RDONLY) + assert.NoError(t, err) + _, ok := fd.(*ReadFileHandle) + assert.True(t, ok) + require.NoError(t, fd.Close()) + + fd, err = file.Open(os.O_WRONLY) + assert.NoError(t, err) + _, ok = fd.(*WriteFileHandle) + assert.True(t, ok) + require.NoError(t, fd.Close()) + + fd, err = file.Open(os.O_RDWR | os.O_TRUNC) + assert.NoError(t, err) + _, ok = fd.(*WriteFileHandle) + assert.True(t, ok) + require.NoError(t, fd.Close()) + + fd, err = file.Open(os.O_RDWR) + assert.Equal(t, EPERM, err) + + fd, err = file.Open(3) + assert.Equal(t, EPERM, err) +} diff --git a/vfs/read.go b/vfs/read.go index c8f99377b..6e30f6aef 100644 --- a/vfs/read.go +++ b/vfs/read.go @@ -49,7 +49,6 @@ func newReadFileHandle(f *File, o fs.Object) (*ReadFileHandle, error) { file: f, hash: hash, } - fs.Stats.Transferring(fh.o.Remote()) return fh, nil } @@ -65,6 +64,7 @@ func (fh *ReadFileHandle) openPending() (err error) { } fh.r = fs.NewAccount(r, fh.o).WithBuffer() // account the transfer fh.opened = true + fs.Stats.Transferring(fh.o.Remote()) return nil } @@ -165,7 +165,7 @@ func (fh *ReadFileHandle) Seek(offset int64, whence int) (n int64, err error) { func (fh *ReadFileHandle) ReadAt(p []byte, off int64) (n int, err error) { fh.mu.Lock() defer fh.mu.Unlock() - err = fh.openPending() // FIXME pending open could be more efficient in the presense of seek (and retried) + err = fh.openPending() // FIXME pending open could be more efficient in the presense of seek (and retries) if err != nil { return 0, err } @@ -175,6 +175,9 @@ func (fh *ReadFileHandle) ReadAt(p []byte, off int64) (n int, err error) { return 0, EBADF } doSeek := off != fh.offset + if doSeek && fh.noSeek { + return 0, ESPIPE + } var newOffset int64 retries := 0 reqSize := len(p) @@ -186,7 +189,7 @@ func (fh *ReadFileHandle) ReadAt(p []byte, off int64) (n int, err error) { // file in an unchanged state. if off >= fh.o.Size() { fs.Debugf(fh.o, "ReadFileHandle.Read attempt to read beyond end of file: %d > %d", off, fh.o.Size()) - return 0, nil + return 0, io.EOF } // Otherwise do the seek err = fh.seek(off, doReopen) @@ -234,6 +237,11 @@ func (fh *ReadFileHandle) ReadAt(p []byte, off int64) (n int, err error) { return 0, err } } + + // If we have no error and we didn't fill the buffer, must be EOF + if n != len(p) { + err = io.EOF + } } return n, err } @@ -297,13 +305,26 @@ func (fh *ReadFileHandle) close() error { return EBADF } fh.closed = true - fs.Stats.DoneTransferring(fh.o.Remote(), true) - if err := fh.checkHash(); err != nil { - return err + if fh.opened { + fs.Stats.DoneTransferring(fh.o.Remote(), true) + err1 := fh.checkHash() + err2 := fh.r.Close() + if err1 != nil { + return err1 + } + if err2 != nil { + return err2 + } } + return nil +} - return fh.r.Close() +// Close closes the file +func (fh *ReadFileHandle) Close() error { + fh.mu.Lock() + defer fh.mu.Unlock() + return fh.close() } // Flush is called each time the file or directory is closed. @@ -359,13 +380,3 @@ func (fh *ReadFileHandle) Size() int64 { func (fh *ReadFileHandle) Stat() (os.FileInfo, error) { return fh.file, nil } - -// Close closes the file calling Flush then Release -func (fh *ReadFileHandle) Close() error { - err := fh.Flush() - err2 := fh.Release() - if err != nil { - return err - } - return err2 -} diff --git a/vfs/read_test.go b/vfs/read_test.go new file mode 100644 index 000000000..76613dfe4 --- /dev/null +++ b/vfs/read_test.go @@ -0,0 +1,238 @@ +package vfs + +import ( + "io" + "os" + "testing" + + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Open a file for write +func readHandleCreate(t *testing.T, r *fstest.Run) (*VFS, *ReadFileHandle) { + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("dir/file1", "0123456789abcdef", t1) + fstest.CheckItems(t, r.Fremote, file1) + + h, err := vfs.OpenFile("dir/file1", os.O_RDONLY, 0777) + require.NoError(t, err) + fh, ok := h.(*ReadFileHandle) + require.True(t, ok) + + return vfs, fh +} + +// read data from the string +func readString(t *testing.T, fh *ReadFileHandle, n int) string { + buf := make([]byte, n) + n, err := fh.Read(buf) + if err != io.EOF { + assert.NoError(t, err) + } + return string(buf[:n]) +} + +func TestReadFileHandleMethods(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := readHandleCreate(t, r) + + // String + assert.Equal(t, "dir/file1 (r)", fh.String()) + assert.Equal(t, "", (*ReadFileHandle)(nil).String()) + assert.Equal(t, "", new(ReadFileHandle).String()) + + // Node + node := fh.Node() + assert.Equal(t, "file1", node.Name()) + + // Size + assert.Equal(t, int64(16), fh.Size()) + + // Read 1 + assert.Equal(t, "0", readString(t, fh, 1)) + + // Read remainder + assert.Equal(t, "123456789abcdef", readString(t, fh, 256)) + + // Read EOF + buf := make([]byte, 16) + _, err := fh.Read(buf) + assert.Equal(t, io.EOF, err) + + // Stat + var fi os.FileInfo + fi, err = fh.Stat() + assert.NoError(t, err) + assert.Equal(t, int64(16), fi.Size()) + assert.Equal(t, "file1", fi.Name()) + + // Close + assert.False(t, fh.closed) + assert.Equal(t, nil, fh.Close()) + assert.True(t, fh.closed) + + // Close again + assert.Equal(t, EBADF, fh.Close()) +} + +func TestReadFileHandleSeek(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := readHandleCreate(t, r) + + assert.Equal(t, "0", readString(t, fh, 1)) + + // 0 means relative to the origin of the file, + n, err := fh.Seek(5, 0) + assert.NoError(t, err) + assert.Equal(t, int64(5), n) + assert.Equal(t, "5", readString(t, fh, 1)) + + // 1 means relative to the current offset + n, err = fh.Seek(-3, 1) + assert.NoError(t, err) + assert.Equal(t, int64(3), n) + assert.Equal(t, "3", readString(t, fh, 1)) + + // 2 means relative to the end. + n, err = fh.Seek(-3, 2) + assert.NoError(t, err) + assert.Equal(t, int64(13), n) + assert.Equal(t, "d", readString(t, fh, 1)) + + // Seek off the end + n, err = fh.Seek(100, 0) + assert.NoError(t, err) + + // Get the error on read + buf := make([]byte, 16) + l, err := fh.Read(buf) + assert.Equal(t, io.EOF, err) + assert.Equal(t, 0, l) + + // Check if noSeek is set we get an error + fh.noSeek = true + _, err = fh.Seek(0, 0) + assert.Equal(t, ESPIPE, err) + + // Close + assert.Equal(t, nil, fh.Close()) +} + +func TestReadFileHandleReadAt(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := readHandleCreate(t, r) + + // read from start + buf := make([]byte, 1) + n, err := fh.ReadAt(buf, 0) + require.NoError(t, err) + assert.Equal(t, 1, n) + assert.Equal(t, "0", string(buf[:n])) + + // seek forwards + n, err = fh.ReadAt(buf, 5) + require.NoError(t, err) + assert.Equal(t, 1, n) + assert.Equal(t, "5", string(buf[:n])) + + // seek backwards + n, err = fh.ReadAt(buf, 1) + require.NoError(t, err) + assert.Equal(t, 1, n) + assert.Equal(t, "1", string(buf[:n])) + + // read exactly to the end + buf = make([]byte, 6) + n, err = fh.ReadAt(buf, 10) + require.NoError(t, err) + assert.Equal(t, 6, n) + assert.Equal(t, "abcdef", string(buf[:n])) + + // read off the end + buf = make([]byte, 256) + n, err = fh.ReadAt(buf, 10) + assert.Equal(t, io.EOF, err) + assert.Equal(t, 6, n) + assert.Equal(t, "abcdef", string(buf[:n])) + + // read starting off the end + n, err = fh.ReadAt(buf, 100) + assert.Equal(t, io.EOF, err) + assert.Equal(t, 0, n) + + // check noSeek gives an error + fh.noSeek = true + n, err = fh.ReadAt(buf, 100) + assert.Equal(t, ESPIPE, err) + + // Properly close the file + assert.NoError(t, fh.Close()) + + // check reading on closed file + fh.noSeek = true + n, err = fh.ReadAt(buf, 100) + assert.Equal(t, EBADF, err) +} + +func TestReadFileHandleFlush(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := readHandleCreate(t, r) + + // Check Flush does nothing if read not called + err := fh.Flush() + assert.NoError(t, err) + assert.False(t, fh.closed) + + // Read data + buf := make([]byte, 256) + n, err := fh.Read(buf) + assert.Equal(t, io.EOF, err) + assert.Equal(t, 16, n) + + // Check Flush does nothing if read called + err = fh.Flush() + assert.NoError(t, err) + assert.False(t, fh.closed) + + // Check flush does nothing if called again + err = fh.Flush() + assert.NoError(t, err) + assert.False(t, fh.closed) + + // Properly close the file + assert.NoError(t, fh.Close()) +} + +func TestReadFileHandleRelease(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := readHandleCreate(t, r) + + // Check Release does nothing if file not read from + err := fh.Release() + assert.NoError(t, err) + assert.False(t, fh.closed) + + // Read data + buf := make([]byte, 256) + n, err := fh.Read(buf) + assert.Equal(t, io.EOF, err) + assert.Equal(t, 16, n) + + // Check Release closes file + err = fh.Release() + assert.NoError(t, err) + assert.True(t, fh.closed) + + // Check Release does nothing if called again + err = fh.Release() + assert.NoError(t, err) + assert.True(t, fh.closed) +} diff --git a/vfs/vfs.go b/vfs/vfs.go index 8c5a1e723..ca182c923 100644 --- a/vfs/vfs.go +++ b/vfs/vfs.go @@ -35,7 +35,7 @@ var DefaultOpt = Options{ Umask: 0, UID: ^uint32(0), // these values instruct WinFSP-FUSE to use the current user GID: ^uint32(0), // overriden for non windows in mount_unix.go - DirPerms: os.FileMode(0777), + DirPerms: os.FileMode(0777) | os.ModeDir, FilePerms: os.FileMode(0666), } @@ -175,6 +175,9 @@ func New(f fs.Fs, opt *Options) *VFS { vfs.Opt.DirPerms &= ^os.FileMode(vfs.Opt.Umask) vfs.Opt.FilePerms &= ^os.FileMode(vfs.Opt.Umask) + // Make sure directories are returned as directories + vfs.Opt.DirPerms |= os.ModeDir + // Create root directory vfs.root = newDir(vfs, f, nil, fsDir) diff --git a/vfs/vfs_test.go b/vfs/vfs_test.go new file mode 100644 index 000000000..c332520fe --- /dev/null +++ b/vfs/vfs_test.go @@ -0,0 +1,252 @@ +// Test suite for vfs + +package vfs + +import ( + "os" + "testing" + + _ "github.com/ncw/rclone/fs/all" // import all the file systems + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Some times used in the tests +var ( + t1 = fstest.Time("2001-02-03T04:05:06.499999999Z") + t2 = fstest.Time("2011-12-25T12:59:59.123456789Z") + t3 = fstest.Time("2011-12-30T12:59:59.000000000Z") +) + +// TestMain drives the tests +func TestMain(m *testing.M) { + fstest.TestMain(m) +} + +// Check baseHandle performs as advertised +func TestVFSbaseHandle(t *testing.T) { + fh := baseHandle{} + + err := fh.Chdir() + assert.Equal(t, ENOSYS, err) + + err = fh.Chmod(0) + assert.Equal(t, ENOSYS, err) + + err = fh.Chown(0, 0) + assert.Equal(t, ENOSYS, err) + + err = fh.Close() + assert.Equal(t, ENOSYS, err) + + fd := fh.Fd() + assert.Equal(t, uintptr(0), fd) + + name := fh.Name() + assert.Equal(t, "", name) + + _, err = fh.Read(nil) + assert.Equal(t, ENOSYS, err) + + _, err = fh.ReadAt(nil, 0) + assert.Equal(t, ENOSYS, err) + + _, err = fh.Readdir(0) + assert.Equal(t, ENOSYS, err) + + _, err = fh.Readdirnames(0) + assert.Equal(t, ENOSYS, err) + + _, err = fh.Seek(0, 0) + assert.Equal(t, ENOSYS, err) + + _, err = fh.Stat() + assert.Equal(t, ENOSYS, err) + + err = fh.Sync() + assert.Equal(t, nil, err) + + err = fh.Truncate(0) + assert.Equal(t, ENOSYS, err) + + _, err = fh.Write(nil) + assert.Equal(t, ENOSYS, err) + + _, err = fh.WriteAt(nil, 0) + assert.Equal(t, ENOSYS, err) + + _, err = fh.WriteString("") + assert.Equal(t, ENOSYS, err) + + err = fh.Flush() + assert.Equal(t, ENOSYS, err) + + err = fh.Release() + assert.Equal(t, ENOSYS, err) + + node := fh.Node() + assert.Nil(t, node) +} + +// TestNew sees if the New command works properly +func TestVFSNew(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + + // Check making a VFS with nil options + vfs := New(r.Fremote, nil) + assert.Equal(t, vfs.Opt, DefaultOpt) + assert.Equal(t, vfs.f, r.Fremote) + + // Check the initialisation + var opt = DefaultOpt + opt.DirPerms = 0777 + opt.FilePerms = 0666 + opt.Umask = 0002 + vfs = New(r.Fremote, &opt) + assert.Equal(t, os.FileMode(0775)|os.ModeDir, vfs.Opt.DirPerms) + assert.Equal(t, os.FileMode(0664), vfs.Opt.FilePerms) +} + +// TestRoot checks root directory is present and correct +func TestVFSRoot(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + root, err := vfs.Root() + require.NoError(t, err) + assert.Equal(t, vfs.root, root) + assert.True(t, root.IsDir()) + assert.Equal(t, vfs.Opt.DirPerms.Perm(), root.Mode().Perm()) +} + +func TestVFSStat(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("file1", "file1 contents", t1) + file2 := r.WriteObject("dir/file2", "file2 contents", t2) + fstest.CheckItems(t, r.Fremote, file1, file2) + + node, err := vfs.Stat("file1") + require.NoError(t, err) + assert.True(t, node.IsFile()) + assert.Equal(t, "file1", node.Name()) + + node, err = vfs.Stat("dir") + require.NoError(t, err) + assert.True(t, node.IsDir()) + assert.Equal(t, "dir", node.Name()) + + node, err = vfs.Stat("dir/file2") + require.NoError(t, err) + assert.True(t, node.IsFile()) + assert.Equal(t, "file2", node.Name()) + + node, err = vfs.Stat("not found") + assert.Equal(t, os.ErrNotExist, err) + + node, err = vfs.Stat("dir/not found") + assert.Equal(t, os.ErrNotExist, err) + + node, err = vfs.Stat("not found/not found") + assert.Equal(t, os.ErrNotExist, err) + + node, err = vfs.Stat("file1/under a file") + assert.Equal(t, os.ErrNotExist, err) +} + +func TestVFSStatParent(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("file1", "file1 contents", t1) + file2 := r.WriteObject("dir/file2", "file2 contents", t2) + fstest.CheckItems(t, r.Fremote, file1, file2) + + node, leaf, err := vfs.StatParent("file1") + require.NoError(t, err) + assert.True(t, node.IsDir()) + assert.Equal(t, "/", node.Name()) + assert.Equal(t, "file1", leaf) + + node, leaf, err = vfs.StatParent("dir/file2") + require.NoError(t, err) + assert.True(t, node.IsDir()) + assert.Equal(t, "dir", node.Name()) + assert.Equal(t, "file2", leaf) + + node, leaf, err = vfs.StatParent("not found") + require.NoError(t, err) + assert.True(t, node.IsDir()) + assert.Equal(t, "/", node.Name()) + assert.Equal(t, "not found", leaf) + + node, leaf, err = vfs.StatParent("not found dir/not found") + assert.Equal(t, os.ErrNotExist, err) + + node, leaf, err = vfs.StatParent("file1/under a file") + assert.Equal(t, os.ErrExist, err) +} + +func TestVFSOpenFile(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("file1", "file1 contents", t1) + file2 := r.WriteObject("dir/file2", "file2 contents", t2) + fstest.CheckItems(t, r.Fremote, file1, file2) + + fd, err := vfs.OpenFile("file1", os.O_RDONLY, 0777) + require.NoError(t, err) + assert.NotNil(t, fd) + require.NoError(t, fd.Close()) + + fd, err = vfs.OpenFile("dir", os.O_RDONLY, 0777) + require.NoError(t, err) + assert.NotNil(t, fd) + require.NoError(t, fd.Close()) + + fd, err = vfs.OpenFile("dir/new_file.txt", os.O_RDONLY, 0777) + assert.Equal(t, os.ErrNotExist, err) + assert.Nil(t, fd) + + fd, err = vfs.OpenFile("dir/new_file.txt", os.O_WRONLY|os.O_CREATE, 0777) + require.NoError(t, err) + assert.NotNil(t, fd) + require.NoError(t, fd.Close()) + + fd, err = vfs.OpenFile("not found/new_file.txt", os.O_WRONLY|os.O_CREATE, 0777) + assert.Equal(t, os.ErrNotExist, err) + assert.Nil(t, fd) +} + +func TestVFSRename(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs := New(r.Fremote, nil) + + file1 := r.WriteObject("dir/file2", "file2 contents", t2) + fstest.CheckItems(t, r.Fremote, file1) + + err := vfs.Rename("dir/file2", "dir/file1") + require.NoError(t, err) + file1.Path = "dir/file1" + fstest.CheckItems(t, r.Fremote, file1) + + err = vfs.Rename("dir/file1", "file0") + require.NoError(t, err) + file1.Path = "file0" + fstest.CheckItems(t, r.Fremote, file1) + + err = vfs.Rename("not found/file0", "file0") + assert.Equal(t, os.ErrNotExist, err) + + err = vfs.Rename("file0", "not found/file0") + assert.Equal(t, os.ErrNotExist, err) +} diff --git a/vfs/write.go b/vfs/write.go index 3e90d103b..0d82fa5ce 100644 --- a/vfs/write.go +++ b/vfs/write.go @@ -39,6 +39,7 @@ func newWriteFileHandle(d *Dir, f *File, src fs.ObjectInfo) (*WriteFileHandle, e var pipeReader *io.PipeReader pipeReader, fh.pipeWriter = io.Pipe() go func() { + // NB Rcat deals with Stats.Transferring etc o, err := fs.Rcat(d.f, src.Remote(), pipeReader, time.Now()) if err != nil { fs.Errorf(fh.remote, "WriteFileHandle.New Rcat failed: %v", err) @@ -85,14 +86,14 @@ func (fh *WriteFileHandle) WriteAt(p []byte, off int64) (n int, err error) { // fs.Debugf(fh.remote, "WriteFileHandle.Write len=%d", len(p)) fh.mu.Lock() defer fh.mu.Unlock() - if fh.offset != off { - fs.Errorf(fh.remote, "WriteFileHandle.Write can't seek in file") - return 0, ESPIPE - } if fh.closed { fs.Errorf(fh.remote, "WriteFileHandle.Write error: %v", EBADF) return 0, EBADF } + if fh.offset != off { + fs.Errorf(fh.remote, "WriteFileHandle.Write can't seek in file") + return 0, ESPIPE + } fh.writeCalled = true n, err = fh.pipeWriter.Write(p) fh.offset += int64(n) @@ -141,6 +142,13 @@ func (fh *WriteFileHandle) close() error { return err } +// Close closes the file +func (fh *WriteFileHandle) Close() error { + fh.mu.Lock() + defer fh.mu.Unlock() + return fh.close() +} + // Flush is called on each close() of a file descriptor. So if a // filesystem wants to return write errors in close() and the file has // cached dirty data, this is a good place to write back data and @@ -159,6 +167,10 @@ func (fh *WriteFileHandle) close() error { func (fh *WriteFileHandle) Flush() error { fh.mu.Lock() defer fh.mu.Unlock() + if fh.closed { + fs.Debugf(fh.remote, "WriteFileHandle.Flush nothing to do") + return nil + } // fs.Debugf(fh.remote, "WriteFileHandle.Flush") // If Write hasn't been called then ignore the Flush - Release // will pick it up @@ -201,13 +213,3 @@ func (fh *WriteFileHandle) Release() error { func (fh *WriteFileHandle) Stat() (os.FileInfo, error) { return fh.file, nil } - -// Close closes the file calling Flush then Release -func (fh *WriteFileHandle) Close() error { - err := fh.Flush() - err2 := fh.Release() - if err != nil { - return err - } - return err2 -} diff --git a/vfs/write_test.go b/vfs/write_test.go new file mode 100644 index 000000000..fd494d1eb --- /dev/null +++ b/vfs/write_test.go @@ -0,0 +1,160 @@ +package vfs + +import ( + "os" + "testing" + + "github.com/ncw/rclone/fs" + "github.com/ncw/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Open a file for write +func writeHandleCreate(t *testing.T, r *fstest.Run) (*VFS, *WriteFileHandle) { + vfs := New(r.Fremote, nil) + + h, err := vfs.OpenFile("file1", os.O_WRONLY|os.O_CREATE, 0777) + require.NoError(t, err) + fh, ok := h.(*WriteFileHandle) + require.True(t, ok) + + return vfs, fh +} + +func TestWriteFileHandleMethods(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, fh := writeHandleCreate(t, r) + + // String + assert.Equal(t, "file1 (w)", fh.String()) + assert.Equal(t, "", (*WriteFileHandle)(nil).String()) + assert.Equal(t, "", new(WriteFileHandle).String()) + + // Node + node := fh.Node() + assert.Equal(t, "file1", node.Name()) + + // Offset #1 + assert.Equal(t, int64(0), fh.Offset()) + assert.Equal(t, int64(0), node.Size()) + + // Write (smoke test only since heavy lifting done in WriteAt) + n, err := fh.Write([]byte("hello")) + assert.NoError(t, err) + assert.Equal(t, 5, n) + + // Offset #2 + assert.Equal(t, int64(5), fh.Offset()) + assert.Equal(t, int64(5), node.Size()) + + // Stat + var fi os.FileInfo + fi, err = fh.Stat() + assert.NoError(t, err) + assert.Equal(t, int64(5), fi.Size()) + assert.Equal(t, "file1", fi.Name()) + + // Close + assert.NoError(t, fh.Close()) + + // Check double close + err = fh.Close() + assert.Equal(t, EBADF, err) + + // check vfs + root, err := vfs.Root() + checkListing(t, root, []string{"file1,5,false"}) + + // check the underlying r.Fremote but not the modtime + file1 := fstest.NewItem("file1", "hello", t1) + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{}, fs.ModTimeNotSupported) +} + +func TestWriteFileHandleWriteAt(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + vfs, fh := writeHandleCreate(t, r) + + // Preconditions + assert.Equal(t, int64(0), fh.offset) + assert.False(t, fh.writeCalled) + + // Write the data + n, err := fh.WriteAt([]byte("hello"), 0) + assert.NoError(t, err) + assert.Equal(t, 5, n) + + // After write + assert.Equal(t, int64(5), fh.offset) + assert.True(t, fh.writeCalled) + + // Check can't seek + n, err = fh.WriteAt([]byte("hello"), 100) + assert.Equal(t, ESPIPE, err) + assert.Equal(t, 0, n) + + // Write more data + n, err = fh.WriteAt([]byte(" world"), 5) + assert.NoError(t, err) + assert.Equal(t, 6, n) + + // Close + assert.NoError(t, fh.Close()) + + // Check can't write on closed handle + n, err = fh.WriteAt([]byte("hello"), 0) + assert.Equal(t, EBADF, err) + assert.Equal(t, 0, n) + + // check vfs + root, err := vfs.Root() + checkListing(t, root, []string{"file1,11,false"}) + + // check the underlying r.Fremote but not the modtime + file1 := fstest.NewItem("file1", "hello world", t1) + fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{}, fs.ModTimeNotSupported) +} + +func TestWriteFileHandleFlush(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := writeHandleCreate(t, r) + + // Check Flush does nothing if write not called + err := fh.Flush() + assert.NoError(t, err) + assert.False(t, fh.closed) + + // Write some data + n, err := fh.Write([]byte("hello")) + assert.NoError(t, err) + assert.Equal(t, 5, n) + + // Check Flush closes file if write called + err = fh.Flush() + assert.NoError(t, err) + assert.True(t, fh.closed) + + // Check flush does nothing if called again + err = fh.Flush() + assert.NoError(t, err) + assert.True(t, fh.closed) +} + +func TestWriteFileHandleRelease(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + _, fh := writeHandleCreate(t, r) + + // Check Release closes file + err := fh.Release() + assert.NoError(t, err) + assert.True(t, fh.closed) + + // Check Release does nothing if called again + err = fh.Release() + assert.NoError(t, err) + assert.True(t, fh.closed) +}