Merge pull request #2201 from noffle/unpin-2155

Resolve paths in 'pin rm' /wo network lookup

core/corerepo/pinning.go

@@ -60,22 +60,21 @@ func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool)
 
 func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) ([]key.Key, error) {
 
-	dagnodes := make([]*merkledag.Node, 0)
-	for _, fpath := range paths {
-		dagnode, err := core.Resolve(ctx, n, path.Path(fpath))
+	var unpinned []key.Key
+	for _, p := range paths {
+		p, err := path.ParsePath(p)
 		if err != nil {
 			return nil, err
 		}
-		dagnodes = append(dagnodes, dagnode)
-	}
 
-	var unpinned []key.Key
-	for _, dagnode := range dagnodes {
-		k, _ := dagnode.Key()
+		k, err := core.ResolveToKey(ctx, n, p)
+		if err != nil {
+			return nil, err
+		}
 
 		ctx, cancel := context.WithCancel(ctx)
 		defer cancel()
-		err := n.Pinning.Unpin(ctx, k, recursive)
+		err = n.Pinning.Unpin(ctx, k, recursive)
 		if err != nil {
 			return nil, err
 		}

core/pathresolver.go

@@ -6,6 +6,7 @@ import (
 
 	context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"
 
+	key "github.com/ipfs/go-ipfs/blocks/key"
 	merkledag "github.com/ipfs/go-ipfs/merkledag"
 	path "github.com/ipfs/go-ipfs/path"
 )
@@ -55,3 +56,37 @@ func Resolve(ctx context.Context, n *IpfsNode, p path.Path) (*merkledag.Node, er
 	// ok, we have an ipfs path now (or what we'll treat as one)
 	return n.Resolver.ResolvePath(ctx, p)
 }
+
+// ResolveToKey resolves a path to a key.
+//
+// It first checks if the path is already in the form of just a key (<key> or
+// /ipfs/<key>) and returns immediately if so. Otherwise, it falls back onto
+// Resolve to perform resolution of the dagnode being referenced.
+func ResolveToKey(ctx context.Context, n *IpfsNode, p path.Path) (key.Key, error) {
+
+	// If the path is simply a key, parse and return it. Parsed paths are already
+	// normalized (read: prepended with /ipfs/ if needed), so segment[1] should
+	// always be the key.
+	if p.IsJustAKey() {
+		return key.B58KeyDecode(p.Segments()[1]), nil
+	}
+
+	// Fall back onto regular dagnode resolution. Retrieve the second-to-last
+	// segment of the path and resolve its link to the last segment.
+	head, tail, err := p.PopLastSegment()
+	if err != nil {
+		return key.Key(""), err
+	}
+	dagnode, err := Resolve(ctx, n, head)
+	if err != nil {
+		return key.Key(""), err
+	}
+
+	// Extract and return the key of the link to the target dag node.
+	link, err := dagnode.GetNodeLink(tail)
+	if err != nil {
+		return key.Key(""), err
+	}
+
+	return key.Key(link.Hash), nil
+}

path/path.go

@@ -44,6 +44,30 @@ func (p Path) String() string {
 	return string(p)
 }
 
+// IsJustAKey returns true if the path is of the form <key> or /ipfs/<key>.
+func (p Path) IsJustAKey() bool {
+	parts := p.Segments()
+	return (len(parts) == 2 && parts[0] == "ipfs")
+}
+
+// PopLastSegment returns a new Path without its final segment, and the final
+// segment, separately. If there is no more to pop (the path is just a key),
+// the original path is returned.
+func (p Path) PopLastSegment() (Path, string, error) {
+
+	if p.IsJustAKey() {
+		return p, "", nil
+	}
+
+	segs := p.Segments()
+	newPath, err := ParsePath("/" + strings.Join(segs[:len(segs)-1], "/"))
+	if err != nil {
+		return "", "", err
+	}
+
+	return newPath, segs[len(segs)-1], nil
+}
+
 func FromSegments(prefix string, seg ...string) (Path, error) {
 	return ParsePath(prefix + strings.Join(seg, "/"))
 }

path/path_test.go

@@ -28,3 +28,52 @@ func TestPathParsing(t *testing.T) {
 		}
 	}
 }
+
+func TestIsJustAKey(t *testing.T) {
+	cases := map[string]bool{
+		"QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n":           true,
+		"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n":     true,
+		"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a":   false,
+		"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a/b": false,
+		"/ipns/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n":     false,
+	}
+
+	for p, expected := range cases {
+		path, err := ParsePath(p)
+		if err != nil {
+			t.Fatalf("ParsePath failed to parse \"%s\", but should have succeeded", p)
+		}
+		result := path.IsJustAKey()
+		if result != expected {
+			t.Fatalf("expected IsJustAKey(%s) to return %v, not %v", p, expected, result)
+		}
+	}
+}
+
+func TestPopLastSegment(t *testing.T) {
+	cases := map[string][]string{
+		"QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n":             []string{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n", ""},
+		"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n":       []string{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n", ""},
+		"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a":     []string{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n", "a"},
+		"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a/b":   []string{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a", "b"},
+		"/ipns/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/x/y/z": []string{"/ipns/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/x/y", "z"},
+	}
+
+	for p, expected := range cases {
+		path, err := ParsePath(p)
+		if err != nil {
+			t.Fatalf("ParsePath failed to parse \"%s\", but should have succeeded", p)
+		}
+		head, tail, err := path.PopLastSegment()
+		if err != nil {
+			t.Fatalf("PopLastSegment failed, but should have succeeded: %s", err)
+		}
+		headStr := head.String()
+		if headStr != expected[0] {
+			t.Fatalf("expected head of PopLastSegment(%s) to return %v, not %v", p, expected[0], headStr)
+		}
+		if tail != expected[1] {
+			t.Fatalf("expected tail of PopLastSegment(%s) to return %v, not %v", p, expected[1], tail)
+		}
+	}
+}

test/sharness/t0081-repo-pinning.sh

@@ -272,6 +272,15 @@ test_expect_success "test add nopin dir" '
 
 '
 
+FICTIONAL_HASH="QmXV4f9v8a56MxWKBhP3ETsz4EaafudU1cKfPaaJnenc48"
+test_launch_ipfs_daemon
+test_expect_success "test unpinning a hash that's not pinned" "
+  test_expect_code 1 ipfs pin rm $FICTIONAL_HASH --timeout=5s
+  test_expect_code 1 ipfs pin rm $FICTIONAL_HASH/a --timeout=5s
+  test_expect_code 1 ipfs pin rm $FICTIONAL_HASH/a/b --timeout=5s
+"
+test_kill_ipfs_daemon
+
 # test_kill_ipfs_daemon
 
 test_done