Repository: spf13/afero
Pull request #372 (2022-07-19 09:06:59)

Fix sorting in IOFS.ReadDir
We recently added a check for fs.ReadDirFile in IOFS.ReadDir, but forgot to apply a sort to the
result as defined in the spec.

This fixes that and adds a test case for it.



Patch:
diff --git a/iofs.go b/iofs.go
index 8bc9735..d422bcc 100644
--- a/iofs.go
+++ b/iofs.go
@@ -76,7 +76,12 @@ func (iofs IOFS) ReadDir(name string) ([]fs.DirEntry, error) {
 	defer f.Close()
 
 	if rdf, ok := f.(fs.ReadDirFile); ok {
-		return rdf.ReadDir(-1)
+		items, err := rdf.ReadDir(-1)
+		if err != nil {
+			return nil, iofs.wrapError("readdir", name, err)
+		}
+		sort.Slice(items, func(i, j int) bool { return items[i].Name() < items[j].Name() })
+		return items, nil
 	}
 
 	items, err := f.Readdir(-1)
diff --git a/ioutil.go b/ioutil.go
index a403133..386c9cd 100644
--- a/ioutil.go
+++ b/ioutil.go
@@ -141,7 +141,7 @@ func WriteFile(fs Fs, filename string, data []byte, perm os.FileMode) error {
 // We generate random temporary file names so that there's a good
 // chance the file doesn't exist yet - keeps the number of tries in
 // TempFile to a minimum.
-var rand uint32
+var randNum uint32
 var randmu sync.Mutex
 
 func reseed() uint32 {
@@ -150,12 +150,12 @@ func reseed() uint32 {
 
 func nextRandom() string {
 	randmu.Lock()
-	r := rand
+	r := randNum
 	if r == 0 {
 		r = reseed()
 	}
 	r = r*1664525 + 1013904223 // constants from Numerical Recipes
-	rand = r
+	randNum = r
 	randmu.Unlock()
 	return strconv.Itoa(int(1e9 + r%1e9))[1:]
 }
@@ -194,7 +194,7 @@ func TempFile(fs Fs, dir, pattern string) (f File, err error) {
 		if os.IsExist(err) {
 			if nconflict++; nconflict > 10 {
 				randmu.Lock()
-				rand = reseed()
+				randNum = reseed()
 				randmu.Unlock()
 			}
 			continue
@@ -226,7 +226,7 @@ func TempDir(fs Fs, dir, prefix string) (name string, err error) {
 		if os.IsExist(err) {
 			if nconflict++; nconflict > 10 {
 				randmu.Lock()
-				rand = reseed()
+				randNum = reseed()
 				randmu.Unlock()
 			}
 			continue


Test patch:
diff --git a/iofs_test.go b/iofs_test.go
index 32f0d9d..66c0c5e 100644
--- a/iofs_test.go
+++ b/iofs_test.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"math/rand"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -76,7 +77,17 @@ func TestIOFSNativeDirEntryWhenPossible(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	for i := 1; i <= 2; i++ {
+	const numFiles = 10
+
+	var fileNumbers []int
+	for i := 0; i < numFiles; i++ {
+		fileNumbers = append(fileNumbers, i)
+	}
+	rand.Shuffle(len(fileNumbers), func(i, j int) {
+		fileNumbers[i], fileNumbers[j] = fileNumbers[j], fileNumbers[i]
+	})
+
+	for _, i := range fileNumbers {
 		f, err := osfs.Create(fmt.Sprintf("dir1/dir2/test%d.txt", i))
 		if err != nil {
 			t.Fatal(err)
@@ -89,14 +100,17 @@ func TestIOFSNativeDirEntryWhenPossible(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	assertDirEntries := func(entries []fs.DirEntry) {
-		if len(entries) != 2 {
-			t.Fatalf("expected 2, got %d", len(entries))
+	assertDirEntries := func(entries []fs.DirEntry, ordered bool) {
+		if len(entries) != numFiles {
+			t.Fatalf("expected %d, got %d", numFiles, len(entries))
 		}
-		for _, entry := range entries {
+		for i, entry := range entries {
 			if _, ok := entry.(dirEntry); ok {
 				t.Fatal("DirEntry not native")
 			}
+			if ordered && entry.Name() != fmt.Sprintf("test%d.txt", i) {
+				t.Fatalf("expected %s, got %s", fmt.Sprintf("test%d.txt", i), entry.Name())
+			}
 		}
 	}
 
@@ -104,10 +118,16 @@ func TestIOFSNativeDirEntryWhenPossible(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	assertDirEntries(dirEntries)
+	assertDirEntries(dirEntries, false)
 
 	iofs := NewIOFS(osfs)
 
+	dirEntries, err = iofs.ReadDir("dir1/dir2")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertDirEntries(dirEntries, true)
+
 	fileCount := 0
 	err = fs.WalkDir(iofs, "", func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
@@ -130,8 +150,8 @@ func TestIOFSNativeDirEntryWhenPossible(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if fileCount != 2 {
-		t.Fatalf("expected 2, got %d", fileCount)
+	if fileCount != numFiles {
+		t.Fatalf("expected %d, got %d", numFiles, fileCount)
 	}
 
 }