Use conditions but not repo ids as query condition (#16839)

* Use conditions but not repo ids as query condition

* Improve the performance of pulls/issue

* Remove duplicated code

* fix lint

* Fix bug

* Fix stats

* More fixes

* Fix build

* Fix lint

* Fix test

* Fix build

* Adjust the logic

* Merge

* Fix conflicts

* improve the performance

* Add comments for the query conditions functions

* Some improvements

Author: lunny

models/issue.go

@@ -1186,6 +1186,9 @@ type IssuesOptions struct {
 	// prioritize issues from this repo
 	PriorityRepoID int64
 	IsArchived     util.OptionalBool
+	Org            *Organization    // issues permission scope
+	Team           *Team            // issues permission scope
+	User           *user_model.User // issues permission scope
 }
 
 // sortIssuesSession sort an issues-related session based on the provided
@@ -1337,6 +1340,44 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
 				From("milestone").
 				Where(builder.In("name", opts.IncludeMilestones)))
 	}
+
+	if opts.User != nil {
+		sess.And(
+			issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()),
+		)
+	}
+}
+
+// issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table
+func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *Organization, team *Team, isPull bool) builder.Cond {
+	var cond = builder.NewCond()
+	var unitType = unit.TypeIssues
+	if isPull {
+		unitType = unit.TypePullRequests
+	}
+	if org != nil {
+		if team != nil {
+			cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos
+		} else {
+			cond = cond.And(
+				builder.Or(
+					userOrgUnitRepoCond(repoIDstr, userID, org.ID, unitType), // team member repos
+					userOrgPublicUnitRepoCond(userID, org.ID),                // user org public non-member repos, TODO: check repo has issues
+				),
+			)
+		}
+	} else {
+		cond = cond.And(
+			builder.Or(
+				userOwnedRepoCond(userID),                          // owned repos
+				userCollaborationRepoCond(repoIDstr, userID),       // collaboration repos
+				userAssignedRepoCond(repoIDstr, userID),            // user has been assigned accessible public repos
+				userMentionedRepoCond(repoIDstr, userID),           // user has been mentioned accessible public repos
+				userCreateIssueRepoCond(repoIDstr, userID, isPull), // user has created issue/pr accessible public repos
+			),
+		)
+	}
+	return cond
 }
 
 func applyReposCondition(sess *xorm.Session, repoIDs []int64) *xorm.Session {
@@ -1646,15 +1687,16 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats,
 
 // UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
 type UserIssueStatsOptions struct {
-	UserID      int64
-	RepoIDs     []int64
-	UserRepoIDs []int64
-	FilterMode  int
-	IsPull      bool
-	IsClosed    bool
-	IssueIDs    []int64
-	IsArchived  util.OptionalBool
-	LabelIDs    []int64
+	UserID     int64
+	RepoIDs    []int64
+	FilterMode int
+	IsPull     bool
+	IsClosed   bool
+	IssueIDs   []int64
+	IsArchived util.OptionalBool
+	LabelIDs   []int64
+	Org        *Organization
+	Team       *Team
 }
 
 // GetUserIssueStats returns issue statistic information for dashboard by given conditions.
@@ -1671,28 +1713,34 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
 		cond = cond.And(builder.In("issue.id", opts.IssueIDs))
 	}
 
+	if opts.UserID > 0 {
+		cond = cond.And(issuePullAccessibleRepoCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull))
+	}
+
 	sess := func(cond builder.Cond) *xorm.Session {
 		s := db.GetEngine(db.DefaultContext).Where(cond)
 		if len(opts.LabelIDs) > 0 {
 			s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id").
 				In("issue_label.label_id", opts.LabelIDs)
 		}
-		if opts.IsArchived != util.OptionalBoolNone {
-			s.Join("INNER", "repository", "issue.repo_id = repository.id").
-				And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
+		if opts.UserID > 0 || opts.IsArchived != util.OptionalBoolNone {
+			s.Join("INNER", "repository", "issue.repo_id = repository.id")
+			if opts.IsArchived != util.OptionalBoolNone {
+				s.And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
+			}
 		}
 		return s
 	}
 
 	switch opts.FilterMode {
 	case FilterModeAll:
-		stats.OpenCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).
+		stats.OpenCount, err = sess(cond).
 			And("issue.is_closed = ?", false).
 			Count(new(Issue))
 		if err != nil {
 			return nil, err
 		}
-		stats.ClosedCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).
+		stats.ClosedCount, err = sess(cond).
 			And("issue.is_closed = ?", true).
 			Count(new(Issue))
 		if err != nil {
@@ -1768,7 +1816,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
 		return nil, err
 	}
 
