diff --git a/go.mod b/go.mod index 9381694c1..ba96c7cb0 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/cli/cli/v2 go 1.22 require ( - dario.cat/mergo v1.0.0 github.com/AlecAivazis/survey/v2 v2.3.7 github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.18.1 diff --git a/go.sum b/go.sum index bfd00f3a8..1c759cbeb 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,6 @@ cloud.google.com/go/iam v1.1.5 h1:1jTsCu4bcsNsE4iiqNT5SHwrDRCfRmIaaaVFhRveTJI= cloud.google.com/go/iam v1.1.5/go.mod h1:rB6P/Ic3mykPbFio+vo7403drjlgvoWfYpJhMXEbzv8= cloud.google.com/go/kms v1.15.5 h1:pj1sRfut2eRbD9pFRjNnPNg/CzJPuQAzUujMIM1vVeM= cloud.google.com/go/kms v1.15.5/go.mod h1:cU2H5jnp6G2TDpUGZyqTCoy1n16fbubHZjmVXSMtwDI= -dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= -dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= github.com/AdamKorcz/go-fuzz-headers-1 v0.0.0-20230919221257-8b5d3ce2d11d h1:zjqpY4C7H15HjRPEenkS4SAn3Jy2eRRjkjZbGR30TOg= diff --git a/internal/jsonmerge/array.go b/internal/jsonmerge/array.go deleted file mode 100644 index 3a8c2c744..000000000 --- a/internal/jsonmerge/array.go +++ /dev/null @@ -1,76 +0,0 @@ -package jsonmerge - -import "io" - -type arrayMerger struct { - isFirstPage bool -} - -// NewArrayMerger creates a Merger for JSON arrays. -func NewArrayMerger() Merger { - return &arrayMerger{ - isFirstPage: true, - } -} - -func (merger *arrayMerger) NewPage(r io.Reader, isLastPage bool) io.ReadCloser { - return &arrayMergerPage{ - merger: merger, - Reader: r, - isLastPage: isLastPage, - } -} - -func (m *arrayMerger) Close() error { - // arrayMerger merges when reading, so any output was already written - // and there's nothing to do on Close. - return nil -} - -type arrayMergerPage struct { - merger *arrayMerger - - io.Reader - isLastPage bool - - isSubsequentRead bool - cachedByte byte -} - -func (page *arrayMergerPage) Read(p []byte) (int, error) { - var n int - var err error - - if page.cachedByte != 0 && len(p) > 0 { - p[0] = page.cachedByte - n, err = page.Reader.Read(p[1:]) - n += 1 - page.cachedByte = 0 - } else { - n, err = page.Reader.Read(p) - } - - if !page.isSubsequentRead && !page.merger.isFirstPage && n > 0 && p[0] == '[' { - if n > 1 && p[1] == ']' { - // Empty array case. - p[0] = ' ' - } else { - // Avoid starting a new array and continue with a comma instead. - p[0] = ',' - } - } - - if !page.isLastPage && n > 0 && p[n-1] == ']' { - // Avoid closing off an array in case we determine we are at EOF. - page.cachedByte = p[n-1] - n -= 1 - } - - page.isSubsequentRead = true - return n, err -} - -func (page *arrayMergerPage) Close() error { - page.merger.isFirstPage = false - return nil -} diff --git a/internal/jsonmerge/array_test.go b/internal/jsonmerge/array_test.go deleted file mode 100644 index 8106ded30..000000000 --- a/internal/jsonmerge/array_test.go +++ /dev/null @@ -1,86 +0,0 @@ -package jsonmerge - -import ( - "bytes" - "io" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestArrayMerger_singleEmptyArray(t *testing.T) { - merger := NewArrayMerger() - w := &bytes.Buffer{} - - 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()) -} - -func TestArrayMerger_finalEmptyArray(t *testing.T) { - merger := NewArrayMerger() - w := &bytes.Buffer{} - - r1 := bytes.NewBufferString(`["a","b"]`) - p1 := merger.NewPage(r1, false) - n, err := io.Copy(w, p1) - require.NoError(t, err) - assert.Equal(t, int64(8), 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()) -} - -func TestArrayMerger_multiplePages(t *testing.T) { - merger := NewArrayMerger() - w := &bytes.Buffer{} - - r1 := bytes.NewBufferString(`["a","b"]`) - p1 := merger.NewPage(r1, false) - n, err := io.Copy(w, p1) - require.NoError(t, err) - assert.Equal(t, int64(8), n) - assert.NoError(t, p1.Close()) - assert.Equal(t, `["a","b"`, w.String()) - - r2 := bytes.NewBufferString(`["c","d"]`) - p2 := merger.NewPage(r2, true) - n, err = io.Copy(w, p2) - require.NoError(t, err) - assert.Equal(t, int64(9), n) - assert.NoError(t, p2.Close()) - assert.Equal(t, `["a","b","c","d"]`, w.String()) - - require.NoError(t, merger.Close()) -} - -func TestArrayMerger_emptyObject(t *testing.T) { - merger := NewArrayMerger() - w := &bytes.Buffer{} - - 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()) -} diff --git a/internal/jsonmerge/merge.go b/internal/jsonmerge/merge.go deleted file mode 100644 index a40e4ee08..000000000 --- a/internal/jsonmerge/merge.go +++ /dev/null @@ -1,12 +0,0 @@ -// jsonmerge implements readers to merge JSON arrays or objects. -package jsonmerge - -import ( - "io" -) - -// Merger is implemented to merge JSON arrays or objects. -type Merger interface { - NewPage(r io.Reader, isLastPage bool) io.ReadCloser - Close() error -} diff --git a/internal/jsonmerge/nop.go b/internal/jsonmerge/nop.go deleted file mode 100644 index d11a545b9..000000000 --- a/internal/jsonmerge/nop.go +++ /dev/null @@ -1,19 +0,0 @@ -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 -} diff --git a/internal/jsonmerge/nop_test.go b/internal/jsonmerge/nop_test.go deleted file mode 100644 index 713ac5501..000000000 --- a/internal/jsonmerge/nop_test.go +++ /dev/null @@ -1,114 +0,0 @@ -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()) -} diff --git a/internal/jsonmerge/object.go b/internal/jsonmerge/object.go deleted file mode 100644 index b8f077bbf..000000000 --- a/internal/jsonmerge/object.go +++ /dev/null @@ -1,73 +0,0 @@ -package jsonmerge - -import ( - "bytes" - "encoding/json" - "io" - - "dario.cat/mergo" -) - -type objectMerger struct { - io.Writer - dst map[string]interface{} -} - -// NewObjectMerger creates a Merger for JSON objects. -func NewObjectMerger(w io.Writer) Merger { - return &objectMerger{ - Writer: w, - } -} - -func (merger *objectMerger) NewPage(r io.Reader, isLastPage bool) io.ReadCloser { - return &objectMergerPage{ - merger: merger, - Reader: r, - } -} - -func (merger *objectMerger) Close() error { - if merger.dst == nil { - return nil - } - - // Marshal to JSON and write to output. - buf, err := json.Marshal(merger.dst) - if err != nil { - return err - } - - _, err = merger.Writer.Write(buf) - return err -} - -type objectMergerPage struct { - merger *objectMerger - - io.Reader - buffer bytes.Buffer -} - -// Read caches the data in an internal buffer to be merged in Close. -// No data is copied into p so it's not written to stdout. -func (page *objectMergerPage) Read(p []byte) (int, error) { - _, err := io.CopyN(&page.buffer, page.Reader, int64(len(p))) - return 0, err -} - -// Close converts the internal buffer to a JSON object and merges it with the final JSON object. -func (page *objectMergerPage) Close() error { - var src map[string]interface{} - - err := json.Unmarshal(page.buffer.Bytes(), &src) - if err != nil { - return err - } - - if page.merger.dst == nil { - page.merger.dst = make(map[string]interface{}) - } - - return mergo.Merge(&page.merger.dst, src, mergo.WithAppendSlice, mergo.WithOverride) -} diff --git a/internal/jsonmerge/object_test.go b/internal/jsonmerge/object_test.go deleted file mode 100644 index 48074e33a..000000000 --- a/internal/jsonmerge/object_test.go +++ /dev/null @@ -1,106 +0,0 @@ -package jsonmerge - -import ( - "bytes" - "io" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestObjectMerger_nothingWritten(t *testing.T) { - w := &bytes.Buffer{} - merger := NewObjectMerger(w) - - require.NoError(t, merger.Close()) - assert.Equal(t, ``, w.String()) -} - -func TestObjectMerger_singleEmptyObject(t *testing.T) { - w := &bytes.Buffer{} - merger := NewObjectMerger(w) - - r1 := bytes.NewBufferString(`{}`) - p1 := merger.NewPage(r1, true) - n, err := io.Copy(w, p1) - require.NoError(t, err) - assert.Equal(t, int64(0), n) - assert.NoError(t, p1.Close()) - assert.Equal(t, ``, w.String()) - - require.NoError(t, merger.Close()) - assert.JSONEq(t, `{}`, w.String()) -} - -func TestObjectMerger_finalEmptyObject(t *testing.T) { - w := &bytes.Buffer{} - merger := NewObjectMerger(w) - - 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(0), n) - assert.NoError(t, p1.Close()) - assert.Equal(t, ``, w.String()) - - r2 := bytes.NewBufferString(`{}`) - p2 := merger.NewPage(r2, true) - n, err = io.Copy(w, p2) - require.NoError(t, err) - assert.Equal(t, int64(0), n) - assert.NoError(t, p2.Close()) - assert.Equal(t, ``, w.String()) - - require.NoError(t, merger.Close()) - assert.JSONEq(t, `{"a":1,"b":2}`, w.String()) -} - -func TestObjectMerger_multiplePages(t *testing.T) { - w := &bytes.Buffer{} - merger := NewObjectMerger(w) - - r1 := bytes.NewBufferString(`{"a":1,"b":2,"arr":["a","b"]}`) - p1 := merger.NewPage(r1, false) - n, err := io.Copy(w, p1) - require.NoError(t, err) - assert.Equal(t, int64(0), n) - assert.NoError(t, p1.Close()) - assert.Equal(t, ``, w.String()) - - r2 := bytes.NewBufferString(`{"b":3,"c":{"d":4},"arr":["c","d"]}`) - p2 := merger.NewPage(r2, true) - n, err = io.Copy(w, p2) - require.NoError(t, err) - assert.Equal(t, int64(0), n) - assert.NoError(t, p2.Close()) - assert.Equal(t, ``, w.String()) - - require.NoError(t, merger.Close()) - assert.JSONEq(t, `{"a":1,"b":3,"c":{"d":4},"arr":["a","b","c","d"]}`, w.String()) -} - -func TestObjectMerger_invalidJSON(t *testing.T) { - w := &bytes.Buffer{} - merger := NewObjectMerger(w) - - r1 := bytes.NewBufferString(`invalid`) - p1 := merger.NewPage(r1, true) - n, err := io.Copy(w, p1) - require.NoError(t, err) - assert.Equal(t, int64(0), n) - assert.Error(t, p1.Close()) -} - -func TestObjectMerger_array(t *testing.T) { - w := &bytes.Buffer{} - merger := NewObjectMerger(w) - - r1 := bytes.NewBufferString(`[]`) - p1 := merger.NewPage(r1, true) - n, err := io.Copy(w, p1) - require.NoError(t, err) - assert.Equal(t, int64(0), n) - assert.Error(t, p1.Close()) -} diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 35b99eef6..c821c84c8 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -20,7 +20,6 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/jsonmerge" "github.com/cli/cli/v2/pkg/cmd/factory" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -30,6 +29,10 @@ import ( "github.com/spf13/cobra" ) +const ( + ttyIndent = " " +) + type ApiOptions struct { AppVersion string BaseRepo func() (ghrepo.Interface, error) @@ -37,7 +40,6 @@ type ApiOptions struct { Config func() (config.Config, error) HttpClient func() (*http.Client, error) IO *iostreams.IOStreams - merger jsonmerge.Merger Hostname string RequestMethod string @@ -50,7 +52,7 @@ type ApiOptions struct { Previews []string ShowResponseHeaders bool Paginate bool - MergePages bool + Slurp bool Silent bool Template string CacheTTL time.Duration @@ -174,8 +176,8 @@ 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=' + # without --slurp you will get a different percentage for each page + $ gh api graphql --paginate --slurp -f query=' query($endCursor: String) { viewer { repositories(first: 100, after: $endCursor) { @@ -187,8 +189,8 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command } } } - ' | jq 'def count(s): reduce s as $_ (0;.+1); - .data.viewer.repositories.nodes as $r | count(select($r[].isFork)) / count($r[])' + ' | jq 'def count(e): reduce e as $_ (0;.+1); + [.[].data.viewer.repositories.nodes[]] as $r | count(select($r[].isFork))/count($r[])' `), Annotations: map[string]string{ "help:environment": heredoc.Doc(` @@ -231,8 +233,17 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command return err } - if opts.MergePages && !opts.Paginate { - return cmdutil.FlagErrorf("`--paginate` required when passing `--merge-pages`") + if opts.Slurp && !opts.Paginate { + return cmdutil.FlagErrorf("`--paginate` required when passing `--slurp`") + } + + if err := cmdutil.MutuallyExclusive( + "the `--slurp` option is not supported with `--jq` or `--template`", + opts.Slurp, + opts.FilterOutput != "", + opts.Template != "", + ); err != nil { + return err } if err := cmdutil.MutuallyExclusive( @@ -259,7 +270,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command cmd.Flags().StringArrayVarP(&opts.RequestHeaders, "header", "H", nil, "Add a HTTP request header in `key:value` format") cmd.Flags().StringSliceVarP(&opts.Previews, "preview", "p", nil, "GitHub API preview `names` to request (without the \"-preview\" suffix)") cmd.Flags().BoolVarP(&opts.ShowResponseHeaders, "include", "i", false, "Include HTTP response status line and headers in the output") - cmd.Flags().BoolVar(&opts.MergePages, "merge-pages", false, "Use with \"--paginate\" to merge all pages of JSON arrays or objects when piping or redirecting standard output") + cmd.Flags().BoolVar(&opts.Slurp, "slurp", false, "Use with \"--paginate\" to return an array of all pages of either JSON arrays or objects") 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 (use \"-\" to read from standard input)") cmd.Flags().BoolVar(&opts.Silent, "silent", false, "Do not print the response body") @@ -307,20 +318,16 @@ func apiRun(opts *ApiOptions) error { requestPath = addPerPage(requestPath, 100, params) } - // 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.FilterOutput == "" && opts.Template == "" { - if !isGraphQL { - opts.merger = jsonmerge.NewArrayMerger() - } else if opts.MergePages { - opts.merger = jsonmerge.NewObjectMerger(bodyWriter) + // Similar to `jq --slurp`, write all pages JSON arrays or objects into a JSON array. + if opts.Paginate && opts.Slurp { + w := &jsonArrayWriter{ + Writer: bodyWriter, + color: opts.IO.ColorEnabled(), } - } + defer w.Close() - if opts.merger == nil { - opts.merger = jsonmerge.NewNopMerger() + bodyWriter = w } - defer opts.merger.Close() if opts.RequestInputFile != "" { file, size, err := openUserFile(opts.RequestInputFile, opts.IO.In) @@ -400,6 +407,12 @@ func apiRun(opts *ApiOptions) error { requestBody = nil // prevent repeating GET parameters } + // Tell optional jsonArrayWriter to start a new page. + err = startPage(bodyWriter) + if err != nil { + return err + } + endCursor, err := processResponse(resp, opts, bodyWriter, headersWriter, tmpl, isFirstPage, !hasNextPage) if err != nil { return err @@ -459,7 +472,7 @@ func processResponse(resp *http.Response, opts *ApiOptions, bodyWriter, headersW // TODO: reuse parsed query across pagination invocations indent := "" if opts.IO.IsStdoutTTY() { - indent = " " + indent = ttyIndent } err = jq.EvaluateFormatted(responseBody, bodyWriter, opts.FilterOutput, indent, opts.IO.ColorEnabled()) if err != nil { @@ -471,20 +484,16 @@ func processResponse(resp *http.Response, opts *ApiOptions, bodyWriter, headersW return } } else if isJSON && opts.IO.ColorEnabled() { - err = jsoncolor.Write(bodyWriter, responseBody, " ") + err = jsoncolor.Write(bodyWriter, responseBody, ttyIndent) } else { - if isJSON && opts.Paginate && !opts.ShowResponseHeaders { - responseBody = opts.merger.NewPage(responseBody, isLastPage) + if isJSON && opts.Paginate && !opts.Slurp && !isGraphQLPaginate && !opts.ShowResponseHeaders { + responseBody = &paginatedArrayReader{ + Reader: responseBody, + isFirstPage: isFirstPage, + isLastPage: isLastPage, + } } - _, err = io.Copy(bodyWriter, responseBody) - if err != nil { - return - } - - if closer, ok := responseBody.(io.ReadCloser); ok { - err = closer.Close() - } } if err != nil { return diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 0d02eff61..5244f04d4 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -329,8 +329,18 @@ func Test_NewCmdApi(t *testing.T) { wantsErr: true, }, { - name: "--merge-pages without --paginate", - cli: "user --merge-pages", + name: "--slurp without --paginate", + cli: "user --slurp", + wantsErr: true, + }, + { + name: "slurp with --jq", + cli: "user --paginate --slurp --jq .foo", + wantsErr: true, + }, + { + name: "slurp with --template", + cli: "user --paginate --slurp --template '{{.foo}}'", wantsErr: true, }, { @@ -993,7 +1003,7 @@ func Test_apiRun_paginationGraphQL(t *testing.T) { assert.Equal(t, "PAGE1_END", endCursor) } -func Test_apiRun_paginationGraphQL_merge(t *testing.T) { +func Test_apiRun_paginationGraphQL_slurp(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() requestCount := 0 @@ -1047,121 +1057,33 @@ func Test_apiRun_paginationGraphQL_merge(t *testing.T) { RequestMethod: "POST", RequestPath: "graphql", Paginate: true, - MergePages: true, + Slurp: 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_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{ + assert.JSONEq(t, stdout.String(), `[ { - 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 - } + "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 - } + + "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 { diff --git a/pkg/cmd/api/pagination.go b/pkg/cmd/api/pagination.go index 65d816480..bf4a2f794 100644 --- a/pkg/cmd/api/pagination.go +++ b/pkg/cmd/api/pagination.go @@ -8,6 +8,8 @@ import ( "net/url" "regexp" "strings" + + "github.com/cli/cli/v2/pkg/jsoncolor" ) var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) @@ -106,3 +108,134 @@ func addPerPage(p string, perPage int, params map[string]interface{}) string { return fmt.Sprintf("%s%sper_page=%d", p, sep, perPage) } + +// paginatedArrayReader wraps a Reader to omit the opening and/or the closing square bracket of a +// JSON array in order to apply pagination context between multiple API requests. +type paginatedArrayReader struct { + io.Reader + isFirstPage bool + isLastPage bool + + isSubsequentRead bool + cachedByte byte +} + +func (r *paginatedArrayReader) Read(p []byte) (int, error) { + var n int + var err error + if r.cachedByte != 0 && len(p) > 0 { + p[0] = r.cachedByte + n, err = r.Reader.Read(p[1:]) + n += 1 + r.cachedByte = 0 + } else { + n, err = r.Reader.Read(p) + } + if !r.isSubsequentRead && !r.isFirstPage && n > 0 && p[0] == '[' { + if n > 1 && p[1] == ']' { + // empty array case + p[0] = ' ' + } else { + // avoid starting a new array and continue with a comma instead + p[0] = ',' + } + } + if !r.isLastPage && n > 0 && p[n-1] == ']' { + // avoid closing off an array in case we determine we are at EOF + r.cachedByte = p[n-1] + n -= 1 + } + r.isSubsequentRead = true + return n, err +} + +// jsonArrayWriter wraps a Writer which writes multiple pages of both JSON arrays +// and objects. Call Close to write the end of the array. +type jsonArrayWriter struct { + io.Writer + started bool + color bool +} + +func (w *jsonArrayWriter) Preface() []json.Delim { + if w.started { + return []json.Delim{'['} + } + return nil +} + +// ReadFrom implements io.ReaderFrom to write more data than read, +// which otherwise results in an error from io.Copy(). +func (w *jsonArrayWriter) ReadFrom(r io.Reader) (int64, error) { + var written int64 + buf := make([]byte, 4069) + for { + n, err := r.Read(buf) + if n > 0 { + n, err := w.Write(buf[:n]) + written += int64(n) + + if err != nil { + return written, err + } + } + if err == io.EOF { + break + } + if err != nil { + return written, err + } + } + + return written, nil +} + +func (w *jsonArrayWriter) Close() error { + var delims string + if w.started { + delims = "]" + } else { + delims = "[]" + } + + w.started = false + if w.color { + return jsoncolor.WriteDelims(w, delims, ttyIndent) + } + + _, err := w.Writer.Write([]byte(delims)) + return err +} + +func startPage(w io.Writer) error { + if jaw, ok := w.(*jsonArrayWriter); ok { + var delims string + var indent bool + + if !jaw.started { + delims = "[" + jaw.started = true + } else { + delims = "," + indent = true + } + + if jaw.color { + if indent { + _, err := jaw.Write([]byte(ttyIndent)) + if err != nil { + return err + } + } + + return jsoncolor.WriteDelims(w, delims, ttyIndent) + } + + _, err := jaw.Write([]byte(delims)) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/cmd/api/pagination_test.go b/pkg/cmd/api/pagination_test.go index 3bb1a8ec5..746a73c4a 100644 --- a/pkg/cmd/api/pagination_test.go +++ b/pkg/cmd/api/pagination_test.go @@ -5,6 +5,9 @@ import ( "io" "net/http" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_findNextPage(t *testing.T) { @@ -167,3 +170,107 @@ func Test_addPerPage(t *testing.T) { }) } } + +func TestJsonArrayWriter(t *testing.T) { + tests := []struct { + name string + pages []string + want string + }{ + { + name: "empty", + pages: nil, + want: "[]", + }, + { + name: "single array", + pages: []string{`[1,2]`}, + want: `[[1,2]]`, + }, + { + name: "multiple arrays", + pages: []string{`[1,2]`, `[3]`}, + want: `[[1,2],[3]]`, + }, + { + name: "single object", + pages: []string{`{"foo":1,"bar":"a"}`}, + want: `[{"foo":1,"bar":"a"}]`, + }, + { + name: "multiple pages", + pages: []string{`{"foo":1,"bar":"a"}`, `{"foo":2,"bar":"b"}`}, + want: `[{"foo":1,"bar":"a"},{"foo":2,"bar":"b"}]`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + w := &jsonArrayWriter{ + Writer: buf, + } + + for _, page := range tt.pages { + require.NoError(t, startPage(w)) + + n, err := w.Write([]byte(page)) + require.NoError(t, err) + assert.Equal(t, len(page), n) + } + + require.NoError(t, w.Close()) + assert.Equal(t, tt.want, buf.String()) + }) + } +} + +func TestJsonArrayWriter_Copy(t *testing.T) { + tests := []struct { + name string + limit int + }{ + { + name: "unlimited", + }, + { + name: "limited", + limit: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + w := &jsonArrayWriter{ + Writer: buf, + } + + r := &noWriteToReader{ + Reader: bytes.NewBufferString(`[1,2]`), + limit: tt.limit, + } + + require.NoError(t, startPage(w)) + + n, err := io.Copy(w, r) + require.NoError(t, err) + assert.Equal(t, int64(5), n) + + require.NoError(t, w.Close()) + assert.Equal(t, `[[1,2]]`, buf.String()) + }) + } +} + +type noWriteToReader struct { + io.Reader + limit int +} + +func (r *noWriteToReader) Read(p []byte) (int, error) { + if r.limit > 0 { + p = p[:r.limit] + } + return r.Reader.Read(p) +} diff --git a/pkg/jsoncolor/jsoncolor.go b/pkg/jsoncolor/jsoncolor.go index dbe3d9a4b..8e20a1161 100644 --- a/pkg/jsoncolor/jsoncolor.go +++ b/pkg/jsoncolor/jsoncolor.go @@ -16,6 +16,10 @@ const ( colorBool = "33" // yellow ) +type JsonWriter interface { + Preface() []json.Delim +} + // Write colorized JSON output parsed from reader func Write(w io.Writer, r io.Reader, indent string) error { dec := json.NewDecoder(r) @@ -24,6 +28,10 @@ func Write(w io.Writer, r io.Reader, indent string) error { var idx int var stack []json.Delim + if jsonWriter, ok := w.(JsonWriter); ok { + stack = append(stack, jsonWriter.Preface()...) + } + for { t, err := dec.Token() if err == io.EOF { @@ -96,6 +104,20 @@ func Write(w io.Writer, r io.Reader, indent string) error { return nil } +// WriteDelims writes delims in color and with the appropriate indent +// based on the stack size returned from an io.Writer that implements JsonWriter.Preface(). +func WriteDelims(w io.Writer, delims, indent string) error { + var stack []json.Delim + if jaw, ok := w.(JsonWriter); ok { + stack = jaw.Preface() + } + + fmt.Fprintf(w, "\x1b[%sm%s\x1b[m", colorDelim, delims) + fmt.Fprint(w, "\n", strings.Repeat(indent, len(stack))) + + return nil +} + // marshalJSON works like json.Marshal but with HTML-escaping disabled func marshalJSON(v interface{}) ([]byte, error) { buf := bytes.Buffer{}