Rewrite of Go client.

[antihax]

PR checklist

• Read the contribution guildelines.
• Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)
• Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Rewrite of Go client. This will need community review. This is modeled after some other API clients (go-github, godo, etc).

• Removed most of go-resty.
• Moved http.Client to APIClient structure (for keep-alive, http2 multiplexing).
• Authentication through contexts.
• Able to pass custom http.Clients (caching across clusters, custom timeouts, rate limiting, tracing).
• Catches http errors
• Optional parameters via map[string]interface
• Documentation updated
• Examples in README.md

[antihax]

Hmm. so the test environment is going to need golang.org/x/oauth2 and golang.org/x/context.

Context is now a standard lib in Go 1.7 but i use the old path for compatibility with Go 1.6

Perhaps the script can go go get prior to go test ?

The tests pass locally, I also have it in production using oauth2.

[wing328]

@antihax thanks for the PR. The CI failure with Appveyor can be ignored.

cc @guohuang @neilotoole

[wing328]

@wy-z can you please also review this particular change? Your feedback is much appreciated.

[wy-z]

It seems that auth context should be a parameter of NewAPIClient ... 😶
What is your opinion? 😃 @antihax

go-github

import "golang.org/x/oauth2"

func main() {
  ts := oauth2.StaticTokenSource(
    &oauth2.Token{AccessToken: "... your access token ..."},
  )
  tc := oauth2.NewClient(oauth2.NoContext, ts)

  client := github.NewClient(tc)

  // list all repositories for the authenticated user
  repos, _, err := client.Repositories.List("", nil)
}

godo

import "golang.org/x/oauth2"

pat := "mytoken"
type TokenSource struct {
    AccessToken string
}

func (t *TokenSource) Token() (*oauth2.Token, error) {
    token := &oauth2.Token{
        AccessToken: t.AccessToken,
    }
    return token, nil
}

tokenSource := &TokenSource{
    AccessToken: pat,
}
oauthClient := oauth2.NewClient(oauth2.NoContext, tokenSource)
client := godo.NewClient(oauthClient)

[wing328]

@wy-z yes, that's one way to do it. For API clients in other languages (e.g. Python, Ruby, PHP, C#), there's a Configuration class/object to store authentication-related setting so I suggest we do something similar for Go API client to make the developer experience more consistent across API clients in different languages (which is not say godo/go-github approach is bad at all)

[antihax]

I am still learning go, but this is my understanding so far. If I am off base, let me know.

Go is not really a client, it is a pool of connections that get reused with requests multiplexed (http2) over the same connection at the same time with idle connections reused rather than opening new ones.

So in the scenario I have, we can (plan to) have upwards of 30,000 oauth tokens which I need to pull API calls on concurrently. Feeding the token in the request seems like the most optimal design since it is a request header, and we have stateful requests/response.

If we attach the token to the client, we would be forced to create/destroy 30,000 and clients. If we offer provisions to change the token, we will have to do so with a mutex to prevent racing on the token through the request and lower our concurrency.

For example, this is what I have in production.
3-4 servers, 1 client each, 50 go routines each calling the function below (total of 150-200 go routines).

func (c *EVEConsumer) assetsCheckQueue(r redis.Conn) error {
	ret, err := r.Do("SPOP", "EVEDATA_assetQueue")
	if err != nil || ret == nil {
		return err
	}

	cid, err := redis.Int(ret, err)
	if err != nil {
		return err
	}

	token, err := c.getToken(cid)

	// authentication token context for destination char
	auth := context.WithValue(context.TODO(), esi.ContextOAuth2, token)

	assets, res, err := c.ctx.ESI.AssetsApi.GetCharactersCharacterIdAssets(auth, (int32)(cid), nil)
	if err != nil {
		return err
	} else {
		...
	}
	return nil
}

[vpolouchkine]

There's an issue here if the spec defines multiple security definitions wrt OAuth2.

securityDefinitions:
  Client Authentication:
    type: oauth2
    flow: application
    tokenUrl: foo
  User Authentication:
    type: oauth2
    flow: accessCode
    tokenUrl: foo
    authorizationUrl: bar

This generates endpoints like this:

func (a PublicApiService) Get(ctx context.Context, ctx context.Context, param string) (SomeResponse,  *http.Response, error) {}

[vpolouchkine]

I love that this now accepts http.Clients, but I'm not sold on using Context.context for passing tokens. This article has some counter-arguments. Never seen a Go SDK take this approach.

[antihax]

I am rewriting this over the weekend hopefully and can resolve the double ctx issue. I am going back to the config with one TokenSource default and then offer a .WithOAuth(tokenSource).Operation() to allow tokens per request for those who need it.

The reasoning with ctx is that you can pass various types of authentication or none at all.

Again much of this comes about because I needed request based tokens, instead of tokens tied to a client.

I have run into a number of other issues since this PR... No Date format in Go, another issue around []int in returns, etc.

[antihax]

So I am unfortunately back to contexts again... chained functions are not safe. https://play.golang.org/p/YepaybwrJH

If anyone has any other suggestions let me know. My original problem is that is i have one APIClient, ~200 concurrent workers running operations, and 200+ TokenSource's for OAuth2. I would like this to function concurrently without a mutex or running multiple APIClients.

[ePaul]

From a first look-through, this seems to solve the {{paramName}}/{{baseName}} problem from #4898 for go. Thanks.

[wing328]

Closed via #5037