Merge pull request #16 from mbertschler/master

fix Readdir() and Readdirnames()

README.md

@@ -100,8 +100,10 @@ the ability to drop in other filesystems as desired.
 Then throughout your functions and methods use the methods familiar
 already from the OS package.
 
-Methods Available:
+File System Methods Available:
 
+       Chmod(name string, mode os.FileMode) : error
+       Chtimes(name string, atime time.Time, mtime time.Time) : error
        Create(name string) : File, error
        Mkdir(name string, perm os.FileMode) : error
        MkdirAll(path string, perm os.FileMode) : error
@@ -113,6 +115,22 @@ Methods Available:
        Rename(oldname, newname string) : error
        Stat(name string) : os.FileInfo, error
 
+File Interfaces and Methods Available:
+
+       io.Closer
+       io.Reader
+       io.ReaderAt
+       io.Seeker
+       io.Writer
+       io.WriterAt
+
+       Stat() : os.FileInfo, error
+       Readdir(count int) : []os.FileInfo, error
+       Readdirnames(n int) : []string, error
+       WriteString(s string) : ret int, err error
+       Truncate(size int64) : error
+       Name() : string
+
 In our case we would call `AppFs.Open()` as an example because that is how we’ve defined to
 access our filesystem.
 

fs_test.go

@@ -16,10 +16,12 @@ package afero
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"syscall"
@@ -34,12 +36,11 @@ var dot = []string{
 	"memmap.go",
 }
 
-var testDir = "/tmp/fun"
+var testDir = "/tmp/afero"
+var testSubDir = "/tmp/afero/we/have/to/go/deeper"
 var testName = "test.txt"
 var Fss = []Fs{&MemMapFs{}, &OsFs{}}
 
-//var Fss = []Fs{OsFs{}}
-
 //Read with length 0 should not return EOF.
 func TestRead0(t *testing.T) {
 	for _, fs := range Fss {
@@ -137,6 +138,22 @@ func TestRemove(t *testing.T) {
 		if !os.IsNotExist(err) {
 			t.Errorf("%v: Remove() didn't raise error for non-existent file", fs.Name())
 		}
+
+		f, err := fs.Open(testDir)
+		if err != nil {
+			t.Error("TestDir should still exist:", err)
+		}
+
+		names, err := f.Readdirnames(-1)
+		if err != nil {
+			t.Error("Readdirnames failed:", err)
+		}
+
+		for _, e := range names {
+			if e == testName {
+				t.Error("File was not removed from parent directory")
+			}
+		}
 	}
 }
 
@@ -249,19 +266,227 @@ func TestWriteAt(t *testing.T) {
 	}
 }
 
