From 870df2431a19da8c07961b506b4fc20f4fee35c4 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 21 Jul 2015 12:38:29 -0700 Subject: [PATCH 1/7] allow patch to optionally create intermediate dirs License: MIT Signed-off-by: Jeromy --- core/commands/object.go | 45 ++++++++++++++++++++++------------- test/sharness/t0051-object.sh | 10 ++++++++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/core/commands/object.go b/core/commands/object.go index c4976f63f..99e9c90d8 100644 --- a/core/commands/object.go +++ b/core/commands/object.go @@ -453,7 +453,9 @@ This removes the link named foo from the hash in $FOO_BAR and returns the resulting object hash. `, }, - Options: []cmds.Option{}, + Options: []cmds.Option{ + cmds.BoolOption("create", "p", "create intermediate directories on add-link"), + }, Arguments: []cmds.Argument{ cmds.StringArg("root", true, false, "the hash of the node to modify"), cmds.StringArg("command", true, false, "the operation to perform"), @@ -610,7 +612,12 @@ func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { parts := strings.Split(path, "/") - nnode, err := insertNodeAtPath(req.Context(), nd.DAG, root, parts, childk) + create, _, err := req.Option("create").Bool() + if err != nil { + return "", err + } + + nnode, err := insertNodeAtPath(req.Context(), nd.DAG, root, parts, childk, create) if err != nil { return "", err } @@ -619,12 +626,14 @@ func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { func addLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname string, childk key.Key) (*dag.Node, error) { ctx, cancel := context.WithTimeout(ctx, time.Second*30) + defer cancel() childnd, err := ds.Get(ctx, childk) if err != nil { - cancel() return nil, err } - cancel() + + // ensure no link with that name already exists + _ = root.RemoveNodeLink(childname) // ignore error, only option is ErrNotFound err = root.AddNodeLinkClean(childname, childnd) if err != nil { @@ -638,31 +647,33 @@ func addLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname s return root, nil } -func insertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string, toinsert key.Key) (*dag.Node, error) { +func insertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string, toinsert key.Key, create bool) (*dag.Node, error) { if len(path) == 1 { return addLink(ctx, ds, root, path[0], toinsert) } + var nd *dag.Node child, err := root.GetNodeLink(path[0]) if err != nil { - return nil, err + // if 'create' is true, we create directories on the way down as needed + if err == dag.ErrNotFound && create { + nd = &dag.Node{Data: ft.FolderPBData()} + } else { + return nil, err + } + } else { + nd, err = child.GetNode(ctx, ds) + if err != nil { + return nil, err + } } - nd, err := child.GetNode(ctx, ds) - if err != nil { - return nil, err - } - - ndprime, err := insertNodeAtPath(ctx, ds, nd, path[1:], toinsert) - if err != nil { - return nil, err - } - - err = root.RemoveNodeLink(path[0]) + ndprime, err := insertNodeAtPath(ctx, ds, nd, path[1:], toinsert, create) if err != nil { return nil, err } + _ = root.RemoveNodeLink(path[0]) err = root.AddNodeLinkClean(path[0], ndprime) if err != nil { return nil, err diff --git a/test/sharness/t0051-object.sh b/test/sharness/t0051-object.sh index 7ee088a5a..e3dd69c88 100755 --- a/test/sharness/t0051-object.sh +++ b/test/sharness/t0051-object.sh @@ -145,6 +145,16 @@ test_object_cmd() { echo QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn > rmlink_exp && test_cmp rmlink_exp rmlink_output ' + + test_expect_success "object patch --create works" ' + OUT=$(ipfs object patch --create $EMPTY add-link a/b/c $FILE) + ' + + test_expect_success "result looks good" ' + ipfs cat $OUT/a/b/c > p2_hwfile && + test_cmp hwfile p2_hwfile + ' + } # should work offline From bfe4e4be4f0b2b07df21a24bb2d32eca8e5caf39 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 21 Jul 2015 17:19:17 -0700 Subject: [PATCH 2/7] let rm understand paths License: MIT Signed-off-by: Jeromy --- core/commands/object.go | 52 +++++++++++++++++++++++++++++++---- test/sharness/t0051-object.sh | 10 ++++++- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/core/commands/object.go b/core/commands/object.go index 99e9c90d8..d123083bb 100644 --- a/core/commands/object.go +++ b/core/commands/object.go @@ -582,19 +582,59 @@ func rmLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { return "", err } - name := req.Arguments()[2] + path := strings.Split(req.Arguments()[2], "/") - err = root.RemoveNodeLink(name) + nnode, err := rmLink(req.Context(), nd.DAG, root, path) if err != nil { return "", err } - newkey, err := nd.DAG.Add(root) - if err != nil { - return "", err + return nnode.Key() +} + +func rmLink(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string) (*dag.Node, error) { + if len(path) == 1 { + // base case, remove node in question + err := root.RemoveNodeLink(path[0]) + if err != nil { + return nil, err + } + + _, err = ds.Add(root) + if err != nil { + return nil, err + } + + return root, nil } - return newkey, nil + nchild, err := root.GetNodeLink(path[0]) + if err != nil { + return nil, err + } + + nd, err := nchild.GetNode(ctx, ds) + if err != nil { + return nil, err + } + + nnode, err := rmLink(ctx, ds, nd, path[1:]) + if err != nil { + return nil, err + } + + _ = root.RemoveNodeLink(path[0]) + err = root.AddNodeLinkClean(path[0], nnode) + if err != nil { + return nil, err + } + + _, err = ds.Add(root) + if err != nil { + return nil, err + } + + return root, nil } func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { diff --git a/test/sharness/t0051-object.sh b/test/sharness/t0051-object.sh index e3dd69c88..2e1d442d5 100755 --- a/test/sharness/t0051-object.sh +++ b/test/sharness/t0051-object.sh @@ -146,6 +146,15 @@ test_object_cmd() { test_cmp rmlink_exp rmlink_output ' + test_expect_success "multilayer rm-link should work" ' + ipfs object patch $(cat multi_patch) rm-link a/b/c > multi_link_rm_out + ' + + test_expect_success "output looks good" ' + echo "QmZD3r9cZjzU8huNY2JS9TC6n8daDfT8TmE8zBSqG31Wvq" > multi_link_rm_exp && + test_cmp multi_link_rm_out multi_link_rm_exp + ' + test_expect_success "object patch --create works" ' OUT=$(ipfs object patch --create $EMPTY add-link a/b/c $FILE) ' @@ -154,7 +163,6 @@ test_object_cmd() { ipfs cat $OUT/a/b/c > p2_hwfile && test_cmp hwfile p2_hwfile ' - } # should work offline From 194eb7c0dc36aff31ed0f66dcf3b32bd1c982d8c Mon Sep 17 00:00:00 2001 From: Jeromy Date: Wed, 22 Jul 2015 09:46:06 -0700 Subject: [PATCH 3/7] more tests and better path handling in object License: MIT Signed-off-by: Jeromy --- core/commands/object.go | 12 ++++++++++-- test/sharness/t0051-object.sh | 27 +++++++++++++++++++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/core/commands/object.go b/core/commands/object.go index d123083bb..277c9616e 100644 --- a/core/commands/object.go +++ b/core/commands/object.go @@ -469,9 +469,13 @@ resulting object hash. return } - rhash := key.B58KeyDecode(req.Arguments()[0]) + rootarg := req.Arguments()[0] + if strings.HasPrefix(rootarg, "/ipfs/") { + rootarg = rootarg[6:] + } + rhash := key.B58KeyDecode(rootarg) if rhash == "" { - res.SetError(fmt.Errorf("incorrectly formatted root hash"), cmds.ErrNormal) + res.SetError(fmt.Errorf("incorrectly formatted root hash: %s", req.Arguments()[0]), cmds.ErrNormal) return } @@ -665,6 +669,10 @@ func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { } func addLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname string, childk key.Key) (*dag.Node, error) { + if childname == "" { + return nil, errors.New("cannot create link with no name!") + } + ctx, cancel := context.WithTimeout(ctx, time.Second*30) defer cancel() childnd, err := ds.Get(ctx, childk) diff --git a/test/sharness/t0051-object.sh b/test/sharness/t0051-object.sh index 2e1d442d5..0b984f783 100755 --- a/test/sharness/t0051-object.sh +++ b/test/sharness/t0051-object.sh @@ -10,6 +10,22 @@ test_description="Test object command" test_init_ipfs +test_patch_create_path() { + root=$1 + name=$2 + target=$3 + + test_expect_success "object patch --create works" ' + PCOUT=$(ipfs object patch --create $root add-link $name $target) + ' + + test_expect_success "output looks good" ' + ipfs cat $PCOUT/$name > tpcp_out && + ipfs cat $target > tpcp_exp && + test_cmp tpcp_out tpcp_exp + ' +} + test_object_cmd() { test_expect_success "'ipfs add testData' succeeds" ' @@ -155,13 +171,12 @@ test_object_cmd() { test_cmp multi_link_rm_out multi_link_rm_exp ' - test_expect_success "object patch --create works" ' - OUT=$(ipfs object patch --create $EMPTY add-link a/b/c $FILE) - ' + test_patch_create_path $EMPTY a/b/c $FILE + test_patch_create_path $EMPTY a $FILE + test_patch_create_path $EMPTY a/b/b/b/b $FILE - test_expect_success "result looks good" ' - ipfs cat $OUT/a/b/c > p2_hwfile && - test_cmp hwfile p2_hwfile + test_expect_success "create bad path fails" ' + test_must_fail ipfs object patch --create $EMPTY add-link / $FILE ' } From 7c510662ae2ea9beef3702e1149ab05cb7c0cf1d Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 24 Jul 2015 12:45:33 -0700 Subject: [PATCH 4/7] break merkledag utils into its own package License: MIT Signed-off-by: Jeromy --- core/commands/object.go | 117 +-------------------------------- merkledag/node.go | 9 +++ merkledag/utils/utils.go | 113 ++++++++++++++++++++++++++++++++ merkledag/utils/utils_test.go | 118 ++++++++++++++++++++++++++++++++++ 4 files changed, 243 insertions(+), 114 deletions(-) create mode 100644 merkledag/utils/utils.go create mode 100644 merkledag/utils/utils_test.go diff --git a/core/commands/object.go b/core/commands/object.go index 277c9616e..d30c5d1f2 100644 --- a/core/commands/object.go +++ b/core/commands/object.go @@ -18,6 +18,7 @@ import ( cmds "github.com/ipfs/go-ipfs/commands" core "github.com/ipfs/go-ipfs/core" dag "github.com/ipfs/go-ipfs/merkledag" + dagutils "github.com/ipfs/go-ipfs/merkledag/utils" path "github.com/ipfs/go-ipfs/path" ft "github.com/ipfs/go-ipfs/unixfs" u "github.com/ipfs/go-ipfs/util" @@ -588,7 +589,7 @@ func rmLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { path := strings.Split(req.Arguments()[2], "/") - nnode, err := rmLink(req.Context(), nd.DAG, root, path) + nnode, err := dagutils.RmLink(req.Context(), nd.DAG, root, path) if err != nil { return "", err } @@ -596,51 +597,6 @@ func rmLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { return nnode.Key() } -func rmLink(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string) (*dag.Node, error) { - if len(path) == 1 { - // base case, remove node in question - err := root.RemoveNodeLink(path[0]) - if err != nil { - return nil, err - } - - _, err = ds.Add(root) - if err != nil { - return nil, err - } - - return root, nil - } - - nchild, err := root.GetNodeLink(path[0]) - if err != nil { - return nil, err - } - - nd, err := nchild.GetNode(ctx, ds) - if err != nil { - return nil, err - } - - nnode, err := rmLink(ctx, ds, nd, path[1:]) - if err != nil { - return nil, err - } - - _ = root.RemoveNodeLink(path[0]) - err = root.AddNodeLinkClean(path[0], nnode) - if err != nil { - return nil, err - } - - _, err = ds.Add(root) - if err != nil { - return nil, err - } - - return root, nil -} - func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { if len(req.Arguments()) < 4 { return "", fmt.Errorf("not enough arguments for add-link") @@ -661,80 +617,13 @@ func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { return "", err } - nnode, err := insertNodeAtPath(req.Context(), nd.DAG, root, parts, childk, create) + nnode, err := dagutils.InsertNodeAtPath(req.Context(), nd.DAG, root, parts, childk, create) if err != nil { return "", err } return nnode.Key() } -func addLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname string, childk key.Key) (*dag.Node, error) { - if childname == "" { - return nil, errors.New("cannot create link with no name!") - } - - ctx, cancel := context.WithTimeout(ctx, time.Second*30) - defer cancel() - childnd, err := ds.Get(ctx, childk) - if err != nil { - return nil, err - } - - // ensure no link with that name already exists - _ = root.RemoveNodeLink(childname) // ignore error, only option is ErrNotFound - - err = root.AddNodeLinkClean(childname, childnd) - if err != nil { - return nil, err - } - - _, err = ds.Add(root) - if err != nil { - return nil, err - } - return root, nil -} - -func insertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string, toinsert key.Key, create bool) (*dag.Node, error) { - if len(path) == 1 { - return addLink(ctx, ds, root, path[0], toinsert) - } - - var nd *dag.Node - child, err := root.GetNodeLink(path[0]) - if err != nil { - // if 'create' is true, we create directories on the way down as needed - if err == dag.ErrNotFound && create { - nd = &dag.Node{Data: ft.FolderPBData()} - } else { - return nil, err - } - } else { - nd, err = child.GetNode(ctx, ds) - if err != nil { - return nil, err - } - } - - ndprime, err := insertNodeAtPath(ctx, ds, nd, path[1:], toinsert, create) - if err != nil { - return nil, err - } - - _ = root.RemoveNodeLink(path[0]) - err = root.AddNodeLinkClean(path[0], ndprime) - if err != nil { - return nil, err - } - - _, err = ds.Add(root) - if err != nil { - return nil, err - } - - return root, nil -} - func nodeFromTemplate(template string) (*dag.Node, error) { switch template { case "unixfs-dir": diff --git a/merkledag/node.go b/merkledag/node.go index 71a5b5b32..e90ac95e8 100644 --- a/merkledag/node.go +++ b/merkledag/node.go @@ -153,6 +153,15 @@ func (n *Node) GetNodeLink(name string) (*Link, error) { return nil, ErrNotFound } +func (n *Node) GetLinkedNode(ctx context.Context, ds DAGService, name string) (*Node, error) { + lnk, err := n.GetNodeLink(name) + if err != nil { + return nil, err + } + + return lnk.GetNode(ctx, ds) +} + // Copy returns a copy of the node. // NOTE: does not make copies of Node objects in the links. func (n *Node) Copy() *Node { diff --git a/merkledag/utils/utils.go b/merkledag/utils/utils.go new file mode 100644 index 000000000..e82d00229 --- /dev/null +++ b/merkledag/utils/utils.go @@ -0,0 +1,113 @@ +package dagutils + +import ( + "errors" + "time" + + context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" + + key "github.com/ipfs/go-ipfs/blocks/key" + dag "github.com/ipfs/go-ipfs/merkledag" + ft "github.com/ipfs/go-ipfs/unixfs" +) + +func AddLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname string, childk key.Key) (*dag.Node, error) { + if childname == "" { + return nil, errors.New("cannot create link with no name!") + } + + ctx, cancel := context.WithTimeout(ctx, time.Second*30) + defer cancel() + childnd, err := ds.Get(ctx, childk) + if err != nil { + return nil, err + } + + // ensure no link with that name already exists + _ = root.RemoveNodeLink(childname) // ignore error, only option is ErrNotFound + + err = root.AddNodeLinkClean(childname, childnd) + if err != nil { + return nil, err + } + + _, err = ds.Add(root) + if err != nil { + return nil, err + } + return root, nil +} + +func InsertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string, toinsert key.Key, create bool) (*dag.Node, error) { + if len(path) == 1 { + return AddLink(ctx, ds, root, path[0], toinsert) + } + + nd, err := root.GetLinkedNode(ctx, ds, path[0]) + if err != nil { + // if 'create' is true, we create directories on the way down as needed + if err == dag.ErrNotFound && create { + nd = &dag.Node{Data: ft.FolderPBData()} + } else { + return nil, err + } + } + + ndprime, err := InsertNodeAtPath(ctx, ds, nd, path[1:], toinsert, create) + if err != nil { + return nil, err + } + + _ = root.RemoveNodeLink(path[0]) + err = root.AddNodeLinkClean(path[0], ndprime) + if err != nil { + return nil, err + } + + _, err = ds.Add(root) + if err != nil { + return nil, err + } + + return root, nil +} + +func RmLink(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string) (*dag.Node, error) { + if len(path) == 1 { + // base case, remove node in question + err := root.RemoveNodeLink(path[0]) + if err != nil { + return nil, err + } + + _, err = ds.Add(root) + if err != nil { + return nil, err + } + + return root, nil + } + + nd, err := root.GetLinkedNode(ctx, ds, path[0]) + if err != nil { + return nil, err + } + + nnode, err := RmLink(ctx, ds, nd, path[1:]) + if err != nil { + return nil, err + } + + _ = root.RemoveNodeLink(path[0]) + err = root.AddNodeLinkClean(path[0], nnode) + if err != nil { + return nil, err + } + + _, err = ds.Add(root) + if err != nil { + return nil, err + } + + return root, nil +} diff --git a/merkledag/utils/utils_test.go b/merkledag/utils/utils_test.go new file mode 100644 index 000000000..36da81687 --- /dev/null +++ b/merkledag/utils/utils_test.go @@ -0,0 +1,118 @@ +package dagutils + +import ( + "testing" + + key "github.com/ipfs/go-ipfs/blocks/key" + dag "github.com/ipfs/go-ipfs/merkledag" + mdtest "github.com/ipfs/go-ipfs/merkledag/test" + + context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" +) + +func TestAddLink(t *testing.T) { + ds := mdtest.Mock(t) + fishnode := &dag.Node{ + Data: []byte("fishcakes!"), + } + + fk, err := ds.Add(fishnode) + if err != nil { + t.Fatal(err) + } + + nd := new(dag.Node) + nnode, err := AddLink(context.Background(), ds, nd, "fish", fk) + if err != nil { + t.Fatal(err) + } + + fnprime, err := nnode.GetLinkedNode(context.Background(), ds, "fish") + if err != nil { + t.Fatal(err) + } + + fnpkey, err := fnprime.Key() + if err != nil { + t.Fatal(err) + } + + if fnpkey != fk { + t.Fatal("wrong child node found!") + } +} + +func assertNodeAtPath(t *testing.T, ds dag.DAGService, root *dag.Node, path []string, exp key.Key) { + cur := root + for _, e := range path { + nxt, err := cur.GetLinkedNode(context.Background(), ds, e) + if err != nil { + t.Fatal(err) + } + + cur = nxt + } + + curk, err := cur.Key() + if err != nil { + t.Fatal(err) + } + + if curk != exp { + t.Fatal("node not as expected at end of path") + } +} + +func TestInsertNode(t *testing.T) { + ds := mdtest.Mock(t) + root := new(dag.Node) + + childa := &dag.Node{ + Data: []byte("This is child A"), + } + ak, err := ds.Add(childa) + if err != nil { + t.Fatal(err) + } + + path := []string{"a", "b", "c", "d"} + root_a, err := InsertNodeAtPath(context.Background(), ds, root, path, ak, true) + if err != nil { + t.Fatal(err) + } + assertNodeAtPath(t, ds, root_a, path, ak) + + childb := &dag.Node{Data: []byte("this is the second child")} + bk, err := ds.Add(childb) + if err != nil { + t.Fatal(err) + } + + // this one should fail, we are specifying a non-existant path + // with create == false + path2 := []string{"a", "b", "e", "f"} + _, err = InsertNodeAtPath(context.Background(), ds, root_a, path2, bk, false) + if err == nil { + t.Fatal("that shouldnt have worked") + } + if err != dag.ErrNotFound { + t.Fatal("expected this to fail with 'not found'") + } + + // inserting a path of length one should work with create == false + path3 := []string{"x"} + root_b, err := InsertNodeAtPath(context.Background(), ds, root_a, path3, bk, false) + if err != nil { + t.Fatal(err) + } + + assertNodeAtPath(t, ds, root_b, path3, bk) + + // now try overwriting a path + root_c, err := InsertNodeAtPath(context.Background(), ds, root_b, path, bk, false) + if err != nil { + t.Fatal(err) + } + + assertNodeAtPath(t, ds, root_c, path, bk) +} From 314f7bbfea72743b1bafdfafede6fcd9710ae834 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Fri, 24 Jul 2015 12:48:21 -0700 Subject: [PATCH 5/7] space out sharness test calls License: MIT Signed-off-by: Jeromy --- test/sharness/t0051-object.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/sharness/t0051-object.sh b/test/sharness/t0051-object.sh index 0b984f783..84746c229 100755 --- a/test/sharness/t0051-object.sh +++ b/test/sharness/t0051-object.sh @@ -172,7 +172,9 @@ test_object_cmd() { ' test_patch_create_path $EMPTY a/b/c $FILE + test_patch_create_path $EMPTY a $FILE + test_patch_create_path $EMPTY a/b/b/b/b $FILE test_expect_success "create bad path fails" ' From b32d0ef15325a44861ec58cadf3e8d3286a32815 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 28 Jul 2015 14:12:01 -0700 Subject: [PATCH 6/7] implement 'editor' abstraction License: MIT Signed-off-by: Jeromy --- core/commands/object.go | 24 +++++++-- merkledag/node.go | 20 +++++-- merkledag/utils/utils.go | 66 ++++++++++++++++++----- merkledag/utils/utils_test.go | 98 +++++++++++++++++++---------------- 4 files changed, 140 insertions(+), 68 deletions(-) diff --git a/core/commands/object.go b/core/commands/object.go index d30c5d1f2..bb066a740 100644 --- a/core/commands/object.go +++ b/core/commands/object.go @@ -587,13 +587,17 @@ func rmLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { return "", err } - path := strings.Split(req.Arguments()[2], "/") + path := req.Arguments()[2] - nnode, err := dagutils.RmLink(req.Context(), nd.DAG, root, path) + e := dagutils.NewDagEditor(nd.DAG, root) + + err = e.RmLink(req.Context(), path) if err != nil { return "", err } + nnode := e.GetNode() + return nnode.Key() } @@ -610,17 +614,27 @@ func addLinkCaller(req cmds.Request, root *dag.Node) (key.Key, error) { path := req.Arguments()[2] childk := key.B58KeyDecode(req.Arguments()[3]) - parts := strings.Split(path, "/") - create, _, err := req.Option("create").Bool() if err != nil { return "", err } - nnode, err := dagutils.InsertNodeAtPath(req.Context(), nd.DAG, root, parts, childk, create) + var createfunc func() *dag.Node + if create { + createfunc = func() *dag.Node { + return &dag.Node{Data: ft.FolderPBData()} + } + } + + e := dagutils.NewDagEditor(nd.DAG, root) + + err = e.InsertNodeAtPath(req.Context(), path, childk, createfunc) if err != nil { return "", err } + + nnode := e.GetNode() + return nnode.Key() } diff --git a/merkledag/node.go b/merkledag/node.go index e90ac95e8..8d06077c0 100644 --- a/merkledag/node.go +++ b/merkledag/node.go @@ -129,13 +129,23 @@ func (n *Node) AddRawLink(name string, l *Link) error { // Remove a link on this node by the given name func (n *Node) RemoveNodeLink(name string) error { n.encoded = nil - for i, l := range n.Links { - if l.Name == name { - n.Links = append(n.Links[:i], n.Links[i+1:]...) - return nil + good := make([]*Link, 0, len(n.Links)) + var found bool + + for _, l := range n.Links { + if l.Name != name { + good = append(good, l) + } else { + found = true } } - return ErrNotFound + n.Links = good + + if !found { + return ErrNotFound + } + + return nil } // Return a copy of the link with given name diff --git a/merkledag/utils/utils.go b/merkledag/utils/utils.go index e82d00229..6ab612c17 100644 --- a/merkledag/utils/utils.go +++ b/merkledag/utils/utils.go @@ -2,22 +2,44 @@ package dagutils import ( "errors" - "time" + "strings" context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" key "github.com/ipfs/go-ipfs/blocks/key" dag "github.com/ipfs/go-ipfs/merkledag" - ft "github.com/ipfs/go-ipfs/unixfs" ) -func AddLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname string, childk key.Key) (*dag.Node, error) { +type Editor struct { + root *dag.Node + ds dag.DAGService +} + +func NewDagEditor(ds dag.DAGService, root *dag.Node) *Editor { + return &Editor{ + root: root, + ds: ds, + } +} + +func (e *Editor) GetNode() *dag.Node { + return e.root.Copy() +} + +func (e *Editor) AddLink(ctx context.Context, childname string, childk key.Key) error { + nd, err := addLink(ctx, e.ds, e.root, childname, childk) + if err != nil { + return err + } + e.root = nd + return nil +} + +func addLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname string, childk key.Key) (*dag.Node, error) { if childname == "" { return nil, errors.New("cannot create link with no name!") } - ctx, cancel := context.WithTimeout(ctx, time.Second*30) - defer cancel() childnd, err := ds.Get(ctx, childk) if err != nil { return nil, err @@ -38,22 +60,32 @@ func AddLink(ctx context.Context, ds dag.DAGService, root *dag.Node, childname s return root, nil } -func InsertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string, toinsert key.Key, create bool) (*dag.Node, error) { +func (e *Editor) InsertNodeAtPath(ctx context.Context, path string, toinsert key.Key, create func() *dag.Node) error { + splpath := strings.Split(path, "/") + nd, err := insertNodeAtPath(ctx, e.ds, e.root, splpath, toinsert, create) + if err != nil { + return err + } + e.root = nd + return nil +} + +func insertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string, toinsert key.Key, create func() *dag.Node) (*dag.Node, error) { if len(path) == 1 { - return AddLink(ctx, ds, root, path[0], toinsert) + return addLink(ctx, ds, root, path[0], toinsert) } nd, err := root.GetLinkedNode(ctx, ds, path[0]) if err != nil { // if 'create' is true, we create directories on the way down as needed - if err == dag.ErrNotFound && create { - nd = &dag.Node{Data: ft.FolderPBData()} + if err == dag.ErrNotFound && create != nil { + nd = create() } else { return nil, err } } - ndprime, err := InsertNodeAtPath(ctx, ds, nd, path[1:], toinsert, create) + ndprime, err := insertNodeAtPath(ctx, ds, nd, path[1:], toinsert, create) if err != nil { return nil, err } @@ -72,7 +104,17 @@ func InsertNodeAtPath(ctx context.Context, ds dag.DAGService, root *dag.Node, pa return root, nil } -func RmLink(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string) (*dag.Node, error) { +func (e *Editor) RmLink(ctx context.Context, path string) error { + splpath := strings.Split(path, "/") + nd, err := rmLink(ctx, e.ds, e.root, splpath) + if err != nil { + return err + } + e.root = nd + return nil +} + +func rmLink(ctx context.Context, ds dag.DAGService, root *dag.Node, path []string) (*dag.Node, error) { if len(path) == 1 { // base case, remove node in question err := root.RemoveNodeLink(path[0]) @@ -93,7 +135,7 @@ func RmLink(ctx context.Context, ds dag.DAGService, root *dag.Node, path []strin return nil, err } - nnode, err := RmLink(ctx, ds, nd, path[1:]) + nnode, err := rmLink(ctx, ds, nd, path[1:]) if err != nil { return nil, err } diff --git a/merkledag/utils/utils_test.go b/merkledag/utils/utils_test.go index 36da81687..39b1a519d 100644 --- a/merkledag/utils/utils_test.go +++ b/merkledag/utils/utils_test.go @@ -1,6 +1,7 @@ package dagutils import ( + "strings" "testing" key "github.com/ipfs/go-ipfs/blocks/key" @@ -22,7 +23,7 @@ func TestAddLink(t *testing.T) { } nd := new(dag.Node) - nnode, err := AddLink(context.Background(), ds, nd, "fish", fk) + nnode, err := addLink(context.Background(), ds, nd, "fish", fk) if err != nil { t.Fatal(err) } @@ -42,9 +43,10 @@ func TestAddLink(t *testing.T) { } } -func assertNodeAtPath(t *testing.T, ds dag.DAGService, root *dag.Node, path []string, exp key.Key) { +func assertNodeAtPath(t *testing.T, ds dag.DAGService, root *dag.Node, path string, exp key.Key) { + parts := strings.Split(path, "/") cur := root - for _, e := range path { + for _, e := range parts { nxt, err := cur.GetLinkedNode(context.Background(), ds, e) if err != nil { t.Fatal(err) @@ -66,53 +68,57 @@ func assertNodeAtPath(t *testing.T, ds dag.DAGService, root *dag.Node, path []st func TestInsertNode(t *testing.T) { ds := mdtest.Mock(t) root := new(dag.Node) + e := NewDagEditor(ds, root) - childa := &dag.Node{ - Data: []byte("This is child A"), - } - ak, err := ds.Add(childa) + testInsert(t, e, "a", "anodefortesting", false, "") + testInsert(t, e, "a/b", "data", false, "") + testInsert(t, e, "a/b/c/d/e", "blah", false, "merkledag: not found") + testInsert(t, e, "a/b/c/d/e", "foo", true, "") + testInsert(t, e, "a/b/c/d/f", "baz", true, "") + testInsert(t, e, "a/b/c/d/f", "bar", true, "") + + testInsert(t, e, "", "bar", true, "cannot create link with no name!") + testInsert(t, e, "////", "slashes", true, "cannot create link with no name!") + + k, err := e.GetNode().Key() if err != nil { t.Fatal(err) } - path := []string{"a", "b", "c", "d"} - root_a, err := InsertNodeAtPath(context.Background(), ds, root, path, ak, true) - if err != nil { - t.Fatal(err) + if k.B58String() != "QmThorWojP6YzLJwDukxiYCoKQSwyrMCvdt4WZ6rPm221t" { + t.Fatal("output was different than expected") } - assertNodeAtPath(t, ds, root_a, path, ak) - - childb := &dag.Node{Data: []byte("this is the second child")} - bk, err := ds.Add(childb) - if err != nil { - t.Fatal(err) - } - - // this one should fail, we are specifying a non-existant path - // with create == false - path2 := []string{"a", "b", "e", "f"} - _, err = InsertNodeAtPath(context.Background(), ds, root_a, path2, bk, false) - if err == nil { - t.Fatal("that shouldnt have worked") - } - if err != dag.ErrNotFound { - t.Fatal("expected this to fail with 'not found'") - } - - // inserting a path of length one should work with create == false - path3 := []string{"x"} - root_b, err := InsertNodeAtPath(context.Background(), ds, root_a, path3, bk, false) - if err != nil { - t.Fatal(err) - } - - assertNodeAtPath(t, ds, root_b, path3, bk) - - // now try overwriting a path - root_c, err := InsertNodeAtPath(context.Background(), ds, root_b, path, bk, false) - if err != nil { - t.Fatal(err) - } - - assertNodeAtPath(t, ds, root_c, path, bk) +} + +func testInsert(t *testing.T, e *Editor, path, data string, create bool, experr string) { + child := &dag.Node{Data: []byte(data)} + ck, err := e.ds.Add(child) + if err != nil { + t.Fatal(err) + } + + var c func() *dag.Node + if create { + c = func() *dag.Node { + return &dag.Node{} + } + } + + err = e.InsertNodeAtPath(context.TODO(), path, ck, c) + if experr != "" { + var got string + if err != nil { + got = err.Error() + } + if got != experr { + t.Fatalf("expected '%s' but got '%s'", experr, got) + } + return + } + + if err != nil { + t.Fatal(err) + } + + assertNodeAtPath(t, e.ds, e.root, path, ck) } From aa7d9467161f6b8144d3c8d7b70f51b99bfe5d69 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 28 Jul 2015 21:15:35 -0700 Subject: [PATCH 7/7] a little more test coverage on merkledag License: MIT Signed-off-by: Jeromy --- merkledag/coding.go | 12 +------ merkledag/merkledag_test.go | 68 +++++++++++++++++++++++++++++++++++++ merkledag/node_test.go | 54 +++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 merkledag/node_test.go diff --git a/merkledag/coding.go b/merkledag/coding.go index f8cc326a4..7baf863c8 100644 --- a/merkledag/coding.go +++ b/merkledag/coding.go @@ -37,16 +37,6 @@ func (n *Node) Unmarshal(encoded []byte) error { return nil } -// MarshalTo encodes a *Node instance into a given byte slice. -// The conversion uses an intermediate PBNode. -func (n *Node) MarshalTo(encoded []byte) error { - pbn := n.getPBNode() - if _, err := pbn.MarshalTo(encoded); err != nil { - return fmt.Errorf("Marshal failed. %v", err) - } - return nil -} - // Marshal encodes a *Node instance into a new byte slice. // The conversion uses an intermediate PBNode. func (n *Node) Marshal() ([]byte, error) { @@ -82,7 +72,7 @@ func (n *Node) Encoded(force bool) ([]byte, error) { var err error n.encoded, err = n.Marshal() if err != nil { - return []byte{}, err + return nil, err } n.cached = u.Hash(n.encoded) } diff --git a/merkledag/merkledag_test.go b/merkledag/merkledag_test.go index d2961d3ad..fc110bfd7 100644 --- a/merkledag/merkledag_test.go +++ b/merkledag/merkledag_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "io/ioutil" + "strings" "sync" "testing" @@ -221,3 +222,70 @@ func runBatchFetchTest(t *testing.T, read io.Reader) { wg.Wait() } + +func TestRecursiveAdd(t *testing.T) { + a := &Node{Data: []byte("A")} + b := &Node{Data: []byte("B")} + c := &Node{Data: []byte("C")} + d := &Node{Data: []byte("D")} + e := &Node{Data: []byte("E")} + + err := a.AddNodeLink("blah", b) + if err != nil { + t.Fatal(err) + } + + err = b.AddNodeLink("foo", c) + if err != nil { + t.Fatal(err) + } + + err = b.AddNodeLink("bar", d) + if err != nil { + t.Fatal(err) + } + + err = d.AddNodeLink("baz", e) + if err != nil { + t.Fatal(err) + } + + dsp := getDagservAndPinner(t) + err = dsp.ds.AddRecursive(a) + if err != nil { + t.Fatal(err) + } + + assertCanGet(t, dsp.ds, a) + assertCanGet(t, dsp.ds, b) + assertCanGet(t, dsp.ds, c) + assertCanGet(t, dsp.ds, d) + assertCanGet(t, dsp.ds, e) +} + +func assertCanGet(t *testing.T, ds DAGService, n *Node) { + k, err := n.Key() + if err != nil { + t.Fatal(err) + } + + _, err = ds.Get(context.TODO(), k) + if err != nil { + t.Fatal(err) + } +} + +func TestCantGet(t *testing.T) { + dsp := getDagservAndPinner(t) + a := &Node{Data: []byte("A")} + + k, err := a.Key() + if err != nil { + t.Fatal(err) + } + + _, err = dsp.ds.Get(context.TODO(), k) + if !strings.Contains(err.Error(), "not found") { + t.Fatal("expected err not found, got: ", err) + } +} diff --git a/merkledag/node_test.go b/merkledag/node_test.go new file mode 100644 index 000000000..75aa4c988 --- /dev/null +++ b/merkledag/node_test.go @@ -0,0 +1,54 @@ +package merkledag + +import ( + "testing" +) + +func TestRemoveLink(t *testing.T) { + nd := &Node{ + Links: []*Link{ + &Link{Name: "a"}, + &Link{Name: "b"}, + &Link{Name: "a"}, + &Link{Name: "a"}, + &Link{Name: "c"}, + &Link{Name: "a"}, + }, + } + + err := nd.RemoveNodeLink("a") + if err != nil { + t.Fatal(err) + } + + if len(nd.Links) != 2 { + t.Fatal("number of links incorrect") + } + + if nd.Links[0].Name != "b" { + t.Fatal("link order wrong") + } + + if nd.Links[1].Name != "c" { + t.Fatal("link order wrong") + } + + // should fail + err = nd.RemoveNodeLink("a") + if err != ErrNotFound { + t.Fatal("should have failed to remove link") + } + + // ensure nothing else got touched + if len(nd.Links) != 2 { + t.Fatal("number of links incorrect") + } + + if nd.Links[0].Name != "b" { + t.Fatal("link order wrong") + } + + if nd.Links[1].Name != "c" { + t.Fatal("link order wrong") + } +}