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

This reverts commit 78438d310b.
This commit is contained in:
Lunny Xiao 2019-10-09 01:55:16 +08:00 committed by Lauris BH
parent 78438d310b
commit 170743c8a0
5 changed files with 177 additions and 109 deletions

View file

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

View file

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

View file

@ -615,35 +615,50 @@ func (u *User) GetRepositories(page, pageSize int) (err error) {
return err return err
} }
// UnitRepositoriesSubQuery returns repositories query builder according units // GetRepositoryIDs returns repositories IDs where user owned and has unittypes
func (u *User) UnitRepositoriesSubQuery(units ...UnitType) *builder.Builder { func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
b := builder.Select("repository.id").From("repository") var ids []int64
sess := x.Table("repository").Cols("repository.id")
if len(units) > 0 { if len(units) > 0 {
b.Join("INNER", "repo_unit", builder.Expr("repository.id = repo_unit.repo_id"). sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id")
And(builder.In("repo_unit.type", units)), sess = sess.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)
} }
// OrgUnitRepositoriesSubQuery returns repositories query builder according orgnizations and units // GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes
func (u *User) OrgUnitRepositoriesSubQuery(userID int64, units ...UnitType) *builder.Builder { func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
b := builder. var ids []int64
Select("team_repo.repo_id").
From("team_repo"). sess := x.Table("repository").
Join("INNER", "team_user", builder.Eq{"team_user.uid": userID}.And( Cols("repository.id").
builder.Expr("team_user.team_id = team_repo.team_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)
if len(units) > 0 { if len(units) > 0 {
b.Join("INNER", "team_unit", builder.Eq{"team_unit.org_id": u.ID}.And( sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id")
builder.Expr("team_unit.team_id = team_repo.team_id").And( sess = sess.In("team_unit.type", units)
builder.In("`type`", units),
),
))
} }
return b.Where(builder.Eq{"team_repo.org_id": u.ID}).
GroupBy("team_repo.repo_id") 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
} }
// GetMirrorRepositories returns mirror repositories that user owns, including private repositories. // GetMirrorRepositories returns mirror repositories that user owns, including private repositories.

View file

@ -275,6 +275,28 @@ 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) { func TestNewGitSig(t *testing.T) {
users := make([]*User, 0, 20) users := make([]*User, 0, 20)
sess := x.NewSession() sess := x.NewSession()

View file

@ -14,11 +14,13 @@ import (
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"github.com/keybase/go-crypto/openpgp" "github.com/keybase/go-crypto/openpgp"
"github.com/keybase/go-crypto/openpgp/armor" "github.com/keybase/go-crypto/openpgp/armor"
"github.com/unknwon/com"
) )
const ( const (
@ -150,24 +152,6 @@ func Dashboard(ctx *context.Context) {
// Issues render the user issues page // Issues render the user issues page
func Issues(ctx *context.Context) { func Issues(ctx *context.Context) {
isPullList := ctx.Params(":type") == "pulls" 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 { if isPullList {
ctx.Data["Title"] = ctx.Tr("pull_requests") ctx.Data["Title"] = ctx.Tr("pull_requests")
ctx.Data["PageIsPulls"] = true ctx.Data["PageIsPulls"] = true
@ -210,32 +194,58 @@ func Issues(ctx *context.Context) {
page = 1 page = 1
} }
var ( repoID := ctx.QueryInt64("repo")
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. // Get repositories.
if repoID > 0 { var err error
opts.RepoIDs = []int64{repoID} var userRepoIDs []int64
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 { } else {
unitType := models.UnitTypeIssues unitType := models.UnitTypeIssues
if isPullList { if isPullList {
unitType = models.UnitTypePullRequests unitType = models.UnitTypePullRequests
} }
if ctxUser.IsOrganization() { userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
opts.RepoSubQuery = ctxUser.OrgUnitRepositoriesSubQuery(ctx.User.ID, unitType) if err != nil {
} else { ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
opts.RepoSubQuery = ctxUser.UnitRepositoriesSubQuery(unitType) return
} }
} }
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 { 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: case models.FilterModeAssign:
opts.AssigneeID = ctxUser.ID opts.AssigneeID = ctxUser.ID
case models.FilterModeCreate: case models.FilterModeCreate:
@ -244,6 +254,14 @@ func Issues(ctx *context.Context) {
opts.MentionedID = ctxUser.ID 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 var labelIDs []int64
selectLabels := ctx.Query("labels") selectLabels := ctx.Query("labels")
if len(selectLabels) > 0 && selectLabels != "0" { if len(selectLabels) > 0 && selectLabels != "0" {
@ -255,15 +273,6 @@ func Issues(ctx *context.Context) {
} }
opts.LabelIDs = labelIDs 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) issues, err := models.Issues(opts)
if err != nil { if err != nil {
ctx.ServerError("Issues", err) ctx.ServerError("Issues", err)
@ -280,6 +289,41 @@ func Issues(ctx *context.Context) {
showReposMap[repoID] = repo 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) showRepos := models.RepositoryListOfMap(showReposMap)
sort.Sort(showRepos) sort.Sort(showRepos)
if err = showRepos.LoadAttributes(); err != nil { if err = showRepos.LoadAttributes(); err != nil {
@ -297,12 +341,12 @@ func Issues(ctx *context.Context) {
} }
issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{ issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
UserID: ctxUser.ID, UserID: ctxUser.ID,
RepoID: repoID, RepoID: repoID,
RepoSubQuery: opts.RepoSubQuery, UserRepoIDs: userRepoIDs,
FilterMode: filterMode, FilterMode: filterMode,
IsPull: isPullList, IsPull: isPullList,
IsClosed: isShowClosed, IsClosed: isShowClosed,
}) })
if err != nil { if err != nil {
ctx.ServerError("GetUserIssueStats", err) ctx.ServerError("GetUserIssueStats", err)