-//func TestReaddirnames(t *testing.T) {
-//for _, fs := range Fss {
-//testReaddirnames(fs, ".", dot, t)
-////testReaddirnames(sysdir.name, fs, sysdir.files, t)
-//}
-//}
+func setupTestDir(t *testing.T) {
+	for _, fs := range Fss {
+		err := fs.RemoveAll(testDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = fs.MkdirAll(testSubDir, 0700)
+		if err != nil && !os.IsExist(err) {
+			t.Fatal(err)
+		}
+		f, err := fs.Create(filepath.Join(testSubDir, "testfile1"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		f.WriteString("Testfile 1 content")
+		f.Close()
 
-//func TestReaddir(t *testing.T) {
-//for _, fs := range Fss {
-//testReaddir(fs, ".", dot, t)
-////testReaddir(sysdir.name, fs, sysdir.files, t)
-//}
-//}
+		f, err = fs.Create(filepath.Join(testSubDir, "testfile2"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		f.WriteString("Testfile 2 content")
+		f.Close()
+
+		f, err = fs.Create(filepath.Join(testSubDir, "testfile3"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		f.WriteString("Testfile 3 content")
+		f.Close()
+
+		f, err = fs.Create(filepath.Join(testSubDir, "testfile4"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		f.WriteString("Testfile 4 content")
+		f.Close()
+	}
+}
+
+func TestReaddirnames(t *testing.T) {
+	setupTestDir(t)
+	for _, fs := range Fss {
+		root, err := fs.Open(testDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		namesRoot, err := root.Readdirnames(-1)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		sub, err := fs.Open(testSubDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		namesSub, err := sub.Readdirnames(-1)
+		if err != nil {
+			t.Fatal(err)
+		}
+		findNames(t, namesRoot, namesSub, fs)
+	}
+}
+
+func TestReaddirSimple(t *testing.T) {
+	for _, fs := range Fss {
+		root, err := fs.Open(testDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		rootInfo, err := root.Readdir(1)
+		if err != nil {
+			t.Log(myFileInfo(rootInfo))
+			t.Error(err)
+		}
+		rootInfo, err = root.Readdir(5)
+		if err != io.EOF {
+			t.Log(myFileInfo(rootInfo))
+			t.Error(err)
+		}
+
+		sub, err := fs.Open(testSubDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		subInfo, err := sub.Readdir(5)
+		if err != nil {
+			t.Log(myFileInfo(subInfo))
+			t.Error(err)
+		}
+	}
+}
+
+func TestReaddir(t *testing.T) {
+	for num := 0; num < 6; num++ {
+		outputs := make([]string, len(Fss))
+		infos := make([]string, len(Fss))
+		for i, fs := range Fss {
+			root, err := fs.Open(testSubDir)
+			if err != nil {
+				t.Fatal(err)
+			}
+			for j := 0; j < 6; j++ {
+				info, err := root.Readdir(num)
+				outputs[i] += fmt.Sprintf("%v  Error: %v\n", myFileInfo(info), err)
+				infos[i] += fmt.Sprintln(len(info), err)
+			}
+		}
+
+		fail := false
+		for i, o := range infos {
+			if i == 0 {
+				continue
+			}
+			if o != infos[i-1] {
+				fail = true
+				break
+			}
+		}
+		if fail {
+			t.Log("Readdir outputs not equal for Readdir(", num, ")")
+			for i, o := range outputs {
+				t.Log(Fss[i].Name())
+				t.Log(o)
+			}
+			t.Fail()
+		}
+	}
+}
+
+type myFileInfo []os.FileInfo
+
+func (m myFileInfo) String() string {
+	out := "Fileinfos:\n"
+	for _, e := range m {
+		out += "  " + e.Name() + "\n"
+	}
+	return out
+}
+
+func TestReaddirAll(t *testing.T) {
+	defer removeTestDir(t)
+	for _, fs := range Fss {
+		root, err := fs.Open(testDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		rootInfo, err := root.Readdir(-1)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var namesRoot = []string{}
+		for _, e := range rootInfo {
+			namesRoot = append(namesRoot, e.Name())
+		}
+
+		sub, err := fs.Open(testSubDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		subInfo, err := sub.Readdir(-1)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var namesSub = []string{}
+		for _, e := range subInfo {
+			namesSub = append(namesSub, e.Name())
+		}
+
+		findNames(t, namesRoot, namesSub, fs)
+	}
+}
+
+func findNames(t *testing.T, root, sub []string, fs Fs) {
+	t.Logf("Names root: %v", root)
+	t.Logf("Names sub: %v", sub)
+
+	var foundRoot bool
+	for _, e := range root {
+		_, err := fs.Open(path.Join(testDir, e))
+		if err != nil {
+			t.Error("Open", e, ":", err)
+		}
+		if equal(e, "we") {
+			foundRoot = true
+		}
+	}
+	if !foundRoot {
+		t.Error("Didn't find subdirectory we")
+	}
+
+	var found1, found2 bool
+	for _, e := range sub {
+		_, err := fs.Open(path.Join(testSubDir, e))
+		if err != nil {
+			t.Error("Open", e, ":", err)
+		}
+		if equal(e, "testfile1") {
+			found1 = true
+		}
+		if equal(e, "testfile2") {
+			found2 = true
+		}
+	}
+
+	if !found1 {
+		t.Error("Didn't find testfile1")
+	}
+	if !found2 {
+		t.Error("Didn't find testfile2")
+	}
+}
+
+func removeTestDir(t *testing.T) {
+	for _, fs := range Fss {
+		err := fs.RemoveAll(testDir)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+}
 
 func newFile(testName string, fs Fs, t *testing.T) (f File) {
 	// Use a local file system, not NFS.
@@ -296,61 +521,6 @@ func writeFile(t *testing.T, fs Fs, fname string, flag int, text string) string
 	return string(data)
 }
 
-func testReaddirnames(fs Fs, dir string, contents []string, t *testing.T) {
-	file, err := fs.Open(dir)
-	if err != nil {
-		t.Fatalf("open %q failed: %v", dir, err)
-	}
-	defer file.Close()
-	s, err2 := file.Readdirnames(-1)
-	if err2 != nil {
-		t.Fatalf("readdirnames %q failed: %v", dir, err2)
-	}
-	for _, m := range contents {
-		found := false
-		for _, n := range s {
-			if n == "." || n == ".." {
-				t.Errorf("got %s in directory", n)
-			}
-			if equal(m, n) {
-				if found {
-					t.Error("present twice:", m)
-				}
-				found = true
-			}
-		}
-		if !found {
-			t.Error("could not find", m)
-		}
-	}
-}
-
-func testReaddir(fs Fs, dir string, contents []string, t *testing.T) {
-	file, err := fs.Open(dir)
-	if err != nil {
-		t.Fatalf("open %q failed: %v", dir, err)
-	}
-	defer file.Close()
-	s, err2 := file.Readdir(-1)
-	if err2 != nil {
-		t.Fatalf("readdir %q failed: %v", dir, err2)
-	}
-	for _, m := range contents {
-		found := false
-		for _, n := range s {
-			if equal(m, n.Name()) {
-				if found {
-					t.Error("present twice:", m)
-				}
-				found = true
-			}
-		}
-		if !found {
-			t.Error("could not find", m)
-		}
-	}
-}
-
 func equal(name1, name2 string) (r bool) {
 	switch runtime.GOOS {
 	case "windows":

memfile.go

@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"io"
 	"os"
+	"path"
 	"sync"
 	"sync/atomic"
 )
@@ -34,14 +35,15 @@ type MemDir interface {
 
 type InMemoryFile struct {
 	sync.Mutex
-	at      int64
-	name    string
-	data    []byte
-	memDir  MemDir
-	dir     bool
-	closed  bool
-	mode    os.FileMode
-	modtime time.Time
+	at           int64
+	name         string
+	data         []byte
+	memDir       MemDir
+	dir          bool
+	closed       bool
+	mode         os.FileMode
+	modtime      time.Time
+	readDirCount int64
 }
 
 func MemFileCreate(name string) *InMemoryFile {
@@ -50,6 +52,7 @@ func MemFileCreate(name string) *InMemoryFile {
 
 func (f *InMemoryFile) Open() error {
 	atomic.StoreInt64(&f.at, 0)
+	atomic.StoreInt64(&f.readDirCount, 0)
 	f.Lock()
 	f.closed = false
 	f.Unlock()
@@ -57,7 +60,6 @@ func (f *InMemoryFile) Open() error {
 }
 
 func (f *InMemoryFile) Close() error {
-	atomic.StoreInt64(&f.at, 0)
 	f.Lock()
 	f.closed = true
 	f.Unlock()
@@ -73,36 +75,38 @@ func (f *InMemoryFile) Stat() (os.FileInfo, error) {
 }
 
 func (f *InMemoryFile) Readdir(count int) (res []os.FileInfo, err error) {
-	files := f.memDir.Files()
-	limit := len(files)
-
-	if len(files) == 0 {
-		return
-	}
+	var outLength int64
 
+	f.Lock()
+	files := f.memDir.Files()[f.readDirCount:]
 	if count > 0 {
-		limit = count
+		if len(files) < count {
+			outLength = int64(len(files))
+		} else {
+			outLength = int64(count)
+		}
+		if len(files) == 0 {
+			err = io.EOF
+		}
+	} else {
+		outLength = int64(len(files))
+	}
+	f.readDirCount += outLength
+	f.Unlock()
+
+	res = make([]os.FileInfo, outLength)
+	for i := range res {
+		res[i], _ = files[i].Stat()
 	}
 
-	if len(files) < limit {
-		err = io.EOF
-	}
-
-	res = make([]os.FileInfo, f.memDir.Len())
-
-	i := 0
-	for _, file := range f.memDir.Files() {
-		res[i], _ = file.Stat()
-		i++
-	}
-	return res, nil
+	return res, err
 }
 
 func (f *InMemoryFile) Readdirnames(n int) (names []string, err error) {
 	fi, err := f.Readdir(n)
 	names = make([]string, len(fi))
 	for i, f := range fi {
-		names[i] = f.Name()
+		_, names[i] = path.Split(f.Name())
 	}
 	return names, err
 }
@@ -202,7 +206,10 @@ type InMemoryFileInfo struct {
 }
 
 // Implements os.FileInfo
-func (s *InMemoryFileInfo) Name() string       { return s.file.Name() }
+func (s *InMemoryFileInfo) Name() string {
+	_, name := path.Split(s.file.Name())
+	return name
+}
 func (s *InMemoryFileInfo) Mode() os.FileMode  { return s.file.mode }
 func (s *InMemoryFileInfo) ModTime() time.Time { return s.file.modtime }
 func (s *InMemoryFileInfo) IsDir() bool        { return s.file.dir }

memmap.go

@@ -16,9 +16,11 @@ package afero
 import (
 	"errors"
 	"fmt"
+	"log"
 	"os"
 	"path"
 	"path/filepath"
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -64,9 +66,17 @@ func (m MemDirMap) Files() (files []File) {
 	for _, f := range m {
 		files = append(files, f)
 	}
+	sort.Sort(filesSorter(files))
 	return files
 }
 
+type filesSorter []File
+
+// implement sort.Interface for []File
+func (s filesSorter) Len() int           { return len(s) }
+func (s filesSorter) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s filesSorter) Less(i, j int) bool { return s[i].Name() < s[j].Name() }
+
 func (m MemDirMap) Names() (names []string) {
 	for x := range m {
 		names = append(names, x)
@@ -80,56 +90,69 @@ func (m *MemMapFs) Create(name string) (File, error) {
 	m.lock()
 	file := MemFileCreate(name)
 	m.getData()[name] = file
+	m.registerWithParent(file)
 	m.unlock()
-	m.registerDirs(file)
 	return file, nil
 }
 
-func (m *MemMapFs) registerDirs(f File) {
-	var x = f.Name()
-	for x != "/" {
-		f := m.registerWithParent(f)
-		if f == nil {
-			break
+func (m *MemMapFs) unRegisterWithParent(fileName string) {
+	f, err := m.lockfreeOpen(fileName)
+	if err != nil {
+		if os.IsNotExist(err) {
+			log.Println("Open err:", err)
 		}
-		x = f.Name()
+		return
 	}
-}
-
-func (m *MemMapFs) unRegisterWithParent(f File) File {
 	parent := m.findParent(f)
+	if parent == nil {
+		log.Fatal("parent of ", f.Name(), " is nil")
+	}
 	pmem := parent.(*InMemoryFile)
 	pmem.memDir.Remove(f)
-	return parent
 }
 
 func (m *MemMapFs) findParent(f File) File {
-	dirs, _ := path.Split(f.Name())
-	if len(dirs) > 1 {
-		_, parent := path.Split(path.Clean(dirs))
-		if len(parent) > 0 {
-			pfile, err := m.Open(parent)
-			if err != nil {
-				return pfile
-			}
-		}
-	}
-	return nil
-}
-
-func (m *MemMapFs) registerWithParent(f File) File {
-	if f == nil {
+	pdir, _ := path.Split(f.Name())
+	pdir = path.Clean(pdir)
+	pfile, err := m.lockfreeOpen(pdir)
+	if err != nil {
 		return nil
 	}
-	parent := m.findParent(f)
-	if parent != nil {
-		pmem := parent.(*InMemoryFile)
-		pmem.memDir.Add(f)
-	} else {
-		pdir := filepath.Dir(path.Clean(f.Name()))
-		m.Mkdir(pdir, 0777)
+	return pfile
+}
+
+func (m *MemMapFs) registerWithParent(f File) {
+	if f == nil {
+		return
 	}
-	return parent
+	parent := m.findParent(f)
+	if parent == nil {
+		pdir := filepath.Dir(path.Clean(f.Name()))
+		err := m.lockfreeMkdir(pdir, 0777)
+		if err != nil {
+			log.Println("Mkdir error:", err)
+			return
+		}
+		parent, err = m.lockfreeOpen(pdir)
+		if err != nil {
+			log.Println("Open after Mkdir error:", err)
+			return
+		}
+	}
+	pmem := parent.(*InMemoryFile)
+	pmem.memDir.Add(f)
+}
+
+func (m *MemMapFs) lockfreeMkdir(name string, perm os.FileMode) error {
+	_, ok := m.getData()[name]
+	if ok {
+		return ErrFileExists
+	} else {
+		item := &InMemoryFile{name: name, memDir: &MemDirMap{}, dir: true}
+		m.getData()[name] = item
+		m.registerWithParent(item)
+	}
+	return nil
 }
 
 func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
@@ -142,8 +165,8 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
 		m.lock()
 		item := &InMemoryFile{name: name, memDir: &MemDirMap{}, dir: true}
 		m.getData()[name] = item
+		m.registerWithParent(item)
 		m.unlock()
-		m.registerDirs(item)
 	}
 	return nil
 }
@@ -168,6 +191,19 @@ func (m *MemMapFs) Open(name string) (File, error) {
 	}
 }
 
+func (m *MemMapFs) lockfreeOpen(name string) (File, error) {
+	f, ok := m.getData()[name]
+	ff, ok := f.(*InMemoryFile)
+	if ok {
+		ff.Open()
+	}
+	if ok {
+		return f, nil
+	} else {
+		return nil, ErrFileNotFound
+	}
+}
+
 func (m *MemMapFs) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
 	return m.Open(name)
 }
@@ -177,6 +213,7 @@ func (m *MemMapFs) Remove(name string) error {
 	defer m.unlock()
 
 	if _, ok := m.getData()[name]; ok {
+		m.unRegisterWithParent(name)
 		delete(m.getData(), name)
 	} else {
 		return &os.PathError{"remove", name, os.ErrNotExist}
@@ -185,8 +222,13 @@ func (m *MemMapFs) Remove(name string) error {
 }
 
 func (m *MemMapFs) RemoveAll(path string) error {
+	m.lock()
+	m.unRegisterWithParent(path)
+	m.unlock()
+
 	m.rlock()
 	defer m.runlock()
+
 	for p, _ := range m.getData() {
 		if strings.HasPrefix(p, path) {
 			m.runlock()