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 9c56e0f9a0..dbd0414194 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 43a062cc2a..c757e473fc 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 542bd93b98..5fe67aafa7 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 4c5d2df7d8..5476cd86b6 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 301c60f5e8..0836f217ee 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 0000000000..0697165717 --- /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 fafae45bab..6e4ed4f125 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 03017ce674..ac212dd767 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 4a9dabac6f..d373a07953 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 774b31ab98..5e06fdebc2 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 27577cd35b..54414ba707 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 98995cd69c..2a5729ff33 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 618294d434..77665fd8f9 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 1fc761034d..31b6d2bc73 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 879b5cb550..31eee10984 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 0aec52f27b..e996ad777c 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 d8e5e27b89..bcda75cb2a 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 ca4bcce881..3230ae540a 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 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 @@
{{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 c757e473fc..cdd2483993 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 ff4960a132..d34f671557 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 1d01099428..f00a29850e 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 be08a7c314..b23d967eeb 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 e5823c9cdd..6b43acab18 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 afc3d56ea5..5ff972bea8 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 cdd2483993..f1f74f7680 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 6b43acab18..77ebaee9d1 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 {