12483: Support file create/write via webdav.
authorTom Clegg <tclegg@veritasgenetics.com>
Wed, 15 Nov 2017 18:24:16 +0000 (13:24 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Sat, 18 Nov 2017 07:28:39 +0000 (02:28 -0500)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

sdk/go/arvados/client.go
sdk/go/arvados/collection.go
services/keep-web/cache.go
services/keep-web/cadaver_test.go
services/keep-web/handler.go
services/keep-web/handler_test.go
services/keep-web/webdav.go
services/keep-web/webdav_test.go [new file with mode: 0644]

index 47a953ac2c02ab4422eeec34e5c18c27fbf61947..a38d95c2e68ee90e1d9f0d41bdef2b341127fd27 100644 (file)
@@ -5,6 +5,7 @@
 package arvados
 
 import (
+       "bytes"
        "crypto/tls"
        "encoding/json"
        "fmt"
@@ -180,6 +181,10 @@ func anythingToValues(params interface{}) (url.Values, error) {
 //
 // path must not contain a query string.
 func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.Reader, params interface{}) error {
+       if body, ok := body.(io.Closer); ok {
+               // Ensure body is closed even if we error out early
+               defer body.Close()
+       }
        urlString := c.apiURL(path)
        urlValues, err := anythingToValues(params)
        if err != nil {
@@ -202,6 +207,24 @@ func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.
        return c.DoAndDecode(dst, req)
 }
 
+type resource interface {
+       resourceName() string
+}
+
+// UpdateBody returns an io.Reader suitable for use as an http.Request
+// Body for a create or update API call.
+func (c *Client) UpdateBody(rsc resource) io.Reader {
+       j, err := json.Marshal(rsc)
+       if err != nil {
+               // Return a reader that returns errors.
+               r, w := io.Pipe()
+               w.CloseWithError(err)
+               return r
+       }
+       v := url.Values{rsc.resourceName(): {string(j)}}
+       return bytes.NewBufferString(v.Encode())
+}
+
 func (c *Client) httpClient() *http.Client {
        switch {
        case c.Client != nil:
index 61bcd7fe8f367f2ca3d3d48017213bbd5335d33e..999b4e9d483454ace177cad829e90f85ddccc44c 100644 (file)
@@ -30,6 +30,10 @@ type Collection struct {
        IsTrashed              bool       `json:"is_trashed,omitempty"`
 }
 
+func (c Collection) resourceName() string {
+       return "collection"
+}
+
 // SizedDigests returns the hash+size part of each data block
 // referenced by the collection.
 func (c *Collection) SizedDigests() ([]SizedDigest, error) {
index ce1168acd2c1d07bcd6e8623c5421bcbe905f04c..9ee99903c8d1e537d487a67d1c77d848fc93c807 100644 (file)
@@ -86,6 +86,29 @@ func (c *cache) Stats() cacheStats {
        }
 }
 
+// Update saves a modified version (fs) to an existing collection
+// (coll) and, if successful, updates the relevant cache entries so
+// subsequent calls to Get() reflect the modifications.
+func (c *cache) Update(client *arvados.Client, coll arvados.Collection, fs arvados.CollectionFileSystem) error {
+       c.setupOnce.Do(c.setup)
+
+       if m, err := fs.MarshalManifest("."); err != nil || m == coll.ManifestText {
+               return err
+       } else {
+               coll.ManifestText = m
+       }
+       var updated arvados.Collection
+       defer c.pdhs.Remove(coll.UUID)
+       err := client.RequestAndDecode(&updated, "PATCH", "/arvados/v1/collections/"+coll.UUID, client.UpdateBody(coll), nil)
+       if err == nil {
+               c.collections.Add(client.AuthToken+"\000"+coll.PortableDataHash, &cachedCollection{
+                       expire:     time.Now().Add(time.Duration(c.TTL)),
+                       collection: &updated,
+               })
+       }
+       return err
+}
+
 func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceReload bool) (*arvados.Collection, error) {
        c.setupOnce.Do(c.setup)
 
index 87a712f04c4f2d381dfcd3dccb5cbdc93680eaf1..eab4a70688b03ac16912498bc1baa06a76dda1a8 100644 (file)
@@ -7,42 +7,99 @@ package main
 import (
        "bytes"
        "io"
+       "io/ioutil"
+       "net/url"
+       "os"
        "os/exec"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        check "gopkg.in/check.v1"
 )
 
 func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
-       basePath := "/c=" + arvadostest.FooAndBarFilesInDirUUID + "/t=" + arvadostest.ActiveToken + "/"
+       testdata := []byte("the human tragedy consists in the necessity of living with the consequences of actions performed under the pressure of compulsions we do not understand")
+
+       localfile, err := ioutil.TempFile("", "localfile")
+       c.Assert(err, check.IsNil)
+       defer os.Remove(localfile.Name())
+       localfile.Write(testdata)
+
+       emptyfile, err := ioutil.TempFile("", "emptyfile")
+       c.Assert(err, check.IsNil)
+       defer os.Remove(emptyfile.Name())
+
+       checkfile, err := ioutil.TempFile("", "checkfile")
+       c.Assert(err, check.IsNil)
+       defer os.Remove(checkfile.Name())
+
+       var newCollection arvados.Collection
+       arv := arvados.NewClientFromEnv()
+       arv.AuthToken = arvadostest.ActiveToken
+       err = arv.RequestAndDecode(&newCollection, "POST", "/arvados/v1/collections", bytes.NewBufferString(url.Values{"collection": {"{}"}}.Encode()), nil)
+       c.Assert(err, check.IsNil)
+       writePath := "/c=" + newCollection.UUID + "/t=" + arv.AuthToken + "/"
+
+       readPath := "/c=" + arvadostest.FooAndBarFilesInDirUUID + "/t=" + arvadostest.ActiveToken + "/"
        type testcase struct {
                path  string
                cmd   string
                match string
+               data  []byte
        }
        for _, trial := range []testcase{
                {
-                       path:  basePath,
+                       path:  readPath,
                        cmd:   "ls\n",
                        match: `(?ms).*dir1 *0 .*`,
                },
                {
-                       path:  basePath,
+                       path:  readPath,
                        cmd:   "ls dir1\n",
                        match: `(?ms).*bar *3.*foo *3 .*`,
                },
                {
-                       path:  basePath + "_/dir1",
+                       path:  readPath + "_/dir1",
                        cmd:   "ls\n",
                        match: `(?ms).*bar *3.*foo *3 .*`,
                },
                {
-                       path:  basePath + "dir1/",
+                       path:  readPath + "dir1/",
                        cmd:   "ls\n",
                        match: `(?ms).*bar *3.*foo *3 .*`,
                },
+               {
+                       path:  writePath,
+                       cmd:   "get emptyfile '" + checkfile.Name() + "'\n",
+                       match: `(?ms).*Not Found.*`,
+               },
+               {
+                       path:  writePath,
+                       cmd:   "put '" + emptyfile.Name() + "' emptyfile\n",
+                       match: `(?ms).*Uploading .* succeeded.*`,
+               },
+               {
+                       path:  writePath,
+                       cmd:   "get emptyfile '" + checkfile.Name() + "'\n",
+                       match: `(?ms).*Downloading .* succeeded.*`,
+                       data:  []byte{},
+               },
+               {
+                       path:  writePath,
+                       cmd:   "put '" + localfile.Name() + "' testfile\n",
+                       match: `(?ms).*Uploading .* succeeded.*`,
+               },
+               {
+                       path:  writePath,
+                       cmd:   "get testfile '" + checkfile.Name() + "'\n",
+                       match: `(?ms).*succeeded.*`,
+                       data:  testdata,
+               },
        } {
-               c.Logf("%s %#v", "http://"+s.testServer.Addr, trial)
+               c.Logf("%s %+v", "http://"+s.testServer.Addr, trial)
+
+               os.Remove(checkfile.Name())
+
                cmd := exec.Command("cadaver", "http://"+s.testServer.Addr+trial.path)
                cmd.Stdin = bytes.NewBufferString(trial.cmd)
                stdout, err := cmd.StdoutPipe()
@@ -56,5 +113,15 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
                err = cmd.Wait()
                c.Check(err, check.Equals, nil)
                c.Check(buf.String(), check.Matches, trial.match)
+
+               if trial.data == nil {
+                       continue
+               }
+               checkfile, err = os.Open(checkfile.Name())
+               c.Assert(err, check.IsNil)
+               checkfile.Seek(0, os.SEEK_SET)
+               got, err := ioutil.ReadAll(checkfile)
+               c.Check(got, check.DeepEquals, trial.data)
+               c.Check(err, check.IsNil)
        }
 }
index b4f3062c80e3ba21eb458bee4c73e8ce8539828a..42f6c513bbd98b52b723871cb3f5f5ba17aca66f 100644 (file)
@@ -100,6 +100,7 @@ var (
        webdavMethod = map[string]bool{
                "OPTIONS":  true,
                "PROPFIND": true,
+               "PUT":      true,
        }
        browserMethod = map[string]bool{
                "GET":  true,
@@ -147,7 +148,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
                        return
                }
                w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type, Range")
-               w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS, PROPFIND")
+               w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS, PROPFIND, PUT")
                w.Header().Set("Access-Control-Allow-Origin", "*")
                w.Header().Set("Access-Control-Max-Age", "86400")
                statusCode = http.StatusOK
@@ -352,19 +353,29 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
        }
        applyContentDispositionHdr(w, r, basename, attachment)
 
-       fs, err := collection.FileSystem(&arvados.Client{
+       client := &arvados.Client{
                APIHost:   arv.ApiServer,
                AuthToken: arv.ApiToken,
                Insecure:  arv.ApiInsecure,
-       }, kc)
+       }
+       fs, err := collection.FileSystem(client, kc)
        if err != nil {
                statusCode, statusText = http.StatusInternalServerError, err.Error()
                return
        }
        if webdavMethod[r.Method] {
+               var update func() error
+               if !arvadosclient.PDHMatch(targetID) {
+                       update = func() error {
+                               return h.Config.Cache.Update(client, *collection, fs)
+                       }
+               }
                h := webdav.Handler{
-                       Prefix:     "/" + strings.Join(pathParts[:stripParts], "/"),
-                       FileSystem: &webdavFS{collfs: fs},
+                       Prefix: "/" + strings.Join(pathParts[:stripParts], "/"),
+                       FileSystem: &webdavFS{
+                               collfs: fs,
+                               update: update,
+                       },
                        LockSystem: h.webdavLS,
                        Logger: func(_ *http.Request, err error) {
                                if os.IsNotExist(err) {
index 6bd34d71130aaeefd4984e835f8285c01c6899ad..32174cb149cbf17631114fcdd8c4fd19b9fb70cc 100644 (file)
@@ -45,7 +45,7 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
        c.Check(resp.Code, check.Equals, http.StatusOK)
        c.Check(resp.Body.String(), check.Equals, "")
        c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*")
-       c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "GET, POST, OPTIONS, PROPFIND")
+       c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "GET, POST, OPTIONS, PROPFIND, PUT")
        c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range")
 
        // Check preflight for a disallowed request
index 57f3f53a99ef6a73944c6cb600c1672c67cb6696..87d9c47f1430e9a8c028ff35c0eb2b16e6772d82 100644 (file)
@@ -9,9 +9,7 @@ import (
        "errors"
        "fmt"
        prand "math/rand"
-       "net/http"
        "os"
-       "sync"
        "sync/atomic"
        "time"
 
@@ -27,24 +25,27 @@ var (
        errReadOnly           = errors.New("read-only filesystem")
 )
 
-// webdavFS implements a read-only webdav.FileSystem by wrapping an
+// webdavFS implements a webdav.FileSystem by wrapping an
 // arvados.CollectionFilesystem.
 type webdavFS struct {
        collfs arvados.CollectionFileSystem
+       update func() error
 }
 
-var _ webdav.FileSystem = &webdavFS{}
-
 func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
-       return errReadOnly
+       return fs.collfs.Mkdir(name, 0755)
 }
 
-func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (webdav.File, error) {
-       fi, err := fs.collfs.Stat(name)
-       if err != nil {
-               return nil, err
+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 && fs.update == nil {
+               return nil, errReadOnly
+       }
+       f, err = fs.collfs.OpenFile(name, flag, perm)
+       if writing && err == nil {
+               f = writingFile{File: f, update: fs.update}
        }
-       return &webdavFile{collfs: fs.collfs, fileInfo: fi, name: name}, nil
+       return
 }
 
 func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
@@ -59,71 +60,16 @@ func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error)
        return fs.collfs.Stat(name)
 }
 
-// webdavFile implements a read-only webdav.File by wrapping
-// http.File.
-//
-// The http.File is opened from an arvados.CollectionFileSystem, but
-// not until Seek, Read, or Readdir is called. This deferred-open
-// strategy makes webdav's OpenFile-Stat-Close cycle fast even though
-// the collfs's Open method is slow. This is relevant because webdav
-// does OpenFile-Stat-Close on each file when preparing directory
-// listings.
-//
-// Writes to a webdavFile always fail.
-type webdavFile struct {
-       // fields populated by (*webdavFS).OpenFile()
-       collfs   http.FileSystem
-       fileInfo os.FileInfo
-       name     string
-
-       // internal fields
-       file     http.File
-       loadOnce sync.Once
-       err      error
-}
-
-func (f *webdavFile) load() {
-       f.file, f.err = f.collfs.Open(f.name)
-}
-
-func (f *webdavFile) Write([]byte) (int, error) {
-       return 0, errReadOnly
-}
-
-func (f *webdavFile) Seek(offset int64, whence int) (int64, error) {
-       f.loadOnce.Do(f.load)
-       if f.err != nil {
-               return 0, f.err
-       }
-       return f.file.Seek(offset, whence)
-}
-
-func (f *webdavFile) Read(buf []byte) (int, error) {
-       f.loadOnce.Do(f.load)
-       if f.err != nil {
-               return 0, f.err
-       }
-       return f.file.Read(buf)
-}
-
-func (f *webdavFile) Close() error {
-       if f.file == nil {
-               // We never called load(), or load() failed
-               return f.err
-       }
-       return f.file.Close()
+type writingFile struct {
+       webdav.File
+       update func() error
 }
 
-func (f *webdavFile) Readdir(n int) ([]os.FileInfo, error) {
-       f.loadOnce.Do(f.load)
-       if f.err != nil {
-               return nil, f.err
+func (f writingFile) Close() error {
+       if err := f.File.Close(); err != nil || f.update == nil {
+               return err
        }
-       return f.file.Readdir(n)
-}
-
-func (f *webdavFile) Stat() (os.FileInfo, error) {
-       return f.fileInfo, nil
+       return f.update()
 }
 
 // noLockSystem implements webdav.LockSystem by returning success for
diff --git a/services/keep-web/webdav_test.go b/services/keep-web/webdav_test.go
new file mode 100644 (file)
index 0000000..52db776
--- /dev/null
@@ -0,0 +1,5 @@
+package main
+
+import "golang.org/x/net/webdav"
+
+var _ webdav.FileSystem = &webdavFS{}