Skip to content

Commit

Permalink
Use conditions but not repo ids as query condition (go-gitea#16839)
Browse files Browse the repository at this point in the history
* Use conditions but not repo ids as query condition

* Improve the performance of pulls/issue

* Remove duplicated code

* fix lint

* Fix bug

* Fix stats

* More fixes

* Fix build

* Fix lint

* Fix test

* Fix build

* Adjust the logic

* Merge

* Fix conflicts

* improve the performance

* Add comments for the query conditions functions

* Some improvements
  • Loading branch information
lunny authored and Stelios Malathouras committed Mar 28, 2022
1 parent bdbc50a commit 9710ad4
Show file tree
Hide file tree
Showing 11 changed files with 397 additions and 423 deletions.
78 changes: 63 additions & 15 deletions models/issue.go
Expand Up @@ -1186,6 +1186,9 @@ type IssuesOptions struct {
// prioritize issues from this repo
PriorityRepoID int64
IsArchived util.OptionalBool
Org *Organization // issues permission scope
Team *Team // issues permission scope
User *user_model.User // issues permission scope
}

// sortIssuesSession sort an issues-related session based on the provided
Expand Down Expand Up @@ -1337,6 +1340,44 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
From("milestone").
Where(builder.In("name", opts.IncludeMilestones)))
}

if opts.User != nil {
sess.And(
issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()),
)
}
}

// issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table
func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *Organization, team *Team, isPull bool) builder.Cond {
var cond = builder.NewCond()
var unitType = unit.TypeIssues
if isPull {
unitType = unit.TypePullRequests
}
if org != nil {
if team != nil {
cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos
} else {
cond = cond.And(
builder.Or(
userOrgUnitRepoCond(repoIDstr, userID, org.ID, unitType), // team member repos
userOrgPublicUnitRepoCond(userID, org.ID), // user org public non-member repos, TODO: check repo has issues
),
)
}
} else {
cond = cond.And(
builder.Or(
userOwnedRepoCond(userID), // owned repos
userCollaborationRepoCond(repoIDstr, userID), // collaboration repos
userAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos
userMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible public repos
userCreateIssueRepoCond(repoIDstr, userID, isPull), // user has created issue/pr accessible public repos
),
)
}
return cond
}

func applyReposCondition(sess *xorm.Session, repoIDs []int64) *xorm.Session {
Expand Down Expand Up @@ -1646,15 +1687,16 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats,

// UserIssueStatsOptions contains parameters accepted by GetUserIssueStats.
type UserIssueStatsOptions struct {
UserID int64
RepoIDs []int64
UserRepoIDs []int64
FilterMode int
IsPull bool
IsClosed bool
IssueIDs []int64
IsArchived util.OptionalBool
LabelIDs []int64
UserID int64
RepoIDs []int64
FilterMode int
IsPull bool
IsClosed bool
IssueIDs []int64
IsArchived util.OptionalBool
LabelIDs []int64
Org *Organization
Team *Team
}

// GetUserIssueStats returns issue statistic information for dashboard by given conditions.
Expand All @@ -1671,28 +1713,34 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
cond = cond.And(builder.In("issue.id", opts.IssueIDs))
}

if opts.UserID > 0 {
cond = cond.And(issuePullAccessibleRepoCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull))
}

sess := func(cond builder.Cond) *xorm.Session {
s := db.GetEngine(db.DefaultContext).Where(cond)
if len(opts.LabelIDs) > 0 {
s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id").
In("issue_label.label_id", opts.LabelIDs)
}
if opts.IsArchived != util.OptionalBoolNone {
s.Join("INNER", "repository", "issue.repo_id = repository.id").
And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
if opts.UserID > 0 || opts.IsArchived != util.OptionalBoolNone {
s.Join("INNER", "repository", "issue.repo_id = repository.id")
if opts.IsArchived != util.OptionalBoolNone {
s.And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()})
}
}
return s
}

switch opts.FilterMode {
case FilterModeAll:
stats.OpenCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).
stats.OpenCount, err = sess(cond).
And("issue.is_closed = ?", false).
Count(new(Issue))
if err != nil {
return nil, err
}
stats.ClosedCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).
stats.ClosedCount, err = sess(cond).
And("issue.is_closed = ?", true).
Count(new(Issue))
if err != nil {
Expand Down Expand Up @@ -1768,7 +1816,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
return nil, err
}

stats.YourRepositoriesCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).Count(new(Issue))
stats.YourRepositoriesCount, err = sess(cond).Count(new(Issue))
if err != nil {
return nil, err
}
Expand Down
11 changes: 9 additions & 2 deletions models/issue_list.go
Expand Up @@ -25,6 +25,9 @@ const (
func (issues IssueList) getRepoIDs() []int64 {
repoIDs := make(map[int64]struct{}, len(issues))
for _, issue := range issues {
if issue.Repo != nil {
continue
}
if _, ok := repoIDs[issue.RepoID]; !ok {
repoIDs[issue.RepoID] = struct{}{}
}
Expand Down Expand Up @@ -56,8 +59,12 @@ func (issues IssueList) loadRepositories(e db.Engine) ([]*repo_model.Repository,
}

for _, issue := range issues {
issue.Repo = repoMaps[issue.RepoID]
if issue.PullRequest != nil {
if issue.Repo == nil {
issue.Repo = repoMaps[issue.RepoID]
} else {
repoMaps[issue.RepoID] = issue.Repo
}
if issue.PullRequest != nil && issue.PullRequest.BaseRepo == nil {
issue.PullRequest.BaseRepo = issue.Repo
}
}
Expand Down
77 changes: 40 additions & 37 deletions models/issue_test.go
Expand Up @@ -206,52 +206,52 @@ func TestGetUserIssueStats(t *testing.T) {
FilterMode: FilterModeAll,
},
IssueStats{
YourRepositoriesCount: 0,
AssignCount: 1,
CreateCount: 1,
OpenCount: 0,
ClosedCount: 0,
YourRepositoriesCount: 1, // 6
AssignCount: 1, // 6
CreateCount: 1, // 6
OpenCount: 1, // 6
ClosedCount: 1, // 1
},
},
{
UserIssueStatsOptions{
UserID: 1,
FilterMode: FilterModeAssign,
RepoIDs: []int64{1},
FilterMode: FilterModeAll,
IsClosed: true,
},
IssueStats{
YourRepositoriesCount: 0,
AssignCount: 2,
CreateCount: 2,
OpenCount: 2,
ClosedCount: 0,
YourRepositoriesCount: 1, // 6
AssignCount: 0,
CreateCount: 0,
OpenCount: 1, // 6
ClosedCount: 1, // 1
},
},
{
UserIssueStatsOptions{
UserID: 1,
FilterMode: FilterModeCreate,
FilterMode: FilterModeAssign,
},
IssueStats{
YourRepositoriesCount: 0,
AssignCount: 2,
CreateCount: 2,
OpenCount: 2,
YourRepositoriesCount: 1, // 6
AssignCount: 1, // 6
CreateCount: 1, // 6
OpenCount: 1, // 6
ClosedCount: 0,
},
},
{
UserIssueStatsOptions{
UserID: 2,
UserRepoIDs: []int64{1, 2},
FilterMode: FilterModeAll,
IsClosed: true,
UserID: 1,
FilterMode: FilterModeCreate,
},
IssueStats{
YourRepositoriesCount: 2,
AssignCount: 0,
CreateCount: 2,
OpenCount: 2,
ClosedCount: 2,
YourRepositoriesCount: 1, // 6
AssignCount: 1, // 6
CreateCount: 1, // 6
OpenCount: 1, // 6
ClosedCount: 0,
},
},
{
Expand All @@ -260,9 +260,10 @@ func TestGetUserIssueStats(t *testing.T) {
FilterMode: FilterModeMention,
},
IssueStats{
YourRepositoriesCount: 0,
AssignCount: 2,
CreateCount: 2,
YourRepositoriesCount: 1, // 6
AssignCount: 1, // 6
CreateCount: 1, // 6
MentionCount: 0,
OpenCount: 0,
ClosedCount: 0,
},
Expand All @@ -274,19 +275,21 @@ func TestGetUserIssueStats(t *testing.T) {
IssueIDs: []int64{1},
},
IssueStats{
YourRepositoriesCount: 0,
AssignCount: 1,
CreateCount: 1,
OpenCount: 1,
YourRepositoriesCount: 1, // 1
AssignCount: 1, // 1
CreateCount: 1, // 1
OpenCount: 1, // 1
ClosedCount: 0,
},
},
} {
stats, err := GetUserIssueStats(test.Opts)
if !assert.NoError(t, err) {
continue
}
assert.Equal(t, test.ExpectedIssueStats, *stats)
t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) {
stats, err := GetUserIssueStats(test.Opts)
if !assert.NoError(t, err) {
return
}
assert.Equal(t, test.ExpectedIssueStats, *stats)
})
}
}

Expand Down
31 changes: 0 additions & 31 deletions models/repo/repo.go
Expand Up @@ -731,15 +731,6 @@ func CountUserRepositories(userID int64, private bool) int64 {
return countRepositories(userID, private)
}

// GetUserMirrorRepositories returns a list of mirror repositories of given user.
func GetUserMirrorRepositories(userID int64) ([]*Repository, error) {
repos := make([]*Repository, 0, 10)
return repos, db.GetEngine(db.DefaultContext).
Where("owner_id = ?", userID).
And("is_mirror = ?", true).
Find(&repos)
}

func getRepositoryCount(e db.Engine, ownerID int64) (int64, error) {
return e.Count(&Repository{OwnerID: ownerID})
}
Expand All @@ -766,25 +757,3 @@ func GetPublicRepositoryCount(u *user_model.User) (int64, error) {
func GetPrivateRepositoryCount(u *user_model.User) (int64, error) {
return getPrivateRepositoryCount(db.GetEngine(db.DefaultContext), u)
}

// IterateRepository iterate repositories
func IterateRepository(f func(repo *Repository) error) error {
var start int
batchSize := setting.Database.IterateBufferSize
for {
repos := make([]*Repository, 0, batchSize)
if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&repos); err != nil {
return err
}
if len(repos) == 0 {
return nil
}
start += len(repos)

for _, repo := range repos {
if err := f(repo); err != nil {
return err
}
}
}
}
46 changes: 46 additions & 0 deletions models/repo/repo_list.go
@@ -0,0 +1,46 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package repo

import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/setting"
)

// GetUserMirrorRepositories returns a list of mirror repositories of given user.
func GetUserMirrorRepositories(userID int64) ([]*Repository, error) {
repos := make([]*Repository, 0, 10)
return repos, db.GetEngine(db.DefaultContext).
Where("owner_id = ?", userID).
And("is_mirror = ?", true).
Find(&repos)
}

// IterateRepository iterate repositories
func IterateRepository(f func(repo *Repository) error) error {
var start int
batchSize := setting.Database.IterateBufferSize
for {
repos := make([]*Repository, 0, batchSize)
if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&repos); err != nil {
return err
}
if len(repos) == 0 {
return nil
}
start += len(repos)

for _, repo := range repos {
if err := f(repo); err != nil {
return err
}
}
}
}

// FindReposMapByIDs find repos as map
func FindReposMapByIDs(repoIDs []int64, res map[int64]*Repository) error {
return db.GetEngine(db.DefaultContext).In("id", repoIDs).Find(&res)
}

0 comments on commit 9710ad4

Please sign in to comment.