Fix get system setting bug when enabled redis cache (#22298)

backport #22295, fix #22281 Co-authored-by: Lauris BH <lauris@nix.lv>
go-gitea · Jan 1, 2023 · 0697075 · 0697075
1 parent f1e07d8
commit 0697075
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 53 deletions.
diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go
@@ -154,8 +154,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
 		return DefaultAvatarLink()
 	}
 
-	enableFederatedAvatarSetting, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar)
-	enableFederatedAvatar := enableFederatedAvatarSetting.GetValueBool()
+	enableFederatedAvatar := system_model.GetSettingBool(system_model.KeyPictureEnableFederatedAvatar)
 
 	var err error
 	if enableFederatedAvatar && system_model.LibravatarService != nil {
@@ -176,9 +175,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
 		return urlStr
 	}
 
-	disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
-
-	disableGravatar := disableGravatarSetting.GetValueBool()
+	disableGravatar := system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
 	if !disableGravatar {
 		// copy GravatarSourceURL, because we will modify its Path.
 		avatarURLCopy := *system_model.GravatarSourceURL

diff --git a/models/system/setting.go b/models/system/setting.go
@@ -93,21 +93,22 @@ func GetSettingNoCache(key string) (*Setting, error) {
 }
 
 // GetSetting returns the setting value via the key
-func GetSetting(key string) (*Setting, error) {
-	return cache.Get(genSettingCacheKey(key), func() (*Setting, error) {
+func GetSetting(key string) (string, error) {
+	return cache.GetString(genSettingCacheKey(key), func() (string, error) {
 		res, err := GetSettingNoCache(key)
 		if err != nil {
-			return nil, err
+			return "", err
 		}
-		return res, nil
+		return res.SettingValue, nil
 	})
 }
 
 // GetSettingBool return bool value of setting,
 // none existing keys and errors are ignored and result in false
 func GetSettingBool(key string) bool {
 	s, _ := GetSetting(key)
-	return s.GetValueBool()
+	v, _ := strconv.ParseBool(s)
+	return v
 }
 
 // GetSettings returns specific settings
@@ -184,8 +185,8 @@ func SetSettingNoVersion(key, value string) error {
 
 // SetSetting updates a users' setting for a specific key
 func SetSetting(setting *Setting) error {
-	_, err := cache.Set(genSettingCacheKey(setting.SettingKey), func() (*Setting, error) {
-		return setting, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
+	_, err := cache.GetString(genSettingCacheKey(setting.SettingKey), func() (string, error) {
+		return setting.SettingValue, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
 	})
 	if err != nil {
 		return err

diff --git a/models/user/avatar.go b/models/user/avatar.go
@@ -68,9 +68,7 @@ func (u *User) AvatarLinkWithSize(size int) string {
 	useLocalAvatar := false
 	autoGenerateAvatar := false
 
-	disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
-
-	disableGravatar := disableGravatarSetting.GetValueBool()
+	disableGravatar := system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
 
 	switch {
 	case u.UseCustomAvatar:

diff --git a/models/user/setting.go b/models/user/setting.go
@@ -54,13 +54,13 @@ func genSettingCacheKey(userID int64, key string) string {
 }
 
 // GetSetting returns the setting value via the key
-func GetSetting(uid int64, key string) (*Setting, error) {
-	return cache.Get(genSettingCacheKey(uid, key), func() (*Setting, error) {
+func GetSetting(uid int64, key string) (string, error) {
+	return cache.GetString(genSettingCacheKey(uid, key), func() (string, error) {
 		res, err := GetSettingNoCache(uid, key)
 		if err != nil {
-			return nil, err
+			return "", err
 		}
-		return res, nil
+		return res.SettingValue, nil
 	})
 }
 
@@ -155,7 +155,7 @@ func SetUserSetting(userID int64, key, value string) error {
 		return err
 	}
 
-	_, err := cache.Set(genSettingCacheKey(userID, key), func() (string, error) {
+	_, err := cache.GetString(genSettingCacheKey(userID, key), func() (string, error) {
 		return value, upsertUserSettingValue(userID, key, value)
 	})
 

diff --git a/modules/cache/cache.go b/modules/cache/cache.go
@@ -46,39 +46,6 @@ func GetCache() mc.Cache {
 	return conn
 }
 
-// Get returns the key value from cache with callback when no key exists in cache
-func Get[V interface{}](key string, getFunc func() (V, error)) (V, error) {
-	if conn == nil || setting.CacheService.TTL == 0 {
-		return getFunc()
-	}
-
-	cached := conn.Get(key)
-	if value, ok := cached.(V); ok {
-		return value, nil
-	}
-
-	value, err := getFunc()
-	if err != nil {
-		return value, err
-	}
-
-	return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
-}
-
-// Set updates and returns the key value in the cache with callback. The old value is only removed if the updateFunc() is successful
-func Set[V interface{}](key string, valueFunc func() (V, error)) (V, error) {
-	if conn == nil || setting.CacheService.TTL == 0 {
-		return valueFunc()
-	}
-
-	value, err := valueFunc()
-	if err != nil {
-		return value, err
-	}
-
-	return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
-}
-
 // GetString returns the key value from cache with callback when no key exists in cache
 func GetString(key string, getFunc func() (string, error)) (string, error) {
 	if conn == nil || setting.CacheService.TTL == 0 {