Skip to content

Commit 8c4f0f0

Browse files
wxiaoguangwxiaoguang
authored
Refactor user package (#33423)
and avoid global variables
1 parent a9577e0 commit 8c4f0f0

File tree

6 files changed

+59
-52
lines changed

6 files changed

+59
-52
lines changed

‎models/issues/reaction.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"bytes"
88
"context"
99
"fmt"
10+
"strings"
1011

1112
"code.gitea.io/gitea/models/db"
1213
repo_model "code.gitea.io/gitea/models/repo"
@@ -321,6 +322,11 @@ func valuesUser(m map[int64]*user_model.User) []*user_model.User {
321322
return values
322323
}
323324

325+
// newMigrationOriginalUser creates and returns a fake user for external user
326+
func newMigrationOriginalUser(name string) *user_model.User {
327+
return &user_model.User{ID: 0, Name: name, LowerName: strings.ToLower(name)}
328+
}
329+
324330
// LoadUsers loads reactions' all users
325331
func (list ReactionList) LoadUsers(ctx context.Context, repo *repo_model.Repository) ([]*user_model.User, error) {
326332
if len(list) == 0 {
@@ -338,7 +344,7 @@ func (list ReactionList) LoadUsers(ctx context.Context, repo *repo_model.Reposit
338344

339345
for _, reaction := range list {
340346
if reaction.OriginalAuthor != "" {
341-
reaction.User = user_model.NewReplaceUser(fmt.Sprintf("%s(%s)", reaction.OriginalAuthor, repo.OriginalServiceType.Name()))
347+
reaction.User = newMigrationOriginalUser(fmt.Sprintf("%s(%s)", reaction.OriginalAuthor, repo.OriginalServiceType.Name()))
342348
} else if user, ok := userMaps[reaction.UserID]; ok {
343349
reaction.User = user
344350
} else {

‎models/user/email_address.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"context"
99
"fmt"
1010
"net/mail"
11-
"regexp"
1211
"strings"
1312
"time"
1413

@@ -153,8 +152,6 @@ func UpdateEmailAddress(ctx context.Context, email *EmailAddress) error {
153152
return err
154153
}
155154

156-
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])?)*$")
157-
158155
// ValidateEmail check if email is a valid & allowed address
159156
func ValidateEmail(email string) error {
160157
if err := validateEmailBasic(email); err != nil {
@@ -514,7 +511,7 @@ func validateEmailBasic(email string) error {
514511
return ErrEmailInvalid{email}
515512
}
516513

517-
if !emailRegexp.MatchString(email) {
514+
if !globalVars().emailRegexp.MatchString(email) {
518515
return ErrEmailCharIsNotSupported{email}
519516
}
520517

‎models/user/openid.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ import (
1111
"code.gitea.io/gitea/modules/util"
1212
)
1313

14-
// ErrOpenIDNotExist openid is not known
15-
var ErrOpenIDNotExist = util.NewNotExistErrorf("OpenID is unknown")
16-
1714
// UserOpenID is the list of all OpenID identities of a user.
1815
// Since this is a middle table, name it OpenID is not suitable, so we ignore the lint here
1916
type UserOpenID struct { //revive:disable-line:exported
@@ -99,7 +96,7 @@ func DeleteUserOpenID(ctx context.Context, openid *UserOpenID) (err error) {
9996
if err != nil {
10097
return err
10198
} else if deleted != 1 {
102-
return ErrOpenIDNotExist
99+
return util.NewNotExistErrorf("OpenID is unknown")
103100
}
104101
return nil
105102
}

‎models/user/user.go

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"path/filepath"
1515
"regexp"
1616
"strings"
17+
"sync"
1718
"time"
1819
"unicode"
1920

@@ -417,19 +418,9 @@ func (u *User) DisplayName() string {
417418
return u.Name
418419
}
419420

420-
var emailToReplacer = strings.NewReplacer(
421-
"\n", "",
422-
"\r", "",
423-
"<", "",
424-
">", "",
425-
",", "",
426-
":", "",
427-
";", "",
428-
)
429-
430421
// EmailTo returns a string suitable to be put into a e-mail `To:` header.
431422
func (u *User) EmailTo() string {
432-
sanitizedDisplayName := emailToReplacer.Replace(u.DisplayName())
423+
sanitizedDisplayName := globalVars().emailToReplacer.Replace(u.DisplayName())
433424

434425
// should be an edge case but nice to have
435426
if sanitizedDisplayName == u.Email {
@@ -526,28 +517,52 @@ func GetUserSalt() (string, error) {
526517
if err != nil {
527518
return "", err
528519
}
529-
// Returns a 32 bytes long string.
520+
// Returns a 32-byte long string.
530521
return hex.EncodeToString(rBytes), nil
531522
}
532523

533-
// Note: The set of characters here can safely expand without a breaking change,
534-
// but characters removed from this set can cause user account linking to break
535-
var (
536-
customCharsReplacement = strings.NewReplacer("Æ", "AE")
537-
removeCharsRE = regexp.MustCompile("['`´]")
538-
transformDiacritics = transform.Chain(norm.NFD, runes.Remove(runes.In(unicode.Mn)), norm.NFC)
539-
replaceCharsHyphenRE = regexp.MustCompile(`[\s~+]`)
540-
)
524+
type globalVarsStruct struct {
525+
customCharsReplacement *strings.Replacer
526+
removeCharsRE *regexp.Regexp
527+
transformDiacritics transform.Transformer
528+
replaceCharsHyphenRE *regexp.Regexp
529+
emailToReplacer *strings.Replacer
530+
emailRegexp *regexp.Regexp
531+
}
532+
533+
var globalVars = sync.OnceValue(func() *globalVarsStruct {
534+
return &globalVarsStruct{
535+
// Note: The set of characters here can safely expand without a breaking change,
536+
// but characters removed from this set can cause user account linking to break
537+
customCharsReplacement: strings.NewReplacer("Æ", "AE"),
538+
539+
removeCharsRE: regexp.MustCompile("['`´]"),
540+
transformDiacritics: transform.Chain(norm.NFD, runes.Remove(runes.In(unicode.Mn)), norm.NFC),
541+
replaceCharsHyphenRE: regexp.MustCompile(`[\s~+]`),
542+
543+
emailToReplacer: strings.NewReplacer(
544+
"\n", "",
545+
"\r", "",
546+
"<", "",
547+
">", "",
548+
",", "",
549+
":", "",
550+
";", "",
551+
),
552+
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])?)*$"),
553+
}
554+
})
541555

542556
// NormalizeUserName only takes the name part if it is an email address, transforms it diacritics to ASCII characters.
543557
// It returns a string with the single-quotes removed, and any other non-supported username characters are replaced with a `-` character
544558
func NormalizeUserName(s string) (string, error) {
559+
vars := globalVars()
545560
s, _, _ = strings.Cut(s, "@")
546-
strDiacriticsRemoved, n, err := transform.String(transformDiacritics, customCharsReplacement.Replace(s))
561+
strDiacriticsRemoved, n, err := transform.String(vars.transformDiacritics, vars.customCharsReplacement.Replace(s))
547562
if err != nil {
548563
return "", fmt.Errorf("failed to normalize the string of provided username %q at position %d", s, n)
549564
}
550-
return replaceCharsHyphenRE.ReplaceAllLiteralString(removeCharsRE.ReplaceAllLiteralString(strDiacriticsRemoved, ""), "-"), nil
565+
return vars.replaceCharsHyphenRE.ReplaceAllLiteralString(vars.removeCharsRE.ReplaceAllLiteralString(strDiacriticsRemoved, ""), "-"), nil
551566
}
552567

553568
var (

‎models/user/user_system.go

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,23 @@ import (
1010
)
1111

1212
const (
13-
GhostUserID = -1
14-
GhostUserName = "Ghost"
15-
GhostUserLowerName = "ghost"
13+
GhostUserID = -1
14+
GhostUserName = "Ghost"
1615
)
1716

1817
// NewGhostUser creates and returns a fake user for someone has deleted their account.
1918
func NewGhostUser() *User {
2019
return &User{
2120
ID: GhostUserID,
2221
Name: GhostUserName,
23-
LowerName: GhostUserLowerName,
22+
LowerName: strings.ToLower(GhostUserName),
2423
}
2524
}
2625

26+
func IsGhostUserName(name string) bool {
27+
return strings.EqualFold(name, GhostUserName)
28+
}
29+
2730
// IsGhost check if user is fake user for a deleted account
2831
func (u *User) IsGhost() bool {
2932
if u == nil {
@@ -32,20 +35,10 @@ func (u *User) IsGhost() bool {
3235
return u.ID == GhostUserID && u.Name == GhostUserName
3336
}
3437

35-
// NewReplaceUser creates and returns a fake user for external user
36-
func NewReplaceUser(name string) *User {
37-
return &User{
38-
ID: 0,
39-
Name: name,
40-
LowerName: strings.ToLower(name),
41-
}
42-
}
43-
4438
const (
45-
ActionsUserID = -2
46-
ActionsUserName = "gitea-actions"
47-
ActionsFullName = "Gitea Actions"
48-
ActionsEmail = "[email protected]"
39+
ActionsUserID = -2
40+
ActionsUserName = "gitea-actions"
41+
ActionsUserEmail = "[email protected]"
4942
)
5043

5144
// NewActionsUser creates and returns a fake user for running the actions.
@@ -55,8 +48,8 @@ func NewActionsUser() *User {
5548
Name: ActionsUserName,
5649
LowerName: ActionsUserName,
5750
IsActive: true,
58-
FullName: ActionsFullName,
59-
Email: ActionsEmail,
51+
FullName: "Gitea Actions",
52+
Email: ActionsUserEmail,
6053
KeepEmailPrivate: true,
6154
LoginName: ActionsUserName,
6255
Type: UserTypeIndividual,

‎routers/web/user/avatar.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package user
55

66
import (
7-
"strings"
87
"time"
98

109
"code.gitea.io/gitea/models/avatars"
@@ -27,7 +26,7 @@ func AvatarByUserName(ctx *context.Context) {
2726
size := int(ctx.PathParamInt64("size"))
2827

2928
var user *user_model.User
30-
if strings.ToLower(userName) != user_model.GhostUserLowerName {
29+
if !user_model.IsGhostUserName(userName) {
3130
var err error
3231
if user, err = user_model.GetUserByName(ctx, userName); err != nil {
3332
if user_model.IsErrUserNotExist(err) {

0 commit comments

Comments
 (0)