From 7e91da20673f97cac2ff0729402d678751f5cae3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Mur=C3=A9?= <batolettre@gmail.com>
Date: Mon, 8 May 2017 17:00:00 +0900
Subject: [PATCH] Future-proof keystore.Has by returning an error as well
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

License: MIT
Signed-off-by: Michael Muré <batolettre@gmail.com>
---
 core/commands/keystore.go    | 14 +++++++++++---
 keystore/keystore.go         | 14 +++++++++++---
 keystore/keystore_test.go    | 12 ++++++++++--
 keystore/memkeystore.go      |  4 ++--
 keystore/memkeystore_test.go | 12 ++++++++++--
 5 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/core/commands/keystore.go b/core/commands/keystore.go
index abd71ed89..e0d9b5d10 100644
--- a/core/commands/keystore.go
+++ b/core/commands/keystore.go
@@ -260,13 +260,21 @@ var keyRenameCmd = &cmds.Command{
 
 		overwrite := false
 		force, _, _ := res.Request().Option("f").Bool()
-		if force && ks.Has(newName) {
-			overwrite = true
-			err := ks.Delete(newName)
+		if force {
+			exist, err := ks.Has(newName)
 			if err != nil {
 				res.SetError(err, cmds.ErrNormal)
 				return
 			}
+
+			if exist {
+				overwrite = true
+				err := ks.Delete(newName)
+				if err != nil {
+					res.SetError(err, cmds.ErrNormal)
+					return
+				}
+			}
 		}
 
 		err = ks.Put(newName, oldKey)
diff --git a/keystore/keystore.go b/keystore/keystore.go
index de0b62dc6..e38211480 100644
--- a/keystore/keystore.go
+++ b/keystore/keystore.go
@@ -11,7 +11,7 @@ import (
 )
 
 type Keystore interface {
-	Has(string) bool
+	Has(string) (bool, error)
 	Put(string, ci.PrivKey) error
 	Get(string) (ci.PrivKey, error)
 	Delete(string) error
@@ -55,12 +55,20 @@ func NewFSKeystore(dir string) (*FSKeystore, error) {
 	return &FSKeystore{dir}, nil
 }
 
-func (ks *FSKeystore) Has(name string) bool {
+func (ks *FSKeystore) Has(name string) (bool, error) {
 	kp := filepath.Join(ks.dir, name)
 
 	_, err := os.Stat(kp)
 
-	return err == nil
+	if os.IsNotExist(err) {
+		return false, nil
+	}
+
+	if err != nil {
+		return false, err
+	}
+
+	return true, nil
 }
 
 func (ks *FSKeystore) Put(name string, k ci.PrivKey) error {
diff --git a/keystore/keystore_test.go b/keystore/keystore_test.go
index 505d9119d..53c30b0d7 100644
--- a/keystore/keystore_test.go
+++ b/keystore/keystore_test.go
@@ -82,13 +82,21 @@ func TestKeystoreBasics(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if !ks.Has("foo") {
+	exist, err := ks.Has("foo")
+	if !exist {
 		t.Fatal("should know it has a key named foo")
 	}
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	if ks.Has("nonexistingkey") {
+	exist, err = ks.Has("nonexistingkey")
+	if exist {
 		t.Fatal("should know it doesn't have a key named nonexistingkey")
 	}
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if err := ks.Delete("bar"); err != nil {
 		t.Fatal(err)
diff --git a/keystore/memkeystore.go b/keystore/memkeystore.go
index 0018ade4d..626ad8bc0 100644
--- a/keystore/memkeystore.go
+++ b/keystore/memkeystore.go
@@ -10,9 +10,9 @@ func NewMemKeystore() *MemKeystore {
 	return &MemKeystore{make(map[string]ci.PrivKey)}
 }
 
-func (mk *MemKeystore) Has(name string) bool {
+func (mk *MemKeystore) Has(name string) (bool, error) {
 	_, ok := mk.keys[name]
-	return ok
+	return ok, nil
 }
 
 func (mk *MemKeystore) Put(name string, k ci.PrivKey) error {
diff --git a/keystore/memkeystore_test.go b/keystore/memkeystore_test.go
index 913f14b83..62533d54b 100644
--- a/keystore/memkeystore_test.go
+++ b/keystore/memkeystore_test.go
@@ -47,13 +47,21 @@ func TestMemKeyStoreBasics(t *testing.T) {
 		t.Fatal("should not be able to overwrite key")
 	}
 
-	if !ks.Has("foo") {
+	exist, err := ks.Has("foo")
+	if !exist {
 		t.Fatal("should know it has a key named foo")
 	}
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	if ks.Has("nonexistingkey") {
+	exist, err = ks.Has("nonexistingkey")
+	if exist {
 		t.Fatal("should know it doesn't have a key named nonexistingkey")
 	}
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if err := ks.Delete("bar"); err != nil {
 		t.Fatal(err)