Skip to content

Commit

Permalink
Remove 'context' dependency from API
Browse files Browse the repository at this point in the history
Since we only ever used context.Background() we can simplify our API
code by assuming the client only cares about the context.

This means we only need to setup and pass a client in from commands.

Leaving this only dependency when calling API functions we can also
remove a type and simplify our commands as well.
  • Loading branch information
Zachary Scott committed Dec 27, 2018
1 parent 7e18181 commit 2a97807
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 162 deletions.
119 changes: 56 additions & 63 deletions api/api.go

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@ func prepareRequest(ctx context.Context, address string, request *Request) (*htt
// Run sends an HTTP request to the GraphQL server and deserializes the response or returns an error.
// TODO(zzak): This function is fairly complex, we should refactor it
// nolint: gocyclo
func (cl *Client) Run(ctx context.Context, request *Request, resp interface{}) error {
func (cl *Client) Run(request *Request, resp interface{}) error {
l := log.New(os.Stderr, "", 0)
ctx := context.Background()

select {
case <-ctx.Done():
Expand Down
33 changes: 7 additions & 26 deletions client/client_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package client

import (
"context"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"regexp"
"testing"
"time"
)

func TestServerAddress(t *testing.T) {
Expand Down Expand Up @@ -72,15 +70,12 @@ func TestDoJSON(t *testing.T) {
}))
defer srv.Close()

ctx := context.Background()
client := NewClient(srv.URL, "/", "token", false)

ctx, cancel := context.WithTimeout(ctx, 1*time.Second)
defer cancel()
var resp struct {
Something string
}
err := client.Run(ctx, &Request{Query: "query {}"}, &resp)
err := client.Run(&Request{Query: "query {}"}, &resp)
if err != nil {
t.Errorf(err.Error())
}
Expand Down Expand Up @@ -111,8 +106,6 @@ func TestQueryJSON(t *testing.T) {
}
}))
defer srv.Close()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

client := NewClient(srv.URL, "/", "token", false)

Expand All @@ -131,7 +124,7 @@ func TestQueryJSON(t *testing.T) {
var resp struct {
Value string
}
err := client.Run(ctx, req, &resp)
err := client.Run(req, &resp)
if err != nil {
t.Errorf(err.Error())
}
Expand Down Expand Up @@ -174,15 +167,11 @@ func TestDoJSONErr(t *testing.T) {

defer server.Close()

ctx := context.Background()
client := NewClient(server.URL, "/", "token", false)

ctx, cancel := context.WithTimeout(ctx, 1*time.Second)
defer cancel()

var responseData map[string]interface{}

err := client.Run(ctx, &Request{Query: "query {}"}, &responseData)
err := client.Run(&Request{Query: "query {}"}, &responseData)
if err.Error() != "Something went wrong\nSomething else went wrong" {
t.Errorf(err.Error())
}
Expand All @@ -202,8 +191,6 @@ func TestHeader(t *testing.T) {
}
}))
defer srv.Close()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

client := NewClient(srv.URL, "/", "token", false)

Expand All @@ -213,7 +200,7 @@ func TestHeader(t *testing.T) {
var resp struct {
Value string
}
err := client.Run(ctx, req, &resp)
err := client.Run(req, &resp)
if err != nil {
t.Errorf(err.Error())
}
Expand All @@ -240,16 +227,14 @@ func TestStatusCode200(t *testing.T) {
}
}))
defer srv.Close()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

client := NewClient(srv.URL, "/", "token", false)

req := NewUnauthorizedRequest("query {}")

var resp interface{}

err := client.Run(ctx, req, &resp)
err := client.Run(req, &resp)
if err != nil {
t.Errorf(err.Error())
}
Expand All @@ -272,16 +257,14 @@ func TestStatusCode500(t *testing.T) {
}
}))
defer srv.Close()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

client := NewClient(srv.URL, "/", "token", false)

req := NewUnauthorizedRequest("query {}")

var resp interface{}

err := client.Run(ctx, req, &resp)
err := client.Run(req, &resp)
if err == nil {
t.Error("expected error")
}
Expand All @@ -308,16 +291,14 @@ func TestStatusCode413(t *testing.T) {
}
}))
defer srv.Close()
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

client := NewClient(srv.URL, "/", "token", false)

req := NewUnauthorizedRequest("query {}")

var resp interface{}

err := client.Run(ctx, req, &resp)
err := client.Run(req, &resp)
if err == nil {
t.Error("expected error")
}
Expand Down
26 changes: 10 additions & 16 deletions cmd/config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"context"
"fmt"

