From e98c3509c65c13550f11fb751953d42c161eaedc Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Sat, 14 Oct 2017 12:03:25 -0700 Subject: [PATCH 1/8] Moved mutex to BackendConfiguration --- Makefile | 4 ++-- error_test.go | 4 +--- stripe.go | 22 +++++++++++----------- stripe_test.go | 20 ++++++++++++++++++++ 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index 79b9ba3ecf..4f3332ea0d 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ all: test bench vet check-gofmt bench: - go test -bench . -run "Benchmark" ./form + go test --race -bench . -run "Benchmark" ./form build: go build ./... @@ -10,7 +10,7 @@ check-gofmt: scripts/check_gofmt.sh test: - go test ./... + go test --race ./... vet: go vet ./... diff --git a/error_test.go b/error_test.go index 427aab772e..23be3f089f 100644 --- a/error_test.go +++ b/error_test.go @@ -23,9 +23,7 @@ func TestErrorResponse(t *testing.T) { defer ts.Close() SetBackend("api", BackendConfiguration{ - APIBackend, - ts.URL, - &http.Client{}, + Type: APIBackend, URL: ts.URL, HTTPClient: &http.Client{}, }) err := GetBackend(APIBackend).Call("GET", "/v1/account", "sk_test_badKey", nil, nil, nil) diff --git a/stripe.go b/stripe.go index 4a63e2edb4..e7a6758227 100644 --- a/stripe.go +++ b/stripe.go @@ -76,6 +76,7 @@ type Backend interface { // BackendConfiguration is the internal implementation for making HTTP calls to Stripe. type BackendConfiguration struct { + sync.Mutex Type SupportedBackend URL string HTTPClient *http.Client @@ -101,7 +102,6 @@ const ( // Backends are the currently supported endpoints. type Backends struct { - mu sync.Mutex API, Uploads Backend } @@ -160,28 +160,27 @@ func SetHTTPClient(client *http.Client) { func NewBackends(httpClient *http.Client) *Backends { return &Backends{ API: BackendConfiguration{ - APIBackend, APIURL, httpClient}, + Type: APIBackend, URL: APIURL, HTTPClient: httpClient}, Uploads: BackendConfiguration{ - UploadsBackend, UploadsURL, httpClient}, + Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient}, } } // GetBackend returns the currently used backend in the binding. func GetBackend(backend SupportedBackend) Backend { var ret Backend - backends.mu.Lock() - defer backends.mu.Unlock() - switch backend { case APIBackend: if backends.API == nil { - backends.API = BackendConfiguration{backend, apiURL, httpClient} + backends.API = BackendConfiguration{ + Type: backend, URL: apiURL, HTTPClient: httpClient} } ret = backends.API case UploadsBackend: if backends.Uploads == nil { - backends.Uploads = BackendConfiguration{backend, uploadsURL, httpClient} + backends.Uploads = BackendConfiguration{ + Type: backend, URL: uploadsURL, HTTPClient: httpClient} } ret = backends.Uploads } @@ -191,9 +190,6 @@ func GetBackend(backend SupportedBackend) Backend { // SetBackend sets the backend used in the binding. func SetBackend(backend SupportedBackend, b Backend) { - backends.mu.Lock() - defer backends.mu.Unlock() - switch backend { case APIBackend: backends.API = b @@ -245,6 +241,8 @@ func (s BackendConfiguration) CallMultipart(method, path, key, boundary string, // NewRequest is used by Call to generate an http.Request. It handles encoding // parameters and attaching the appropriate headers. func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, body io.Reader, params *Params) (*http.Request, error) { + s.Lock() + defer s.Unlock() if !strings.HasPrefix(path, "/") { path = "/" + path } @@ -300,6 +298,8 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, // the backend's HTTP client to execute the request and unmarshals the response // into v. It also handles unmarshaling errors returned by the API. func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error { + s.Lock() + defer s.Unlock() if LogLevel > 1 { Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path) } diff --git a/stripe_test.go b/stripe_test.go index b88e546dea..9f77cc2d7e 100644 --- a/stripe_test.go +++ b/stripe_test.go @@ -5,6 +5,7 @@ import ( "net/http" "regexp" "runtime" + "sync" "testing" assert "github.com/stretchr/testify/require" @@ -22,6 +23,25 @@ func TestCheckinUseBearerAuth(t *testing.T) { assert.Equal(t, "Bearer "+key, req.Header.Get("Authorization")) } +// TestMultipleAPICalls will fail the test run if a race condition is thrown while running multople NewRequest calls. +func TestMultipleAPICalls(t *testing.T) { + wg := &sync.WaitGroup{} + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + c := &stripe.BackendConfiguration{URL: stripe.APIURL} + key := "apiKey" + + req, err := c.NewRequest("", "", key, "", nil, nil) + assert.NoError(t, err) + + assert.Equal(t, "Bearer "+key, req.Header.Get("Authorization")) + }() + } + wg.Wait() +} + func TestIdempotencyKey(t *testing.T) { c := &stripe.BackendConfiguration{URL: stripe.APIURL} p := &stripe.Params{IdempotencyKey: "idempotency-key"} From e611f43699ce1070cdba6085ec0289cd98e90b61 Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 06:51:58 -0700 Subject: [PATCH 2/8] Added exported mutex to solve go-vet and pass CI. --- error_test.go | 3 ++- stripe.go | 18 +++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/error_test.go b/error_test.go index 23be3f089f..defcabf14a 100644 --- a/error_test.go +++ b/error_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "sync" "testing" assert "github.com/stretchr/testify/require" @@ -23,7 +24,7 @@ func TestErrorResponse(t *testing.T) { defer ts.Close() SetBackend("api", BackendConfiguration{ - Type: APIBackend, URL: ts.URL, HTTPClient: &http.Client{}, + Mutex: &sync.Mutex{}, Type: APIBackend, URL: ts.URL, HTTPClient: &http.Client{}, }) err := GetBackend(APIBackend).Call("GET", "/v1/account", "sk_test_badKey", nil, nil, nil) diff --git a/stripe.go b/stripe.go index e7a6758227..407e9edf86 100644 --- a/stripe.go +++ b/stripe.go @@ -76,7 +76,7 @@ type Backend interface { // BackendConfiguration is the internal implementation for making HTTP calls to Stripe. type BackendConfiguration struct { - sync.Mutex + Mutex *sync.Mutex Type SupportedBackend URL string HTTPClient *http.Client @@ -160,9 +160,9 @@ func SetHTTPClient(client *http.Client) { func NewBackends(httpClient *http.Client) *Backends { return &Backends{ API: BackendConfiguration{ - Type: APIBackend, URL: APIURL, HTTPClient: httpClient}, + Mutex: &sync.Mutex{}, Type: APIBackend, URL: APIURL, HTTPClient: httpClient}, Uploads: BackendConfiguration{ - Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient}, + Mutex: &sync.Mutex{}, Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient}, } } @@ -173,14 +173,14 @@ func GetBackend(backend SupportedBackend) Backend { case APIBackend: if backends.API == nil { backends.API = BackendConfiguration{ - Type: backend, URL: apiURL, HTTPClient: httpClient} + Mutex: &sync.Mutex{}, Type: backend, URL: apiURL, HTTPClient: httpClient} } ret = backends.API case UploadsBackend: if backends.Uploads == nil { backends.Uploads = BackendConfiguration{ - Type: backend, URL: uploadsURL, HTTPClient: httpClient} + Mutex: &sync.Mutex{}, Type: backend, URL: uploadsURL, HTTPClient: httpClient} } ret = backends.Uploads } @@ -241,8 +241,8 @@ func (s BackendConfiguration) CallMultipart(method, path, key, boundary string, // NewRequest is used by Call to generate an http.Request. It handles encoding // parameters and attaching the appropriate headers. func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, body io.Reader, params *Params) (*http.Request, error) { - s.Lock() - defer s.Unlock() + s.Mutex.Lock() + defer s.Mutex.Unlock() if !strings.HasPrefix(path, "/") { path = "/" + path } @@ -298,8 +298,8 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, // the backend's HTTP client to execute the request and unmarshals the response // into v. It also handles unmarshaling errors returned by the API. func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error { - s.Lock() - defer s.Unlock() + s.Mutex.Lock() + defer s.Mutex.Unlock() if LogLevel > 1 { Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path) } From 58e3196279f72a94a6eaa7f99882f9292b22e12e Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 07:04:39 -0700 Subject: [PATCH 3/8] Updated tests and stripe.go, not sure how this was passing locally. Did not update branch when switching environments --- stripe_test.go | 18 +++++++++--------- testing/testing.go | 2 ++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/stripe_test.go b/stripe_test.go index 9f77cc2d7e..fc0bd4fcf0 100644 --- a/stripe_test.go +++ b/stripe_test.go @@ -14,7 +14,7 @@ import ( ) func TestCheckinUseBearerAuth(t *testing.T) { - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} key := "apiKey" req, err := c.NewRequest("", "", key, "", nil, nil) @@ -30,7 +30,7 @@ func TestMultipleAPICalls(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} key := "apiKey" req, err := c.NewRequest("", "", key, "", nil, nil) @@ -43,7 +43,7 @@ func TestMultipleAPICalls(t *testing.T) { } func TestIdempotencyKey(t *testing.T) { - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} p := &stripe.Params{IdempotencyKey: "idempotency-key"} req, err := c.NewRequest("", "", "", "", nil, p) @@ -53,7 +53,7 @@ func TestIdempotencyKey(t *testing.T) { } func TestStripeAccount(t *testing.T) { - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} p := &stripe.Params{StripeAccount: TestMerchantID} req, err := c.NewRequest("", "", "", "", nil, p) @@ -72,7 +72,7 @@ func TestStripeAccount(t *testing.T) { } func TestUserAgent(t *testing.T) { - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -94,7 +94,7 @@ func TestUserAgentWithAppInfo(t *testing.T) { stripe.SetAppInfo(appInfo) defer stripe.SetAppInfo(nil) - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -108,7 +108,7 @@ func TestUserAgentWithAppInfo(t *testing.T) { } func TestStripeClientUserAgent(t *testing.T) { - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -142,7 +142,7 @@ func TestStripeClientUserAgentWithAppInfo(t *testing.T) { stripe.SetAppInfo(appInfo) defer stripe.SetAppInfo(nil) - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -161,7 +161,7 @@ func TestStripeClientUserAgentWithAppInfo(t *testing.T) { } func TestResponseToError(t *testing.T) { - c := &stripe.BackendConfiguration{URL: stripe.APIURL} + c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} // A test response that includes a status code and request ID. res := &http.Response{ diff --git a/testing/testing.go b/testing/testing.go index 55dc955fbb..880da2cdac 100644 --- a/testing/testing.go +++ b/testing/testing.go @@ -6,6 +6,7 @@ import ( "os" "strconv" "strings" + "sync" stripe "github.com/stripe/stripe-go" "github.com/stripe/stripe-go/form" @@ -56,6 +57,7 @@ func init() { stripe.Key = "sk_test_myTestKey" stripe.SetBackend("api", stripe.BackendConfiguration{ + Mutex: &sync.Mutex{}, Type: stripe.APIBackend, URL: "http://localhost:" + port + "/v1", HTTPClient: &http.Client{}, From 453a5bb728b4edb86dbb39a25305cf7666fa2801 Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 10:50:48 -0700 Subject: [PATCH 4/8] Changed method receiver to pointer, removing the need for a mutex address --- error_test.go | 5 ++--- stripe.go | 30 +++++++++++++++--------------- stripe_test.go | 18 +++++++++--------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/error_test.go b/error_test.go index defcabf14a..919a6137cc 100644 --- a/error_test.go +++ b/error_test.go @@ -4,7 +4,6 @@ import ( "fmt" "net/http" "net/http/httptest" - "sync" "testing" assert "github.com/stretchr/testify/require" @@ -23,8 +22,8 @@ func TestErrorResponse(t *testing.T) { })) defer ts.Close() - SetBackend("api", BackendConfiguration{ - Mutex: &sync.Mutex{}, Type: APIBackend, URL: ts.URL, HTTPClient: &http.Client{}, + SetBackend("api", &BackendConfiguration{ + Type: APIBackend, URL: ts.URL, HTTPClient: &http.Client{}, }) err := GetBackend(APIBackend).Call("GET", "/v1/account", "sk_test_badKey", nil, nil, nil) diff --git a/stripe.go b/stripe.go index 407e9edf86..7a8d40592b 100644 --- a/stripe.go +++ b/stripe.go @@ -76,10 +76,10 @@ type Backend interface { // BackendConfiguration is the internal implementation for making HTTP calls to Stripe. type BackendConfiguration struct { - Mutex *sync.Mutex Type SupportedBackend URL string HTTPClient *http.Client + sync.Mutex } // SupportedBackend is an enumeration of supported Stripe endpoints. @@ -159,10 +159,10 @@ func SetHTTPClient(client *http.Client) { // should only need to use this for testing purposes or on App Engine. func NewBackends(httpClient *http.Client) *Backends { return &Backends{ - API: BackendConfiguration{ - Mutex: &sync.Mutex{}, Type: APIBackend, URL: APIURL, HTTPClient: httpClient}, - Uploads: BackendConfiguration{ - Mutex: &sync.Mutex{}, Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient}, + API: &BackendConfiguration{ + Type: APIBackend, URL: APIURL, HTTPClient: httpClient}, + Uploads: &BackendConfiguration{ + Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient}, } } @@ -172,15 +172,15 @@ func GetBackend(backend SupportedBackend) Backend { switch backend { case APIBackend: if backends.API == nil { - backends.API = BackendConfiguration{ - Mutex: &sync.Mutex{}, Type: backend, URL: apiURL, HTTPClient: httpClient} + backends.API = &BackendConfiguration{ + Type: backend, URL: apiURL, HTTPClient: httpClient} } ret = backends.API case UploadsBackend: if backends.Uploads == nil { - backends.Uploads = BackendConfiguration{ - Mutex: &sync.Mutex{}, Type: backend, URL: uploadsURL, HTTPClient: httpClient} + backends.Uploads = &BackendConfiguration{ + Type: backend, URL: uploadsURL, HTTPClient: httpClient} } ret = backends.Uploads } @@ -199,7 +199,7 @@ func SetBackend(backend SupportedBackend, b Backend) { } // Call is the Backend.Call implementation for invoking Stripe APIs. -func (s BackendConfiguration) Call(method, path, key string, form *form.Values, params *Params, v interface{}) error { +func (s *BackendConfiguration) Call(method, path, key string, form *form.Values, params *Params, v interface{}) error { var body io.Reader if form != nil && !form.Empty() { data := form.Encode() @@ -223,7 +223,7 @@ func (s BackendConfiguration) Call(method, path, key string, form *form.Values, } // CallMultipart is the Backend.CallMultipart implementation for invoking Stripe APIs. -func (s BackendConfiguration) CallMultipart(method, path, key, boundary string, body io.Reader, params *Params, v interface{}) error { +func (s *BackendConfiguration) CallMultipart(method, path, key, boundary string, body io.Reader, params *Params, v interface{}) error { contentType := "multipart/form-data; boundary=" + boundary req, err := s.NewRequest(method, path, key, contentType, body, params) @@ -241,8 +241,8 @@ func (s BackendConfiguration) CallMultipart(method, path, key, boundary string, // NewRequest is used by Call to generate an http.Request. It handles encoding // parameters and attaching the appropriate headers. func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, body io.Reader, params *Params) (*http.Request, error) { - s.Mutex.Lock() - defer s.Mutex.Unlock() + s.Lock() + defer s.Unlock() if !strings.HasPrefix(path, "/") { path = "/" + path } @@ -298,8 +298,8 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, // the backend's HTTP client to execute the request and unmarshals the response // into v. It also handles unmarshaling errors returned by the API. func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error { - s.Mutex.Lock() - defer s.Mutex.Unlock() + s.Lock() + defer s.Unlock() if LogLevel > 1 { Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path) } diff --git a/stripe_test.go b/stripe_test.go index fc0bd4fcf0..9f77cc2d7e 100644 --- a/stripe_test.go +++ b/stripe_test.go @@ -14,7 +14,7 @@ import ( ) func TestCheckinUseBearerAuth(t *testing.T) { - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} key := "apiKey" req, err := c.NewRequest("", "", key, "", nil, nil) @@ -30,7 +30,7 @@ func TestMultipleAPICalls(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} key := "apiKey" req, err := c.NewRequest("", "", key, "", nil, nil) @@ -43,7 +43,7 @@ func TestMultipleAPICalls(t *testing.T) { } func TestIdempotencyKey(t *testing.T) { - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} p := &stripe.Params{IdempotencyKey: "idempotency-key"} req, err := c.NewRequest("", "", "", "", nil, p) @@ -53,7 +53,7 @@ func TestIdempotencyKey(t *testing.T) { } func TestStripeAccount(t *testing.T) { - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} p := &stripe.Params{StripeAccount: TestMerchantID} req, err := c.NewRequest("", "", "", "", nil, p) @@ -72,7 +72,7 @@ func TestStripeAccount(t *testing.T) { } func TestUserAgent(t *testing.T) { - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -94,7 +94,7 @@ func TestUserAgentWithAppInfo(t *testing.T) { stripe.SetAppInfo(appInfo) defer stripe.SetAppInfo(nil) - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -108,7 +108,7 @@ func TestUserAgentWithAppInfo(t *testing.T) { } func TestStripeClientUserAgent(t *testing.T) { - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -142,7 +142,7 @@ func TestStripeClientUserAgentWithAppInfo(t *testing.T) { stripe.SetAppInfo(appInfo) defer stripe.SetAppInfo(nil) - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} req, err := c.NewRequest("", "", "", "", nil, nil) assert.NoError(t, err) @@ -161,7 +161,7 @@ func TestStripeClientUserAgentWithAppInfo(t *testing.T) { } func TestResponseToError(t *testing.T) { - c := &stripe.BackendConfiguration{Mutex: &sync.Mutex{}, URL: stripe.APIURL} + c := &stripe.BackendConfiguration{URL: stripe.APIURL} // A test response that includes a status code and request ID. res := &http.Response{ From 5935c86521d15714334a8843d69b39599014a2fe Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 10:59:00 -0700 Subject: [PATCH 5/8] Updated testing BackendConfig -- no idea why this passes with 'make' locally --- testing/testing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/testing.go b/testing/testing.go index 880da2cdac..e7447846e1 100644 --- a/testing/testing.go +++ b/testing/testing.go @@ -56,7 +56,7 @@ func init() { } stripe.Key = "sk_test_myTestKey" - stripe.SetBackend("api", stripe.BackendConfiguration{ + stripe.SetBackend("api", &stripe.BackendConfiguration{ Mutex: &sync.Mutex{}, Type: stripe.APIBackend, URL: "http://localhost:" + port + "/v1", From 745a98f73e18571b999411ce8fd4aec16c3f7608 Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 11:02:11 -0700 Subject: [PATCH 6/8] Removed mutex. --- testing/testing.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/testing/testing.go b/testing/testing.go index e7447846e1..3fcecc5e51 100644 --- a/testing/testing.go +++ b/testing/testing.go @@ -6,7 +6,6 @@ import ( "os" "strconv" "strings" - "sync" stripe "github.com/stripe/stripe-go" "github.com/stripe/stripe-go/form" @@ -57,7 +56,6 @@ func init() { stripe.Key = "sk_test_myTestKey" stripe.SetBackend("api", &stripe.BackendConfiguration{ - Mutex: &sync.Mutex{}, Type: stripe.APIBackend, URL: "http://localhost:" + port + "/v1", HTTPClient: &http.Client{}, From ab5be532db7841c33c7a38926613a4f0e31b6da4 Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 13:42:01 -0700 Subject: [PATCH 7/8] Named mutex --- stripe.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/stripe.go b/stripe.go index 7a8d40592b..6cb6a4cbe6 100644 --- a/stripe.go +++ b/stripe.go @@ -76,10 +76,10 @@ type Backend interface { // BackendConfiguration is the internal implementation for making HTTP calls to Stripe. type BackendConfiguration struct { + mu sync.Mutex Type SupportedBackend URL string HTTPClient *http.Client - sync.Mutex } // SupportedBackend is an enumeration of supported Stripe endpoints. @@ -241,8 +241,8 @@ func (s *BackendConfiguration) CallMultipart(method, path, key, boundary string, // NewRequest is used by Call to generate an http.Request. It handles encoding // parameters and attaching the appropriate headers. func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, body io.Reader, params *Params) (*http.Request, error) { - s.Lock() - defer s.Unlock() + s.mu.Lock() + defer s.mu.Unlock() if !strings.HasPrefix(path, "/") { path = "/" + path } @@ -298,8 +298,8 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, // the backend's HTTP client to execute the request and unmarshals the response // into v. It also handles unmarshaling errors returned by the API. func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error { - s.Lock() - defer s.Unlock() + s.mu.Lock() + defer s.mu.Unlock() if LogLevel > 1 { Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path) } From a8ae95163f86b133d1a7f7bae4390ee2486d04b5 Mon Sep 17 00:00:00 2001 From: Nathan Hyland Date: Mon, 16 Oct 2017 16:50:50 -0700 Subject: [PATCH 8/8] Updated with PR comments --- Makefile | 4 ++-- error_test.go | 4 +++- stripe.go | 40 ++++++++++++++++++++++------------------ 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/Makefile b/Makefile index 4f3332ea0d..c2cadaed60 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ all: test bench vet check-gofmt bench: - go test --race -bench . -run "Benchmark" ./form + go test -race -bench . -run "Benchmark" ./form build: go build ./... @@ -10,7 +10,7 @@ check-gofmt: scripts/check_gofmt.sh test: - go test --race ./... + go test -race ./... vet: go vet ./... diff --git a/error_test.go b/error_test.go index 919a6137cc..693579e4c5 100644 --- a/error_test.go +++ b/error_test.go @@ -23,7 +23,9 @@ func TestErrorResponse(t *testing.T) { defer ts.Close() SetBackend("api", &BackendConfiguration{ - Type: APIBackend, URL: ts.URL, HTTPClient: &http.Client{}, + APIBackend, + ts.URL, + &http.Client{}, }) err := GetBackend(APIBackend).Call("GET", "/v1/account", "sk_test_badKey", nil, nil, nil) diff --git a/stripe.go b/stripe.go index 6cb6a4cbe6..8a6f731fb8 100644 --- a/stripe.go +++ b/stripe.go @@ -76,7 +76,6 @@ type Backend interface { // BackendConfiguration is the internal implementation for making HTTP calls to Stripe. type BackendConfiguration struct { - mu sync.Mutex Type SupportedBackend URL string HTTPClient *http.Client @@ -103,6 +102,7 @@ const ( // Backends are the currently supported endpoints. type Backends struct { API, Uploads Backend + mu sync.RWMutex } // stripeClientUserAgent contains information about the current runtime which @@ -160,32 +160,40 @@ func SetHTTPClient(client *http.Client) { func NewBackends(httpClient *http.Client) *Backends { return &Backends{ API: &BackendConfiguration{ - Type: APIBackend, URL: APIURL, HTTPClient: httpClient}, + APIBackend, APIURL, httpClient}, Uploads: &BackendConfiguration{ - Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient}, + UploadsBackend, UploadsURL, httpClient}, } } // GetBackend returns the currently used backend in the binding. func GetBackend(backend SupportedBackend) Backend { - var ret Backend switch backend { case APIBackend: - if backends.API == nil { - backends.API = &BackendConfiguration{ - Type: backend, URL: apiURL, HTTPClient: httpClient} + backends.mu.RLock() + ret := backends.API + backends.mu.RUnlock() + if ret != nil { + return ret } - - ret = backends.API + backends.mu.Lock() + defer backends.mu.Unlock() + backends.API = &BackendConfiguration{backend, apiURL, httpClient} + return backends.API case UploadsBackend: - if backends.Uploads == nil { - backends.Uploads = &BackendConfiguration{ - Type: backend, URL: uploadsURL, HTTPClient: httpClient} + backends.mu.RLock() + ret := backends.Uploads + backends.mu.RUnlock() + if ret != nil { + return ret } - ret = backends.Uploads + backends.mu.Lock() + defer backends.mu.Unlock() + backends.Uploads = &BackendConfiguration{backend, uploadsURL, httpClient} + return backends.Uploads } - return ret + return nil } // SetBackend sets the backend used in the binding. @@ -241,8 +249,6 @@ func (s *BackendConfiguration) CallMultipart(method, path, key, boundary string, // NewRequest is used by Call to generate an http.Request. It handles encoding // parameters and attaching the appropriate headers. func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, body io.Reader, params *Params) (*http.Request, error) { - s.mu.Lock() - defer s.mu.Unlock() if !strings.HasPrefix(path, "/") { path = "/" + path } @@ -298,8 +304,6 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string, // the backend's HTTP client to execute the request and unmarshals the response // into v. It also handles unmarshaling errors returned by the API. func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error { - s.mu.Lock() - defer s.mu.Unlock() if LogLevel > 1 { Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path) }