diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go index e40aa3f5424..b0bfa440897 100644 --- a/models/avatars/avatar.go +++ b/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) } diff --git a/models/avatars/avatar_test.go b/models/avatars/avatar_test.go index 59daaeb6695..c8f7a6574bf 100644 --- a/models/avatars/avatar_test.go +++ b/models/avatars/avatar_test.go @@ -10,6 +10,7 @@ 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" ) @@ -17,19 +18,16 @@ import ( 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), diff --git a/models/migrations/v1_18/v227.go b/models/migrations/v1_18/v227.go index 78c80f74ece..5fe5dcd0c95 100644 --- a/models/migrations/v1_18/v227.go +++ b/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)) } diff --git a/models/repo.go b/models/repo.go index de0f7ee3452..82433ca0552 100644 --- a/models/repo.go +++ b/models/repo.go @@ -16,7 +16,6 @@ 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" @@ -24,10 +23,7 @@ import ( // 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 { diff --git a/models/system/setting.go b/models/system/setting.go index 9fffa74e94a..1ae6f5c6528 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -5,26 +5,21 @@ package system import ( "context" - "fmt" - "net/url" - "strconv" - "strings" + "math" + "sync" + "time" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/cache" - setting_module "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting/config" "code.gitea.io/gitea/modules/timeutil" - - "strk.kbt.io/projects/go/libravatar" - "xorm.io/builder" ) -// Setting is a key value store of user settings type Setting struct { ID int64 `xorm:"pk autoincr"` - SettingKey string `xorm:"varchar(255) unique"` // ensure key is always lowercase + SettingKey string `xorm:"varchar(255) unique"` // key should be lowercase SettingValue string `xorm:"text"` - Version int `xorm:"version"` // prevent to override + Version int `xorm:"version"` Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` } @@ -34,281 +29,119 @@ func (s *Setting) TableName() string { return "system_setting" } -func (s *Setting) GetValueBool() bool { - if s == nil { - return false - } - - b, _ := strconv.ParseBool(s.SettingValue) - return b -} - func init() { db.RegisterModel(new(Setting)) } -// ErrSettingIsNotExist represents an error that a setting is not exist with special key -type ErrSettingIsNotExist struct { - Key string -} - -// Error implements error -func (err ErrSettingIsNotExist) Error() string { - return fmt.Sprintf("System setting[%s] is not exist", err.Key) -} - -// IsErrSettingIsNotExist return true if err is ErrSettingIsNotExist -func IsErrSettingIsNotExist(err error) bool { - _, ok := err.(ErrSettingIsNotExist) - return ok -} - -// ErrDataExpired represents an error that update a record which has been updated by another thread -type ErrDataExpired struct { - Key string -} - -// Error implements error -func (err ErrDataExpired) Error() string { - return fmt.Sprintf("System setting[%s] has been updated by another thread", err.Key) -} - -// IsErrDataExpired return true if err is ErrDataExpired -func IsErrDataExpired(err error) bool { - _, ok := err.(ErrDataExpired) - return ok -} - -// GetSetting returns specific setting without using the cache -func GetSetting(ctx context.Context, key string) (*Setting, error) { - v, err := GetSettings(ctx, []string{key}) - if err != nil { - return nil, err - } - if len(v) == 0 { - return nil, ErrSettingIsNotExist{key} - } - return v[strings.ToLower(key)], nil -} - -const contextCacheKey = "system_setting" - -// GetSettingWithCache returns the setting value via the key -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 - }) - }) -} +const keyRevision = "revision" -// GetSettingBool return bool value of setting, -// none existing keys and errors are ignored and result in 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 +func GetRevision(ctx context.Context) int { + revision := &Setting{SettingKey: keyRevision} + if has, err := db.GetByBean(ctx, revision); err != nil { + return 0 + } else if !has { + err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) + if err != nil { + return 0 + } + return 1 + } else if revision.Version <= 0 || revision.Version >= math.MaxInt-1 { + _, err = db.Exec(ctx, "UPDATE system_setting SET version=1 WHERE setting_key=?", keyRevision) + if err != nil { + return 0 + } + return 1 } + return revision.Version } -func GetSettingWithCacheBool(ctx context.Context, key string, defaultVal bool) bool { - s, _ := GetSettingWithCache(ctx, key, strconv.FormatBool(defaultVal)) - v, _ := strconv.ParseBool(s) - return v -} - -// GetSettings returns specific settings -func GetSettings(ctx context.Context, keys []string) (map[string]*Setting, error) { - for i := 0; i < len(keys); i++ { - keys[i] = strings.ToLower(keys[i]) - } - settings := make([]*Setting, 0, len(keys)) +func GetAllSettings(ctx context.Context) (revision int, res map[string]string, err error) { + _ = GetRevision(ctx) // prepare the "revision" key ahead + var settings []*Setting if err := db.GetEngine(ctx). - Where(builder.In("setting_key", keys)). Find(&settings); err != nil { - return nil, err + return 0, nil, err } - settingsMap := make(map[string]*Setting) + res = make(map[string]string) for _, s := range settings { - settingsMap[s.SettingKey] = s - } - return settingsMap, nil -} - -type AllSettings map[string]*Setting - -func (settings AllSettings) Get(key string) Setting { - if v, ok := settings[strings.ToLower(key)]; ok { - return *v - } - return Setting{} -} - -func (settings AllSettings) GetBool(key string) bool { - b, _ := strconv.ParseBool(settings.Get(key).SettingValue) - return b -} - -func (settings AllSettings) GetVersion(key string) int { - return settings.Get(key).Version -} - -// GetAllSettings returns all settings from user -func GetAllSettings(ctx context.Context) (AllSettings, error) { - settings := make([]*Setting, 0, 5) - if err := db.GetEngine(ctx). - Find(&settings); err != nil { - return nil, err - } - settingsMap := make(map[string]*Setting) - for _, s := range settings { - settingsMap[s.SettingKey] = s - } - return settingsMap, nil -} - -// DeleteSetting deletes a specific setting for a user -func DeleteSetting(ctx context.Context, setting *Setting) error { - cache.RemoveContextData(ctx, contextCacheKey, setting.SettingKey) - cache.Remove(genSettingCacheKey(setting.SettingKey)) - _, err := db.GetEngine(ctx).Delete(setting) - return err -} - -func SetSettingNoVersion(ctx context.Context, key, value string) error { - s, err := GetSetting(ctx, key) - if IsErrSettingIsNotExist(err) { - return SetSetting(ctx, &Setting{ - SettingKey: key, - SettingValue: value, - }) - } - if err != nil { - return err - } - s.SettingValue = value - return SetSetting(ctx, s) -} - -// SetSetting updates a users' setting for a specific key -func SetSetting(ctx context.Context, setting *Setting) error { - if err := upsertSettingValue(ctx, strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil { - return err - } - - setting.Version++ - - cc := cache.GetCache() - if cc != nil { - if err := cc.Put(genSettingCacheKey(setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds()); err != nil { - return err + if s.SettingKey == keyRevision { + revision = s.Version } + res[s.SettingKey] = s.SettingValue } - cache.SetContextData(ctx, contextCacheKey, setting.SettingKey, setting.SettingValue) - return nil + return revision, res, nil } -func upsertSettingValue(parentCtx context.Context, key, value string, version int) error { - return db.WithTx(parentCtx, func(ctx context.Context) error { +func SetSettings(ctx context.Context, settings map[string]string) error { + _ = GetRevision(ctx) // prepare the "revision" key ahead + return db.WithTx(ctx, func(ctx context.Context) error { e := db.GetEngine(ctx) - - // here we use a general method to do a safe upsert for different databases (and most transaction levels) - // 1. try to UPDATE the record and acquire the transaction write lock - // if UPDATE returns non-zero rows are changed, OK, the setting is saved correctly - // if UPDATE returns "0 rows changed", two possibilities: (a) record doesn't exist (b) value is not changed - // 2. do a SELECT to check if the row exists or not (we already have the transaction lock) - // 3. if the row doesn't exist, do an INSERT (we are still protected by the transaction lock, so it's safe) - // - // to optimize the SELECT in step 2, we can use an extra column like `revision=revision+1` - // to make sure the UPDATE always returns a non-zero value for existing (unchanged) records. - - res, err := e.Exec("UPDATE system_setting SET setting_value=?, version = version+1 WHERE setting_key=? AND version=?", value, key, version) - if err != nil { - return err - } - rows, _ := res.RowsAffected() - if rows > 0 { - // the existing row is updated, so we can return - return nil - } - - // in case the value isn't changed, update would return 0 rows changed, so we need this check - has, err := e.Exist(&Setting{SettingKey: key}) + _, err := db.Exec(ctx, "UPDATE system_setting SET version=version+1 WHERE setting_key=?", keyRevision) if err != nil { return err } - if has { - return ErrDataExpired{Key: key} + for k, v := range settings { + res, err := e.Exec("UPDATE system_setting SET setting_value=? WHERE setting_key=?", v, k) + if err != nil { + return err + } + rows, _ := res.RowsAffected() + if rows == 0 { // if no existing row, insert a new row + if _, err = e.Insert(&Setting{SettingKey: k, SettingValue: v}); err != nil { + return err + } + } } - - // if no existing row, insert a new row - _, err = e.Insert(&Setting{SettingKey: key, SettingValue: value}) - return err + return nil }) } -var ( - GravatarSourceURL *url.URL - LibravatarService *libravatar.Libravatar -) +type dbConfigCachedGetter struct { + mu sync.RWMutex -func Init(ctx context.Context) error { - disableGravatar, err := GetSettingBool(ctx, KeyPictureDisableGravatar, setting_module.GetDefaultDisableGravatar()) - if err != nil { - return err - } + cacheTime time.Time + revision int + settings map[string]string +} - enableFederatedAvatar, err := GetSettingBool(ctx, KeyPictureEnableFederatedAvatar, setting_module.GetDefaultEnableFederatedAvatar(disableGravatar)) - if err != nil { - return err - } +var _ config.DynKeyGetter = (*dbConfigCachedGetter)(nil) - if setting_module.OfflineMode { - if !disableGravatar { - if err := SetSettingNoVersion(ctx, KeyPictureDisableGravatar, "true"); err != nil { - return fmt.Errorf("failed to set setting %q: %w", KeyPictureDisableGravatar, err) - } - } - disableGravatar = true +func (d *dbConfigCachedGetter) GetValue(ctx context.Context, key string) (v string, has bool) { + d.mu.RLock() + defer d.mu.RUnlock() + v, has = d.settings[key] + return v, has +} - if enableFederatedAvatar { - if err := SetSettingNoVersion(ctx, KeyPictureEnableFederatedAvatar, "false"); err != nil { - return fmt.Errorf("failed to set setting %q: %w", KeyPictureEnableFederatedAvatar, err) - } - } - enableFederatedAvatar = false +func (d *dbConfigCachedGetter) GetRevision(ctx context.Context) int { + d.mu.RLock() + defer d.mu.RUnlock() + if time.Since(d.cacheTime) < time.Second { + return d.revision } - - if enableFederatedAvatar || !disableGravatar { - var err error - GravatarSourceURL, err = url.Parse(setting_module.GravatarSource) + if GetRevision(ctx) != d.revision { + d.mu.RUnlock() + d.mu.Lock() + rev, set, err := GetAllSettings(ctx) if err != nil { - return fmt.Errorf("failed to parse Gravatar URL(%s): %w", setting_module.GravatarSource, err) - } - } - - if GravatarSourceURL != nil && enableFederatedAvatar { - LibravatarService = libravatar.New() - if GravatarSourceURL.Scheme == "https" { - LibravatarService.SetUseHTTPS(true) - LibravatarService.SetSecureFallbackHost(GravatarSourceURL.Host) + log.Error("Unable to get all settings: %v", err) } else { - LibravatarService.SetUseHTTPS(false) - LibravatarService.SetFallbackHost(GravatarSourceURL.Host) + d.cacheTime = time.Now() + d.revision = rev + d.settings = set } + d.mu.Unlock() + d.mu.RLock() } - return nil + return d.revision +} + +func (d *dbConfigCachedGetter) InvalidateCache() { + d.mu.Lock() + d.cacheTime = time.Time{} + d.mu.Unlock() +} + +func NewDatabaseDynKeyGetter() config.DynKeyGetter { + return &dbConfigCachedGetter{} } diff --git a/models/system/setting_key.go b/models/system/setting_key.go deleted file mode 100644 index ad083ed1ead..00000000000 --- a/models/system/setting_key.go +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package system - -// enumerate all system setting keys -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 -} diff --git a/models/system/setting_test.go b/models/system/setting_test.go index e6997ad7832..4117420e5f2 100644 --- a/models/system/setting_test.go +++ b/models/system/setting_test.go @@ -4,7 +4,6 @@ package system_test import ( - "strings" "testing" "code.gitea.io/gitea/models/db" @@ -15,43 +14,29 @@ import ( ) func TestSettings(t *testing.T) { - keyName := "server.LFS_LOCKS_PAGING_NUM" + keyName := "test.key" assert.NoError(t, unittest.PrepareTestDatabase()) - newSetting := &system.Setting{SettingKey: keyName, SettingValue: "50"} + assert.NoError(t, db.TruncateBeans(db.DefaultContext, &system.Setting{})) - // create setting - err := system.SetSetting(db.DefaultContext, newSetting) + rev, settings, err := system.GetAllSettings(db.DefaultContext) assert.NoError(t, err) - // test about saving unchanged values - err = system.SetSetting(db.DefaultContext, newSetting) - assert.NoError(t, err) - - // get specific setting - settings, err := system.GetSettings(db.DefaultContext, []string{keyName}) - assert.NoError(t, err) - assert.Len(t, settings, 1) - assert.EqualValues(t, newSetting.SettingValue, settings[strings.ToLower(keyName)].SettingValue) + assert.EqualValues(t, 1, rev) + assert.Len(t, settings, 1) // there is only one "revision" key - // updated setting - updatedSetting := &system.Setting{SettingKey: keyName, SettingValue: "100", Version: settings[strings.ToLower(keyName)].Version} - err = system.SetSetting(db.DefaultContext, updatedSetting) + err = system.SetSettings(db.DefaultContext, map[string]string{keyName: "true"}) assert.NoError(t, err) - - value, err := system.GetSetting(db.DefaultContext, keyName) + rev, settings, err = system.GetAllSettings(db.DefaultContext) assert.NoError(t, err) - assert.EqualValues(t, updatedSetting.SettingValue, value.SettingValue) - - // get all settings - settings, err = system.GetAllSettings(db.DefaultContext) - assert.NoError(t, err) - assert.Len(t, settings, 3) - assert.EqualValues(t, updatedSetting.SettingValue, settings[strings.ToLower(updatedSetting.SettingKey)].SettingValue) + assert.EqualValues(t, 2, rev) + assert.Len(t, settings, 2) + assert.EqualValues(t, "true", settings[keyName]) - // delete setting - err = system.DeleteSetting(db.DefaultContext, &system.Setting{SettingKey: strings.ToLower(keyName)}) + err = system.SetSettings(db.DefaultContext, map[string]string{keyName: "false"}) assert.NoError(t, err) - settings, err = system.GetAllSettings(db.DefaultContext) + rev, settings, err = system.GetAllSettings(db.DefaultContext) assert.NoError(t, err) + assert.EqualValues(t, 3, rev) assert.Len(t, settings, 2) + assert.EqualValues(t, "false", settings[keyName]) } diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 6db99cd393e..4c668ad04bf 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -13,11 +13,12 @@ import ( "testing" "code.gitea.io/gitea/models/db" - system_model "code.gitea.io/gitea/models/system" + "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/auth/password/hash" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/setting/config" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" @@ -145,13 +146,11 @@ func MainTest(m *testing.M, testOpts ...*TestOptions) { setting.IncomingEmail.ReplyToAddress = "incoming+%{token}@localhost" + config.SetDynGetter(system.NewDatabaseDynKeyGetter()) + if err = storage.Init(); err != nil { fatalTestError("storage.Init: %v\n", err) } - if err = system_model.Init(db.DefaultContext); err != nil { - fatalTestError("models.Init: %v\n", err) - } - if err = util.RemoveAll(repoRootPath); err != nil { fatalTestError("util.RemoveAll: %v\n", err) } diff --git a/models/user/avatar.go b/models/user/avatar.go index 7f996fa79ab..8c9abecbea9 100644 --- a/models/user/avatar.go +++ b/models/user/avatar.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/models/avatars" "code.gitea.io/gitea/models/db" - system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/avatar" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -67,9 +66,7 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string { useLocalAvatar := false autoGenerateAvatar := false - disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar, - setting.GetDefaultDisableGravatar(), - ) + disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx) switch { case u.UseCustomAvatar: diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 09310925971..827b2a98493 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -12,7 +12,6 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" - system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" @@ -103,12 +102,6 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) { assert.EqualValues(t, []string{"readme.md"}, headCommit.Modified) } -func initGravatarSource(t *testing.T) { - setting.GravatarSource = "https://secure.gravatar.com/avatar" - err := system_model.Init(db.DefaultContext) - assert.NoError(t, err) -} - func TestPushCommits_AvatarLink(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) @@ -132,7 +125,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { }, } - initGravatarSource(t) + setting.GravatarSource = "https://secure.gravatar.com/avatar" assert.Equal(t, "https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon&s="+strconv.Itoa(28*setting.Avatar.RenderedSizeFactor), diff --git a/modules/setting/config.go b/modules/setting/config.go new file mode 100644 index 00000000000..db189f44ac5 --- /dev/null +++ b/modules/setting/config.go @@ -0,0 +1,55 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "sync" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting/config" +) + +type PictureStruct struct { + DisableGravatar *config.Value[bool] + EnableFederatedAvatar *config.Value[bool] +} + +type ConfigStruct struct { + Picture *PictureStruct +} + +var ( + defaultConfig *ConfigStruct + defaultConfigOnce sync.Once +) + +func initDefaultConfig() { + config.SetCfgSecKeyGetter(&cfgSecKeyGetter{}) + defaultConfig = &ConfigStruct{ + Picture: &PictureStruct{ + DisableGravatar: config.Bool(false, config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}, "picture.disable_gravatar"), + EnableFederatedAvatar: config.Bool(false, config.CfgSecKey{Sec: "picture", Key: "ENABLE_FEDERATED_AVATAR"}, "picture.enable_federated_avatar"), + }, + } +} + +func Config() *ConfigStruct { + defaultConfigOnce.Do(initDefaultConfig) + return defaultConfig +} + +type cfgSecKeyGetter struct{} + +func (c cfgSecKeyGetter) GetValue(sec, key string) (v string, has bool) { + cfgSec, err := CfgProvider.GetSection(sec) + if err != nil { + log.Error("Unable to get config section: %q", sec) + return "", false + } + cfgKey := ConfigSectionKey(cfgSec, key) + if cfgKey == nil { + return "", false + } + return cfgKey.Value(), true +} diff --git a/modules/setting/config/getter.go b/modules/setting/config/getter.go new file mode 100644 index 00000000000..99f9a4775a8 --- /dev/null +++ b/modules/setting/config/getter.go @@ -0,0 +1,49 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package config + +import ( + "context" + "sync" +) + +var getterMu sync.RWMutex + +type CfgSecKeyGetter interface { + GetValue(sec, key string) (v string, has bool) +} + +var cfgSecKeyGetterInternal CfgSecKeyGetter + +func SetCfgSecKeyGetter(p CfgSecKeyGetter) { + getterMu.Lock() + cfgSecKeyGetterInternal = p + getterMu.Unlock() +} + +func GetCfgSecKeyGetter() CfgSecKeyGetter { + getterMu.RLock() + defer getterMu.RUnlock() + return cfgSecKeyGetterInternal +} + +type DynKeyGetter interface { + GetValue(ctx context.Context, key string) (v string, has bool) + GetRevision(ctx context.Context) int + InvalidateCache() +} + +var dynKeyGetterInternal DynKeyGetter + +func SetDynGetter(p DynKeyGetter) { + getterMu.Lock() + dynKeyGetterInternal = p + getterMu.Unlock() +} + +func GetDynGetter() DynKeyGetter { + getterMu.RLock() + defer getterMu.RUnlock() + return dynKeyGetterInternal +} diff --git a/modules/setting/config/value.go b/modules/setting/config/value.go new file mode 100644 index 00000000000..817fcdb7862 --- /dev/null +++ b/modules/setting/config/value.go @@ -0,0 +1,81 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package config + +import ( + "context" + "strconv" + "sync" +) + +type CfgSecKey struct { + Sec, Key string +} + +type Value[T any] struct { + mu sync.RWMutex + + cfgSecKey CfgSecKey + dynKey string + + def, value T + revision int +} + +func (value *Value[T]) parse(s string) (v T) { + switch any(v).(type) { + case bool: + b, _ := strconv.ParseBool(s) + return any(b).(T) + default: + panic("unsupported config type, please complete the code") + } +} + +func (value *Value[T]) Value(ctx context.Context) (v T) { + dg := GetDynGetter() + if dg == nil { + // this is an edge case: the database is not initialized but the system setting is going to be used + // it should panic to avoid inconsistent config values (from config / system setting) and fix the code + panic("no config dyn value getter") + } + + rev := dg.GetRevision(ctx) + + // if the revision in database doesn't change, use the last value + value.mu.RLock() + if rev == value.revision { + v = value.value + value.mu.RUnlock() + return v + } + value.mu.RUnlock() + + // try to parse the config and cache it + var valStr *string + if dynVal, has := dg.GetValue(ctx, value.dynKey); has { + valStr = &dynVal + } else if cfgVal, has := GetCfgSecKeyGetter().GetValue(value.cfgSecKey.Sec, value.cfgSecKey.Key); has { + valStr = &cfgVal + } + if valStr == nil { + v = value.def + } else { + v = value.parse(*valStr) + } + + value.mu.Lock() + value.value = v + value.revision = rev + value.mu.Unlock() + return v +} + +func (value *Value[T]) DynKey() string { + return value.dynKey +} + +func Bool(def bool, cfgSecKey CfgSecKey, dynKey string) *Value[bool] { + return &Value[bool]{def: def, cfgSecKey: cfgSecKey, dynKey: dynKey} +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ebd55f39be0..4420af1588d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3157,7 +3157,6 @@ config.access_log_mode = Access Log Mode config.access_log_template = Access Log Template config.xorm_log_sql = Log SQL -config.get_setting_failed = Get setting %s failed config.set_setting_failed = Set setting %s failed monitor.stats = Stats diff --git a/routers/common/db.go b/routers/common/db.go index 2e86fbd0fd4..547f727ce24 100644 --- a/routers/common/db.go +++ b/routers/common/db.go @@ -10,8 +10,10 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/migrations" + system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/setting/config" "xorm.io/xorm" ) @@ -36,6 +38,7 @@ func InitDBEngine(ctx context.Context) (err error) { time.Sleep(setting.Database.DBConnectBackoff) } db.HasEngine = true + config.SetDynGetter(system_model.NewDatabaseDynKeyGetter()) return nil } diff --git a/routers/install/install.go b/routers/install/install.go index 3eb16b9ce85..185e4bf6bf6 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -430,15 +430,14 @@ func SubmitInstall(ctx *context.Context) { cfg.Section("service").Key("ENABLE_NOTIFY_MAIL").SetValue(fmt.Sprint(form.MailNotify)) cfg.Section("server").Key("OFFLINE_MODE").SetValue(fmt.Sprint(form.OfflineMode)) - // if you are reinstalling, this maybe not right because of missing version - if err := system_model.SetSettingNoVersion(ctx, system_model.KeyPictureDisableGravatar, strconv.FormatBool(form.DisableGravatar)); err != nil { - ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form) - return - } - if err := system_model.SetSettingNoVersion(ctx, system_model.KeyPictureEnableFederatedAvatar, strconv.FormatBool(form.EnableFederatedAvatar)); err != nil { + if err := system_model.SetSettings(ctx, map[string]string{ + setting.Config().Picture.DisableGravatar.DynKey(): strconv.FormatBool(form.DisableGravatar), + setting.Config().Picture.EnableFederatedAvatar.DynKey(): strconv.FormatBool(form.EnableFederatedAvatar), + }); err != nil { ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form) return } + cfg.Section("openid").Key("ENABLE_OPENID_SIGNIN").SetValue(fmt.Sprint(form.EnableOpenIDSignIn)) cfg.Section("openid").Key("ENABLE_OPENID_SIGNUP").SetValue(fmt.Sprint(form.EnableOpenIDSignUp)) cfg.Section("service").Key("DISABLE_REGISTRATION").SetValue(fmt.Sprint(form.DisableRegistration)) diff --git a/routers/web/admin/config.go b/routers/web/admin/config.go index c70a2d1c951..c827f2a4f50 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -5,19 +5,19 @@ package admin import ( - "fmt" "net/http" "net/url" - "strconv" "strings" system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/setting/config" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/mailer" @@ -101,16 +101,6 @@ func Config(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("admin.config") ctx.Data["PageIsAdminConfig"] = true - systemSettings, err := system_model.GetAllSettings(ctx) - if err != nil { - ctx.ServerError("system_model.GetAllSettings", err) - return - } - - // All editable settings from UI - ctx.Data["SystemSettings"] = systemSettings - ctx.PageData["adminConfigPage"] = true - ctx.Data["CustomConf"] = setting.CustomConf ctx.Data["AppUrl"] = setting.AppURL ctx.Data["AppBuiltWith"] = setting.AppBuiltWith @@ -170,7 +160,8 @@ func Config(ctx *context.Context) { ctx.Data["LogSQL"] = setting.Database.LogSQL ctx.Data["Loggers"] = log.GetManager().DumpLoggers() - + config.GetDynGetter().InvalidateCache() + ctx.Data["SystemConfig"] = setting.Config() prepareDeprecatedWarningsAlert(ctx) ctx.HTML(http.StatusOK, tplConfig) @@ -178,51 +169,19 @@ func Config(ctx *context.Context) { func ChangeConfig(ctx *context.Context) { key := strings.TrimSpace(ctx.FormString("key")) - if key == "" { - ctx.JSONRedirect(ctx.Req.URL.String()) - return - } value := ctx.FormString("value") - version := ctx.FormInt("version") - - if check, ok := changeConfigChecks[key]; ok { - if err := check(ctx, value); err != nil { - log.Warn("refused to set setting: %v", err) - ctx.JSON(http.StatusOK, map[string]string{ - "err": ctx.Tr("admin.config.set_setting_failed", key), - }) - return - } + cfg := setting.Config() + allowedKeys := container.SetOf(cfg.Picture.DisableGravatar.DynKey(), cfg.Picture.EnableFederatedAvatar.DynKey()) + if !allowedKeys.Contains(key) { + ctx.JSONError(ctx.Tr("admin.config.set_setting_failed", key)) + return } - - if err := system_model.SetSetting(ctx, &system_model.Setting{ - SettingKey: key, - SettingValue: value, - Version: version, - }); err != nil { + if err := system_model.SetSettings(ctx, map[string]string{key: value}); err != nil { log.Error("set setting failed: %v", err) - ctx.JSON(http.StatusOK, map[string]string{ - "err": ctx.Tr("admin.config.set_setting_failed", key), - }) + ctx.JSONError(ctx.Tr("admin.config.set_setting_failed", key)) return } - ctx.JSON(http.StatusOK, map[string]any{ - "version": version + 1, - }) -} - -var changeConfigChecks = map[string]func(ctx *context.Context, newValue string) error{ - system_model.KeyPictureDisableGravatar: func(_ *context.Context, newValue string) error { - if v, _ := strconv.ParseBool(newValue); setting.OfflineMode && !v { - return fmt.Errorf("%q should be true when OFFLINE_MODE is true", system_model.KeyPictureDisableGravatar) - } - return nil - }, - system_model.KeyPictureEnableFederatedAvatar: func(_ *context.Context, newValue string) error { - if v, _ := strconv.ParseBool(newValue); setting.OfflineMode && v { - return fmt.Errorf("%q cannot be false when OFFLINE_MODE is true", system_model.KeyPictureEnableFederatedAvatar) - } - return nil - }, + config.GetDynGetter().InvalidateCache() + ctx.JSONOK() } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 4c6551501e4..18d7313e720 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/models/db" org_model "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" - system_model "code.gitea.io/gitea/models/system" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/password" "code.gitea.io/gitea/modules/base" @@ -308,17 +307,18 @@ func ViewUser(ctx *context.Context) { ctx.HTML(http.StatusOK, tplUserView) } -// EditUser show editing user page -func EditUser(ctx *context.Context) { +func editUserCommon(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("admin.users.edit_account") ctx.Data["PageIsAdminUsers"] = true ctx.Data["DisableRegularOrgCreation"] = setting.Admin.DisableRegularOrgCreation ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() - ctx.Data["DisableGravatar"] = system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar, - setting.GetDefaultDisableGravatar(), - ) + ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx) +} +// EditUser show editing user page +func EditUser(ctx *context.Context) { + editUserCommon(ctx) prepareUserInfo(ctx) if ctx.Written() { return @@ -329,19 +329,13 @@ func EditUser(ctx *context.Context) { // EditUserPost response for editing user func EditUserPost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.AdminEditUserForm) - ctx.Data["Title"] = ctx.Tr("admin.users.edit_account") - ctx.Data["PageIsAdminUsers"] = true - ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations - ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() - ctx.Data["DisableGravatar"] = system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar, - setting.GetDefaultDisableGravatar()) - + editUserCommon(ctx) u := prepareUserInfo(ctx) if ctx.Written() { return } + form := web.GetForm(ctx).(*forms.AdminEditUserForm) if ctx.HasError() { ctx.HTML(http.StatusOK, tplUserEdit) return diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index f40c2f30879..72e60bda587 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" - system_model "code.gitea.io/gitea/models/system" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" @@ -44,9 +43,7 @@ func Profile(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings.profile") ctx.Data["PageIsSettingsProfile"] = true ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() - ctx.Data["DisableGravatar"] = system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar, - setting.GetDefaultDisableGravatar(), - ) + ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx) ctx.HTML(http.StatusOK, tplSettingsProfile) } @@ -88,9 +85,7 @@ func ProfilePost(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsProfile"] = true ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() - ctx.Data["DisableGravatar"] = system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar, - setting.GetDefaultDisableGravatar(), - ) + ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx) if ctx.HasError() { ctx.HTML(http.StatusOK, tplSettingsProfile) diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index 57722f98988..7eb9d086e61 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -292,15 +292,15 @@
{{ctx.Locale.Tr "admin.config.disable_gravatar"}}
-
- +
+
{{ctx.Locale.Tr "admin.config.enable_federated_avatar"}}
-
- +
+
diff --git a/web_src/js/features/admin/config.js b/web_src/js/features/admin/config.js index f5d8fae8fac..c3823425adb 100644 --- a/web_src/js/features/admin/config.js +++ b/web_src/js/features/admin/config.js @@ -1,37 +1,24 @@ -import $ from 'jquery'; import {showTemporaryTooltip} from '../../modules/tippy.js'; +import {POST} from '../../modules/fetch.js'; -const {appSubUrl, csrfToken, pageData} = window.config; +const {appSubUrl} = window.config; export function initAdminConfigs() { - const isAdminConfigPage = pageData?.adminConfigPage; - if (!isAdminConfigPage) return; + const elAdminConfig = document.querySelector('.page-content.admin.config'); + if (!elAdminConfig) return; - $("input[type='checkbox']").on('change', (e) => { - const $this = $(e.currentTarget); - $.ajax({ - url: `${appSubUrl}/admin/config`, - type: 'POST', - data: { - _csrf: csrfToken, - key: $this.attr('name'), - value: $this.is(':checked'), - version: $this.attr('version'), - } - }).done((resp) => { - if (resp) { - if (resp.redirect) { - window.location.href = resp.redirect; - } else if (resp.version) { - $this.attr('version', resp.version); - } else if (resp.err) { - showTemporaryTooltip(e.currentTarget, resp.err); - $this.prop('checked', !$this.is(':checked')); - } + for (const el of elAdminConfig.querySelectorAll('input[type="checkbox"][data-config-dyn-key]')) { + el.addEventListener('change', async () => { + try { + const resp = await POST(`${appSubUrl}/admin/config`, { + data: new URLSearchParams({key: el.getAttribute('data-config-dyn-key'), value: el.checked}), + }); + const json = await resp.json(); + if (json.errorMessage) throw new Error(json.errorMessage); + } catch (ex) { + showTemporaryTooltip(el, ex.toString()); + el.checked = !el.checked; } }); - - e.preventDefault(); - return false; - }); + } }