From 7907def88081f519bf3266ca975e80e5d519e434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 18:16:49 +0200 Subject: [PATCH 1/4] api command: add support for REST pagination --- pkg/cmd/api/api.go | 52 ++++++++++++++++++++-- pkg/cmd/api/api_test.go | 98 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index a5540cf2d..dff60245c 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -3,6 +3,7 @@ package api import ( "bytes" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -32,6 +33,7 @@ type ApiOptions struct { RawFields []string RequestHeaders []string ShowResponseHeaders bool + Paginate bool HttpClient func() (*http.Client, error) BaseRepo func() (ghrepo.Interface, error) @@ -93,6 +95,13 @@ Pass "-" to read from standard input. In this mode, parameters specified via opts.RequestPath = args[0] opts.RequestMethodPassed = c.Flags().Changed("method") + if opts.Paginate && !strings.EqualFold(opts.RequestMethod, "GET") && opts.RequestPath != "graphql" { + return &cmdutil.FlagError{Err: errors.New(`the '--paginate' option is not supported for non-GET requests`)} + } + if opts.Paginate && opts.RequestInputFile != "" { + return &cmdutil.FlagError{Err: errors.New(`the '--paginate' option is not supported with '--input'`)} + } + if runF != nil { return runF(&opts) } @@ -105,6 +114,7 @@ Pass "-" to read from standard input. In this mode, parameters specified via cmd.Flags().StringArrayVarP(&opts.RawFields, "raw-field", "f", nil, "Add a string parameter") cmd.Flags().StringArrayVarP(&opts.RequestHeaders, "header", "H", nil, "Add an additional HTTP request header") cmd.Flags().BoolVarP(&opts.ShowResponseHeaders, "include", "i", false, "Include HTTP response headers in the output") + cmd.Flags().BoolVar(&opts.Paginate, "paginate", false, "Make additional HTTP requests to fetch all pages of results") cmd.Flags().StringVar(&opts.RequestInputFile, "input", "", "The file to use as body for the HTTP request") return cmd } @@ -145,11 +155,46 @@ func apiRun(opts *ApiOptions) error { return err } - resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) - if err != nil { - return err + hasNextPage := true + for hasNextPage { + resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) + if err != nil { + return err + } + + err = processResponse(resp, opts) + if err != nil { + return err + } + + if !opts.Paginate { + break + } + requestPath, hasNextPage = findNextPage(resp) + + if hasNextPage && opts.ShowResponseHeaders { + fmt.Fprint(opts.IO.Out, "\n") + } } + return nil +} + +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) + +func findNextPage(resp *http.Response) (string, bool) { + for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { + if len(m) < 2 { + continue + } + if m[2] == "next" { + return m[1], true + } + } + return "", false +} + +func processResponse(resp *http.Response, opts *ApiOptions) error { if opts.ShowResponseHeaders { fmt.Fprintln(opts.IO.Out, resp.Proto, resp.Status) printHeaders(opts.IO.Out, resp.Header, opts.IO.ColorEnabled()) @@ -164,6 +209,7 @@ func apiRun(opts *ApiOptions) error { isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type")) + var err error var serverError string if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) { responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 605e4bf9e..c83cf3b5a 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -36,6 +36,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -51,6 +52,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -66,6 +68,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string{"body=@file.txt"}, RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -81,9 +84,52 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string{"accept: text/plain"}, ShowResponseHeaders: true, + Paginate: false, }, wantsErr: false, }, + { + name: "with pagination", + cli: "repos/OWNER/REPO/issues --paginate", + wants: ApiOptions{ + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "repos/OWNER/REPO/issues", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: true, + }, + wantsErr: false, + }, + { + name: "POST pagination", + cli: "-XPOST repos/OWNER/REPO/issues --paginate", + wantsErr: true, + }, + { + name: "GraphQL pagination", + cli: "-XPOST graphql --paginate", + wants: ApiOptions{ + RequestMethod: "POST", + RequestMethodPassed: true, + RequestPath: "graphql", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: true, + }, + wantsErr: false, + }, + { + name: "input pagination", + cli: "--input repos/OWNER/REPO/issues --paginate", + wantsErr: true, + }, { name: "with request body from file", cli: "user --input myfile", @@ -96,6 +142,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -246,6 +293,57 @@ func Test_apiRun(t *testing.T) { } } +func Test_apiRun_pagination(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + + requestCount := 0 + responses := []*http.Response{ + { + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"page":1}`)), + Header: http.Header{ + "Link": []string{`; rel="next", ; rel="last"`}, + }, + }, + { + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"page":2}`)), + Header: http.Header{ + "Link": []string{`; rel="next", ; rel="last"`}, + }, + }, + { + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"page":3}`)), + Header: http.Header{}, + }, + } + + options := ApiOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + var tr roundTripper = func(req *http.Request) (*http.Response, error) { + resp := responses[requestCount] + resp.Request = req + requestCount++ + return resp, nil + } + return &http.Client{Transport: tr}, nil + }, + + Paginate: true, + } + + err := apiRun(&options) + assert.NoError(t, err) + + assert.Equal(t, `{"page":1}{"page":2}{"page":3}`, stdout.String(), "stdout") + assert.Equal(t, "", stderr.String(), "stderr") + + assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=2", responses[1].Request.URL.String()) + assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=3", responses[2].Request.URL.String()) +} + func Test_apiRun_inputFile(t *testing.T) { tests := []struct { name string From 3f940c98f1872d439109b4363139fe59b6097ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 18:19:39 +0200 Subject: [PATCH 2/4] Add assertion for 1st api request before pagination --- pkg/cmd/api/api_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index c83cf3b5a..9debf7896 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -331,7 +331,8 @@ func Test_apiRun_pagination(t *testing.T) { return &http.Client{Transport: tr}, nil }, - Paginate: true, + RequestPath: "issues", + Paginate: true, } err := apiRun(&options) @@ -340,6 +341,7 @@ func Test_apiRun_pagination(t *testing.T) { assert.Equal(t, `{"page":1}{"page":2}{"page":3}`, stdout.String(), "stdout") assert.Equal(t, "", stderr.String(), "stderr") + assert.Equal(t, "https://api.github.com/issues", responses[0].Request.URL.String()) assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=2", responses[1].Request.URL.String()) assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=3", responses[2].Request.URL.String()) } From f4ecd365a69d3491e82d52eb16b10aee66d75166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 17 Jun 2020 18:02:20 +0200 Subject: [PATCH 3/4] api command: add GraphQL support for `--paginate` --- pkg/cmd/api/api.go | 76 +++++++++++++-------- pkg/cmd/api/api_test.go | 81 +++++++++++++++++++++- pkg/cmd/api/pagination.go | 87 ++++++++++++++++++++++++ pkg/cmd/api/pagination_test.go | 118 +++++++++++++++++++++++++++++++++ 4 files changed, 335 insertions(+), 27 deletions(-) create mode 100644 pkg/cmd/api/pagination.go create mode 100644 pkg/cmd/api/pagination_test.go diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index dff60245c..067ef6bc8 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -76,7 +76,11 @@ on the format of the value: Raw request body may be passed from the outside via a file specified by '--input'. Pass "-" to read from standard input. In this mode, parameters specified via '--field' flags are serialized into URL query parameters. -`, + +In '--paginate' mode, all pages of results will sequentially be requested until +there are no more pages of results. For GraphQL requests, this requires that the +original query accepts an '$endCursor: String' variable and that it fetches the +'pageInfo{ hasNextPage, endCursor }' set of fields from a collection.`, Example: heredoc.Doc(` $ gh api repos/:owner/:repo/releases @@ -89,6 +93,20 @@ Pass "-" to read from standard input. In this mode, parameters specified via } } ' + + $ gh api graphql --paginate -f query=' + query($endCursor: String) { + viewer { + repositories(first: 100, after: $endCursor) { + nodes { nameWithOwner } + pageInfo { + hasNextPage + endCursor + } + } + } + } + ' `), Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { @@ -162,7 +180,7 @@ func apiRun(opts *ApiOptions) error { return err } - err = processResponse(resp, opts) + endCursor, err := processResponse(resp, opts) if err != nil { return err } @@ -170,7 +188,15 @@ func apiRun(opts *ApiOptions) error { if !opts.Paginate { break } - requestPath, hasNextPage = findNextPage(resp) + + if opts.RequestPath == "graphql" { + hasNextPage = endCursor != "" + if hasNextPage { + params["endCursor"] = endCursor + } + } else { + requestPath, hasNextPage = findNextPage(resp) + } if hasNextPage && opts.ShowResponseHeaders { fmt.Fprint(opts.IO.Out, "\n") @@ -180,21 +206,7 @@ func apiRun(opts *ApiOptions) error { return nil } -var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) - -func findNextPage(resp *http.Response) (string, bool) { - for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { - if len(m) < 2 { - continue - } - if m[2] == "next" { - return m[1], true - } - } - return "", false -} - -func processResponse(resp *http.Response, opts *ApiOptions) error { +func processResponse(resp *http.Response, opts *ApiOptions) (endCursor string, err error) { if opts.ShowResponseHeaders { fmt.Fprintln(opts.IO.Out, resp.Proto, resp.Status) printHeaders(opts.IO.Out, resp.Header, opts.IO.ColorEnabled()) @@ -202,43 +214,55 @@ func processResponse(resp *http.Response, opts *ApiOptions) error { } if resp.StatusCode == 204 { - return nil + return } var responseBody io.Reader = resp.Body defer resp.Body.Close() isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type")) - var err error var serverError string if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) { responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode) if err != nil { - return err + return } } + var bodyCopy *bytes.Buffer + isGraphQLPaginate := isJSON && resp.StatusCode == 200 && opts.Paginate && opts.RequestPath == "graphql" + if isGraphQLPaginate { + bodyCopy = &bytes.Buffer{} + responseBody = io.TeeReader(responseBody, bodyCopy) + } + if isJSON && opts.IO.ColorEnabled() { err = jsoncolor.Write(opts.IO.Out, responseBody, " ") if err != nil { - return err + return } } else { _, err = io.Copy(opts.IO.Out, responseBody) if err != nil { - return err + return } } if serverError != "" { fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", serverError) - return cmdutil.SilentError + err = cmdutil.SilentError + return } else if resp.StatusCode > 299 { fmt.Fprintf(opts.IO.ErrOut, "gh: HTTP %d\n", resp.StatusCode) - return cmdutil.SilentError + err = cmdutil.SilentError + return } - return nil + if isGraphQLPaginate { + endCursor = findEndCursor(bodyCopy) + } + + return } var placeholderRE = regexp.MustCompile(`\:(owner|repo)\b`) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 9debf7896..dc7a44e41 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "encoding/json" "fmt" "io/ioutil" "net/http" @@ -13,6 +14,7 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_NewCmdApi(t *testing.T) { @@ -293,7 +295,7 @@ func Test_apiRun(t *testing.T) { } } -func Test_apiRun_pagination(t *testing.T) { +func Test_apiRun_paginationREST(t *testing.T) { io, _, stdout, stderr := iostreams.Test() requestCount := 0 @@ -346,6 +348,83 @@ func Test_apiRun_pagination(t *testing.T) { assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=3", responses[2].Request.URL.String()) } +func Test_apiRun_paginationGraphQL(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + + requestCount := 0 + responses := []*http.Response{ + { + StatusCode: 200, + Header: http.Header{"Content-Type": []string{`application/json`}}, + Body: ioutil.NopCloser(bytes.NewBufferString(`{ + "data": { + "nodes": ["page one"], + "pageInfo": { + "endCursor": "PAGE1_END", + "hasNextPage": true + } + } + }`)), + }, + { + StatusCode: 200, + Header: http.Header{"Content-Type": []string{`application/json`}}, + Body: ioutil.NopCloser(bytes.NewBufferString(`{ + "data": { + "nodes": ["page two"], + "pageInfo": { + "endCursor": "PAGE2_END", + "hasNextPage": false + } + } + }`)), + }, + } + + options := ApiOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + var tr roundTripper = func(req *http.Request) (*http.Response, error) { + resp := responses[requestCount] + resp.Request = req + requestCount++ + return resp, nil + } + return &http.Client{Transport: tr}, nil + }, + + RequestMethod: "POST", + RequestPath: "graphql", + Paginate: true, + } + + err := apiRun(&options) + require.NoError(t, err) + + assert.Contains(t, stdout.String(), `"page one"`) + assert.Contains(t, stdout.String(), `"page two"`) + assert.Equal(t, "", stderr.String(), "stderr") + + var requestData struct { + Variables map[string]interface{} + } + + bb, err := ioutil.ReadAll(responses[0].Request.Body) + require.NoError(t, err) + err = json.Unmarshal(bb, &requestData) + require.NoError(t, err) + _, hasCursor := requestData.Variables["endCursor"].(string) + assert.Equal(t, false, hasCursor) + + bb, err = ioutil.ReadAll(responses[1].Request.Body) + require.NoError(t, err) + err = json.Unmarshal(bb, &requestData) + require.NoError(t, err) + endCursor, hasCursor := requestData.Variables["endCursor"].(string) + assert.Equal(t, true, hasCursor) + assert.Equal(t, "PAGE1_END", endCursor) +} + func Test_apiRun_inputFile(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/api/pagination.go b/pkg/cmd/api/pagination.go new file mode 100644 index 000000000..3bd00b822 --- /dev/null +++ b/pkg/cmd/api/pagination.go @@ -0,0 +1,87 @@ +package api + +import ( + "encoding/json" + "io" + "net/http" + "regexp" +) + +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) + +func findNextPage(resp *http.Response) (string, bool) { + for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { + if len(m) >= 2 && m[2] == "next" { + return m[1], true + } + } + return "", false +} + +func findEndCursor(r io.Reader) string { + dec := json.NewDecoder(r) + + var idx int + var stack []json.Delim + var lastKey string + var contextKey string + + var endCursor string + var hasNextPage bool + var foundEndCursor bool + var foundNextPage bool + +loop: + for { + t, err := dec.Token() + if err == io.EOF { + break + } + if err != nil { + return "" + } + + switch tt := t.(type) { + case json.Delim: + switch tt { + case '{', '[': + stack = append(stack, tt) + contextKey = lastKey + idx = 0 + case '}', ']': + stack = stack[:len(stack)-1] + contextKey = "" + idx = 0 + } + default: + isKey := len(stack) > 0 && stack[len(stack)-1] == '{' && idx%2 == 0 + idx++ + + switch tt := t.(type) { + case string: + if isKey { + lastKey = tt + } else if contextKey == "pageInfo" && lastKey == "endCursor" { + endCursor = tt + foundEndCursor = true + if foundNextPage { + break loop + } + } + case bool: + if contextKey == "pageInfo" && lastKey == "hasNextPage" { + hasNextPage = tt + foundNextPage = true + if foundEndCursor { + break loop + } + } + } + } + } + + if hasNextPage { + return endCursor + } + return "" +} diff --git a/pkg/cmd/api/pagination_test.go b/pkg/cmd/api/pagination_test.go new file mode 100644 index 000000000..0ed4566da --- /dev/null +++ b/pkg/cmd/api/pagination_test.go @@ -0,0 +1,118 @@ +package api + +import ( + "bytes" + "io" + "net/http" + "testing" +) + +func Test_findNextPage(t *testing.T) { + tests := []struct { + name string + resp *http.Response + want string + want1 bool + }{ + { + name: "no Link header", + resp: &http.Response{}, + want: "", + want1: false, + }, + { + name: "no next page in Link", + resp: &http.Response{ + Header: http.Header{ + "Link": []string{`; rel="last"`}, + }, + }, + want: "", + want1: false, + }, + { + name: "has next page", + resp: &http.Response{ + Header: http.Header{ + "Link": []string{`; rel="next", ; rel="last"`}, + }, + }, + want: "https://api.github.com/issues?page=2", + want1: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got1 := findNextPage(tt.resp) + if got != tt.want { + t.Errorf("findNextPage() got = %v, want %v", got, tt.want) + } + if got1 != tt.want1 { + t.Errorf("findNextPage() got1 = %v, want %v", got1, tt.want1) + } + }) + } +} + +func Test_findEndCursor(t *testing.T) { + tests := []struct { + name string + json io.Reader + want string + }{ + { + name: "blank", + json: bytes.NewBufferString(`{}`), + want: "", + }, + { + name: "unrelated fields", + json: bytes.NewBufferString(`{ + "hasNextPage": true, + "endCursor": "THE_END" + }`), + want: "", + }, + { + name: "has next page", + json: bytes.NewBufferString(`{ + "pageInfo": { + "hasNextPage": true, + "endCursor": "THE_END" + } + }`), + want: "THE_END", + }, + { + name: "more pageInfo blocks", + json: bytes.NewBufferString(`{ + "pageInfo": { + "hasNextPage": true, + "endCursor": "THE_END" + }, + "pageInfo": { + "hasNextPage": true, + "endCursor": "NOT_THIS" + } + }`), + want: "THE_END", + }, + { + name: "no next page", + json: bytes.NewBufferString(`{ + "pageInfo": { + "hasNextPage": false, + "endCursor": "THE_END" + } + }`), + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := findEndCursor(tt.json); got != tt.want { + t.Errorf("findEndCursor() = %v, want %v", got, tt.want) + } + }) + } +} From c945fb4336ff6d6229bed30eb68d946deb110320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 23 Jun 2020 18:42:57 +0200 Subject: [PATCH 4/4] Automatically add `per_page=100` to paginated REST requests Most endpoints respect this parameter by default. Those that don't will just ignore it. The `per_page=100` parameter is not added if there is already a `per_page` parameter specified in the request. --- pkg/cmd/api/api.go | 7 ++++- pkg/cmd/api/api_test.go | 2 +- pkg/cmd/api/pagination.go | 21 ++++++++++++++ pkg/cmd/api/pagination_test.go | 51 ++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 067ef6bc8..e1edabbc0 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -143,6 +143,7 @@ func apiRun(opts *ApiOptions) error { return err } + isGraphQL := opts.RequestPath == "graphql" requestPath, err := fillPlaceholders(opts.RequestPath, opts) if err != nil { return fmt.Errorf("unable to expand placeholder in path: %w", err) @@ -155,6 +156,10 @@ func apiRun(opts *ApiOptions) error { method = "POST" } + if opts.Paginate && !isGraphQL { + requestPath = addPerPage(requestPath, 100, params) + } + if opts.RequestInputFile != "" { file, size, err := openUserFile(opts.RequestInputFile, opts.IO.In) if err != nil { @@ -189,7 +194,7 @@ func apiRun(opts *ApiOptions) error { break } - if opts.RequestPath == "graphql" { + if isGraphQL { hasNextPage = endCursor != "" if hasNextPage { params["endCursor"] = endCursor diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index dc7a44e41..7b89d715c 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -343,7 +343,7 @@ func Test_apiRun_paginationREST(t *testing.T) { assert.Equal(t, `{"page":1}{"page":2}{"page":3}`, stdout.String(), "stdout") assert.Equal(t, "", stderr.String(), "stderr") - assert.Equal(t, "https://api.github.com/issues", responses[0].Request.URL.String()) + assert.Equal(t, "https://api.github.com/issues?per_page=100", responses[0].Request.URL.String()) assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=2", responses[1].Request.URL.String()) assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=3", responses[2].Request.URL.String()) } diff --git a/pkg/cmd/api/pagination.go b/pkg/cmd/api/pagination.go index 3bd00b822..fce88fb92 100644 --- a/pkg/cmd/api/pagination.go +++ b/pkg/cmd/api/pagination.go @@ -2,9 +2,12 @@ package api import ( "encoding/json" + "fmt" "io" "net/http" + "net/url" "regexp" + "strings" ) var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) @@ -85,3 +88,21 @@ loop: } return "" } + +func addPerPage(p string, perPage int, params map[string]interface{}) string { + if _, hasPerPage := params["per_page"]; hasPerPage { + return p + } + + idx := strings.IndexRune(p, '?') + sep := "?" + + if idx >= 0 { + if qp, err := url.ParseQuery(p[idx+1:]); err == nil && qp.Get("per_page") != "" { + return p + } + sep = "&" + } + + return fmt.Sprintf("%s%sper_page=%d", p, sep, perPage) +} diff --git a/pkg/cmd/api/pagination_test.go b/pkg/cmd/api/pagination_test.go index 0ed4566da..3bb1a8ec5 100644 --- a/pkg/cmd/api/pagination_test.go +++ b/pkg/cmd/api/pagination_test.go @@ -116,3 +116,54 @@ func Test_findEndCursor(t *testing.T) { }) } } + +func Test_addPerPage(t *testing.T) { + type args struct { + p string + perPage int + params map[string]interface{} + } + tests := []struct { + name string + args args + want string + }{ + { + name: "adds per_page", + args: args{ + p: "items", + perPage: 13, + params: nil, + }, + want: "items?per_page=13", + }, + { + name: "avoids adding per_page if already in params", + args: args{ + p: "items", + perPage: 13, + params: map[string]interface{}{ + "state": "open", + "per_page": 99, + }, + }, + want: "items", + }, + { + name: "avoids adding per_page if already in query", + args: args{ + p: "items?per_page=6&state=open", + perPage: 13, + params: nil, + }, + want: "items?per_page=6&state=open", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := addPerPage(tt.args.p, tt.args.perPage, tt.args.params); got != tt.want { + t.Errorf("addPerPage() = %v, want %v", got, tt.want) + } + }) + } +}