From f68d49cb9e88a309d4d4f1ed6fc97b0593d3d444 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Mar 2025 20:53:30 -0700 Subject: [PATCH] Move ParseBool to optional (#33979) (cherry picked from commit 25b6f388651c893ecafce918f27bef7e4ae9a967) --- modules/optional/option.go | 11 +++++++++++ modules/optional/option_test.go | 13 +++++++++++++ modules/setting/setting.go | 4 ++-- modules/util/util.go | 11 ----------- modules/util/util_test.go | 14 -------------- routers/web/admin/users.go | 11 +++++------ 6 files changed, 31 insertions(+), 33 deletions(-) diff --git a/modules/optional/option.go b/modules/optional/option.go index af9e5ac852..ccbad259c2 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -3,6 +3,8 @@ package optional +import "strconv" + type Option[T any] []T func None[T any]() Option[T] { @@ -43,3 +45,12 @@ func (o Option[T]) ValueOrDefault(v T) T { } return v } + +// ParseBool get the corresponding optional.Option[bool] of a string using strconv.ParseBool +func ParseBool(s string) Option[bool] { + v, e := strconv.ParseBool(s) + if e != nil { + return None[bool]() + } + return Some(v) +} diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index f6d22d2431..a674caf633 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -57,3 +57,16 @@ func TestOption(t *testing.T) { assert.True(t, opt3.Has()) assert.Equal(t, int(1), opt3.Value()) } + +func Test_ParseBool(t *testing.T) { + assert.Equal(t, optional.None[bool](), optional.ParseBool("")) + assert.Equal(t, optional.None[bool](), optional.ParseBool("x")) + + assert.Equal(t, optional.Some(false), optional.ParseBool("0")) + assert.Equal(t, optional.Some(false), optional.ParseBool("f")) + assert.Equal(t, optional.Some(false), optional.ParseBool("False")) + + assert.Equal(t, optional.Some(true), optional.ParseBool("1")) + assert.Equal(t, optional.Some(true), optional.ParseBool("t")) + assert.Equal(t, optional.Some(true), optional.ParseBool("True")) +} diff --git a/modules/setting/setting.go b/modules/setting/setting.go index eb7b9e9373..80db4dfa3c 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -12,8 +12,8 @@ import ( "time" "forgejo.org/modules/log" + "forgejo.org/modules/optional" "forgejo.org/modules/user" - "forgejo.org/modules/util" ) var ForgejoVersion = "1.0.0" @@ -162,7 +162,7 @@ func loadRunModeFrom(rootCfg ConfigProvider) { // The following is a purposefully undocumented option. Please do not run Forgejo as root. It will only cause future headaches. // Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly. unsafeAllowRunAsRoot := ConfigSectionKeyBool(rootSec, "I_AM_BEING_UNSAFE_RUNNING_AS_ROOT") - unsafeAllowRunAsRoot = unsafeAllowRunAsRoot || util.OptionalBoolParse(os.Getenv("GITEA_I_AM_BEING_UNSAFE_RUNNING_AS_ROOT")).Value() + unsafeAllowRunAsRoot = unsafeAllowRunAsRoot || optional.ParseBool(os.Getenv("GITEA_I_AM_BEING_UNSAFE_RUNNING_AS_ROOT")).Value() RunMode = os.Getenv("GITEA_RUN_MODE") if RunMode == "" { RunMode = rootSec.Key("RUN_MODE").MustString("prod") diff --git a/modules/util/util.go b/modules/util/util.go index da405c9c4b..fc3fcc35f0 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -14,22 +14,11 @@ import ( "strconv" "strings" - "forgejo.org/modules/optional" - "golang.org/x/crypto/ssh" "golang.org/x/text/cases" "golang.org/x/text/language" ) -// OptionalBoolParse get the corresponding optional.Option[bool] of a string using strconv.ParseBool -func OptionalBoolParse(s string) optional.Option[bool] { - v, e := strconv.ParseBool(s) - if e != nil { - return optional.None[bool]() - } - return optional.Some(v) -} - // IsEmptyString checks if the provided string is empty func IsEmptyString(s string) bool { return len(strings.TrimSpace(s)) == 0 diff --git a/modules/util/util_test.go b/modules/util/util_test.go index d2fef1a035..61f98de88a 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -11,7 +11,6 @@ import ( "strings" "testing" - "forgejo.org/modules/optional" "forgejo.org/modules/test" "forgejo.org/modules/util" @@ -181,19 +180,6 @@ func Test_RandomBytes(t *testing.T) { assert.NotEqual(t, bytes3, bytes4) } -func TestOptionalBoolParse(t *testing.T) { - assert.Equal(t, optional.None[bool](), util.OptionalBoolParse("")) - assert.Equal(t, optional.None[bool](), util.OptionalBoolParse("x")) - - assert.Equal(t, optional.Some(false), util.OptionalBoolParse("0")) - assert.Equal(t, optional.Some(false), util.OptionalBoolParse("f")) - assert.Equal(t, optional.Some(false), util.OptionalBoolParse("False")) - - assert.Equal(t, optional.Some(true), util.OptionalBoolParse("1")) - assert.Equal(t, optional.Some(true), util.OptionalBoolParse("t")) - assert.Equal(t, optional.Some(true), util.OptionalBoolParse("True")) -} - // Test case for any function which accepts and returns a single string. type StringTest struct { in, out string diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index f53a0197cb..cefdfadc53 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -22,7 +22,6 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/setting" - "forgejo.org/modules/util" "forgejo.org/modules/validation" "forgejo.org/modules/web" "forgejo.org/routers/web/explore" @@ -77,11 +76,11 @@ func Users(ctx *context.Context) { PageSize: setting.UI.Admin.UserPagingNum, }, SearchByEmail: true, - IsActive: util.OptionalBoolParse(statusFilterMap["is_active"]), - IsAdmin: util.OptionalBoolParse(statusFilterMap["is_admin"]), - IsRestricted: util.OptionalBoolParse(statusFilterMap["is_restricted"]), - IsTwoFactorEnabled: util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]), - IsProhibitLogin: util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]), + IsActive: optional.ParseBool(statusFilterMap["is_active"]), + IsAdmin: optional.ParseBool(statusFilterMap["is_admin"]), + IsRestricted: optional.ParseBool(statusFilterMap["is_restricted"]), + IsTwoFactorEnabled: optional.ParseBool(statusFilterMap["is_2fa_enabled"]), + IsProhibitLogin: optional.ParseBool(statusFilterMap["is_prohibit_login"]), IncludeReserved: true, // administrator needs to list all accounts include reserved, bot, remote ones Load2FAStatus: true, ExtraParamStrings: extraParamStrings,