Merge pull request #5843 from overbool/refactor/coreapi/pin-rm

cmds/pin: use coreapi/pin

core/commands/pin.go

@@ -10,15 +10,14 @@ import (
 	core "github.com/ipfs/go-ipfs/core"
 	cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv"
 	e "github.com/ipfs/go-ipfs/core/commands/e"
-	iface "github.com/ipfs/go-ipfs/core/coreapi/interface"
+	coreiface "github.com/ipfs/go-ipfs/core/coreapi/interface"
 	options "github.com/ipfs/go-ipfs/core/coreapi/interface/options"
-	corerepo "github.com/ipfs/go-ipfs/core/corerepo"
 	pin "github.com/ipfs/go-ipfs/pin"
 
 	cid "gx/ipfs/QmR8BauakNcBa3RbE4nbQu76PDiJgoQgz8AJdhJuiU4TAw/go-cid"
 	bserv "gx/ipfs/QmVKQHuzni68SWByzJgBUCwHvvr4TWiXfutNWWwpZpp4rE/go-blockservice"
 	cmds "gx/ipfs/QmWGm4AbZEbnmdgVTza52MSNpEmBdFVqzmAysRbjrRyGbH/go-ipfs-cmds"
-	"gx/ipfs/QmYMQuypUbgsdNHmuCBSUJV6wdQVsBHRivNAp3efHJwZJD/go-verifcid"
+	verifcid "gx/ipfs/QmYMQuypUbgsdNHmuCBSUJV6wdQVsBHRivNAp3efHJwZJD/go-verifcid"
 	offline "gx/ipfs/QmYZwey1thDTynSrvd6qQkX24UpTka6TFhQ2v569UpoqxD/go-ipfs-exchange-offline"
 	dag "gx/ipfs/Qmb2UEG2TAeVrEJSjqsZF7Y2he7wRDkrdt6c3bECxwZf4k/go-merkledag"
 	cidenc "gx/ipfs/QmdPQx9fvN5ExVwMhRmh7YpCQJzJrFhd1AjVBwJmRMFJeX/go-cidutil/cidenc"
@@ -68,18 +67,11 @@ var addPinCmd = &cmds.Command{
 	},
 	Type: AddPinOutput{},
 	Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
-		n, err := cmdenv.GetNode(env)
-		if err != nil {
-			return err
-		}
-
 		api, err := cmdenv.GetApi(env, req)
 		if err != nil {
 			return err
 		}
 
-		defer n.Blockstore.PinLock().Unlock()
-
 		// set recursive flag
 		recursive, _ := req.Options[pinRecursiveOptionName].(bool)
 		showProgress, _ := req.Options[pinProgressOptionName].(bool)
@@ -94,24 +86,25 @@ var addPinCmd = &cmds.Command{
 		}
 
 		if !showProgress {
-			added, err := corerepo.Pin(n.Pinning, api, req.Context, req.Arguments, recursive)
+			added, err := pinAddMany(req.Context, api, enc, req.Arguments, recursive)
 			if err != nil {
 				return err
 			}
-			return cmds.EmitOnce(res, &AddPinOutput{Pins: cidsToStrings(added, enc)})
+
+			return cmds.EmitOnce(res, &AddPinOutput{Pins: added})
 		}
 
 		v := new(dag.ProgressTracker)
 		ctx := v.DeriveContext(req.Context)
 
 		type pinResult struct {
-			pins []cid.Cid
+			pins []string
 			err  error
 		}
 
 		ch := make(chan pinResult, 1)
 		go func() {
-			added, err := corerepo.Pin(n.Pinning, api, ctx, req.Arguments, recursive)
+			added, err := pinAddMany(ctx, api, enc, req.Arguments, recursive)
 			ch <- pinResult{pins: added, err: err}
 		}()
 
@@ -130,7 +123,7 @@ var addPinCmd = &cmds.Command{
 						return err
 					}
 				}
