From ef604f6100d0843e3e97b4274b6c1b7c10cd639f Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 6 Feb 2017 21:17:45 +0000 Subject: [PATCH] mount: implement renaming directories - fixes #954 This also fixes various caching issues renaming files. --- cmd/mount/dir.go | 83 +++++++++++++++++++++++++------------------ cmd/mount/dir_test.go | 36 ++++++++++--------- cmd/mount/file.go | 18 ++++++++-- cmd/mount/fs.go | 4 ++- 4 files changed, 86 insertions(+), 55 deletions(-) diff --git a/cmd/mount/dir.go b/cmd/mount/dir.go index 29535f637..6885feccc 100644 --- a/cmd/mount/dir.go +++ b/cmd/mount/dir.go @@ -43,6 +43,17 @@ func newDir(f fs.Fs, fsDir *fs.Dir) *Dir { } } +// rename should be called after the directory is renamed +// +// Reset the directory to new state, discarding all the objects and +// reading everything again +func (d *Dir) rename(newParent *Dir, fsDir *fs.Dir) { + d.path = fsDir.Name + d.modTime = fsDir.When + d.items = nil + d.read = time.Time{} +} + // addObject adds a new object or directory to the directory // // note that we add new objects rather than updating old ones @@ -160,7 +171,6 @@ var _ fusefs.Node = (*Dir)(nil) // Attr updates the attribes of a directory func (d *Dir) Attr(ctx context.Context, a *fuse.Attr) error { - fs.Debugf(d.path, "Dir.Attr") a.Gid = gid a.Uid = uid a.Mode = os.ModeDir | dirPerms @@ -168,7 +178,8 @@ func (d *Dir) Attr(ctx context.Context, a *fuse.Attr) error { a.Mtime = d.modTime a.Ctime = d.modTime a.Crtime = d.modTime - // FIXME include Valid so get some caching? Also mtime + // FIXME include Valid so get some caching? + fs.Debugf(d.path, "Dir.Attr %+v", a) return nil } @@ -194,7 +205,7 @@ func (d *Dir) lookupNode(leaf string) (item *DirEntry, err error) { return nil, err } item = d.addObject(item.o, node) - return item, err + return item, nil } // Check interface satisfied @@ -367,46 +378,55 @@ func (d *Dir) Rename(ctx context.Context, req *fuse.RenameRequest, newDir fusefs return err } var newObj fs.BasicInfo + oldNode := oldItem.node switch x := oldItem.o.(type) { case fs.Object: oldObject := x - do, ok := d.f.(fs.Mover) - if !ok { - err := errors.Errorf("Fs %q can't Move files", d.f) + // FIXME: could Copy then Delete if Move not available + // - though care needed if case insensitive... + doMove := d.f.Features().Move + if doMove == nil { + err := errors.Errorf("Fs %q can't rename files (no Move)", d.f) fs.Errorf(oldPath, "Dir.Rename error: %v", err) return err } - newObject, err := do.Move(oldObject, newPath) + newObject, err := doMove(oldObject, newPath) if err != nil { fs.Errorf(oldPath, "Dir.Rename error: %v", err) return err } newObj = newObject + // Update the node with the new details + if oldNode != nil { + if oldFile, ok := oldNode.(*File); ok { + fs.Debugf(oldItem.o, "Updating file with %v %p", newObject, oldFile) + oldFile.rename(destDir, newObject) + } + } case *fs.Dir: - oldDir := oldItem.node.(*Dir) - empty, err := oldDir.isEmpty() - if err != nil { - fs.Errorf(oldPath, "Dir.Rename dir error: %v", err) + doDirMove := d.f.Features().DirMove + if doDirMove == nil { + err := errors.Errorf("Fs %q can't rename directories (no DirMove)", d.f) + fs.Errorf(oldPath, "Dir.Rename error: %v", err) return err } - if !empty { - // return fuse.ENOTEMPTY - doesn't exist though so use EEXIST - fs.Errorf(oldPath, "Dir.Rename can't rename non empty directory") - return fuse.EEXIST - } - err = d.f.Rmdir(oldPath) + srcRemote := x.Name + dstRemote := newPath + err = doDirMove(d.f, srcRemote, dstRemote) if err != nil { - fs.Errorf(oldPath, "Dir.Rename failed to remove directory: %v", err) + fs.Errorf(oldPath, "Dir.Rename error: %v", err) return err } - err = d.f.Mkdir(newPath) - if err != nil { - fs.Errorf(newPath, "Dir.Rename failed to create directory: %v", err) - return err - } - newObj = &fs.Dir{ - Name: newPath, - When: time.Now(), + newDir := new(fs.Dir) + *newDir = *x + newDir.Name = newPath + newObj = newDir + // Update the node with the new details + if oldNode != nil { + if oldDir, ok := oldNode.(*Dir); ok { + fs.Debugf(oldItem.o, "Updating dir with %v %p", newDir, oldDir) + oldDir.rename(destDir, newDir) + } } default: err = errors.Errorf("unknown type %T", oldItem) @@ -416,16 +436,9 @@ func (d *Dir) Rename(ctx context.Context, req *fuse.RenameRequest, newDir fusefs // Show moved - delete from old dir and add to new d.delObject(req.OldName) - destDir.addObject(newObj, nil) + destDir.addObject(newObj, oldNode) - // FIXME need to flush the dir also - - // FIXME use DirMover to move a directory? - // or maybe use MoveDir which can move anything - // fallback to Copy/Delete if no Move? - // if dir is empty then can move it - - fs.Errorf(newPath, "Dir.Rename renamed from %q", oldPath) + fs.Debugf(newPath, "Dir.Rename renamed from %q", oldPath) return nil } diff --git a/cmd/mount/dir_test.go b/cmd/mount/dir_test.go index b8e49dc79..45987b600 100644 --- a/cmd/mount/dir_test.go +++ b/cmd/mount/dir_test.go @@ -3,6 +3,7 @@ package mount import ( + "io/ioutil" "os" "testing" @@ -70,14 +71,22 @@ func TestDirRenameFile(t *testing.T) { run.createFile(t, "file", "potato") run.checkDir(t, "dir/|file 6") - err := os.Rename(run.path("file"), run.path("dir/file2")) + err := os.Rename(run.path("file"), run.path("file2")) require.NoError(t, err) - run.checkDir(t, "dir/|dir/file2 6") + run.checkDir(t, "dir/|file2 6") - err = os.Rename(run.path("dir/file2"), run.path("dir/file3")) + data, err := ioutil.ReadFile(run.path("file2")) + require.NoError(t, err) + assert.Equal(t, "potato", string(data)) + + err = os.Rename(run.path("file2"), run.path("dir/file3")) require.NoError(t, err) run.checkDir(t, "dir/|dir/file3 6") + data, err = ioutil.ReadFile(run.path("dir/file3")) + require.NoError(t, err) + assert.Equal(t, "potato", string(data)) + run.rm(t, "dir/file3") run.rmdir(t, "dir") run.checkDir(t, "") @@ -112,22 +121,15 @@ func TestDirRenameFullDir(t *testing.T) { run.checkDir(t, "dir/|dir1/|dir1/potato.txt 11") err := os.Rename(run.path("dir1"), run.path("dir/dir2")) - require.Error(t, err, "file exists") - // Can't currently rename directories with stuff in - /* - require.NoError(t, err) - run.checkDir(t, "dir/|dir/dir2/|dir/dir2/potato.txt 11") + require.NoError(t, err) + run.checkDir(t, "dir/|dir/dir2/|dir/dir2/potato.txt 11") - err = os.Rename(run.path("dir/dir2"), run.path("dir/dir3")) - require.NoError(t, err) - run.checkDir(t, "dir/|dir/dir3/|dir/dir3/potato.txt 11") + err = os.Rename(run.path("dir/dir2"), run.path("dir/dir3")) + require.NoError(t, err) + run.checkDir(t, "dir/|dir/dir3/|dir/dir3/potato.txt 11") - run.rm(t, "dir/dir3/potato.txt") - run.rmdir(t, "dir/dir3") - */ - - run.rm(t, "dir1/potato.txt") - run.rmdir(t, "dir1") + run.rm(t, "dir/dir3/potato.txt") + run.rmdir(t, "dir/dir3") run.rmdir(t, "dir") run.checkDir(t, "") } diff --git a/cmd/mount/file.go b/cmd/mount/file.go index 01f552fa2..b9ee78380 100644 --- a/cmd/mount/file.go +++ b/cmd/mount/file.go @@ -31,6 +31,14 @@ func newFile(d *Dir, o fs.Object) *File { } } +// rename should be called to update f.o and f.d after a rename +func (f *File) rename(d *Dir, o fs.Object) { + f.mu.Lock() + f.o = o + f.d = d + f.mu.Unlock() +} + // addWriters increments or decrements the writers func (f *File) addWriters(n int) { f.mu.Lock() @@ -45,7 +53,6 @@ var _ fusefs.Node = (*File)(nil) func (f *File) Attr(ctx context.Context, a *fuse.Attr) error { f.mu.Lock() defer f.mu.Unlock() - fs.Debugf(f.o, "File.Attr") a.Gid = gid a.Uid = uid a.Mode = filePerms @@ -63,6 +70,7 @@ func (f *File) Attr(ctx context.Context, a *fuse.Attr) error { } } a.Blocks = (a.Size + 511) / 512 + fs.Debugf(f.o, "File.Attr %+v", a) return nil } @@ -118,12 +126,18 @@ func (f *File) Open(ctx context.Context, req *fuse.OpenRequest, resp *fuse.OpenR if noSeek { resp.Flags |= fuse.OpenNonSeekable } - return newReadFileHandle(o) + fh, err := newReadFileHandle(o) + if err != nil { + fs.Debugf(o, "File.Open failed to open for read: %v", err) + return nil, err + } + return fh, nil case req.Flags.IsWriteOnly() || (req.Flags.IsReadWrite() && (req.Flags&fuse.OpenTruncate) != 0): resp.Flags |= fuse.OpenNonSeekable src := newCreateInfo(f.d.f, o.Remote()) fh, err := newWriteFileHandle(f.d, f, src) if err != nil { + fs.Debugf(o, "File.Open failed to open for write: %v", err) return nil, err } return fh, nil diff --git a/cmd/mount/fs.go b/cmd/mount/fs.go index 3aea54e70..5b75c1972 100644 --- a/cmd/mount/fs.go +++ b/cmd/mount/fs.go @@ -85,10 +85,12 @@ func mount(f fs.Fs, mountpoint string) (<-chan error, error) { f: f, } + server := fusefs.New(c, nil) + // Serve the mount point in the background returning error to errChan errChan := make(chan error, 1) go func() { - err := fusefs.Serve(c, filesys) + err := server.Serve(filesys) closeErr := c.Close() if err == nil { err = closeErr