Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dashboard ignored system setting cache (#21621) #21759

Merged
merged 1 commit into from
Nov 10, 2022
Merged

Fix dashboard ignored system setting cache (#21621) #21759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions models/avatars/avatar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,11 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
return DefaultAvatarLink()
}

enableFederatedAvatar, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar)
enableFederatedAvatarSetting, _ := system_model.GetSetting(system_model.KeyPictureEnableFederatedAvatar)
enableFederatedAvatar := enableFederatedAvatarSetting.GetValueBool()

var err error
if enableFederatedAvatar != nil && enableFederatedAvatar.GetValueBool() && system_model.LibravatarService != nil {
if enableFederatedAvatar && system_model.LibravatarService != nil {
emailHash := saveEmailHash(email)
if final {
// for final link, we can spend more time on slow external query
Expand All @@ -171,8 +172,10 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
return urlStr
}

disableGravatar, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
if disableGravatar != nil && !disableGravatar.GetValueBool() {
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)

disableGravatar := disableGravatarSetting.GetValueBool()
if !disableGravatar {
// copy GravatarSourceURL, because we will modify its Path.
avatarURLCopy := *system_model.GravatarSourceURL
avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email))
Expand Down
40 changes: 34 additions & 6 deletions models/system/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"

Expand All @@ -35,6 +36,10 @@ func (s *Setting) TableName() string {
}

func (s *Setting) GetValueBool() bool {
if s == nil {
return false
}

b, _ := strconv.ParseBool(s.SettingValue)
return b
}
Expand Down Expand Up @@ -75,8 +80,8 @@ func IsErrDataExpired(err error) bool {
return ok
}

// GetSetting returns specific setting
func GetSetting(key string) (*Setting, error) {
// GetSettingNoCache returns specific setting without using the cache
func GetSettingNoCache(key string) (*Setting, error) {
v, err := GetSettings([]string{key})
if err != nil {
return nil, err
Expand All @@ -87,6 +92,24 @@ func GetSetting(key string) (*Setting, error) {
return v[key], nil
}

// GetSetting returns the setting value via the key
func GetSetting(key string) (*Setting, error) {
return cache.Get(genSettingCacheKey(key), func() (*Setting, error) {
res, err := GetSettingNoCache(key)
if err != nil {
return nil, err
}
return res, 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()
}

// GetSettings returns specific settings
func GetSettings(keys []string) (map[string]*Setting, error) {
for i := 0; i < len(keys); i++ {
Expand Down Expand Up @@ -139,12 +162,13 @@ func GetAllSettings() (AllSettings, error) {

// DeleteSetting deletes a specific setting for a user
func DeleteSetting(setting *Setting) error {
cache.Remove(genSettingCacheKey(setting.SettingKey))
_, err := db.GetEngine(db.DefaultContext).Delete(setting)
return err
}

func SetSettingNoVersion(key, value string) error {
s, err := GetSetting(key)
s, err := GetSettingNoCache(key)
if IsErrSettingIsNotExist(err) {
return SetSetting(&Setting{
SettingKey: key,
Expand All @@ -160,9 +184,13 @@ func SetSettingNoVersion(key, value string) error {

// SetSetting updates a users' setting for a specific key
func SetSetting(setting *Setting) error {
if err := upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil {
_, err := cache.Set(genSettingCacheKey(setting.SettingKey), func() (*Setting, error) {
return setting, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
})
if err != nil {
return err
}

setting.Version++
return nil
}
Expand Down Expand Up @@ -213,7 +241,7 @@ var (

func Init() error {
var disableGravatar bool
disableGravatarSetting, err := GetSetting(KeyPictureDisableGravatar)
disableGravatarSetting, err := GetSettingNoCache(KeyPictureDisableGravatar)
if IsErrSettingIsNotExist(err) {
disableGravatar = setting.GetDefaultDisableGravatar()
disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
Expand All @@ -224,7 +252,7 @@ func Init() error {
}

var enableFederatedAvatar bool
enableFederatedAvatarSetting, err := GetSetting(KeyPictureEnableFederatedAvatar)
enableFederatedAvatarSetting, err := GetSettingNoCache(KeyPictureEnableFederatedAvatar)
if IsErrSettingIsNotExist(err) {
enableFederatedAvatar = setting.GetDefaultEnableFederatedAvatar(disableGravatar)
enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}
Expand Down
5 changes: 5 additions & 0 deletions models/system/setting_key.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ const (
KeyPictureDisableGravatar = "picture.disable_gravatar"
KeyPictureEnableFederatedAvatar = "picture.enable_federated_avatar"
)

// genSettingCacheKey returns the cache key for some configuration
func genSettingCacheKey(key string) string {
return "system.setting." + key
}
6 changes: 2 additions & 4 deletions models/user/avatar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,9 @@ func (u *User) AvatarLinkWithSize(size int) string {
useLocalAvatar := false
autoGenerateAvatar := false

var disableGravatar bool
disableGravatarSetting, _ := system_model.GetSetting(system_model.KeyPictureDisableGravatar)
if disableGravatarSetting != nil {
disableGravatar = disableGravatarSetting.GetValueBool()
}

disableGravatar := disableGravatarSetting.GetValueBool()

switch {
case u.UseCustomAvatar:
Expand Down
36 changes: 31 additions & 5 deletions models/user/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/cache"

"xorm.io/builder"
)
Expand Down Expand Up @@ -47,9 +48,25 @@ func IsErrUserSettingIsNotExist(err error) bool {
return ok
}

// GetSetting returns specific setting
// genSettingCacheKey returns the cache key for some configuration
func genSettingCacheKey(userID int64, key string) string {
return fmt.Sprintf("user_%d.setting.%s", userID, key)
}

// GetSetting returns the setting value via the key
func GetSetting(uid int64, key string) (*Setting, error) {
v, err := GetUserSettings(uid, []string{key})
return cache.Get(genSettingCacheKey(uid, key), func() (*Setting, error) {
res, err := GetSettingNoCache(uid, key)
if err != nil {
return nil, err
}
return res, nil
})
}

// GetSettingNoCache returns specific setting without using the cache
func GetSettingNoCache(uid int64, key string) (*Setting, error) {
v, err := GetSettings(uid, []string{key})
if err != nil {
return nil, err
}
Expand All @@ -59,8 +76,8 @@ func GetSetting(uid int64, key string) (*Setting, error) {
return v[key], nil
}

// GetUserSettings returns specific settings from user
func GetUserSettings(uid int64, keys []string) (map[string]*Setting, error) {
// GetSettings returns specific settings from user
func GetSettings(uid int64, keys []string) (map[string]*Setting, error) {
settings := make([]*Setting, 0, len(keys))
if err := db.GetEngine(db.DefaultContext).
Where("user_id=?", uid).
Expand Down Expand Up @@ -105,6 +122,7 @@ func GetUserSetting(userID int64, key string, def ...string) (string, error) {
if err := validateUserSettingKey(key); err != nil {
return "", err
}

setting := &Setting{UserID: userID, SettingKey: key}
has, err := db.GetEngine(db.DefaultContext).Get(setting)
if err != nil {
Expand All @@ -124,7 +142,10 @@ func DeleteUserSetting(userID int64, key string) error {
if err := validateUserSettingKey(key); err != nil {
return err
}

cache.Remove(genSettingCacheKey(userID, key))
_, err := db.GetEngine(db.DefaultContext).Delete(&Setting{UserID: userID, SettingKey: key})

return err
}

Expand All @@ -133,7 +154,12 @@ func SetUserSetting(userID int64, key, value string) error {
if err := validateUserSettingKey(key); err != nil {
return err
}
return upsertUserSettingValue(userID, key, value)

_, err := cache.Set(genSettingCacheKey(userID, key), func() (string, error) {
return value, upsertUserSettingValue(userID, key, value)
})

return err
}

func upsertUserSettingValue(userID int64, key, value string) error {
Expand Down
2 changes: 1 addition & 1 deletion models/user/setting_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestSettings(t *testing.T) {
assert.NoError(t, err)

// get specific setting
settings, err := user_model.GetUserSettings(99, []string{keyName})
settings, err := user_model.GetSettings(99, []string{keyName})
assert.NoError(t, err)
assert.Len(t, settings, 1)
assert.EqualValues(t, newSetting.SettingValue, settings[keyName].SettingValue)
Expand Down
2 changes: 1 addition & 1 deletion modules/activitypub/user_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// GetKeyPair function returns a user's private and public keys
func GetKeyPair(user *user_model.User) (pub, priv string, err error) {
var settings map[string]*user_model.Setting
settings, err = user_model.GetUserSettings(user.ID, []string{user_model.UserActivityPubPrivPem, user_model.UserActivityPubPubPem})
settings, err = user_model.GetSettings(user.ID, []string{user_model.UserActivityPubPrivPem, user_model.UserActivityPubPubPem})
if err != nil {
return
} else if len(settings) == 0 {
Expand Down
Loading