-				return res.Emit(&AddPinOutput{Pins: cidsToStrings(val.pins, enc)})
+				return res.Emit(&AddPinOutput{Pins: val.pins})
 			case <-ticker.C:
 				if err := res.Emit(&AddPinOutput{Progress: v.Value()}); err != nil {
 					return err
@@ -187,6 +180,28 @@ var addPinCmd = &cmds.Command{
 	},
 }
 
+func pinAddMany(ctx context.Context, api coreiface.CoreAPI, enc cidenc.Encoder, paths []string, recursive bool) ([]string, error) {
+	added := make([]string, len(paths))
+	for i, b := range paths {
+		p, err := coreiface.ParsePath(b)
+		if err != nil {
+			return nil, err
+		}
+
+		rp, err := api.ResolvePath(ctx, p)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := api.Pin().Add(ctx, rp, options.Pin.Recursive(recursive)); err != nil {
+			return nil, err
+		}
+		added[i] = enc.Encode(rp.Cid())
+	}
+
+	return added, nil
+}
+
 var rmPinCmd = &cmds.Command{
 	Helptext: cmdkit.HelpText{
 		Tagline: "Remove pinned objects from local storage.",
@@ -204,11 +219,6 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.)
 	},
 	Type: PinOutput{},
 	Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
-		n, err := cmdenv.GetNode(env)
-		if err != nil {
-			return err
-		}
-
 		api, err := cmdenv.GetApi(env, req)
 		if err != nil {
 			return err
@@ -226,18 +236,33 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.)
 			return err
 		}
 
-		removed, err := corerepo.Unpin(n.Pinning, api, req.Context, req.Arguments, recursive)
-		if err != nil {
-			return err
+		pins := make([]string, 0, len(req.Arguments))
+		for _, b := range req.Arguments {
+			p, err := coreiface.ParsePath(b)
+			if err != nil {
+				return err
+			}
+
+			rp, err := api.ResolvePath(req.Context, p)
+			if err != nil {
+				return err
+			}
+
+			id := enc.Encode(rp.Cid())
+			pins = append(pins, id)
+			if err := api.Pin().Rm(req.Context, rp, options.Pin.RmRecursive(recursive)); err != nil {
+				return err
+			}
 		}
 
-		return cmds.EmitOnce(res, &PinOutput{cidsToStrings(removed, enc)})
+		return cmds.EmitOnce(res, &PinOutput{pins})
 	},
 	Encoders: cmds.EncoderMap{
 		cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, out *PinOutput) error {
 			for _, k := range out.Pins {
 				fmt.Fprintf(w, "unpinned %s\n", k)
 			}
+
 			return nil
 		}),
 	},
@@ -392,12 +417,12 @@ new pin and removing the old one.
 
 		unpin, _ := req.Options[pinUnpinOptionName].(bool)
 
-		from, err := iface.ParsePath(req.Arguments[0])
+		from, err := coreiface.ParsePath(req.Arguments[0])
 		if err != nil {
 			return err
 		}
 
-		to, err := iface.ParsePath(req.Arguments[1])
+		to, err := coreiface.ParsePath(req.Arguments[1])
 		if err != nil {
 			return err
 		}
@@ -479,7 +504,7 @@ type RefKeyList struct {
 	Keys map[string]RefKeyObject
 }
 
