Skip to content

Commit a1c3074

Browse files
authored
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.
1 parent 0f4e1b9 commit a1c3074

File tree

5 files changed

+16
-53
lines changed

5 files changed

+16
-53
lines changed

Diff for: models/avatars/avatar.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
153153
return DefaultAvatarLink()
154154
}
155155

156-
enableFederatedAvatarSetting, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar)
157-
enableFederatedAvatar := enableFederatedAvatarSetting.GetValueBool()
156+
enableFederatedAvatar := system_model.GetSettingBool(system_model.KeyPictureEnableFederatedAvatar)
158157

159158
var err error
160159
if enableFederatedAvatar && system_model.LibravatarService != nil {
@@ -175,9 +174,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
175174
return urlStr
176175
}
177176

178-
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
179-
180-
disableGravatar := disableGravatarSetting.GetValueBool()
177+
disableGravatar := system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
181178
if !disableGravatar {
182179
// copy GravatarSourceURL, because we will modify its Path.
183180
avatarURLCopy := *system_model.GravatarSourceURL

Diff for: models/system/setting.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,22 @@ func GetSettingNoCache(key string) (*Setting, error) {
9292
}
9393

9494
// GetSetting returns the setting value via the key
95-
func GetSetting(key string) (*Setting, error) {
96-
return cache.Get(genSettingCacheKey(key), func() (*Setting, error) {
95+
func GetSetting(key string) (string, error) {
96+
return cache.GetString(genSettingCacheKey(key), func() (string, error) {
9797
res, err := GetSettingNoCache(key)
9898
if err != nil {
99-
return nil, err
99+
return "", err
100100
}
101-
return res, nil
101+
return res.SettingValue, nil
102102
})
103103
}
104104

105105
// GetSettingBool return bool value of setting,
106106
// none existing keys and errors are ignored and result in false
107107
func GetSettingBool(key string) bool {
108108
s, _ := GetSetting(key)
109-
return s.GetValueBool()
109+
v, _ := strconv.ParseBool(s)
110+
return v
110111
}
111112

112113
// GetSettings returns specific settings
@@ -183,8 +184,8 @@ func SetSettingNoVersion(key, value string) error {
183184

184185
// SetSetting updates a users' setting for a specific key
185186
func SetSetting(setting *Setting) error {
186-
_, err := cache.Set(genSettingCacheKey(setting.SettingKey), func() (*Setting, error) {
187-
return setting, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
187+
_, err := cache.GetString(genSettingCacheKey(setting.SettingKey), func() (string, error) {
188+
return setting.SettingValue, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
188189
})
189190
if err != nil {
190191
return err

Diff for: models/user/avatar.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ func (u *User) AvatarLinkWithSize(size int) string {
6767
useLocalAvatar := false
6868
autoGenerateAvatar := false
6969

70-
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
71-
72-
disableGravatar := disableGravatarSetting.GetValueBool()
70+
disableGravatar := system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
7371

7472
switch {
7573
case u.UseCustomAvatar:

Diff for: models/user/setting.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ func genSettingCacheKey(userID int64, key string) string {
5353
}
5454

5555
// GetSetting returns the setting value via the key
56-
func GetSetting(uid int64, key string) (*Setting, error) {
57-
return cache.Get(genSettingCacheKey(uid, key), func() (*Setting, error) {
56+
func GetSetting(uid int64, key string) (string, error) {
57+
return cache.GetString(genSettingCacheKey(uid, key), func() (string, error) {
5858
res, err := GetSettingNoCache(uid, key)
5959
if err != nil {
60-
return nil, err
60+
return "", err
6161
}
62-
return res, nil
62+
return res.SettingValue, nil
6363
})
6464
}
6565

@@ -154,7 +154,7 @@ func SetUserSetting(userID int64, key, value string) error {
154154
return err
155155
}
156156

157-
_, err := cache.Set(genSettingCacheKey(userID, key), func() (string, error) {
157+
_, err := cache.GetString(genSettingCacheKey(userID, key), func() (string, error) {
158158
return value, upsertUserSettingValue(userID, key, value)
159159
})
160160

Diff for: modules/cache/cache.go

-33
Original file line numberDiff line numberDiff line change
@@ -45,39 +45,6 @@ func GetCache() mc.Cache {
4545
return conn
4646
}
4747

48-
// Get returns the key value from cache with callback when no key exists in cache
49-
func Get[V interface{}](key string, getFunc func() (V, error)) (V, error) {
50-
if conn == nil || setting.CacheService.TTL == 0 {
51-
return getFunc()
52-
}
53-
54-
cached := conn.Get(key)
55-
if value, ok := cached.(V); ok {
56-
return value, nil
57-
}
58-
59-
value, err := getFunc()
60-
if err != nil {
61-
return value, err
62-
}
63-
64-
return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
65-
}
66-
67-
// Set updates and returns the key value in the cache with callback. The old value is only removed if the updateFunc() is successful
68-
func Set[V interface{}](key string, valueFunc func() (V, error)) (V, error) {
69-
if conn == nil || setting.CacheService.TTL == 0 {
70-
return valueFunc()
71-
}
72-
73-
value, err := valueFunc()
74-
if err != nil {
75-
return value, err
76-
}
77-
78-
return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
79-
}
80-
8148
// GetString returns the key value from cache with callback when no key exists in cache
8249
func GetString(key string, getFunc func() (string, error)) (string, error) {
8350
if conn == nil || setting.CacheService.TTL == 0 {

0 commit comments

Comments
 (0)