Remove delayIsLeaf requirement simplify ListObjects further (#7593)

cmd/disk-cache.go

@@ -375,7 +375,7 @@ func (c cacheObjects) GetObjectInfo(ctx context.Context, bucket, object string,
 // Returns function "listDir" of the type listDirFunc.
 // isLeaf - is used by listDir function to check if an entry is a leaf or non-leaf entry.
 // disks - list of fsObjects
-func listDirCacheFactory(isLeaf IsLeafFunc, disks []*cacheFSObjects) ListDirFunc {
+func listDirCacheFactory(isLeaf func(string, string) bool, disks []*cacheFSObjects) ListDirFunc {
 	listCacheDirs := func(bucket, prefixDir, prefixEntry string) (dirs []string) {
 		var entries []string
 		for _, disk := range disks {
@@ -391,6 +391,12 @@ func listDirCacheFactory(isLeaf IsLeafFunc, disks []*cacheFSObjects) ListDirFunc
 				continue
 			}
 
+			for i := range entries {
+				if isLeaf(bucket, entries[i]) {
+					entries[i] = strings.TrimSuffix(entries[i], slashSeparator)
+				}
+			}
+
 			// Filter entries that have the prefix prefixEntry.
 			entries = filterMatchingPrefix(entries, prefixEntry)
 			dirs = append(dirs, entries...)
@@ -399,7 +405,7 @@ func listDirCacheFactory(isLeaf IsLeafFunc, disks []*cacheFSObjects) ListDirFunc
 	}
 
 	// listDir - lists all the entries at a given prefix and given entry in the prefix.
-	listDir := func(bucket, prefixDir, prefixEntry string) (mergedEntries []string, delayIsLeaf bool) {
+	listDir := func(bucket, prefixDir, prefixEntry string) (mergedEntries []string) {
 		cacheEntries := listCacheDirs(bucket, prefixDir, prefixEntry)
 		for _, entry := range cacheEntries {
 			// Find elements in entries which are not in mergedEntries
@@ -411,7 +417,7 @@ func listDirCacheFactory(isLeaf IsLeafFunc, disks []*cacheFSObjects) ListDirFunc
 			mergedEntries = append(mergedEntries, entry)
 			sort.Strings(mergedEntries)
 		}
-		return mergedEntries, false
+		return mergedEntries
 	}
 	return listDir
 }
@@ -430,25 +436,16 @@ func (c cacheObjects) listCacheObjects(ctx context.Context, bucket, prefix, mark
 	walkResultCh, endWalkCh := c.listPool.Release(listParams{bucket, recursive, marker, prefix})
 	if walkResultCh == nil {
 		endWalkCh = make(chan struct{})
-		isLeaf := func(bucket, object string) bool {
+
+		listDir := listDirCacheFactory(func(bucket, object string) bool {
 			fs, err := c.cache.getCacheFS(ctx, bucket, object)
 			if err != nil {
 				return false
 			}
 			_, err = fs.getObjectInfo(ctx, bucket, object)
 			return err == nil
-		}
+		}, c.cache.cfs)
-
+		walkResultCh = startTreeWalk(ctx, bucket, prefix, marker, recursive, listDir, endWalkCh)
-		isLeafDir := func(bucket, object string) bool {
-			fs, err := c.cache.getCacheFS(ctx, bucket, object)
-			if err != nil {
-				return false
-			}
-			return fs.isObjectDir(bucket, object)
-		}
-
-		listDir := listDirCacheFactory(isLeaf, c.cache.cfs)
-		walkResultCh = startTreeWalk(ctx, bucket, prefix, marker, recursive, listDir, isLeaf, isLeafDir, endWalkCh)
 	}
 
 	for i := 0; i < maxKeys; {

cmd/fs-v1.go

@@ -1001,9 +1001,9 @@ func (fs *FSObjects) DeleteObject(ctx context.Context, bucket, object string) er
 // Returns function "listDir" of the type listDirFunc.
 // isLeaf - is used by listDir function to check if an entry
 // is a leaf or non-leaf entry.
-func (fs *FSObjects) listDirFactory(isLeaf IsLeafFunc) ListDirFunc {
+func (fs *FSObjects) listDirFactory() ListDirFunc {
 	// listDir - lists all the entries at a given prefix and given entry in the prefix.
-	listDir := func(bucket, prefixDir, prefixEntry string) (entries []string, delayIsLeaf bool) {
+	listDir := func(bucket, prefixDir, prefixEntry string) (entries []string) {
 		var err error
 		entries, err = readDir(pathJoin(fs.fsPath, bucket, prefixDir))
 		if err != nil && err != errFileNotFound {
@@ -1011,7 +1011,7 @@ func (fs *FSObjects) listDirFactory(isLeaf IsLeafFunc) ListDirFunc {
 			return
 		}
 		sort.Strings(entries)
-		return filterListEntries(bucket, prefixDir, entries, prefixEntry, isLeaf)
+		return filterMatchingPrefix(entries, prefixEntry)
 	}
 
 	// Return list factory instance.
@@ -1097,22 +1097,8 @@ func (fs *FSObjects) getObjectETag(ctx context.Context, bucket, entry string, lo
 // ListObjects - list all objects at prefix upto maxKeys., optionally delimited by '/'. Maintains the list pool
 // state for future re-entrant list requests.
 func (fs *FSObjects) ListObjects(ctx context.Context, bucket, prefix, marker, delimiter string, maxKeys int) (loi ListObjectsInfo, e error) {
-	isLeaf := func(bucket, object string) bool {
+	return listObjects(ctx, fs, bucket, prefix, marker, delimiter, maxKeys, fs.listPool,
-		// bucket argument is unused as we don't need to StatFile
+		fs.listDirFactory(), fs.getObjectInfo, fs.getObjectInfo)
-		// to figure if it's a file, just need to check that the
-		// object string does not end with "/".
-		return !hasSuffix(object, slashSeparator)
-	}
-	// Return true if the specified object is an empty directory
-	isLeafDir := func(bucket, object string) bool {
-		if !hasSuffix(object, slashSeparator) {
-			return false
-		}
-		return fs.isObjectDir(bucket, object)
-	}
-	listDir := fs.listDirFactory(isLeaf)
-
-	return listObjects(ctx, fs, bucket, prefix, marker, delimiter, maxKeys, fs.listPool, isLeaf, isLeafDir, listDir, fs.getObjectInfo, fs.getObjectInfo)
 }
 
 // ReloadFormat - no-op for fs, Valid only for XL.

cmd/gateway-common.go

@@ -44,8 +44,8 @@ var (
 	// ListObjects function alias.
 	ListObjects = listObjects
 
-	// FilterListEntries function alias.
+	// FilterMatchingPrefix function alias.
-	FilterListEntries = filterListEntries
+	FilterMatchingPrefix = filterMatchingPrefix
 
 	// IsStringEqual is string equal.
 	IsStringEqual = isStringEqual

cmd/gateway/hdfs/gateway-hdfs.go

@@ -287,24 +287,9 @@ func (n *hdfsObjects) ListBuckets(ctx context.Context) (buckets []minio.BucketIn
 	return buckets, nil
 }
 
-func (n *hdfsObjects) isObjectDir(bucket, object string) bool {
+func (n *hdfsObjects) listDirFactory() minio.ListDirFunc {
-	f, err := n.clnt.Open(minio.PathJoin(hdfsSeparator, bucket, object))
-	if err != nil {
-		return false
-	}
-	defer f.Close()
-
-	entries, err := f.Readdir(1)
-	if err != nil {
-		return false
-	}
-
-	return len(entries) == 0
-}
-
-func (n *hdfsObjects) listDirFactory(isLeaf minio.IsLeafFunc) minio.ListDirFunc {
 	// listDir - lists all the entries at a given prefix and given entry in the prefix.
-	listDir := func(bucket, prefixDir, prefixEntry string) (entries []string, delayIsLeaf bool) {
+	listDir := func(bucket, prefixDir, prefixEntry string) (entries []string) {
 		f, err := n.clnt.Open(minio.PathJoin(hdfsSeparator, bucket, prefixDir))
 		if err != nil {
 			if os.IsNotExist(err) {
@@ -327,8 +312,7 @@ func (n *hdfsObjects) listDirFactory(isLeaf minio.IsLeafFunc) minio.ListDirFunc
 			}
 		}
 		fis = nil
-		entries, delayIsLeaf = minio.FilterListEntries(bucket, prefixDir, entries, prefixEntry, isLeaf)
+		return minio.FilterMatchingPrefix(entries, prefixEntry)
-		return entries, delayIsLeaf
 	}
 
 	// Return list factory instance.
@@ -337,27 +321,11 @@ func (n *hdfsObjects) listDirFactory(isLeaf minio.IsLeafFunc) minio.ListDirFunc
 
 // ListObjects lists all blobs in HDFS bucket filtered by prefix.
 func (n *hdfsObjects) ListObjects(ctx context.Context, bucket, prefix, marker, delimiter string, maxKeys int) (loi minio.ListObjectsInfo, err error) {
-	isLeaf := func(bucket, object string) bool {
-		// bucket argument is unused as we don't need to StatFile
-		// to figure if it's a file, just need to check that the
-		// object string does not end with "/".
-		return !strings.HasSuffix(object, hdfsSeparator)
-	}
-
-	// Return true if the specified object is an empty directory
-	isLeafDir := func(bucket, object string) bool {
-		if !strings.HasSuffix(object, hdfsSeparator) {
-			return false
-		}
-		return n.isObjectDir(bucket, object)
-	}
-	listDir := n.listDirFactory(isLeaf)
-
 	getObjectInfo := func(ctx context.Context, bucket, entry string) (minio.ObjectInfo, error) {
 		return n.GetObjectInfo(ctx, bucket, entry, minio.ObjectOptions{})
 	}
 
-	return minio.ListObjects(ctx, n, bucket, prefix, marker, delimiter, maxKeys, n.listPool, isLeaf, isLeafDir, listDir, getObjectInfo, getObjectInfo)
+	return minio.ListObjects(ctx, n, bucket, prefix, marker, delimiter, maxKeys, n.listPool, n.listDirFactory(), getObjectInfo, getObjectInfo)
 }
 
 // Check if the given error corresponds to ENOTEMPTY for unix

cmd/object-api-common.go

@@ -162,7 +162,7 @@ func removeListenerConfig(ctx context.Context, objAPI ObjectLayer, bucket string
 	return objAPI.DeleteObject(ctx, minioMetaBucket, lcPath)
 }
 
-func listObjects(ctx context.Context, obj ObjectLayer, bucket, prefix, marker, delimiter string, maxKeys int, tpool *TreeWalkPool, isLeaf IsLeafFunc, isLeafDir IsLeafDirFunc, listDir ListDirFunc, getObjInfo func(context.Context, string, string) (ObjectInfo, error), getObjectInfoDirs ...func(context.Context, string, string) (ObjectInfo, error)) (loi ListObjectsInfo, err error) {
+func listObjects(ctx context.Context, obj ObjectLayer, bucket, prefix, marker, delimiter string, maxKeys int, tpool *TreeWalkPool, listDir ListDirFunc, getObjInfo func(context.Context, string, string) (ObjectInfo, error), getObjectInfoDirs ...func(context.Context, string, string) (ObjectInfo, error)) (loi ListObjectsInfo, err error) {
 	if err := checkListObjsArgs(ctx, bucket, prefix, marker, delimiter, obj); err != nil {
 		return loi, err
 	}
@@ -203,7 +203,7 @@ func listObjects(ctx context.Context, obj ObjectLayer, bucket, prefix, marker, d
 	walkResultCh, endWalkCh := tpool.Release(listParams{bucket, recursive, marker, prefix})
 	if walkResultCh == nil {
 		endWalkCh = make(chan struct{})
-		walkResultCh = startTreeWalk(ctx, bucket, prefix, marker, recursive, listDir, isLeaf, isLeafDir, endWalkCh)
+		walkResultCh = startTreeWalk(ctx, bucket, prefix, marker, recursive, listDir, endWalkCh)
 	}
 
 	var objInfos []ObjectInfo

cmd/object-api-listobjects_test.go

@@ -35,8 +35,8 @@ func TestListObjects(t *testing.T) {
 }
 
 // Unit test for ListObjects in general.
-func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
+func testListObjects(obj ObjectLayer, instanceType string, t1 TestErrHandler) {
-
+	t, _ := t1.(*testing.T)
 	testBuckets := []string{
 		// This bucket is used for testing ListObject operations.
 		"test-bucket-list-object",
@@ -66,7 +66,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 		{"obj0", "obj0", nil},
 		{"obj1", "obj1", nil},
 		{"obj2", "obj2", nil},
-		{"z-empty-dir/", "", nil},
 	}
 	for _, object := range testObjects {
 		md5Bytes := md5.Sum([]byte(object.content))
@@ -96,7 +95,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-1.
@@ -193,7 +191,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-10.
@@ -205,7 +202,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-11.
@@ -215,7 +211,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 			Objects: []ObjectInfo{
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-12.
@@ -224,7 +219,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 			IsTruncated: false,
 			Objects: []ObjectInfo{
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-13.
@@ -238,7 +232,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-14.
@@ -255,7 +248,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-15.
@@ -270,7 +262,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-16.
@@ -284,7 +275,6 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 				{Name: "obj0"},
 				{Name: "obj1"},
 				{Name: "obj2"},
-				{Name: "z-empty-dir/"},
 			},
 		},
 		// ListObjectsResult-17.
@@ -535,61 +525,66 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
 	}
 
 	for i, testCase := range testCases {
-		result, err := obj.ListObjects(context.Background(), testCase.bucketName, testCase.prefix, testCase.marker, testCase.delimeter, int(testCase.maxKeys))
+		testCase := testCase
-		if err != nil && testCase.shouldPass {
+		t.Run(fmt.Sprintf("Test%d-%s", i+1, instanceType), func(t *testing.T) {
-			t.Errorf("Test %d: %s:  Expected to pass, but failed with: <ERROR> %s", i+1, instanceType, err.Error())
+			result, err := obj.ListObjects(context.Background(), testCase.bucketName,
-		}
+				testCase.prefix, testCase.marker, testCase.delimeter, int(testCase.maxKeys))
-		if err == nil && !testCase.shouldPass {
+			if err != nil && testCase.shouldPass {
-			t.Errorf("Test %d: %s: Expected to fail with <ERROR> \"%s\", but passed instead", i+1, instanceType, testCase.err.Error())
+				t.Errorf("Test %d: %s:  Expected to pass, but failed with: <ERROR> %s", i+1, instanceType, err.Error())
-		}
-		// Failed as expected, but does it fail for the expected reason.
-		if err != nil && !testCase.shouldPass {
-			if !strings.Contains(err.Error(), testCase.err.Error()) {
-				t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but instead failed with error \"%s\" instead", i+1, instanceType, testCase.err.Error(), err.Error())
 			}
-		}
+			if err == nil && !testCase.shouldPass {
-		// Since there are cases for which ListObjects fails, this is
+				t.Errorf("Test %d: %s: Expected to fail with <ERROR> \"%s\", but passed instead", i+1, instanceType, testCase.err.Error())
-		// necessary. Test passes as expected, but the output values
-		// are verified for correctness here.
-		if err == nil && testCase.shouldPass {
-			// The length of the expected ListObjectsResult.Objects
-			// should match in both expected result from test cases
-			// and in the output. On failure calling t.Fatalf,
-			// otherwise it may lead to index out of range error in
-			// assertion following this.
-			if len(testCase.result.Objects) != len(result.Objects) {
-				t.Fatalf("Test %d: %s: Expected number of object in the result to be '%d', but found '%d' objects instead", i+1, instanceType, len(testCase.result.Objects), len(result.Objects))
 			}
-			for j := 0; j < len(testCase.result.Objects); j++ {
+			// Failed as expected, but does it fail for the expected reason.
-				if testCase.result.Objects[j].Name != result.Objects[j].Name {
+			if err != nil && !testCase.shouldPass {
-					t.Errorf("Test %d: %s: Expected object name to be \"%s\", but found \"%s\" instead", i+1, instanceType, testCase.result.Objects[j].Name, result.Objects[j].Name)
+				if !strings.Contains(err.Error(), testCase.err.Error()) {
+					t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but instead failed with error \"%s\" instead", i+1, instanceType, testCase.err.Error(), err.Error())
 				}
-				//FIXME: we should always check for ETag
+			}
-				if result.Objects[j].ETag == "" && !strings.HasSuffix(result.Objects[j].Name, slashSeparator) {
+			// Since there are cases for which ListObjects fails, this is
-					t.Errorf("Test %d: %s: Expected ETag to be not empty, but found empty instead (%v)", i+1, instanceType, result.Objects[j].Name)
+			// necessary. Test passes as expected, but the output values
+			// are verified for correctness here.
+			if err == nil && testCase.shouldPass {
+				// The length of the expected ListObjectsResult.Objects
+				// should match in both expected result from test cases
+				// and in the output. On failure calling t.Fatalf,
+				// otherwise it may lead to index out of range error in
+				// assertion following this.
+				if len(testCase.result.Objects) != len(result.Objects) {
+					t.Fatalf("Test %d: %s: Expected number of object in the result to be '%d', but found '%d' objects instead", i+1, instanceType, len(testCase.result.Objects), len(result.Objects))
+				}
+				for j := 0; j < len(testCase.result.Objects); j++ {
+					if testCase.result.Objects[j].Name != result.Objects[j].Name {
+						t.Errorf("Test %d: %s: Expected object name to be \"%s\", but found \"%s\" instead", i+1, instanceType, testCase.result.Objects[j].Name, result.Objects[j].Name)
+					}
+					// FIXME: we should always check for ETag
+					if result.Objects[j].ETag == "" && !strings.HasSuffix(result.Objects[j].Name, slashSeparator) {
+						t.Errorf("Test %d: %s: Expected ETag to be not empty, but found empty instead (%v)", i+1, instanceType, result.Objects[j].Name)
+					}
+
+				}
+				if testCase.result.IsTruncated != result.IsTruncated {
+					t.Errorf("Test %d: %s: Expected IsTruncated flag to be %v, but instead found it to be %v", i+1, instanceType, testCase.result.IsTruncated, result.IsTruncated)
+				}
+
+				if testCase.result.IsTruncated && result.NextMarker == "" {
+					t.Errorf("Test %d: %s: Expected NextContinuationToken to contain a string since listing is truncated, but instead found it to be empty", i+1, instanceType)
+				}
+
+				if !testCase.result.IsTruncated && result.NextMarker != "" {
+					t.Errorf("Test %d: %s: Expected NextContinuationToken to be empty since listing is not truncated, but instead found `%v`", i+1, instanceType, result.NextMarker)
 				}
 
 			}
-			if testCase.result.IsTruncated != result.IsTruncated {
+			// Take ListObject treeWalk go-routine to completion, if available in the treewalk pool.
-				t.Errorf("Test %d: %s: Expected IsTruncated flag to be %v, but instead found it to be %v", i+1, instanceType, testCase.result.IsTruncated, result.IsTruncated)
+			if result.IsTruncated {
+				_, err = obj.ListObjects(context.Background(), testCase.bucketName,
+					testCase.prefix, result.NextMarker, testCase.delimeter, 1000)
+				if err != nil {
+					t.Fatal(err)
+				}
 			}
-
+		})
-			if testCase.result.IsTruncated && result.NextMarker == "" {
-				t.Errorf("Test %d: %s: Expected NextContinuationToken to contain a string since listing is truncated, but instead found it to be empty", i+1, instanceType)
-			}
-
-			if !testCase.result.IsTruncated && result.NextMarker != "" {
-				t.Errorf("Test %d: %s: Expected NextContinuationToken to be empty since listing is not truncated, but instead found `%v`", i+1, instanceType, result.NextMarker)
-			}
-
-		}
-		// Take ListObject treeWalk go-routine to completion, if available in the treewalk pool.
-		if result.IsTruncated {
-			_, err = obj.ListObjects(context.Background(), testCase.bucketName, testCase.prefix, result.NextMarker, testCase.delimeter, 1000)
-			if err != nil {
-				t.Fatal(err)
-			}
-		}
 	}
 }
 

cmd/tree-walk.go

@@ -28,36 +28,6 @@ type TreeWalkResult struct {
 	end   bool
 }
 
-// posix.ListDir returns entries with trailing "/" for directories. At the object layer
-// we need to remove this trailing "/" for objects and retain "/" for prefixes before
-// sorting because the trailing "/" can affect the sorting results for certain cases.
-// Ex. lets say entries = ["a-b/", "a/"] and both are objects.
-//     sorting with out trailing "/" = ["a", "a-b"]
-//     sorting with trailing "/"     = ["a-b/", "a/"]
-// Hence if entries[] does not have a case like the above example then isLeaf() check
-// can be delayed till the entry is pushed into the TreeWalkResult channel.
-// delayIsLeafCheck() returns true if isLeaf can be delayed or false if
-// isLeaf should be done in listDir()
-func delayIsLeafCheck(entries []string) bool {
-	for i, entry := range entries {
-		if i == len(entries)-1 {
-			break
-		}
-		// If any byte in the "entry" string is less than '/' then the
-		// next "entry" should not contain '/' at the same same byte position.
-		for j := 0; j < len(entry); j++ {
-			if entry[j] < '/' {
-				if len(entries[i+1]) > j {
-					if entries[i+1][j] == '/' {
-						return false
-					}
-				}
-			}
-		}
-	}
-	return true
-}
-
 // Return entries that have prefix prefixEntry.
 // Note: input entries are expected to be sorted.
 func filterMatchingPrefix(entries []string, prefixEntry string) []string {
@@ -81,49 +51,15 @@ func filterMatchingPrefix(entries []string, prefixEntry string) []string {
 		}
 		end--
 	}
+	sort.Strings(entries[start:end])
 	return entries[start:end]
 }
 
 // ListDirFunc - "listDir" function of type listDirFunc returned by listDirFactory() - explained below.
-type ListDirFunc func(bucket, prefixDir, prefixEntry string) (entries []string, delayIsLeaf bool)
+type ListDirFunc func(bucket, prefixDir, prefixEntry string) (entries []string)
-
-// IsLeafFunc - A function isLeaf of type isLeafFunc is used to detect if an entry is a leaf entry. There are four scenarios
-// where isLeaf should behave differently:
-// 1. FS backend object listing - isLeaf is true if the entry has a trailing "/"
-// 2. FS backend multipart listing - isLeaf is true if the entry is a directory and contains uploads.json
-// 3. XL backend object listing - isLeaf is true if the entry is a directory and contains xl.json
-// 4. XL backend multipart listing - isLeaf is true if the entry is a directory and contains uploads.json
-type IsLeafFunc func(string, string) bool
-
-// IsLeafDirFunc - A function isLeafDir of type isLeafDirFunc is used to detect if an entry represents an empty directory.
-type IsLeafDirFunc func(string, string) bool
-
-// Note: input entries are expected to be sorted.
-func filterListEntries(bucket, prefixDir string, entries []string, prefixEntry string, isLeaf IsLeafFunc) ([]string, bool) {
-	// Filter entries that have the prefix prefixEntry.
-	entries = filterMatchingPrefix(entries, prefixEntry)
-
-	// Can isLeaf() check be delayed till when it has to be sent down the
-	// TreeWalkResult channel?
-	delayIsLeaf := delayIsLeafCheck(entries)
-	if delayIsLeaf {
-		return entries, true
-	}
-
-	// isLeaf() check has to happen here so that trailing "/" for objects can be removed.
-	for i, entry := range entries {
-		if isLeaf(bucket, pathJoin(prefixDir, entry)) {
-			entries[i] = strings.TrimSuffix(entry, slashSeparator)
-		}
-	}
-	// Sort again after removing trailing "/" for objects as the previous sort
-	// does not hold good anymore.
-	sort.Strings(entries)
-	return entries, false
-}
 
 // treeWalk walks directory tree recursively pushing TreeWalkResult into the channel as and when it encounters files.
-func doTreeWalk(ctx context.Context, bucket, prefixDir, entryPrefixMatch, marker string, recursive bool, listDir ListDirFunc, isLeaf IsLeafFunc, isLeafDir IsLeafDirFunc, resultCh chan TreeWalkResult, endWalkCh chan struct{}, isEnd bool) error {
+func doTreeWalk(ctx context.Context, bucket, prefixDir, entryPrefixMatch, marker string, recursive bool, listDir ListDirFunc, resultCh chan TreeWalkResult, endWalkCh chan struct{}, isEnd bool) error {
 	// Example:
 	// if prefixDir="one/two/three/" and marker="four/five.txt" treeWalk is recursively
 	// called with prefixDir="one/two/three/four/" and marker="five.txt"
@@ -139,12 +75,7 @@ func doTreeWalk(ctx context.Context, bucket, prefixDir, entryPrefixMatch, marker
 		}
 	}
 
-	entries, delayIsLeaf := listDir(bucket, prefixDir, entryPrefixMatch)
+	entries := listDir(bucket, prefixDir, entryPrefixMatch)
-	// When isleaf check is delayed, make sure that it is set correctly here.
-	if delayIsLeaf && isLeaf == nil {
-		return errInvalidArgument
-	}
-
 	// For an empty list return right here.
 	if len(entries) == 0 {
 		return nil
@@ -163,21 +94,12 @@ func doTreeWalk(ctx context.Context, bucket, prefixDir, entryPrefixMatch, marker
 	}
 
 	for i, entry := range entries {
-		var leaf, leafDir bool
-
 		pentry := pathJoin(prefixDir, entry)
-		// Decision to do isLeaf check was pushed from listDir() to here.
-		if delayIsLeaf {
-			leaf = isLeaf(bucket, pentry)
-			if leaf {
-				entry = strings.TrimSuffix(entry, slashSeparator)
-			}
-		} else {
-			leaf = !strings.HasSuffix(entry, slashSeparator)
-		}
 
-		if strings.HasSuffix(entry, slashSeparator) {
+		leaf := !hasSuffix(pentry, slashSeparator)
-			leafDir = isLeafDir(bucket, pentry)
+		var leafDir bool
+		if !leaf {
+			leafDir = false
 		}
 
 		isDir := !leafDir && !leaf
@@ -208,7 +130,7 @@ func doTreeWalk(ctx context.Context, bucket, prefixDir, entryPrefixMatch, marker
 			// true at the end of the treeWalk stream.
 			markIsEnd := i == len(entries)-1 && isEnd
 			if err := doTreeWalk(ctx, bucket, pentry, prefixMatch, markerArg, recursive,
-				listDir, isLeaf, isLeafDir, resultCh, endWalkCh, markIsEnd); err != nil {
+				listDir, resultCh, endWalkCh, markIsEnd); err != nil {
 				return err
 			}
 			continue
@@ -228,7 +150,7 @@ func doTreeWalk(ctx context.Context, bucket, prefixDir, entryPrefixMatch, marker
 }
 
 // Initiate a new treeWalk in a goroutine.
-func startTreeWalk(ctx context.Context, bucket, prefix, marker string, recursive bool, listDir ListDirFunc, isLeaf IsLeafFunc, isLeafDir IsLeafDirFunc, endWalkCh chan struct{}) chan TreeWalkResult {
+func startTreeWalk(ctx context.Context, bucket, prefix, marker string, recursive bool, listDir ListDirFunc, endWalkCh chan struct{}) chan TreeWalkResult {
 	// Example 1
 	// If prefix is "one/two/three/" and marker is "one/two/three/four/five.txt"
 	// treeWalk is called with prefixDir="one/two/three/" and marker="four/five.txt"
@@ -250,7 +172,7 @@ func startTreeWalk(ctx context.Context, bucket, prefix, marker string, recursive
 	marker = strings.TrimPrefix(marker, prefixDir)
 	go func() {
 		isEnd := true // Indication to start walking the tree with end as true.
-		doTreeWalk(ctx, bucket, prefixDir, entryPrefixMatch, marker, recursive, listDir, isLeaf, isLeafDir, resultCh, endWalkCh, isEnd)
+		doTreeWalk(ctx, bucket, prefixDir, entryPrefixMatch, marker, recursive, listDir, resultCh, endWalkCh, isEnd)
 		close(resultCh)
 	}()
 	return resultCh

cmd/tree-walk_test.go

@@ -30,49 +30,6 @@ import (
 // Fixed volume name that could be used across tests
 const volume = "testvolume"
 
-// Test for delayIsLeafCheck.
-func TestDelayIsLeafCheck(t *testing.T) {
-	testCases := []struct {
-		entries []string
-		delay   bool
-	}{
-		// Test cases where isLeaf check can't be delayed.
-		{
-			[]string{"a-b/", "a/"},
-			false,
-		},
-		{
-			[]string{"a%b/", "a/"},
-			false,
-		},
-		{
-			[]string{"a-b-c", "a-b/"},
-			false,
-		},
-
-		// Test cases where isLeaf check can be delayed.
-		{
-			[]string{"a-b/", "aa/"},
-			true,
-		},
-		{
-			[]string{"a", "a-b"},
-			true,
-		},
-		{
-			[]string{"aaa", "bbb"},
-			true,
-		},
-	}
-	for i, testCase := range testCases {
-		expected := testCase.delay
-		got := delayIsLeafCheck(testCase.entries)
-		if expected != got {
-			t.Errorf("Test %d : Expected %t got %t", i+1, expected, got)
-		}
-	}
-}
-
 // Test for filterMatchingPrefix.
 func TestFilterMatchingPrefix(t *testing.T) {
 	entries := []string{"a", "aab", "ab", "abbbb", "zzz"}
@@ -128,11 +85,11 @@ func createNamespace(disk StorageAPI, volume string, files []string) error {
 
 // Test if tree walker returns entries matching prefix alone are received
 // when a non empty prefix is supplied.
-func testTreeWalkPrefix(t *testing.T, listDir ListDirFunc, isLeaf IsLeafFunc, isLeafDir IsLeafDirFunc) {
+func testTreeWalkPrefix(t *testing.T, listDir ListDirFunc) {
 	// Start the tree walk go-routine.
 	prefix := "d/"
 	endWalkCh := make(chan struct{})
-	twResultCh := startTreeWalk(context.Background(), volume, prefix, "", true, listDir, isLeaf, isLeafDir, endWalkCh)
+	twResultCh := startTreeWalk(context.Background(), volume, prefix, "", true, listDir, endWalkCh)
 
 	// Check if all entries received on the channel match the prefix.
 	for res := range twResultCh {
@@ -143,11 +100,11 @@ func testTreeWalkPrefix(t *testing.T, listDir ListDirFunc, isLeaf IsLeafFunc, is
 }
 
 // Test if entries received on tree walk's channel appear after the supplied marker.
-func testTreeWalkMarker(t *testing.T, listDir ListDirFunc, isLeaf IsLeafFunc, isLeafDir IsLeafDirFunc) {
+func testTreeWalkMarker(t *testing.T, listDir ListDirFunc) {
 	// Start the tree walk go-routine.
 	prefix := ""
 	endWalkCh := make(chan struct{})
-	twResultCh := startTreeWalk(context.Background(), volume, prefix, "d/g", true, listDir, isLeaf, isLeafDir, endWalkCh)
+	twResultCh := startTreeWalk(context.Background(), volume, prefix, "d/g", true, listDir, endWalkCh)
 
 	// Check if only 3 entries, namely d/g/h, i/j/k, lmn are received on the channel.
 	expectedCount := 3
@@ -184,23 +141,13 @@ func TestTreeWalk(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	isLeaf := func(volume, prefix string) bool {
+	listDir := listDirFactory(context.Background(), disk)
-		return !hasSuffix(prefix, slashSeparator)
-	}
 
-	isLeafDir := func(volume, prefix string) bool {
-		entries, listErr := disk.ListDir(volume, prefix, 1, "")
-		if listErr != nil {
-			return false
-		}
-		return len(entries) == 0
-	}
-
-	listDir := listDirFactory(context.Background(), isLeaf, disk)
 	// Simple test for prefix based walk.
-	testTreeWalkPrefix(t, listDir, isLeaf, isLeafDir)
+	testTreeWalkPrefix(t, listDir)
 	// Simple test when marker is set.
-	testTreeWalkMarker(t, listDir, isLeaf, isLeafDir)
+	testTreeWalkMarker(t, listDir)
+
 	err = os.RemoveAll(fsDir)
 	if err != nil {
 		t.Fatal(err)
@@ -228,19 +175,7 @@ func TestTreeWalkTimeout(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	isLeaf := func(volume, prefix string) bool {
+	listDir := listDirFactory(context.Background(), disk)
-		return !hasSuffix(prefix, slashSeparator)
-	}
-
-	isLeafDir := func(volume, prefix string) bool {
-		entries, listErr := disk.ListDir(volume, prefix, 1, "")
-		if listErr != nil {
-			return false
-		}
-		return len(entries) == 0
-	}
-
-	listDir := listDirFactory(context.Background(), isLeaf, disk)
 
 	// TreeWalk pool with 2 seconds timeout for tree-walk go routines.
 	pool := NewTreeWalkPool(2 * time.Second)
@@ -249,7 +184,7 @@ func TestTreeWalkTimeout(t *testing.T) {
 	prefix := ""
 	marker := ""
 	recursive := true
-	resultCh := startTreeWalk(context.Background(), volume, prefix, marker, recursive, listDir, isLeaf, isLeafDir, endWalkCh)
+	resultCh := startTreeWalk(context.Background(), volume, prefix, marker, recursive, listDir, endWalkCh)
 
 	params := listParams{
 		bucket:    volume,
@@ -313,9 +248,7 @@ func TestListDir(t *testing.T) {
 	}
 
 	// create listDir function.
-	listDir := listDirFactory(context.Background(), func(volume, prefix string) bool {
+	listDir := listDirFactory(context.Background(), disk1, disk2)
-		return !hasSuffix(prefix, slashSeparator)
-	}, disk1, disk2)
 
 	// Create file1 in fsDir1 and file2 in fsDir2.
 	disks := []StorageAPI{disk1, disk2}
@@ -327,7 +260,7 @@ func TestListDir(t *testing.T) {
 	}
 
 	// Should list "file1" from fsDir1.
-	entries, _ := listDir(volume, "", "")
+	entries := listDir(volume, "", "")
 	if len(entries) != 2 {
 		t.Fatal("Expected the number of entries to be 2")
 	}
@@ -345,7 +278,7 @@ func TestListDir(t *testing.T) {
 	}
 
 	// Should list "file2" from fsDir2.
-	entries, _ = listDir(volume, "", "")
+	entries = listDir(volume, "", "")
 	if len(entries) != 1 {
 		t.Fatal("Expected the number of entries to be 1")
 	}
@@ -373,21 +306,8 @@ func TestRecursiveTreeWalk(t *testing.T) {
 		t.Fatalf("Unable to create StorageAPI: %s", err)
 	}
 
-	// Simple isLeaf check, returns true if there is no trailing "/"
-	isLeaf := func(volume, prefix string) bool {
-		return !hasSuffix(prefix, slashSeparator)
-	}
-
-	isLeafDir := func(volume, prefix string) bool {
-		entries, listErr := disk1.ListDir(volume, prefix, 1, "")
-		if listErr != nil {
-			return false
-		}
-		return len(entries) == 0
-	}
-
 	// Create listDir function.
-	listDir := listDirFactory(context.Background(), isLeaf, disk1)
+	listDir := listDirFactory(context.Background(), disk1)
 
 	// Create the namespace.
 	var files = []string{
@@ -461,13 +381,16 @@ func TestRecursiveTreeWalk(t *testing.T) {
 		}},
 	}
 	for i, testCase := range testCases {
-		for entry := range startTreeWalk(context.Background(), volume,
+		testCase := testCase
-			testCase.prefix, testCase.marker, testCase.recursive,
+		t.Run(fmt.Sprintf("Test%d", i+1), func(t *testing.T) {
-			listDir, isLeaf, isLeafDir, endWalkCh) {
+			for entry := range startTreeWalk(context.Background(), volume,
-			if _, found := testCase.expected[entry.entry]; !found {
+				testCase.prefix, testCase.marker, testCase.recursive,
-				t.Errorf("Test %d: Expected %s, but couldn't find", i+1, entry.entry)
+				listDir, endWalkCh) {
+				if _, found := testCase.expected[entry.entry]; !found {
+					t.Errorf("Expected %s, but couldn't find", entry.entry)
+				}
 			}
-		}
+		})
 	}
 	err = os.RemoveAll(fsDir1)
 	if err != nil {
@@ -488,21 +411,8 @@ func TestSortedness(t *testing.T) {
 		t.Fatalf("Unable to create StorageAPI: %s", err)
 	}
 
-	// Simple isLeaf check, returns true if there is no trailing "/"
-	isLeaf := func(volume, prefix string) bool {
-		return !hasSuffix(prefix, slashSeparator)
-	}
-
-	isLeafDir := func(volume, prefix string) bool {
-		entries, listErr := disk1.ListDir(volume, prefix, 1, "")
-		if listErr != nil {
-			return false
-		}
-		return len(entries) == 0
-	}
-
 	// Create listDir function.
-	listDir := listDirFactory(context.Background(), isLeaf, disk1)
+	listDir := listDirFactory(context.Background(), disk1)
 
 	// Create the namespace.
 	var files = []string{
@@ -544,7 +454,7 @@ func TestSortedness(t *testing.T) {
 		var actualEntries []string
 		for entry := range startTreeWalk(context.Background(), volume,
 			test.prefix, test.marker, test.recursive,
-			listDir, isLeaf, isLeafDir, endWalkCh) {
+			listDir, endWalkCh) {
 			actualEntries = append(actualEntries, entry.entry)
 		}
 		if !sort.IsSorted(sort.StringSlice(actualEntries)) {
@@ -572,20 +482,8 @@ func TestTreeWalkIsEnd(t *testing.T) {
 		t.Fatalf("Unable to create StorageAPI: %s", err)
 	}
 
-	isLeaf := func(volume, prefix string) bool {
-		return !hasSuffix(prefix, slashSeparator)
-	}
-
-	isLeafDir := func(volume, prefix string) bool {
-		entries, listErr := disk1.ListDir(volume, prefix, 1, "")
-		if listErr != nil {
-			return false
-		}
-		return len(entries) == 0
-	}
-
 	// Create listDir function.
-	listDir := listDirFactory(context.Background(), isLeaf, disk1)
+	listDir := listDirFactory(context.Background(), disk1)
 
 	// Create the namespace.
 	var files = []string{
@@ -626,7 +524,8 @@ func TestTreeWalkIsEnd(t *testing.T) {
 	}
 	for i, test := range testCases {
 		var entry TreeWalkResult
-		for entry = range startTreeWalk(context.Background(), volume, test.prefix, test.marker, test.recursive, listDir, isLeaf, isLeafDir, endWalkCh) {
+		for entry = range startTreeWalk(context.Background(), volume, test.prefix,
+			test.marker, test.recursive, listDir, endWalkCh) {
 		}
 		if entry.entry != test.expectedEntry {
 			t.Errorf("Test %d: Expected entry %s, but received %s with the EOF marker", i, test.expectedEntry, entry.entry)

cmd/xl-sets.go

@@ -643,7 +643,7 @@ func (s *xlSets) CopyObject(ctx context.Context, srcBucket, srcObject, destBucke
 // Returns function "listDir" of the type listDirFunc.
 // isLeaf - is used by listDir function to check if an entry is a leaf or non-leaf entry.
 // disks - used for doing disk.ListDir(). Sets passes set of disks.
-func listDirSetsFactory(ctx context.Context, isLeaf IsLeafFunc, isLeafDir IsLeafDirFunc, sets ...*xlObjects) ListDirFunc {
+func listDirSetsFactory(ctx context.Context, sets ...*xlObjects) ListDirFunc {
 	listDirInternal := func(bucket, prefixDir, prefixEntry string, disks []StorageAPI) (mergedEntries []string) {
 		var diskEntries = make([][]string, len(disks))
 		var wg sync.WaitGroup
@@ -684,7 +684,7 @@ func listDirSetsFactory(ctx context.Context, isLeaf IsLeafFunc, isLeafDir IsLeaf
 	}
 
 	// listDir - lists all the entries at a given prefix and given entry in the prefix.
-	listDir := func(bucket, prefixDir, prefixEntry string) (mergedEntries []string, delayIsLeaf bool) {
+	listDir := func(bucket, prefixDir, prefixEntry string) (mergedEntries []string) {
 		for _, set := range sets {
 			var newEntries []string
 			// Find elements in entries which are not in mergedEntries
@@ -703,7 +703,7 @@ func listDirSetsFactory(ctx context.Context, isLeaf IsLeafFunc, isLeafDir IsLeaf
 				sort.Strings(mergedEntries)
 			}
 		}
-		return filterListEntries(bucket, prefixDir, mergedEntries, prefixEntry, isLeaf)
+		return filterMatchingPrefix(mergedEntries, prefixEntry)
 	}
 	return listDir
 }
@@ -712,15 +712,7 @@ func listDirSetsFactory(ctx context.Context, isLeaf IsLeafFunc, isLeafDir IsLeaf
 // listed and subsequently merge lexically sorted inside listDirSetsFactory(). Resulting
 // value through the walk channel receives the data properly lexically sorted.
 func (s *xlSets) ListObjects(ctx context.Context, bucket, prefix, marker, delimiter string, maxKeys int) (ListObjectsInfo, error) {
-	isLeaf := func(bucket, entry string) bool {
+	listDir := listDirSetsFactory(ctx, s.sets...)
-		return !hasSuffix(entry, slashSeparator)
-	}
-
-	isLeafDir := func(bucket, entry string) bool {
-		return false
-	}
-
-	listDir := listDirSetsFactory(ctx, isLeaf, isLeafDir, s.sets...)
 
 	var getObjectInfoDirs []func(context.Context, string, string) (ObjectInfo, error)
 	// Verify prefixes in all sets.
@@ -732,7 +724,7 @@ func (s *xlSets) ListObjects(ctx context.Context, bucket, prefix, marker, delimi
 		return s.getHashedSet(entry).getObjectInfo(ctx, bucket, entry)
 	}
 
-	return listObjects(ctx, s, bucket, prefix, marker, delimiter, maxKeys, s.listPool, isLeaf, isLeafDir, listDir, getObjectInfo, getObjectInfoDirs...)
+	return listObjects(ctx, s, bucket, prefix, marker, delimiter, maxKeys, s.listPool, listDir, getObjectInfo, getObjectInfoDirs...)
 }
 
 func (s *xlSets) ListMultipartUploads(ctx context.Context, bucket, prefix, keyMarker, uploadIDMarker, delimiter string, maxUploads int) (result ListMultipartsInfo, err error) {
@@ -1245,16 +1237,8 @@ func (s *xlSets) HealObjects(ctx context.Context, bucket, prefix string, healObj
 	recursive := true
 
 	endWalkCh := make(chan struct{})
-	isLeaf := func(bucket, entry string) bool {
+	listDir := listDirSetsFactory(ctx, s.sets...)
-		return hasSuffix(entry, xlMetaJSONFile)
+	walkResultCh := startTreeWalk(ctx, bucket, prefix, "", recursive, listDir, endWalkCh)
-	}
-
-	isLeafDir := func(bucket, entry string) bool {
-		return false
-	}
-
-	listDir := listDirSetsFactory(ctx, isLeaf, isLeafDir, s.sets...)
-	walkResultCh := startTreeWalk(ctx, bucket, prefix, "", recursive, listDir, isLeaf, isLeafDir, endWalkCh)
 	for {
 		walkResult, ok := <-walkResultCh
 		if !ok {

cmd/xl-v1-common.go

@@ -85,34 +85,3 @@ func (xl xlObjects) isObject(bucket, prefix string) (ok bool) {
 
 	return reduceReadQuorumErrs(context.Background(), errs, objectOpIgnoredErrs, readQuorum) == nil
 }
-
-// isObjectDir returns if the specified path represents an empty directory.
-func (xl xlObjects) isObjectDir(bucket, prefix string) (ok bool) {
-	var errs = make([]error, len(xl.getDisks()))
-	var wg sync.WaitGroup
-	for index, disk := range xl.getDisks() {
-		if disk == nil {
-			continue
-		}
-		wg.Add(1)
-		go func(index int, disk StorageAPI) {
-			defer wg.Done()
-			// Check if 'prefix' is an object on this 'disk', else continue the check the next disk
-			entries, err := disk.ListDir(bucket, prefix, 1, "")
-			if err != nil {
-				errs[index] = err
-				return
-			}
-			if len(entries) > 0 {
-				errs[index] = errVolumeNotEmpty
-				return
-			}
-		}(index, disk)
-	}
-
-	wg.Wait()
-
-	readQuorum := len(xl.getDisks()) / 2
-
-	return reduceReadQuorumErrs(context.Background(), errs, objectOpIgnoredErrs, readQuorum) == nil
-}

cmd/xl-v1-list-objects.go

@@ -22,11 +22,10 @@ import (
 )
 
 // Returns function "listDir" of the type listDirFunc.
-// isLeaf - is used by listDir function to check if an entry is a leaf or non-leaf entry.
 // disks - used for doing disk.ListDir()
-func listDirFactory(ctx context.Context, isLeaf IsLeafFunc, disks ...StorageAPI) ListDirFunc {
+func listDirFactory(ctx context.Context, disks ...StorageAPI) ListDirFunc {
 	// Returns sorted merged entries from all the disks.
-	listDir := func(bucket, prefixDir, prefixEntry string) (mergedEntries []string, delayIsLeaf bool) {
+	listDir := func(bucket, prefixDir, prefixEntry string) (mergedEntries []string) {
 		for _, disk := range disks {
 			if disk == nil {
 				continue
@@ -55,8 +54,7 @@ func listDirFactory(ctx context.Context, isLeaf IsLeafFunc, disks ...StorageAPI)
 				sort.Strings(mergedEntries)
 			}
 		}
-		mergedEntries, delayIsLeaf = filterListEntries(bucket, prefixDir, mergedEntries, prefixEntry, isLeaf)
+		return filterMatchingPrefix(mergedEntries, prefixEntry)
-		return mergedEntries, delayIsLeaf
 	}
 	return listDir
 }
@@ -72,10 +70,8 @@ func (xl xlObjects) listObjects(ctx context.Context, bucket, prefix, marker, del
 	walkResultCh, endWalkCh := xl.listPool.Release(listParams{bucket, recursive, marker, prefix})
 	if walkResultCh == nil {
 		endWalkCh = make(chan struct{})
-		isLeaf := xl.isObject
+		listDir := listDirFactory(ctx, xl.getLoadBalancedDisks()...)
-		isLeafDir := xl.isObjectDir
+		walkResultCh = startTreeWalk(ctx, bucket, prefix, marker, recursive, listDir, endWalkCh)
-		listDir := listDirFactory(ctx, isLeaf, xl.getLoadBalancedDisks()...)
-		walkResultCh = startTreeWalk(ctx, bucket, prefix, marker, recursive, listDir, isLeaf, isLeafDir, endWalkCh)
 	}
 
 	var objInfos []ObjectInfo