Improve /api/v1/repos/issues/search by just getting repo ids (#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 #14560
Related #12827

Signed-off-by: Andrew Thornton <art27@cantab.net>

diff --git a/models/repo_list.go b/models/repo_list.go
index 1e06f2511e4b63f7530cb2c65af3032cdcffce48..74bc256190df555a900bb12840d7e59cb4e30f8f 100644 (file)
--- a/models/repo_list.go
+++ b/models/repo_list.go
@@ -12,6 +12,7 @@ import (
        "code.gitea.io/gitea/modules/util"
 
        "xorm.io/builder"
+       "xorm.io/xorm"
 )
 
 // RepositoryListDefaultPageSize is the default number of repositories
@@ -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
        }
@@ -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
@@ -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

diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index 17dad97945330e0757db50f571da8a38207882be..683a2a43b7db0398446a01ac8aedfe728c9fee34 100644 (file)
--- a/routers/api/v1/repo/issue.go
+++ b/routers/api/v1/repo/issue.go
@@ -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"
@@ -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,
@@ -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