Fix EnumerateChildren & hasChild to take a *cid.Cid instead of []*mdag.Link

Author: Kevin Atkinson <k@kevina.org>

Fix EnumerateChildren & hasChild to take a *cid.Cid instead of []*mdag.Link

Author: Jeromy Johnson <why@ipfs.io>

make FetchGraph use a cid

pin: fix TestPinRecursiveFail

License: MIT
Signed-off-by: Jeromy <why@ipfs.io>

License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>

core/commands/dht.go

@@ -370,12 +370,8 @@ func provideKeysRec(ctx context.Context, r routing.IpfsRouting, dserv dag.DAGSer
 	provided := cid.NewSet()
 	for _, c := range cids {
 		kset := cid.NewSet()
-		node, err := dserv.Get(ctx, c)
-		if err != nil {
-			return err
-		}
 
-		err = dag.EnumerateChildrenAsync(ctx, dserv, node, kset.Visit)
+		err := dag.EnumerateChildrenAsync(ctx, dserv, c, kset.Visit)
 		if err != nil {
 			return err
 		}

core/commands/pin.go

@@ -328,11 +328,7 @@ func pinLsAll(typeStr string, ctx context.Context, n *core.IpfsNode) (map[string
 	if typeStr == "indirect" || typeStr == "all" {
 		set := cid.NewSet()
 		for _, k := range n.Pinning.RecursiveKeys() {
-			links, err := n.DAG.GetLinks(ctx, k)
-			if err != nil {
-				return nil, err
-			}
-			err = dag.EnumerateChildren(n.Context(), n.DAG, links, set.Visit, false)
+			err := dag.EnumerateChildren(n.Context(), n.DAG, k, set.Visit, false)
 			if err != nil {
 				return nil, err
 			}

core/coreunix/add_test.go

@@ -156,13 +156,9 @@ func TestAddGCLive(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
 	defer cancel()
-	root, err := node.DAG.Get(ctx, last)
-	if err != nil {
-		t.Fatal(err)
-	}
 
 	set := cid.NewSet()
-	err = dag.EnumerateChildren(ctx, node.DAG, root.Links, set.Visit, false)
+	err = dag.EnumerateChildren(ctx, node.DAG, last, set.Visit, false)
 	if err != nil {
 		t.Fatal(err)
 	}

merkledag/merkledag.go

@@ -126,8 +126,8 @@ func (n *dagService) Remove(nd *Node) error {
 }
 
 // FetchGraph fetches all nodes that are children of the given node
-func FetchGraph(ctx context.Context, root *Node, serv DAGService) error {
-	return EnumerateChildrenAsync(ctx, serv, root, cid.NewSet().Visit)
+func FetchGraph(ctx context.Context, c *cid.Cid, serv DAGService) error {
+	return EnumerateChildrenAsync(ctx, serv, c, cid.NewSet().Visit)
 }
 
 // FindLinks searches this nodes links for the given key,
@@ -394,19 +394,17 @@ func legacyCidFromLink(lnk *Link) *cid.Cid {
 // EnumerateChildren will walk the dag below the given root node and add all
 // unseen children to the passed in set.
 // TODO: parallelize to avoid disk latency perf hits?
-func EnumerateChildren(ctx context.Context, ds LinkService, links []*Link, visit func(*cid.Cid) bool, bestEffort bool) error {
+func EnumerateChildren(ctx context.Context, ds LinkService, root *cid.Cid, visit func(*cid.Cid) bool, bestEffort bool) error {
+	links, err := ds.GetLinks(ctx, root)
+	if bestEffort && err == ErrNotFound {
+		return nil
+	} else if err != nil {
+		return err
+	}
 	for _, lnk := range links {
 		c := legacyCidFromLink(lnk)
 		if visit(c) {
-			children, err := ds.GetLinks(ctx, c)
-			if err != nil {
-				if bestEffort && err == ErrNotFound {
-					continue
-				} else {
-					return err
-				}
-			}
-			err = EnumerateChildren(ctx, ds, children, visit, bestEffort)
+			err = EnumerateChildren(ctx, ds, c, visit, bestEffort)
 			if err != nil {
 				return err
 			}
@@ -415,7 +413,7 @@ func EnumerateChildren(ctx context.Context, ds LinkService, links []*Link, visit
 	return nil
 }
 
-func EnumerateChildrenAsync(ctx context.Context, ds DAGService, root *Node, visit func(*cid.Cid) bool) error {
+func EnumerateChildrenAsync(ctx context.Context, ds DAGService, c *cid.Cid, visit func(*cid.Cid) bool) error {
 	toprocess := make(chan []*cid.Cid, 8)
 	nodes := make(chan *NodeOption, 8)
 
@@ -425,6 +423,11 @@ func EnumerateChildrenAsync(ctx context.Context, ds DAGService, root *Node, visi
 
 	go fetchNodes(ctx, ds, toprocess, nodes)
 
+	root, err := ds.Get(ctx, c)
+	if err != nil {
+		return err
+	}
+
 	nodes <- &NodeOption{Node: root}
 	live := 1
 

merkledag/merkledag_test.go

@@ -231,7 +231,7 @@ func TestFetchGraph(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = FetchGraph(context.TODO(), root, dservs[1])
+	err = FetchGraph(context.TODO(), root.Cid(), dservs[1])
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -241,7 +241,7 @@ func TestFetchGraph(t *testing.T) {
 
 	offline_ds := NewDAGService(bs)
 
-	err = EnumerateChildren(context.Background(), offline_ds, root.Links, func(_ *cid.Cid) bool { return true }, false)
+	err = EnumerateChildren(context.Background(), offline_ds, root.Cid(), func(_ *cid.Cid) bool { return true }, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -258,7 +258,7 @@ func TestEnumerateChildren(t *testing.T) {
 	}
 
 	set := cid.NewSet()
-	err = EnumerateChildren(context.Background(), ds, root.Links, set.Visit, false)
+	err = EnumerateChildren(context.Background(), ds, root.Cid(), set.Visit, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -269,7 +269,7 @@ func TestEnumerateChildren(t *testing.T) {
 		for _, lnk := range n.Links {
 			c := cid.NewCidV0(lnk.Hash)
 			if !set.Has(c) {
-				t.Fatal("missing key in set!")
+				t.Fatal("missing key in set! ", lnk.Hash.B58String())
 			}
 			child, err := ds.Get(context.Background(), c)
 			if err != nil {

pin/gc/gc.go

@@ -71,13 +71,9 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, ls dag.LinkService, pn pin.
 func Descendants(ctx context.Context, ls dag.LinkService, set key.KeySet, roots []*cid.Cid, bestEffort bool) error {
 	for _, c := range roots {
 		set.Add(key.Key(c.Hash()))
-		links, err := ls.GetLinks(ctx, c)
-		if err != nil {
-			return err
-		}
 
 		// EnumerateChildren recursively walks the dag and adds the keys to the given set
-		err = dag.EnumerateChildren(ctx, ls, links, func(c *cid.Cid) bool {
+		err := dag.EnumerateChildren(ctx, ls, c, func(c *cid.Cid) bool {
 			k := key.Key(c.Hash())
 			seen := set.Has(k)
 			if seen {

pin/pin.go

@@ -178,7 +178,7 @@ func (p *pinner) Pin(ctx context.Context, node *mdag.Node, recurse bool) error {
 		}
 
 		// fetch entire graph
-		err := mdag.FetchGraph(ctx, node, p.dserv)
+		err := mdag.FetchGraph(ctx, c, p.dserv)
 		if err != nil {
 			return err
 		}
@@ -279,12 +279,7 @@ func (p *pinner) isPinnedWithType(c *cid.Cid, mode PinMode) (string, bool, error
 
 	// Default is Indirect
 	for _, rc := range p.recursePin.Keys() {
-		links, err := p.dserv.GetLinks(context.Background(), rc)
-		if err != nil {
-			return "", false, err
-		}
-
-		has, err := hasChild(p.dserv, links, k)
+		has, err := hasChild(p.dserv, rc, k)
 		if err != nil {
 			return "", false, err
 		}
@@ -521,19 +516,18 @@ func (p *pinner) PinWithMode(c *cid.Cid, mode PinMode) {
 	}
 }
 
-func hasChild(ds mdag.LinkService, links []*mdag.Link, child key.Key) (bool, error) {
+func hasChild(ds mdag.LinkService, root *cid.Cid, child key.Key) (bool, error) {
+	links, err := ds.GetLinks(context.Background(), root)
+	if err != nil {
+		return false, err
+	}
 	for _, lnk := range links {
 		c := cid.NewCidV0(lnk.Hash)
 		if key.Key(c.Hash()) == child {
 			return true, nil
 		}
 
-		children, err := ds.GetLinks(context.Background(), c)
-		if err != nil {
-			return false, err
-		}
-
-		has, err := hasChild(ds, children, child)
+		has, err := hasChild(ds, c, child)
 		if err != nil {
 			return false, err
 		}

pin/pin_test.go

@@ -225,6 +225,11 @@ func TestPinRecursiveFail(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	_, err = dserv.Add(a)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	// this one is time based... but shouldnt cause any issues
 	mctx, _ = context.WithTimeout(ctx, time.Second)
 	err = p.Pin(mctx, a, true)