diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2019-10-08 10:57:41 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-10-08 10:57:41 +0800 |
commit | 78438d310be42f9c5e0e2937ee54e6050cc8f381 (patch) | |
tree | 235c28d5bd22d38dcf70dd5d4eb8c412f66899ac /models | |
parent | 1a269f7ef81b1a7865c4679b91a4037059ad4938 (diff) | |
download | gitea-78438d310be42f9c5e0e2937ee54e6050cc8f381.tar.gz gitea-78438d310be42f9c5e0e2937ee54e6050cc8f381.zip |
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
Diffstat (limited to 'models')
-rw-r--r-- | models/issue.go | 54 | ||||
-rw-r--r-- | models/issue_test.go | 11 | ||||
-rw-r--r-- | models/user.go | 61 | ||||
-rw-r--r-- | models/user_test.go | 22 |
4 files changed, 62 insertions, 86 deletions
diff --git a/models/issue.go b/models/issue.go index e4cc1291c2..cfa6191b47 100644 --- a/models/issue.go +++ b/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 - AssigneeID int64 - PosterID int64 - MentionedID int64 - MilestoneID int64 - Page int - PageSize int - IsClosed util.OptionalBool - IsPull util.OptionalBool - LabelIDs []int64 - SortType string - IssueIDs []int64 + RepoIDs []int64 // include all repos if empty + RepoSubQuery *builder.Builder + AssigneeID int64 + PosterID int64 + MentionedID int64 + MilestoneID int64 + Page int + PageSize int + IsClosed util.OptionalBool + IsPull util.OptionalBool + LabelIDs []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 - RepoID int64 - UserRepoIDs []int64 - FilterMode int - IsPull bool - IsClosed bool + UserID int64 + RepoID int64 + RepoSubQuery *builder.Builder + FilterMode int + IsPull 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 diff --git a/models/issue_test.go b/models/issue_test.go index 9cd9ff0ad9..65f4d6ba66 100644 --- a/models/issue_test.go +++ b/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, - UserRepoIDs: []int64{1, 2}, - FilterMode: FilterModeAll, - IsClosed: true, + UserID: 2, + RepoSubQuery: builder.Select("repository.id"). + From("repository"). + Where(builder.In("repository.id", []int64{1, 2})), + FilterMode: FilterModeAll, + IsClosed: true, }, IssueStats{ YourRepositoriesCount: 2, diff --git a/models/user.go b/models/user.go index 030e23c383..8c4befb139 100644 --- a/models/user.go +++ b/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 -func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) { - var ids []int64 - - sess := x.Table("repository").Cols("repository.id") +// UnitRepositoriesSubQuery returns repositories query builder according units +func (u *User) UnitRepositoriesSubQuery(units ...UnitType) *builder.Builder { + b := builder.Select("repository.id").From("repository") if len(units) > 0 { - sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id") - sess = sess.In("repo_unit.type", units) + b.Join("INNER", "repo_unit", builder.Expr("repository.id = repo_unit.repo_id"). + And(builder.In("repo_unit.type", units)), + ) } - - return ids, sess.Where("owner_id = ?", u.ID).Find(&ids) + return b.Where(builder.Eq{"repository.owner_id": u.ID}) } -// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes -func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) { - var ids []int64 - - sess := x.Table("repository"). - Cols("repository.id"). - Join("INNER", "team_user", "repository.owner_id = team_user.org_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) - +// OrgUnitRepositoriesSubQuery returns repositories query builder according orgnizations and units +func (u *User) OrgUnitRepositoriesSubQuery(userID int64, units ...UnitType) *builder.Builder { + b := builder. + Select("team_repo.repo_id"). + From("team_repo"). + Join("INNER", "team_user", builder.Eq{"team_user.uid": userID}.And( + builder.Expr("team_user.team_id = team_repo.team_id"), + )) if len(units) > 0 { - sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id") - sess = sess.In("team_unit.type", units) - } - - return ids, sess. - Where("team_user.uid = ?", u.ID). - GroupBy("repository.id").Find(&ids) -} - -// 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 + b.Join("INNER", "team_unit", builder.Eq{"team_unit.org_id": u.ID}.And( + builder.Expr("team_unit.team_id = team_repo.team_id").And( + builder.In("`type`", units), + ), + )) + } + return b.Where(builder.Eq{"team_repo.org_id": u.ID}). + GroupBy("team_repo.repo_id") } // GetMirrorRepositories returns mirror repositories that user owns, including private repositories. diff --git a/models/user_test.go b/models/user_test.go index bcb955817c..75d806eadc 100644 --- a/models/user_test.go +++ b/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() |