From fec358bedfe1d9ab1e4847f20cfe0aa474bda930 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 4 Jan 2023 08:25:12 +0800 Subject: [PATCH 1/5] Fix bug --- models/system/setting.go | 11 ++--------- models/system/setting_test.go | 5 +++++ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/models/system/setting.go b/models/system/setting.go index 8e16547d929ec..465bcc48fd361 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -184,15 +184,8 @@ func SetSettingNoVersion(key, value string) error { // SetSetting updates a users' setting for a specific key func SetSetting(setting *Setting) error { - _, 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 - } - - setting.Version++ - return nil + cache.Remove(genSettingCacheKey(setting.SettingKey)) + return upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version) } func upsertSettingValue(key, value string, version int) error { diff --git a/models/system/setting_test.go b/models/system/setting_test.go index 3ff5ba2520890..5ab97692a1521 100644 --- a/models/system/setting_test.go +++ b/models/system/setting_test.go @@ -37,6 +37,11 @@ func TestSettings(t *testing.T) { err = system.SetSetting(updatedSetting) assert.NoError(t, err) + // make sure it's not cached + updatedSetting = &system.Setting{SettingKey: keyName, SettingValue: "100", Version: newSetting.Version} + err = system.SetSetting(updatedSetting) + assert.NoError(t, err) + // get all settings settings, err = system.GetAllSettings() assert.NoError(t, err) From a8bf87bd3cde31a6acb5c8cfbbf5e0c99c62337c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 4 Jan 2023 13:24:16 +0800 Subject: [PATCH 2/5] Fix bug --- models/system/setting.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/models/system/setting.go b/models/system/setting.go index 465bcc48fd361..941de3118f213 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/setting" + setting_module "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "strk.kbt.io/projects/go/libravatar" @@ -184,8 +185,16 @@ func SetSettingNoVersion(key, value string) error { // SetSetting updates a users' setting for a specific key func SetSetting(setting *Setting) error { - cache.Remove(genSettingCacheKey(setting.SettingKey)) - return upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version) + if err := upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil { + return err + } + + cc := cache.GetCache() + if cc != nil { + return cc.Put(genSettingCacheKey(setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds()) + } + + return nil } func upsertSettingValue(key, value string, version int) error { From fa0dab0ba422276e3b69b16cd7dbe578785076bd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 4 Jan 2023 13:27:48 +0800 Subject: [PATCH 3/5] improve test --- models/system/setting_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/models/system/setting_test.go b/models/system/setting_test.go index 5ab97692a1521..b1a9037bc60bd 100644 --- a/models/system/setting_test.go +++ b/models/system/setting_test.go @@ -37,10 +37,9 @@ func TestSettings(t *testing.T) { err = system.SetSetting(updatedSetting) assert.NoError(t, err) - // make sure it's not cached - updatedSetting = &system.Setting{SettingKey: keyName, SettingValue: "100", Version: newSetting.Version} - err = system.SetSetting(updatedSetting) + value, err := system.GetSetting(keyName) assert.NoError(t, err) + assert.EqualValues(t, updatedSetting.SettingValue, value) // get all settings settings, err = system.GetAllSettings() From 84ecceb3d417345647208f3ee6ce067643c05817 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 4 Jan 2023 15:51:38 +0800 Subject: [PATCH 4/5] fix lint --- models/system/setting.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/models/system/setting.go b/models/system/setting.go index 941de3118f213..a6bdedb2704b7 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -12,7 +12,6 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/cache" - "code.gitea.io/gitea/modules/setting" setting_module "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -245,7 +244,7 @@ func Init() error { var disableGravatar bool disableGravatarSetting, err := GetSettingNoCache(KeyPictureDisableGravatar) if IsErrSettingIsNotExist(err) { - disableGravatar = setting.GetDefaultDisableGravatar() + disableGravatar = setting_module.GetDefaultDisableGravatar() disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)} } else if err != nil { return err @@ -256,7 +255,7 @@ func Init() error { var enableFederatedAvatar bool enableFederatedAvatarSetting, err := GetSettingNoCache(KeyPictureEnableFederatedAvatar) if IsErrSettingIsNotExist(err) { - enableFederatedAvatar = setting.GetDefaultEnableFederatedAvatar(disableGravatar) + enableFederatedAvatar = setting_module.GetDefaultEnableFederatedAvatar(disableGravatar) enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)} } else if err != nil { return err @@ -264,16 +263,16 @@ func Init() error { enableFederatedAvatar = disableGravatarSetting.GetValueBool() } - if setting.OfflineMode { + if setting_module.OfflineMode { disableGravatar = true enableFederatedAvatar = false } if enableFederatedAvatar || !disableGravatar { var err error - GravatarSourceURL, err = url.Parse(setting.GravatarSource) + GravatarSourceURL, err = url.Parse(setting_module.GravatarSource) if err != nil { - return fmt.Errorf("Failed to parse Gravatar URL(%s): %w", setting.GravatarSource, err) + return fmt.Errorf("Failed to parse Gravatar URL(%s): %w", setting_module.GravatarSource, err) } } From 58e6746fec65378063713c590e813185d7e60780 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 6 Jan 2023 23:10:16 +0800 Subject: [PATCH 5/5] Fix bug --- models/system/setting.go | 6 ++++-- models/system/setting_test.go | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/models/system/setting.go b/models/system/setting.go index 2ea714db4c4eb..0701c4bf48dc4 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -88,7 +88,7 @@ func GetSettingNoCache(key string) (*Setting, error) { if len(v) == 0 { return nil, ErrSettingIsNotExist{key} } - return v[key], nil + return v[strings.ToLower(key)], nil } // GetSetting returns the setting value via the key @@ -131,7 +131,7 @@ func GetSettings(keys []string) (map[string]*Setting, error) { type AllSettings map[string]*Setting func (settings AllSettings) Get(key string) Setting { - if v, ok := settings[key]; ok { + if v, ok := settings[strings.ToLower(key)]; ok { return *v } return Setting{} @@ -188,6 +188,8 @@ func SetSetting(setting *Setting) error { return err } + setting.Version++ + cc := cache.GetCache() if cc != nil { return cc.Put(genSettingCacheKey(setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds()) diff --git a/models/system/setting_test.go b/models/system/setting_test.go index b1a9037bc60bd..c43d2e30841d2 100644 --- a/models/system/setting_test.go +++ b/models/system/setting_test.go @@ -33,7 +33,7 @@ func TestSettings(t *testing.T) { assert.EqualValues(t, newSetting.SettingValue, settings[strings.ToLower(keyName)].SettingValue) // updated setting - updatedSetting := &system.Setting{SettingKey: keyName, SettingValue: "100", Version: newSetting.Version} + updatedSetting := &system.Setting{SettingKey: keyName, SettingValue: "100", Version: settings[strings.ToLower(keyName)].Version} err = system.SetSetting(updatedSetting) assert.NoError(t, err)