Refactor system setting (go-gitea#27000)

This PR reduces the complexity of the system setting system.

It only needs one line to introduce a new option, and the option can be
used anywhere out-of-box.

It is still high-performant (and more performant) because the config
values are cached in the config system.

models/avatars/avatar.go

@@ -5,18 +5,20 @@ package avatars
 
 import (
 	"context"
+	"fmt"
 	"net/url"
 	"path"
 	"strconv"
 	"strings"
-	"sync"
+	"sync/atomic"
 
 	"code.gitea.io/gitea/models/db"
-	system_model "code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/cache"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+
+	"strk.kbt.io/projects/go/libravatar"
 )
 
 const (
@@ -36,24 +38,54 @@ func init() {
 	db.RegisterModel(new(EmailHash))
 }
 
-var (
+type avatarSettingStruct struct {
 	defaultAvatarLink string
-	once              sync.Once
-)
+	gravatarSource    string
+	gravatarSourceURL *url.URL
+	libravatar        *libravatar.Libravatar
+}
 
-// DefaultAvatarLink the default avatar link
-func DefaultAvatarLink() string {
-	once.Do(func() {
+var avatarSettingAtomic atomic.Pointer[avatarSettingStruct]
+
+func loadAvatarSetting() (*avatarSettingStruct, error) {
+	s := avatarSettingAtomic.Load()
+	if s == nil || s.gravatarSource != setting.GravatarSource {
+		s = &avatarSettingStruct{}
 		u, err := url.Parse(setting.AppSubURL)
 		if err != nil {
-			log.Error("Can not parse AppSubURL: %v", err)
-			return
+			return nil, fmt.Errorf("unable to parse AppSubURL: %w", err)
 		}
 
 		u.Path = path.Join(u.Path, "/assets/img/avatar_default.png")
-		defaultAvatarLink = u.String()
-	})
-	return defaultAvatarLink
+		s.defaultAvatarLink = u.String()
+
+		s.gravatarSourceURL, err = url.Parse(setting.GravatarSource)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse GravatarSource %q: %w", setting.GravatarSource, err)
+		}
+
+		s.libravatar = libravatar.New()
+		if s.gravatarSourceURL.Scheme == "https" {
+			s.libravatar.SetUseHTTPS(true)
+			s.libravatar.SetSecureFallbackHost(s.gravatarSourceURL.Host)
+		} else {
+			s.libravatar.SetUseHTTPS(false)
+			s.libravatar.SetFallbackHost(s.gravatarSourceURL.Host)
+		}
+
+		avatarSettingAtomic.Store(s)
+	}
+	return s, nil
+}
+
+// DefaultAvatarLink the default avatar link
+func DefaultAvatarLink() string {
+	a, err := loadAvatarSetting()
+	if err != nil {
+		log.Error("Failed to loadAvatarSetting: %v", err)
+		return ""
+	}
+	return a.defaultAvatarLink
 }
 
 // HashEmail hashes email address to MD5 string. https://en.gravatar.com/site/implement/hash/
@@ -76,7 +108,11 @@ func GetEmailForHash(md5Sum string) (string, error) {
 // LibravatarURL returns the URL for the given email. Slow due to the DNS lookup.
 // This function should only be called if a federated avatar service is enabled.
 func LibravatarURL(email string) (*url.URL, error) {
-	urlStr, err := system_model.LibravatarService.FromEmail(email)
+	a, err := loadAvatarSetting()
+	if err != nil {
+		return nil, err
+	}
+	urlStr, err := a.libravatar.FromEmail(email)
 	if err != nil {
 		log.Error("LibravatarService.FromEmail(email=%s): error %v", email, err)
 		return nil, err
@@ -153,15 +189,13 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
 		return DefaultAvatarLink()
 	}
 
-	disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar,
-		setting.GetDefaultDisableGravatar(),
-	)
-
-	enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar,
-		setting.GetDefaultEnableFederatedAvatar(disableGravatar))
+	avatarSetting, err := loadAvatarSetting()
+	if err != nil {
+		return DefaultAvatarLink()
+	}
 
-	var err error
-	if enableFederatedAvatar && system_model.LibravatarService != nil {
+	enableFederatedAvatar := setting.Config().Picture.EnableFederatedAvatar.Value(ctx)
+	if enableFederatedAvatar {
 		emailHash := saveEmailHash(email)
 		if final {
 			// for final link, we can spend more time on slow external query
@@ -179,9 +213,10 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
 		return urlStr
 	}
 
+	disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx)
 	if !disableGravatar {
 		// copy GravatarSourceURL, because we will modify its Path.
-		avatarURLCopy := *system_model.GravatarSourceURL
+		avatarURLCopy := *avatarSetting.gravatarSourceURL
 		avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email))
 		return generateRecognizedAvatarURL(avatarURLCopy, size)
 	}

models/avatars/avatar_test.go

@@ -10,26 +10,24 @@ import (
 	"code.gitea.io/gitea/models/db"
 	system_model "code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/setting/config"
 
 	"github.com/stretchr/testify/assert"
 )
 
 const gravatarSource = "https://secure.gravatar.com/avatar/"
 
 func disableGravatar(t *testing.T) {
-	err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureEnableFederatedAvatar, "false")
+	err := system_model.SetSettings(db.DefaultContext, map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"})
 	assert.NoError(t, err)
-	err = system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "true")
+	err = system_model.SetSettings(db.DefaultContext, map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "true"})
 	assert.NoError(t, err)
