Skip to content

Commit 78438d3

Browse files
authored
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
1 parent 1a269f7 commit 78438d3

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
@@ -1306,18 +1306,19 @@ func GetIssuesByIDs(issueIDs []int64) ([]*Issue, error) {
13061306

13071307
// IssuesOptions represents options of an issue.
13081308
type IssuesOptions struct {
1309-
RepoIDs []int64 // include all repos if empty
1310-
AssigneeID int64
1311-
PosterID int64
1312-
MentionedID int64
1313-
MilestoneID int64
1314-
Page int
1315-
PageSize int
1316-
IsClosed util.OptionalBool
1317-
IsPull util.OptionalBool
1318-
LabelIDs []int64
1319-
SortType string
1320-
IssueIDs []int64
1309+
RepoIDs []int64 // include all repos if empty
1310+
RepoSubQuery *builder.Builder
1311+
AssigneeID int64
1312+
PosterID int64
1313+
MentionedID int64
1314+
MilestoneID int64
1315+
Page int
1316+
PageSize int
1317+
IsClosed util.OptionalBool
1318+
IsPull util.OptionalBool
1319+
LabelIDs []int64
1320+
SortType string
1321+
IssueIDs []int64
13211322
}
13221323

13231324
// sortIssuesSession sort an issues-related session based on the provided
@@ -1360,7 +1361,9 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
13601361
sess.In("issue.id", opts.IssueIDs)
13611362
}
13621363

1363-
if len(opts.RepoIDs) > 0 {
1364+
if opts.RepoSubQuery != nil {
1365+
sess.In("issue.repo_id", opts.RepoSubQuery)
1366+
} else if len(opts.RepoIDs) > 0 {
13641367
// In case repository IDs are provided but actually no repository has issue.
13651368
sess.In("issue.repo_id", opts.RepoIDs)
13661369
}
@@ -1627,12 +1630,12 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
16271630

16281631
// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
16291632
type UserIssueStatsOptions struct {
1630-
UserID int64
1631-
RepoID int64
1632-
UserRepoIDs []int64
1633-
FilterMode int
1634-
IsPull bool
1635-
IsClosed bool
1633+
UserID int64
1634+
RepoID int64
1635+
RepoSubQuery *builder.Builder
1636+
FilterMode int
1637+
IsPull bool
1638+
IsClosed bool
16361639
}
16371640

16381641
// GetUserIssueStats returns issue statistic information for dashboard by given conditions.
@@ -1646,16 +1649,23 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
16461649
cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
16471650
}
16481651

1652+
var repoCond = builder.NewCond()
1653+
if opts.RepoSubQuery != nil {
1654+
repoCond = builder.In("issue.repo_id", opts.RepoSubQuery)
1655+
} else {
1656+
repoCond = builder.Expr("0=1")
1657+
}
1658+
16491659
switch opts.FilterMode {
16501660
case FilterModeAll:
16511661
stats.OpenCount, err = x.Where(cond).And("is_closed = ?", false).
1652-
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
1662+
And(repoCond).
16531663
Count(new(Issue))
16541664
if err != nil {
16551665
return nil, err
16561666
}
16571667
stats.ClosedCount, err = x.Where(cond).And("is_closed = ?", true).
1658-
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
1668+
And(repoCond).
16591669
Count(new(Issue))
16601670
if err != nil {
16611671
return nil, err
@@ -1730,7 +1740,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
17301740
}
17311741

17321742
stats.YourRepositoriesCount, err = x.Where(cond).
1733-
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
1743+
And(repoCond).
17341744
Count(new(Issue))
17351745
if err != nil {
17361746
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
@@ -615,50 +615,35 @@ func (u *User) GetRepositories(page, pageSize int) (err error) {
615615
return err
616616
}
617617

618-
// GetRepositoryIDs returns repositories IDs where user owned and has unittypes
619-
func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
620-
var ids []int64
621-
622-
sess := x.Table("repository").Cols("repository.id")
618+
// UnitRepositoriesSubQuery returns repositories query builder according units
619+
func (u *User) UnitRepositoriesSubQuery(units ...UnitType) *builder.Builder {
620+
b := builder.Select("repository.id").From("repository")
623621

624622
if len(units) > 0 {
625-
sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
626-
sess = sess.In("repo_unit.type", units)
623+
b.Join("INNER", "repo_unit", builder.Expr("repository.id = repo_unit.repo_id").
624+
And(builder.In("repo_unit.type", units)),
625+
)
627626
}
628-
629-
return ids, sess.Where("owner_id = ?", u.ID).Find(&ids)
627+
return b.Where(builder.Eq{"repository.owner_id": u.ID})
630628
}
631629

632-
// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
633-
func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
634-
var ids []int64
635-
636-
sess := x.Table("repository").
637-
Cols("repository.id").
638-
Join("INNER", "team_user", "repository.owner_id = team_user.org_id").
639-
Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true)
640-
630+
// OrgUnitRepositoriesSubQuery returns repositories query builder according orgnizations and units
631+
func (u *User) OrgUnitRepositoriesSubQuery(userID int64, units ...UnitType) *builder.Builder {
632+
b := builder.
633+
Select("team_repo.repo_id").
634+
From("team_repo").
635+
Join("INNER", "team_user", builder.Eq{"team_user.uid": userID}.And(
636+
builder.Expr("team_user.team_id = team_repo.team_id"),
637+
))
641638
if len(units) > 0 {
642-
sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id")
643-
sess = sess.In("team_unit.type", units)
644-
}
645-
646-
return ids, sess.
647-
Where("team_user.uid = ?", u.ID).
648-
GroupBy("repository.id").Find(&ids)
649-
}
650-
651-
// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations
652-
func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) {
653-
ids, err := u.GetRepositoryIDs(units...)
654-
if err != nil {
655-
return nil, err
656-
}
657-
ids2, err := u.GetOrgRepositoryIDs(units...)
658-
if err != nil {
659-
return nil, err
660-
}
661-
return append(ids, ids2...), nil
639+
b.Join("INNER", "team_unit", builder.Eq{"team_unit.org_id": u.ID}.And(
640+
builder.Expr("team_unit.team_id = team_repo.team_id").And(
641+
builder.In("`type`", units),
642+
),
643+
))
644+
}
645+
return b.Where(builder.Eq{"team_repo.org_id": u.ID}).
646+
GroupBy("team_repo.repo_id")
662647
}
663648

664649
// 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
@@ -275,28 +275,6 @@ func BenchmarkHashPassword(b *testing.B) {
275275
}
276276
}
277277

278-
func TestGetOrgRepositoryIDs(t *testing.T) {
279-
assert.NoError(t, PrepareTestDatabase())
280-
user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
281-
user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
282-
user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
283-
284-
accessibleRepos, err := user2.GetOrgRepositoryIDs()
285-
assert.NoError(t, err)
286-
// User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization
287-
assert.Equal(t, []int64{3, 5, 23, 24, 32}, accessibleRepos)
288-
289-
accessibleRepos, err = user4.GetOrgRepositoryIDs()
290-
assert.NoError(t, err)
291-
// User 4's team has access to private repo 3, repo 32 is a public repo of the organization
292-
assert.Equal(t, []int64{3, 32}, accessibleRepos)
293-
294-
accessibleRepos, err = user5.GetOrgRepositoryIDs()
295-
assert.NoError(t, err)
296-
// User 5's team has no access to any repo
297-
assert.Len(t, accessibleRepos, 0)
298-
}
299-
300278
func TestNewGitSig(t *testing.T) {
301279
users := make([]*User, 0, 20)
302280
sess := x.NewSession()

0 commit comments

Comments
 (0)