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.
diff --git a/request-example.go b/request-example.go
index ae7bba4..86a4728 100644
--- a/request-example.go
+++ b/request-example.go
@@ -10,6 +10,8 @@
"io"
"os"
"path/filepath"
+ "sort"
+ "strconv"
"sync"
"time"
)
@@ -104,13 +106,32 @@
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 {
diff --git a/request-server_test.go b/request-server_test.go
index 04554a3..ee9621b 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -5,7 +5,6 @@
"io"
"net"
"os"
- "sort"
"testing"
"github.com/stretchr/testify/assert"
@@ -317,14 +316,14 @@
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)
}
diff --git a/request.go b/request.go
index cd79e48..6d7d6ff 100644
--- a/request.go
+++ b/request.go
@@ -29,9 +29,10 @@
}
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 @@
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 @@
r.state.writerAt = s
case io.ReaderAt:
r.state.readerAt = s
- case bool:
- r.state.endofdir = s
+
}
}
@@ -93,6 +109,14 @@
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 @@
if err != nil {
return nil, err
}
- r.setState(reader)
+ r.setFileState(reader)
}
pd := r.popPacket()
@@ -174,7 +198,7 @@
if err != nil {
return nil, err
}
- r.setState(writer)
+ r.setFileState(writer)
}
pd := r.popPacket()
@@ -224,7 +248,14 @@
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 {