-	system_model.LibravatarService = nil
 }
 
 func enableGravatar(t *testing.T) {
-	err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false")
+	err := system_model.SetSettings(db.DefaultContext, map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "false"})
 	assert.NoError(t, err)
 	setting.GravatarSource = gravatarSource
-	err = system_model.Init(db.DefaultContext)
-	assert.NoError(t, err)
 }
 
 func TestHashEmail(t *testing.T) {
@@ -47,10 +45,12 @@ func TestSizedAvatarLink(t *testing.T) {
 	setting.AppSubURL = "/testsuburl"
 
 	disableGravatar(t)
+	config.GetDynGetter().InvalidateCache()
 	assert.Equal(t, "/testsuburl/assets/img/avatar_default.png",
 		avatars_model.GenerateEmailAvatarFastLink(db.DefaultContext, "gitea@example.com", 100))
 
 	enableGravatar(t)
+	config.GetDynGetter().InvalidateCache()
 	assert.Equal(t,
 		"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100",
 		avatars_model.GenerateEmailAvatarFastLink(db.DefaultContext, "gitea@example.com", 100),

models/migrations/v1_18/v227.go

@@ -4,10 +4,6 @@
 package v1_18 //nolint
 
 import (
-	"fmt"
-	"strconv"
-
-	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
 
 	"xorm.io/xorm"
@@ -22,42 +18,6 @@ type SystemSetting struct {
 	Updated      timeutil.TimeStamp `xorm:"updated"`
 }
 
-func insertSettingsIfNotExist(x *xorm.Engine, sysSettings []*SystemSetting) error {
-	sess := x.NewSession()
-	defer sess.Close()
-	if err := sess.Begin(); err != nil {
-		return err
-	}
-	for _, setting := range sysSettings {
-		exist, err := sess.Table("system_setting").Where("setting_key=?", setting.SettingKey).Exist()
-		if err != nil {
-			return err
-		}
-		if !exist {
-			if _, err := sess.Insert(setting); err != nil {
-				return err
-			}
-		}
-	}
-	return sess.Commit()
-}
-
 func CreateSystemSettingsTable(x *xorm.Engine) error {
-	if err := x.Sync(new(SystemSetting)); err != nil {
-		return fmt.Errorf("sync2: %w", err)
-	}
-
-	// migrate xx to database
-	sysSettings := []*SystemSetting{
-		{
-			SettingKey:   "picture.disable_gravatar",
-			SettingValue: strconv.FormatBool(setting.DisableGravatar),
-		},
-		{
-			SettingKey:   "picture.enable_federated_avatar",
-			SettingValue: strconv.FormatBool(setting.EnableFederatedAvatar),
-		},
-	}
-
-	return insertSettingsIfNotExist(x, sysSettings)
+	return x.Sync(new(SystemSetting))
 }

models/repo.go

@@ -16,18 +16,14 @@ import (
 	issues_model "code.gitea.io/gitea/models/issues"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
-	system_model "code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/log"
 )
 
 // Init initialize model
 func Init(ctx context.Context) error {
-	if err := unit.LoadUnitConfig(); err != nil {
-		return err
-	}
-	return system_model.Init(ctx)
+	return unit.LoadUnitConfig()
 }
 
 type repoChecker struct {