Move Request.handle to *Request
Updates #192
Move handle() onto *Request, this has flow on effects to the interface
types declared in request-interface.go, the examples and the tests.
The decision to address #192 started with advice from gopl.io, but I
think has uncovered several bugs as many of the request methods operate
on copies of data stored in the RequestServer's cache. I think this is
unintentional, but may point to some correctness issues, see #193.
diff --git a/request-example.go b/request-example.go
index c355fbd..f66b060 100644
--- a/request-example.go
+++ b/request-example.go
@@ -25,7 +25,7 @@
}
// Handlers
-func (fs *root) Fileread(r Request) (io.ReaderAt, error) {
+func (fs *root) Fileread(r *Request) (io.ReaderAt, error) {
fs.filesLock.Lock()
defer fs.filesLock.Unlock()
file, err := fs.fetch(r.Filepath)
@@ -41,7 +41,7 @@
return file.ReaderAt()
}
-func (fs *root) Filewrite(r Request) (io.WriterAt, error) {
+func (fs *root) Filewrite(r *Request) (io.WriterAt, error) {
fs.filesLock.Lock()
defer fs.filesLock.Unlock()
file, err := fs.fetch(r.Filepath)
@@ -59,7 +59,7 @@
return file.WriterAt()
}
-func (fs *root) Filecmd(r Request) error {
+func (fs *root) Filecmd(r *Request) error {
fs.filesLock.Lock()
defer fs.filesLock.Unlock()
switch r.Method {
@@ -115,7 +115,7 @@
return n, nil
}
-func (fs *root) Filelist(r Request) (ListerAt, error) {
+func (fs *root) Filelist(r *Request) (ListerAt, error) {
fs.filesLock.Lock()
defer fs.filesLock.Unlock()
diff --git a/request-interfaces.go b/request-interfaces.go
index f7afb14..05a9dac 100644
--- a/request-interfaces.go
+++ b/request-interfaces.go
@@ -10,22 +10,22 @@
// FileReader should return an io.Reader for the filepath
type FileReader interface {
- Fileread(Request) (io.ReaderAt, error)
+ Fileread(*Request) (io.ReaderAt, error)
}
// FileWriter should return an io.Writer for the filepath
type FileWriter interface {
- Filewrite(Request) (io.WriterAt, error)
+ Filewrite(*Request) (io.WriterAt, error)
}
// FileCmder should return an error (rename, remove, setstate, etc.)
type FileCmder interface {
- Filecmd(Request) error
+ Filecmd(*Request) error
}
// FileLister should return file info interface and errors (readdir, stat)
type FileLister interface {
- Filelist(Request) (ListerAt, error)
+ Filelist(*Request) (ListerAt, error)
}
// ListerAt does for file lists what io.ReaderAt does for files.
diff --git a/request.go b/request.go
index 906e9ce..3fa438e 100644
--- a/request.go
+++ b/request.go
@@ -152,26 +152,23 @@
}
// called from worker to handle packet/request
-func (r Request) handle(handlers Handlers) (responsePacket, error) {
- var err error
- var rpkt responsePacket
+func (r *Request) handle(handlers Handlers) (responsePacket, error) {
switch r.Method {
case "Get":
- rpkt, err = fileget(handlers.FileGet, r)
+ return fileget(handlers.FileGet, r)
case "Put": // add "Append" to this to handle append only file writes
- rpkt, err = fileput(handlers.FilePut, r)
+ return fileput(handlers.FilePut, r)
case "Setstat", "Rename", "Rmdir", "Mkdir", "Symlink", "Remove":
- rpkt, err = filecmd(handlers.FileCmd, r)
+ return filecmd(handlers.FileCmd, r)
case "List", "Stat", "Readlink":
- rpkt, err = filelist(handlers.FileList, r)
+ return filelist(handlers.FileList, r)
default:
- return rpkt, errors.Errorf("unexpected method: %s", r.Method)
+ return nil, errors.Errorf("unexpected method: %s", r.Method)
}
- return rpkt, err
}
// wrap FileReader handler
-func fileget(h FileReader, r Request) (responsePacket, error) {
+func fileget(h FileReader, r *Request) (responsePacket, error) {
var err error
reader := r.getReader()
if reader == nil {
@@ -197,7 +194,7 @@
}
// wrap FileWriter handler
-func fileput(h FileWriter, r Request) (responsePacket, error) {
+func fileput(h FileWriter, r *Request) (responsePacket, error) {
var err error
writer := r.getWriter()
if writer == nil {
@@ -217,11 +214,12 @@
ID: pd.id,
StatusError: StatusError{
Code: ssh_FX_OK,
- }}, nil
+ },
+ }, nil
}
// wrap FileCmder handler
-func filecmd(h FileCmder, r Request) (responsePacket, error) {
+func filecmd(h FileCmder, r *Request) (responsePacket, error) {
err := h.Filecmd(r)
if err != nil {
return nil, err
@@ -230,11 +228,12 @@
ID: r.pkt_id,
StatusError: StatusError{
Code: ssh_FX_OK,
- }}, nil
+ },
+ }, nil
}
// wrap FileLister handler
-func filelist(h FileLister, r Request) (responsePacket, error) {
+func filelist(h FileLister, r *Request) (responsePacket, error) {
var err error
lister := r.getLister()
if lister == nil {
diff --git a/request_test.go b/request_test.go
index efcedcd..078f1dc 100644
--- a/request_test.go
+++ b/request_test.go
@@ -18,28 +18,25 @@
err error // dummy error, should be file related
}
-func (t *testHandler) Fileread(r Request) (io.ReaderAt, error) {
+func (t *testHandler) Fileread(r *Request) (io.ReaderAt, error) {
if t.err != nil {
return nil, t.err
}
return bytes.NewReader(t.filecontents), nil
}
-func (t *testHandler) Filewrite(r Request) (io.WriterAt, error) {
+func (t *testHandler) Filewrite(r *Request) (io.WriterAt, error) {
if t.err != nil {
return nil, t.err
}
return io.WriterAt(t.output), nil
}
-func (t *testHandler) Filecmd(r Request) error {
- if t.err != nil {
- return t.err
- }
- return nil
+func (t *testHandler) Filecmd(r *Request) error {
+ return t.err
}
-func (t *testHandler) Filelist(r Request) (ListerAt, error) {
+func (t *testHandler) Filelist(r *Request) (ListerAt, error) {
if t.err != nil {
return nil, t.err
}
