From ed2ba845252573498a8c1103d407e2070dfd198f Mon Sep 17 00:00:00 2001
From: Bo-Yi Wu <appleboy.tw@gmail.com>
Date: Thu, 22 Feb 2018 00:15:00 +0800
Subject: [PATCH] refactor: reduce sql query in retrieveFeeds (#3554)

---
 models/action.go      | 11 ++++-
 models/action_list.go | 98 +++++++++++++++++++++++++++++++++++++++++++
 routers/user/home.go  | 43 +++++--------------
 3 files changed, 118 insertions(+), 34 deletions(-)
 create mode 100644 models/action_list.go

diff --git a/models/action.go b/models/action.go
index 5333f6277..b551d79bb 100644
--- a/models/action.go
+++ b/models/action.go
@@ -742,5 +742,14 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) {
 	}
 
 	actions := make([]*Action, 0, 20)
-	return actions, x.Limit(20).Desc("id").Where(cond).Find(&actions)
+
+	if err := x.Limit(20).Desc("id").Where(cond).Find(&actions); err != nil {
+		return nil, fmt.Errorf("Find: %v", err)
+	}
+
+	if err := ActionList(actions).LoadAttributes(); err != nil {
+		return nil, fmt.Errorf("LoadAttributes: %v", err)
+	}
+
+	return actions, nil
 }
diff --git a/models/action_list.go b/models/action_list.go
new file mode 100644
index 000000000..6f726f4b3
--- /dev/null
+++ b/models/action_list.go
@@ -0,0 +1,98 @@
+// Copyright 2018 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+import "fmt"
+
+// ActionList defines a list of actions
+type ActionList []*Action
+
+func (actions ActionList) getUserIDs() []int64 {
+	userIDs := make(map[int64]struct{}, len(actions))
+	for _, action := range actions {
+		if _, ok := userIDs[action.ActUserID]; !ok {
+			userIDs[action.ActUserID] = struct{}{}
+		}
+	}
+	return keysInt64(userIDs)
+}
+
+func (actions ActionList) loadUsers(e Engine) ([]*User, error) {
+	if len(actions) == 0 {
+		return nil, nil
+	}
+
+	userIDs := actions.getUserIDs()
+	userMaps := make(map[int64]*User, len(userIDs))
+	err := e.
+		In("id", userIDs).
+		Find(&userMaps)
+	if err != nil {
+		return nil, fmt.Errorf("find user: %v", err)
+	}
+
+	for _, action := range actions {
+		action.ActUser = userMaps[action.ActUserID]
+	}
+	return valuesUser(userMaps), nil
+}
+
+// LoadUsers loads actions' all users
+func (actions ActionList) LoadUsers() ([]*User, error) {
+	return actions.loadUsers(x)
+}
+
+func (actions ActionList) getRepoIDs() []int64 {
+	repoIDs := make(map[int64]struct{}, len(actions))
+	for _, action := range actions {
+		if _, ok := repoIDs[action.RepoID]; !ok {
+			repoIDs[action.RepoID] = struct{}{}
+		}
+	}
+	return keysInt64(repoIDs)
+}
+
+func (actions ActionList) loadRepositories(e Engine) ([]*Repository, error) {
+	if len(actions) == 0 {
+		return nil, nil
+	}
+
+	repoIDs := actions.getRepoIDs()
+	repoMaps := make(map[int64]*Repository, len(repoIDs))
+	err := e.
+		In("id", repoIDs).
+		Find(&repoMaps)
+	if err != nil {
+		return nil, fmt.Errorf("find repository: %v", err)
+	}
+
+	for _, action := range actions {
+		action.Repo = repoMaps[action.RepoID]
+	}
+	return valuesRepository(repoMaps), nil
+}
+
+// LoadRepositories loads actions' all repositories
+func (actions ActionList) LoadRepositories() ([]*Repository, error) {
+	return actions.loadRepositories(x)
+}
+
+// loadAttributes loads all attributes
+func (actions ActionList) loadAttributes(e Engine) (err error) {
+	if _, err = actions.loadUsers(e); err != nil {
+		return
+	}
+
+	if _, err = actions.loadRepositories(e); err != nil {
+		return
+	}
+
+	return nil
+}
+
+// LoadAttributes loads attributes of the actions
+func (actions ActionList) LoadAttributes() error {
+	return actions.loadAttributes(x)
+}
diff --git a/routers/user/home.go b/routers/user/home.go
index 5687cb4f1..2a193bbde 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -66,12 +66,14 @@ func retrieveFeeds(ctx *context.Context, options models.GetFeedsOptions) {
 	if ctx.User != nil {
 		userCache[ctx.User.ID] = ctx.User
 	}
-	repoCache := map[int64]*models.Repository{}
 	for _, act := range actions {
-		// Cache results to reduce queries.
-		u, ok := userCache[act.ActUserID]
+		if act.ActUser != nil {
+			userCache[act.ActUserID] = act.ActUser
+		}
+
+		repoOwner, ok := userCache[act.Repo.OwnerID]
 		if !ok {
-			u, err = models.GetUserByID(act.ActUserID)
+			repoOwner, err = models.GetUserByID(act.Repo.OwnerID)
 			if err != nil {
 				if models.IsErrUserNotExist(err) {
 					continue
@@ -79,35 +81,9 @@ func retrieveFeeds(ctx *context.Context, options models.GetFeedsOptions) {
 				ctx.ServerError("GetUserByID", err)
 				return
 			}
-			userCache[act.ActUserID] = u
+			userCache[repoOwner.ID] = repoOwner
 		}
-		act.ActUser = u
-
-		repo, ok := repoCache[act.RepoID]
-		if !ok {
-			repo, err = models.GetRepositoryByID(act.RepoID)
-			if err != nil {
-				if models.IsErrRepoNotExist(err) {
-					continue
-				}
-				ctx.ServerError("GetRepositoryByID", err)
-				return
-			}
-		}
-		act.Repo = repo
-
-		repoOwner, ok := userCache[repo.OwnerID]
-		if !ok {
-			repoOwner, err = models.GetUserByID(repo.OwnerID)
-			if err != nil {
-				if models.IsErrUserNotExist(err) {
-					continue
-				}
-				ctx.ServerError("GetUserByID", err)
-				return
-			}
-		}
-		repo.Owner = repoOwner
+		act.Repo.Owner = repoOwner
 	}
 	ctx.Data["Feeds"] = actions
 }
@@ -154,7 +130,8 @@ func Dashboard(ctx *context.Context) {
 	ctx.Data["MirrorCount"] = len(mirrors)
 	ctx.Data["Mirrors"] = mirrors
 
-	retrieveFeeds(ctx, models.GetFeedsOptions{RequestedUser: ctxUser,
+	retrieveFeeds(ctx, models.GetFeedsOptions{
+		RequestedUser:   ctxUser,
 		IncludePrivate:  true,
 		OnlyPerformedBy: false,
 		IncludeDeleted:  false,