Fixes #184; request server file list batching

The request server needs to support batching of file list requests. To
address this the methods LsSave(string) and LsNext()string are added to
the Request object. They should work to store a token to keep your place.

The Handler should now return an EOF when the end of the directory list is
reached. If it doesn't but returns an empty list, the wrapper will send the EOF
for you.

The old behaviour of just returning 1 batch and sending the EOF is preserved if
you don't set the token (you don't use LsSave). It will return the 1 list and
EOF on the next underlying readdir call. This is to preserve backwards
compatibility and it not the recommended way to handle file lists.

request-example.go

@@ -10,6 +10,8 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sort"
+	"strconv"
 	"sync"
 	"time"
 )
@@ -104,13 +106,32 @@ func (fs *root) Fileinfo(r Request) ([]os.FileInfo, error) {
 	defer fs.filesLock.Unlock()
 	switch r.Method {
 	case "List":
+		var err error
 		list := []os.FileInfo{}
+		batch_size := 10
+		current_offset := 0
+		if token := r.LsNext(); token != "" {
+			current_offset, err = strconv.Atoi(token)
+			if err != nil {
+				return list, os.ErrInvalid
+			}
+		}
 		for fn, fi := range fs.files {
 			if filepath.Dir(fn) == r.Filepath {
 				list = append(list, fi)
 			}
 		}
-		return list, nil
+		sort.Slice(list,
+			func(i, j int) bool { return list[i].Name() < list[j].Name() })
+		if len(list) < current_offset {
+			return nil, io.EOF
+		}
+		new_offset := current_offset + batch_size
+		if new_offset > len(list) {
+			new_offset = len(list)
+		}
+		r.LsSave(strconv.Itoa(new_offset))
+		return list[current_offset:new_offset], nil
 	case "Stat":
 		file, err := fs.fetch(r.Filepath)
 		if err != nil {

request-server_test.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"net"
 	"os"
-	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -317,14 +316,14 @@ func TestRequestReadlink(t *testing.T) {
 func TestRequestReaddir(t *testing.T) {
 	p := clientRequestServerPair(t)
 	defer p.Close()
-	_, err := putTestFile(p.cli, "/foo", "hello")
-	assert.Nil(t, err)
-	_, err = putTestFile(p.cli, "/bar", "goodbye")
-	assert.Nil(t, err)
+	for i := 0; i < 100; i++ {
+		fname := fmt.Sprintf("/foo_%02d", i)
+		_, err := putTestFile(p.cli, fname, fname)
+		assert.Nil(t, err)
+	}
 	di, err := p.cli.ReadDir("/")
 	assert.Nil(t, err)
-	assert.Len(t, di, 2)
-	names := []string{di[0].Name(), di[1].Name()}
-	sort.Strings(names)
-	assert.Equal(t, []string{"bar", "foo"}, names)
+	assert.Len(t, di, 100)
+	names := []string{di[18].Name(), di[81].Name()}
+	assert.Equal(t, []string{"foo_18", "foo_81"}, names)
 }

request.go

@@ -29,9 +29,10 @@ type Request struct {
 }
 
 type state struct {
-	writerAt io.WriterAt
-	readerAt io.ReaderAt
-	endofdir bool // need to track when to send EOF for readdir
+	writerAt     io.WriterAt
+	readerAt     io.ReaderAt
+	endofdir     bool // in case handler doesn't use EOF on file list
+	readdirToken string
 }
 
 type packet_data struct {
@@ -67,8 +68,24 @@ func NewRequest(method, path string) Request {
 	return request
 }
 
-// manage state
-func (r Request) setState(s interface{}) {
+// LsSave takes a token to keep track of file list batches. Openssh uses a
+// batch size of 100, so I suggest sticking close to that.
+func (r Request) LsSave(token string) {
+	r.stateLock.RLock()
+	defer r.stateLock.RUnlock()
+	r.state.readdirToken = token
+}
+
+// LsNext should return the token from the previous call to know which batch
+// to return next.
+func (r Request) LsNext() string {
+	r.stateLock.RLock()
+	defer r.stateLock.RUnlock()
+	return r.state.readdirToken
+}
+
+// manage file read/write state
+func (r Request) setFileState(s interface{}) {
 	r.stateLock.Lock()
 	defer r.stateLock.Unlock()
 	switch s := s.(type) {
@@ -76,8 +93,7 @@ func (r Request) setState(s interface{}) {
 		r.state.writerAt = s
 	case io.ReaderAt:
 		r.state.readerAt = s
-	case bool:
-		r.state.endofdir = s
+
 	}
 }
 
@@ -93,6 +109,14 @@ func (r Request) getReader() io.ReaderAt {
 	return r.state.readerAt
 }
 
+// For backwards compatibility. The Handler didn't have batch handling at
+// first, and just always assumed 1 batch. This preserves that behavior.
+func (r Request) setEOD(eod bool) {
+	r.stateLock.RLock()
+	defer r.stateLock.RUnlock()
+	r.state.endofdir = eod
+}
+
 func (r Request) getEOD() bool {
 	r.stateLock.RLock()
 	defer r.stateLock.RUnlock()
@@ -149,7 +173,7 @@ func fileget(h FileReader, r Request) (responsePacket, error) {
 		if err != nil {
 			return nil, err
 		}
-		r.setState(reader)
+		r.setFileState(reader)
 	}
 
 	pd := r.popPacket()
@@ -174,7 +198,7 @@ func fileput(h FileWriter, r Request) (responsePacket, error) {
 		if err != nil {
 			return nil, err
 		}
-		r.setState(writer)
+		r.setFileState(writer)
 	}
 
 	pd := r.popPacket()
@@ -224,7 +248,14 @@ func fileinfo(h FileInfoer, r Request) (responsePacket, error) {
 				Attrs:    []interface{}{fi},
 			})
 		}
-		r.setState(true)
+		// No entries means we should return EOF as the Handler didn't.
+		if len(finfo) == 0 {
+			return nil, io.EOF
+		}
+		// If files are returned but no token is set, return EOF next call.
+		if r.LsNext() == "" {
+			r.setEOD(true)
+		}
 		return ret, nil
 	case "Stat":
 		if len(finfo) == 0 {