Improve performance of dashboard (#4977)

models/action.go

@@ -194,6 +194,10 @@ func (a *Action) GetRepoLink() string {
 
 // GetCommentLink returns link to action comment.
 func (a *Action) GetCommentLink() string {
+ return a.getCommentLink(x)
+}
+
+func (a *Action) getCommentLink(e Engine) string {
  if a == nil {
  return "#"
  }
@@ -213,11 +217,15 @@ func (a *Action) GetCommentLink() string {
  return "#"
  }
 
- issue, err := GetIssueByID(issueID)
+ issue, err := getIssueByID(e, issueID)
  if err != nil {
  return "#"
  }
 
+ if err = issue.loadRepo(e); err != nil {
+ return "#"
+ }
+
  return issue.HTMLURL()
 }
 
@@ -330,30 +338,50 @@ type PushCommits struct {
  Commits []*PushCommit
  CompareURL string
 
- avatars map[string]string
+ avatars map[string]string
+ emailUsers map[string]*User
 }
 
 // NewPushCommits creates a new PushCommits object.
 func NewPushCommits() *PushCommits {
  return &PushCommits{
- avatars: make(map[string]string),
+ avatars: make(map[string]string),
+ emailUsers: make(map[string]*User),
  }
 }
 
 // ToAPIPayloadCommits converts a PushCommits object to
 // api.PayloadCommit format.
 func (pc *PushCommits) ToAPIPayloadCommits(repoLink string) []*api.PayloadCommit {
  commits := make([]*api.PayloadCommit, len(pc.Commits))
+
+ if pc.emailUsers == nil {
+ pc.emailUsers = make(map[string]*User)
+ }
+ var err error
  for i, commit := range pc.Commits {
  authorUsername := ""
- author, err := GetUserByEmail(commit.AuthorEmail)
- if err == nil {
+ author, ok := pc.emailUsers[commit.AuthorEmail]
+ if !ok {
+ author, err = GetUserByEmail(commit.AuthorEmail)
+ if err == nil {
+ authorUsername = author.Name
+ pc.emailUsers[commit.AuthorEmail] = author
+ }
+ } else {
  authorUsername = author.Name
  }
+
  committerUsername := ""
- committer, err := GetUserByEmail(commit.CommitterEmail)
- if err == nil {
- // TODO: check errors other than email not found.
+ committer, ok := pc.emailUsers[commit.CommitterEmail]
+ if !ok {
+ committer, err = GetUserByEmail(commit.CommitterEmail)
+ if err == nil {
+ // TODO: check errors other than email not found.
+ committerUsername = committer.Name
+ pc.emailUsers[commit.CommitterEmail] = committer
+ }
+ } else {
  committerUsername = committer.Name
  }
  commits[i] = &api.PayloadCommit{
@@ -379,18 +407,28 @@ func (pc *PushCommits) ToAPIPayloadCommits(repoLink string) []*api.PayloadCommit
 // AvatarLink tries to match user in database with e-mail
 // in order to show custom avatar, and falls back to general avatar link.
 func (pc *PushCommits) AvatarLink(email string) string {
- _, ok := pc.avatars[email]
+ avatar, ok := pc.avatars[email]
+ if ok {
+ return avatar
+ }
+
+ u, ok := pc.emailUsers[email]
  if !ok {
- u, err := GetUserByEmail(email)
+ var err error
+ u, err = GetUserByEmail(email)
  if err != nil {
  pc.avatars[email] = base.AvatarLink(email)
  if !IsErrUserNotExist(err) {
  log.Error(4, "GetUserByEmail: %v", err)
+ return ""
  }
  } else {
- pc.avatars[email] = u.RelAvatarLink()
+ pc.emailUsers[email] = u
  }
  }
+ if u != nil {
+ pc.avatars[email] = u.RelAvatarLink()
+ }
 
  return pc.avatars[email]
 }
@@ -479,7 +517,8 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err
  continue
  }
 
- if err = issue.ChangeStatus(doer, repo, true); err != nil {
+ issue.Repo = repo
+ if err = issue.ChangeStatus(doer, true); err != nil {
  // Don't return an error when dependencies are open as this would let the push fail
  if IsErrDependenciesLeft(err) {
  return nil
@@ -504,7 +543,8 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err
  continue
  }
 
- if err = issue.ChangeStatus(doer, repo, false); err != nil {
+ issue.Repo = repo
+ if err = issue.ChangeStatus(doer, false); err != nil {
  return err
  }
  }

models/issue.go

@@ -86,6 +86,11 @@ func (issue *Issue) IsOverdue() bool {
  return util.TimeStampNow() >= issue.DeadlineUnix
 }
 
+// LoadRepo loads issue's repository
+func (issue *Issue) LoadRepo() error {
+ return issue.loadRepo(x)
+}
+
 func (issue *Issue) loadRepo(e Engine) (err error) {
  if issue.Repo == nil {
  issue.Repo, err = getRepositoryByID(e, issue.RepoID)
@@ -129,6 +134,11 @@ func (issue *Issue) loadLabels(e Engine) (err error) {
  return nil
 }
 
+// LoadPoster loads poster
+func (issue *Issue) LoadPoster() error {
+ return issue.loadPoster(x)
+}
+
 func (issue *Issue) loadPoster(e Engine) (err error) {
  if issue.Poster == nil {
  issue.Poster, err = getUserByID(e, issue.PosterID)
@@ -154,10 +164,16 @@ func (issue *Issue) loadPullRequest(e Engine) (err error) {
  }
  return fmt.Errorf("getPullRequestByIssueID [%d]: %v", issue.ID, err)
  }
+ issue.PullRequest.Issue = issue
  }
  return nil
 }
 
+// LoadPullRequest loads pull request info
+func (issue *Issue) LoadPullRequest() error {
+ return issue.loadPullRequest(x)
+}
+
 func (issue *Issue) loadComments(e Engine) (err error) {
  if issue.Comments != nil {
  return nil
@@ -310,11 +326,18 @@ func (issue *Issue) State() api.StateType {
 // Required - Poster, Labels,
 // Optional - Milestone, Assignee, PullRequest
 func (issue *Issue) APIFormat() *api.Issue {
+ return issue.apiFormat(x)
+}
+
+func (issue *Issue) apiFormat(e Engine) *api.Issue {
+ issue.loadLabels(e)
  apiLabels := make([]*api.Label, len(issue.Labels))
  for i := range issue.Labels {
  apiLabels[i] = issue.Labels[i].APIFormat()
  }
 
+ issue.loadPoster(e)
+ issue.loadRepo(e)
  apiIssue := &api.Issue{
  ID: issue.ID,
  URL: issue.APIURL(),
@@ -336,13 +359,16 @@ func (issue *Issue) APIFormat() *api.Issue {
  if issue.Milestone != nil {
  apiIssue.Milestone = issue.Milestone.APIFormat()
  }
+ issue.loadAssignees(e)
+
  if len(issue.Assignees) > 0 {
  for _, assignee := range issue.Assignees {
  apiIssue.Assignees = append(apiIssue.Assignees, assignee.APIFormat())
  }
  apiIssue.Assignee = issue.Assignees[0].APIFormat() // For compatibility, we're keeping the first assignee as `apiIssue.Assignee`
  }
  if issue.IsPull {
+ issue.loadPullRequest(e)
  apiIssue.PullRequest = &api.PullRequestMeta{
  HasMerged: issue.PullRequest.HasMerged,
  }
@@ -656,7 +682,7 @@ func UpdateIssueCols(issue *Issue, cols ...string) error {
  return updateIssueCols(x, issue, cols...)
 }
 
-func (issue *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository, isClosed bool) (err error) {
+func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (err error) {
  // Nothing should be performed if current status is same as target status
  if issue.IsClosed == isClosed {
  return nil
@@ -707,22 +733,29 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository,
  }
 
  // New action comment
- if _, err = createStatusComment(e, doer, repo, issue); err != nil {
+ if _, err = createStatusComment(e, doer, issue); err != nil {
  return err
  }
 
  return nil
 }
 
 // ChangeStatus changes issue status to open or closed.
-func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (err error) {
+func (issue *Issue) ChangeStatus(doer *User, isClosed bool) (err error) {
  sess := x.NewSession()
  defer sess.Close()
  if err = sess.Begin(); err != nil {
  return err
  }
 
- if err = issue.changeStatus(sess, doer, repo, isClosed); err != nil {
+ if err = issue.loadRepo(sess); err != nil {
+ return err
+ }
+ if err = issue.loadPoster(sess); err != nil {
+ return err
+ }
+
+ if err = issue.changeStatus(sess, doer, isClosed); err != nil {
  return err
  }
 
@@ -733,38 +766,40 @@ func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (e
 
  mode, _ := AccessLevel(issue.Poster, issue.Repo)
  if issue.IsPull {
+ if err = issue.loadPullRequest(sess); err != nil {
+ return err
+ }
  // Merge pull request calls issue.changeStatus so we need to handle separately.
- issue.PullRequest.Issue = issue
  apiPullRequest := &api.PullRequestPayload{
  Index: issue.Index,
  PullRequest: issue.PullRequest.APIFormat(),
- Repository: repo.APIFormat(mode),
+ Repository: issue.Repo.APIFormat(mode),
  Sender: doer.APIFormat(),
  }
  if isClosed {
  apiPullRequest.Action = api.HookIssueClosed
  } else {
  apiPullRequest.Action = api.HookIssueReOpened
  }
- err = PrepareWebhooks(repo, HookEventPullRequest, apiPullRequest)
+ err = PrepareWebhooks(issue.Repo, HookEventPullRequest, apiPullRequest)
  } else {
  apiIssue := &api.IssuePayload{
  Index: issue.Index,
  Issue: issue.APIFormat(),
- Repository: repo.APIFormat(mode),
+ Repository: issue.Repo.APIFormat(mode),
  Sender: doer.APIFormat(),
  }
  if isClosed {
  apiIssue.Action = api.HookIssueClosed
  } else {
  apiIssue.Action = api.HookIssueReOpened
  }
- err = PrepareWebhooks(repo, HookEventIssues, apiIssue)
+ err = PrepareWebhooks(issue.Repo, HookEventIssues, apiIssue)
  }
  if err != nil {
  log.Error(4, "PrepareWebhooks [is_pull: %v, is_closed: %v]: %v", issue.IsPull, isClosed, err)
  } else {
- go HookQueue.Add(repo.ID)
+ go HookQueue.Add(issue.Repo.ID)
  }
 
  return nil
@@ -785,6 +820,10 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
  return fmt.Errorf("updateIssueCols: %v", err)
  }
 
+ if err = issue.loadRepo(sess); err != nil {
+ return fmt.Errorf("loadRepo: %v", err)
+ }
+
  if _, err = createChangeTitleComment(sess, doer, issue.Repo, issue, oldTitle, title); err != nil {
  return fmt.Errorf("createChangeTitleComment: %v", err)
  }
@@ -795,6 +834,9 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
 
  mode, _ := AccessLevel(issue.Poster, issue.Repo)
  if issue.IsPull {
+ if err = issue.loadPullRequest(sess); err != nil {
+ return fmt.Errorf("loadPullRequest: %v", err)
+ }
  issue.PullRequest.Issue = issue
  err = PrepareWebhooks(issue.Repo, HookEventPullRequest, &api.PullRequestPayload{
  Action: api.HookIssueEdited,
@@ -1099,8 +1141,8 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in
  return nil
 }
 
-// GetRawIssueByIndex returns raw issue without loading attributes by index in a repository.
-func GetRawIssueByIndex(repoID, index int64) (*Issue, error) {
+// GetIssueByIndex returns raw issue without loading attributes by index in a repository.
+func GetIssueByIndex(repoID, index int64) (*Issue, error) {
  issue := &Issue{
  RepoID: repoID,
  Index: index,
@@ -1114,9 +1156,9 @@ func GetRawIssueByIndex(repoID, index int64) (*Issue, error) {
  return issue, nil
 }
 
-// GetIssueByIndex returns issue by index in a repository.
-func GetIssueByIndex(repoID, index int64) (*Issue, error) {
- issue, err := GetRawIssueByIndex(repoID, index)
+// GetIssueWithAttrsByIndex returns issue by index in a repository.
+func GetIssueWithAttrsByIndex(repoID, index int64) (*Issue, error) {
+ issue, err := GetIssueByIndex(repoID, index)
  if err != nil {
  return nil, err
  }
@@ -1131,7 +1173,16 @@ func getIssueByID(e Engine, id int64) (*Issue, error) {
  } else if !has {
  return nil, ErrIssueNotExist{id, 0, 0}
  }
- return issue, issue.loadAttributes(e)
+ return issue, nil
+}
+
+// GetIssueWithAttrsByID returns an issue with attributes by given ID.
+func GetIssueWithAttrsByID(id int64) (*Issue, error) {
+ issue, err := getIssueByID(x, id)
+ if err != nil {
+ return nil, err
+ }
+ return issue, issue.loadAttributes(x)
 }
 
 // GetIssueByID returns an issue by given ID.

models/issue_assignees.go

@@ -174,7 +174,7 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in
  apiPullRequest := &api.PullRequestPayload{
  Index: issue.Index,
  PullRequest: issue.PullRequest.APIFormat(),
- Repository: issue.Repo.APIFormat(mode),
+ Repository: issue.Repo.innerAPIFormat(sess, mode, false),
  Sender: doer.APIFormat(),
  }
  if removed {
@@ -191,8 +191,8 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in
 
  apiIssue := &api.IssuePayload{
  Index: issue.Index,
- Issue: issue.APIFormat(),
- Repository: issue.Repo.APIFormat(mode),
+ Issue: issue.apiFormat(sess),
+ Repository: issue.Repo.innerAPIFormat(sess, mode, false),
  Sender: doer.APIFormat(),
  }
  if removed {

models/issue_assignees_test.go

@@ -14,7 +14,7 @@ func TestUpdateAssignee(t *testing.T) {
  assert.NoError(t, PrepareTestDatabase())
 
  // Fake issue with assignees
- issue, err := GetIssueByID(1)
+ issue, err := GetIssueWithAttrsByID(1)
  assert.NoError(t, err)
 
  // Assign multiple users