fix(redis): Fixed a get/set error not returning

Since the previous "store.Store" interface did not return an error, the upper caller could not know that the storage error occurred. This problem will cause the upper caller to know the specific error message when a storage error occurs during the generation or verification of the verification code.
mojocn · Jul 3, 2019 · 927a16d · 927a16d
1 parent 87c9c59
commit 927a16d
Show file tree

Hide file tree

Showing 8 changed files with 231 additions and 42 deletions.
diff --git a/_examples_redis/main.go b/_examples_redis/main.go
@@ -19,15 +19,16 @@ type customizeRdsStore struct {
 }
 
 // customizeRdsStore implementing Set method of  Store interface
-func (s *customizeRdsStore) Set(id string, value string) {
-	err := s.redisClient.Set(id, value, time.Minute*10).Err()
+func (s *customizeRdsStore) Set(id string, value string) (err error) {
+	err = s.redisClient.Set(id, value, time.Minute*10).Err()
 	if err != nil {
 		panic(err)
 	}
+	return
 }
 
 // customizeRdsStore implementing Get method of  Store interface
-func (s *customizeRdsStore) Get(id string, clear bool) (value string) {
+func (s *customizeRdsStore) Get(id string, clear bool) (value string, err error) {
 	val, err := s.redisClient.Get(id).Result()
 	if err != nil {
 		panic(err)
@@ -38,7 +39,7 @@ func (s *customizeRdsStore) Get(id string, clear bool) (value string) {
 			panic(err)
 		}
 	}
-	return val
+	return val, err
 }
 
 func init() {
@@ -49,9 +50,9 @@ func init() {
 		DB:       0,  // use default DB
 	})
 	// init redis store
-	customeStore := customizeRdsStore{client}
+	customerStore := customizeRdsStore{client}
 
-	base64Captcha.SetCustomStore(&customeStore)
+	base64Captcha.SetCustomStore(&customerStore)
 
 }
 

diff --git a/captcha.go b/captcha.go
@@ -98,21 +98,54 @@ type CaptchaItem struct {
 
 // VerifyCaptcha by given id key and remove the captcha value in store, return boolean value.
 // 验证图像验证码,返回boolean.
+func VerifyCaptchaV2(identifier, verifyValue string) (bool, error) {
+	result, err := VerifyCaptchaAndIsClearV2(identifier, verifyValue, true)
+	if err != nil {
+		return false, err
+	}
+
+	return result, nil
+}
+
+// @deprecated
+// Please use VerifyCaptchaV2, this function can't let you know the internal error because there is no return error.
 func VerifyCaptcha(identifier, verifyValue string) bool {
-	return VerifyCaptchaAndIsClear(identifier, verifyValue, true)
+	result, err := VerifyCaptchaAndIsClearV2(identifier, verifyValue, true)
+	if err != nil {
+		log.Printf("get store error, %v", err)
+		return false
+	}
+
+	return result
 }
 
 // VerifyCaptchaAndIsClear verify captcha, return boolean value.
 // identifier is the captcha id,
 // verifyValue is the captcha image value,
 // isClear is whether to clear the value in store.
 // 验证图像验证码,返回boolean.
-func VerifyCaptchaAndIsClear(identifier, verifyValue string, isClear bool) bool {
+func VerifyCaptchaAndIsClearV2(identifier, verifyValue string, isClear bool) (bool, error) {
 	if verifyValue == "" {
+		return false, nil
+	}
+	storeValue, err := globalStore.Get(identifier, isClear)
+	if err != nil {
+		return false, err
+	}
+	return strings.ToLower(storeValue) == strings.ToLower(verifyValue), nil
+}
+
+// @deprecated
+// Please use VerifyCaptchaAndIsClearV2, this function can't let you know the internal error because there is no return error.
+func VerifyCaptchaAndIsClear(identifier, verifyValue string, isClear bool) bool {
+
+	result, err := VerifyCaptchaAndIsClearV2(identifier, verifyValue, isClear)
+	if err != nil {
+		log.Printf("get store error, %v", err)
 		return false
 	}
-	storeValue := globalStore.Get(identifier, isClear)
-	return strings.ToLower(storeValue) == strings.ToLower(verifyValue)
+
+	return result
 }
 
 // GenerateCaptcha create captcha by config struct and id.
@@ -165,7 +198,8 @@ func VerifyCaptchaAndIsClear(identifier, verifyValue string, isClear bool) bool
 // 	idKeyD,capD := base64Captcha.GenerateCaptcha("",configD)
 // 	//write to base64 string.
 // 	base64stringD := base64Captcha.CaptchaWriteToBase64Encoding(capD)
-func GenerateCaptcha(idKey string, configuration interface{}) (id string, captchaInstance CaptchaInterface) {
+func GenerateCaptchaV2(idKey string, configuration interface{}) (id string, captchaInstance CaptchaInterface, err error) {
+
 	if idKey == "" {
 		idKey = randomId()
 	}
@@ -191,9 +225,23 @@ func GenerateCaptcha(idKey string, configuration interface{}) (id string, captch
 		log.Fatal("config type not supported", config)
 	}
 
-	globalStore.Set(idKey, verifyValue)
+	err = globalStore.Set(idKey, verifyValue)
+
+	return idKey, captchaInstance, err
+}
+
+// @deprecated
+// Please use GenerateCaptchaV2, this function can't let you know the internal error because there is no return error.
+func GenerateCaptcha(idKey string, configuration interface{}) (id string, captchaInstance CaptchaInterface) {
+
+	var err error
+	id, captchaInstance, err = GenerateCaptchaV2(idKey, configuration)
+	if err != nil {
+		log.Printf("set store error, %v", err)
+		return "", nil
+	}
 
-	return idKey, captchaInstance
+	return
 }
 
 func pathExists(path string) bool {

diff --git a/captcha_test.go b/captcha_test.go
@@ -1,6 +1,8 @@
 package base64Captcha
 
 import (
+	"fmt"
+	"github.com/go-redis/redis"
 	"github.com/mojocn/base64Captcha/store"
 	"github.com/stretchr/testify/assert"
 	"io/ioutil"
@@ -72,6 +74,19 @@ func TestGenerateCaptcha(t *testing.T) {
 	}
 }
 
+func TestGenerateCaptchaOnFailed(t *testing.T) {
+
+	preStore := globalStore
+	s := store.NewRedisStore(redis.NewClient(&redis.Options{
+		Addr: "127.0.0.1:6378",
+	}), "test:", 5*time.Minute)
+	SetCustomStore(s)
+	idkey, capt := GenerateCaptcha("", configA)
+	assert.Equal(t, "", idkey)
+	assert.Nil(t, capt)
+	SetCustomStore(preStore)
+}
+
 func TestCaptchaWriteToBase64Encoding(t *testing.T) {
 	_, cap := GenerateCaptcha("", configD)
 	base64string := CaptchaWriteToBase64Encoding(cap)
@@ -90,7 +105,9 @@ func TestCaptchaWriteToBase64Encoding(t *testing.T) {
 
 func TestVerifyCaptcha(t *testing.T) {
 	idkey, _ := GenerateCaptcha("", configD)
-	verifyValue := globalStore.Get(idkey, false)
+	verifyValue, err := globalStore.Get(idkey, false)
+	assert.Nil(t, err)
+	fmt.Printf("idKey: %s, verifyValue: %s", idkey, verifyValue)
 	if VerifyCaptcha(idkey, verifyValue) {
 		t.Log(idkey, verifyValue)
 	} else {
@@ -99,7 +116,110 @@ func TestVerifyCaptcha(t *testing.T) {
 
 	VerifyCaptcha("", "")
 	VerifyCaptcha("dsafasf", "ddd")
+}
+
+func TestVerifyCaptchaOnFailed(t *testing.T) {
+
+	preStore := globalStore
+	s := store.NewRedisStore(redis.NewClient(&redis.Options{
+		Addr: "127.0.0.1:6378",
+	}), "test:", 5*time.Minute)
+	SetCustomStore(s)
+	assert.False(t, VerifyCaptcha("sss", "32321"))
+	SetCustomStore(preStore)
+}
+
+func TestVerifyCaptchaV2(t *testing.T) {
+	idkey, _, err := GenerateCaptchaV2("", configD)
+	assert.Nil(t, err)
+	verifyValue, err := globalStore.Get(idkey, false)
+	assert.Nil(t, err)
+	fmt.Printf("idKey: %s, verifyValue: %s", idkey, verifyValue)
+	result, err := VerifyCaptchaV2(idkey, verifyValue)
+	assert.Nil(t, err)
+	if result {
+		t.Log(idkey, verifyValue)
+	} else {
+		t.Error("verify captcha content is failed.")
+	}
+
+	VerifyCaptchaV2("", "")
+	VerifyCaptchaV2("dsafasf", "ddd")
+}
+
+func TestVerifyCaptchaV2OnFailed(t *testing.T) {
 
+	preStore := globalStore
+	s := store.NewRedisStore(redis.NewClient(&redis.Options{
+		Addr: "127.0.0.1:6378",
+	}), "test:", 5*time.Minute)
+	SetCustomStore(s)
+	result, err := VerifyCaptchaV2("sss", "32321")
+	assert.NotNil(t, err)
+	assert.False(t, result)
+	SetCustomStore(preStore)
+}
+
+func TestVerifyCaptchaAndIsClear(t *testing.T) {
+
+	idkey, _ := GenerateCaptcha("", configD)
+	verifyValue, err := globalStore.Get(idkey, false)
+	assert.Nil(t, err)
+	fmt.Printf("idKey: %s, verifyValue: %s", idkey, verifyValue)
+	if VerifyCaptchaAndIsClear(idkey, verifyValue, true) {
+		t.Log(idkey, verifyValue)
+	} else {
+		t.Error("verify captcha content is failed.")
+	}
+
+	VerifyCaptcha("", "")
+	VerifyCaptcha("dsafasf", "ddd")
+}
+
+func TestVerifyCaptchaAndIsClearOnFailed(t *testing.T) {
+
+	preStore := globalStore
+	s := store.NewRedisStore(redis.NewClient(&redis.Options{
+		Addr: "127.0.0.1:6378",
+	}), "test:", 5*time.Minute)
+	SetCustomStore(s)
+	assert.False(t, VerifyCaptchaAndIsClear("sss", "wded", true))
+	SetCustomStore(preStore)
+}
+
+func TestVerifyCaptchaAndIsClearV2(t *testing.T) {
+
+	idkey, _ := GenerateCaptcha("", configD)
+	verifyValue, err := globalStore.Get(idkey, false)
+	assert.Nil(t, err)
+	fmt.Printf("idKey: %s, verifyValue: %s", idkey, verifyValue)
+	result, err := VerifyCaptchaAndIsClearV2(idkey, verifyValue, true)
+	assert.Nil(t, err)
+	if result {
+		t.Log(idkey, verifyValue)
+	} else {
+		t.Error("verify captcha content is failed.")
+	}
+
+	result, err = VerifyCaptchaV2("", "")
+	assert.Nil(t, err)
+	assert.False(t, result)
+	result, err = VerifyCaptchaV2("dsafasf", "ddd")
+	assert.Nil(t, err)
+	assert.False(t, result)
+}
+
+func TestVerifyCaptchaAndIsClearV2OnFailed(t *testing.T) {
+
+	preStore := globalStore
+	s := store.NewRedisStore(redis.NewClient(&redis.Options{
+		Addr: "127.0.0.1:6378",
+	}), "test:", 5*time.Minute)
+	SetCustomStore(s)
+	result, err := VerifyCaptchaAndIsClearV2("sss", "dddd", true)
+	assert.NotNil(t, err)
+	assert.False(t, result)
+	SetCustomStore(preStore)
 }
 
 func TestPathExists(t *testing.T) {
@@ -130,7 +250,7 @@ func TestCaptchaWriteToFileCreateFileFailed(t *testing.T) {
 	assert.Nil(t, err)
 
 	err = CaptchaWriteToFile(captcha, noPermissionDirPath, idKey, "png")
-	//has no permission must failed
+	// has no permission must failed
 	if runtime.GOOS == "windows" {
 		assert.Nil(t, err)
 	} else {

diff --git a/store/interface.go b/store/interface.go
@@ -9,9 +9,9 @@ package store
 // method after the certain amount of captchas has been stored.)
 type Store interface {
 	// Set sets the digits for the captcha id.
-	Set(id string, value string)
+	Set(id string, value string) error
 
 	// Get returns stored digits for the captcha id. Clear indicates
 	// whether the captcha must be deleted from the store.
-	Get(id string, clear bool) string
+	Get(id string, clear bool) (string, error)
 }
diff --git a/store/memory.go b/store/memory.go
@@ -53,7 +53,7 @@ func NewMemoryStore(collectNum int, expiration time.Duration) Store {
 	return s
 }
 
-func (s *memoryStore) Set(id string, value string) {
+func (s *memoryStore) Set(id string, value string) (err error) {
 	s.Lock()
 	s.digitsById[id] = value
 	s.idByTime.PushBack(idByTimeValue{time.Now(), id})
@@ -62,9 +62,10 @@ func (s *memoryStore) Set(id string, value string) {
 	if s.numStored > s.collectNum {
 		go s.collect()
 	}
+	return nil
 }
 
-func (s *memoryStore) Get(id string, clear bool) (value string) {
+func (s *memoryStore) Get(id string, clear bool) (value string, err error) {
 	if !clear {
 		// When we don't need to clear captcha, acquire read lock.
 		s.RLock()