diff --git a/internal/services/pulls.go b/internal/services/pulls.go index 3469caab..d8bb19e3 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -176,24 +176,42 @@ func (s Pulls) List(ctx context.Context, options PullListOptions) ([]Pull, bool, } // ListPage is List that also reports how many pull requests match, when the server counts them -// or every pull request was read for a filter without a server equivalent. +// or every pull request was read for a filter without a server equivalent. Such a filter reads +// pages only until one pull request past the requested page matches. func (s Pulls) ListPage(ctx context.Context, options PullListOptions) (Page[Pull], error) { if !options.filtered() { return s.listPage(ctx, options) } + if options.Limit <= 0 || options.Page <= 0 { + return Page[Pull]{}, errors.New("limit and page must be positive") + } query := options if query.State == "merged" { query.State = "closed" } - all, err := listAll(func(page int) ([]Pull, error) { + matching, wanted := []Pull{}, options.Page*options.Limit+1 + for page := 1; ; page++ { + if page > maxListPages { + return Page[Pull]{}, fmt.Errorf("listing exceeded %d pages", maxListPages) + } query.Limit, query.Page = 50, page window, err := s.listPage(ctx, query) - return window.Items, err - }) - if err != nil { - return Page[Pull]{}, err + if err != nil { + return Page[Pull]{}, err + } + for _, item := range window.Items { + if options.matches(item) { + matching = append(matching, item) + } + } + if len(matching) >= wanted { + items, err := pageOf(matching, options.Limit, options.Page) + return Page[Pull]{Items: items, More: true}, err + } + if len(window.Items) == 0 || (!window.More && len(window.Items) < query.Limit) { + break + } } - matching := slices.DeleteFunc(all, func(item Pull) bool { return !options.matches(item) }) items, err := pageOf(matching, options.Limit, options.Page) if err != nil { return Page[Pull]{}, err diff --git a/internal/services/pulls_test.go b/internal/services/pulls_test.go index 4cd5742d..22bec272 100644 --- a/internal/services/pulls_test.go +++ b/internal/services/pulls_test.go @@ -326,3 +326,40 @@ func TestPullFromToleratesMissingBranches(t *testing.T) { require.Equal(t, Pull{Number: 1, Assignees: []string{}, Labels: []Label{}, Reviewers: []string{}, ReviewerTeams: []string{}}, pullFrom(decode[gitea.PullRequest](t, `{"number":1,"head":null,"base":null}`))) } + +func TestPullClientFiltersStopReadingOncePageIsFull(t *testing.T) { + requests := 0 + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + items := make([]string, 0, 50) + for i := range 50 { + items = append(items, `{"number":`+strconv.Itoa((page-1)*50+i+1)+`,"draft":true}`) + } + w.Header().Set("X-Total-Count", "5000") + _, _ = w.Write([]byte("[" + strings.Join(items, ",") + "]")) + })) + page, err := (Pulls{Client: client, Repository: "alice/project"}).ListPage(context.Background(), PullListOptions{State: "open", Draft: true, Limit: 30, Page: 2}) + require.NoError(t, err) + require.Equal(t, 2, requests) + require.Len(t, page.Items, 30) + require.EqualValues(t, 31, page.Items[0].Number) + require.True(t, page.More) + require.False(t, page.Counted) +} + +func TestPullClientFiltersCountWhenEveryPullWasRead(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("page") != "1" { + _, _ = w.Write([]byte(`[]`)) + return + } + _, _ = w.Write([]byte(`[{"number":1,"draft":true},{"number":2},{"number":3,"draft":true}]`)) + })) + page, err := (Pulls{Client: client, Repository: "alice/project"}).ListPage(context.Background(), PullListOptions{State: "open", Draft: true, Limit: 2, Page: 1}) + require.NoError(t, err) + require.Len(t, page.Items, 2) + require.False(t, page.More) + require.True(t, page.Counted) + require.Equal(t, 2, page.Total) +}