diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2023-01-02 00:06:52 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-02 00:06:52 +0800 |
commit | a1c30740bb04ed55905415dcd195ee9fb97547de (patch) | |
tree | 60dd79afebadd9d092b84d790a1cf4678c4619d4 | |
parent | 0f4e1b9ac66b8ffa0083a5a2516e4710393bb0da (diff) | |
download | gitea-a1c30740bb04ed55905415dcd195ee9fb97547de.tar.gz gitea-a1c30740bb04ed55905415dcd195ee9fb97547de.zip |
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.
-rw-r--r-- | models/avatars/avatar.go | 7 | ||||
-rw-r--r-- | models/system/setting.go | 15 | ||||
-rw-r--r-- | models/user/avatar.go | 4 | ||||
-rw-r--r-- | models/user/setting.go | 10 | ||||
-rw-r--r-- | modules/cache/cache.go | 33 |
5 files changed, 16 insertions, 53 deletions
diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go index 7ccb782fa6..4386142861 100644 --- a/models/avatars/avatar.go +++ b/models/avatars/avatar.go @@ -153,8 +153,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 { @@ -175,9 +174,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 index 6f333d6fd1..8e16547d92 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -92,13 +92,13 @@ 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 }) } @@ -106,7 +106,8 @@ func GetSetting(key string) (*Setting, error) { // 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 @@ -183,8 +184,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 index ce25139b5f..e6ca49efd0 100644 --- a/models/user/avatar.go +++ b/models/user/avatar.go @@ -67,9 +67,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 index a17483f9af..f5cfef5b33 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -53,13 +53,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 }) } @@ -154,7 +154,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 index 2e7d5bb603..edaf483135 100644 --- a/modules/cache/cache.go +++ b/modules/cache/cache.go @@ -45,39 +45,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 { |