From 8c4f0f02ef7ab094cbd71c062e60ebaf71f65a24 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 29 Jan 2025 07:14:35 +0800 Subject: [PATCH] Refactor user package (#33423) and avoid global variables --- models/issues/reaction.go | 8 ++++- models/user/email_address.go | 5 +-- models/user/openid.go | 5 +-- models/user/user.go | 59 ++++++++++++++++++++++-------------- models/user/user_system.go | 31 ++++++++----------- routers/web/user/avatar.go | 3 +- 6 files changed, 59 insertions(+), 52 deletions(-) diff --git a/models/issues/reaction.go b/models/issues/reaction.go index 11b3c6be203..f24001fd231 100644 --- a/models/issues/reaction.go +++ b/models/issues/reaction.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "fmt" + "strings" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -321,6 +322,11 @@ func valuesUser(m map[int64]*user_model.User) []*user_model.User { return values } +// newMigrationOriginalUser creates and returns a fake user for external user +func newMigrationOriginalUser(name string) *user_model.User { + return &user_model.User{ID: 0, Name: name, LowerName: strings.ToLower(name)} +} + // LoadUsers loads reactions' all users func (list ReactionList) LoadUsers(ctx context.Context, repo *repo_model.Repository) ([]*user_model.User, error) { if len(list) == 0 { @@ -338,7 +344,7 @@ func (list ReactionList) LoadUsers(ctx context.Context, repo *repo_model.Reposit for _, reaction := range list { if reaction.OriginalAuthor != "" { - reaction.User = user_model.NewReplaceUser(fmt.Sprintf("%s(%s)", reaction.OriginalAuthor, repo.OriginalServiceType.Name())) + reaction.User = newMigrationOriginalUser(fmt.Sprintf("%s(%s)", reaction.OriginalAuthor, repo.OriginalServiceType.Name())) } else if user, ok := userMaps[reaction.UserID]; ok { reaction.User = user } else { diff --git a/models/user/email_address.go b/models/user/email_address.go index 74ba5f617a5..7c9c6140acf 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -8,7 +8,6 @@ import ( "context" "fmt" "net/mail" - "regexp" "strings" "time" @@ -153,8 +152,6 @@ func UpdateEmailAddress(ctx context.Context, email *EmailAddress) error { return err } -var emailRegexp = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") - // ValidateEmail check if email is a valid & allowed address func ValidateEmail(email string) error { if err := validateEmailBasic(email); err != nil { @@ -514,7 +511,7 @@ func validateEmailBasic(email string) error { return ErrEmailInvalid{email} } - if !emailRegexp.MatchString(email) { + if !globalVars().emailRegexp.MatchString(email) { return ErrEmailCharIsNotSupported{email} } diff --git a/models/user/openid.go b/models/user/openid.go index ee4ecabae0b..420c67ca183 100644 --- a/models/user/openid.go +++ b/models/user/openid.go @@ -11,9 +11,6 @@ import ( "code.gitea.io/gitea/modules/util" ) -// ErrOpenIDNotExist openid is not known -var ErrOpenIDNotExist = util.NewNotExistErrorf("OpenID is unknown") - // UserOpenID is the list of all OpenID identities of a user. // Since this is a middle table, name it OpenID is not suitable, so we ignore the lint here type UserOpenID struct { //revive:disable-line:exported @@ -99,7 +96,7 @@ func DeleteUserOpenID(ctx context.Context, openid *UserOpenID) (err error) { if err != nil { return err } else if deleted != 1 { - return ErrOpenIDNotExist + return util.NewNotExistErrorf("OpenID is unknown") } return nil } diff --git a/models/user/user.go b/models/user/user.go index f790392a9bd..e93ad41ade8 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -14,6 +14,7 @@ import ( "path/filepath" "regexp" "strings" + "sync" "time" "unicode" @@ -417,19 +418,9 @@ func (u *User) DisplayName() string { return u.Name } -var emailToReplacer = strings.NewReplacer( - "\n", "", - "\r", "", - "<", "", - ">", "", - ",", "", - ":", "", - ";", "", -) - // EmailTo returns a string suitable to be put into a e-mail `To:` header. func (u *User) EmailTo() string { - sanitizedDisplayName := emailToReplacer.Replace(u.DisplayName()) + sanitizedDisplayName := globalVars().emailToReplacer.Replace(u.DisplayName()) // should be an edge case but nice to have if sanitizedDisplayName == u.Email { @@ -526,28 +517,52 @@ func GetUserSalt() (string, error) { if err != nil { return "", err } - // Returns a 32 bytes long string. + // Returns a 32-byte long string. return hex.EncodeToString(rBytes), nil } -// Note: The set of characters here can safely expand without a breaking change, -// but characters removed from this set can cause user account linking to break -var ( - customCharsReplacement = strings.NewReplacer("Æ", "AE") - removeCharsRE = regexp.MustCompile("['`´]") - transformDiacritics = transform.Chain(norm.NFD, runes.Remove(runes.In(unicode.Mn)), norm.NFC) - replaceCharsHyphenRE = regexp.MustCompile(`[\s~+]`) -) +type globalVarsStruct struct { + customCharsReplacement *strings.Replacer + removeCharsRE *regexp.Regexp + transformDiacritics transform.Transformer + replaceCharsHyphenRE *regexp.Regexp + emailToReplacer *strings.Replacer + emailRegexp *regexp.Regexp +} + +var globalVars = sync.OnceValue(func() *globalVarsStruct { + return &globalVarsStruct{ + // Note: The set of characters here can safely expand without a breaking change, + // but characters removed from this set can cause user account linking to break + customCharsReplacement: strings.NewReplacer("Æ", "AE"), + + removeCharsRE: regexp.MustCompile("['`´]"), + transformDiacritics: transform.Chain(norm.NFD, runes.Remove(runes.In(unicode.Mn)), norm.NFC), + replaceCharsHyphenRE: regexp.MustCompile(`[\s~+]`), + + emailToReplacer: strings.NewReplacer( + "\n", "", + "\r", "", + "<", "", + ">", "", + ",", "", + ":", "", + ";", "", + ), + emailRegexp: regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$"), + } +}) // NormalizeUserName only takes the name part if it is an email address, transforms it diacritics to ASCII characters. // It returns a string with the single-quotes removed, and any other non-supported username characters are replaced with a `-` character func NormalizeUserName(s string) (string, error) { + vars := globalVars() s, _, _ = strings.Cut(s, "@") - strDiacriticsRemoved, n, err := transform.String(transformDiacritics, customCharsReplacement.Replace(s)) + strDiacriticsRemoved, n, err := transform.String(vars.transformDiacritics, vars.customCharsReplacement.Replace(s)) if err != nil { return "", fmt.Errorf("failed to normalize the string of provided username %q at position %d", s, n) } - return replaceCharsHyphenRE.ReplaceAllLiteralString(removeCharsRE.ReplaceAllLiteralString(strDiacriticsRemoved, ""), "-"), nil + return vars.replaceCharsHyphenRE.ReplaceAllLiteralString(vars.removeCharsRE.ReplaceAllLiteralString(strDiacriticsRemoved, ""), "-"), nil } var ( diff --git a/models/user/user_system.go b/models/user/user_system.go index 612cdb2caef..6e7638377ee 100644 --- a/models/user/user_system.go +++ b/models/user/user_system.go @@ -10,9 +10,8 @@ import ( ) const ( - GhostUserID = -1 - GhostUserName = "Ghost" - GhostUserLowerName = "ghost" + GhostUserID = -1 + GhostUserName = "Ghost" ) // NewGhostUser creates and returns a fake user for someone has deleted their account. @@ -20,10 +19,14 @@ func NewGhostUser() *User { return &User{ ID: GhostUserID, Name: GhostUserName, - LowerName: GhostUserLowerName, + LowerName: strings.ToLower(GhostUserName), } } +func IsGhostUserName(name string) bool { + return strings.EqualFold(name, GhostUserName) +} + // IsGhost check if user is fake user for a deleted account func (u *User) IsGhost() bool { if u == nil { @@ -32,20 +35,10 @@ func (u *User) IsGhost() bool { return u.ID == GhostUserID && u.Name == GhostUserName } -// NewReplaceUser creates and returns a fake user for external user -func NewReplaceUser(name string) *User { - return &User{ - ID: 0, - Name: name, - LowerName: strings.ToLower(name), - } -} - const ( - ActionsUserID = -2 - ActionsUserName = "gitea-actions" - ActionsFullName = "Gitea Actions" - ActionsEmail = "teabot@gitea.io" + ActionsUserID = -2 + ActionsUserName = "gitea-actions" + ActionsUserEmail = "teabot@gitea.io" ) // NewActionsUser creates and returns a fake user for running the actions. @@ -55,8 +48,8 @@ func NewActionsUser() *User { Name: ActionsUserName, LowerName: ActionsUserName, IsActive: true, - FullName: ActionsFullName, - Email: ActionsEmail, + FullName: "Gitea Actions", + Email: ActionsUserEmail, KeepEmailPrivate: true, LoginName: ActionsUserName, Type: UserTypeIndividual, diff --git a/routers/web/user/avatar.go b/routers/web/user/avatar.go index f77bd602b3d..f70b2803770 100644 --- a/routers/web/user/avatar.go +++ b/routers/web/user/avatar.go @@ -4,7 +4,6 @@ package user import ( - "strings" "time" "code.gitea.io/gitea/models/avatars" @@ -27,7 +26,7 @@ func AvatarByUserName(ctx *context.Context) { size := int(ctx.PathParamInt64("size")) var user *user_model.User - if strings.ToLower(userName) != user_model.GhostUserLowerName { + if !user_model.IsGhostUserName(userName) { var err error if user, err = user_model.GetUserByName(ctx, userName); err != nil { if user_model.IsErrUserNotExist(err) {