13111: Clear up some type assertions.
authorTom Clegg <tclegg@veritasgenetics.com>
Wed, 20 Dec 2017 22:11:07 +0000 (17:11 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Wed, 3 Jan 2018 05:24:19 +0000 (00:24 -0500)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

sdk/go/arvados/fs_base.go
sdk/go/arvados/fs_collection.go
sdk/go/arvados/fs_site.go

index 8d987d4cacd06a9dd7bbd8870a4f5bc8d9e67e81..fe185a4874d42745fc26afb9f9da3f0efd6eed95 100644 (file)
@@ -51,6 +51,7 @@ type FileSystem interface {
        http.FileSystem
        fsBackend
 
+       rootnode() inode
        newDirnode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error)
        newFilenode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error)
 
@@ -83,6 +84,7 @@ type FileSystem interface {
 }
 
 type inode interface {
+       SetParent(inode)
        Parent() inode
        FS() FileSystem
        Read([]byte, filenodePtr) (int, filenodePtr, error)
@@ -204,6 +206,12 @@ func (n *treenode) FS() FileSystem {
        return n.fs
 }
 
+func (n *treenode) SetParent(p inode) {
+       n.RLock()
+       defer n.RUnlock()
+       n.parent = p
+}
+
 func (n *treenode) Parent() inode {
        n.RLock()
        defer n.RUnlock()
@@ -218,11 +226,13 @@ func (n *treenode) Child(name string, replace func(inode) inode) (child inode) {
        // TODO: special treatment for "", ".", ".."
        child = n.inodes[name]
        if replace != nil {
-               child = replace(child)
-               if child == nil {
+               newchild := replace(child)
+               if newchild == nil {
                        delete(n.inodes, name)
-               } else {
-                       n.inodes[name] = child
+               } else if newchild != child {
+                       n.inodes[name] = newchild
+                       newchild.SetParent(n)
+                       child = newchild
                }
        }
        return
@@ -254,6 +264,10 @@ type fileSystem struct {
        fsBackend
 }
 
+func (fs *fileSystem) rootnode() inode {
+       return fs.root
+}
+
 // OpenFile is analogous to os.OpenFile().
 func (fs *fileSystem) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
        return fs.openFile(name, flag, perm)
@@ -306,9 +320,9 @@ func (fs *fileSystem) openFile(name string, flag int, perm os.FileMode) (*fileha
                var err error
                n = parent.Child(name, func(inode) inode {
                        if perm.IsDir() {
-                               n, err = fs.newDirnode(parent, name, perm|0755, time.Now())
+                               n, err = parent.FS().newDirnode(parent, name, perm|0755, time.Now())
                        } else {
-                               n, err = fs.newFilenode(parent, name, perm|0755, time.Now())
+                               n, err = parent.FS().newFilenode(parent, name, perm|0755, time.Now())
                        }
                        return n
                })
@@ -357,7 +371,7 @@ func (fs *fileSystem) Mkdir(name string, perm os.FileMode) (err error) {
                return os.ErrExist
        }
        child := n.Child(name, func(inode) (child inode) {
-               child, err = fs.newDirnode(n, name, perm, time.Now())
+               child, err = n.FS().newDirnode(n, name, perm, time.Now())
                return
        })
        if err != nil {
@@ -404,12 +418,12 @@ func (fs *fileSystem) Rename(oldname, newname string) error {
 
        // When acquiring locks on multiple nodes, all common
        // ancestors must be locked first in order to avoid
-       // deadlock. This is assured by locking the path from root to
-       // newdir, then locking the path from root to olddir, skipping
-       // any already-locked nodes.
+       // deadlock. This is assured by locking the path from
+       // filesystem root to newdir, then locking the path from
+       // filesystem root to olddir, skipping any already-locked
+       // nodes.
        needLock := []sync.Locker{}
-       for _, f := range []*filehandle{olddirf, newdirf} {
-               node := f.inode
+       for _, node := range []inode{olddirf.inode, newdirf.inode} {
                needLock = append(needLock, node)
                for node.Parent() != node && node.Parent().FS() == node.FS() {
                        node = node.Parent()
@@ -499,41 +513,6 @@ func (fs *fileSystem) remove(name string, recursive bool) (err error) {
        return err
 }
 
-// Caller must have parent lock, and must have already ensured
-// parent.Child(name,nil) is nil.
-func (fs *fileSystem) newDirnode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
-       if name == "" || name == "." || name == ".." {
-               return nil, ErrInvalidArgument
-       }
-       return &dirnode{
-               treenode: treenode{
-                       fs:     parent.FS(),
-                       parent: parent,
-                       fileinfo: fileinfo{
-                               name:    name,
-                               mode:    perm | os.ModeDir,
-                               modTime: modTime,
-                       },
-                       inodes: make(map[string]inode),
-               },
-       }, nil
-}
-
-func (fs *fileSystem) newFilenode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
-       if name == "" || name == "." || name == ".." {
-               return nil, ErrInvalidArgument
-       }
-       return &filenode{
-               fs:     parent.FS(),
-               parent: parent,
-               fileinfo: fileinfo{
-                       name:    name,
-                       mode:    perm & ^os.ModeDir,
-                       modTime: modTime,
-               },
-       }, nil
-}
-
 func (fs *fileSystem) Sync() error {
        log.Printf("TODO: sync fileSystem")
        return ErrInvalidOperation
index fc00335ce5eca8a20d8e4db03384be71417a671b..36a92aff7edf94fa85ea1027e540d312cb6399b6 100644 (file)
@@ -64,14 +64,14 @@ func (c *Collection) FileSystem(client apiClient, kc keepClient) (CollectionFile
                modTime = *c.ModifiedAt
        }
        fs := &collectionFileSystem{
+               uuid: c.UUID,
                fileSystem: fileSystem{
                        fsBackend: keepBackend{apiClient: client, keepClient: kc},
                },
-               uuid: c.UUID,
        }
-       dn := &dirnode{
+       root := &dirnode{
+               fs: fs,
                treenode: treenode{
-                       fs: fs,
                        fileinfo: fileinfo{
                                name:    ".",
                                mode:    os.ModeDir | 0755,
@@ -80,11 +80,11 @@ func (c *Collection) FileSystem(client apiClient, kc keepClient) (CollectionFile
                        inodes: make(map[string]inode),
                },
        }
-       dn.parent = dn
-       fs.fileSystem.root = dn
-       if err := dn.loadManifest(c.ManifestText); err != nil {
+       root.SetParent(root)
+       if err := root.loadManifest(c.ManifestText); err != nil {
                return nil, err
        }
+       fs.root = root
        return fs, nil
 }
 
@@ -93,6 +93,39 @@ type collectionFileSystem struct {
        uuid string
 }
 
+// Caller must have parent lock, and must have already ensured
+// parent.Child(name,nil) is nil.
+func (fs *collectionFileSystem) newDirnode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
+       if name == "" || name == "." || name == ".." {
+               return nil, ErrInvalidArgument
+       }
+       return &dirnode{
+               fs: fs,
+               treenode: treenode{
+                       fileinfo: fileinfo{
+                               name:    name,
+                               mode:    perm | os.ModeDir,
+                               modTime: modTime,
+                       },
+                       inodes: make(map[string]inode),
+               },
+       }, nil
+}
+
+func (fs *collectionFileSystem) newFilenode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
+       if name == "" || name == "." || name == ".." {
+               return nil, ErrInvalidArgument
+       }
+       return &filenode{
+               fs: fs,
+               fileinfo: fileinfo{
+                       name:    name,
+                       mode:    perm & ^os.ModeDir,
+                       modTime: modTime,
+               },
+       }, nil
+}
+
 func (fs *collectionFileSystem) Sync() error {
        log.Printf("cfs.Sync()")
        if fs.uuid == "" {
@@ -114,12 +147,12 @@ func (fs *collectionFileSystem) Sync() error {
        return err
 }
 
-func (fs *collectionFileSystem) Child(name string, replace func(inode) inode) inode {
-       if name == ".arvados#collection" {
-               return &getternode{Getter: func() ([]byte, error) {
+func (dn *dirnode) Child(name string, replace func(inode) inode) inode {
+       if dn == dn.fs.rootnode() && name == ".arvados#collection" {
+               gn := &getternode{Getter: func() ([]byte, error) {
                        var coll Collection
                        var err error
-                       coll.ManifestText, err = fs.MarshalManifest(".")
+                       coll.ManifestText, err = dn.fs.MarshalManifest(".")
                        if err != nil {
                                return nil, err
                        }
@@ -129,8 +162,10 @@ func (fs *collectionFileSystem) Child(name string, replace func(inode) inode) in
                        }
                        return data, err
                }}
+               gn.SetParent(dn)
+               return gn
        }
-       return fs.fileSystem.root.Child(name, replace)
+       return dn.treenode.Child(name, replace)
 }
 
 func (fs *collectionFileSystem) MarshalManifest(prefix string) (string, error) {
@@ -232,6 +267,12 @@ func (fn *filenode) appendSegment(e segment) {
        fn.fileinfo.size += int64(e.Len())
 }
 
+func (fn *filenode) SetParent(p inode) {
+       fn.RLock()
+       defer fn.RUnlock()
+       fn.parent = p
+}
+
 func (fn *filenode) Parent() inode {
        fn.RLock()
        defer fn.RUnlock()
@@ -496,7 +537,7 @@ func (fn *filenode) pruneMemSegments() {
                }
                fn.memsize -= int64(seg.Len())
                fn.segments[idx] = storedSegment{
-                       kc:      fn.parent.(fsBackend),
+                       kc:      fn.parent.FS(),
                        locator: locator,
                        size:    seg.Len(),
                        offset:  0,
@@ -506,9 +547,14 @@ func (fn *filenode) pruneMemSegments() {
 }
 
 type dirnode struct {
+       fs *collectionFileSystem
        treenode
 }
 
+func (dn *dirnode) FS() FileSystem {
+       return dn.fs
+}
+
 // sync flushes in-memory data (for all files in the tree rooted at
 // dn) to persistent storage. Caller must hold dn.Lock().
 func (dn *dirnode) sync() error {
@@ -801,7 +847,7 @@ func (dn *dirnode) createFileAndParents(path string) (fn *filenode, err error) {
                                // can't be sure parent will be a *dirnode
                                return nil, ErrInvalidArgument
                        }
-                       node = node.Parent().(*dirnode)
+                       node = node.Parent()
                        continue
                }
                node.Child(name, func(child inode) inode {
@@ -832,7 +878,7 @@ func (dn *dirnode) createFileAndParents(path string) (fn *filenode, err error) {
                        err = ErrIsDirectory
                        return child
                default:
-                       err = ErrInvalidOperation
+                       err = ErrInvalidArgument
                        return child
                }
        })
index 53261317b1a321f85a1e7e05f38254f1e54d1fa5..fd842f0968717b362cd38b9d5f50e0fd0062c7ca 100644 (file)
@@ -9,17 +9,21 @@ import (
        "time"
 )
 
+type siteFileSystem struct {
+       fileSystem
+}
+
 // SiteFileSystem returns a FileSystem that maps collections and other
 // Arvados objects onto a filesystem layout.
 //
 // This is experimental: the filesystem layout is not stable, and
 // there are significant known bugs and shortcomings. For example,
-// although the FileSystem allows files to be added and modified in
-// collections, these changes are not persistent or visible to other
-// Arvados clients.
+// writes are not persisted until Sync() is called.
 func (c *Client) SiteFileSystem(kc keepClient) FileSystem {
-       fs := &fileSystem{
-               fsBackend: keepBackend{apiClient: c, keepClient: kc},
+       fs := &siteFileSystem{
+               fileSystem: fileSystem{
+                       fsBackend: keepBackend{apiClient: c, keepClient: kc},
+               },
        }
        root := &treenode{
                fs: fs,
@@ -34,7 +38,7 @@ func (c *Client) SiteFileSystem(kc keepClient) FileSystem {
        root.Child("by_id", func(inode) inode {
                var vn inode
                vn = &vdirnode{
-                       treenode: treenode{
+                       inode: &treenode{
                                fs:     fs,
                                parent: root,
                                inodes: make(map[string]inode),
@@ -44,9 +48,7 @@ func (c *Client) SiteFileSystem(kc keepClient) FileSystem {
                                        mode:    0755 | os.ModeDir,
                                },
                        },
-                       create: func(name string) inode {
-                               return newEntByID(vn, name)
-                       },
+                       create: fs.mountCollection,
                }
                return vn
        })
@@ -54,33 +56,51 @@ func (c *Client) SiteFileSystem(kc keepClient) FileSystem {
        return fs
 }
 
-func newEntByID(parent inode, id string) inode {
+func (fs *siteFileSystem) newDirnode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
+       return nil, ErrInvalidOperation
+}
+
+func (fs *siteFileSystem) newFilenode(parent inode, name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
+       return nil, ErrInvalidOperation
+}
+
+func (fs *siteFileSystem) mountCollection(parent inode, id string) inode {
        var coll Collection
-       err := parent.FS().RequestAndDecode(&coll, "GET", "arvados/v1/collections/"+id, nil, nil)
+       err := fs.RequestAndDecode(&coll, "GET", "arvados/v1/collections/"+id, nil, nil)
        if err != nil {
                return nil
        }
-       fs, err := coll.FileSystem(parent.FS(), parent.FS())
+       cfs, err := coll.FileSystem(fs, fs)
        if err != nil {
                return nil
        }
-       root := fs.(*collectionFileSystem).root.(*dirnode)
-       root.fileinfo.name = id
-       root.parent = parent
+       root := cfs.rootnode()
+       root.SetParent(parent)
+       root.(*dirnode).fileinfo.name = id
        return root
 }
 
+// vdirnode wraps an inode by ignoring any requests to add/replace
+// children, and calling a create() func when a non-existing child is
+// looked up.
+//
+// create() can return either a new node, which will be added to the
+// treenode, or nil for ENOENT.
 type vdirnode struct {
-       treenode
-       create func(string) inode
+       inode
+       create func(parent inode, name string) inode
 }
 
 func (vn *vdirnode) Child(name string, _ func(inode) inode) inode {
-       return vn.treenode.Child(name, func(existing inode) inode {
+       return vn.inode.Child(name, func(existing inode) inode {
                if existing != nil {
                        return existing
                } else {
-                       return vn.create(name)
+                       n := vn.create(vn, name)
+                       if n != nil {
+                               n.SetParent(vn)
+                       }
+                       return n
                }
        })
 }