mount: implement renaming directories - fixes #954

This also fixes various caching issues renaming files.

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)
+		// FIXME: could Copy then Delete if Move not available
-		if !ok {
+		// - though care needed if case insensitive...
-			err := errors.Errorf("Fs %q can't Move files", d.f)
+		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)
+		doDirMove := d.f.Features().DirMove
-		empty, err := oldDir.isEmpty()
+		if doDirMove == nil {
-		if err != nil {
+			err := errors.Errorf("Fs %q can't rename directories (no DirMove)", d.f)
-			fs.Errorf(oldPath, "Dir.Rename dir error: %v", err)
+			fs.Errorf(oldPath, "Dir.Rename error: %v", err)
 			return err
 		}
-		if !empty {
+		srcRemote := x.Name
-			// return fuse.ENOTEMPTY - doesn't exist though so use EEXIST
+		dstRemote := newPath
-			fs.Errorf(oldPath, "Dir.Rename can't rename non empty directory")
+		err = doDirMove(d.f, srcRemote, dstRemote)
-			return fuse.EEXIST
-		}
-		err = d.f.Rmdir(oldPath)
 		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)
+		newDir := new(fs.Dir)
-		if err != nil {
+		*newDir = *x
-			fs.Errorf(newPath, "Dir.Rename failed to create directory: %v", err)
+		newDir.Name = newPath
-			return err
+		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)
 			}
-		newObj = &fs.Dir{
-			Name: newPath,
-			When: time.Now(),
 		}
 	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
+	fs.Debugf(newPath, "Dir.Rename renamed from %q", oldPath)
-
-	// 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)
 	return nil
 }
 

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,9 +121,6 @@ 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")
 
@@ -124,10 +130,6 @@ func TestDirRenameFullDir(t *testing.T) {
 
 	run.rm(t, "dir/dir3/potato.txt")
 	run.rmdir(t, "dir/dir3")
-	*/
-
-	run.rm(t, "dir1/potato.txt")
-	run.rmdir(t, "dir1")
 	run.rmdir(t, "dir")
 	run.checkDir(t, "")
 }

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

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