-	stats.YourRepositoriesCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).Count(new(Issue))
+	stats.YourRepositoriesCount, err = sess(cond).Count(new(Issue))
 	if err != nil {
 		return nil, err
 	}

models/issue_list.go

@@ -25,6 +25,9 @@ const (
 func (issues IssueList) getRepoIDs() []int64 {
 	repoIDs := make(map[int64]struct{}, len(issues))
 	for _, issue := range issues {
+		if issue.Repo != nil {
+			continue
+		}
 		if _, ok := repoIDs[issue.RepoID]; !ok {
 			repoIDs[issue.RepoID] = struct{}{}
 		}
@@ -56,8 +59,12 @@ func (issues IssueList) loadRepositories(e db.Engine) ([]*repo_model.Repository,
 	}
 
 	for _, issue := range issues {
-		issue.Repo = repoMaps[issue.RepoID]
-		if issue.PullRequest != nil {
+		if issue.Repo == nil {
+			issue.Repo = repoMaps[issue.RepoID]
+		} else {
+			repoMaps[issue.RepoID] = issue.Repo
+		}
+		if issue.PullRequest != nil && issue.PullRequest.BaseRepo == nil {
 			issue.PullRequest.BaseRepo = issue.Repo
 		}
 	}

models/issue_test.go

@@ -206,52 +206,52 @@ func TestGetUserIssueStats(t *testing.T) {
 				FilterMode: FilterModeAll,
 			},
 			IssueStats{
-				YourRepositoriesCount: 0,
-				AssignCount:           1,
-				CreateCount:           1,
-				OpenCount:             0,
-				ClosedCount:           0,
+				YourRepositoriesCount: 1, // 6
+				AssignCount:           1, // 6
+				CreateCount:           1, // 6
+				OpenCount:             1, // 6
+				ClosedCount:           1, // 1
 			},
 		},
 		{
 			UserIssueStatsOptions{
 				UserID:     1,
-				FilterMode: FilterModeAssign,
+				RepoIDs:    []int64{1},
+				FilterMode: FilterModeAll,
+				IsClosed:   true,
 			},
 			IssueStats{
-				YourRepositoriesCount: 0,
-				AssignCount:           2,
-				CreateCount:           2,
-				OpenCount:             2,
-				ClosedCount:           0,
+				YourRepositoriesCount: 1, // 6
+				AssignCount:           0,
+				CreateCount:           0,
+				OpenCount:             1, // 6
+				ClosedCount:           1, // 1
 			},
 		},
 		{
 			UserIssueStatsOptions{
 				UserID:     1,
-				FilterMode: FilterModeCreate,
+				FilterMode: FilterModeAssign,
 			},
 			IssueStats{
-				YourRepositoriesCount: 0,
-				AssignCount:           2,
-				CreateCount:           2,
-				OpenCount:             2,
+				YourRepositoriesCount: 1, // 6
+				AssignCount:           1, // 6
+				CreateCount:           1, // 6
+				OpenCount:             1, // 6
 				ClosedCount:           0,
 			},
 		},
 		{
 			UserIssueStatsOptions{
-				UserID:      2,
-				UserRepoIDs: []int64{1, 2},
-				FilterMode:  FilterModeAll,
-				IsClosed:    true,
+				UserID:     1,
+				FilterMode: FilterModeCreate,
 			},
 			IssueStats{
-				YourRepositoriesCount: 2,
-				AssignCount:           0,
-				CreateCount:           2,
-				OpenCount:             2,
-				ClosedCount:           2,
+				YourRepositoriesCount: 1, // 6
+				AssignCount:           1, // 6
+				CreateCount:           1, // 6
+				OpenCount:             1, // 6
+				ClosedCount:           0,
 			},
 		},
 		{
@@ -260,9 +260,10 @@ func TestGetUserIssueStats(t *testing.T) {
 				FilterMode: FilterModeMention,
 			},
 			IssueStats{
-				YourRepositoriesCount: 0,
-				AssignCount:           2,
-				CreateCount:           2,
+				YourRepositoriesCount: 1, // 6
+				AssignCount:           1, // 6
+				CreateCount:           1, // 6
+				MentionCount:          0,
 				OpenCount:             0,
 				ClosedCount:           0,
 			},
@@ -274,19 +275,21 @@ func TestGetUserIssueStats(t *testing.T) {
 				IssueIDs:   []int64{1},
 			},
 			IssueStats{
-				YourRepositoriesCount: 0,
-				AssignCount:           1,
-				CreateCount:           1,
-				OpenCount:             1,
+				YourRepositoriesCount: 1, // 1
+				AssignCount:           1, // 1
+				CreateCount:           1, // 1
+				OpenCount:             1, // 1
 				ClosedCount:           0,
 			},
 		},
 	} {
-		stats, err := GetUserIssueStats(test.Opts)
-		if !assert.NoError(t, err) {
-			continue
-		}
-		assert.Equal(t, test.ExpectedIssueStats, *stats)
+		t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) {
+			stats, err := GetUserIssueStats(test.Opts)
+			if !assert.NoError(t, err) {
+				return
+			}
+			assert.Equal(t, test.ExpectedIssueStats, *stats)
+		})
 	}
 }
 

