From 6f880885c44b08f49f68ff3bd317bcc826602728 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 1 Feb 2023 13:37:53 +0800 Subject: [PATCH] support search issues pagination --- modules/indexer/issues/indexer.go | 10 ++-- modules/indexer/issues/indexer_test.go | 16 +++---- routers/api/v1/repo/issue.go | 53 ++++++++++++---------- routers/web/repo/issue.go | 63 ++++++++++++++------------ routers/web/user/home.go | 2 +- 5 files changed, 76 insertions(+), 68 deletions(-) diff --git a/modules/indexer/issues/indexer.go b/modules/indexer/issues/indexer.go index 55d3c7bc0914a..24d3c53491822 100644 --- a/modules/indexer/issues/indexer.go +++ b/modules/indexer/issues/indexer.go @@ -381,22 +381,22 @@ func DeleteRepoIssueIndexer(ctx context.Context, repo *repo_model.Repository) { // SearchIssuesByKeyword search issue ids by keywords and repo id // WARNNING: You have to ensure user have permission to visit repoIDs' issues -func SearchIssuesByKeyword(ctx context.Context, repoIDs []int64, keyword string) ([]int64, error) { +func SearchIssuesByKeyword(ctx context.Context, repoIDs []int64, keyword string, start, limit int) (int64, []int64, error) { var issueIDs []int64 indexer := holder.get() if indexer == nil { log.Error("SearchIssuesByKeyword(): unable to get indexer!") - return nil, fmt.Errorf("unable to get issue indexer") + return 0, nil, fmt.Errorf("unable to get issue indexer") } - res, err := indexer.Search(ctx, keyword, repoIDs, 50, 0) + res, err := indexer.Search(ctx, keyword, repoIDs, limit, start) if err != nil { - return nil, err + return 0, nil, err } for _, r := range res.Hits { issueIDs = append(issueIDs, r.ID) } - return issueIDs, nil + return res.Total, issueIDs, nil } // IsAvailable checks if issue indexer is available diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index ff0541d049a03..e56db84ef4ff5 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -51,19 +51,19 @@ func TestBleveSearchIssues(t *testing.T) { time.Sleep(5 * time.Second) - ids, err := SearchIssuesByKeyword(context.TODO(), []int64{1}, "issue2") + _, ids, err := SearchIssuesByKeyword(context.TODO(), []int64{1}, "issue2", 0, 50) assert.NoError(t, err) assert.EqualValues(t, []int64{2}, ids) - ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "first") + _, ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "first", 0, 50) assert.NoError(t, err) assert.EqualValues(t, []int64{1}, ids) - ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "for") + _, ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "for", 0, 50) assert.NoError(t, err) assert.ElementsMatch(t, []int64{1, 2, 3, 5, 11}, ids) - ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "good") + _, ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "good", 0, 50) assert.NoError(t, err) assert.EqualValues(t, []int64{1}, ids) } @@ -74,19 +74,19 @@ func TestDBSearchIssues(t *testing.T) { setting.Indexer.IssueType = "db" InitIssueIndexer(true) - ids, err := SearchIssuesByKeyword(context.TODO(), []int64{1}, "issue2") + _, ids, err := SearchIssuesByKeyword(context.TODO(), []int64{1}, "issue2", 0, 50) assert.NoError(t, err) assert.EqualValues(t, []int64{2}, ids) - ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "first") + _, ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "first", 0, 50) assert.NoError(t, err) assert.EqualValues(t, []int64{1}, ids) - ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "for") + _, ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "for", 0, 50) assert.NoError(t, err) assert.ElementsMatch(t, []int64{1, 2, 3, 5, 11}, ids) - ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "good") + _, ids, err = SearchIssuesByKeyword(context.TODO(), []int64{1}, "good", 0, 50) assert.NoError(t, err) assert.EqualValues(t, []int64{1}, ids) } diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 458838b935624..0ff50c97eaf66 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -189,9 +189,18 @@ func SearchIssues(ctx *context.APIContext) { if strings.IndexByte(keyword, 0) >= 0 { keyword = "" } + // this api is also used in UI, + // so the default limit is set to fit UI needs + limit := ctx.FormInt("limit") + if limit == 0 { + limit = setting.UI.IssuePagingNum + } else if limit > setting.API.MaxResponseItems { + limit = setting.API.MaxResponseItems + } + var issueIDs []int64 if len(keyword) > 0 && len(repoIDs) > 0 { - if issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, repoIDs, keyword); err != nil { + if filteredCount, issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, repoIDs, keyword, limit*ctx.FormInt("page"), limit); err != nil { ctx.Error(http.StatusInternalServerError, "SearchIssuesByKeyword", err) return } @@ -219,15 +228,6 @@ func SearchIssues(ctx *context.APIContext) { includedMilestones = strings.Split(milestones, ",") } - // this api is also used in UI, - // so the default limit is set to fit UI needs - limit := ctx.FormInt("limit") - if limit == 0 { - limit = setting.UI.IssuePagingNum - } else if limit > setting.API.MaxResponseItems { - limit = setting.API.MaxResponseItems - } - // Only fetch the issues if we either don't have a keyword or the search returned issues // This would otherwise return all issues if no issues were found by the search. if len(keyword) == 0 || len(issueIDs) > 0 || len(includedLabelNames) > 0 || len(includedMilestones) > 0 { @@ -272,12 +272,14 @@ func SearchIssues(ctx *context.APIContext) { return } - issuesOpt.ListOptions = db.ListOptions{ - Page: -1, - } - if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { - ctx.Error(http.StatusInternalServerError, "CountIssues", err) - return + if filteredCount == 0 { + issuesOpt.ListOptions = db.ListOptions{ + Page: -1, + } + if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { + ctx.Error(http.StatusInternalServerError, "CountIssues", err) + return + } } } @@ -386,8 +388,9 @@ func ListIssues(ctx *context.APIContext) { } var issueIDs []int64 var labelIDs []int64 + listOptions := utils.GetListOptions(ctx) if len(keyword) > 0 { - issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, []int64{ctx.Repo.Repository.ID}, keyword) + filteredCount, issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, []int64{ctx.Repo.Repository.ID}, keyword, listOptions.Page*listOptions.PageSize, listOptions.PageSize) if err != nil { ctx.Error(http.StatusInternalServerError, "SearchIssuesByKeyword", err) return @@ -432,8 +435,6 @@ func ListIssues(ctx *context.APIContext) { } } - listOptions := utils.GetListOptions(ctx) - var isPull util.OptionalBool switch ctx.FormString("type") { case "pulls": @@ -481,12 +482,14 @@ func ListIssues(ctx *context.APIContext) { return } - issuesOpt.ListOptions = db.ListOptions{ - Page: -1, - } - if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { - ctx.Error(http.StatusInternalServerError, "CountIssues", err) - return + if filteredCount == 0 { + issuesOpt.ListOptions = db.ListOptions{ + Page: -1, + } + if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { + ctx.Error(http.StatusInternalServerError, "CountIssues", err) + return + } } } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 5bff9e67f340a..1757d194215f2 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -182,7 +182,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti var issueIDs []int64 if len(keyword) > 0 { - issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, []int64{repo.ID}, keyword) + // FIXME: we don't know how many opened/closed + _, issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, []int64{repo.ID}, keyword, 0, 50) if err != nil { if issue_indexer.IsAvailable() { ctx.ServerError("issueIndexer.Search", err) @@ -2327,13 +2328,22 @@ func SearchIssues(ctx *context.Context) { var issues []*issues_model.Issue var filteredCount int64 + // this api is also used in UI, + // so the default limit is set to fit UI needs + limit := ctx.FormInt("limit") + if limit == 0 { + limit = setting.UI.IssuePagingNum + } else if limit > setting.API.MaxResponseItems { + limit = setting.API.MaxResponseItems + } + keyword := ctx.FormTrim("q") if strings.IndexByte(keyword, 0) >= 0 { keyword = "" } var issueIDs []int64 if len(keyword) > 0 && len(repoIDs) > 0 { - if issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, repoIDs, keyword); err != nil { + if filteredCount, issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, repoIDs, keyword, ctx.FormInt("page")*limit, limit); err != nil { ctx.Error(http.StatusInternalServerError, "SearchIssuesByKeyword", err.Error()) return } @@ -2363,15 +2373,6 @@ func SearchIssues(ctx *context.Context) { projectID := ctx.FormInt64("project") - // this api is also used in UI, - // so the default limit is set to fit UI needs - limit := ctx.FormInt("limit") - if limit == 0 { - limit = setting.UI.IssuePagingNum - } else if limit > setting.API.MaxResponseItems { - limit = setting.API.MaxResponseItems - } - // Only fetch the issues if we either don't have a keyword or the search returned issues // This would otherwise return all issues if no issues were found by the search. if len(keyword) == 0 || len(issueIDs) > 0 || len(includedLabelNames) > 0 || len(includedMilestones) > 0 { @@ -2417,12 +2418,14 @@ func SearchIssues(ctx *context.Context) { return } - issuesOpt.ListOptions = db.ListOptions{ - Page: -1, - } - if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { - ctx.Error(http.StatusInternalServerError, "CountIssues", err.Error()) - return + if filteredCount == 0 { + issuesOpt.ListOptions = db.ListOptions{ + Page: -1, + } + if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { + ctx.Error(http.StatusInternalServerError, "CountIssues", err.Error()) + return + } } } @@ -2475,10 +2478,15 @@ func ListIssues(ctx *context.Context) { if strings.IndexByte(keyword, 0) >= 0 { keyword = "" } + listOptions := db.ListOptions{ + Page: ctx.FormInt("page"), + PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")), + } + var issueIDs []int64 var labelIDs []int64 if len(keyword) > 0 { - issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, []int64{ctx.Repo.Repository.ID}, keyword) + filteredCount, issueIDs, err = issue_indexer.SearchIssuesByKeyword(ctx, []int64{ctx.Repo.Repository.ID}, keyword, listOptions.Page*listOptions.PageSize, listOptions.PageSize) if err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return @@ -2525,11 +2533,6 @@ func ListIssues(ctx *context.Context) { projectID := ctx.FormInt64("project") - listOptions := db.ListOptions{ - Page: ctx.FormInt("page"), - PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")), - } - var isPull util.OptionalBool switch ctx.FormString("type") { case "pulls": @@ -2578,12 +2581,14 @@ func ListIssues(ctx *context.Context) { return } - issuesOpt.ListOptions = db.ListOptions{ - Page: -1, - } - if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) - return + if filteredCount == 0 { + issuesOpt.ListOptions = db.ListOptions{ + Page: -1, + } + if filteredCount, err = issues_model.CountIssues(ctx, issuesOpt); err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } } } diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 36d9d4f019f7d..697cc3bc579de 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -698,7 +698,7 @@ func issueIDsFromSearch(ctx *context.Context, ctxUser *user_model.User, keyword if err != nil { return nil, fmt.Errorf("GetRepoIDsForIssuesOptions: %w", err) } - issueIDsFromSearch, err := issue_indexer.SearchIssuesByKeyword(ctx, searchRepoIDs, keyword) + _, issueIDsFromSearch, err := issue_indexer.SearchIssuesByKeyword(ctx, searchRepoIDs, keyword, 0, 50) if err != nil { return nil, fmt.Errorf("SearchIssuesByKeyword: %w", err) }