[ARVADOS] updated: 1.1.0-162-g11127c5
Git user
git at public.curoverse.com
Mon Nov 20 18:49:10 EST 2017
Summary of changes:
sdk/go/arvados/collection_fs.go | 79 ++++++++++++++++++++++--------------
sdk/go/arvados/collection_fs_test.go | 10 +++--
services/keep-web/cadaver_test.go | 65 +++++++++++++++++++++++++++++
services/keep-web/handler.go | 5 ++-
services/keep-web/handler_test.go | 4 +-
services/keep-web/webdav.go | 48 +++++++++++++++++++---
6 files changed, 168 insertions(+), 43 deletions(-)
via 11127c5b67a10a46f60c1c1c53a2c2639b7914e1 (commit)
from 127e916894bcd16f6978aa6488c81e79a9ca2812 (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 11127c5b67a10a46f60c1c1c53a2c2639b7914e1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Mon Nov 20 18:43:46 2017 -0500
12483: Make webdav rename/remove work. Tidy up code.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 4c97af5..3f5fb5e 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -735,8 +735,7 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
sort.Strings(names)
for _, name := range names {
- node := dn.inodes[name]
- switch node := node.(type) {
+ switch node := dn.inodes[name].(type) {
case *dirnode:
subdir, err := node.marshalManifest(prefix + "/" + name)
if err != nil {
@@ -793,14 +792,14 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
}
func (dn *dirnode) loadManifest(txt string) error {
- // FIXME: faster
var dirname string
streams := strings.Split(txt, "\n")
if streams[len(streams)-1] != "" {
return fmt.Errorf("line %d: no trailing newline", len(streams))
}
- var extents []storedExtent
- for i, stream := range streams[:len(streams)-1] {
+ streams = streams[:len(streams)-1]
+ extents := []storedExtent{}
+ for i, stream := range streams {
lineno := i + 1
var anyFileTokens bool
var pos int64
@@ -863,7 +862,7 @@ func (dn *dirnode) loadManifest(txt string) error {
// situation might be rare anyway)
extIdx, pos = 0, 0
}
- for next := int64(0); extIdx < len(extents); extIdx, pos = extIdx+1, next {
+ for next := int64(0); extIdx < len(extents); extIdx++ {
e := extents[extIdx]
next = pos + int64(e.Len())
if next <= offset || e.Len() == 0 {
@@ -890,6 +889,8 @@ func (dn *dirnode) loadManifest(txt string) error {
})
if next > offset+length {
break
+ } else {
+ pos = next
}
}
if extIdx == len(extents) && pos < offset+length {
@@ -910,36 +911,35 @@ func (dn *dirnode) loadManifest(txt string) error {
// only safe to call from loadManifest -- no locking
func (dn *dirnode) createFileAndParents(path string) (fn *filenode, err error) {
names := strings.Split(path, "/")
- if basename := names[len(names)-1]; basename == "" || basename == "." || basename == ".." {
+ basename := names[len(names)-1]
+ if basename == "" || basename == "." || basename == ".." {
err = fmt.Errorf("invalid filename")
return
}
- var node inode = dn
- for i, name := range names {
- dn, ok := node.(*dirnode)
- if !ok {
- err = ErrFileExists
- return
- }
- if name == "" || name == "." {
- continue
- }
- if name == ".." {
- node = dn.parent
- continue
- }
- node, ok = dn.inodes[name]
- if !ok {
- if i == len(names)-1 {
- fn = dn.newFilenode(name, 0755)
+ for _, name := range names[:len(names)-1] {
+ switch name {
+ case "", ".":
+ case "..":
+ dn = dn.parent
+ default:
+ switch node := dn.inodes[name].(type) {
+ case nil:
+ dn = dn.newDirnode(name, 0755)
+ case *dirnode:
+ dn = node
+ case *filenode:
+ err = ErrFileExists
return
}
- node = dn.newDirnode(name, 0755)
}
}
- var ok bool
- if fn, ok = node.(*filenode); !ok {
- err = ErrInvalidArgument
+ switch node := dn.inodes[basename].(type) {
+ case nil:
+ fn = dn.newFilenode(basename, 0755)
+ case *filenode:
+ fn = node
+ case *dirnode:
+ err = ErrIsDirectory
}
return
}
@@ -957,11 +957,17 @@ func (dn *dirnode) Mkdir(name string, perm os.FileMode) error {
}
func (dn *dirnode) Remove(name string) error {
- return dn.remove(name, false)
+ return dn.remove(strings.TrimRight(name, "/"), false)
}
func (dn *dirnode) RemoveAll(name string) error {
- return dn.remove(name, true)
+ err := dn.remove(strings.TrimRight(name, "/"), true)
+ if os.IsNotExist(err) {
+ // "If the path does not exist, RemoveAll returns
+ // nil." (see "os" pkg)
+ err = nil
+ }
+ return err
}
func (dn *dirnode) remove(name string, recursive bool) error {
@@ -1054,9 +1060,20 @@ func (dn *dirnode) Rename(oldname, newname string) error {
}
}
} else {
+ if newdn.inodes == nil {
+ newdn.inodes = make(map[string]inode)
+ }
newdn.fileinfo.size++
}
newdn.inodes[newname] = oldinode
+ switch n := oldinode.(type) {
+ case *dirnode:
+ n.parent = newdn
+ case *filenode:
+ n.parent = newdn
+ default:
+ panic(fmt.Sprintf("bad inode type %T", n))
+ }
delete(olddn.inodes, oldname)
olddn.fileinfo.size--
return nil
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 07d00be..dabb884 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -560,14 +560,14 @@ func (s *CollectionFSSuite) TestRemove(c *check.C) {
c.Assert(err, check.IsNil)
err = fs.Mkdir("dir1/dir2", 0755)
c.Assert(err, check.IsNil)
+ err = fs.Mkdir("dir1/dir3", 0755)
+ c.Assert(err, check.IsNil)
err = fs.Remove("dir0")
c.Check(err, check.IsNil)
err = fs.Remove("dir0")
c.Check(err, check.Equals, os.ErrNotExist)
- err = fs.Remove("dir1/dir2/")
- c.Check(err, check.Equals, ErrInvalidArgument)
err = fs.Remove("dir1/dir2/.")
c.Check(err, check.Equals, ErrInvalidArgument)
err = fs.Remove("dir1/dir2/..")
@@ -576,10 +576,12 @@ func (s *CollectionFSSuite) TestRemove(c *check.C) {
c.Check(err, check.Equals, ErrDirectoryNotEmpty)
err = fs.Remove("dir1/dir2/../../../dir1")
c.Check(err, check.Equals, ErrDirectoryNotEmpty)
+ err = fs.Remove("dir1/dir3/")
+ c.Check(err, check.IsNil)
err = fs.RemoveAll("dir1")
c.Check(err, check.IsNil)
err = fs.RemoveAll("dir1")
- c.Check(err, check.Equals, os.ErrNotExist)
+ c.Check(err, check.IsNil)
}
func (s *CollectionFSSuite) TestRename(c *check.C) {
@@ -630,7 +632,7 @@ func (s *CollectionFSSuite) TestRename(c *check.C) {
// oldname does not exist
err := fs.Rename(
fmt.Sprintf("dir%d/dir%d/missing", i, j),
- fmt.Sprintf("dir%d/irelevant", outer-i-1))
+ fmt.Sprintf("dir%d/dir%d/file%d", outer-i-1, j, j))
c.Check(err, check.ErrorMatches, `.*does not exist`)
// newname parent dir does not exist
diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go
index eab4a70..991e923 100644
--- a/services/keep-web/cadaver_test.go
+++ b/services/keep-web/cadaver_test.go
@@ -95,6 +95,71 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
match: `(?ms).*succeeded.*`,
data: testdata,
},
+ {
+ path: writePath,
+ cmd: "move testfile newdir0\n",
+ 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",
+ match: `(?ms).*Moving .* failed.*`,
+ },
+ {
+ path: writePath,
+ cmd: "ls\n",
+ match: `(?ms).*newdir0.* 0 .*`,
+ },
+ {
+ path: writePath,
+ cmd: "move newdir0/testfile emptyfile/bogus/\n",
+ match: `(?ms).*Moving .* failed.*`,
+ },
+ {
+ path: writePath,
+ cmd: "mkcol newdir1\n",
+ match: `(?ms).*Creating .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "move newdir0/testfile newdir1\n",
+ match: `(?ms).*Moving .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "put '" + localfile.Name() + "' newdir1/testfile1\n",
+ match: `(?ms).*Uploading .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "mkcol newdir2\n",
+ match: `(?ms).*Creating .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "put '" + localfile.Name() + "' newdir2/testfile2\n",
+ match: `(?ms).*Uploading .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "get newdir2/testfile2 '" + checkfile.Name() + "'\n",
+ match: `(?ms).*succeeded.*`,
+ data: testdata,
+ },
+ {
+ path: writePath,
+ cmd: "rmcol newdir2\n",
+ match: `(?ms).*Deleting collection .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "get newdir2/testfile2 '" + checkfile.Name() + "'\n",
+ match: `(?ms).*Downloading .* failed.*`,
+ },
} {
c.Logf("%s %+v", "http://"+s.testServer.Addr, trial)
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index f6c12c3..f10cb59 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -98,10 +98,13 @@ func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) {
var (
webdavMethod = map[string]bool{
+ "DELETE": true,
+ "MKCOL": true,
"MOVE": true,
"OPTIONS": true,
"PROPFIND": true,
"PUT": true,
+ "RMCOL": true,
}
browserMethod = map[string]bool{
"GET": true,
@@ -149,7 +152,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, MOVE, OPTIONS, POST, PROPFIND, PUT")
+ w.Header().Set("Access-Control-Allow-Methods", "DELETE, GET, MKCOL, MOVE, OPTIONS, POST, PROPFIND, PUT, RMCOL")
w.Header().Set("Access-Control-Allow-Origin", "*")
w.Header().Set("Access-Control-Max-Age", "86400")
statusCode = http.StatusOK
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 6add804..85a8e50 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -45,12 +45,12 @@ 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, MOVE, OPTIONS, POST, PROPFIND, PUT")
+ c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "DELETE, GET, MKCOL, MOVE, OPTIONS, POST, PROPFIND, PUT, RMCOL")
c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range")
// Check preflight for a disallowed request
resp = httptest.NewRecorder()
- req.Header.Set("Access-Control-Request-Method", "DELETE")
+ req.Header.Set("Access-Control-Request-Method", "MAKE-COFFEE")
h.ServeHTTP(resp, req)
c.Check(resp.Body.String(), check.Equals, "")
c.Check(resp.Code, check.Equals, http.StatusMethodNotAllowed)
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 7798a69..1a306e3 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -10,6 +10,8 @@ import (
"fmt"
prand "math/rand"
"os"
+ "path"
+ "strings"
"sync/atomic"
"time"
@@ -27,19 +29,45 @@ var (
// webdavFS implements a webdav.FileSystem by wrapping an
// arvados.CollectionFilesystem.
+//
+// Collections don't preserve empty directories, so Mkdir is
+// effectively a no-op, and we need to make parent dirs spring into
+// existence automatically so sequences like "mkcol foo; put foo/bar"
+// work as expected.
type webdavFS struct {
collfs arvados.CollectionFileSystem
update func() error
}
+func (fs *webdavFS) makeparents(name string) {
+ dir, name := path.Split(name)
+ if dir == "" || dir == "/" {
+ return
+ }
+ dir = dir[:len(dir)-1]
+ fs.makeparents(dir)
+ fs.collfs.Mkdir(dir, 0755)
+}
+
func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
- return fs.collfs.Mkdir(name, 0755)
+ if fs.update == nil {
+ return errReadOnly
+ }
+ name = strings.TrimRight(name, "/")
+ fs.makeparents(name)
+ if err := fs.collfs.Mkdir(name, 0755); err != nil {
+ return err
+ }
+ return fs.update()
}
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
+ if writing {
+ if fs.update == nil {
+ return nil, errReadOnly
+ }
+ fs.makeparents(name)
}
f, err = fs.collfs.OpenFile(name, flag, perm)
if writing && err == nil {
@@ -49,17 +77,27 @@ func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os
}
func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
- return fs.collfs.RemoveAll(name)
+ if err := fs.collfs.RemoveAll(name); err != nil {
+ return err
+ }
+ return fs.update()
}
func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
if fs.update == nil {
return errReadOnly
}
- return fs.collfs.Rename(oldName, newName)
+ fs.makeparents(newName)
+ if err := fs.collfs.Rename(oldName, newName); err != nil {
+ return err
+ }
+ return fs.update()
}
func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {
+ if fs.update != nil {
+ fs.makeparents(name)
+ }
return fs.collfs.Stat(name)
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list