From 2e88600c9366f149b9bd526f2488a0daf79104b0 Mon Sep 17 00:00:00 2001 From: kolaente Date: Sun, 31 Jan 2021 12:32:46 +0100 Subject: [PATCH] Change keyvalue.Get to return if a value exists or not instead of an error --- pkg/metrics/active_users.go | 2 +- pkg/metrics/metrics.go | 10 +++++----- pkg/modules/auth/openid/providers.go | 11 +++++------ pkg/modules/avatar/initials/initials.go | 13 ++++++------- pkg/modules/avatar/upload/upload.go | 7 +++---- pkg/modules/background/unsplash/unsplash.go | 7 +++---- pkg/modules/keyvalue/keyvalue.go | 4 ++-- pkg/modules/keyvalue/memory/memory.go | 7 +------ pkg/modules/keyvalue/redis/redis.go | 10 ++-------- 9 files changed, 28 insertions(+), 43 deletions(-) diff --git a/pkg/metrics/active_users.go b/pkg/metrics/active_users.go index f25ee1d2..ff8cd6fa 100644 --- a/pkg/metrics/active_users.go +++ b/pkg/metrics/active_users.go @@ -91,7 +91,7 @@ func SetUserActive(a web.Auth) (err error) { // getActiveUsers returns the active users from redis func getActiveUsers() (users activeUsersMap, err error) { - u, err := keyvalue.Get(ActiveUsersKey) + u, _, err := keyvalue.Get(ActiveUsersKey) if err != nil { return nil, err } diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index b29696f9..5f51b038 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -20,7 +20,6 @@ import ( "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/modules/keyvalue" - e "code.vikunja.io/api/pkg/modules/keyvalue/error" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" ) @@ -97,13 +96,14 @@ func InitMetrics() { // GetCount returns the current count from redis func GetCount(key string) (count int64, err error) { - cnt, err := keyvalue.Get(key) + cnt, exists, err := keyvalue.Get(key) if err != nil { - if e.IsErrValueNotFoundForKey(err) { - return 0, nil - } return 0, err } + if !exists { + return 0, nil + } + count = cnt.(int64) return diff --git a/pkg/modules/auth/openid/providers.go b/pkg/modules/auth/openid/providers.go index dad9aee6..7c6b7eba 100644 --- a/pkg/modules/auth/openid/providers.go +++ b/pkg/modules/auth/openid/providers.go @@ -24,7 +24,6 @@ import ( "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/modules/keyvalue" - kerr "code.vikunja.io/api/pkg/modules/keyvalue/error" "github.com/coreos/go-oidc" "golang.org/x/oauth2" ) @@ -35,8 +34,8 @@ func GetAllProviders() (providers []*Provider, err error) { return nil, nil } - ps, err := keyvalue.Get("openid_providers") - if err != nil && kerr.IsErrValueNotFoundForKey(err) { + ps, exists, err := keyvalue.Get("openid_providers") + if !exists { rawProviders := config.AuthOpenIDProviders.Get() if rawProviders == nil { return nil, nil @@ -73,14 +72,14 @@ func GetAllProviders() (providers []*Provider, err error) { // GetProvider retrieves a provider from keyvalue func GetProvider(key string) (provider *Provider, err error) { var p interface{} - p, err = keyvalue.Get("openid_provider_" + key) - if err != nil && kerr.IsErrValueNotFoundForKey(err) { + p, exists, err := keyvalue.Get("openid_provider_" + key) + if exists { _, err = GetAllProviders() // This will put all providers in cache if err != nil { return nil, err } - p, err = keyvalue.Get("openid_provider_" + key) + p, _, err = keyvalue.Get("openid_provider_" + key) } if p != nil { diff --git a/pkg/modules/avatar/initials/initials.go b/pkg/modules/avatar/initials/initials.go index 8d12550e..cc100b75 100644 --- a/pkg/modules/avatar/initials/initials.go +++ b/pkg/modules/avatar/initials/initials.go @@ -27,7 +27,6 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/modules/keyvalue" - e "code.vikunja.io/api/pkg/modules/keyvalue/error" "code.vikunja.io/api/pkg/user" "github.com/disintegration/imaging" "github.com/golang/freetype/truetype" @@ -128,12 +127,12 @@ func getCacheKey(prefix string, keys ...int64) string { func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) { cacheKey := getCacheKey("full", u.ID) - a, err := keyvalue.Get(cacheKey) - if err != nil && !e.IsErrValueNotFoundForKey(err) { + a, exists, err := keyvalue.Get(cacheKey) + if err != nil { return nil, err } - if err != nil && e.IsErrValueNotFoundForKey(err) { + if !exists { log.Debugf("Initials avatar for user %d not cached, creating...", u.ID) firstRune := []rune(strings.ToUpper(u.Username))[0] bg := avatarBgColors[int(u.ID)%len(avatarBgColors)] // Random color based on the user id @@ -157,12 +156,12 @@ func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) { func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) { cacheKey := getCacheKey("resized", u.ID, size) - a, err := keyvalue.Get(cacheKey) - if err != nil && !e.IsErrValueNotFoundForKey(err) { + a, exists, err := keyvalue.Get(cacheKey) + if err != nil { return nil, "", err } - if err != nil && e.IsErrValueNotFoundForKey(err) { + if !exists { log.Debugf("Initials avatar for user %d and size %d not cached, creating...", u.ID, size) fullAvatar, err := getAvatarForUser(u) if err != nil { diff --git a/pkg/modules/avatar/upload/upload.go b/pkg/modules/avatar/upload/upload.go index fc486a16..08ef3ab2 100644 --- a/pkg/modules/avatar/upload/upload.go +++ b/pkg/modules/avatar/upload/upload.go @@ -26,7 +26,6 @@ import ( "code.vikunja.io/api/pkg/files" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/modules/keyvalue" - e "code.vikunja.io/api/pkg/modules/keyvalue/error" "code.vikunja.io/api/pkg/user" "github.com/disintegration/imaging" ) @@ -40,8 +39,8 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType cacheKey := "avatar_upload_" + strconv.Itoa(int(u.ID)) - ai, err := keyvalue.Get(cacheKey) - if err != nil && !e.IsErrValueNotFoundForKey(err) { + ai, exists, err := keyvalue.Get(cacheKey) + if err != nil { return nil, "", err } @@ -51,7 +50,7 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType cached = ai.(map[int64][]byte) } - if err != nil && e.IsErrValueNotFoundForKey(err) { + if !exists { // Nothing ever cached for this user so we need to create the size map to avoid panics cached = make(map[int64][]byte) } else { diff --git a/pkg/modules/background/unsplash/unsplash.go b/pkg/modules/background/unsplash/unsplash.go index a689265b..ed175559 100644 --- a/pkg/modules/background/unsplash/unsplash.go +++ b/pkg/modules/background/unsplash/unsplash.go @@ -34,7 +34,6 @@ import ( "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/background" "code.vikunja.io/api/pkg/modules/keyvalue" - e "code.vikunja.io/api/pkg/modules/keyvalue/error" "code.vikunja.io/web" ) @@ -123,12 +122,12 @@ func getImageID(fullURL string) string { // Gets an unsplash photo either from cache or directly from the unsplash api func getUnsplashPhotoInfoByID(photoID string) (photo *Photo, err error) { - p, err := keyvalue.Get(cachePrefix + photoID) - if err != nil && !e.IsErrValueNotFoundForKey(err) { + p, exists, err := keyvalue.Get(cachePrefix + photoID) + if err != nil { return nil, err } - if err != nil && e.IsErrValueNotFoundForKey(err) { + if !exists { log.Debugf("Image information for unsplash photo %s not cached, requesting from unsplash...", photoID) photo = &Photo{} err = doGet("photos/"+photoID, photo) diff --git a/pkg/modules/keyvalue/keyvalue.go b/pkg/modules/keyvalue/keyvalue.go index e84ae323..319495f9 100644 --- a/pkg/modules/keyvalue/keyvalue.go +++ b/pkg/modules/keyvalue/keyvalue.go @@ -25,7 +25,7 @@ import ( // Storage defines an interface for saving key-value pairs type Storage interface { Put(key string, value interface{}) (err error) - Get(key string) (value interface{}, err error) + Get(key string) (value interface{}, exists bool, err error) Del(key string) (err error) IncrBy(key string, update int64) (err error) DecrBy(key string, update int64) (err error) @@ -51,7 +51,7 @@ func Put(key string, value interface{}) error { } // Get returns a value from a storage backend -func Get(key string) (value interface{}, err error) { +func Get(key string) (value interface{}, exists bool, err error) { return store.Get(key) } diff --git a/pkg/modules/keyvalue/memory/memory.go b/pkg/modules/keyvalue/memory/memory.go index f9bea40f..c6b82305 100644 --- a/pkg/modules/keyvalue/memory/memory.go +++ b/pkg/modules/keyvalue/memory/memory.go @@ -44,16 +44,11 @@ func (s *Storage) Put(key string, value interface{}) (err error) { } // Get retrieves a saved value from memory storage -func (s *Storage) Get(key string) (value interface{}, err error) { +func (s *Storage) Get(key string) (value interface{}, exists bool, err error) { s.mutex.Lock() defer s.mutex.Unlock() - var exists bool value, exists = s.store[key] - if !exists { - return nil, &e.ErrValueNotFoundForKey{Key: key} - } - return } diff --git a/pkg/modules/keyvalue/redis/redis.go b/pkg/modules/keyvalue/redis/redis.go index 190309b1..b35345c5 100644 --- a/pkg/modules/keyvalue/redis/redis.go +++ b/pkg/modules/keyvalue/redis/redis.go @@ -20,9 +20,6 @@ import ( "context" "encoding/json" - "github.com/go-errors/errors" - - e "code.vikunja.io/api/pkg/modules/keyvalue/error" "code.vikunja.io/api/pkg/red" "github.com/go-redis/redis/v8" ) @@ -52,13 +49,10 @@ func (s *Storage) Put(key string, value interface{}) (err error) { } // Get retrieves a saved value from redis -func (s *Storage) Get(key string) (value interface{}, err error) { +func (s *Storage) Get(key string) (value interface{}, exists bool, err error) { b, err := s.client.Get(context.Background(), key).Bytes() if err != nil { - if errors.Is(err, redis.Nil) { - return nil, &e.ErrValueNotFoundForKey{Key: key} - } - return nil, err + return nil, false, err } err = json.Unmarshal(b, value)