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

Fix #22281

In #21621 , `Get[V]` and `Set[V]` has been introduced, so that cache
value will be `*Setting`. For memory cache it's OK. But for redis cache,
it can only store `string` for the current implementation. This PR
revert some of changes of that and just store or return a `string` for
system setting.
This commit is contained in:
Lunny Xiao 2023-01-02 00:06:52 +08:00 committed by GitHub
parent 0f4e1b9ac6
commit a1c30740bb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 16 additions and 53 deletions

View file

@ -153,8 +153,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
return DefaultAvatarLink() return DefaultAvatarLink()
} }
enableFederatedAvatarSetting, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar) enableFederatedAvatar := system_model.GetSettingBool(system_model.KeyPictureEnableFederatedAvatar)
enableFederatedAvatar := enableFederatedAvatarSetting.GetValueBool()
var err error var err error
if enableFederatedAvatar && system_model.LibravatarService != nil { if enableFederatedAvatar && system_model.LibravatarService != nil {
@ -175,9 +174,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
return urlStr return urlStr
} }
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar) disableGravatar := system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
disableGravatar := disableGravatarSetting.GetValueBool()
if !disableGravatar { if !disableGravatar {
// copy GravatarSourceURL, because we will modify its Path. // copy GravatarSourceURL, because we will modify its Path.
avatarURLCopy := *system_model.GravatarSourceURL avatarURLCopy := *system_model.GravatarSourceURL

View file

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

View file

@ -67,9 +67,7 @@ func (u *User) AvatarLinkWithSize(size int) string {
useLocalAvatar := false useLocalAvatar := false
autoGenerateAvatar := false autoGenerateAvatar := false
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar) disableGravatar := system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
disableGravatar := disableGravatarSetting.GetValueBool()
switch { switch {
case u.UseCustomAvatar: case u.UseCustomAvatar:

View file

@ -53,13 +53,13 @@ func genSettingCacheKey(userID int64, key string) string {
} }
// GetSetting returns the setting value via the key // GetSetting returns the setting value via the key
func GetSetting(uid int64, key string) (*Setting, error) { func GetSetting(uid int64, key string) (string, error) {
return cache.Get(genSettingCacheKey(uid, key), func() (*Setting, error) { return cache.GetString(genSettingCacheKey(uid, key), func() (string, error) {
res, err := GetSettingNoCache(uid, key) res, err := GetSettingNoCache(uid, key)
if err != nil { if err != nil {
return nil, err return "", err
} }
return res, nil return res.SettingValue, nil
}) })
} }
@ -154,7 +154,7 @@ func SetUserSetting(userID int64, key, value string) error {
return err 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) return value, upsertUserSettingValue(userID, key, value)
}) })

View file

@ -45,39 +45,6 @@ func GetCache() mc.Cache {
return conn 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 // 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) { func GetString(key string, getFunc func() (string, error)) (string, error) {
if conn == nil || setting.CacheService.TTL == 0 { if conn == nil || setting.CacheService.TTL == 0 {