Fix dashboard ignored system setting cache (#21621)

This is a performance regression from #18058

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Andrew Thornton <art27@cantab.net>

5 files changed, 87 insertions, 129 deletions

diff --git a/modules/activitypub/user_settings.go b/modules/activitypub/user_settings.go
index fc9775b0f0..d192b9cdb2 100644
--- a/modules/activitypub/user_settings.go
+++ b/modules/activitypub/user_settings.go
@@ -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 {
diff --git a/modules/cache/cache.go b/modules/cache/cache.go
index 21d0cd0a04..d98b0a0cec 100644
--- a/modules/cache/cache.go
+++ b/modules/cache/cache.go
@@ -46,32 +46,64 @@ 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 {
 		return getFunc()
 	}
-	if !conn.IsExist(key) {
-		var (
-			value string
-			err   error
-		)
-		if value, err = getFunc(); err != nil {
-			return value, err
-		}
-		err = conn.Put(key, value, setting.CacheService.TTLSeconds())
+
+	cached := conn.Get(key)
+
+	if cached == nil {
+		value, err := getFunc()
 		if err != nil {
-			return "", err
+			return value, err
 		}
+		return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
 	}
-	value := conn.Get(key)
-	if v, ok := value.(string); ok {
-		return v, nil
+
+	if value, ok := cached.(string); ok {
+		return value, nil
 	}
-	if v, ok := value.(fmt.Stringer); ok {
-		return v.String(), nil
+
+	if stringer, ok := cached.(fmt.Stringer); ok {
+		return stringer.String(), nil
 	}
-	return fmt.Sprintf("%s", conn.Get(key)), nil
+
+	return fmt.Sprintf("%s", cached), nil
 }
 
 // GetInt returns key value from cache with callback when no key exists in cache
@@ -79,30 +111,33 @@ func GetInt(key string, getFunc func() (int, error)) (int, error) {
 	if conn == nil || setting.CacheService.TTL == 0 {
 		return getFunc()
 	}
-	if !conn.IsExist(key) {
-		var (
-			value int
-			err   error
-		)
-		if value, err = getFunc(); err != nil {
-			return value, err
-		}
-		err = conn.Put(key, value, setting.CacheService.TTLSeconds())
+
+	cached := conn.Get(key)
+
+	if cached == nil {
+		value, err := getFunc()
 		if err != nil {
-			return 0, err
+			return value, err
 		}
+
+		return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
 	}
-	switch value := conn.Get(key).(type) {
+
+	switch v := cached.(type) {
 	case int:
-		return value, nil
+		return v, nil
 	case string:
-		v, err := strconv.Atoi(value)
+		value, err := strconv.Atoi(v)
 		if err != nil {
 			return 0, err
 		}
-		return v, nil
+		return value, nil
 	default:
-		return 0, fmt.Errorf("Unsupported cached value type: %v", value)
+		value, err := getFunc()
+		if err != nil {
+			return value, err
+		}
+		return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
 	}
 }
 
@@ -111,30 +146,34 @@ func GetInt64(key string, getFunc func() (int64, error)) (int64, error) {
 	if conn == nil || setting.CacheService.TTL == 0 {
 		return getFunc()
 	}
-	if !conn.IsExist(key) {
-		var (
-			value int64
-			err   error
-		)
-		if value, err = getFunc(); err != nil {
-			return value, err
-		}
-		err = conn.Put(key, value, setting.CacheService.TTLSeconds())
+
+	cached := conn.Get(key)
+
+	if cached == nil {
+		value, err := getFunc()
 		if err != nil {
-			return 0, err
+			return value, err
 		}
+
+		return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
 	}
-	switch value := conn.Get(key).(type) {
+
+	switch v := conn.Get(key).(type) {
 	case int64:
-		return value, nil
+		return v, nil
 	case string:
-		v, err := strconv.ParseInt(value, 10, 64)
+		value, err := strconv.ParseInt(v, 10, 64)
 		if err != nil {
 			return 0, err
 		}
-		return v, nil
+		return value, nil
 	default:
-		return 0, fmt.Errorf("Unsupported cached value type: %v", value)
+		value, err := getFunc()
+		if err != nil {
+			return value, err
+		}
+
+		return value, conn.Put(key, value, setting.CacheService.TTLSeconds())
 	}
 }
 
diff --git a/modules/system/setting.go b/modules/system/setting.go
deleted file mode 100644
index aebf24a501..0000000000
--- a/modules/system/setting.go
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright 2021 The Gitea Authors. All rights reserved.
-// Use of this source code is governed by a MIT-style
-// license that can be found in the LICENSE file.
-
-package system
-
-import (
-	"strconv"
-
-	"code.gitea.io/gitea/models/system"
-	"code.gitea.io/gitea/modules/cache"
-)
-
-func genKey(key string) string {
-	return "system.setting." + key
-}
-
-// GetSetting returns the setting value via the key
-func GetSetting(key string) (string, error) {
-	return cache.GetString(genKey(key), func() (string, error) {
-		res, err := system.GetSetting(key)
-		if err != nil {
-			return "", err
-		}
-		return res.SettingValue, 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)
-	b, _ := strconv.ParseBool(s)
-	return b
-}
-
-// SetSetting sets the setting value
-func SetSetting(key, value string, version int) error {
-	cache.Remove(genKey(key))
-
-	return system.SetSetting(&system.Setting{
-		SettingKey:   key,
-		SettingValue: value,
-		Version:      version,
-	})
-}
diff --git a/modules/system/user_setting.go b/modules/system/user_setting.go
deleted file mode 100644
index eaf146c08d..0000000000
--- a/modules/system/user_setting.go
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2021 The Gitea Authors. All rights reserved.
-// Use of this source code is governed by a MIT-style
-// license that can be found in the LICENSE file.
-
-package system
-
-import (
-	"fmt"
-
-	"code.gitea.io/gitea/models/user"
-	"code.gitea.io/gitea/modules/cache"
-)
-
-func genUserKey(userID int64, key string) string {
-	return fmt.Sprintf("user_%d.setting.%s", userID, key)
-}
-
-// GetUserSetting returns the user setting value via the key
-func GetUserSetting(userID int64, key string) (string, error) {
-	return cache.GetString(genUserKey(userID, key), func() (string, error) {
-		res, err := user.GetSetting(userID, key)
-		if err != nil {
-			return "", err
-		}
-		return res.SettingValue, nil
-	})
-}
-
-// SetUserSetting sets the user setting value
-func SetUserSetting(userID int64, key, value string) error {
-	cache.Remove(genUserKey(userID, key))
-
-	return user.SetUserSetting(userID, key, value)
-}
diff --git a/modules/templates/helper.go b/modules/templates/helper.go
index c5434b7c63..d0866d3e2c 100644
--- a/modules/templates/helper.go
+++ b/modules/templates/helper.go
@@ -43,7 +43,6 @@ import (
 	"code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/svg"
-	system_module "code.gitea.io/gitea/modules/system"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/gitdiff"
@@ -88,7 +87,7 @@ func NewFuncMap() []template.FuncMap {
 			return setting.AssetVersion
 		},
 		"DisableGravatar": func() bool {
-			return system_module.GetSettingBool(system_model.KeyPictureDisableGravatar)
+			return system_model.GetSettingBool(system_model.KeyPictureDisableGravatar)
 		},
 		"DefaultShowFullName": func() bool {
 			return setting.UI.DefaultShowFullName