From acf004671818bfdba2f06d3cb6e42083dedf4930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 11 Jun 2020 15:00:29 +0200 Subject: [PATCH 1/2] api command: support `{owner}` and `{repo}` placeholders When `{owner}` and `{repo}` strings are found in request path (for REST requests) or `query` (for GraphQL), they are replaced with values from the repository of the current working directory. --- command/root.go | 7 +++ pkg/cmd/api/api.go | 39 ++++++++++++++- pkg/cmd/api/api_test.go | 104 ++++++++++++++++++++++++++++++++++++++++ pkg/cmdutil/factory.go | 2 + 4 files changed, 151 insertions(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 12979ad5f..cfe975779 100644 --- a/command/root.go +++ b/command/root.go @@ -75,8 +75,10 @@ func init() { HttpClient: func() (*http.Client, error) { token := os.Getenv("GITHUB_TOKEN") if len(token) == 0 { + // TODO: decouple from `context` ctx := context.New() var err error + // TODO: pass IOStreams to this so that the auth flow knows if it's interactive or not token, err = ctx.AuthToken() if err != nil { return nil, err @@ -84,6 +86,11 @@ func init() { } return httpClient(token), nil }, + BaseRepo: func() (ghrepo.Interface, error) { + // TODO: decouple from `context` + ctx := context.New() + return ctx.BaseRepo() + }, } RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil)) } diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index b4a8dbdff..c7f6dd703 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/jsoncolor" @@ -32,12 +33,14 @@ type ApiOptions struct { ShowResponseHeaders bool HttpClient func() (*http.Client, error) + BaseRepo func() (ghrepo.Interface, error) } func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command { opts := ApiOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + BaseRepo: f.BaseRepo, } cmd := &cobra.Command{ @@ -93,8 +96,11 @@ func apiRun(opts *ApiOptions) error { return err } + requestPath, params, err := fillPlaceholders(opts, params) + if err != nil { + return fmt.Errorf("unable to expand `{...}` placeholders in query: %w", err) + } method := opts.RequestMethod - requestPath := opts.RequestPath requestHeaders := opts.RequestHeaders var requestBody interface{} = params @@ -170,6 +176,37 @@ func apiRun(opts *ApiOptions) error { return nil } +// fillPlaceholders replaces `{owner}` and `{repo}` placeholders with values from the current repository +func fillPlaceholders(opts *ApiOptions, params map[string]interface{}) (string, map[string]interface{}, error) { + query := opts.RequestPath + isGraphQL := opts.RequestPath == "graphql" + + if isGraphQL { + if q, ok := params["query"].(string); ok { + query = q + } + } + + if !strings.Contains(query, "{owner}") && !strings.Contains(query, "{repo}") { + return opts.RequestPath, params, nil + } + + baseRepo, err := opts.BaseRepo() + if err != nil { + return opts.RequestPath, params, err + } + + query = strings.ReplaceAll(query, "{owner}", baseRepo.RepoOwner()) + query = strings.ReplaceAll(query, "{repo}", baseRepo.RepoName()) + + if isGraphQL { + params["query"] = query + return opts.RequestPath, params, nil + } + + return query, params, nil +} + func printHeaders(w io.Writer, headers http.Header, colorize bool) { var names []string for name := range headers { diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 100c1257e..0069e4635 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -7,8 +7,10 @@ import ( "io/ioutil" "net/http" "os" + "reflect" "testing" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/google/shlex" @@ -451,3 +453,105 @@ func Test_openUserFile(t *testing.T) { assert.Equal(t, int64(13), length) assert.Equal(t, "file contents", string(fb)) } + +func Test_fillPlaceholders(t *testing.T) { + type args struct { + opts *ApiOptions + params map[string]interface{} + } + tests := []struct { + name string + args args + wantPath string + wantParams map[string]interface{} + wantErr bool + }{ + { + name: "no changes", + args: args{ + opts: &ApiOptions{ + RequestPath: "repos/owner/repo/releases", + BaseRepo: nil, + }, + params: map[string]interface{}{ + "query": "{owner}/{repo}", + }, + }, + wantPath: "repos/owner/repo/releases", + wantParams: map[string]interface{}{ + "query": "{owner}/{repo}", + }, + wantErr: false, + }, + { + name: "REST path substitute", + args: args{ + opts: &ApiOptions{ + RequestPath: "repos/{owner}/{repo}/releases", + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + params: map[string]interface{}{ + "query": "{owner}/{repo}", + }, + }, + wantPath: "repos/hubot/robot-uprising/releases", + wantParams: map[string]interface{}{ + "query": "{owner}/{repo}", + }, + wantErr: false, + }, + { + name: "GraphQL query substitute", + args: args{ + opts: &ApiOptions{ + RequestPath: "graphql", + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + params: map[string]interface{}{ + "query": "{owner}/{repo}/pulls/{owner}", + }, + }, + wantPath: "graphql", + wantParams: map[string]interface{}{ + "query": "hubot/robot-uprising/pulls/hubot", + }, + wantErr: false, + }, + { + name: "GraphQL no query", + args: args{ + opts: &ApiOptions{ + RequestPath: "graphql", + BaseRepo: nil, + }, + params: map[string]interface{}{ + "foo": "{owner}/{repo}", + }, + }, + wantPath: "graphql", + wantParams: map[string]interface{}{ + "foo": "{owner}/{repo}", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got1, err := fillPlaceholders(tt.args.opts, tt.args.params) + if (err != nil) != tt.wantErr { + t.Errorf("fillPlaceholders() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.wantPath { + t.Errorf("fillPlaceholders() got = %v, want %v", got, tt.wantPath) + } + if !reflect.DeepEqual(got1, tt.wantParams) { + t.Errorf("fillPlaceholders() got1 = %v, want %v", got1, tt.wantParams) + } + }) + } +} diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go index 578b29561..ad7162415 100644 --- a/pkg/cmdutil/factory.go +++ b/pkg/cmdutil/factory.go @@ -3,10 +3,12 @@ package cmdutil import ( "net/http" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/iostreams" ) type Factory struct { IOStreams *iostreams.IOStreams HttpClient func() (*http.Client, error) + BaseRepo func() (ghrepo.Interface, error) } From 3f6d0bff45d8d52be9fd3153f823a796c246ce2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 11 Jun 2020 21:46:27 +0200 Subject: [PATCH 2/2] Switch to `:owner`/`:repo` syntax for placeholders --- go.mod | 1 + go.sum | 2 + pkg/cmd/api/api.go | 73 +++++++++++++++---------- pkg/cmd/api/api_test.go | 117 +++++++++++++++++----------------------- 4 files changed, 95 insertions(+), 98 deletions(-) diff --git a/go.mod b/go.mod index 7f1484a03..6857e1947 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.13 require ( github.com/AlecAivazis/survey/v2 v2.0.7 + github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.11.1 github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058 github.com/dlclark/regexp2 v1.2.0 // indirect diff --git a/go.sum b/go.sum index d0d7585bf..42f30698a 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,8 @@ github.com/AlecAivazis/survey/v2 v2.0.7 h1:+f825XHLse/hWd2tE/V5df04WFGimk34Eyg/z github.com/AlecAivazis/survey/v2 v2.0.7/go.mod h1:mlizQTaPjnR4jcpwRSaSlkbsRfYFEyKgLQvYTzxxiHA= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= +github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8 h1:xzYJEypr/85nBpB11F9br+3HUrpgb+fcm5iADzXXYEw= github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8/go.mod h1:oX5x61PbNXchhh0oikYAH+4Pcfw5LKv21+Jnpr6r6Pc= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index c7f6dd703..a5540cf2d 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -48,13 +49,16 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command Short: "Make an authenticated GitHub API request", Long: `Makes an authenticated HTTP request to the GitHub API and prints the response. -The argument should either be a path of a GitHub API v3 endpoint, or +The endpoint argument should either be a path of a GitHub API v3 endpoint, or "graphql" to access the GitHub API v4. +Placeholder values ":owner" and ":repo" in the endpoint argument will get replaced +with values from the repository of the current directory. + The default HTTP request method is "GET" normally and "POST" if any parameters were added. Override the method with '--method'. -Pass one or more '--raw-field' values in "=" format to add +Pass one or more '--raw-field' values in "key=value" format to add JSON-encoded string parameters to the POST body. The '--field' flag behaves like '--raw-field' with magic type conversion based @@ -62,6 +66,8 @@ on the format of the value: - literal values "true", "false", "null", and integer numbers get converted to appropriate JSON types; +- placeholder values ":owner" and ":repo" get populated with values from the + repository of the current directory; - if the value starts with "@", the rest of the value is interpreted as a filename to read the value from. Pass "-" to read from standard input. @@ -69,6 +75,19 @@ 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. `, + Example: heredoc.Doc(` + $ gh api repos/:owner/:repo/releases + + $ gh api graphql -F owner=':owner' -F name=':repo' -f query=' + query($name: String!, $owner: String!) { + repository(owner: $owner, name: $name) { + releases(last: 3) { + nodes { tagName } + } + } + } + ' + `), Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.RequestPath = args[0] @@ -96,9 +115,9 @@ func apiRun(opts *ApiOptions) error { return err } - requestPath, params, err := fillPlaceholders(opts, params) + requestPath, err := fillPlaceholders(opts.RequestPath, opts) if err != nil { - return fmt.Errorf("unable to expand `{...}` placeholders in query: %w", err) + return fmt.Errorf("unable to expand placeholder in path: %w", err) } method := opts.RequestMethod requestHeaders := opts.RequestHeaders @@ -176,35 +195,31 @@ func apiRun(opts *ApiOptions) error { return nil } -// fillPlaceholders replaces `{owner}` and `{repo}` placeholders with values from the current repository -func fillPlaceholders(opts *ApiOptions, params map[string]interface{}) (string, map[string]interface{}, error) { - query := opts.RequestPath - isGraphQL := opts.RequestPath == "graphql" +var placeholderRE = regexp.MustCompile(`\:(owner|repo)\b`) - if isGraphQL { - if q, ok := params["query"].(string); ok { - query = q - } - } - - if !strings.Contains(query, "{owner}") && !strings.Contains(query, "{repo}") { - return opts.RequestPath, params, nil +// fillPlaceholders populates `:owner` and `:repo` placeholders with values from the current repository +func fillPlaceholders(value string, opts *ApiOptions) (string, error) { + if !placeholderRE.MatchString(value) { + return value, nil } baseRepo, err := opts.BaseRepo() if err != nil { - return opts.RequestPath, params, err + return value, err } - query = strings.ReplaceAll(query, "{owner}", baseRepo.RepoOwner()) - query = strings.ReplaceAll(query, "{repo}", baseRepo.RepoName()) + value = placeholderRE.ReplaceAllStringFunc(value, func(m string) string { + switch m { + case ":owner": + return baseRepo.RepoOwner() + case ":repo": + return baseRepo.RepoName() + default: + panic(fmt.Sprintf("invalid placeholder: %q", m)) + } + }) - if isGraphQL { - params["query"] = query - return opts.RequestPath, params, nil - } - - return query, params, nil + return value, nil } func printHeaders(w io.Writer, headers http.Header, colorize bool) { @@ -241,7 +256,7 @@ func parseFields(opts *ApiOptions) (map[string]interface{}, error) { if err != nil { return params, err } - value, err := magicFieldValue(strValue, opts.IO.In) + value, err := magicFieldValue(strValue, opts) if err != nil { return params, fmt.Errorf("error parsing %q value: %w", key, err) } @@ -258,9 +273,9 @@ func parseField(f string) (string, string, error) { return f[0:idx], f[idx+1:], nil } -func magicFieldValue(v string, stdin io.ReadCloser) (interface{}, error) { +func magicFieldValue(v string, opts *ApiOptions) (interface{}, error) { if strings.HasPrefix(v, "@") { - return readUserFile(v[1:], stdin) + return readUserFile(v[1:], opts.IO.In) } if n, err := strconv.Atoi(v); err == nil { @@ -275,7 +290,7 @@ func magicFieldValue(v string, stdin io.ReadCloser) (interface{}, error) { case "null": return nil, nil default: - return v, nil + return fillPlaceholders(v, opts) } } diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 0069e4635..605e4bf9e 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -3,11 +3,9 @@ package api import ( "bytes" "fmt" - "io" "io/ioutil" "net/http" "os" - "reflect" "testing" "github.com/cli/cli/internal/ghrepo" @@ -368,9 +366,11 @@ func Test_magicFieldValue(t *testing.T) { f.Close() t.Cleanup(func() { os.Remove(f.Name()) }) + io, _, _, _ := iostreams.Test() + type args struct { - v string - stdin io.ReadCloser + v string + opts *ApiOptions } tests := []struct { name string @@ -403,21 +403,41 @@ func Test_magicFieldValue(t *testing.T) { wantErr: false, }, { - name: "file", - args: args{v: "@" + f.Name()}, + name: "placeholder", + args: args{ + v: ":owner", + opts: &ApiOptions{ + IO: io, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "hubot", + wantErr: false, + }, + { + name: "file", + args: args{ + v: "@" + f.Name(), + opts: &ApiOptions{IO: io}, + }, want: []byte("file contents"), wantErr: false, }, { - name: "file error", - args: args{v: "@"}, + name: "file error", + args: args{ + v: "@", + opts: &ApiOptions{IO: io}, + }, want: nil, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := magicFieldValue(tt.args.v, tt.args.stdin) + got, err := magicFieldValue(tt.args.v, tt.args.opts) if (err != nil) != tt.wantErr { t.Errorf("magicFieldValue() error = %v, wantErr %v", err, tt.wantErr) return @@ -456,101 +476,60 @@ func Test_openUserFile(t *testing.T) { func Test_fillPlaceholders(t *testing.T) { type args struct { - opts *ApiOptions - params map[string]interface{} + value string + opts *ApiOptions } tests := []struct { - name string - args args - wantPath string - wantParams map[string]interface{} - wantErr bool + name string + args args + want string + wantErr bool }{ { name: "no changes", args: args{ + value: "repos/owner/repo/releases", opts: &ApiOptions{ - RequestPath: "repos/owner/repo/releases", - BaseRepo: nil, - }, - params: map[string]interface{}{ - "query": "{owner}/{repo}", + BaseRepo: nil, }, }, - wantPath: "repos/owner/repo/releases", - wantParams: map[string]interface{}{ - "query": "{owner}/{repo}", - }, + want: "repos/owner/repo/releases", wantErr: false, }, { - name: "REST path substitute", + name: "has substitutes", args: args{ + value: "repos/:owner/:repo/releases", opts: &ApiOptions{ - RequestPath: "repos/{owner}/{repo}/releases", BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("hubot", "robot-uprising"), nil }, }, - params: map[string]interface{}{ - "query": "{owner}/{repo}", - }, - }, - wantPath: "repos/hubot/robot-uprising/releases", - wantParams: map[string]interface{}{ - "query": "{owner}/{repo}", }, + want: "repos/hubot/robot-uprising/releases", wantErr: false, }, { - name: "GraphQL query substitute", + name: "no greedy substitutes", args: args{ + value: ":ownership/:repository", opts: &ApiOptions{ - RequestPath: "graphql", - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("hubot", "robot-uprising"), nil - }, - }, - params: map[string]interface{}{ - "query": "{owner}/{repo}/pulls/{owner}", + BaseRepo: nil, }, }, - wantPath: "graphql", - wantParams: map[string]interface{}{ - "query": "hubot/robot-uprising/pulls/hubot", - }, - wantErr: false, - }, - { - name: "GraphQL no query", - args: args{ - opts: &ApiOptions{ - RequestPath: "graphql", - BaseRepo: nil, - }, - params: map[string]interface{}{ - "foo": "{owner}/{repo}", - }, - }, - wantPath: "graphql", - wantParams: map[string]interface{}{ - "foo": "{owner}/{repo}", - }, + want: ":ownership/:repository", wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1, err := fillPlaceholders(tt.args.opts, tt.args.params) + got, err := fillPlaceholders(tt.args.value, tt.args.opts) if (err != nil) != tt.wantErr { t.Errorf("fillPlaceholders() error = %v, wantErr %v", err, tt.wantErr) return } - if got != tt.wantPath { - t.Errorf("fillPlaceholders() got = %v, want %v", got, tt.wantPath) - } - if !reflect.DeepEqual(got1, tt.wantParams) { - t.Errorf("fillPlaceholders() got1 = %v, want %v", got1, tt.wantParams) + if got != tt.want { + t.Errorf("fillPlaceholders() got = %v, want %v", got, tt.want) } }) }