Fix issues/pr list broken when there are many repositories (#8409)

* fix issues/pr list broken when there are many repositories

* remove unused codes

* fix counting error on issues/prs

* keep the old logic

* fix panic

* fix tests

models/issue.go

@@ -1306,18 +1306,19 @@ func GetIssuesByIDs(issueIDs []int64) ([]*Issue, error) {
 
 // IssuesOptions represents options of an issue.
 type IssuesOptions struct {
-	RepoIDs     []int64 // include all repos if empty
+	RepoIDs      []int64 // include all repos if empty
-	AssigneeID  int64
+	RepoSubQuery *builder.Builder
-	PosterID    int64
+	AssigneeID   int64
-	MentionedID int64
+	PosterID     int64
-	MilestoneID int64
+	MentionedID  int64
-	Page        int
+	MilestoneID  int64
-	PageSize    int
+	Page         int
-	IsClosed    util.OptionalBool
+	PageSize     int
-	IsPull      util.OptionalBool
+	IsClosed     util.OptionalBool
-	LabelIDs    []int64
+	IsPull       util.OptionalBool
-	SortType    string
+	LabelIDs     []int64
-	IssueIDs    []int64
+	SortType     string
+	IssueIDs     []int64
 }
 
 // sortIssuesSession sort an issues-related session based on the provided
@@ -1360,7 +1361,9 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
 		sess.In("issue.id", opts.IssueIDs)
 	}
 
-	if len(opts.RepoIDs) > 0 {
+	if opts.RepoSubQuery != nil {
+		sess.In("issue.repo_id", opts.RepoSubQuery)
+	} else if len(opts.RepoIDs) > 0 {
 		// In case repository IDs are provided but actually no repository has issue.
 		sess.In("issue.repo_id", opts.RepoIDs)
 	}
@@ -1627,12 +1630,12 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
 
 // UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
 type UserIssueStatsOptions struct {
-	UserID      int64
+	UserID       int64
-	RepoID      int64
+	RepoID       int64
-	UserRepoIDs []int64
+	RepoSubQuery *builder.Builder
-	FilterMode  int
+	FilterMode   int
-	IsPull      bool
+	IsPull       bool
-	IsClosed    bool
+	IsClosed     bool
 }
 
 // GetUserIssueStats returns issue statistic information for dashboard by given conditions.
@@ -1646,16 +1649,23 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
 		cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
 	}
 
+	var repoCond = builder.NewCond()
+	if opts.RepoSubQuery != nil {
+		repoCond = builder.In("issue.repo_id", opts.RepoSubQuery)
+	} else {
+		repoCond = builder.Expr("0=1")
+	}
+
 	switch opts.FilterMode {
 	case FilterModeAll:
 		stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
-			And(builder.In("issue.repo_id", opts.UserRepoIDs)).
+			And(repoCond).
 			Count(new(Issue))
 		if err != nil {
 			return nil, err
 		}
 		stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
-			And(builder.In("issue.repo_id", opts.UserRepoIDs)).
+			And(repoCond).
 			Count(new(Issue))
 		if err != nil {
 			return nil, err
@@ -1730,7 +1740,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
 	}
 
 	stats.YourRepositoriesCount, err = x.Where(cond).
-		And(builder.In("issue.repo_id", opts.UserRepoIDs)).
+		And(repoCond).
 		Count(new(Issue))
 	if err != nil {
 		return nil, err

models/issue_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"xorm.io/builder"
 )
 
 func TestIssue_ReplaceLabels(t *testing.T) {
@@ -266,10 +267,12 @@ func TestGetUserIssueStats(t *testing.T) {
 		},
 		{
 			UserIssueStatsOptions{
-				UserID:      2,
+				UserID: 2,
-				UserRepoIDs: []int64{1, 2},
+				RepoSubQuery: builder.Select("repository.id").
-				FilterMode:  FilterModeAll,
+					From("repository").
-				IsClosed:    true,
+					Where(builder.In("repository.id", []int64{1, 2})),
+				FilterMode: FilterModeAll,
+				IsClosed:   true,
 			},
 			IssueStats{
 				YourRepositoriesCount: 2,

models/user.go

@@ -615,50 +615,35 @@ func (u *User) GetRepositories(page, pageSize int) (err error) {
 	return err
 }
 
-// GetRepositoryIDs returns repositories IDs where user owned and has unittypes
+// UnitRepositoriesSubQuery returns repositories query builder according units
-func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
+func (u *User) UnitRepositoriesSubQuery(units ...UnitType) *builder.Builder {
-	var ids []int64
+	b := builder.Select("repository.id").From("repository")
-
-	sess := x.Table("repository").Cols("repository.id")
 
 	if len(units) > 0 {
-		sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
+		b.Join("INNER", "repo_unit", builder.Expr("repository.id = repo_unit.repo_id").
-		sess = sess.In("repo_unit.type", units)
+			And(builder.In("repo_unit.type", units)),
+		)
 	}
-
+	return b.Where(builder.Eq{"repository.owner_id": u.ID})
-	return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
 }
 
-// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
+// OrgUnitRepositoriesSubQuery returns repositories query builder according orgnizations and units
-func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
+func (u *User) OrgUnitRepositoriesSubQuery(userID int64, units ...UnitType) *builder.Builder {
-	var ids []int64
+	b := builder.
-
+		Select("team_repo.repo_id").
-	sess := x.Table("repository").
+		From("team_repo").
-		Cols("repository.id").
+		Join("INNER", "team_user", builder.Eq{"team_user.uid": userID}.And(
-		Join("INNER", "team_user", "repository.owner_id = team_user.org_id").
+			builder.Expr("team_user.team_id = team_repo.team_id"),
-		Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true)
+		))
-
 	if len(units) > 0 {
-		sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id")
+		b.Join("INNER", "team_unit", builder.Eq{"team_unit.org_id": u.ID}.And(
-		sess = sess.In("team_unit.type", units)
+			builder.Expr("team_unit.team_id = team_repo.team_id").And(
-	}
+				builder.In("`type`", units),
-
+			),
-	return ids, sess.
+		))
-		Where("team_user.uid = ?", u.ID).
+	}
-		GroupBy("repository.id").Find(&ids)
+	return b.Where(builder.Eq{"team_repo.org_id": u.ID}).
-}
+		GroupBy("team_repo.repo_id")
-
-// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations
-func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
-	ids, err := u.GetRepositoryIDs(units...)
-	if err != nil {
-		return nil, err
-	}
-	ids2, err := u.GetOrgRepositoryIDs(units...)
-	if err != nil {
-		return nil, err
-	}
-	return append(ids, ids2...), nil
 }
 
 // GetMirrorRepositories returns mirror repositories that user owns, including private repositories.

models/user_test.go

@@ -275,28 +275,6 @@ func BenchmarkHashPassword(b *testing.B) {
 	}
 }
 
-func TestGetOrgRepositoryIDs(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
-	user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
-	user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
-
-	accessibleRepos, err := user2.GetOrgRepositoryIDs()
-	assert.NoError(t, err)
-	// User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization
-	assert.Equal(t, []int64{3, 5, 23, 24, 32}, accessibleRepos)
-
-	accessibleRepos, err = user4.GetOrgRepositoryIDs()
-	assert.NoError(t, err)
-	// User 4's team has access to private repo 3, repo 32 is a public repo of the organization
-	assert.Equal(t, []int64{3, 32}, accessibleRepos)
-
-	accessibleRepos, err = user5.GetOrgRepositoryIDs()
-	assert.NoError(t, err)
-	// User 5's team has no access to any repo
-	assert.Len(t, accessibleRepos, 0)
-}
-
 func TestNewGitSig(t *testing.T) {
 	users := make([]*User, 0, 20)
 	sess := x.NewSession()

routers/user/home.go

@@ -14,13 +14,11 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/keybase/go-crypto/openpgp"
 	"github.com/keybase/go-crypto/openpgp/armor"
-	"github.com/unknwon/com"
 )
 
 const (
@@ -152,6 +150,24 @@ func Dashboard(ctx *context.Context) {
 // Issues render the user issues page
 func Issues(ctx *context.Context) {
 	isPullList := ctx.Params(":type") == "pulls"
+	repoID := ctx.QueryInt64("repo")
+	if repoID > 0 {
+		repo, err := models.GetRepositoryByID(repoID)
+		if err != nil {
+			ctx.ServerError("GetRepositoryByID", err)
+			return
+		}
+		perm, err := models.GetUserRepoPermission(repo, ctx.User)
+		if err != nil {
+			ctx.ServerError("GetUserRepoPermission", err)
+			return
+		}
+		if !perm.CanReadIssuesOrPulls(isPullList) {
+			ctx.NotFound("Repository does not exist or you have no permission", nil)
+			return
+		}
+	}
+
 	if isPullList {
 		ctx.Data["Title"] = ctx.Tr("pull_requests")
 		ctx.Data["PageIsPulls"] = true
@@ -194,58 +210,32 @@ func Issues(ctx *context.Context) {
 		page = 1
 	}
 
-	repoID := ctx.QueryInt64("repo")
+	var (
-	isShowClosed := ctx.Query("state") == "closed"
+		isShowClosed = ctx.Query("state") == "closed"
+		err          error
+		opts         = &models.IssuesOptions{
+			IsClosed: util.OptionalBoolOf(isShowClosed),
+			IsPull:   util.OptionalBoolOf(isPullList),
+			SortType: sortType,
+		}
+	)
 
 	// Get repositories.
-	var err error
+	if repoID > 0 {
-	var userRepoIDs []int64
+		opts.RepoIDs = []int64{repoID}
-	if ctxUser.IsOrganization() {
-		env, err := ctxUser.AccessibleReposEnv(ctx.User.ID)
-		if err != nil {
-			ctx.ServerError("AccessibleReposEnv", err)
-			return
-		}
-		userRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos)
-		if err != nil {
-			ctx.ServerError("env.RepoIDs", err)
-			return
-		}
 	} else {
 		unitType := models.UnitTypeIssues
 		if isPullList {
 			unitType = models.UnitTypePullRequests
 		}
-		userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
+		if ctxUser.IsOrganization() {
-		if err != nil {
+			opts.RepoSubQuery = ctxUser.OrgUnitRepositoriesSubQuery(ctx.User.ID, unitType)
-			ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
+		} else {
-			return
+			opts.RepoSubQuery = ctxUser.UnitRepositoriesSubQuery(unitType)
 		}
 	}
-	if len(userRepoIDs) == 0 {
-		userRepoIDs = []int64{-1}
-	}
-
-	opts := &models.IssuesOptions{
-		IsClosed: util.OptionalBoolOf(isShowClosed),
-		IsPull:   util.OptionalBoolOf(isPullList),
-		SortType: sortType,
-	}
-
-	if repoID > 0 {
-		opts.RepoIDs = []int64{repoID}
-	}
 
 	switch filterMode {
-	case models.FilterModeAll:
-		if repoID > 0 {
-			if !com.IsSliceContainsInt64(userRepoIDs, repoID) {
-				// force an empty result
-				opts.RepoIDs = []int64{-1}
-			}
-		} else {
-			opts.RepoIDs = userRepoIDs
-		}
 	case models.FilterModeAssign:
 		opts.AssigneeID = ctxUser.ID
 	case models.FilterModeCreate:
@@ -254,14 +244,6 @@ func Issues(ctx *context.Context) {
 		opts.MentionedID = ctxUser.ID
 	}
 
-	counts, err := models.CountIssuesByRepo(opts)
-	if err != nil {
-		ctx.ServerError("CountIssuesByRepo", err)
-		return
-	}
-
-	opts.Page = page
-	opts.PageSize = setting.UI.IssuePagingNum
 	var labelIDs []int64
 	selectLabels := ctx.Query("labels")
 	if len(selectLabels) > 0 && selectLabels != "0" {
@@ -273,6 +255,15 @@ func Issues(ctx *context.Context) {
 	}
 	opts.LabelIDs = labelIDs
 
+	counts, err := models.CountIssuesByRepo(opts)
+	if err != nil {
+		ctx.ServerError("CountIssuesByRepo", err)
+		return
+	}
+
+	opts.Page = page
+	opts.PageSize = setting.UI.IssuePagingNum
+
 	issues, err := models.Issues(opts)
 	if err != nil {
 		ctx.ServerError("Issues", err)
@@ -289,41 +280,6 @@ func Issues(ctx *context.Context) {
 		showReposMap[repoID] = repo
 	}
 
-	if repoID > 0 {
-		if _, ok := showReposMap[repoID]; !ok {
-			repo, err := models.GetRepositoryByID(repoID)
-			if models.IsErrRepoNotExist(err) {
-				ctx.NotFound("GetRepositoryByID", err)
-				return
-			} else if err != nil {
-				ctx.ServerError("GetRepositoryByID", fmt.Errorf("[%d]%v", repoID, err))
-				return
-			}
-			showReposMap[repoID] = repo
-		}
-
-		repo := showReposMap[repoID]
-
-		// Check if user has access to given repository.
-		perm, err := models.GetUserRepoPermission(repo, ctxUser)
-		if err != nil {
-			ctx.ServerError("GetUserRepoPermission", fmt.Errorf("[%d]%v", repoID, err))
-			return
-		}
-		if !perm.CanRead(models.UnitTypeIssues) {
-			if log.IsTrace() {
-				log.Trace("Permission Denied: User %-v cannot read %-v of repo %-v\n"+
-					"User in repo has Permissions: %-+v",
-					ctxUser,
-					models.UnitTypeIssues,
-					repo,
-					perm)
-			}
-			ctx.Status(404)
-			return
-		}
-	}
-
 	showRepos := models.RepositoryListOfMap(showReposMap)
 	sort.Sort(showRepos)
 	if err = showRepos.LoadAttributes(); err != nil {
@@ -341,12 +297,12 @@ func Issues(ctx *context.Context) {
 	}
 
 	issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
-		UserID:      ctxUser.ID,
+		UserID:       ctxUser.ID,
-		RepoID:      repoID,
+		RepoID:       repoID,
-		UserRepoIDs: userRepoIDs,
+		RepoSubQuery: opts.RepoSubQuery,
-		FilterMode:  filterMode,
+		FilterMode:   filterMode,
-		IsPull:      isPullList,
+		IsPull:       isPullList,
-		IsClosed:    isShowClosed,
+		IsClosed:     isShowClosed,
 	})
 	if err != nil {
 		ctx.ServerError("GetUserIssueStats", err)