addressing comments from CR

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>

pin/gc/gc.go

@@ -100,7 +100,7 @@ func ColoredSet(pn pin.Pinner, ds dag.DAGService) (key.KeySet, error) {
 		gcs.Add(k)
 	}
 
-	err = Color(ds, gcs, pn.InternalPins())
+	err = Descendants(ds, gcs, pn.InternalPins())
 	if err != nil {
 		return nil, err
 	}

pin/pin.go

@@ -35,7 +35,7 @@ const (
 )
 
 type Pinner interface {
-	IsPinned(key.Key) bool
+	IsPinned(key.Key) (string, bool, error)
 	Pin(context.Context, *mdag.Node, bool) error
 	Unpin(context.Context, key.Key, bool) error
 
@@ -147,12 +147,38 @@ func (p *pinner) isInternalPin(key key.Key) bool {
 }
 
 // IsPinned returns whether or not the given key is pinned
-func (p *pinner) IsPinned(key key.Key) bool {
+// and an explanation of why its pinned
+func (p *pinner) IsPinned(k key.Key) (string, bool, error) {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
-	return p.recursePin.HasKey(key) ||
-		p.directPin.HasKey(key) ||
-		p.isInternalPin(key)
+	if p.recursePin.HasKey(k) {
+		return "recursive", true, nil
+	}
+	if p.directPin.HasKey(k) {
+		return "direct", true, nil
+	}
+	if p.isInternalPin(k) {
+		return "internal", true, nil
+	}
+
+	for _, rk := range p.recursePin.GetKeys() {
+		ss := &searchSet{target: k}
+
+		rnd, err := p.dserv.Get(context.Background(), rk)
+		if err != nil {
+			return "", false, err
+		}
+
+		err = mdag.EnumerateChildren(context.Background(), p.dserv, rnd, ss)
+		if err != nil {
+			return "", false, err
+		}
+
+		if ss.found {
+			return rk.B58String(), true, nil
+		}
+	}
+	return "", false, nil
 }
 
 func (p *pinner) RemovePinWithMode(key key.Key, mode PinMode) {
@@ -308,3 +334,27 @@ func (p *pinner) PinWithMode(k key.Key, mode PinMode) {
 		p.directPin.AddBlock(k)
 	}
 }
+
+// searchSet implements key.KeySet in
+type searchSet struct {
+	target key.Key
+	found  bool
+}
+
+func (ss *searchSet) Add(k key.Key) {
+	if ss.target == k {
+		ss.found = true
+	}
+}
+
+func (ss *searchSet) Has(k key.Key) bool {
+	// returning true to all Has queries will cause EnumerateChildren to return
+	// almost immediately
+	return ss.found
+}
+
+func (ss *searchSet) Keys() []key.Key {
+	return nil
+}
+
+func (ss *searchSet) Remove(key.Key) {}

pin/pin_test.go

@@ -24,6 +24,17 @@ func randNode() (*mdag.Node, key.Key) {
 	return nd, k
 }
 
+func assertPinned(t *testing.T, p Pinner, k key.Key, failmsg string) {
+	_, pinned, err := p.IsPinned(k)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !pinned {
+		t.Fatal(failmsg)
+	}
+}
+
 func TestPinnerBasic(t *testing.T) {
 	ctx := context.Background()
 
@@ -48,13 +59,11 @@ func TestPinnerBasic(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if !p.IsPinned(ak) {
-		t.Fatal("Failed to find key")
-	}
+	assertPinned(t, p, ak, "Failed to find key")
 
 	// create new node c, to be indirectly pinned through b
 	c, _ := randNode()
-	_, err = dserv.Add(c)
+	ck, err := dserv.Add(c)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -82,10 +91,10 @@ func TestPinnerBasic(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	assertPinned(t, p, ck, "child of recursively pinned node not found")
+
 	bk, _ := b.Key()
-	if !p.IsPinned(bk) {
-		t.Fatal("Recursively pinned node not found..")
-	}
+	assertPinned(t, p, bk, "Recursively pinned node not found..")
 
 	d, _ := randNode()
 	d.AddNodeLink("a", a)
@@ -107,9 +116,7 @@ func TestPinnerBasic(t *testing.T) {
 	}
 
 	dk, _ := d.Key()
-	if !p.IsPinned(dk) {
-		t.Fatal("pinned node not found.")
-	}
+	assertPinned(t, p, dk, "pinned node not found.")
 
 	// Test recursive unpin
 	err = p.Unpin(ctx, dk, true)
@@ -128,14 +135,10 @@ func TestPinnerBasic(t *testing.T) {
 	}
 
 	// Test directly pinned
-	if !np.IsPinned(ak) {
-		t.Fatal("Could not find pinned node!")
-	}
+	assertPinned(t, np, ak, "Could not find pinned node!")
 
 	// Test recursively pinned
-	if !np.IsPinned(bk) {
-		t.Fatal("could not find recursively pinned node")
-	}
+	assertPinned(t, np, bk, "could not find recursively pinned node")
 }
 
 func TestDuplicateSemantics(t *testing.T) {
@@ -187,8 +190,46 @@ func TestFlush(t *testing.T) {
 	if err := p.Flush(); err != nil {
 		t.Fatal(err)
 	}
-	if !p.IsPinned(k) {
-		t.Fatal("expected key to still be pinned")
+	assertPinned(t, p, k, "expected key to still be pinned")
+}
+
+func TestPinRecursiveFail(t *testing.T) {
+	ctx := context.Background()
+	dstore := dssync.MutexWrap(ds.NewMapDatastore())
+	bstore := blockstore.NewBlockstore(dstore)
+	bserv, err := bs.New(bstore, offline.Exchange(bstore))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dserv := mdag.NewDAGService(bserv)
+
+	p := NewPinner(dstore, dserv)
+
+	a, _ := randNode()
+	b, _ := randNode()
+	err = a.AddNodeLinkClean("child", b)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Note: this isnt a time based test, we expect the pin to fail
+	mctx, _ := context.WithTimeout(ctx, time.Millisecond)
+	err = p.Pin(mctx, a, true)
+	if err == nil {
+		t.Fatal("should have failed to pin here")
+	}
+
+	_, err = dserv.Add(b)
+	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)
+	if err != nil {
+		t.Fatal(err)
 	}
 }