STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Resolve PR comments

Browse source
This commit is contained in:
Heath Stewart 2024-02-15 00:59:12 -08:00
parent 0dfe6ec4b4
commit f41876d64c
No known key found for this signature in database
4 changed files with 283 additions and 181 deletions
Download patch file Download diff file Expand all files Collapse all files

19
internal/jsonmerge/nop.go Normal file
View file

@ -0,0 +1,19 @@
package jsonmerge
import (
"io"
)
type nopMerger struct{}
func NewNopMerger() Merger {
return &nopMerger{}
}
func (m *nopMerger) NewPage(r io.Reader, _ bool) io.ReadCloser {
return io.NopCloser(r)
}
func (m *nopMerger) Close() error {
return nil
}

114
internal/jsonmerge/nop_test.go Normal file
View file

@ -0,0 +1,114 @@
package jsonmerge
import (
"bytes"
"io"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNopMerger_nothingWritten(t *testing.T) {
w := &bytes.Buffer{}
merger := NewNopMerger()
require.NoError(t, merger.Close())
assert.Equal(t, ``, w.String())
}
func TestNopMerger_singleEmptyObject(t *testing.T) {
w := &bytes.Buffer{}
merger := NewNopMerger()
r1 := bytes.NewBufferString(`{}`)
p1 := merger.NewPage(r1, true)
n, err := io.Copy(w, p1)
require.NoError(t, err)
assert.Equal(t, int64(2), n)
assert.NoError(t, p1.Close())
assert.Equal(t, `{}`, w.String())
require.NoError(t, merger.Close())
assert.Equal(t, `{}`, w.String())
}
func TestNopMerger_finalEmptyObject(t *testing.T) {
w := &bytes.Buffer{}
merger := NewNopMerger()
r1 := bytes.NewBufferString(`{"a":1,"b":2}`)
p1 := merger.NewPage(r1, false)
n, err := io.Copy(w, p1)
require.NoError(t, err)
assert.Equal(t, int64(13), n)
assert.NoError(t, p1.Close())
assert.Equal(t, `{"a":1,"b":2}`, w.String())
r2 := bytes.NewBufferString(`{"c":3}`)
p2 := merger.NewPage(r2, true)
n, err = io.Copy(w, p2)
require.NoError(t, err)
assert.Equal(t, int64(7), n)
assert.NoError(t, p2.Close())
assert.Equal(t, `{"a":1,"b":2}{"c":3}`, w.String())
require.NoError(t, merger.Close())
assert.Equal(t, `{"a":1,"b":2}{"c":3}`, w.String())
}
func TestNopMerger_invalidJSON(t *testing.T) {
w := &bytes.Buffer{}
merger := NewNopMerger()
r1 := bytes.NewBufferString(`invalid`)
p1 := merger.NewPage(r1, true)
n, err := io.Copy(w, p1)
require.NoError(t, err)
assert.Equal(t, int64(7), n)
assert.NoError(t, p1.Close())
assert.Equal(t, `invalid`, w.String())
require.NoError(t, merger.Close())
assert.Equal(t, `invalid`, w.String())
}
func TestNopMerger_singleEmptyArray(t *testing.T) {
w := &bytes.Buffer{}
merger := NewNopMerger()
r1 := bytes.NewBufferString(`[]`)
p1 := merger.NewPage(r1, true)
n, err := io.Copy(w, p1)
require.NoError(t, err)
assert.Equal(t, int64(2), n)
assert.NoError(t, p1.Close())
assert.Equal(t, `[]`, w.String())
require.NoError(t, merger.Close())
assert.Equal(t, `[]`, w.String())
}
func TestNopMerger_finalEmptyArray(t *testing.T) {
w := &bytes.Buffer{}
merger := NewNopMerger()
r1 := bytes.NewBufferString(`["a","b"]`)
p1 := merger.NewPage(r1, false)
n, err := io.Copy(w, p1)
require.NoError(t, err)
assert.Equal(t, int64(9), n)
assert.NoError(t, p1.Close())
assert.Equal(t, `["a","b"]`, w.String())
r2 := bytes.NewBufferString(`[]`)
p2 := merger.NewPage(r2, true)
n, err = io.Copy(w, p2)
require.NoError(t, err)
assert.Equal(t, int64(2), n)
assert.NoError(t, p2.Close())
assert.Equal(t, `["a","b"][]`, w.String())
require.NoError(t, merger.Close())
assert.Equal(t, `["a","b"][]`, w.String())
}

38
pkg/cmd/api/api.go
View file

@ -159,7 +159,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command
'
# list all repositories for a user
$ gh api graphql --paginate --merge-pages -f query='
$ gh api graphql --paginate -f query='
query($endCursor: String) {
viewer {
repositories(first: 100, after: $endCursor) {
@ -172,6 +172,23 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command
}
}
'
# get the percentage of forks for the current user
# without --merge-pages you will get a different percentage for each page
$ gh api graphql --paginate --merge-pages -f query='
query($endCursor: String) {
viewer {
repositories(first: 100, after: $endCursor) {
nodes { isFork }
pageInfo {
hasNextPage
endCursor
}
}
}
}
' | jq 'def count(s): reduce s as $_ (0;.+1);
.data.viewer.repositories.nodes as $r | count(select($r[].isFork)) / count($r[])'
`),
Annotations: map[string]string{
"help:environment": heredoc.Doc(`
@ -292,14 +309,19 @@ func apiRun(opts *ApiOptions) error {
// Merge JSON arrays and objects if paginating without filtering or templating.
// MergePages retains compatibility with older versions but may be the default behavior with future major releases.
if opts.Paginate && opts.MergePages && opts.FilterOutput == "" && opts.Template == "" {
if isGraphQL {
opts.merger = jsonmerge.NewObjectMerger(bodyWriter)
} else {
if opts.Paginate && opts.FilterOutput == "" && opts.Template == "" {
if !isGraphQL {
opts.merger = jsonmerge.NewArrayMerger()
} else if opts.MergePages {
opts.merger = jsonmerge.NewObjectMerger(bodyWriter)
}
}
if opts.merger == nil {
opts.merger = jsonmerge.NewNopMerger()
}
defer opts.merger.Close()
if opts.RequestInputFile != "" {
file, size, err := openUserFile(opts.RequestInputFile, opts.IO.In)
if err != nil {
@ -400,10 +422,6 @@ func apiRun(opts *ApiOptions) error {
}
}
if opts.merger != nil {
return opts.merger.Close()
}
return tmpl.Flush()
}
@ -455,7 +473,7 @@ func processResponse(resp *http.Response, opts *ApiOptions, bodyWriter, headersW
} else if isJSON && opts.IO.ColorEnabled() {
err = jsoncolor.Write(bodyWriter, responseBody, " ")
} else {
if isJSON && opts.merger != nil && !opts.ShowResponseHeaders {
if isJSON && opts.Paginate && !opts.ShowResponseHeaders {
responseBody = opts.merger.NewPage(responseBody, isLastPage)
}

293
pkg/cmd/api/api_test.go
View file

@ -686,7 +686,7 @@ func Test_apiRun_paginationREST(t *testing.T) {
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":1}]`)),
Body: io.NopCloser(bytes.NewBufferString(`{"page":1}`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=2>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
@ -697,7 +697,7 @@ func Test_apiRun_paginationREST(t *testing.T) {
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":2}]`)),
Body: io.NopCloser(bytes.NewBufferString(`{"page":2}`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=3>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
@ -708,7 +708,7 @@ func Test_apiRun_paginationREST(t *testing.T) {
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":3}]`)),
Body: io.NopCloser(bytes.NewBufferString(`{"page":3}`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"X-Github-Request-Id": []string{"3"},
@ -741,153 +741,7 @@ func Test_apiRun_paginationREST(t *testing.T) {
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/issues?page=1&per_page=50", 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())
}
func Test_apiRun_paginationREST_merge(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
requestCount := 0
responses := []*http.Response{
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":1}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=2>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
"X-Github-Request-Id": []string{"1"},
},
},
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":2}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=3>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
"X-Github-Request-Id": []string{"2"},
},
},
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":3}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"X-Github-Request-Id": []string{"3"},
},
},
}
options := ApiOptions{
IO: ios,
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
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
RequestMethod: "GET",
RequestMethodPassed: true,
RequestPath: "issues",
Paginate: true,
MergePages: true,
RawFields: []string{"per_page=50", "page=1"},
}
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/issues?page=1&per_page=50", 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())
}
func Test_apiRun_paginationREST_with_headers(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
requestCount := 0
responses := []*http.Response{
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":1}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=2>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
"X-Github-Request-Id": []string{"1"},
},
},
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":2}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=3>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
"X-Github-Request-Id": []string{"2"},
},
},
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"page":3}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"X-Github-Request-Id": []string{"3"},
},
},
}
options := ApiOptions{
IO: ios,
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
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
RequestMethod: "GET",
RequestMethodPassed: true,
RequestPath: "issues",
Paginate: true,
RawFields: []string{"per_page=50", "page=1"},
ShowResponseHeaders: true,
}
err := apiRun(&options)
assert.NoError(t, err)
assert.Equal(t, "HTTP/1.1 200 OK\nContent-Type: application/json\r\nLink: <https://api.github.com/repositories/1227/issues?page=2>; rel=\"next\", <https://api.github.com/repositories/1227/issues?page=3>; rel=\"last\"\r\nX-Github-Request-Id: 1\r\n\r\n[{\"page\":1}]\nHTTP/1.1 200 OK\nContent-Type: application/json\r\nLink: <https://api.github.com/repositories/1227/issues?page=3>; rel=\"next\", <https://api.github.com/repositories/1227/issues?page=3>; rel=\"last\"\r\nX-Github-Request-Id: 2\r\n\r\n[{\"page\":2}]\nHTTP/1.1 200 OK\nContent-Type: application/json\r\nX-Github-Request-Id: 3\r\n\r\n[{\"page\":3}]", stdout.String(), "stdout")
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?page=1&per_page=50", responses[0].Request.URL.String())
@ -959,7 +813,7 @@ func Test_apiRun_arrayPaginationREST(t *testing.T) {
err := apiRun(&options)
assert.NoError(t, err)
assert.Equal(t, `[{"item":1},{"item":2}][{"item":3},{"item":4}][{"item":5}][]`, stdout.String(), "stdout")
assert.Equal(t, `[{"item":1},{"item":2},{"item":3},{"item":4},{"item":5} ]`, stdout.String(), "stdout")
assert.Equal(t, "", stderr.String(), "stderr")
assert.Equal(t, "https://api.github.com/issues?page=1&per_page=50", responses[0].Request.URL.String())
@ -967,41 +821,41 @@ func Test_apiRun_arrayPaginationREST(t *testing.T) {
assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=3", responses[2].Request.URL.String())
}
func Test_apiRun_arrayPaginationREST_merge(t *testing.T) {
func Test_apiRun_arrayPaginationREST_with_headers(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(false)
requestCount := 0
responses := []*http.Response{
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"item":1},{"item":2}]`)),
Body: io.NopCloser(bytes.NewBufferString(`[{"page":1}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=2>; rel="next", <https://api.github.com/repositories/1227/issues?page=4>; rel="last"`},
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=2>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
"X-Github-Request-Id": []string{"1"},
},
},
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"item":3},{"item":4}]`)),
Body: io.NopCloser(bytes.NewBufferString(`[{"page":2}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=3>; rel="next", <https://api.github.com/repositories/1227/issues?page=4>; rel="last"`},
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=3>; rel="next", <https://api.github.com/repositories/1227/issues?page=3>; rel="last"`},
"X-Github-Request-Id": []string{"2"},
},
},
{
Proto: "HTTP/1.1",
Status: "200 OK",
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[{"item":5}]`)),
Body: io.NopCloser(bytes.NewBufferString(`[{"page":3}]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Link": []string{`<https://api.github.com/repositories/1227/issues?page=4>; rel="next", <https://api.github.com/repositories/1227/issues?page=4>; rel="last"`},
},
},
{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`[]`)),
Header: http.Header{
"Content-Type": []string{"application/json"},
"Content-Type": []string{"application/json"},
"X-Github-Request-Id": []string{"3"},
},
},
}
@ -1025,14 +879,14 @@ func Test_apiRun_arrayPaginationREST_merge(t *testing.T) {
RequestMethodPassed: true,
RequestPath: "issues",
Paginate: true,
MergePages: true,
RawFields: []string{"per_page=50", "page=1"},
ShowResponseHeaders: true,
}
err := apiRun(&options)
assert.NoError(t, err)
assert.Equal(t, `[{"item":1},{"item":2},{"item":3},{"item":4},{"item":5} ]`, stdout.String(), "stdout")
assert.Equal(t, "HTTP/1.1 200 OK\nContent-Type: application/json\r\nLink: <https://api.github.com/repositories/1227/issues?page=2>; rel=\"next\", <https://api.github.com/repositories/1227/issues?page=3>; rel=\"last\"\r\nX-Github-Request-Id: 1\r\n\r\n[{\"page\":1}]\nHTTP/1.1 200 OK\nContent-Type: application/json\r\nLink: <https://api.github.com/repositories/1227/issues?page=3>; rel=\"next\", <https://api.github.com/repositories/1227/issues?page=3>; rel=\"last\"\r\nX-Github-Request-Id: 2\r\n\r\n[{\"page\":2}]\nHTTP/1.1 200 OK\nContent-Type: application/json\r\nX-Github-Request-Id: 3\r\n\r\n[{\"page\":3}]", stdout.String(), "stdout")
assert.Equal(t, "", stderr.String(), "stderr")
assert.Equal(t, "https://api.github.com/issues?page=1&per_page=50", responses[0].Request.URL.String())
@ -1233,6 +1087,103 @@ func Test_apiRun_paginationGraphQL_merge(t *testing.T) {
assert.Equal(t, "PAGE1_END", endCursor)
}
func Test_apiRun_paginationGraphQL_merge_tty(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
// Backcompat: setting color-enabled disables merging, but not TTY.
ios.SetStdoutTTY(true)
requestCount := 0
responses := []*http.Response{
{
StatusCode: 200,
Header: http.Header{"Content-Type": []string{`application/json`}},
Body: io.NopCloser(bytes.NewBufferString(heredoc.Doc(`
{
"data": {
"nodes": ["page one"],
"pageInfo": {
"endCursor": "PAGE1_END",
"hasNextPage": true
}
}
}`))),
},
{
StatusCode: 200,
Header: http.Header{"Content-Type": []string{`application/json`}},
Body: io.NopCloser(bytes.NewBufferString(heredoc.Doc(`
{
"data": {
"nodes": ["page two"],
"pageInfo": {
"endCursor": "PAGE2_END",
"hasNextPage": false
}
}
}`))),
},
}
options := ApiOptions{
IO: ios,
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
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
RawFields: []string{"foo=bar"},
RequestMethod: "POST",
RequestPath: "graphql",
Paginate: true,
MergePages: true,
}
err := apiRun(&options)
require.NoError(t, err)
assert.JSONEq(t, stdout.String(), `{
"data": {
"nodes": [
"page one",
"page two"
],
"pageInfo": {
"endCursor": "PAGE2_END",
"hasNextPage": false
}
}
}`)
assert.Equal(t, "", stderr.String(), "stderr")
var requestData struct {
Variables map[string]interface{}
}
bb, err := io.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 = io.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_paginated_template(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(true)