refactor: move load repoOwner of act.Repo into models

Author: 6543

models/action_list.go

@@ -25,7 +25,7 @@ func (actions ActionList) getUserIDs() []int64 {
 	return keysInt64(userIDs)
 }
 
-func (actions ActionList) loadUsers(e db.Engine) ([]*user_model.User, error) {
+func (actions ActionList) loadUsers(e db.Engine) (map[int64]*user_model.User, error) {
 	if len(actions) == 0 {
 		return nil, nil
 	}
@@ -42,7 +42,7 @@ func (actions ActionList) loadUsers(e db.Engine) ([]*user_model.User, error) {
 	for _, action := range actions {
 		action.ActUser = userMaps[action.ActUserID]
 	}
-	return valuesUser(userMaps), nil
+	return userMaps, nil
 }
 
 func (actions ActionList) getRepoIDs() []int64 {
@@ -55,35 +55,57 @@ func (actions ActionList) getRepoIDs() []int64 {
 	return keysInt64(repoIDs)
 }
 
-func (actions ActionList) loadRepositories(e db.Engine) ([]*repo_model.Repository, error) {
+func (actions ActionList) loadRepositories(e db.Engine) error {
 	if len(actions) == 0 {
-		return nil, nil
+		return nil
 	}
 
 	repoIDs := actions.getRepoIDs()
 	repoMaps := make(map[int64]*repo_model.Repository, len(repoIDs))
-	err := e.
-		In("id", repoIDs).
-		Find(&repoMaps)
+	err := e.In("id", repoIDs).Find(&repoMaps)
 	if err != nil {
-		return nil, fmt.Errorf("find repository: %v", err)
+		return fmt.Errorf("find repository: %v", err)
 	}
 
 	for _, action := range actions {
 		action.Repo = repoMaps[action.RepoID]
 	}
-	return valuesRepository(repoMaps), nil
+	return nil
 }
 
-// loadAttributes loads all attributes
-func (actions ActionList) loadAttributes(e db.Engine) (err error) {
-	if _, err = actions.loadUsers(e); err != nil {
-		return
+func (actions ActionList) loadRepoOwner(e db.Engine, userMap map[int64]*user_model.User) (err error) {
+	if userMap == nil {
+		userMap = make(map[int64]*user_model.User)
 	}
 
-	if _, err = actions.loadRepositories(e); err != nil {
-		return
+	for _, action := range actions {
+		repoOwner, ok := userMap[action.Repo.OwnerID]
+		if !ok {
+			repoOwner, err = user_model.GetUserByID(action.Repo.OwnerID)
+			if err != nil {
+				if user_model.IsErrUserNotExist(err) {
+					continue
+				}
+				return err
+			}
+			userMap[repoOwner.ID] = repoOwner
+		}
+		action.Repo.Owner = repoOwner
 	}
 
 	return nil
 }
+
+// loadAttributes loads all attributes
+func (actions ActionList) loadAttributes(e db.Engine) error {
+	userMap, err := actions.loadUsers(e)
+	if err != nil {
+		return err
+	}
+
+	if err := actions.loadRepositories(e); err != nil {
+		return err
+	}
+
+	return actions.loadRepoOwner(e, userMap)
+}

routers/web/feed/profile.go

@@ -23,34 +23,6 @@ func RetrieveFeeds(ctx *context.Context, options models.GetFeedsOptions) models.
 		return nil
 	}
 
-	// TODO: move load repoOwner of act.Repo into models.GetFeeds->loadAttributes()
-	{
-		userCache := map[int64]*user_model.User{options.RequestedUser.ID: options.RequestedUser}
-		if ctx.User != nil {
-			userCache[ctx.User.ID] = ctx.User
-		}
-		for _, act := range actions {
-			if act.ActUser != nil {
-				userCache[act.ActUserID] = act.ActUser
-			}
-		}
-		for _, act := range actions {
-			repoOwner, ok := userCache[act.Repo.OwnerID]
-			if !ok {
-				repoOwner, err = user_model.GetUserByID(act.Repo.OwnerID)
-				if err != nil {
-					if user_model.IsErrUserNotExist(err) {
-						continue
-					}
-					ctx.ServerError("GetUserByID", err)
-					return nil
-				}
-				userCache[repoOwner.ID] = repoOwner
-			}
-			act.Repo.Owner = repoOwner
-		}
-	}
-
 	return actions
 }