Browse Source

Fix data race surrounding Client.Rate.

Previously, concurrent calls to Client.Do would have data races writing
Client.Rate. Protect it with a mutex.

Resolves #253.
Dmitri Shuralyov 10 years ago
parent
commit
e5099d7537
2 changed files with 29 additions and 18 deletions
  1. +17
    -6
      github/github.go
  2. +12
    -12
      github/github_test.go

+ 17
- 6
github/github.go View File

@ -17,6 +17,7 @@ import (
"reflect"
"strconv"
"strings"
"sync"
"time"
"github.com/google/go-querystring/query"
@ -64,11 +65,8 @@ type Client struct {
// User agent used when communicating with the GitHub API.
UserAgent string
// Rate specifies the current rate limit for the client as determined by the
// most recent API call. If the client is used in a multi-user application,
// this rate may not always be up-to-date. Call RateLimits() to check the
// current rate.
Rate Rate
rateMu sync.Mutex
rate Rate
// Services used for talking to different parts of the GitHub API.
Activity *ActivityService
@ -292,6 +290,17 @@ func (r *Response) populateRate() {
}
}
// Rate specifies the current rate limit for the client as determined by the
// most recent API call. If the client is used in a multi-user application,
// this rate may not always be up-to-date. Call RateLimits() to check the
// current rate.
func (c *Client) Rate() Rate {
c.rateMu.Lock()
rate := c.rate
c.rateMu.Unlock()
return rate
}
// Do sends an API request and returns the API response. The API response is
// JSON decoded and stored in the value pointed to by v, or returned as an
// error if an API error has occurred. If v implements the io.Writer
@ -307,7 +316,9 @@ func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
response := newResponse(resp)
c.Rate = response.Rate
c.rateMu.Lock()
c.rate = response.Rate
c.rateMu.Unlock()
err = CheckResponse(resp)
if err != nil {


+ 12
- 12
github/github_test.go View File

@ -378,28 +378,28 @@ func TestDo_rateLimit(t *testing.T) {
w.Header().Add(headerRateReset, "1372700873")
})
if got, want := client.Rate.Limit, 0; got != want {
if got, want := client.Rate().Limit, 0; got != want {
t.Errorf("Client rate limit = %v, want %v", got, want)
}
if got, want := client.Rate.Remaining, 0; got != want {
if got, want := client.Rate().Remaining, 0; got != want {
t.Errorf("Client rate remaining = %v, got %v", got, want)
}
if !client.Rate.Reset.IsZero() {
if !client.Rate().Reset.IsZero() {
t.Errorf("Client rate reset not initialized to zero value")
}
req, _ := client.NewRequest("GET", "/", nil)
client.Do(req, nil)
if got, want := client.Rate.Limit, 60; got != want {
if got, want := client.Rate().Limit, 60; got != want {
t.Errorf("Client rate limit = %v, want %v", got, want)
}
if got, want := client.Rate.Remaining, 59; got != want {
if got, want := client.Rate().Remaining, 59; got != want {
t.Errorf("Client rate remaining = %v, want %v", got, want)
}
reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC)
if client.Rate.Reset.UTC() != reset {
t.Errorf("Client rate reset = %v, want %v", client.Rate.Reset, reset)
if client.Rate().Reset.UTC() != reset {
t.Errorf("Client rate reset = %v, want %v", client.Rate().Reset, reset)
}
}
@ -418,15 +418,15 @@ func TestDo_rateLimit_errorResponse(t *testing.T) {
req, _ := client.NewRequest("GET", "/", nil)
client.Do(req, nil)
if got, want := client.Rate.Limit, 60; got != want {
if got, want := client.Rate().Limit, 60; got != want {
t.Errorf("Client rate limit = %v, want %v", got, want)
}
if got, want := client.Rate.Remaining, 59; got != want {
if got, want := client.Rate().Remaining, 59; got != want {
t.Errorf("Client rate remaining = %v, want %v", got, want)
}
reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC)
if client.Rate.Reset.UTC() != reset {
t.Errorf("Client rate reset = %v, want %v", client.Rate.Reset, reset)
if client.Rate().Reset.UTC() != reset {
t.Errorf("Client rate reset = %v, want %v", client.Rate().Reset, reset)
}
}
@ -453,7 +453,7 @@ func TestCheckResponse(t *testing.T) {
res := &http.Response{
Request: &http.Request{},
StatusCode: http.StatusBadRequest,
Body: ioutil.NopCloser(strings.NewReader(`{"message":"m",
Body: ioutil.NopCloser(strings.NewReader(`{"message":"m",
"errors": [{"resource": "r", "field": "f", "code": "c"}]}`)),
}
err := CheckResponse(res).(*ErrorResponse)


Loading…
Cancel
Save