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>
diff --git a/webdav/file.go b/webdav/file.go
index eac339f..eb6abd5 100644
--- a/webdav/file.go
+++ b/webdav/file.go
@@ -194,7 +194,7 @@
Err: os.ErrNotExist,
}
}
- if !child.IsDir() {
+ if !child.mode.IsDir() {
return &os.PathError{
Op: op,
Path: original,
@@ -245,7 +245,6 @@
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 @@
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 @@
}
if n == nil {
n = &memFSNode{
- name: frag,
mode: perm.Perm(),
}
dir.children[frag] = n
@@ -298,11 +296,12 @@
}
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 @@
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 @@
}
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 @@
// 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 {
+func (n *memFSNode) stat(name string) *memFileInfo {
n.mu.Lock()
defer n.mu.Unlock()
- return int64(len(n.data))
+ return &memFileInfo{
+ name: name,
+ size: int64(len(n.data)),
+ mode: n.mode,
+ modTime: n.modTime,
+ }
}
-func (n *memFSNode) Mode() os.FileMode {
- n.mu.Lock()
- defer n.mu.Unlock()
- return n.mode
+type memFileInfo struct {
+ name string
+ size int64
+ mode os.FileMode
+ modTime time.Time
}
-func (n *memFSNode) ModTime() time.Time {
- n.mu.Lock()
- defer n.mu.Unlock()
- return n.modTime
-}
-
-func (n *memFSNode) IsDir() bool {
- return n.Mode().IsDir()
-}
-
-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) Stat() (os.FileInfo, error) {
- return f.n, nil
+ return f.n.stat(f.nameSnapshot), nil
}
func (f *memFile) Write(p []byte) (int, error) {
diff --git a/webdav/file_test.go b/webdav/file_test.go
index a9bb3b7..6601fce 100644
--- a/webdav/file_test.go
+++ b/webdav/file_test.go
@@ -11,6 +11,8 @@
"os"
"path"
"path/filepath"
+ "reflect"
+ "sort"
"strconv"
"strings"
"testing"
@@ -202,10 +204,10 @@
}
}
- 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 @@
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 @@
}
}
+// 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 @@
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 @@
" 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 @@
" 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 @@
" 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 @@
" 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 @@
" 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 @@
"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 @@
" 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 @@
}
}
+ 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 @@
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 == "" {
