Skip to content

Commit eefb6d1

Browse files
probablycoreyprobablycorey
authored
Merge pull request cli#34 from github/no-global
Eliminate package-level global state
2 parents 51b08f8 + 862db45 commit eefb6d1

24 files changed

+589
-403
lines changed

‎api/client.go‎

Lines changed: 72 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -4,47 +4,78 @@ import (
44
"bytes"
55
"encoding/json"
66
"fmt"
7+
"io"
78
"io/ioutil"
89
"net/http"
9-
"os"
10-
11-
"github.com/github/gh-cli/context"
12-
"github.com/github/gh-cli/version"
1310
)
1411

15-
type graphQLResponse struct {
16-
Data interface{}
17-
Errors []struct {
18-
Message string
12+
// ClientOption represents an argument to NewClient
13+
type ClientOption = func(http.RoundTripper) http.RoundTripper
14+
15+
// NewClient initializes a Client
16+
func NewClient(opts ...ClientOption) *Client {
17+
tr := http.DefaultTransport
18+
for _, opt := range opts {
19+
tr = opt(tr)
1920
}
21+
http := &http.Client{Transport: tr}
22+
client := &Client{http: http}
23+
return client
2024
}
2125

22-
/*
23-
GraphQL: Declared as an external variable so it can be mocked in tests
26+
// AddHeader turns a RoundTripper into one that adds a request header
27+
func AddHeader(name, value string) ClientOption {
28+
return func(tr http.RoundTripper) http.RoundTripper {
29+
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
30+
req.Header.Add(name, value)
31+
return tr.RoundTrip(req)
32+
}}
33+
}
34+
}
2435

25-
type repoResponse struct {
26-
Repository struct {
27-
CreatedAt string
36+
// VerboseLog enables request/response logging within a RoundTripper
37+
func VerboseLog(out io.Writer) ClientOption {
38+
return func(tr http.RoundTripper) http.RoundTripper {
39+
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
40+
fmt.Fprintf(out, "> %s %s\n", req.Method, req.URL.RequestURI())
41+
res, err := tr.RoundTrip(req)
42+
if err == nil {
43+
fmt.Fprintf(out, "< HTTP %s\n", res.Status)
44+
}
45+
return res, err
46+
}}
2847
}
2948
}
3049

31-
query := `query {
32-
repository(owner: "golang", name: "go") {
33-
createdAt
50+
// ReplaceTripper substitutes the underlying RoundTripper with a custom one
51+
func ReplaceTripper(tr http.RoundTripper) ClientOption {
52+
return func(http.RoundTripper) http.RoundTripper {
53+
return tr
3454
}
35-
}`
55+
}
3656

37-
variables := map[string]string{}
57+
type funcTripper struct {
58+
roundTrip func(*http.Request) (*http.Response, error)
59+
}
60+
61+
func (tr funcTripper) RoundTrip(req *http.Request) (*http.Response, error) {
62+
return tr.roundTrip(req)
63+
}
3864

39-
var resp repoResponse
40-
err := graphql(query, map[string]string{}, &resp)
41-
if err != nil {
42-
panic(err)
65+
// Client facilitates making HTTP requests to the GitHub API
66+
type Client struct {
67+
http *http.Client
4368
}
4469

45-
fmt.Printf("%+v\n", resp)
46-
*/
47-
var GraphQL = func(query string, variables map[string]string, data interface{}) error {
70+
type graphQLResponse struct {
71+
Data interface{}
72+
Errors []struct {
73+
Message string
74+
}
75+
}
76+
77+
// GraphQL performs a GraphQL request and parses the response
78+
func (c Client) GraphQL(query string, variables map[string]interface{}, data interface{}) error {
4879
url := "https://api.github.com/graphql"
4980
reqBody, err := json.Marshal(map[string]interface{}{"query": query, "variables": variables})
5081
if err != nil {
@@ -56,42 +87,31 @@ var GraphQL = func(query string, variables map[string]string, data interface{})
5687
return err
5788
}
5889

59-
token, err := context.Current().AuthToken()
60-
if err != nil {
61-
return err
62-
}
63-
64-
req.Header.Set("Authorization", "token "+token)
6590
req.Header.Set("Content-Type", "application/json; charset=utf-8")
66-
req.Header.Set("User-Agent", "GitHub CLI "+version.Version)
6791

68-
debugRequest(req, string(reqBody))
69-
70-
client := &http.Client{}
71-
resp, err := client.Do(req)
92+
resp, err := c.http.Do(req)
7293
if err != nil {
7394
return err
7495
}
7596
defer resp.Body.Close()
7697

77-
body, err := ioutil.ReadAll(resp.Body)
78-
if err != nil {
79-
return err
80-
}
81-
82-
debugResponse(resp, string(body))
83-
return handleResponse(resp, body, data)
98+
return handleResponse(resp, data)
8499
}
85100

86-
func handleResponse(resp *http.Response, body []byte, data interface{}) error {
101+
func handleResponse(resp *http.Response, data interface{}) error {
87102
success := resp.StatusCode >= 200 && resp.StatusCode < 300
88103

89104
if !success {
90-
return handleHTTPError(resp, body)
105+
return handleHTTPError(resp)
106+
}
107+
108+
body, err := ioutil.ReadAll(resp.Body)
109+
if err != nil {
110+
return err
91111
}
92112

93113
gr := &graphQLResponse{Data: data}
94-
err := json.Unmarshal(body, &gr)
114+
err = json.Unmarshal(body, &gr)
95115
if err != nil {
96116
return err
97117
}
@@ -107,12 +127,16 @@ func handleResponse(resp *http.Response, body []byte, data interface{}) error {
107127

108128
}
109129

110-
func handleHTTPError(resp *http.Response, body []byte) error {
130+
func handleHTTPError(resp *http.Response) error {
111131
var message string
112132
var parsedBody struct {
113133
Message string `json:"message"`
114134
}
115-
err := json.Unmarshal(body, &parsedBody)
135+
body, err := ioutil.ReadAll(resp.Body)
136+
if err != nil {
137+
return err
138+
}
139+
err = json.Unmarshal(body, &parsedBody)
116140
if err != nil {
117141
message = string(body)
118142
} else {
@@ -121,19 +145,3 @@ func handleHTTPError(resp *http.Response, body []byte) error {
121145

122146
return fmt.Errorf("http error, '%s' failed (%d): '%s'", resp.Request.URL, resp.StatusCode, message)
123147
}
124-
125-
func debugRequest(req *http.Request, body string) {
126-
if _, ok := os.LookupEnv("DEBUG"); !ok {
127-
return
128-
}
129-
130-
fmt.Printf("DEBUG: GraphQL request to %s:\n %s\n\n", req.URL, body)
131-
}
132-
133-
func debugResponse(resp *http.Response, body string) {
134-
if _, ok := os.LookupEnv("DEBUG"); !ok {
135-
return
136-
}
137-
138-
fmt.Printf("DEBUG: GraphQL response:\n%+v\n\n%s\n\n", resp, body)
139-
}

‎api/client_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package api
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"io/ioutil"
7+
"reflect"
8+
"testing"
9+
)
10+
11+
func eq(t *testing.T, got interface{}, expected interface{}) {
12+
t.Helper()
13+
if !reflect.DeepEqual(got, expected) {
14+
t.Errorf("expected: %v, got: %v", expected, got)
15+
}
16+
}
17+
18+
func TestGraphQL(t *testing.T) {
19+
http := &FakeHTTP{}
20+
client := NewClient(
21+
ReplaceTripper(http),
22+
AddHeader("Authorization", "token OTOKEN"),
23+
)
24+
25+
vars := map[string]interface{}{"name": "Mona"}
26+
response := struct {
27+
Viewer struct {
28+
Login string
29+
}
30+
}{}
31+
32+
http.StubResponse(200, bytes.NewBufferString(`{"data":{"viewer":{"login":"hubot"}}}`))
33+
err := client.GraphQL("QUERY", vars, &response)
34+
eq(t, err, nil)
35+
eq(t, response.Viewer.Login, "hubot")
36+
37+
req := http.Requests[0]
38+
reqBody, _ := ioutil.ReadAll(req.Body)
39+
eq(t, string(reqBody), `{"query":"QUERY","variables":{"name":"Mona"}}`)
40+
eq(t, req.Header.Get("Authorization"), "token OTOKEN")
41+
}
42+
43+
func TestGraphQLError(t *testing.T) {
44+
http := &FakeHTTP{}
45+
client := NewClient(ReplaceTripper(http))
46+
47+
response := struct{}{}
48+
http.StubResponse(200, bytes.NewBufferString(`{"errors":[{"message":"OH NO"}]}`))
49+
err := client.GraphQL("", nil, &response)
50+
eq(t, err, fmt.Errorf("graphql error: 'OH NO'"))
51+
}

‎api/fake_http.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package api
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"io/ioutil"
7+
"net/http"
8+
)
9+
10+
// FakeHTTP provides a mechanism by which to stub HTTP responses through
11+
type FakeHTTP struct {
12+
// Requests stores references to sequental requests that RoundTrip has received
13+
Requests []*http.Request
14+
count int
15+
responseStubs []*http.Response
16+
}
17+
18+
// StubResponse pre-records an HTTP response
19+
func (f *FakeHTTP) StubResponse(status int, body io.Reader) {
20+
resp := &http.Response{
21+
StatusCode: status,
22+
Body: ioutil.NopCloser(body),
23+
}
24+
f.responseStubs = append(f.responseStubs, resp)
25+
}
26+
27+
// RoundTrip satisfies http.RoundTripper
28+
func (f *FakeHTTP) RoundTrip(req *http.Request) (*http.Response, error) {
29+
if len(f.responseStubs) <= f.count {
30+
return nil, fmt.Errorf("FakeHTTP: missing response stub for request %d", f.count)
31+
}
32+
resp := f.responseStubs[f.count]
33+
f.count++
34+
resp.Request = req
35+
f.Requests = append(f.Requests, req)
36+
return resp, nil
37+
}

0 commit comments

Comments
 (0)