Skip to content

Refactor user & avatar #33433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions models/user/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,30 @@ func GenerateRandomAvatar(ctx context.Context, u *User) error {

u.Avatar = avatars.HashEmail(seed)

// Don't share the images so that we can delete them easily
if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
if err := png.Encode(w, img); err != nil {
log.Error("Encode: %v", err)
_, err = storage.Avatars.Stat(u.CustomAvatarRelativePath())
if err != nil {
// If unable to Stat the avatar file (usually it means non-existing), then try to save a new one
// Don't share the images so that we can delete them easily
if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
if err := png.Encode(w, img); err != nil {
log.Error("Encode: %v", err)
}
return nil
}); err != nil {
return fmt.Errorf("failed to save avatar %s: %w", u.CustomAvatarRelativePath(), err)
}
return err
}); err != nil {
return fmt.Errorf("Failed to create dir %s: %w", u.CustomAvatarRelativePath(), err)
}

if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar").Update(u); err != nil {
return err
}

log.Info("New random avatar created: %d", u.ID)
return nil
}

// AvatarLinkWithSize returns a link to the user's avatar with size. size <= 0 means default size
func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
if u.IsGhost() {
if u.IsGhost() || u.IsGiteaActions() {
return avatars.DefaultAvatarLink()
}

Expand Down
40 changes: 40 additions & 0 deletions models/user/avatar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@
package user

import (
"context"
"io"
"strings"
"testing"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/test"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestUserAvatarLink(t *testing.T) {
Expand All @@ -26,3 +32,37 @@ func TestUserAvatarLink(t *testing.T) {
link = u.AvatarLink(db.DefaultContext)
assert.Equal(t, "https://localhost/sub-path/avatars/avatar.png", link)
}

func TestUserAvatarGenerate(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
var err error
tmpDir := t.TempDir()
storage.Avatars, err = storage.NewLocalStorage(context.Background(), &setting.Storage{Path: tmpDir})
require.NoError(t, err)

u := unittest.AssertExistsAndLoadBean(t, &User{ID: 2})

// there was no avatar, generate a new one
assert.Empty(t, u.Avatar)
err = GenerateRandomAvatar(db.DefaultContext, u)
require.NoError(t, err)
assert.NotEmpty(t, u.Avatar)

// make sure the generated one exists
oldAvatarPath := u.CustomAvatarRelativePath()
_, err = storage.Avatars.Stat(u.CustomAvatarRelativePath())
require.NoError(t, err)
// and try to change its content
_, err = storage.Avatars.Save(u.CustomAvatarRelativePath(), strings.NewReader("abcd"), 4)
require.NoError(t, err)

// try to generate again
err = GenerateRandomAvatar(db.DefaultContext, u)
require.NoError(t, err)
assert.Equal(t, oldAvatarPath, u.CustomAvatarRelativePath())
f, err := storage.Avatars.Open(u.CustomAvatarRelativePath())
require.NoError(t, err)
defer f.Close()
content, _ := io.ReadAll(f)
assert.Equal(t, "abcd", string(content))
}
51 changes: 22 additions & 29 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ type globalVarsStruct struct {
replaceCharsHyphenRE *regexp.Regexp
emailToReplacer *strings.Replacer
emailRegexp *regexp.Regexp
systemUserNewFuncs map[int64]func() *User
}

var globalVars = sync.OnceValue(func() *globalVarsStruct {
Expand All @@ -550,6 +551,11 @@ var globalVars = sync.OnceValue(func() *globalVarsStruct {
";", "",
),
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])?)*$"),

systemUserNewFuncs: map[int64]func() *User{
GhostUserID: NewGhostUser,
ActionsUserID: NewActionsUser,
},
}
})

Expand Down Expand Up @@ -978,30 +984,28 @@ func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
return users, err
}

