mfs: make Root value a Directory

Make `Root` value explicitly a `Directory` structure instead of the `FSNode`
interface (which also allowed the `File` type). This helps to make the code
easier to reason about: the root of an MFS layout is always a directory, not a
(single) file.

Rename `GetValue()` to `GetDirectory()` to also make it more explicit, the
renamed function now returns a `Directory` so there is no need for type
assertions that were previously done on the `FSNode` interface to check that it
was actually a `Directory`.

`NewRoot()` now doesn't allow to create `Root` structures from DAG nodes that
contain UnixFS files.

License: MIT
Signed-off-by: Lucas Molas <schomatis@gmail.com>

core/corerepo/gc.go

@@ -71,7 +71,7 @@ func NewGC(n *core.IpfsNode) (*GC, error) {
 }
 
 func BestEffortRoots(filesRoot *mfs.Root) ([]*cid.Cid, error) {
-	rootDag, err := filesRoot.GetValue().GetNode()
+	rootDag, err := filesRoot.GetDirectory().GetNode()
 	if err != nil {
 		return nil, err
 	}

core/coreunix/add.go

@@ -144,7 +144,7 @@ func (adder *Adder) RootNode() (ipld.Node, error) {
 	if err != nil {
 		return nil, err
 	}
-	root, err := mr.GetValue().GetNode()
+	root, err := mr.GetDirectory().GetNode()
 	if err != nil {
 		return nil, err
 	}
@@ -199,7 +199,8 @@ func (adder *Adder) Finalize() (ipld.Node, error) {
 	if err != nil {
 		return nil, err
 	}
-	root := mr.GetValue()
+	var root mfs.FSNode
+	root = mr.GetDirectory()
 
 	err = root.Flush()
 	if err != nil {
@@ -224,10 +225,7 @@ func (adder *Adder) Finalize() (ipld.Node, error) {
 			return nil, err
 		}
 
-		dir, ok := mr.GetValue().(*mfs.Directory)
-		if !ok {
-			return nil, fmt.Errorf("root is not a directory")
-		}
+		dir := mr.GetDirectory()
 
 		root, err = dir.Child(name)
 		if err != nil {

fuse/ipns/ipns_unix.go

@@ -118,14 +118,7 @@ func loadRoot(ctx context.Context, rt *keyRoot, ipfs *core.IpfsNode, name string
 
 	rt.root = root
 
-	switch val := root.GetValue().(type) {
-	case *mfs.Directory:
-		return &Directory{dir: val}, nil
-	case *mfs.File:
-		return &FileNode{fi: val}, nil
-	default:
-		return nil, errors.New("unrecognized type")
-	}
+	return &Directory{dir: root.GetDirectory()}, nil
 }
 
 type keyRoot struct {

mfs/mfs_test.go

@@ -213,7 +213,7 @@ func TestBasic(t *testing.T) {
 	defer cancel()
 	ds, rt := setupRoot(ctx, t)
 
-	rootdir := rt.GetValue().(*Directory)
+	rootdir := rt.GetDirectory()
 
 	// test making a basic dir
 	_, err := rootdir.Mkdir("a")
@@ -243,7 +243,7 @@ func TestMkdir(t *testing.T) {
 	defer cancel()
 	_, rt := setupRoot(ctx, t)
 
-	rootdir := rt.GetValue().(*Directory)
+	rootdir := rt.GetDirectory()
 
 	dirsToMake := []string{"a", "B", "foo", "bar", "cats", "fish"}
 	sort.Strings(dirsToMake) // sort for easy comparing later
@@ -281,7 +281,7 @@ func TestDirectoryLoadFromDag(t *testing.T) {
 	defer cancel()
 	ds, rt := setupRoot(ctx, t)
 
-	rootdir := rt.GetValue().(*Directory)
+	rootdir := rt.GetDirectory()
 
 	nd := getRandFile(t, ds, 1000)
 	err := ds.Add(ctx, nd)
@@ -373,7 +373,7 @@ func TestMfsFile(t *testing.T) {
 	defer cancel()
 	ds, rt := setupRoot(ctx, t)
 
-	rootdir := rt.GetValue().(*Directory)
+	rootdir := rt.GetDirectory()
 
 	fisize := 1000
 	nd := getRandFile(t, ds, 1000)
@@ -686,7 +686,7 @@ func actorReadFile(d *Directory) error {
 }
 
 func testActor(rt *Root, iterations int, errs chan error) {
-	d := rt.GetValue().(*Directory)
+	d := rt.GetDirectory()
 	for i := 0; i < iterations; i++ {
 		switch rand.Intn(5) {
 		case 0:
@@ -763,7 +763,7 @@ func TestConcurrentWriteAndFlush(t *testing.T) {
 	defer cancel()
 	ds, rt := setupRoot(ctx, t)
 
-	d := mkdirP(t, rt.GetValue().(*Directory), "foo/bar/baz")
+	d := mkdirP(t, rt.GetDirectory(), "foo/bar/baz")
 	fn := fileNodeFromReader(t, ds, bytes.NewBuffer(nil))
 	err := d.AddChild("file", fn)
 	if err != nil {
@@ -786,7 +786,7 @@ func TestConcurrentWriteAndFlush(t *testing.T) {
 	}()
 
 	for i := 0; i < nloops; i++ {
-		_, err := rt.GetValue().GetNode()
+		_, err := rt.GetDirectory().GetNode()
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -800,7 +800,7 @@ func TestFlushing(t *testing.T) {
 	defer cancel()
 	_, rt := setupRoot(ctx, t)
 
-	dir := rt.GetValue().(*Directory)
+	dir := rt.GetDirectory()
 	c := mkdirP(t, dir, "a/b/c")
 	d := mkdirP(t, dir, "a/b/d")
 	e := mkdirP(t, dir, "a/b/e")
@@ -901,7 +901,7 @@ func TestConcurrentReads(t *testing.T) {
 
 	ds, rt := setupRoot(ctx, t)
 
-	rootdir := rt.GetValue().(*Directory)
+	rootdir := rt.GetDirectory()
 
 	path := "a/b/c"
 	d := mkdirP(t, rootdir, path)
@@ -976,7 +976,7 @@ func TestConcurrentWrites(t *testing.T) {
 
 	ds, rt := setupRoot(ctx, t)
 
-	rootdir := rt.GetValue().(*Directory)
+	rootdir := rt.GetDirectory()
 
 	path := "a/b/c"
 	d := mkdirP(t, rootdir, path)
@@ -1011,7 +1011,7 @@ func TestFileDescriptors(t *testing.T) {
 	defer cancel()
 
 	ds, rt := setupRoot(ctx, t)
-	dir := rt.GetValue().(*Directory)
+	dir := rt.GetDirectory()
 
 	nd := dag.NodeWithData(ft.FilePBData(nil, 0))
 	fi, err := NewFile("test", nd, dir, ds)

mfs/ops.go

@@ -1,7 +1,6 @@
 package mfs
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	gopath "path"
@@ -129,7 +128,7 @@ func Mkdir(r *Root, pth string, opts MkdirOpts) error {
 		return fmt.Errorf("cannot create directory '/': Already exists")
 	}
 
-	cur := r.GetValue().(*Directory)
+	cur := r.GetDirectory()
 	for i, d := range parts[:len(parts)-1] {
 		fsn, err := cur.Child(d)
 		if err == os.ErrNotExist && opts.Mkparents {
@@ -172,12 +171,11 @@ func Mkdir(r *Root, pth string, opts MkdirOpts) error {
 	return nil
 }
 
+// Lookup extracts the root directory and performs a lookup under it.
+// TODO: Now that the root is always a directory, can this function
+// be collapsed with `DirLookup`? Or at least be made a method of `Root`?
 func Lookup(r *Root, path string) (FSNode, error) {
-	dir, ok := r.GetValue().(*Directory)
-	if !ok {
-		log.Errorf("root not a dir: %#v", r.GetValue())
-		return nil, errors.New("root was not a directory")
-	}
+	dir := r.GetDirectory()
 
 	return DirLookup(dir, path)
 }

mfs/system.go

@@ -53,8 +53,8 @@ type Root struct {
 	// node is the merkledag root.
 	node *dag.ProtoNode
 
-	// val represents the node. It can either be a File or a Directory.
-	val FSNode
+	// Root directory of the MFS layout.
+	dir *Directory
 
 	repub *Republisher
 
@@ -90,33 +90,29 @@ func NewRoot(parent context.Context, ds ipld.DAGService, node *dag.ProtoNode, pf
 
 	switch pbn.GetType() {
 	case ft.TDirectory, ft.THAMTShard:
-		rval, err := NewDirectory(parent, node.String(), node, root, ds)
+		newDir, err := NewDirectory(parent, node.String(), node, root, ds)
 		if err != nil {
 			return nil, err
 		}
 
-		root.val = rval
+		root.dir = newDir
 	case ft.TFile, ft.TMetadata, ft.TRaw:
-		fi, err := NewFile(node.String(), node, root, ds)
-		if err != nil {
-			return nil, err
-		}
-		root.val = fi
+		return nil, fmt.Errorf("root can't be a file (unixfs type: %s)", pbn.GetType())
 	default:
 		return nil, fmt.Errorf("unrecognized unixfs type: %s", pbn.GetType())
 	}
 	return root, nil
 }
 
-// GetValue returns the value of Root.
-func (kr *Root) GetValue() FSNode {
-	return kr.val
+// GetDirectory returns the root directory.
+func (kr *Root) GetDirectory() *Directory {
+	return kr.dir
 }
 
 // Flush signals that an update has occurred since the last publish,
 // and updates the Root republisher.
 func (kr *Root) Flush() error {
-	nd, err := kr.GetValue().GetNode()
+	nd, err := kr.GetDirectory().GetNode()
 	if err != nil {
 		return err
 	}
@@ -136,10 +132,7 @@ func (kr *Root) Flush() error {
 // A better implemented mfs system (one that does smarter internal caching and
 // refcounting) shouldnt need this method.
 func (kr *Root) FlushMemFree(ctx context.Context) error {
-	dir, ok := kr.GetValue().(*Directory)
-	if !ok {
-		return fmt.Errorf("invalid mfs structure, root should be a directory")
-	}
+	dir := kr.GetDirectory()
 
 	if err := dir.Flush(); err != nil {
 		return err
@@ -172,7 +165,7 @@ func (kr *Root) closeChild(name string, nd ipld.Node, sync bool) error {
 }
 
 func (kr *Root) Close() error {
-	nd, err := kr.GetValue().GetNode()
+	nd, err := kr.GetDirectory().GetNode()
 	if err != nil {
 		return err
 	}