diff --git a/models/system/setting.go b/models/system/setting.go index 4472b4c228..cf6b2dea1a 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -150,3 +150,34 @@ func (d *dbConfigCachedGetter) InvalidateCache() { func NewDatabaseDynKeyGetter() config.DynKeyGetter { return &dbConfigCachedGetter{} } + +type dbConfigSetter struct { + mu sync.RWMutex +} + +var _ config.DynKeySetter = (*dbConfigSetter)(nil) + +func (d *dbConfigSetter) SetValue(ctx context.Context, dynKey, value string) error { + d.mu.RLock() + defer d.mu.RUnlock() + _ = GetRevision(ctx) // prepare the "revision" key ahead + return db.WithTx(ctx, func(ctx context.Context) error { + e := db.GetEngine(ctx) + res, err := e.Exec("UPDATE system_setting SET version=version+1, setting_value=? WHERE setting_key=?", value, dynKey) + 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: dynKey, SettingValue: value}); err != nil { + return err + } + } + + return nil + }) +} + +func NewDatabaseDynKeySetter() config.DynKeySetter { + return &dbConfigSetter{} +} diff --git a/modules/setting/config.go b/modules/setting/config.go index 5476cd86b6..ff4960a132 100644 --- a/modules/setting/config.go +++ b/modules/setting/config.go @@ -58,15 +58,15 @@ type ConfigStruct struct { } var ( - defaultConfig *ConfigStruct - defaultConfigOnce sync.Once + defaultConfig *ConfigStruct + ConfigOnce sync.Once ) func initDefaultConfig() { config.SetCfgSecKeyGetter(&cfgSecKeyGetter{}) defaultConfig = &ConfigStruct{ Picture: &PictureStruct{ - EnableGravatar: config.ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert(), + EnableGravatar: config.ValueJSON[bool]("picture.enable_gravatar").SelectFrom("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert(), EnableFederatedAvatar: config.ValueJSON[bool]("picture.enable_federated_avatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "ENABLE_FEDERATED_AVATAR"}), }, Repository: &RepositoryStruct{ @@ -77,7 +77,7 @@ func initDefaultConfig() { } func Config() *ConfigStruct { - defaultConfigOnce.Do(initDefaultConfig) + ConfigOnce.Do(initDefaultConfig) return defaultConfig } diff --git a/modules/setting/config/setter.go b/modules/setting/config/setter.go new file mode 100644 index 0000000000..f8e63971c2 --- /dev/null +++ b/modules/setting/config/setter.go @@ -0,0 +1,29 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package config + +import ( + "context" + "sync" +) + +var setterMu sync.RWMutex + +type DynKeySetter interface { + SetValue(ctx context.Context, dynKey, value string) error +} + +var dynKeySetterInternal DynKeySetter + +func SetDynSetter(p DynKeySetter) { + setterMu.Lock() + dynKeySetterInternal = p + setterMu.Unlock() +} + +func GetDynSetter() DynKeySetter { + getterMu.RLock() + defer getterMu.RUnlock() + return dynKeySetterInternal +} diff --git a/modules/setting/config/value.go b/modules/setting/config/value.go index 0836f217ee..1d01099428 100644 --- a/modules/setting/config/value.go +++ b/modules/setting/config/value.go @@ -5,6 +5,7 @@ package config import ( "context" + "fmt" "sync" "code.gitea.io/gitea/modules/json" @@ -19,8 +20,8 @@ type CfgSecKey struct { type Value[T any] struct { mu sync.RWMutex - cfgSecKey CfgSecKey - dynKey string + cfgSecKey CfgSecKey + dynKey, selectFromKey string def, value T revision int @@ -35,19 +36,41 @@ func (value *Value[T]) parse(key, valStr string) (v T) { } } + return value.invert(v) +} + +func (value *Value[T]) invertBoolStr(val string) (inverted string) { + if val == "true" { + return "false" + } + + return "true" +} + +func (value *Value[T]) invert(val T) (v T) { + v = val if value.flipBoolean { + fmt.Printf("Flipping boolean value '%v'...\n", val) // if value is of type bool - if _, ok := any(v).(bool); ok { + if _, ok := any(val).(bool); ok { // invert the boolean value upon retrieval - v = any(!any(v).(bool)).(T) + v = any(!any(val).(bool)).(T) } else { - log.Warn("Ignoring attempt to invert key '%q' for non boolean type", key) + log.Warn("Ignoring attempt to invert key '%q' for non boolean type", value.selectFromKey) } } return v } +func (value *Value[T]) getKey() string { + if value.selectFromKey != "" { + return value.selectFromKey + } + + return value.dynKey +} + func (value *Value[T]) Value(ctx context.Context) (v T) { dg := GetDynGetter() if dg == nil { @@ -69,7 +92,7 @@ func (value *Value[T]) Value(ctx context.Context) (v T) { // try to parse the config and cache it var valStr *string - if dynVal, has := dg.GetValue(ctx, value.dynKey); has { + if dynVal, has := dg.GetValue(ctx, value.getKey()); has { valStr = &dynVal } else if cfgVal, has := GetCfgSecKeyGetter().GetValue(value.cfgSecKey.Sec, value.cfgSecKey.Key); has { valStr = &cfgVal @@ -91,6 +114,10 @@ func (value *Value[T]) DynKey() string { return value.dynKey } +func (value *Value[T]) SelectFromKey() string { + return value.selectFromKey +} + func (value *Value[T]) WithDefault(def T) *Value[T] { value.def = def return value @@ -110,6 +137,29 @@ func (value *Value[bool]) Invert() *Value[bool] { return value } +func (value *Value[any]) SelectFrom(sectionName string) *Value[any] { + value.selectFromKey = sectionName + return value +} + +func (value *Value[any]) SetValue(val string) error { + ctx := context.Background() + ds := GetDynSetter() + if ds == 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") + } + + fmt.Printf("Setting value '%s' with old key '%s' using key '%s'\n", val, value.selectFromKey, value.dynKey) + + if value.flipBoolean { + return ds.SetValue(ctx, value.getKey(), value.invertBoolStr(val)) + } + + return ds.SetValue(ctx, value.getKey(), val) +} + func ValueJSON[T any](dynKey string) *Value[T] { return &Value[T]{dynKey: dynKey} } diff --git a/modules/setting/config/value_test.go b/modules/setting/config/value_test.go index 0697165717..be08a7c314 100644 --- a/modules/setting/config/value_test.go +++ b/modules/setting/config/value_test.go @@ -33,3 +33,31 @@ func TestValue_parse(t *testing.T) { }) } } + +func TestValue_getKey(t *testing.T) { + tests := []struct { + name string // description of this test case + valueClass *Value[bool] + want string + }{ + { + name: "Custom dynKey name", + valueClass: ValueJSON[bool]("picture.enable_gravatar").SelectFrom("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "", Key: ""}), + want: "picture.enable_gravatar", + }, + { + name: "Normal dynKey name", + valueClass: ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "", Key: ""}), + want: "picture.disable_gravatar", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.valueClass.getKey() + + if got != tt.want { + t.Errorf("getKey() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/routers/common/db.go b/routers/common/db.go index 01c0261427..ccf812f228 100644 --- a/routers/common/db.go +++ b/routers/common/db.go @@ -38,6 +38,7 @@ func InitDBEngine(ctx context.Context) (err error) { log.Info("Backing off for %d seconds", int64(setting.Database.DBConnectBackoff/time.Second)) time.Sleep(setting.Database.DBConnectBackoff) } + config.SetDynSetter(system_model.NewDatabaseDynKeySetter()) config.SetDynGetter(system_model.NewDatabaseDynKeyGetter()) return nil } diff --git a/routers/install/install.go b/routers/install/install.go index d373a07953..e5823c9cdd 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -427,7 +427,8 @@ func SubmitInstall(ctx *context.Context) { cfg.Section("server").Key("OFFLINE_MODE").SetValue(strconv.FormatBool(form.OfflineMode)) if err := system_model.SetSettings(ctx, map[string]string{ - setting.Config().Picture.EnableGravatar.DynKey(): strconv.FormatBool(!form.EnableGravatar), // invert value as it is stored as disable_gravatar for backwards compatability + // Form is submitted on install and should use the SelectFrom key and inverted this enter + setting.Config().Picture.EnableGravatar.SelectFromKey(): strconv.FormatBool(!form.EnableGravatar), setting.Config().Picture.EnableFederatedAvatar.DynKey(): strconv.FormatBool(form.EnableFederatedAvatar), }); err != nil { ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form) diff --git a/routers/web/admin/config.go b/routers/web/admin/config.go index 5e06fdebc2..1e3276e289 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -198,16 +198,15 @@ func ConfigSettings(ctx *context.Context) { func ChangeConfig(ctx *context.Context) { cfg := setting.Config() + subValueSet := map[string]func(string) error{ + cfg.Picture.EnableGravatar.DynKey(): cfg.Picture.EnableGravatar.SetValue, + } + marshalBool := func(v string) ([]byte, error) { b, _ := strconv.ParseBool(v) return json.Marshal(b) } - marshalBoolInvert := func(v string) ([]byte, error) { - b, _ := strconv.ParseBool(v) - return json.Marshal(!b) - } - marshalString := func(emptyDefault string) func(v string) ([]byte, error) { return func(v string) ([]byte, error) { return json.Marshal(util.IfZero(v, emptyDefault)) @@ -237,7 +236,7 @@ func ChangeConfig(ctx *context.Context) { } marshallers := map[string]func(string) ([]byte, error){ - cfg.Picture.EnableGravatar.DynKey(): marshalBoolInvert, // Invert for backwards compatability with old database semantics + cfg.Picture.EnableGravatar.DynKey(): marshalBool, cfg.Picture.EnableFederatedAvatar.DynKey(): marshalBool, cfg.Repository.OpenWithEditorApps.DynKey(): marshalOpenWithApps, cfg.Repository.GitGuideRemoteName.DynKey(): marshalString(cfg.Repository.GitGuideRemoteName.DefaultValue()), @@ -266,7 +265,15 @@ loop: ctx.JSONError(ctx.Tr("admin.config.set_setting_failed", key)) break loop } - configSettings[key] = string(marshaledValue) + + if setter, ok := subValueSet[key]; ok { + if err := setter(string(marshaledValue)); err != nil { + ctx.JSONError(ctx.Tr("admin.config.set_setting_failed", key)) + break loop + } + } else { + configSettings[key] = string(marshaledValue) + } } if ctx.Written() { return diff --git a/templates/admin/config_settings/avatars.tmpl b/templates/admin/config_settings/avatars.tmpl index 31b6d2bc73..afc3d56ea5 100644 --- a/templates/admin/config_settings/avatars.tmpl +++ b/templates/admin/config_settings/avatars.tmpl @@ -6,14 +6,14 @@