// GetPossibleUserByID returns the user if id > 0 or return system usrs if id < 0
// GetPossibleUserByID returns the user if id > 0 or returns system user if id < 0
func GetPossibleUserByID(ctx context.Context, id int64) (*User, error) {
switch id {
case GhostUserID:
return NewGhostUser(), nil
case ActionsUserID:
return NewActionsUser(), nil
case 0:
if id < 0 {
if newFunc, ok := globalVars().systemUserNewFuncs[id]; ok {
return newFunc(), nil
}
return nil, ErrUserNotExist{UID: id}
} else if id == 0 {
return nil, ErrUserNotExist{}
default:
return GetUserByID(ctx, id)
}
return GetUserByID(ctx, id)
}

// GetPossibleUserByIDs returns the users if id > 0 or return system users if id < 0
// GetPossibleUserByIDs returns the users if id > 0 or returns system users if id < 0
func GetPossibleUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
uniqueIDs := container.SetOf(ids...)
users := make([]*User, 0, len(ids))
_ = uniqueIDs.Remove(0)
if uniqueIDs.Remove(GhostUserID) {
users = append(users, NewGhostUser())
}
if uniqueIDs.Remove(ActionsUserID) {
users = append(users, NewActionsUser())
for systemUID, newFunc := range globalVars().systemUserNewFuncs {
if uniqueIDs.Remove(systemUID) {
users = append(users, newFunc())
}
}
res, err := GetUserByIDs(ctx, uniqueIDs.Values())
if err != nil {
Expand All @@ -1011,7 +1015,7 @@ func GetPossibleUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
return users, nil
}

// GetUserByNameCtx returns user by given name.
// GetUserByName returns user by given name.
func GetUserByName(ctx context.Context, name string) (*User, error) {
if len(name) == 0 {
return nil, ErrUserNotExist{Name: name}
Expand Down Expand Up @@ -1042,8 +1046,8 @@ func GetUserEmailsByNames(ctx context.Context, names []string) []string {
return mails
}

// GetMaileableUsersByIDs gets users from ids, but only if they can receive mails
func GetMaileableUsersByIDs(ctx context.Context, ids []int64, isMention bool) ([]*User, error) {
// GetMailableUsersByIDs gets users from ids, but only if they can receive mails
func GetMailableUsersByIDs(ctx context.Context, ids []int64, isMention bool) ([]*User, error) {
if len(ids) == 0 {
return nil, nil
}
Expand All @@ -1068,17 +1072,6 @@ func GetMaileableUsersByIDs(ctx context.Context, ids []int64, isMention bool) ([
Find(&ous)
}

// GetUserNamesByIDs returns usernames for all resolved users from a list of Ids.
func GetUserNamesByIDs(ctx context.Context, ids []int64) ([]string, error) {
unames := make([]string, 0, len(ids))
err := db.GetEngine(ctx).In("id", ids).
Table("user").
Asc("name").
Cols("name").
Find(&unames)
return unames, err
}

// GetUserNameByID returns username for the id
func GetUserNameByID(ctx context.Context, id int64) (string, error) {
var name string
Expand Down
16 changes: 15 additions & 1 deletion models/user/user_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ const (
ActionsUserEmail = "[email protected]"
)

func IsGiteaActionsUserName(name string) bool {
return strings.EqualFold(name, ActionsUserName)
}

// NewActionsUser creates and returns a fake user for running the actions.
func NewActionsUser() *User {
return &User{
Expand All @@ -58,6 +62,16 @@ func NewActionsUser() *User {
}
}

func (u *User) IsActions() bool {
func (u *User) IsGiteaActions() bool {
return u != nil && u.ID == ActionsUserID
}

func GetSystemUserByName(name string) *User {
if IsGhostUserName(name) {
return NewGhostUser()
}
if IsGiteaActionsUserName(name) {
return NewActionsUser()
}
return nil
}
32 changes: 32 additions & 0 deletions models/user/user_system_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package user

import (
"testing"

"code.gitea.io/gitea/models/db"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSystemUser(t *testing.T) {
u, err := GetPossibleUserByID(db.DefaultContext, -1)
require.NoError(t, err)
assert.Equal(t, "Ghost", u.Name)
assert.Equal(t, "ghost", u.LowerName)
assert.True(t, u.IsGhost())
assert.True(t, IsGhostUserName("gHost"))

u, err = GetPossibleUserByID(db.DefaultContext, -2)
require.NoError(t, err)
assert.Equal(t, "gitea-actions", u.Name)
assert.Equal(t, "gitea-actions", u.LowerName)
assert.True(t, u.IsGiteaActions())
assert.True(t, IsGiteaActionsUserName("Gitea-actionS"))

_, err = GetPossibleUserByID(db.DefaultContext, -3)
require.Error(t, err)
}
4 changes: 2 additions & 2 deletions models/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,14 @@ func TestGetUserIDsByNames(t *testing.T) {
func TestGetMaileableUsersByIDs(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

results, err := user_model.GetMaileableUsersByIDs(db.DefaultContext, []int64{1, 4}, false)
results, err := user_model.GetMailableUsersByIDs(db.DefaultContext, []int64{1, 4}, false)
assert.NoError(t, err)
assert.Len(t, results, 1)
if len(results) > 1 {
assert.Equal(t, 1, results[0].ID)
}

results, err = user_model.GetMaileableUsersByIDs(db.DefaultContext, []int64{1, 4}, true)
results, err = user_model.GetMailableUsersByIDs(db.DefaultContext, []int64{1, 4}, true)
assert.NoError(t, err)
assert.Len(t, results, 2)
if len(results) > 2 {
Expand Down
4 changes: 2 additions & 2 deletions modules/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func Clean(storage ObjectStorage) error {
}

// SaveFrom saves data to the ObjectStorage with path p from the callback
func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) error) error {
func SaveFrom(objStorage ObjectStorage, path string, callback func(w io.Writer) error) error {
pr, pw := io.Pipe()
defer pr.Close()
go func() {
Expand All @@ -103,7 +103,7 @@ func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) err
}
}()

_, err := objStorage.Save(p, pr, -1)
_, err := objStorage.Save(path, pr, -1)
return err
}

Expand Down
25 changes: 8 additions & 17 deletions routers/web/user/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,18 @@ func cacheableRedirect(ctx *context.Context, location string) {
ctx.Redirect(location)
}

// AvatarByUserName redirect browser to user avatar of requested size
func AvatarByUserName(ctx *context.Context) {
userName := ctx.PathParam("username")
size := int(ctx.PathParamInt64("size"))

var user *user_model.User
if !user_model.IsGhostUserName(userName) {
// AvatarByUsernameSize redirect browser to user avatar of requested size
func AvatarByUsernameSize(ctx *context.Context) {
username := ctx.PathParam("username")
user := user_model.GetSystemUserByName(username)
if user == nil {
var err error
if user, err = user_model.GetUserByName(ctx, userName); err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.NotFound("GetUserByName", err)
return
}
ctx.ServerError("Invalid user: "+userName, err)
if user, err = user_model.GetUserByName(ctx, username); err != nil {
ctx.NotFoundOrServerError("GetUserByName", user_model.IsErrUserNotExist, err)
return
}
} else {
user = user_model.NewGhostUser()
}

cacheableRedirect(ctx, user.AvatarLinkWithSize(ctx, size))
cacheableRedirect(ctx, user.AvatarLinkWithSize(ctx, int(ctx.PathParamInt64("size"))))
}

// AvatarByEmailHash redirects the browser to the email avatar link
Expand Down
2 changes: 1 addition & 1 deletion routers/web/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ func UsernameSubRoute(ctx *context.Context) {
switch {
case strings.HasSuffix(username, ".png"):
if reloadParam(".png") {
AvatarByUserName(ctx)
AvatarByUsernameSize(ctx)
}
case strings.HasSuffix(username, ".keys"):
if reloadParam(".keys") {
Expand Down
2 changes: 1 addition & 1 deletion routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ func registerRoutes(m *web.Router) {
m.Get("/activate", auth.Activate)
m.Post("/activate", auth.ActivatePost)
m.Any("/activate_email", auth.ActivateEmail)
m.Get("/avatar/{username}/{size}", user.AvatarByUserName)
m.Get("/avatar/{username}/{size}", user.AvatarByUsernameSize)
m.Get("/recover_account", auth.ResetPasswd)
m.Post("/recover_account", auth.ResetPasswdPost)
m.Get("/forgot_password", auth.ForgotPasswd)
Expand Down
2 changes: 1 addition & 1 deletion services/actions/notifier_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (input *notifyInput) Notify(ctx context.Context) {

func notify(ctx context.Context, input *notifyInput) error {
shouldDetectSchedules := input.Event == webhook_module.HookEventPush && input.Ref.BranchName() == input.Repo.DefaultBranch
if input.Doer.IsActions() {
if input.Doer.IsGiteaActions() {
// avoiding triggering cyclically, for example:
// a comment of an issue will trigger the runner to add a new comment as reply,
// and the new comment will trigger the runner again.
Expand Down
2 changes: 1 addition & 1 deletion services/mailer/mail_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []*user_mo
}
visited.AddMultiple(ids...)

unfilteredUsers, err := user_model.GetMaileableUsersByIDs(ctx, unfiltered, false)
unfilteredUsers, err := user_model.GetMailableUsersByIDs(ctx, unfiltered, false)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions services/mailer/mail_release.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func MailNewRelease(ctx context.Context, rel *repo_model.Release) {
return
}

recipients, err := user_model.GetMaileableUsersByIDs(ctx, watcherIDList, false)
recipients, err := user_model.GetMailableUsersByIDs(ctx, watcherIDList, false)
if err != nil {
log.Error("user_model.GetMaileableUsersByIDs: %v", err)
log.Error("user_model.GetMailableUsersByIDs: %v", err)
return
}

Expand Down