From ccb36277ddc13410e76dbae87beb7c0027056ef1 Mon Sep 17 00:00:00 2001
From: Jeromy <jeromyj@gmail.com>
Date: Tue, 20 Jan 2015 03:47:37 +0000
Subject: [PATCH] fix pinning UX, and add tests to match

---
 core/repo/pinning.go        |  2 +-
 pin/pin.go                  | 20 ++++++++++++--------
 pin/pin_test.go             |  4 ++--
 test/sharness/t0080-repo.sh | 11 +++++++++--
 4 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/core/repo/pinning.go b/core/repo/pinning.go
index 4bb3175ee..cc84dcc57 100644
--- a/core/repo/pinning.go
+++ b/core/repo/pinning.go
@@ -55,7 +55,7 @@ func Unpin(n *core.IpfsNode, paths []string, recursive bool) ([]u.Key, error) {
 	var unpinned []u.Key
 	for _, dagnode := range dagnodes {
 		k, _ := dagnode.Key()
-		err := n.Pinning.Unpin(k)
+		err := n.Pinning.Unpin(k, recursive)
 		if err != nil {
 			return nil, err
 		}
diff --git a/pin/pin.go b/pin/pin.go
index 0ce754103..ed3598e4d 100644
--- a/pin/pin.go
+++ b/pin/pin.go
@@ -31,7 +31,7 @@ const (
 type Pinner interface {
 	IsPinned(util.Key) bool
 	Pin(*mdag.Node, bool) error
-	Unpin(util.Key) error
+	Unpin(util.Key, bool) error
 	Flush() error
 	GetManual() ManualPinner
 	DirectKeys() []util.Key
@@ -111,17 +111,21 @@ func (p *pinner) Pin(node *mdag.Node, recurse bool) error {
 }
 
 // Unpin a given key
-func (p *pinner) Unpin(k util.Key) error {
+func (p *pinner) Unpin(k util.Key, recursive bool) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 	if p.recursePin.HasKey(k) {
-		p.recursePin.RemoveBlock(k)
-		node, err := p.dserv.Get(k)
-		if err != nil {
-			return err
-		}
+		if recursive {
+			p.recursePin.RemoveBlock(k)
+			node, err := p.dserv.Get(k)
+			if err != nil {
+				return err
+			}
 
-		return p.unpinLinks(node)
+			return p.unpinLinks(node)
+		} else {
+			return errors.New("Key pinned recursively.")
+		}
 	} else if p.directPin.HasKey(k) {
 		p.directPin.RemoveBlock(k)
 		return nil
diff --git a/pin/pin_test.go b/pin/pin_test.go
index 3a93bdf74..623983a34 100644
--- a/pin/pin_test.go
+++ b/pin/pin_test.go
@@ -100,8 +100,8 @@ func TestPinnerBasic(t *testing.T) {
 		t.Fatal("pinned node not found.")
 	}
 
-	// Test unpin
-	err = p.Unpin(dk)
+	// Test recursive unpin
+	err = p.Unpin(dk, true)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh
index aa987c770..22fe38e05 100755
--- a/test/sharness/t0080-repo.sh
+++ b/test/sharness/t0080-repo.sh
@@ -25,7 +25,7 @@ test_expect_success "added file was pinned" '
 
 test_expect_success "'ipfs pin rm' succeeds" '
 	echo Unpinned `cat hashfile` > expected
-	ipfs pin rm `cat hashfile` > actual
+	ipfs pin rm -r `cat hashfile` > actual
 	test_cmp expected actual
 '
 
@@ -45,14 +45,21 @@ test_expect_success "pinning directly should fail now" '
 	test_cmp expected actual
 '
 
+test_expect_success "'ipfs pin rm <hash>' should fail" '
+	echo Error: Key pinned recursively. > expected
+	ipfs pin rm `cat hashfile` 2> error
+	test_cmp expected error
+'
+
 test_expect_success "remove recursive pin, add direct" '
 	echo Unpinned `cat hashfile` > expected
-	ipfs pin rm `cat hashfile` > actual
+	ipfs pin rm -r `cat hashfile` > actual
 	test_cmp expected actual
 	ipfs pin add `cat hashfile`
 '
 
 test_expect_success "remove direct pin" '
+	echo Unpinned `cat hashfile` > expected
 	ipfs pin rm `cat hashfile` > actual
 	test_cmp expected actual
 '