Skip to content

Commit 0ea4b78

Browse files
6543lafriks
authored andcommitted
[Backport] Fix issues/pr list broken when there are many repositories (#8409) (#8418)
* 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 * rm unused import
1 parent 30718ce commit 0ea4b78

File tree

5 files changed

+110
-178
lines changed

5 files changed

+110
-178
lines changed

models/issue.go

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,18 +1291,19 @@ func GetIssuesByIDs(issueIDs []int64) ([]*Issue, error) {
12911291

12921292
// IssuesOptions represents options of an issue.
12931293
type IssuesOptions struct {
1294-
RepoIDs []int64 // include all repos if empty
1295-
AssigneeID int64
1296-
PosterID int64
1297-
MentionedID int64
1298-
MilestoneID int64
1299-
Page int
1300-
PageSize int
1301-
IsClosed util.OptionalBool
1302-
IsPull util.OptionalBool
1303-
LabelIDs []int64
1304-
SortType string
1305-
IssueIDs []int64
1294+
RepoIDs []int64 // include all repos if empty
1295+
RepoSubQuery *builder.Builder
1296+
AssigneeID int64
1297+
PosterID int64
1298+
MentionedID int64
1299+
MilestoneID int64
1300+
Page int
1301+
PageSize int
1302+
IsClosed util.OptionalBool
1303+
IsPull util.OptionalBool
1304+
LabelIDs []int64
1305+
SortType string
1306+
IssueIDs []int64
13061307
}
13071308

13081309
// sortIssuesSession sort an issues-related session based on the provided
@@ -1345,7 +1346,9 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
13451346
sess.In("issue.id", opts.IssueIDs)
13461347
}
13471348

1348-
if len(opts.RepoIDs) > 0 {
1349+
if opts.RepoSubQuery != nil {
1350+
sess.In("issue.repo_id", opts.RepoSubQuery)
1351+
} else if len(opts.RepoIDs) > 0 {
13491352
// In case repository IDs are provided but actually no repository has issue.
13501353
sess.In("issue.repo_id", opts.RepoIDs)
13511354
}
@@ -1612,12 +1615,12 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
16121615

16131616
// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
16141617
type UserIssueStatsOptions struct {
1615-
UserID int64
1616-
RepoID int64
1617-
UserRepoIDs []int64
1618-
FilterMode int
1619-
IsPull bool
1620-
IsClosed bool
1618+
UserID int64
1619+
RepoID int64
1620+
RepoSubQuery *builder.Builder
1621+
FilterMode int
1622+
IsPull bool
1623+
IsClosed bool
16211624
}
16221625

16231626
// GetUserIssueStats returns issue statistic information for dashboard by given conditions.
@@ -1631,16 +1634,23 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
16311634
cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
16321635
}
16331636

1637+
var repoCond = builder.NewCond()
1638+
if opts.RepoSubQuery != nil {
1639+
repoCond = builder.In("issue.repo_id", opts.RepoSubQuery)
1640+
} else {
1641+
repoCond = builder.Expr("0=1")
1642+
}
1643+
16341644
switch opts.FilterMode {
16351645
case FilterModeAll:
16361646
stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
1637-
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
1647+
And(repoCond).
16381648
Count(new(Issue))
16391649
if err != nil {
16401650
return nil, err
16411651
}
16421652
stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
1643-
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
1653+
And(repoCond).
16441654
Count(new(Issue))
16451655
if err != nil {
16461656
return nil, err
@@ -1692,7 +1702,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
16921702
}
16931703

16941704
stats.YourRepositoriesCount, err = x.Where(cond).
1695-
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
1705+
And(repoCond).
16961706
Count(new(Issue))
16971707
if err != nil {
16981708
return nil, err

models/issue_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"github.com/stretchr/testify/assert"
13+
"xorm.io/builder"
1314
)
1415

1516
func TestIssue_ReplaceLabels(t *testing.T) {
@@ -266,10 +267,12 @@ func TestGetUserIssueStats(t *testing.T) {
266267
},
267268
{
268269
UserIssueStatsOptions{
269-
UserID: 2,
270-
UserRepoIDs: []int64{1, 2},
271-
FilterMode: FilterModeAll,
272-
IsClosed: true,
270+
UserID: 2,
271+
RepoSubQuery: builder.Select("repository.id").
272+
From("repository").
273+
Where(builder.In("repository.id", []int64{1, 2})),
274+
FilterMode: FilterModeAll,
275+
IsClosed: true,
273276
},
274277
IssueStats{
275278
YourRepositoriesCount: 2,

models/user.go

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -561,50 +561,35 @@ func (u *User) GetRepositories(page, pageSize int) (err error) {
561561
return err
562562
}
563563

564-
// GetRepositoryIDs returns repositories IDs where user owned and has unittypes
565-
func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
566-
var ids []int64
567-
568-
sess := x.Table("repository").Cols("repository.id")
564+
// UnitRepositoriesSubQuery returns repositories query builder according units
565+
func (u *User) UnitRepositoriesSubQuery(units ...UnitType) *builder.Builder {
566+
b := builder.Select("repository.id").From("repository")
569567

570568
if len(units) > 0 {
571-
sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
572-
sess = sess.In("repo_unit.type", units)
569+
b.Join("INNER", "repo_unit", builder.Expr("repository.id = repo_unit.repo_id").
570+
And(builder.In("repo_unit.type", units)),
571+
)
573572
}
574-
575-
return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
573+
return b.Where(builder.Eq{"repository.owner_id": u.ID})
576574
}
577575

578-
// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
579-
func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
580-
var ids []int64
581-
582-
sess := x.Table("repository").
583-
Cols("repository.id").
584-
Join("INNER", "team_user", "repository.owner_id = team_user.org_id").
585-
Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true)
586-
576+
// OrgUnitRepositoriesSubQuery returns repositories query builder according orgnizations and units
577+
func (u *User) OrgUnitRepositoriesSubQuery(userID int64, units ...UnitType) *builder.Builder {
578+
b := builder.
579+
Select("team_repo.repo_id").
580+
From("team_repo").
581+
Join("INNER", "team_user", builder.Eq{"team_user.uid": userID}.And(
582+
builder.Expr("team_user.team_id = team_repo.team_id"),
583+
))
587584
if len(units) > 0 {
588-
sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id")
589-
sess = sess.In("team_unit.type", units)
590-
}
591-
592-
return ids, sess.
593-
Where("team_user.uid = ?", u.ID).
594-
GroupBy("repository.id").Find(&ids)
595-
}
596-
597-
// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations
598-
func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
599-
ids, err := u.GetRepositoryIDs(units...)
600-
if err != nil {
601-
return nil, err
602-
}
603-
ids2, err := u.GetOrgRepositoryIDs(units...)
604-
if err != nil {
605-
return nil, err
606-
}
607-
return append(ids, ids2...), nil
585+
b.Join("INNER", "team_unit", builder.Eq{"team_unit.org_id": u.ID}.And(
586+
builder.Expr("team_unit.team_id = team_repo.team_id").And(
587+
builder.In("`type`", units),
588+
),
589+
))
590+
}
591+
return b.Where(builder.Eq{"team_repo.org_id": u.ID}).
592+
GroupBy("team_repo.repo_id")
608593
}
609594

610595
// GetMirrorRepositories returns mirror repositories that user owns, including private repositories.

models/user_test.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -178,28 +178,6 @@ func BenchmarkHashPassword(b *testing.B) {
178178
}
179179
}
180180

181-
func TestGetOrgRepositoryIDs(t *testing.T) {
182-
assert.NoError(t, PrepareTestDatabase())
183-
user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
184-
user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
185-
user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
186-
187-
accessibleRepos, err := user2.GetOrgRepositoryIDs()
188-
assert.NoError(t, err)
189-
// User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization
190-
assert.Equal(t, []int64{3, 5, 23, 24, 32}, accessibleRepos)
191-
192-
accessibleRepos, err = user4.GetOrgRepositoryIDs()
193-
assert.NoError(t, err)
194-
// User 4's team has access to private repo 3, repo 32 is a public repo of the organization
195-
assert.Equal(t, []int64{3, 32}, accessibleRepos)
196-
197-
accessibleRepos, err = user5.GetOrgRepositoryIDs()
198-
assert.NoError(t, err)
199-
// User 5's team has no access to any repo
200-
assert.Len(t, accessibleRepos, 0)
201-
}
202-
203181
func TestNewGitSig(t *testing.T) {
204182
users := make([]*User, 0, 20)
205183
sess := x.NewSession()

0 commit comments

Comments
 (0)