Browse Source

[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
tags/v1.9.4
6543 4 years ago
parent
commit
0ea4b786cb
5 changed files with 110 additions and 178 deletions
  1. 32
    22
      models/issue.go
  2. 7
    4
      models/issue_test.go
  3. 23
    38
      models/user.go
  4. 0
    22
      models/user_test.go
  5. 48
    92
      routers/user/home.go

+ 32
- 22
models/issue.go View File



// 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
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 // sortIssuesSession sort an issues-related session based on the provided
sess.In("issue.id", opts.IssueIDs) 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. // 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)
} }


// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats. // UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
type UserIssueStatsOptions struct { 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. // GetUserIssueStats returns issue statistic information for dashboard by given conditions.
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(builder.In("issue.repo_id", opts.UserRepoIDs)).
And(repoCond).
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(builder.In("issue.repo_id", opts.UserRepoIDs)).
And(repoCond).
Count(new(Issue)) Count(new(Issue))
if err != nil { if err != nil {
return nil, err return nil, err
} }


stats.YourRepositoriesCount, err = x.Where(cond). stats.YourRepositoriesCount, err = x.Where(cond).
And(builder.In("issue.repo_id", opts.UserRepoIDs)).
And(repoCond).
Count(new(Issue)) Count(new(Issue))
if err != nil { if err != nil {
return nil, err return nil, err

+ 7
- 4
models/issue_test.go View File

"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) {
}, },
{ {
UserIssueStatsOptions{ 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{ IssueStats{
YourRepositoriesCount: 2, YourRepositoriesCount: 2,

+ 23
- 38
models/user.go View File

return err 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 { 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 { 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. // GetMirrorRepositories returns mirror repositories that user owns, including private repositories.

+ 0
- 22
models/user_test.go View File

} }
} }


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()

+ 48
- 92
routers/user/home.go View File

"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/Unknwon/com"
"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"
) )
// 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
page = 1 page = 1
} }


repoID := ctx.QueryInt64("repo")
isShowClosed := ctx.Query("state") == "closed"
var (
isShowClosed = ctx.Query("state") == "closed"
err error
opts = &models.IssuesOptions{
IsClosed: util.OptionalBoolOf(isShowClosed),
IsPull: util.OptionalBoolOf(isPullList),
SortType: sortType,
}
)


// Get repositories. // Get repositories.
var err error
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
}
if repoID > 0 {
opts.RepoIDs = []int64{repoID}
} else { } else {
unitType := models.UnitTypeIssues unitType := models.UnitTypeIssues
if isPullList { if isPullList {
unitType = models.UnitTypePullRequests unitType = models.UnitTypePullRequests
} }
userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
if err != nil {
ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
return
if ctxUser.IsOrganization() {
opts.RepoSubQuery = ctxUser.OrgUnitRepositoriesSubQuery(ctx.User.ID, unitType)
} else {
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 { 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:
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" {
} }
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)
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 {
} }


issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{ issueStats, err := models.GetUserIssueStats(models.UserIssueStatsOptions{
UserID: ctxUser.ID,
RepoID: repoID,
UserRepoIDs: userRepoIDs,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
UserID: ctxUser.ID,
RepoID: repoID,
RepoSubQuery: opts.RepoSubQuery,
FilterMode: filterMode,
IsPull: isPullList,
IsClosed: isShowClosed,
}) })
if err != nil { if err != nil {
ctx.ServerError("GetUserIssueStats", err) ctx.ServerError("GetUserIssueStats", err)

Loading…
Cancel
Save