From 589712db2695b7c4522f9009fbb000e5851d8c54 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Sat, 11 Oct 2025 23:12:13 +0000 Subject: [PATCH 1/6] feat: inverted disable_gravatar logic to enable_gravatar Frontend still interacts directly with the database entry name `picture.disable_gravatar` so logic needs flipped when writing, but logic to read automatically flips based on config.Invert() being called during init or INI read. --- models/avatars/avatar.go | 4 +-- models/avatars/avatar_test.go | 6 ++-- models/user/avatar.go | 6 ++-- modules/setting/config.go | 4 +-- modules/setting/config/value.go | 21 ++++++++++-- modules/setting/config/value_test.go | 35 ++++++++++++++++++++ modules/setting/picture.go | 14 ++++---- options/locale/locale_en-US.ini | 6 ++-- routers/install/install.go | 4 +-- routers/web/admin/config.go | 8 ++++- routers/web/admin/users.go | 2 +- routers/web/user/setting/profile.go | 4 +-- services/forms/user_form.go | 2 +- templates/admin/config_settings/avatars.tmpl | 6 ++-- templates/admin/user/edit.tmpl | 2 +- templates/install.tmpl | 6 ++-- templates/user/settings/profile.tmpl | 2 +- web_src/js/features/install.ts | 10 +++--- 18 files changed, 100 insertions(+), 42 deletions(-) create mode 100644 modules/setting/config/value_test.go diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go index 9c56e0f9a0fba..dbd0414194774 100644 --- a/models/avatars/avatar.go +++ b/models/avatars/avatar.go @@ -216,8 +216,8 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final return urlStr } - disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx) - if !disableGravatar { + enableGravatar := setting.Config().Picture.EnableGravatar.Value(ctx) + if enableGravatar { // copy GravatarSourceURL, because we will modify its Path. avatarURLCopy := *avatarSetting.gravatarSourceURL avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email)) diff --git a/models/avatars/avatar_test.go b/models/avatars/avatar_test.go index 43a062cc2a117..c757e473fc6cd 100644 --- a/models/avatars/avatar_test.go +++ b/models/avatars/avatar_test.go @@ -19,12 +19,14 @@ const gravatarSource = "https://secure.gravatar.com/avatar/" func disableGravatar(t *testing.T) { err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"}) assert.NoError(t, err) - err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "true"}) + // EnableGravatar.DynKey == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the true value here is misleading + err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.DynKey(): "true"}) assert.NoError(t, err) } func enableGravatar(t *testing.T) { - err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "false"}) + // EnableGravatar.DynKey == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the false value here is misleading + err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.DynKey(): "false"}) assert.NoError(t, err) setting.GravatarSource = gravatarSource } diff --git a/models/user/avatar.go b/models/user/avatar.go index 542bd93b982ce..5fe67aafa7291 100644 --- a/models/user/avatar.go +++ b/models/user/avatar.go @@ -69,12 +69,12 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string { useLocalAvatar := false autoGenerateAvatar := false - disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx) + enableGravatar := setting.Config().Picture.EnableGravatar.Value(ctx) switch { - case u.UseCustomAvatar: + case u.UseCustomAvatar, enableGravatar: useLocalAvatar = true - case disableGravatar, setting.OfflineMode: + case setting.OfflineMode: useLocalAvatar = true autoGenerateAvatar = true } diff --git a/modules/setting/config.go b/modules/setting/config.go index 4c5d2df7d8a01..5476cd86b64bb 100644 --- a/modules/setting/config.go +++ b/modules/setting/config.go @@ -11,7 +11,7 @@ import ( ) type PictureStruct struct { - DisableGravatar *config.Value[bool] + EnableGravatar *config.Value[bool] EnableFederatedAvatar *config.Value[bool] } @@ -66,7 +66,7 @@ func initDefaultConfig() { config.SetCfgSecKeyGetter(&cfgSecKeyGetter{}) defaultConfig = &ConfigStruct{ Picture: &PictureStruct{ - DisableGravatar: config.ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(config.CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}), + EnableGravatar: config.ValueJSON[bool]("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{ diff --git a/modules/setting/config/value.go b/modules/setting/config/value.go index 301c60f5e8250..0836f217ee903 100644 --- a/modules/setting/config/value.go +++ b/modules/setting/config/value.go @@ -22,8 +22,9 @@ type Value[T any] struct { cfgSecKey CfgSecKey dynKey string - def, value T - revision int + def, value T + revision int + flipBoolean bool } func (value *Value[T]) parse(key, valStr string) (v T) { @@ -33,6 +34,17 @@ func (value *Value[T]) parse(key, valStr string) (v T) { log.Error("Unable to unmarshal json config for key %q, err: %v", key, err) } } + + if value.flipBoolean { + // if value is of type bool + if _, ok := any(v).(bool); ok { + // invert the boolean value upon retrieval + v = any(!any(v).(bool)).(T) + } else { + log.Warn("Ignoring attempt to invert key '%q' for non boolean type", key) + } + } + return v } @@ -93,6 +105,11 @@ func (value *Value[T]) WithFileConfig(cfgSecKey CfgSecKey) *Value[T] { return value } +func (value *Value[bool]) Invert() *Value[bool] { + value.flipBoolean = true + return value +} + 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 new file mode 100644 index 0000000000000..0697165717f3b --- /dev/null +++ b/modules/setting/config/value_test.go @@ -0,0 +1,35 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package config + +import ( + "testing" +) + +func TestValue_parse(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + key string + valStr string + want bool + }{ + { + name: "Parse Invert Retrieval", + key: "picture.disable_gravatar", + valStr: "false", + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + value := ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert() + got := value.parse(tt.key, tt.valStr) + + if got != tt.want { + t.Errorf("parse() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/modules/setting/picture.go b/modules/setting/picture.go index fafae45babafe..6e4ed4f125436 100644 --- a/modules/setting/picture.go +++ b/modules/setting/picture.go @@ -23,7 +23,7 @@ var ( } GravatarSource string - DisableGravatar bool // Depreciated: migrated to database + EnableGravatar bool // Depreciated: migrated to database EnableFederatedAvatar bool // Depreciated: migrated to database RepoAvatar = struct { @@ -65,9 +65,9 @@ func loadAvatarsFrom(rootCfg ConfigProvider) error { GravatarSource = source } - DisableGravatar = sec.Key("DISABLE_GRAVATAR").MustBool(GetDefaultDisableGravatar()) + EnableGravatar = !sec.Key("DISABLE_GRAVATAR").MustBool(GetDefaultDisableGravatar()) deprecatedSettingDB(rootCfg, "", "DISABLE_GRAVATAR") - EnableFederatedAvatar = sec.Key("ENABLE_FEDERATED_AVATAR").MustBool(GetDefaultEnableFederatedAvatar(DisableGravatar)) + EnableFederatedAvatar = sec.Key("ENABLE_FEDERATED_AVATAR").MustBool(GetDefaultEnableFederatedAvatar(EnableGravatar)) deprecatedSettingDB(rootCfg, "", "ENABLE_FEDERATED_AVATAR") return nil @@ -77,14 +77,12 @@ func GetDefaultDisableGravatar() bool { return OfflineMode } -func GetDefaultEnableFederatedAvatar(disableGravatar bool) bool { +func GetDefaultEnableFederatedAvatar(enableGravatar bool) bool { v := !InstallLock - if OfflineMode { - v = false - } - if disableGravatar { + if OfflineMode || !enableGravatar { v = false } + return v } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 03017ce6746df..ac212dd767ea7 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -316,8 +316,8 @@ mail_notify = Enable Email Notifications server_service_title = Server and Third-Party Service Settings offline_mode = Enable Local Mode offline_mode_popup = Disable third-party content delivery networks and serve all resources locally. -disable_gravatar = Disable Gravatar -disable_gravatar_popup = Disable Gravatar and third-party avatar sources. A default avatar will be used unless a user locally uploads an avatar. +enable_gravatar = Enable Gravatar +enable_gravatar_popup = Enable Gravatar and third-party avatar sources. A default avatar will be used unless a user locally uploads an avatar. federated_avatar_lookup = Enable Federated Avatars federated_avatar_lookup_popup = Enable federated avatar lookup using Libravatar. disable_registration = Disable Self-Registration @@ -3433,7 +3433,7 @@ config.cookie_life_time = Cookie Life Time config.picture_config = Picture and Avatar Configuration config.picture_service = Picture Service -config.disable_gravatar = Disable Gravatar +config.enable_gravatar = Enable Gravatar config.enable_federated_avatar = Enable Federated Avatars config.open_with_editor_app_help = The "Open with" editors for the clone menu. If left empty, the default will be used. Expand to see the default. config.git_guide_remote_name = Repository remote name for git commands in the guide diff --git a/routers/install/install.go b/routers/install/install.go index 4a9dabac6fe87..d373a07953230 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -136,7 +136,7 @@ func Install(ctx *context.Context) { // Server and other services settings form.OfflineMode = setting.OfflineMode - form.DisableGravatar = setting.DisableGravatar // when installing, there is no database connection so that given a default value + form.EnableGravatar = setting.EnableGravatar // when installing, there is no database connection so that given a default value form.EnableFederatedAvatar = setting.EnableFederatedAvatar // when installing, there is no database connection so that given a default value form.EnableOpenIDSignIn = setting.Service.EnableOpenIDSignIn @@ -427,7 +427,7 @@ 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.DisableGravatar.DynKey(): strconv.FormatBool(form.DisableGravatar), + setting.Config().Picture.EnableGravatar.DynKey(): strconv.FormatBool(!form.EnableGravatar), // invert value as it is stored as disable_gravatar for backwards compatability 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 774b31ab9842a..5e06fdebc21ef 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -203,6 +203,11 @@ func ChangeConfig(ctx *context.Context) { 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)) @@ -230,8 +235,9 @@ func ChangeConfig(ctx *context.Context) { } return json.Marshal(openWithEditorApps) } + marshallers := map[string]func(string) ([]byte, error){ - cfg.Picture.DisableGravatar.DynKey(): marshalBool, + cfg.Picture.EnableGravatar.DynKey(): marshalBoolInvert, // Invert for backwards compatability with old database semantics cfg.Picture.EnableFederatedAvatar.DynKey(): marshalBool, cfg.Repository.OpenWithEditorApps.DynKey(): marshalOpenWithApps, cfg.Repository.GitGuideRemoteName.DynKey(): marshalString(cfg.Repository.GitGuideRemoteName.DefaultValue()), diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 27577cd35ba1c..54414ba707d30 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -316,7 +316,7 @@ func editUserCommon(ctx *context.Context) { ctx.Data["DisableGitHooks"] = setting.DisableGitHooks ctx.Data["DisableImportLocal"] = !setting.ImportLocalPaths ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() - ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx) + ctx.Data["EnableGravatar"] = setting.Config().Picture.EnableGravatar.Value(ctx) } // EditUser show editing user page diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 98995cd69c435..2a5729ff33580 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -47,7 +47,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"] = setting.Config().Picture.DisableGravatar.Value(ctx) + ctx.Data["EnableGravatar"] = setting.Config().Picture.EnableGravatar.Value(ctx) ctx.Data["UserDisabledFeatures"] = user_model.DisabledFeaturesWithLoginType(ctx.Doer) @@ -59,7 +59,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"] = setting.Config().Picture.DisableGravatar.Value(ctx) + ctx.Data["EnableGravatar"] = setting.Config().Picture.EnableGravatar.Value(ctx) ctx.Data["UserDisabledFeatures"] = user_model.DisabledFeaturesWithLoginType(ctx.Doer) if ctx.HasError() { diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 618294d43412f..77665fd8f9de8 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -46,7 +46,7 @@ type InstallForm struct { MailNotify bool OfflineMode bool - DisableGravatar bool + EnableGravatar bool EnableFederatedAvatar bool EnableOpenIDSignIn bool EnableOpenIDSignUp bool diff --git a/templates/admin/config_settings/avatars.tmpl b/templates/admin/config_settings/avatars.tmpl index 1fc761034d402..31b6d2bc738db 100644 --- a/templates/admin/config_settings/avatars.tmpl +++ b/templates/admin/config_settings/avatars.tmpl @@ -3,10 +3,10 @@
-
{{ctx.Locale.Tr "admin.config.disable_gravatar"}}
+
{{ctx.Locale.Tr "admin.config.enable_gravatar"}}
-
- +
+
diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl index 879b5cb550d30..31eee10984efe 100644 --- a/templates/admin/user/edit.tmpl +++ b/templates/admin/user/edit.tmpl @@ -174,7 +174,7 @@
{{.CsrfTokenHtml}} - {{if not .DisableGravatar}} + {{if .EnableGravatar}}
diff --git a/templates/install.tmpl b/templates/install.tmpl index 0aec52f27baa9..e996ad777c7fb 100644 --- a/templates/install.tmpl +++ b/templates/install.tmpl @@ -210,9 +210,9 @@
-
- - +
+ +
diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index d8e5e27b896a6..bcda75cb2ac59 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -103,7 +103,7 @@
{{.CsrfTokenHtml}} - {{if not .DisableGravatar}} + {{if .EnableGravatar}}
diff --git a/web_src/js/features/install.ts b/web_src/js/features/install.ts index ca4bcce8819d0..3230ae540a273 100644 --- a/web_src/js/features/install.ts +++ b/web_src/js/features/install.ts @@ -62,20 +62,20 @@ function initPreInstall() { // TODO: better handling of exclusive relations. document.querySelector('#offline-mode input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#disable-gravatar input').checked = true; + document.querySelector('#enable-gravatar input').checked = false; document.querySelector('#federated-avatar-lookup input').checked = false; } }); - document.querySelector('#disable-gravatar input').addEventListener('change', function () { + document.querySelector('#enable-gravatar input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#federated-avatar-lookup input').checked = false; + document.querySelector('#federated-avatar-lookup input').checked = true; } else { - document.querySelector('#offline-mode input').checked = false; + document.querySelector('#offline-mode input').checked = true; } }); document.querySelector('#federated-avatar-lookup input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#disable-gravatar input').checked = false; + document.querySelector('#enable-gravatar input').checked = false; document.querySelector('#offline-mode input').checked = false; } }); From bc430bb330493dacfecd0416435c1ddf4f96f3c5 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Wed, 15 Oct 2025 21:22:08 -0400 Subject: [PATCH 2/6] feat: adds setter for config.Value and updates forms Install now submits the proper database name and is properly set using the config.Value class. This extends the getter functionality so now config.Value can be used to both get and set values. --- models/system/setting.go | 31 ++++++++++ modules/setting/config.go | 8 +-- modules/setting/config/setter.go | 29 +++++++++ modules/setting/config/value.go | 62 ++++++++++++++++++-- modules/setting/config/value_test.go | 28 +++++++++ routers/common/db.go | 1 + routers/install/install.go | 3 +- routers/web/admin/config.go | 21 ++++--- templates/admin/config_settings/avatars.tmpl | 4 +- 9 files changed, 167 insertions(+), 20 deletions(-) create mode 100644 modules/setting/config/setter.go diff --git a/models/system/setting.go b/models/system/setting.go index 4472b4c22862b..cf6b2dea1acae 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 5476cd86b64bb..ff4960a1329cd 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 0000000000000..f8e63971c241b --- /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 0836f217ee903..1d01099428ec7 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 0697165717f3b..be08a7c314e81 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 01c026142790d..ccf812f228ba0 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 d373a07953230..e5823c9cddc86 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 5e06fdebc21ef..1e3276e289cf2 100644 --- a/routers/web/admin/config.go +++ b/routers/web/admin/config.go @@ -198,14 +198,13 @@ func ConfigSettings(ctx *context.Context) { func ChangeConfig(ctx *context.Context) { cfg := setting.Config() - marshalBool := func(v string) ([]byte, error) { - b, _ := strconv.ParseBool(v) - return json.Marshal(b) + subValueSet := map[string]func(string) error{ + cfg.Picture.EnableGravatar.DynKey(): cfg.Picture.EnableGravatar.SetValue, } - marshalBoolInvert := func(v string) ([]byte, error) { + marshalBool := func(v string) ([]byte, error) { b, _ := strconv.ParseBool(v) - return json.Marshal(!b) + return json.Marshal(b) } marshalString := func(emptyDefault string) func(v string) ([]byte, error) { @@ -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 31b6d2bc738db..afc3d56ea518f 100644 --- a/templates/admin/config_settings/avatars.tmpl +++ b/templates/admin/config_settings/avatars.tmpl @@ -6,14 +6,14 @@
{{ctx.Locale.Tr "admin.config.enable_gravatar"}}
- +
{{ctx.Locale.Tr "admin.config.enable_federated_avatar"}}
- +
From 734daa96846dff86bdb989a14b04f24da5f355c8 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 16 Oct 2025 20:57:13 -0400 Subject: [PATCH 3/6] feat: adds unit tests and remove prints --- models/avatars/avatar_test.go | 8 +-- modules/setting/config.go | 6 +- modules/setting/config/value.go | 4 -- modules/setting/config/value_test.go | 97 ++++++++++++++++++++++++++-- 4 files changed, 100 insertions(+), 15 deletions(-) diff --git a/models/avatars/avatar_test.go b/models/avatars/avatar_test.go index c757e473fc6cd..cdd2483993d3c 100644 --- a/models/avatars/avatar_test.go +++ b/models/avatars/avatar_test.go @@ -19,14 +19,14 @@ const gravatarSource = "https://secure.gravatar.com/avatar/" func disableGravatar(t *testing.T) { err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"}) assert.NoError(t, err) - // EnableGravatar.DynKey == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the true value here is misleading - err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.DynKey(): "true"}) + // EnableGravatar.SelectFrom == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the true value here is counterintuitive + err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.SelectFromKey(): "true"}) assert.NoError(t, err) } func enableGravatar(t *testing.T) { - // EnableGravatar.DynKey == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the false value here is misleading - err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.DynKey(): "false"}) + // EnableGravatar.SelectFrom == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the false value here is counterintuitive + err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.SelectFromKey(): "false"}) assert.NoError(t, err) setting.GravatarSource = gravatarSource } diff --git a/modules/setting/config.go b/modules/setting/config.go index ff4960a1329cd..d34f671557777 100644 --- a/modules/setting/config.go +++ b/modules/setting/config.go @@ -58,8 +58,8 @@ type ConfigStruct struct { } var ( - defaultConfig *ConfigStruct - ConfigOnce sync.Once + defaultConfig *ConfigStruct + defaultConfigOnce sync.Once ) func initDefaultConfig() { @@ -77,7 +77,7 @@ func initDefaultConfig() { } func Config() *ConfigStruct { - ConfigOnce.Do(initDefaultConfig) + defaultConfigOnce.Do(initDefaultConfig) return defaultConfig } diff --git a/modules/setting/config/value.go b/modules/setting/config/value.go index 1d01099428ec7..f00a29850e35e 100644 --- a/modules/setting/config/value.go +++ b/modules/setting/config/value.go @@ -5,7 +5,6 @@ package config import ( "context" - "fmt" "sync" "code.gitea.io/gitea/modules/json" @@ -50,7 +49,6 @@ func (value *Value[T]) invertBoolStr(val string) (inverted string) { 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(val).(bool); ok { // invert the boolean value upon retrieval @@ -151,8 +149,6 @@ func (value *Value[any]) SetValue(val string) error { 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)) } diff --git a/modules/setting/config/value_test.go b/modules/setting/config/value_test.go index be08a7c314e81..b23d967eebb6e 100644 --- a/modules/setting/config/value_test.go +++ b/modules/setting/config/value_test.go @@ -24,7 +24,7 @@ func TestValue_parse(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - value := ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "picture", Key: "DISABLE_GRAVATAR"}).Invert() + value := ValueJSON[bool]("picture.disable_gravatar").Invert() got := value.parse(tt.key, tt.valStr) if got != tt.want { @@ -42,12 +42,12 @@ func TestValue_getKey(t *testing.T) { }{ { name: "Custom dynKey name", - valueClass: ValueJSON[bool]("picture.enable_gravatar").SelectFrom("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "", Key: ""}), - want: "picture.enable_gravatar", + valueClass: ValueJSON[bool]("picture.enable_gravatar").SelectFrom("picture.disable_gravatar"), + want: "picture.disable_gravatar", }, { name: "Normal dynKey name", - valueClass: ValueJSON[bool]("picture.disable_gravatar").WithFileConfig(CfgSecKey{Sec: "", Key: ""}), + valueClass: ValueJSON[bool]("picture.disable_gravatar"), want: "picture.disable_gravatar", }, } @@ -61,3 +61,92 @@ func TestValue_getKey(t *testing.T) { }) } } + +func TestValue_invert(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + valueClass *Value[bool] + want bool + }{ + { + name: "Invert typed true", + valueClass: ValueJSON[bool]("picture.enable_gravatar").WithDefault(true).Invert(), + want: false, + }, + { + name: "Invert typed false", + valueClass: ValueJSON[bool]("picture.enable_gravatar").WithDefault(false).Invert(), + want: true, + }, + { + name: "Invert typed Does not invert", + valueClass: ValueJSON[bool]("picture.enable_gravatar").WithDefault(false), + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.valueClass.invert(tt.valueClass.def) + + if got != tt.want { + t.Errorf("invert() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestValue_invertBoolStr(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + valueClass *Value[bool] + val string + want string + }{ + { + name: "Invert boolean string true", + valueClass: ValueJSON[bool]("picture.enable_gravatar"), + val: "true", + want: "false", + }, + { + name: "Invert boolean string false", + valueClass: ValueJSON[bool]("picture.enable_gravatar"), + val: "false", + want: "true", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.valueClass.invertBoolStr(tt.val) + if got != tt.want { + t.Errorf("invertBoolStr() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestValue_SelectFromKey(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + valueClass *Value[bool] + want string + }{ + { + name: "SelectFrom set and get", + valueClass: ValueJSON[bool]("picture.enable_gravatar").SelectFrom("picture.disable_gravatar"), + want: "picture.disable_gravatar", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.valueClass.SelectFromKey() + + if got != tt.want { + t.Errorf("SelectFromKey() = %v, want %v", got, tt.want) + } + }) + } +} From 8a9b0e6421c6ebb60891e154463d24de8685c94d Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 16 Oct 2025 21:08:36 -0400 Subject: [PATCH 4/6] chore: fixed comment to be more clear --- routers/install/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/install/install.go b/routers/install/install.go index e5823c9cddc86..6b43acab18ab0 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -427,7 +427,7 @@ 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{ - // Form is submitted on install and should use the SelectFrom key and inverted this enter + // Form is submitted on install and should use the SelectFrom key for backwards compatability; getting the value will properly invert the boolean setting.Config().Picture.EnableGravatar.SelectFromKey(): strconv.FormatBool(!form.EnableGravatar), setting.Config().Picture.EnableFederatedAvatar.DynKey(): strconv.FormatBool(form.EnableFederatedAvatar), }); err != nil { From 524dd741d438eb22638885ec1f06578ed2b47c90 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 16 Oct 2025 21:11:36 -0400 Subject: [PATCH 5/6] fix: accidental federate avatar enable swap Accidental swap of enable_federated_avatar to disable in avatars.tmpl --- templates/admin/config_settings/avatars.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/admin/config_settings/avatars.tmpl b/templates/admin/config_settings/avatars.tmpl index afc3d56ea518f..5ff972bea8cfa 100644 --- a/templates/admin/config_settings/avatars.tmpl +++ b/templates/admin/config_settings/avatars.tmpl @@ -13,7 +13,7 @@
{{ctx.Locale.Tr "admin.config.enable_federated_avatar"}}
- +
From 43320701efb57c692d2f64761065adccdf06f540 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Thu, 16 Oct 2025 21:34:37 -0400 Subject: [PATCH 6/6] chore: addressed spelling errors --- models/avatars/avatar_test.go | 4 ++-- routers/install/install.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/models/avatars/avatar_test.go b/models/avatars/avatar_test.go index cdd2483993d3c..f1f74f768000f 100644 --- a/models/avatars/avatar_test.go +++ b/models/avatars/avatar_test.go @@ -19,13 +19,13 @@ const gravatarSource = "https://secure.gravatar.com/avatar/" func disableGravatar(t *testing.T) { err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"}) assert.NoError(t, err) - // EnableGravatar.SelectFrom == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the true value here is counterintuitive + // EnableGravatar.SelectFrom == picture.disable_gravatar for backwards compatibility; .Value will flip correctly but the true value here is counterintuitive err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.SelectFromKey(): "true"}) assert.NoError(t, err) } func enableGravatar(t *testing.T) { - // EnableGravatar.SelectFrom == picture.disable_gravatar for backwards compatability; .Value will flip correctly but the false value here is counterintuitive + // EnableGravatar.SelectFrom == picture.disable_gravatar for backwards compatibility; .Value will flip correctly but the false value here is counterintuitive err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableGravatar.SelectFromKey(): "false"}) assert.NoError(t, err) setting.GravatarSource = gravatarSource diff --git a/routers/install/install.go b/routers/install/install.go index 6b43acab18ab0..77ebaee9d1293 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -427,7 +427,7 @@ 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{ - // Form is submitted on install and should use the SelectFrom key for backwards compatability; getting the value will properly invert the boolean + // Form is submitted on install and should use the SelectFrom key for backwards compatibility; getting the value will properly invert the boolean setting.Config().Picture.EnableGravatar.SelectFromKey(): strconv.FormatBool(!form.EnableGravatar), setting.Config().Picture.EnableFederatedAvatar.DynKey(): strconv.FormatBool(form.EnableFederatedAvatar), }); err != nil {