models/repo/repo.go

@@ -731,15 +731,6 @@ func CountUserRepositories(userID int64, private bool) int64 {
 	return countRepositories(userID, private)
 }
 
-// GetUserMirrorRepositories returns a list of mirror repositories of given user.
-func GetUserMirrorRepositories(userID int64) ([]*Repository, error) {
-	repos := make([]*Repository, 0, 10)
-	return repos, db.GetEngine(db.DefaultContext).
-		Where("owner_id = ?", userID).
-		And("is_mirror = ?", true).
-		Find(&repos)
-}
-
 func getRepositoryCount(e db.Engine, ownerID int64) (int64, error) {
 	return e.Count(&Repository{OwnerID: ownerID})
 }
@@ -766,25 +757,3 @@ func GetPublicRepositoryCount(u *user_model.User) (int64, error) {
 func GetPrivateRepositoryCount(u *user_model.User) (int64, error) {
 	return getPrivateRepositoryCount(db.GetEngine(db.DefaultContext), u)
 }
-
-// IterateRepository iterate repositories
-func IterateRepository(f func(repo *Repository) error) error {
-	var start int
-	batchSize := setting.Database.IterateBufferSize
-	for {
-		repos := make([]*Repository, 0, batchSize)
-		if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&repos); err != nil {
-			return err
-		}
-		if len(repos) == 0 {
-			return nil
-		}
-		start += len(repos)
-
-		for _, repo := range repos {
-			if err := f(repo); err != nil {
-				return err
-			}
-		}
-	}
-}

models/repo/repo_list.go

@@ -0,0 +1,46 @@
+// Copyright 2021 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 repo
+
+import (
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// GetUserMirrorRepositories returns a list of mirror repositories of given user.
+func GetUserMirrorRepositories(userID int64) ([]*Repository, error) {
+	repos := make([]*Repository, 0, 10)
+	return repos, db.GetEngine(db.DefaultContext).
+		Where("owner_id = ?", userID).
+		And("is_mirror = ?", true).
+		Find(&repos)
+}
+
+// IterateRepository iterate repositories
+func IterateRepository(f func(repo *Repository) error) error {
+	var start int
+	batchSize := setting.Database.IterateBufferSize
+	for {
+		repos := make([]*Repository, 0, batchSize)
+		if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&repos); err != nil {
+			return err
+		}
+		if len(repos) == 0 {
+			return nil
+		}
+		start += len(repos)
+
+		for _, repo := range repos {
+			if err := f(repo); err != nil {
+				return err
+			}
+		}
+	}
+}
+
+// FindReposMapByIDs find repos as map
+func FindReposMapByIDs(repoIDs []int64, res map[int64]*Repository) error {
+	return db.GetEngine(db.DefaultContext).In("id", repoIDs).Find(&res)
+}