fix: data race warnings

v5/core/base_service.go

@@ -87,7 +87,7 @@ func NewBaseService(options *ServiceOptions) (*BaseService, error) {
 		return nil, fmt.Errorf(ERRORMSG_PROP_INVALID, "URL")
 	}
 
-	if options.Authenticator == nil {
+	if IsNil(options.Authenticator) {
 		return nil, fmt.Errorf(ERRORMSG_NO_AUTHENTICATOR)
 	}
 
@@ -315,7 +315,7 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
 	}
 
 	// Add authentication to the outbound request.
-	if service.Options.Authenticator == nil {
+	if IsNil(service.Options.Authenticator) {
 		err = fmt.Errorf(ERRORMSG_NO_AUTHENTICATOR)
 		return
 	}

v5/core/base_service_retries_test.go

@@ -24,12 +24,15 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
 
+var requestCountMutex sync.Mutex
+
 // assertResponse is a convenience function for checking various parts of a response
 func assertResponse(r *DetailedResponse, expectedStatusCode int, expectedContentType string) {
 	Expect(r).ToNot(BeNil())
@@ -74,7 +77,10 @@ var _ = Describe(`Retry scenarios`, func() {
 					defer GinkgoRecover()
 					time.Sleep(1 * time.Second)
 
+					requestCountMutex.Lock()
 					requestCount++
+					requestCountMutex.Unlock()
+
 					w.Header().Set("Retry-After", "1")
 					w.WriteHeader(http.StatusTooManyRequests)
 				}))
@@ -95,7 +101,9 @@ var _ = Describe(`Retry scenarios`, func() {
 				fmt.Fprintf(GinkgoWriter, "Expected error: %s\n", err.Error())
 				Expect(err.Error()).To(ContainSubstring("context deadline exceeded"))
 				Expect(resp).To(BeNil())
+				requestCountMutex.Lock()
 				Expect(requestCount).To(Equal(0))
+				requestCountMutex.Unlock()
 			})
 			It(`Timeout on while doing retries`, func() {
 				service, builder := clientInit("GET", server.URL, 2, 0)

v5/core/base_service_test.go

@@ -1496,6 +1496,19 @@ func TestAuthNotConfigured(t *testing.T) {
 	assert.Nil(t, service)
 }
 
+func TestAuthInterfaceNilValue(t *testing.T) {
+	var authenticator *NoAuthAuthenticator = nil
+
+	options := &ServiceOptions{
+		Authenticator: authenticator,
+	}
+
+	service, err := NewBaseService(options)
+	assert.Nil(t, service)
+	assert.NotNil(t, err)
+	assert.Equal(t, ERRORMSG_NO_AUTHENTICATOR, err.Error())
+}
+
 func testGetErrorMessage(t *testing.T, statusCode int, jsonString string, expectedErrorMsg string) {
 	body := []byte(jsonString)
 	responseMap, err := decodeAsMap(body)

v5/core/cp4d_authenticator.go

@@ -60,6 +60,9 @@ type CloudPakForDataAuthenticator struct {
 
 	// The cached token and expiration time.
 	tokenData *cp4dTokenData
+
+	// Mutex to make the tokenData field thread safe.
+	tokenDataMutex sync.Mutex
 }
 
 var cp4dRequestTokenMutex sync.Mutex
@@ -126,15 +129,15 @@ func newCloudPakForDataAuthenticatorFromMap(properties map[string]string) (*Clou
 }
 
 // AuthenticationType returns the authentication type for this authenticator.
-func (CloudPakForDataAuthenticator) AuthenticationType() string {
+func (*CloudPakForDataAuthenticator) AuthenticationType() string {
 	return AUTHTYPE_CP4D
 }
 
 // Validate the authenticator's configuration.
 //
 // Ensures the username, password, and url are not Nil. Additionally, ensures
 // they do not contain invalid characters.
-func (authenticator CloudPakForDataAuthenticator) Validate() error {
+func (authenticator *CloudPakForDataAuthenticator) Validate() error {
 
 	if authenticator.Username == "" {
 		return fmt.Errorf(ERRORMSG_PROP_MISSING, "Username")
@@ -170,21 +173,37 @@ func (authenticator *CloudPakForDataAuthenticator) Authenticate(request *http.Re
 	return nil
 }
 
+// getTokenData returns the tokenData field from the authenticator.
+func (authenticator *CloudPakForDataAuthenticator) getTokenData() *cp4dTokenData {
+	authenticator.tokenDataMutex.Lock()
+	defer authenticator.tokenDataMutex.Unlock()
+
+	return authenticator.tokenData
+}
+
+// setTokenData sets the given cp4dTokenData to the tokenData field of the authenticator.
+func (authenticator *CloudPakForDataAuthenticator) setTokenData(tokenData *cp4dTokenData) {
+	authenticator.tokenDataMutex.Lock()
+	defer authenticator.tokenDataMutex.Unlock()
+
+	authenticator.tokenData = tokenData
+}
+
 // getToken: returns an access token to be used in an Authorization header.
 // Whenever a new token is needed (when a token doesn't yet exist, needs to be refreshed,
 // or the existing token has expired), a new access token is fetched from the token server.
 func (authenticator *CloudPakForDataAuthenticator) getToken() (string, error) {
-	if authenticator.tokenData == nil || !authenticator.tokenData.isTokenValid() {
+	if authenticator.getTokenData() == nil || !authenticator.getTokenData().isTokenValid() {
 		// synchronously request the token
 		err := authenticator.synchronizedRequestToken()
 		if err != nil {
 			return "", err
 		}
-	} else if authenticator.tokenData.needsRefresh() {
+	} else if authenticator.getTokenData().needsRefresh() {
 		// If refresh needed, kick off a go routine in the background to get a new token
 		ch := make(chan error)
 		go func() {
-			ch <- authenticator.getTokenData()
+			ch <- authenticator.invokeRequestTokenData()
 		}()
 		select {
 		case err := <-ch:
@@ -196,11 +215,11 @@ func (authenticator *CloudPakForDataAuthenticator) getToken() (string, error) {
 	}
 
 	// return an error if the access token is not valid or was not fetched
-	if authenticator.tokenData == nil || authenticator.tokenData.AccessToken == "" {
+	if authenticator.getTokenData() == nil || authenticator.getTokenData().AccessToken == "" {
 		return "", fmt.Errorf("Error while trying to get access token")
 	}
 
-	return authenticator.tokenData.AccessToken, nil
+	return authenticator.getTokenData().AccessToken, nil
 }
 
 // synchronizedRequestToken: synchronously checks if the current token in cache
@@ -210,27 +229,28 @@ func (authenticator *CloudPakForDataAuthenticator) synchronizedRequestToken() er
 	cp4dRequestTokenMutex.Lock()
 	defer cp4dRequestTokenMutex.Unlock()
 	// if cached token is still valid, then just continue to use it
-	if authenticator.tokenData != nil && authenticator.tokenData.isTokenValid() {
+	if authenticator.getTokenData() != nil && authenticator.getTokenData().isTokenValid() {
 		return nil
 	}
 
-	return authenticator.getTokenData()
+	return authenticator.invokeRequestTokenData()
 }
 
-// getTokenData: requests a new token from the token server and
+// invokeRequestTokenData: requests a new token from the token server and
 // unmarshals the token information to the tokenData cache. Returns
 // an error if the token was unable to be fetched, otherwise returns nil
-func (authenticator *CloudPakForDataAuthenticator) getTokenData() error {
+func (authenticator *CloudPakForDataAuthenticator) invokeRequestTokenData() error {
 	tokenResponse, err := authenticator.requestToken()
 	if err != nil {
-		authenticator.tokenData = nil
+		authenticator.setTokenData(nil)
 		return err
 	}
 
-	authenticator.tokenData, err = newCp4dTokenData(tokenResponse)
-	if err != nil {
-		authenticator.tokenData = nil
+	if tokenData, err := newCp4dTokenData(tokenResponse); err != nil {
+		authenticator.setTokenData(nil)
 		return err
+	} else {
+		authenticator.setTokenData(tokenData)
 	}
 
 	return nil