aboutsummaryrefslogtreecommitdiffstats
path: root/models
diff options
context:
space:
mode:
authorLunny Xiao <xiaolunwen@gmail.com>2023-09-13 15:15:00 +0800
committerGitHub <noreply@github.com>2023-09-13 15:15:00 +0800
commit9df573bddcc01bc8c3ab495629678ad577071831 (patch)
tree0fa583e6a0d7b7fd1266408e07cd7ff9e6aeb4bf /models
parentb0a405c5fad2055976747a1c8b2c48dfe2750c9f (diff)
downloadgitea-9df573bddcc01bc8c3ab495629678ad577071831.tar.gz
gitea-9df573bddcc01bc8c3ab495629678ad577071831.zip
Fix context cache bug & enable context cache for dashabord commits' authors(#26991) (#27017)
backport #26991 Unfortunately, when a system setting hasn't been stored in the database, it cannot be cached. Meanwhile, this PR also uses context cache for push email avatar display which should avoid to read user table via email address again and again. According to my local test, this should reduce dashboard elapsed time from 150ms -> 80ms .
Diffstat (limited to 'models')
-rw-r--r--models/avatars/avatar.go8
-rw-r--r--models/fixtures/system_setting.yml4
-rw-r--r--models/system/setting.go62
-rw-r--r--models/user/avatar.go4
4 files changed, 40 insertions, 38 deletions
diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go
index 265ee6428e..e40aa3f542 100644
--- a/models/avatars/avatar.go
+++ b/models/avatars/avatar.go
@@ -153,7 +153,12 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
return DefaultAvatarLink()
}
- enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar)
+ disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar,
+ setting.GetDefaultDisableGravatar(),
+ )
+
+ enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar,
+ setting.GetDefaultEnableFederatedAvatar(disableGravatar))
var err error
if enableFederatedAvatar && system_model.LibravatarService != nil {
@@ -174,7 +179,6 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
return urlStr
}
- disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
if !disableGravatar {
// copy GravatarSourceURL, because we will modify its Path.
avatarURLCopy := *system_model.GravatarSourceURL
diff --git a/models/fixtures/system_setting.yml b/models/fixtures/system_setting.yml
index 6c960168fc..30542bc82a 100644
--- a/models/fixtures/system_setting.yml
+++ b/models/fixtures/system_setting.yml
@@ -1,6 +1,6 @@
-
id: 1
- setting_key: 'disable_gravatar'
+ setting_key: 'picture.disable_gravatar'
setting_value: 'false'
version: 1
created: 1653533198
@@ -8,7 +8,7 @@
-
id: 2
- setting_key: 'enable_federated_avatar'
+ setting_key: 'picture.enable_federated_avatar'
setting_value: 'false'
version: 1
created: 1653533198
diff --git a/models/system/setting.go b/models/system/setting.go
index 6af759dc8a..9fffa74e94 100644
--- a/models/system/setting.go
+++ b/models/system/setting.go
@@ -94,11 +94,14 @@ func GetSetting(ctx context.Context, key string) (*Setting, error) {
const contextCacheKey = "system_setting"
// GetSettingWithCache returns the setting value via the key
-func GetSettingWithCache(ctx context.Context, key string) (string, error) {
+func GetSettingWithCache(ctx context.Context, key, defaultVal string) (string, error) {
return cache.GetWithContextCache(ctx, contextCacheKey, key, func() (string, error) {
return cache.GetString(genSettingCacheKey(key), func() (string, error) {
res, err := GetSetting(ctx, key)
if err != nil {
+ if IsErrSettingIsNotExist(err) {
+ return defaultVal, nil
+ }
return "", err
}
return res.SettingValue, nil
@@ -108,17 +111,21 @@ func GetSettingWithCache(ctx context.Context, key string) (string, error) {
// GetSettingBool return bool value of setting,
// none existing keys and errors are ignored and result in false
-func GetSettingBool(ctx context.Context, key string) bool {
- s, _ := GetSetting(ctx, key)
- if s == nil {
- return false
+func GetSettingBool(ctx context.Context, key string, defaultVal bool) (bool, error) {
+ s, err := GetSetting(ctx, key)
+ switch {
+ case err == nil:
+ v, _ := strconv.ParseBool(s.SettingValue)
+ return v, nil
+ case IsErrSettingIsNotExist(err):
+ return defaultVal, nil
+ default:
+ return false, err
}
- v, _ := strconv.ParseBool(s.SettingValue)
- return v
}
-func GetSettingWithCacheBool(ctx context.Context, key string) bool {
- s, _ := GetSettingWithCache(ctx, key)
+func GetSettingWithCacheBool(ctx context.Context, key string, defaultVal bool) bool {
+ s, _ := GetSettingWithCache(ctx, key, strconv.FormatBool(defaultVal))
v, _ := strconv.ParseBool(s)
return v
}
@@ -259,52 +266,41 @@ var (
)
func Init(ctx context.Context) error {
- var disableGravatar bool
- disableGravatarSetting, err := GetSetting(ctx, KeyPictureDisableGravatar)
- if IsErrSettingIsNotExist(err) {
- disableGravatar = setting_module.GetDefaultDisableGravatar()
- disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
- } else if err != nil {
+ disableGravatar, err := GetSettingBool(ctx, KeyPictureDisableGravatar, setting_module.GetDefaultDisableGravatar())
+ if err != nil {
return err
- } else {
- disableGravatar = disableGravatarSetting.GetValueBool()
}
- var enableFederatedAvatar bool
- enableFederatedAvatarSetting, err := GetSetting(ctx, KeyPictureEnableFederatedAvatar)
- if IsErrSettingIsNotExist(err) {
- enableFederatedAvatar = setting_module.GetDefaultEnableFederatedAvatar(disableGravatar)
- enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}
- } else if err != nil {
+ enableFederatedAvatar, err := GetSettingBool(ctx, KeyPictureEnableFederatedAvatar, setting_module.GetDefaultEnableFederatedAvatar(disableGravatar))
+ if err != nil {
return err
- } else {
- enableFederatedAvatar = disableGravatarSetting.GetValueBool()
}
if setting_module.OfflineMode {
- disableGravatar = true
- enableFederatedAvatar = false
- if !GetSettingBool(ctx, KeyPictureDisableGravatar) {
+ if !disableGravatar {
if err := SetSettingNoVersion(ctx, KeyPictureDisableGravatar, "true"); err != nil {
- return fmt.Errorf("Failed to set setting %q: %w", KeyPictureDisableGravatar, err)
+ return fmt.Errorf("failed to set setting %q: %w", KeyPictureDisableGravatar, err)
}
}
- if GetSettingBool(ctx, KeyPictureEnableFederatedAvatar) {
+ disableGravatar = true
+
+ if enableFederatedAvatar {
if err := SetSettingNoVersion(ctx, KeyPictureEnableFederatedAvatar, "false"); err != nil {
- return fmt.Errorf("Failed to set setting %q: %w", KeyPictureEnableFederatedAvatar, err)
+ return fmt.Errorf("failed to set setting %q: %w", KeyPictureEnableFederatedAvatar, err)
}
}
+ enableFederatedAvatar = false
}
if enableFederatedAvatar || !disableGravatar {
var err error
GravatarSourceURL, err = url.Parse(setting_module.GravatarSource)
if err != nil {
- return fmt.Errorf("Failed to parse Gravatar URL(%s): %w", setting_module.GravatarSource, err)
+ return fmt.Errorf("failed to parse Gravatar URL(%s): %w", setting_module.GravatarSource, err)
}
}
- if GravatarSourceURL != nil && enableFederatedAvatarSetting.GetValueBool() {
+ if GravatarSourceURL != nil && enableFederatedAvatar {
LibravatarService = libravatar.New()
if GravatarSourceURL.Scheme == "https" {
LibravatarService.SetUseHTTPS(true)
diff --git a/models/user/avatar.go b/models/user/avatar.go
index 3d1e2ed20b..7f996fa79a 100644
--- a/models/user/avatar.go
+++ b/models/user/avatar.go
@@ -67,7 +67,9 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
useLocalAvatar := false
autoGenerateAvatar := false
- disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
+ disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar,
+ setting.GetDefaultDisableGravatar(),
+ )
switch {
case u.UseCustomAvatar: