Skip to content

Commit

Permalink
Improve /api/v1/repos/issues/search by just getting repo ids (go-gite…
Browse files Browse the repository at this point in the history
…a#15179)

Backport go-gitea#15179

/api/v1/repos/issues/search is a highly inefficient search which is unfortunately
the basis for our dependency searching algorithm. In particular it currently loads
all of the repositories and their owners and their primary coding language all of
which is immediately thrown away.

This PR makes one simple change - just get the IDs.

Related go-gitea#14560
Related go-gitea#12827

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath committed Mar 29, 2021
1 parent 6130882 commit b9efe0f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 38 deletions.
86 changes: 68 additions & 18 deletions models/repo_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
"xorm.io/xorm"
)

// RepositoryListDefaultPageSize is the default number of repositories
Expand Down Expand Up @@ -363,6 +364,35 @@ func SearchRepository(opts *SearchRepoOptions) (RepositoryList, int64, error) {

// SearchRepositoryByCondition search repositories by condition
func SearchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond, loadAttributes bool) (RepositoryList, int64, error) {
sess, count, err := searchRepositoryByCondition(opts, cond)
if err != nil {
return nil, 0, err
}
defer sess.Close()

defaultSize := 50
if opts.PageSize > 0 {
defaultSize = opts.PageSize
}
repos := make(RepositoryList, 0, defaultSize)
if err := sess.Find(&repos); err != nil {
return nil, 0, fmt.Errorf("Repo: %v", err)
}

if opts.PageSize <= 0 {
count = int64(len(repos))
}

if loadAttributes {
if err := repos.loadAttributes(sess); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %v", err)
}
}

return repos, count, nil
}

func searchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond) (*xorm.Session, int64, error) {
if opts.Page <= 0 {
opts.Page = 1
}
Expand All @@ -376,31 +406,24 @@ func SearchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond, loa
}

sess := x.NewSession()
defer sess.Close()

count, err := sess.
Where(cond).
Count(new(Repository))
if err != nil {
return nil, 0, fmt.Errorf("Count: %v", err)
var count int64
if opts.PageSize > 0 {
var err error
count, err = sess.
Where(cond).
Count(new(Repository))
if err != nil {
_ = sess.Close()
return nil, 0, fmt.Errorf("Count: %v", err)
}
}

repos := make(RepositoryList, 0, opts.PageSize)
sess.Where(cond).OrderBy(opts.OrderBy.String())
if opts.PageSize > 0 {
sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize)
}
if err = sess.Find(&repos); err != nil {
return nil, 0, fmt.Errorf("Repo: %v", err)
}

if loadAttributes {
if err = repos.loadAttributes(sess); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %v", err)
}
}

return repos, count, nil
return sess, count, nil
}

// accessibleRepositoryCondition takes a user a returns a condition for checking if a repository is accessible
Expand Down Expand Up @@ -456,6 +479,33 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (RepositoryList, int64, err
return SearchRepository(opts)
}

// SearchRepositoryIDs takes keyword and part of repository name to search,
// it returns results in given range and number of total results.
func SearchRepositoryIDs(opts *SearchRepoOptions) ([]int64, int64, error) {
opts.IncludeDescription = false

cond := SearchRepositoryCondition(opts)

sess, count, err := searchRepositoryByCondition(opts, cond)
if err != nil {
return nil, 0, err
}
defer sess.Close()

defaultSize := 50
if opts.PageSize > 0 {
defaultSize = opts.PageSize
}

ids := make([]int64, 0, defaultSize)
err = sess.Select("id").Table("repository").Find(&ids)
if opts.PageSize <= 0 {
count = int64(len(ids))
}

return ids, count, err
}

// AccessibleRepoIDsQuery queries accessible repository ids. Usable as a subquery wherever repo ids need to be filtered.
func AccessibleRepoIDsQuery(user *User) *builder.Builder {
// NB: Please note this code needs to still work if user is nil
Expand Down
24 changes: 4 additions & 20 deletions routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/convert"
issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/notification"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
Expand Down Expand Up @@ -113,11 +112,7 @@ func SearchIssues(ctx *context.APIContext) {
}

// find repos user can access (for issue search)
repoIDs := make([]int64, 0)
opts := &models.SearchRepoOptions{
ListOptions: models.ListOptions{
PageSize: 15,
},
Private: false,
AllPublic: true,
TopicOnly: false,
Expand All @@ -132,21 +127,10 @@ func SearchIssues(ctx *context.APIContext) {
opts.AllLimited = true
}

for page := 1; ; page++ {
opts.Page = page
repos, count, err := models.SearchRepositoryByName(opts)
if err != nil {
ctx.Error(http.StatusInternalServerError, "SearchRepositoryByName", err)
return
}

if len(repos) == 0 {
break
}
log.Trace("Processing next %d repos of %d", len(repos), count)
for _, repo := range repos {
repoIDs = append(repoIDs, repo.ID)
}
repoIDs, _, err := models.SearchRepositoryIDs(opts)
if err != nil {
ctx.Error(http.StatusInternalServerError, "SearchRepositoryByName", err)
return
}

var issues []*models.Issue
Expand Down

0 comments on commit b9efe0f

Please sign in to comment.