"github.com/CircleCI-Public/circleci-cli/api"
Expand All @@ -17,9 +16,9 @@ import (
const defaultConfigPath = ".circleci/config.yml"

type configOptions struct {
cfg *settings.Config
apiOpts api.Options
args []string
cfg *settings.Config
cl *client.Client
args []string
}

// Path to the config.yml file to operate on.
Expand All @@ -32,8 +31,7 @@ var configAnnotations = map[string]string{

func newConfigCommand(config *settings.Config) *cobra.Command {
opts := configOptions{
apiOpts: api.Options{},
cfg: config,
cfg: config,
}

configCmd := &cobra.Command{
Expand All @@ -46,8 +44,7 @@ func newConfigCommand(config *settings.Config) *cobra.Command {
Short: "Pack up your CircleCI configuration into a single file.",
PreRun: func(cmd *cobra.Command, args []string) {
opts.args = args
opts.apiOpts.Context = context.Background()
opts.apiOpts.Client = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
opts.cl = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
},
RunE: func(_ *cobra.Command, _ []string) error {
return packConfig(opts)
Expand All @@ -63,8 +60,7 @@ func newConfigCommand(config *settings.Config) *cobra.Command {
Short: "Check that the config file is well formed.",
PreRun: func(cmd *cobra.Command, args []string) {
opts.args = args
opts.apiOpts.Context = context.Background()
opts.apiOpts.Client = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
opts.cl = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
},
RunE: func(_ *cobra.Command, _ []string) error {
return validateConfig(opts)
Expand All @@ -83,8 +79,7 @@ func newConfigCommand(config *settings.Config) *cobra.Command {
Short: "Process the config.",
PreRun: func(cmd *cobra.Command, args []string) {
opts.args = args
opts.apiOpts.Context = context.Background()
opts.apiOpts.Client = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
opts.cl = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
},
RunE: func(_ *cobra.Command, _ []string) error {
return processConfig(opts)
Expand All @@ -99,8 +94,7 @@ func newConfigCommand(config *settings.Config) *cobra.Command {
Short: "Migrate a pre-release 2.0 config to the official release version",
PreRun: func(cmd *cobra.Command, args []string) {
opts.args = args
opts.apiOpts.Context = context.Background()
opts.apiOpts.Client = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
opts.cl = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
},
RunE: func(_ *cobra.Command, _ []string) error {
return migrateConfig(opts)
Expand Down Expand Up @@ -133,7 +127,7 @@ func validateConfig(opts configOptions) error {
path = opts.args[0]
}

_, err := api.ConfigQuery(opts.apiOpts, path)
_, err := api.ConfigQuery(opts.cl, path)

if err != nil {
return err
Expand All @@ -149,7 +143,7 @@ func validateConfig(opts configOptions) error {
}

func processConfig(opts configOptions) error {
response, err := api.ConfigQuery(opts.apiOpts, opts.args[0])
response, err := api.ConfigQuery(opts.cl, opts.args[0])

if err != nil {
return err
Expand Down
17 changes: 7 additions & 10 deletions cmd/diagnostic.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"context"
"fmt"
"os"

Expand All @@ -13,24 +12,22 @@ import (
)

type diagnosticOptions struct {
cfg *settings.Config
apiOpts api.Options
args []string
cfg *settings.Config
cl *client.Client
args []string
}

func newDiagnosticCommand(config *settings.Config) *cobra.Command {
opts := diagnosticOptions{
apiOpts: api.Options{},
cfg: config,
cfg: config,
}

diagnosticCommand := &cobra.Command{
Use: "diagnostic",
Short: "Check the status of your CircleCI CLI.",
PreRun: func(cmd *cobra.Command, args []string) {
opts.args = args
opts.apiOpts.Context = context.Background()
opts.apiOpts.Client = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
opts.cl = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
},
RunE: func(_ *cobra.Command, _ []string) error {
return diagnostic(opts)
Expand All @@ -56,7 +53,7 @@ https://circleci.com/account/api`)

fmt.Println("Trying an introspection query on API... ")

responseIntro, err := api.IntrospectionQuery(opts.apiOpts)
responseIntro, err := api.IntrospectionQuery(opts.cl)
if responseIntro.Schema.QueryType.Name == "" {
fmt.Println("Unable to make a query against the GraphQL API, please check your settings")
if err != nil {
Expand All @@ -73,7 +70,7 @@ https://circleci.com/account/api`)
}
}

responseWho, err := api.WhoamiQuery(opts.apiOpts)
responseWho, err := api.WhoamiQuery(opts.cl)

if err != nil {
return err
Expand Down
15 changes: 6 additions & 9 deletions cmd/namespace.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"context"
"fmt"
"strings"

Expand All @@ -12,15 +11,14 @@ import (
)

type namespaceOptions struct {
apiOpts api.Options
cfg *settings.Config
args []string
cfg *settings.Config
cl *client.Client
args []string
}

func newNamespaceCommand(config *settings.Config) *cobra.Command {
opts := namespaceOptions{
apiOpts: api.Options{},
cfg: config,
cfg: config,
}

namespaceCmd := &cobra.Command{
Expand All @@ -35,8 +33,7 @@ func newNamespaceCommand(config *settings.Config) *cobra.Command {
Please note that at this time all namespaces created in the registry are world-readable.`,
PreRun: func(cmd *cobra.Command, args []string) {
opts.args = args
opts.apiOpts.Context = context.Background()
opts.apiOpts.Client = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
opts.cl = client.NewClient(config.Host, config.Endpoint, config.Token, config.Debug)
},
RunE: func(_ *cobra.Command, _ []string) error {
return createNamespace(opts)
Expand All @@ -57,7 +54,7 @@ Please note that at this time all namespaces created in the registry are world-r
func createNamespace(opts namespaceOptions) error {
namespaceName := opts.args[0]

_, err := api.CreateNamespace(opts.apiOpts, namespaceName, opts.args[2], strings.ToUpper(opts.args[1]))
_, err := api.CreateNamespace(opts.cl, namespaceName, opts.args[2], strings.ToUpper(opts.args[1]))

if err != nil {
return err
Expand Down
Loading

0 comments on commit 2a97807

Please sign in to comment.