diff --git a/models/actions/run.go b/models/actions/run.go index b8c9a59fb49..b4f4d001715 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -120,7 +120,7 @@ func (run *ActionRun) RefTooltip() string { } // LoadAttributes load Repo TriggerUser if not loaded -func (run *ActionRun) LoadAttributes(ctx context.Context) error { +func (run *ActionRun) LoadAttributes(ctx context.Context) (err error) { if run == nil { return nil } @@ -134,11 +134,10 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { } if run.TriggerUser == nil { - u, err := user_model.GetPossibleUserByID(ctx, run.TriggerUserID) + run.TriggerUserID, run.TriggerUser, err = user_model.GetPossibleUserByID(ctx, run.TriggerUserID) if err != nil { return err } - run.TriggerUser = u } return nil diff --git a/models/actions/run_attempt.go b/models/actions/run_attempt.go index 7fd0212522e..8ef2ddf00a3 100644 --- a/models/actions/run_attempt.go +++ b/models/actions/run_attempt.go @@ -50,7 +50,7 @@ func (attempt *ActionRunAttempt) Duration() time.Duration { return calculateDuration(attempt.Started, attempt.Stopped, attempt.Status, attempt.Updated) } -func (attempt *ActionRunAttempt) LoadAttributes(ctx context.Context) error { +func (attempt *ActionRunAttempt) LoadAttributes(ctx context.Context) (err error) { if attempt == nil { return nil } @@ -67,11 +67,10 @@ func (attempt *ActionRunAttempt) LoadAttributes(ctx context.Context) error { } if attempt.TriggerUser == nil { - u, err := user_model.GetPossibleUserByID(ctx, attempt.TriggerUserID) + attempt.TriggerUserID, attempt.TriggerUser, err = user_model.GetPossibleUserByID(ctx, attempt.TriggerUserID) if err != nil { return err } - attempt.TriggerUser = u } return nil diff --git a/models/activities/action.go b/models/activities/action.go index 8e589eda88d..4ffdca842a4 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -186,15 +186,7 @@ func (a *Action) LoadActUser(ctx context.Context) { if a.ActUser != nil { return } - var err error - a.ActUser, err = user_model.GetPossibleUserByID(ctx, a.ActUserID) - if err == nil { - return - } else if user_model.IsErrUserNotExist(err) { - a.ActUser = user_model.NewGhostUser() - } else { - log.Error("GetUserByID(%d): %v", a.ActUserID, err) - } + a.ActUserID, a.ActUser, _ = user_model.GetPossibleUserByID(ctx, a.ActUserID) } func (a *Action) LoadRepo(ctx context.Context) error { diff --git a/models/issues/comment.go b/models/issues/comment.go index 84a7150b9f7..acfc07ff220 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -400,16 +400,7 @@ func (c *Comment) LoadPoster(ctx context.Context) (err error) { if c.Poster != nil { return nil } - - c.Poster, err = user_model.GetPossibleUserByID(ctx, c.PosterID) - if err != nil { - if user_model.IsErrUserNotExist(err) { - c.PosterID = user_model.GhostUserID - c.Poster = user_model.NewGhostUser() - } else { - log.Error("getUserByID[%d]: %v", c.ID, err) - } - } + c.PosterID, c.Poster, err = user_model.GetPossibleUserByID(ctx, c.PosterID) return err } diff --git a/models/issues/issue.go b/models/issues/issue.go index ec58cd04f6e..fe5433fbb20 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -175,17 +175,10 @@ func (issue *Issue) IsTimetrackerEnabled(ctx context.Context) bool { // LoadPoster loads poster func (issue *Issue) LoadPoster(ctx context.Context) (err error) { - if issue.Poster == nil && issue.PosterID != 0 { - issue.Poster, err = user_model.GetPossibleUserByID(ctx, issue.PosterID) - if err != nil { - issue.PosterID = user_model.GhostUserID - issue.Poster = user_model.NewGhostUser() - if !user_model.IsErrUserNotExist(err) { - return fmt.Errorf("getUserByID.(poster) [%d]: %w", issue.PosterID, err) - } - return nil - } + if issue.Poster != nil { + return nil } + issue.PosterID, issue.Poster, err = user_model.GetPossibleUserByID(ctx, issue.PosterID) return err } diff --git a/models/issues/review.go b/models/issues/review.go index 694e384ded4..78ef0d20c23 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -176,15 +176,7 @@ func (r *Review) LoadReviewer(ctx context.Context) (err error) { if r.ReviewerID == 0 || r.Reviewer != nil { return err } - r.Reviewer, err = user_model.GetPossibleUserByID(ctx, r.ReviewerID) - if err != nil { - if !user_model.IsErrUserNotExist(err) { - return fmt.Errorf("GetPossibleUserByID [%d]: %w", r.ReviewerID, err) - } - r.ReviewerID = user_model.GhostUserID - r.Reviewer = user_model.NewGhostUser() - return nil - } + r.ReviewerID, r.Reviewer, err = user_model.GetPossibleUserByID(ctx, r.ReviewerID) return err } diff --git a/models/pull/automerge.go b/models/pull/automerge.go index 7f940a98492..d32dc847d2d 100644 --- a/models/pull/automerge.go +++ b/models/pull/automerge.go @@ -5,14 +5,12 @@ package pull import ( "context" - "errors" "fmt" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" ) // AutoMerge represents a pull request scheduled for merging when checks succeed @@ -78,16 +76,8 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe return false, nil, err } - doer, err := user_model.GetPossibleUserByID(ctx, scheduledPRM.DoerID) - if errors.Is(err, util.ErrNotExist) { - doer, err = user_model.NewGhostUser(), nil - } - if err != nil { - return false, nil, err - } - - scheduledPRM.Doer = doer - return true, scheduledPRM, nil + scheduledPRM.DoerID, scheduledPRM.Doer, err = user_model.GetPossibleUserByID(ctx, scheduledPRM.DoerID) + return true, scheduledPRM, err } // DeleteScheduledAutoMerge delete a scheduled pull request diff --git a/models/user/user.go b/models/user/user.go index c9dc59b5430..69b97e9b478 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -7,6 +7,7 @@ package user import ( "context" "encoding/hex" + "errors" "fmt" "html/template" "mime" @@ -1013,17 +1014,22 @@ func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) { return users, err } -// GetPossibleUserByID returns the user if id > 0 or returns system user if id < 0 -func GetPossibleUserByID(ctx context.Context, id int64) (*User, error) { +// GetPossibleUserByID returns the possible user and its ID. If the user doesn't exist, it returns Ghost user +func GetPossibleUserByID(ctx context.Context, id int64) (_ int64, u *User, err error) { if id < 0 { if newFunc, ok := globalVars().systemUserNewFuncs[id]; ok { - return newFunc(), nil + u = newFunc() } - return nil, ErrUserNotExist{UID: id} - } else if id == 0 { - return nil, ErrUserNotExist{} } - return GetUserByID(ctx, id) + if u == nil { + u, err = GetUserByID(ctx, id) + if errors.Is(err, util.ErrNotExist) { + u = NewGhostUser() + } else if err != nil { + return 0, nil, err + } + } + return u.ID, u, nil } // GetPossibleUserByIDs returns the users if id > 0 or returns system users if id < 0 diff --git a/models/user/user_system_test.go b/models/user/user_system_test.go index 70a900378f3..3ae9c6e3665 100644 --- a/models/user/user_system_test.go +++ b/models/user/user_system_test.go @@ -11,8 +11,9 @@ import ( ) func TestSystemUser(t *testing.T) { - u, err := GetPossibleUserByID(t.Context(), -1) + uid, u, err := GetPossibleUserByID(t.Context(), -1) require.NoError(t, err) + assert.Equal(t, int64(-1), uid) assert.Equal(t, "Ghost", u.Name) assert.Equal(t, "ghost", u.LowerName) assert.True(t, u.IsGhost()) @@ -21,8 +22,9 @@ func TestSystemUser(t *testing.T) { require.NotNil(t, u) assert.Equal(t, "Ghost", u.Name) - u, err = GetPossibleUserByID(t.Context(), -2) + uid, u, err = GetPossibleUserByID(t.Context(), -2) require.NoError(t, err) + assert.Equal(t, int64(-2), uid) assert.Equal(t, "gitea-actions", u.Name) assert.Equal(t, "gitea-actions", u.LowerName) assert.True(t, u.IsGiteaActions()) @@ -31,6 +33,8 @@ func TestSystemUser(t *testing.T) { require.NotNil(t, u) assert.Equal(t, "Gitea Actions", u.FullName) - _, err = GetPossibleUserByID(t.Context(), -3) - require.Error(t, err) + uid, u, err = GetPossibleUserByID(t.Context(), 999999) + require.NoError(t, err) + assert.Equal(t, int64(-1), uid) + assert.Equal(t, "Ghost", u.Name) } diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 1372022ae4a..063bfbe26ac 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -104,13 +104,9 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions) releaseInfos := make([]*ReleaseInfo, 0, len(releases)) for _, r := range releases { if r.Publisher, ok = cacheUsers[r.PublisherID]; !ok { - r.Publisher, err = user_model.GetPossibleUserByID(ctx, r.PublisherID) + r.PublisherID, r.Publisher, err = user_model.GetPossibleUserByID(ctx, r.PublisherID) if err != nil { - if user_model.IsErrUserNotExist(err) { - r.Publisher = user_model.NewGhostUser() - } else { - return nil, err - } + return nil, err } cacheUsers[r.PublisherID] = r.Publisher } diff --git a/services/convert/convert.go b/services/convert/convert.go index 3ef3b4f9cb2..29f46d37ad5 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -831,7 +831,7 @@ func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application { // ToLFSLock convert a LFSLock to api.LFSLock func ToLFSLock(ctx context.Context, l *git_model.LFSLock) *api.LFSLock { - u, err := user_model.GetPossibleUserByID(ctx, l.OwnerID) + _, u, err := user_model.GetPossibleUserByID(ctx, l.OwnerID) if err != nil { return nil }