pin: add context and error return to most of the Pinner functions

gc/gc.go

@@ -201,7 +201,11 @@ func ColoredSet(ctx context.Context, pn pin.Pinner, ng ipld.NodeGetter, bestEffo
 		}
 		return links, nil
 	}
-	err := Descendants(ctx, getLinks, gcs, pn.RecursiveKeys())
+	rkeys, err := pn.RecursiveKeys(ctx)
+	if err != nil {
+		return nil, err
+	}
+	err = Descendants(ctx, getLinks, gcs, rkeys)
 	if err != nil {
 		errors = true
 		select {
@@ -233,11 +237,19 @@ func ColoredSet(ctx context.Context, pn pin.Pinner, ng ipld.NodeGetter, bestEffo
 		}
 	}
 
-	for _, k := range pn.DirectKeys() {
+	dkeys, err := pn.DirectKeys(ctx)
+	if err != nil {
+		return nil, err
+	}
+	for _, k := range dkeys {
 		gcs.Add(k)
 	}
 
-	err = Descendants(ctx, getLinks, gcs, pn.InternalPins())
+	ikeys, err := pn.InternalPins(ctx)
+	if err != nil {
+		return nil, err
+	}
+	err = Descendants(ctx, getLinks, gcs, ikeys)
 	if err != nil {
 		errors = true
 		select {

pin.go

@@ -105,11 +105,11 @@ func StringToMode(s string) (Mode, bool) {
 type Pinner interface {
 	// IsPinned returns whether or not the given cid is pinned
 	// and an explanation of why its pinned
-	IsPinned(cid.Cid) (string, bool, error)
+	IsPinned(ctx context.Context, c cid.Cid) (string, bool, error)
 
 	// IsPinnedWithType returns whether or not the given cid is pinned with the
 	// given pin type, as well as returning the type of pin its pinned with.
-	IsPinnedWithType(cid.Cid, Mode) (string, bool, error)
+	IsPinnedWithType(ctx context.Context, c cid.Cid, mode Mode) (string, bool, error)
 
 	// Pin the given node, optionally recursively.
 	Pin(ctx context.Context, node ipld.Node, recursive bool) error
@@ -125,7 +125,7 @@ type Pinner interface {
 
 	// Check if a set of keys are pinned, more efficient than
 	// calling IsPinned for each key
-	CheckIfPinned(cids ...cid.Cid) ([]Pinned, error)
+	CheckIfPinned(ctx context.Context, cids ...cid.Cid) ([]Pinned, error)
 
 	// PinWithMode is for manually editing the pin structure. Use with
 	// care! If used improperly, garbage collection may not be
@@ -138,17 +138,17 @@ type Pinner interface {
 	RemovePinWithMode(cid.Cid, Mode)
 
 	// Flush writes the pin state to the backing datastore
-	Flush() error
+	Flush(ctx context.Context) error
 
 	// DirectKeys returns all directly pinned cids
-	DirectKeys() []cid.Cid
+	DirectKeys(ctx context.Context) ([]cid.Cid, error)
 
 	// DirectKeys returns all recursively pinned cids
-	RecursiveKeys() []cid.Cid
+	RecursiveKeys(ctx context.Context) ([]cid.Cid, error)
 
 	// InternalPins returns all cids kept pinned for the internal state of the
 	// pinner
-	InternalPins() []cid.Cid
+	InternalPins(ctx context.Context) ([]cid.Cid, error)
 }
 
 // Pinned represents CID which has been pinned with a pinning strategy.
@@ -211,8 +211,6 @@ func NewPinner(dstore ds.Datastore, serv, internal ipld.DAGService) Pinner {
 
 // Pin the given node, optionally recursive
 func (p *pinner) Pin(ctx context.Context, node ipld.Node, recurse bool) error {
-	p.lock.Lock()
-	defer p.lock.Unlock()
 	err := p.dserv.Add(ctx, node)
 	if err != nil {
 		return err
@@ -220,13 +218,16 @@ func (p *pinner) Pin(ctx context.Context, node ipld.Node, recurse bool) error {
 
 	c := node.Cid()
 
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
 	if recurse {
 		if p.recursePin.Has(c) {
 			return nil
 		}
 
 		p.lock.Unlock()
-		// fetch entire graph
+		// temporary unlock to fetch the entire graph
 		err := mdag.FetchGraph(ctx, c, p.dserv)
 		p.lock.Lock()
 		if err != nil {
@@ -243,13 +244,6 @@ func (p *pinner) Pin(ctx context.Context, node ipld.Node, recurse bool) error {
 
 		p.recursePin.Add(c)
 	} else {
-		p.lock.Unlock()
-		_, err := p.dserv.Get(ctx, c)
-		p.lock.Lock()
-		if err != nil {
-			return err
-		}
-
 		if p.recursePin.Has(c) {
 			return fmt.Errorf("%s already pinned recursively", c.String())
 		}
@@ -286,15 +280,13 @@ func (p *pinner) isInternalPin(c cid.Cid) bool {
 
 // IsPinned returns whether or not the given key is pinned
 // and an explanation of why its pinned
-func (p *pinner) IsPinned(c cid.Cid) (string, bool, error) {
-	p.lock.RLock()
-	defer p.lock.RUnlock()
+func (p *pinner) IsPinned(ctx context.Context, c cid.Cid) (string, bool, error) {
 	return p.isPinnedWithType(c, Any)
 }
 
 // IsPinnedWithType returns whether or not the given cid is pinned with the
 // given pin type, as well as returning the type of pin its pinned with.
-func (p *pinner) IsPinnedWithType(c cid.Cid, mode Mode) (string, bool, error) {
+func (p *pinner) IsPinnedWithType(ctx context.Context, c cid.Cid, mode Mode) (string, bool, error) {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 	return p.isPinnedWithType(c, mode)
@@ -347,7 +339,7 @@ func (p *pinner) isPinnedWithType(c cid.Cid, mode Mode) (string, bool, error) {
 
 // CheckIfPinned Checks if a set of keys are pinned, more efficient than
 // calling IsPinned for each key, returns the pinned status of cid(s)
-func (p *pinner) CheckIfPinned(cids ...cid.Cid) ([]Pinned, error) {
+func (p *pinner) CheckIfPinned(ctx context.Context, cids ...cid.Cid) ([]Pinned, error) {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 	pinned := make([]Pinned, 0, len(cids))
@@ -494,19 +486,19 @@ func LoadPinner(d ds.Datastore, dserv, internal ipld.DAGService) (Pinner, error)
 }
 
 // DirectKeys returns a slice containing the directly pinned keys
-func (p *pinner) DirectKeys() []cid.Cid {
+func (p *pinner) DirectKeys(ctx context.Context) ([]cid.Cid, error) {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 
-	return p.directPin.Keys()
+	return p.directPin.Keys(), nil
 }
 
 // RecursiveKeys returns a slice containing the recursively pinned keys
-func (p *pinner) RecursiveKeys() []cid.Cid {
+func (p *pinner) RecursiveKeys(ctx context.Context) ([]cid.Cid, error) {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 
-	return p.recursePin.Keys()
+	return p.recursePin.Keys(), nil
 }
 
 // Update updates a recursive pin from one cid to another
@@ -541,12 +533,10 @@ func (p *pinner) Update(ctx context.Context, from, to cid.Cid, unpin bool) error
 }
 
 // Flush encodes and writes pinner keysets to the datastore
-func (p *pinner) Flush() error {
+func (p *pinner) Flush(ctx context.Context) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
-	ctx := context.TODO()
-
 	internalset := cid.NewSet()
 	recordInternal := internalset.Add
 
@@ -594,12 +584,12 @@ func (p *pinner) Flush() error {
 
 // InternalPins returns all cids kept pinned for the internal state of the
 // pinner
-func (p *pinner) InternalPins() []cid.Cid {
+func (p *pinner) InternalPins(ctx context.Context) ([]cid.Cid, error) {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	var out []cid.Cid
 	out = append(out, p.internalPin.Keys()...)
-	return out
+	return out, nil
 }
 
 // PinWithMode allows the user to have fine grained control over pin

pin_test.go

@@ -31,7 +31,7 @@ func randNode() (*mdag.ProtoNode, cid.Cid) {
 }
 
 func assertPinned(t *testing.T, p Pinner, c cid.Cid, failmsg string) {
-	_, pinned, err := p.IsPinned(c)
+	_, pinned, err := p.IsPinned(context.Background(), c)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -42,7 +42,7 @@ func assertPinned(t *testing.T, p Pinner, c cid.Cid, failmsg string) {
 }
 
 func assertUnpinned(t *testing.T, p Pinner, c cid.Cid, failmsg string) {
-	_, pinned, err := p.IsPinned(c)
+	_, pinned, err := p.IsPinned(context.Background(), c)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -146,7 +146,7 @@ func TestPinnerBasic(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = p.Flush()
+	err = p.Flush(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -327,7 +327,7 @@ func TestFlush(t *testing.T) {
 	_, k := randNode()
 
 	p.PinWithMode(k, Recursive)
-	if err := p.Flush(); err != nil {
+	if err := p.Flush(context.Background()); err != nil {
 		t.Fatal(err)
 	}
 	assertPinned(t, p, k, "expected key to still be pinned")