backend: Rework List()

For a discussion see #1567

internal/backend/local/layout_test.go

@@ -49,8 +49,13 @@ func TestLayout(t *testing.T) {
 			}
 
 			datafiles := make(map[string]bool)
-			for id := range be.List(context.TODO(), restic.DataFile) {
-				datafiles[id] = false
+			err = be.List(context.TODO(), restic.DataFile, func(fi restic.FileInfo) error {
+				datafiles[fi.Name] = false
+				return nil
+			})
+
+			if err != nil {
+				t.Fatalf("List() returned error %v", err)
 			}
 
 			if len(datafiles) == 0 {

internal/backend/local/local.go

@@ -228,50 +228,53 @@ func isFile(fi os.FileInfo) bool {
 
 // List returns a channel that yields all names of blobs of type t. A
 // goroutine is started for this.
-func (b *Local) List(ctx context.Context, t restic.FileType) <-chan string {
+func (b *Local) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error {
 	debug.Log("List %v", t)
 
-	ch := make(chan string)
-
-	go func() {
-		defer close(ch)
-
-		basedir, subdirs := b.Basedir(t)
-		err := fs.Walk(basedir, func(path string, fi os.FileInfo, err error) error {
-			debug.Log("walk on %v\n", path)
-			if err != nil {
-				return err
-			}
-
-			if path == basedir {
-				return nil
-			}
-
-			if !isFile(fi) {
-				return nil
-			}
-
-			if fi.IsDir() && !subdirs {
-				return filepath.SkipDir
-			}
-
-			debug.Log("send %v\n", filepath.Base(path))
-
-			select {
-			case ch <- filepath.Base(path):
-			case <-ctx.Done():
-				return nil
-			}
-
-			return nil
-		})
-
+	basedir, subdirs := b.Basedir(t)
+	err := fs.Walk(basedir, func(path string, fi os.FileInfo, err error) error {
+		debug.Log("walk on %v\n", path)
 		if err != nil {
-			debug.Log("Walk %v", err)
+			return err
 		}
-	}()
 
-	return ch
+		if path == basedir {
+			return nil
+		}
+
+		if !isFile(fi) {
+			return nil
+		}
+
+		if fi.IsDir() && !subdirs {
+			return filepath.SkipDir
+		}
+
+		debug.Log("send %v\n", filepath.Base(path))
+
+		rfi := restic.FileInfo{
+			Name: filepath.Base(path),
+			Size: fi.Size(),
+		}
+
+		err = fn(rfi)
+		if err != nil {
+			return err
+		}
+
+		if ctx.Err() != nil {
+			return ctx.Err()
+		}
+
+		return nil
+	})
+
+	if err != nil {
+		debug.Log("Walk %v", err)
+		return err
+	}
+
+	return ctx.Err()
 }
 
 // Delete removes the repository and all files.

internal/backend/mem/mem_backend.go

@@ -163,34 +163,31 @@ func (be *MemoryBackend) Remove(ctx context.Context, h restic.Handle) error {
 }
 
 // List returns a channel which yields entries from the backend.
-func (be *MemoryBackend) List(ctx context.Context, t restic.FileType) <-chan string {
+func (be *MemoryBackend) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error {
 	be.m.Lock()
 	defer be.m.Unlock()
 
-	ch := make(chan string)
-
-	var ids []string
-	for entry := range be.data {
+	for entry, buf := range be.data {
 		if entry.Type != t {
 			continue
 		}
-		ids = append(ids, entry.Name)
+
+		fi := restic.FileInfo{
+			Name: entry.Name,
+			Size: int64(len(buf)),
+		}
+
+		err := fn(fi)
+		if err != nil {
+			return err
+		}
+
+		if ctx.Err() != nil {
+			return ctx.Err()
+		}
 	}
 
-	debug.Log("list %v: %v", t, ids)
-
-	go func() {
-		defer close(ch)
-		for _, id := range ids {
-			select {
-			case ch <- id:
-			case <-ctx.Done():
-				return
-			}
-		}
-	}()
-
-	return ch
+	return ctx.Err()
 }
 
 // Location returns the location of the backend (RAM).

internal/backend/test/tests.go

@@ -283,12 +283,17 @@ func (s *Suite) TestList(t *testing.T) {
 				s.SetListMaxItems(test.maxItems)
 			}
 
-			for name := range b.List(context.TODO(), restic.DataFile) {
-				id, err := restic.ParseID(name)
+			err := b.List(context.TODO(), restic.DataFile, func(fi restic.FileInfo) error {
+				id, err := restic.ParseID(fi.Name)
 				if err != nil {
 					t.Fatal(err)
 				}
 				list2.Insert(id)
+				return nil
+			})
+
+			if err != nil {
+				t.Fatalf("List returned error %v", err)
 			}
 
 			t.Logf("loaded %v IDs from backend", len(list2))
@@ -556,10 +561,16 @@ func delayedList(t testing.TB, b restic.Backend, tpe restic.FileType, max int, m
 	list := restic.NewIDSet()
 	start := time.Now()
 	for i := 0; i < max; i++ {
-		for s := range b.List(context.TODO(), tpe) {
-			id := restic.TestParseID(s)
+		err := b.List(context.TODO(), tpe, func(fi restic.FileInfo) error {
+			id := restic.TestParseID(fi.Name)
 			list.Insert(id)
+			return nil
+		})
+
+		if err != nil {
+			t.Fatal(err)
 		}
+
 		if len(list) < max && time.Since(start) < maxwait {
 			time.Sleep(500 * time.Millisecond)
 		}

internal/restic/backend.go

@@ -32,10 +32,9 @@ type Backend interface {
 	// Stat returns information about the File identified by h.
 	Stat(ctx context.Context, h Handle) (FileInfo, error)
 
-	// List returns a channel that yields all names of files of type t in an
-	// arbitrary order. A goroutine is started for this, which is stopped when
-	// ctx is cancelled.
-	List(ctx context.Context, t FileType) <-chan string
+	// List runs fn for each file in the backend which has the type t. When an
+	// error occurs (or fn returns an error), List stops and returns it.
+	List(ctx context.Context, t FileType, fn func(FileInfo) error) error
 
 	// IsNotExist returns true if the error was caused by a non-existing file
 	// in the backend.
@@ -45,6 +44,8 @@ type Backend interface {
 	Delete(ctx context.Context) error
 }
 
-// FileInfo is returned by Stat() and contains information about a file in the
-// backend.
-type FileInfo struct{ Size int64 }
+// FileInfo is contains information about a file in the backend.
+type FileInfo struct {
+	Size int64
+	Name string
+}

internal/restic/backend_find.go

@@ -20,15 +20,23 @@ var ErrMultipleIDMatches = errors.New("multiple IDs with prefix found")
 func Find(be Lister, t FileType, prefix string) (string, error) {
 	match := ""
 
-	// TODO: optimize by sorting list etc.
-	for name := range be.List(context.TODO(), t) {
-		if prefix == name[:len(prefix)] {
+	ctx, cancel := context.WithCancel(context.TODO())
+	defer cancel()
+
+	err := be.List(ctx, t, func(fi FileInfo) error {
+		if prefix == fi.Name[:len(prefix)] {
 			if match == "" {
-				match = name
+				match = fi.Name
 			} else {
-				return "", ErrMultipleIDMatches
+				return ErrMultipleIDMatches
 			}
 		}
+
+		return nil
+	})
+
+	if err != nil {
+		return "", err
 	}
 
 	if match != "" {
@@ -45,8 +53,17 @@ const minPrefixLength = 8
 func PrefixLength(be Lister, t FileType) (int, error) {
 	// load all IDs of the given type
 	list := make([]string, 0, 100)
-	for name := range be.List(context.TODO(), t) {
-		list = append(list, name)
+
+	ctx, cancel := context.WithCancel(context.TODO())
+	defer cancel()
+
+	err := be.List(ctx, t, func(fi FileInfo) error {
+		list = append(list, fi.Name)
+		return nil
+	})
+
+	if err != nil {
+		return 0, err
 	}
 
 	// select prefixes of length l, test if the last one is the same as the current one

internal/restic/backend_find_test.go

@@ -6,11 +6,11 @@ import (
 )
 
 type mockBackend struct {
-	list func(context.Context, FileType) <-chan string
+	list func(context.Context, FileType, func(FileInfo) error) error
 }
 
-func (m mockBackend) List(ctx context.Context, t FileType) <-chan string {
-	return m.list(ctx, t)
+func (m mockBackend) List(ctx context.Context, t FileType, fn func(FileInfo) error) error {
+	return m.list(ctx, t, fn)
 }
 
 var samples = IDs{
@@ -28,19 +28,14 @@ func TestPrefixLength(t *testing.T) {
 	list := samples
 
 	m := mockBackend{}
-	m.list = func(ctx context.Context, t FileType) <-chan string {
-		ch := make(chan string)
-		go func() {
-			defer close(ch)
-			for _, id := range list {
-				select {
-				case ch <- id.String():
-				case <-ctx.Done():
-					return
-				}
+	m.list = func(ctx context.Context, t FileType, fn func(FileInfo) error) error {
+		for _, id := range list {
+			err := fn(FileInfo{Name: id.String()})
+			if err != nil {
+				return err
 			}
-		}()
-		return ch
+		}
+		return nil
 	}
 
 	l, err := PrefixLength(m, SnapshotFile)

internal/restic/repository.go

@@ -46,7 +46,7 @@ type Repository interface {
 
 // Lister allows listing files in a backend.
 type Lister interface {
-	List(context.Context, FileType) <-chan string
+	List(context.Context, FileType, func(FileInfo) error) error
 }
 
 // Index keeps track of the blobs are stored within files.