[ARVADOS] updated: 1.1.0-163-gc70be5b
Git user
git at public.curoverse.com
Tue Nov 21 02:01:22 EST 2017
Summary of changes:
sdk/go/httpserver/responsewriter.go | 40 ++++++++++++++-----------
services/keep-web/cadaver_test.go | 6 +---
services/keep-web/handler.go | 59 ++++++++++++++++++++++++++++++++-----
services/keep-web/webdav.go | 42 ++++++--------------------
4 files changed, 85 insertions(+), 62 deletions(-)
via c70be5b306945254a18ee03081cb4641bef925b1 (commit)
from 11127c5b67a10a46f60c1c1c53a2c2639b7914e1 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit c70be5b306945254a18ee03081cb4641bef925b1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Tue Nov 21 01:59:27 2017 -0500
12483: Delay flushing collection until successful http response.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/go/httpserver/responsewriter.go b/sdk/go/httpserver/responsewriter.go
index e6ac4ca..3941c13 100644
--- a/sdk/go/httpserver/responsewriter.go
+++ b/sdk/go/httpserver/responsewriter.go
@@ -8,40 +8,46 @@ import (
"net/http"
)
-// ResponseWriter wraps http.ResponseWriter and exposes the status
+type ResponseWriter interface {
+ http.ResponseWriter
+ WroteStatus() int
+ WroteBodyBytes() int
+}
+
+// responseWriter wraps http.ResponseWriter and exposes the status
// sent, the number of bytes sent to the client, and the last write
// error.
-type ResponseWriter struct {
+type responseWriter struct {
http.ResponseWriter
- wroteStatus *int // Last status given to WriteHeader()
- wroteBodyBytes *int // Bytes successfully written
- err *error // Last error returned from Write()
+ wroteStatus int // Last status given to WriteHeader()
+ wroteBodyBytes int // Bytes successfully written
+ err error // Last error returned from Write()
}
func WrapResponseWriter(orig http.ResponseWriter) ResponseWriter {
- return ResponseWriter{orig, new(int), new(int), new(error)}
+ return &responseWriter{ResponseWriter: orig}
}
-func (w ResponseWriter) WriteHeader(s int) {
- *w.wroteStatus = s
+func (w responseWriter) WriteHeader(s int) {
+ w.wroteStatus = s
w.ResponseWriter.WriteHeader(s)
}
-func (w ResponseWriter) Write(data []byte) (n int, err error) {
+func (w responseWriter) Write(data []byte) (n int, err error) {
n, err = w.ResponseWriter.Write(data)
- *w.wroteBodyBytes += n
- *w.err = err
+ w.wroteBodyBytes += n
+ w.err = err
return
}
-func (w ResponseWriter) WroteStatus() int {
- return *w.wroteStatus
+func (w responseWriter) WroteStatus() int {
+ return w.wroteStatus
}
-func (w ResponseWriter) WroteBodyBytes() int {
- return *w.wroteBodyBytes
+func (w responseWriter) WroteBodyBytes() int {
+ return w.wroteBodyBytes
}
-func (w ResponseWriter) Err() error {
- return *w.err
+func (w responseWriter) Err() error {
+ return w.err
}
diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go
index 991e923..7c05ccc 100644
--- a/services/keep-web/cadaver_test.go
+++ b/services/keep-web/cadaver_test.go
@@ -101,12 +101,8 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
match: `(?ms).*Moving .* succeeded.*`,
},
{
- // Strangely, webdav deletes dst if you do
- // "move nonexistent dst" -- otherwise we
- // would repeat the above "move testfile
- // newdir0" here.
path: writePath,
- cmd: "move testfile nonexistentdir\n",
+ cmd: "move testfile newdir0\n",
match: `(?ms).*Moving .* failed.*`,
},
{
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index f10cb59..a30d40d 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -6,6 +6,7 @@ package main
import (
"encoding/json"
+ "errors"
"fmt"
"html"
"html/template"
@@ -96,6 +97,44 @@ func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) {
json.NewEncoder(w).Encode(status)
}
+// updateOnSuccess wraps httpserver.ResponseWriter. If the handler
+// sends an HTTP header indicating success, updateOnSuccess first
+// calls the provided update func. If the update func fails, a 500
+// response is sent, and the status code and body sent by the handler
+// are ignored (all response writes return errors).
+type updateOnSuccess struct {
+ httpserver.ResponseWriter
+ update func() error
+ sentHeader bool
+ dropBody bool
+}
+
+var errUpdateFailed = errors.New("update failed")
+
+func (uos *updateOnSuccess) Write(p []byte) (int, error) {
+ if uos.dropBody {
+ return 0, errUpdateFailed
+ }
+ if !uos.sentHeader {
+ uos.WriteHeader(http.StatusOK)
+ }
+ return uos.ResponseWriter.Write(p)
+}
+
+func (uos *updateOnSuccess) WriteHeader(code int) {
+ if !uos.sentHeader {
+ if code >= 200 && code < 400 {
+ if err := uos.update(); err != nil {
+ http.Error(uos.ResponseWriter, err.Error(), http.StatusInternalServerError)
+ uos.dropBody = true
+ return
+ }
+ }
+ uos.sentHeader = true
+ }
+ uos.ResponseWriter.WriteHeader(code)
+}
+
var (
webdavMethod = map[string]bool{
"DELETE": true,
@@ -368,17 +407,23 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
return
}
if webdavMethod[r.Method] {
- var update func() error
- if !arvadosclient.PDHMatch(targetID) {
- update = func() error {
- return h.Config.Cache.Update(client, *collection, fs)
- }
+ writing := !arvadosclient.PDHMatch(targetID)
+ if writing {
+ // Save the collection only if/when all
+ // webdav->filesystem operations succeed --
+ // and send a 500 error the modified
+ // collection can't be saved.
+ w = &updateOnSuccess{
+ ResponseWriter: w,
+ update: func() error {
+ return h.Config.Cache.Update(client, *collection, fs)
+ }}
}
h := webdav.Handler{
Prefix: "/" + strings.Join(pathParts[:stripParts], "/"),
FileSystem: &webdavFS{
- collfs: fs,
- update: update,
+ collfs: fs,
+ writing: writing,
},
LockSystem: h.webdavLS,
Logger: func(_ *http.Request, err error) {
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 1a306e3..3e12f27 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -35,8 +35,8 @@ var (
// existence automatically so sequences like "mkcol foo; put foo/bar"
// work as expected.
type webdavFS struct {
- collfs arvados.CollectionFileSystem
- update func() error
+ collfs arvados.CollectionFileSystem
+ writing bool
}
func (fs *webdavFS) makeparents(name string) {
@@ -50,69 +50,45 @@ func (fs *webdavFS) makeparents(name string) {
}
func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
- if fs.update == nil {
+ if !fs.writing {
return errReadOnly
}
name = strings.TrimRight(name, "/")
fs.makeparents(name)
- if err := fs.collfs.Mkdir(name, 0755); err != nil {
- return err
- }
- return fs.update()
+ return fs.collfs.Mkdir(name, 0755)
}
func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (f webdav.File, err error) {
writing := flag&(os.O_WRONLY|os.O_RDWR) != 0
if writing {
- if fs.update == nil {
+ if !fs.writing {
return nil, errReadOnly
}
fs.makeparents(name)
}
f, err = fs.collfs.OpenFile(name, flag, perm)
- if writing && err == nil {
- f = writingFile{File: f, update: fs.update}
- }
return
}
func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
- if err := fs.collfs.RemoveAll(name); err != nil {
- return err
- }
- return fs.update()
+ return fs.collfs.RemoveAll(name)
}
func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
- if fs.update == nil {
+ if !fs.writing {
return errReadOnly
}
fs.makeparents(newName)
- if err := fs.collfs.Rename(oldName, newName); err != nil {
- return err
- }
- return fs.update()
+ return fs.collfs.Rename(oldName, newName)
}
func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {
- if fs.update != nil {
+ if fs.writing {
fs.makeparents(name)
}
return fs.collfs.Stat(name)
}
-type writingFile struct {
- webdav.File
- update func() error
-}
-
-func (f writingFile) Close() error {
- if err := f.File.Close(); err != nil || f.update == nil {
- return err
- }
- return f.update()
-}
-
// noLockSystem implements webdav.LockSystem by returning success for
// every possible locking operation, even though it has no side
// effects such as actually locking anything. This works for a
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list