webdav: make the memFS (not the memFSNode) the canonical source of a

file name.

This fixes inconsistent stat names after a file is renamed.

Change-Id: Ie90f8abaa31d46a87834266053b61d7770f854e2
Reviewed-on: https://go-review.googlesource.com/3051
Reviewed-by: Nick Cooper <nmvc@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>

webdav/file.go

@@ -194,7 +194,7 @@ func (fs *memFS) walk(op, fullname string, f func(dir *memFSNode, frag string, f
 				Err:  os.ErrNotExist,
 			}
 		}
-		if !child.IsDir() {
+		if !child.mode.IsDir() {
 			return &os.PathError{
 				Op:   op,
 				Path: original,
@@ -245,7 +245,6 @@ func (fs *memFS) Mkdir(name string, perm os.FileMode) error {
 		return os.ErrExist
 	}
 	dir.children[frag] = &memFSNode{
-		name:     frag,
 		children: make(map[string]*memFSNode),
 		mode:     perm.Perm() | os.ModeDir,
 		modTime:  time.Now(),
@@ -267,7 +266,7 @@ func (fs *memFS) OpenFile(name string, flag int, perm os.FileMode) (File, error)
 		if flag&(os.O_WRONLY|os.O_RDWR) != 0 {
 			return nil, os.ErrPermission
 		}
-		n = &fs.root
+		n, frag = &fs.root, "/"
 
 	} else {
 		n = dir.children[frag]
@@ -281,7 +280,6 @@ func (fs *memFS) OpenFile(name string, flag int, perm os.FileMode) (File, error)
 			}
 			if n == nil {
 				n = &memFSNode{
-					name: frag,
 					mode: perm.Perm(),
 				}
 				dir.children[frag] = n
@@ -298,11 +296,12 @@ func (fs *memFS) OpenFile(name string, flag int, perm os.FileMode) (File, error)
 	}
 
 	children := make([]os.FileInfo, 0, len(n.children))
-	for _, c := range n.children {
-		children = append(children, c)
+	for cName, c := range n.children {
+		children = append(children, c.stat(cName))
 	}
 	return &memFile{
 		n:                n,
+		nameSnapshot:     frag,
 		childrenSnapshot: children,
 	}, nil
 }
@@ -359,12 +358,9 @@ func (fs *memFS) Rename(oldName, newName string) error {
 	if !ok {
 		return os.ErrNotExist
 	}
-	if oNode.IsDir() {
+	if oNode.children != nil {
 		if nNode, ok := nDir.children[nFrag]; ok {
-			nNode.mu.Lock()
-			isDir := nNode.mode.IsDir()
-			nNode.mu.Unlock()
-			if !isDir {
+			if nNode.children == nil {
 				return errNotADirectory
 			}
 			if len(nNode.children) != 0 {
@@ -387,10 +383,10 @@ func (fs *memFS) Stat(name string) (os.FileInfo, error) {
 	}
 	if dir == nil {
 		// We're stat'ting the root.
-		return &fs.root, nil
+		return fs.root.stat("/"), nil
 	}
 	if n, ok := dir.children[frag]; ok {
-		return n, nil
+		return n.stat(path.Base(name)), nil
 	}
 	return nil, os.ErrNotExist
 }
@@ -398,53 +394,46 @@ func (fs *memFS) Stat(name string) (os.FileInfo, error) {
 // A memFSNode represents a single entry in the in-memory filesystem and also
 // implements os.FileInfo.
 type memFSNode struct {
-	name string
-
 	// children is protected by memFS.mu.
 	children map[string]*memFSNode
 
 	mu      sync.Mutex
-	modTime time.Time
-	mode    os.FileMode
 	data    []byte
+	mode    os.FileMode
+	modTime time.Time
 }
 
-func (n *memFSNode) Name() string {
-	return n.name
-}
-
-func (n *memFSNode) Size() int64 {
-	n.mu.Lock()
-	defer n.mu.Unlock()
-	return int64(len(n.data))
-}
-
-func (n *memFSNode) Mode() os.FileMode {
-	n.mu.Lock()
-	defer n.mu.Unlock()
-	return n.mode
-}
-
-func (n *memFSNode) ModTime() time.Time {
+func (n *memFSNode) stat(name string) *memFileInfo {
 	n.mu.Lock()
 	defer n.mu.Unlock()
-	return n.modTime
+	return &memFileInfo{
+		name:    name,
+		size:    int64(len(n.data)),
+		mode:    n.mode,
+		modTime: n.modTime,
+	}
 }
 
-func (n *memFSNode) IsDir() bool {
-	return n.Mode().IsDir()
+type memFileInfo struct {
+	name    string
+	size    int64
+	mode    os.FileMode
+	modTime time.Time
 }
 
-func (n *memFSNode) Sys() interface{} {
-	return nil
-}
+func (f *memFileInfo) Name() string       { return f.name }
+func (f *memFileInfo) Size() int64        { return f.size }
+func (f *memFileInfo) Mode() os.FileMode  { return f.mode }
+func (f *memFileInfo) ModTime() time.Time { return f.modTime }
+func (f *memFileInfo) IsDir() bool        { return f.mode.IsDir() }
+func (f *memFileInfo) Sys() interface{}   { return nil }
 
 // A memFile is a File implementation for a memFSNode. It is a per-file (not
-// per-node) read/write position, and if the node is a directory, a snapshot of
-// that node's children.
+// per-node) read/write position, and a snapshot of the memFS' tree structure
+// (a node's name and children) for that node.
 type memFile struct {
-	n *memFSNode
-	// childrenSnapshot is a snapshot of n.children.
+	n                *memFSNode
+	nameSnapshot     string
 	childrenSnapshot []os.FileInfo
 	// pos is protected by n.mu.
 	pos int
@@ -518,7 +507,7 @@ func (f *memFile) Seek(offset int64, whence int) (int64, error) {
 }
 
 func (f *memFile) Stat() (os.FileInfo, error) {
-	return f.n, nil
+	return f.n.stat(f.nameSnapshot), nil
 }
 
 func (f *memFile) Write(p []byte) (int, error) {

webdav/file_test.go

@@ -11,6 +11,8 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"reflect"
+	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -202,10 +204,10 @@ func TestWalk(t *testing.T) {
 			}
 		}
 
-		i := 0
+		i, prevFrag := 0, ""
 		err := fs.walk("test", tc.dir, func(dir *memFSNode, frag string, final bool) error {
 			got := walkStep{
-				name:  dir.name,
+				name:  prevFrag,
 				frag:  frag,
 				final: final,
 			}
@@ -214,7 +216,7 @@ func TestWalk(t *testing.T) {
 			if got != want {
 				return fmt.Errorf("got %+v, want %+v", got, want)
 			}
-			i++
+			i, prevFrag = i+1, frag
 			return nil
 		})
 		if err != nil {
@@ -223,6 +225,36 @@ func TestWalk(t *testing.T) {
 	}
 }
 
+// find appends to ss the names of the named file and its children. It is
+// analogous to the Unix find command.
+//
+// The returned strings are not guaranteed to be in any particular order.
+func find(ss []string, fs FileSystem, name string) ([]string, error) {
+	stat, err := fs.Stat(name)
+	if err != nil {
+		return nil, err
+	}
+	ss = append(ss, name)
+	if stat.IsDir() {
+		f, err := fs.OpenFile(name, os.O_RDONLY, 0)
+		if err != nil {
+			return nil, err
+		}
+		defer f.Close()
+		children, err := f.Readdir(-1)
+		if err != nil {
+			return nil, err
+		}
+		for _, c := range children {
+			ss, err = find(ss, fs, path.Join(name, c.Name()))
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+	return ss, nil
+}
+
 func testFS(t *testing.T, fs FileSystem) {
 	errStr := func(err error) string {
 		switch {
@@ -236,8 +268,8 @@ func testFS(t *testing.T, fs FileSystem) {
 		return "ok"
 	}
 
-	// The non-"stat" test cases should change the file system state. The
-	// indentation of the "stat"s helps distinguish such test cases.
+	// The non-"find" non-"stat" test cases should change the file system state. The
+	// indentation of the "find"s and "stat"s helps distinguish such test cases.
 	testCases := []string{
 		"  stat / want dir",
 		"  stat /a want errNotExist",
@@ -252,6 +284,7 @@ func testFS(t *testing.T, fs FileSystem) {
 		"  stat /d want dir",
 		"create /d/e EEE want ok",
 		"  stat /d/e want 3",
+		"  find / /a /d /d/e",
 		"create /d/f FFFF want ok",
 		"create /d/g GGGGGGG want ok",
 		"mk-dir /d/m want ok",
@@ -263,6 +296,7 @@ func testFS(t *testing.T, fs FileSystem) {
 		"  stat /d/h want errNotExist",
 		"  stat /d/m want dir",
 		"  stat /d/m/p want 5",
+		"  find / /a /d /d/e /d/f /d/g /d/m /d/m/p",
 		"rm-all /d want ok",
 		"  stat /a want 1",
 		"  stat /d want errNotExist",
@@ -271,6 +305,7 @@ func testFS(t *testing.T, fs FileSystem) {
 		"  stat /d/g want errNotExist",
 		"  stat /d/m want errNotExist",
 		"  stat /d/m/p want errNotExist",
+		"  find / /a",
 		"mk-dir /d/m want errNotExist",
 		"mk-dir /d want ok",
 		"create /d/f FFFF want ok",
@@ -285,6 +320,7 @@ func testFS(t *testing.T, fs FileSystem) {
 		"  stat /c want errNotExist",
 		"  stat /d want dir",
 		"  stat /d/m want dir",
+		"  find / /a /b /d /d/m",
 		"rename /b /b want ok",
 		"  stat /b want 2",
 		"  stat /c want errNotExist",
@@ -293,6 +329,7 @@ func testFS(t *testing.T, fs FileSystem) {
 		"  stat /c want 2",
 		"  stat /d/m want dir",
 		"  stat /d/n want errNotExist",
+		"  find / /a /c /d /d/m",
 		"rename /d/m /d/n want ok",
 		"create /d/n/q QQQQ want ok",
 		"  stat /d/m want errNotExist",
@@ -302,6 +339,7 @@ func testFS(t *testing.T, fs FileSystem) {
 		"rename /c /d/n/q want ok",
 		"  stat /c want errNotExist",
 		"  stat /d/n/q want 2",
+		"  find / /a /d /d/n /d/n/q",
 		"create /d/n/r RRRRR want ok",
 		"mk-dir /u want ok",
 		"mk-dir /u/v want ok",
@@ -316,10 +354,12 @@ func testFS(t *testing.T, fs FileSystem) {
 		"  stat /t want dir",
 		"  stat /t/q want 2",
 		"  stat /t/r want 5",
+		"  find / /a /d /t /t/q /t/r /u /u/v",
 		"rename /t / want err",
 		"rename /t /u/v want ok",
 		"  stat /u/v/r want 5",
 		"rename / /x want err",
+		"  find / /a /d /u /u/v /u/v/q /u/v/r",
 	}
 
 	for i, tc := range testCases {
@@ -352,6 +392,17 @@ func testFS(t *testing.T, fs FileSystem) {
 				}
 			}
 
+		case "find":
+			got, err := find(nil, fs, "/")
+			if err != nil {
+				t.Fatalf("test case #%d %q: find: %v", i, tc, err)
+			}
+			sort.Strings(got)
+			want := strings.Split(arg, " ")
+			if !reflect.DeepEqual(got, want) {
+				t.Fatalf("test case #%d %q:\ngot  %s\nwant %s", i, tc, got, want)
+			}
+
 		case "mk-dir", "rename", "rm-all", "stat":
 			nParts := 3
 			if op == "rename" {
@@ -372,12 +423,22 @@ func testFS(t *testing.T, fs FileSystem) {
 				opErr = fs.RemoveAll(parts[0])
 			case "stat":
 				var stat os.FileInfo
-				if stat, opErr = fs.Stat(parts[0]); opErr == nil {
+				fileName := parts[0]
+				if stat, opErr = fs.Stat(fileName); opErr == nil {
 					if stat.IsDir() {
 						got = "dir"
 					} else {
 						got = strconv.Itoa(int(stat.Size()))
 					}
+
+					if fileName == "/" {
+						// For a Dir FileSystem, the virtual file system root maps to a
+						// real file system name like "/tmp/webdav-test012345", which does
+						// not end with "/". We skip such cases.
+					} else if statName := stat.Name(); path.Base(fileName) != statName {
+						t.Fatalf("test case #%d %q: file name %q inconsistent with stat name %q",
+							i, tc, fileName, statName)
+					}
 				}
 			}
 			if got == "" {