<p>Hey, i am working on an API client library for Meetup.com. When i design API clients i often have a look at <a href="https://github.com/google/go-github" rel="nofollow">https://github.com/google/go-github</a>, because this is a pretty well structured design IMO.</p>
<p>The Meetup API has several auth methods. One is a typical API Client, another one OAuth2.</p>
<p>The signature of getting a new client is
func NewClient(httpClient *http.Client) *Client</p>
<p>The API key will be added as a query parameter <em>key</em> to every request. Now there are multiple possibilites to add the key to it and that whats this thread is about: Which one is in your opinion the smoother one? And more interesting, why?</p>
<p><strong>Option 1: Setter function</strong></p>
<pre><code>apiKey := "foo"
client := meetup.NewClient(nil)
client.SetAPIKey(apiKey)
group, resp, err := client.Groups.Get(...)
</code></pre>
<p>The <em>apiKey</em> will be added to every URL like</p>
<pre><code>if len(c.apiKey) > 0 {
q := u.Query()
q.Add("c", c.apiKey)
u.RawQuery = q.Encode()
}
</code></pre>
<p><strong>Option 2: http.RoundTripper</strong></p>
<p>I found this method while reading through <a href="https://github.com/google/go-github/blob/master/github/github.go#L817" rel="nofollow">go-github</a>.
The <em>NewClient</em> accepts a <em>http.Client</em>. So you can do something like</p>
<pre><code>type APIKeyTransport struct {
APIKey string
// Transport is the underlying HTTP transport to use when making requests.
// It will default to http.DefaultTransport if nil.
Transport http.RoundTripper
}
// RoundTrip implements the RoundTripper interface.
func (t *APIKeyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
if t.APIKey == "" {
return nil, errors.New("t.APIKey is empty")
}
// To set extra querystring params, we must make a copy of the Request so
// that we don't modify the Request we were given. This is required by the
// specification of http.RoundTripper.
req = cloneRequest(req)
q := req.URL.Query()
q.Set("key", t.APIKey)
req.URL.RawQuery = q.Encode()
// Make the HTTP request.
return t.transport().RoundTrip(req)
}
func (t *APIKeyTransport) Client() *http.Client {
return &http.Client{Transport: t}
}
func (t *APIKeyTransport) transport() http.RoundTripper {
if t.Transport != nil {
return t.Transport
}
return http.DefaultTransport
}
// cloneRequest returns a clone of the provided *http.Request. The clone is a
// shallow copy of the struct and its Header map.
func cloneRequest(r *http.Request) *http.Request {
// shallow copy of the struct
r2 := new(http.Request)
*r2 = *r
// deep copy of the Header
r2.Header = make(http.Header, len(r.Header))
for k, s := range r.Header {
r2.Header[k] = append([]string(nil), s...)
}
return r2
}
</code></pre>
<p>and use it like </p>
<pre><code>t := &meetup.APIKeyTransport{
APIKey: apiKey,
}
client := meetup.NewClient(t.Client())
</code></pre>
<p>to craft a new client. Of course, you can wrap it inside the API if you want for convenience reasons.</p>
<p><strong>Opinion</strong></p>
<p>What i like about this approach is that the Client API / library itself don't know anything about the auth method itself.
You just inject your custom http.Client. And this deals with the auth.
As go-github shows you can use this way to <a href="https://github.com/google/go-github#authentication" rel="nofollow">auth with OAuth2 as well</a>.
Or use the same approach for <a href="https://github.com/google/go-github/blob/master/github/github.go#L882" rel="nofollow">HTTP Basic Authentication</a>.</p>
<p>I like to get your opinion about this.
What would you as an user prefer?
What would you consider as better design from an library author perspective?
What do you think is more "idiomatic"?</p>
<hr/>**评论：**<br/><br/>SeerUD: <pre><p>I prefer the latter. Like you said, it keeps the library unaware of the authentication method being used, meaning that can be kept simpler. That should also make it a bit easier to test too. I'd just make some helpers to make it easy to generate those things, so you could do:</p>
<pre><code>client := meetup.NewClient(meetup.NewTransport(APIKey))
</code></pre>
<p>Or something</p></pre>captncraig: <pre><p>I take the other stance on this. I have written dozens of apps that use the githib api. Every single one of those has the same ~15 lines of faked up http.Transport hackery to make a client suitable for use with a given access token. It is getting old.</p>
<p>I agree that accepting the http.Client is the most flexible way for sure. By all means, provide that. But please please please provide a <code>NewClientWithAuthToken</code> in addition to save your users from having to dance with low level stdlib stuff to use your ting.</p></pre>titpetric: <pre><p>I'm not sure what is better, having a http.Client or a http.RoundTripper in the struct. I tended to navigate towards a http.Client in the past. With your GH client inspiration, the implementation is bound to <code>github.Client</code> and not <code>http.Client</code>, so in the end the implementation details for both approaches would be the same (ie, implement a layer above http.Client instead of using it directly). The implementation details of such authentication layers could be abstracted into an interface, which would take a simplified APIKeyTransport (or alternatives). Preference? Either. RoundTripper does the job without adding on your own interfaces.</p>
<p>As for idiomatic, I'd consider the main net/http API over what's bolted on above. For example, there's <a href="https://golang.org/src/net/http/request.go?s=11618:11677#L315" rel="nofollow">WithContext</a> which makes a shallow copy of the Request already along with internal context of the http request. GH clients opted for passing ctx over github.Client.Do which I consider to be unnecessary. Also, the implementation details between this and cloneRequest above lead me to believe that the above is tailored to GH specifically. GH uses different <code>Accept</code> headers on API calls (search for example), that return different responses, and perhaps unnecessarily copy the request objects headers (or the request object itself). I asumed this was because they didn't want headers leaking out of their RoundTripper implementation, but apparently they are fine with leaking of r.URL changes (URL *url.URL RawQuery). Also: somebody once called me insane and mentioned something like "what is this, 2013?" when I tried passing a Client ID over a URL query, and immediately rewrote that piece of code to send it over an <code>Authorization: Bearer [...]</code> header.</p>
<p>tl;dr by all means, use RoundTripper, but I'd stick closer to net/http APIs directly when it comes to creating and issuing requests, not sure if there's any value to wrapping them unless somebody wants to migrate the transport to a websocket client or a <code>net.Conn</code> for TCP. I don't think that really happens very often.</p></pre>