-func pinLsKeys(ctx context.Context, args []string, typeStr string, n *core.IpfsNode, api iface.CoreAPI) (map[cid.Cid]RefKeyObject, error) {
+func pinLsKeys(ctx context.Context, args []string, typeStr string, n *core.IpfsNode, api coreiface.CoreAPI) (map[cid.Cid]RefKeyObject, error) {
 
 	mode, ok := pin.StringToMode(typeStr)
 	if !ok {
@@ -489,7 +514,7 @@ func pinLsKeys(ctx context.Context, args []string, typeStr string, n *core.IpfsN
 	keys := make(map[cid.Cid]RefKeyObject)
 
 	for _, p := range args {
-		pth, err := iface.ParsePath(p)
+		pth, err := coreiface.ParsePath(p)
 		if err != nil {
 			return nil, err
 		}
@@ -652,11 +677,3 @@ func (r PinVerifyRes) Format(out io.Writer) {
 		}
 	}
 }
-
-func cidsToStrings(cs []cid.Cid, enc cidenc.Encoder) []string {
-	out := make([]string, 0, len(cs))
-	for _, c := range cs {
-		out = append(out, enc.Encode(c))
-	}
-	return out
-}

core/coreapi/interface/options/pin.go

@@ -8,12 +8,22 @@ type PinLsSettings struct {
 	Type string
 }
 
+// PinRmSettings represents the settings of pin rm command
+type PinRmSettings struct {
+	Recursive bool
+}
+
 type PinUpdateSettings struct {
 	Unpin bool
 }
 
 type PinAddOption func(*PinAddSettings) error
-type PinLsOption func(settings *PinLsSettings) error
+
+// PinRmOption pin rm option func
+type PinRmOption func(*PinRmSettings) error
+
+// PinLsOption pin ls option func
+type PinLsOption func(*PinLsSettings) error
 type PinUpdateOption func(*PinUpdateSettings) error
 
 func PinAddOptions(opts ...PinAddOption) (*PinAddSettings, error) {
@@ -31,6 +41,21 @@ func PinAddOptions(opts ...PinAddOption) (*PinAddSettings, error) {
 	return options, nil
 }
 
+// PinRmOptions pin rm options
+func PinRmOptions(opts ...PinRmOption) (*PinRmSettings, error) {
+	options := &PinRmSettings{
+		Recursive: true,
+	}
+
+	for _, opt := range opts {
+		if err := opt(options); err != nil {
+			return nil, err
+		}
+	}
+
+	return options, nil
+}
+
 func PinLsOptions(opts ...PinLsOption) (*PinLsSettings, error) {
 	options := &PinLsSettings{
 		Type: "all",
@@ -102,6 +127,14 @@ func (pinOpts) Recursive(recursive bool) PinAddOption {
 	}
 }
 
+// RmRecursive is an option for Pin.Rm
+func (pinOpts) RmRecursive(recursive bool) PinRmOption {
+	return func(settings *PinRmSettings) error {
+		settings.Recursive = recursive
+		return nil
+	}
+}
+
 // Type is an option for Pin.Ls which allows to specify which pin types should
 // be returned
 //

core/coreapi/interface/pin.go

@@ -43,7 +43,7 @@ type PinAPI interface {
 	Ls(context.Context, ...options.PinLsOption) ([]Pin, error)
 
 	// Rm removes pin for object specified by the path
-	Rm(context.Context, Path) error
+	Rm(context.Context, Path, ...options.PinRmOption) error
 
 	// Update changes one pin to another, skipping checks for matching paths in
 	// the old tree

core/coreapi/pin.go

@@ -6,7 +6,6 @@ import (
 
 	coreiface "github.com/ipfs/go-ipfs/core/coreapi/interface"
 	caopts "github.com/ipfs/go-ipfs/core/coreapi/interface/options"
-	corerepo "github.com/ipfs/go-ipfs/core/corerepo"
 	bserv "gx/ipfs/QmVKQHuzni68SWByzJgBUCwHvvr4TWiXfutNWWwpZpp4rE/go-blockservice"
 	merkledag "gx/ipfs/Qmb2UEG2TAeVrEJSjqsZF7Y2he7wRDkrdt6c3bECxwZf4k/go-merkledag"
 
@@ -17,21 +16,21 @@ import (
 type PinAPI CoreAPI
 
 func (api *PinAPI) Add(ctx context.Context, p coreiface.Path, opts ...caopts.PinAddOption) error {
-	settings, err := caopts.PinAddOptions(opts...)
+	dagNode, err := api.core().ResolveNode(ctx, p)
 	if err != nil {
-		return err
+		return fmt.Errorf("pin: %s", err)
 	}
 
-	rp, err := api.core().ResolvePath(ctx, p)
+	settings, err := caopts.PinAddOptions(opts...)
 	if err != nil {
 		return err
 	}
 
 	defer api.blockstore.PinLock().Unlock()
 
-	_, err = corerepo.Pin(api.pinning, api.core(), ctx, []string{rp.Cid().String()}, settings.Recursive)
+	err = api.pinning.Pin(ctx, dagNode, settings.Recursive)
 	if err != nil {
-		return err
+		return fmt.Errorf("pin: %s", err)
 	}
 
 	return api.pinning.Flush()
@@ -52,12 +51,22 @@ func (api *PinAPI) Ls(ctx context.Context, opts ...caopts.PinLsOption) ([]coreif
 	return api.pinLsAll(settings.Type, ctx)
 }
 
-func (api *PinAPI) Rm(ctx context.Context, p coreiface.Path) error {
-	_, err := corerepo.Unpin(api.pinning, api.core(), ctx, []string{p.String()}, true)
+// Rm pin rm api
+func (api *PinAPI) Rm(ctx context.Context, p coreiface.Path, opts ...caopts.PinRmOption) error {
+	rp, err := api.core().ResolvePath(ctx, p)
 	if err != nil {
 		return err
 	}
 
+	settings, err := caopts.PinRmOptions(opts...)
+	if err != nil {
+		return err
+	}
+
+	if err = api.pinning.Unpin(ctx, rp.Cid(), settings.Recursive); err != nil {
+		return err
+	}
+
 	return api.pinning.Flush()
 }
 

core/corerepo/pinning.go

-/*
-Package corerepo provides pinning and garbage collection for local
-IPFS block services.
-
-IPFS nodes will keep local copies of any object that have either been
-added or requested locally.  Not all of these objects are worth
-preserving forever though, so the node administrator can pin objects
-they want to keep and unpin objects that they don't care about.
-
-Garbage collection sweeps iterate through the local block store
-removing objects that aren't pinned, which frees storage space for new
-objects.
-*/
-package corerepo
-
-import (
-	"context"
-	"fmt"
-	"github.com/ipfs/go-ipfs/pin"
-
-	"github.com/ipfs/go-ipfs/core/coreapi/interface"
-
-	"gx/ipfs/QmR8BauakNcBa3RbE4nbQu76PDiJgoQgz8AJdhJuiU4TAw/go-cid"
-)
-
-func Pin(pinning pin.Pinner, api iface.CoreAPI, ctx context.Context, paths []string, recursive bool) ([]cid.Cid, error) {
-	out := make([]cid.Cid, len(paths))
-
-	for i, fpath := range paths {
-		p, err := iface.ParsePath(fpath)
-		if err != nil {
-			return nil, err
-		}
-
-		dagnode, err := api.ResolveNode(ctx, p)
-		if err != nil {
-			return nil, fmt.Errorf("pin: %s", err)
-		}
-		err = pinning.Pin(ctx, dagnode, recursive)
-		if err != nil {
-			return nil, fmt.Errorf("pin: %s", err)
-		}
-		out[i] = dagnode.Cid()
-	}
-
-	err := pinning.Flush()
-	if err != nil {
-		return nil, err
-	}
-
-	return out, nil
-}
-
-func Unpin(pinning pin.Pinner, api iface.CoreAPI, ctx context.Context, paths []string, recursive bool) ([]cid.Cid, error) {
-	unpinned := make([]cid.Cid, len(paths))
-
-	for i, p := range paths {
-		p, err := iface.ParsePath(p)
-		if err != nil {
-			return nil, err
-		}
-
-		k, err := api.ResolvePath(ctx, p)
-		if err != nil {
-			return nil, err
-		}
-
-		err = pinning.Unpin(ctx, k.Cid(), recursive)
-		if err != nil {
-			return nil, err
-		}
-		unpinned[i] = k.Cid()
-	}
-
-	err := pinning.Flush()
-	if err != nil {
-		return nil, err
-	}
-	return unpinned, nil
-}

test/sharness/t0080-repo.sh

@@ -93,7 +93,7 @@ test_expect_success "pinning directly should fail now" '
 '
 
 test_expect_success "'ipfs pin rm -r=false <hash>' should fail" '
-  echo "Error: $HASH is pinned recursively" >expected4 &&
+  echo "Error: $HASH is pinned recursively" >expected4
   test_must_fail ipfs pin rm -r=false "$HASH" 2>actual4 &&
   test_cmp expected4 actual4
 '