Make Client thread-safe & Add docs (#495)

fix #494

Co-authored-by: Norwin Roosen <git@nroo.de>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/495
Reviewed-by: 6543 <6543@obermui.de>
Reviewed-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Norwin <noerw@noreply.gitea.io>
Co-committed-by: Norwin <noerw@noreply.gitea.io>

gitea/client.go

@@ -26,7 +26,7 @@ func Version() string {
 	return "0.14.0"
 }
 
-// Client represents a Gitea API client.
+// Client represents a thread-safe Gitea API client.
 type Client struct {
 	url            string
 	accessToken    string
@@ -37,6 +37,7 @@ type Client struct {
 	debug          bool
 	client         *http.Client
 	ctx            context.Context
+	mutex          sync.RWMutex
 	serverVersion  *version.Version
 	getVersionOnce sync.Once
 }
@@ -47,6 +48,7 @@ type Response struct {
 }
 
 // NewClient initializes and returns a API client.
+// Usage of all gitea.Client methods is concurrency-safe.
 func NewClient(url string, options ...func(*Client)) (*Client, error) {
 	client := &Client{
 		url:    strings.TrimSuffix(url, "/"),
@@ -72,14 +74,23 @@ func NewClientWithHTTP(url string, httpClient *http.Client) *Client {
 // SetHTTPClient is an option for NewClient to set custom http client
 func SetHTTPClient(httpClient *http.Client) func(client *Client) {
 	return func(client *Client) {
-		client.client = httpClient
+		client.SetHTTPClient(httpClient)
 	}
 }
 
+// SetHTTPClient replaces default http.Client with user given one.
+func (c *Client) SetHTTPClient(client *http.Client) {
+	c.mutex.Lock()
+	c.client = client
+	c.mutex.Unlock()
+}
+
 // SetToken is an option for NewClient to set token
 func SetToken(token string) func(client *Client) {
 	return func(client *Client) {
+		client.mutex.Lock()
 		client.accessToken = token
+		client.mutex.Unlock()
 	}
 }
 
@@ -92,7 +103,9 @@ func SetBasicAuth(username, password string) func(client *Client) {
 
 // SetBasicAuth sets username and password
 func (c *Client) SetBasicAuth(username, password string) {
+	c.mutex.Lock()
 	c.username, c.password = username, password
+	c.mutex.Unlock()
 }
 
 // SetOTP is an option for NewClient to set OTP for 2FA
@@ -104,7 +117,9 @@ func SetOTP(otp string) func(client *Client) {
 
 // SetOTP sets OTP for 2FA
 func (c *Client) SetOTP(otp string) {
+	c.mutex.Lock()
 	c.otp = otp
+	c.mutex.Unlock()
 }
 
 // SetContext is an option for NewClient to set context
@@ -116,12 +131,9 @@ func SetContext(ctx context.Context) func(client *Client) {
 
 // SetContext set context witch is used for http requests
 func (c *Client) SetContext(ctx context.Context) {
+	c.mutex.Lock()
 	c.ctx = ctx
-}
+	c.mutex.Unlock()
-
-// SetHTTPClient replaces default http.Client with user given one.
-func (c *Client) SetHTTPClient(client *http.Client) {
-	c.client = client
 }
 
 // SetSudo is an option for NewClient to set sudo header
@@ -133,43 +145,57 @@ func SetSudo(sudo string) func(client *Client) {
 
 // SetSudo sets username to impersonate.
 func (c *Client) SetSudo(sudo string) {
+	c.mutex.Lock()
 	c.sudo = sudo
+	c.mutex.Unlock()
 }
 
 // SetDebugMode is an option for NewClient to enable debug mode
 func SetDebugMode() func(client *Client) {
 	return func(client *Client) {
+		client.mutex.Lock()
 		client.debug = true
+		client.mutex.Unlock()
 	}
 }
 
 func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *Response, error) {
-	if c.debug {
+	c.mutex.RLock()
+	debug := c.debug
+	if debug {
 		fmt.Printf("%s: %s\nBody: %v\n", method, c.url+path, body)
 	}
 	req, err := http.NewRequestWithContext(c.ctx, method, c.url+path, body)
+
+	client := c.client // client ref can change from this point on so safe it
+	c.mutex.RUnlock()
+
 	if err != nil {
 		return nil, nil, err
 	}
-	resp, err := c.client.Do(req)
+
+	resp, err := client.Do(req)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	defer resp.Body.Close()
 	data, err := ioutil.ReadAll(resp.Body)
-	if c.debug {
+	if debug {
 		fmt.Printf("Response: %v\n\n", resp)
 	}
 	return data, &Response{resp}, nil
 }
 
 func (c *Client) doRequest(method, path string, header http.Header, body io.Reader) (*Response, error) {
-	if c.debug {
+	c.mutex.RLock()
+	debug := c.debug
+	if debug {
 		fmt.Printf("%s: %s\nHeader: %v\nBody: %s\n", method, c.url+"/api/v1"+path, header, body)
 	}
 	req, err := http.NewRequestWithContext(c.ctx, method, c.url+"/api/v1"+path, body)
 	if err != nil {
+		c.mutex.RUnlock()
 		return nil, err
 	}
 	if len(c.accessToken) != 0 {
@@ -184,15 +210,19 @@ func (c *Client) doRequest(method, path string, header http.Header, body io.Read
 	if len(c.sudo) != 0 {
 		req.Header.Set("Sudo", c.sudo)
 	}
+
+	client := c.client // client ref can change from this point on so safe it
+	c.mutex.RUnlock()
+
 	for k, v := range header {
 		req.Header[k] = v
 	}
 
-	resp, err := c.client.Do(req)
+	resp, err := client.Do(req)
 	if err != nil {
 		return nil, err
 	}
-	if c.debug {
+	if debug {
 		fmt.Printf("Response: %v\n\n", resp)
 	}
 	return &Response{resp}, nil

gitea/issue.go

@@ -253,6 +253,8 @@ func (c *Client) EditIssue(owner, repo string, index int64, opt EditIssueOption)
 
 func (c *Client) issueBackwardsCompatibility(issue *Issue) {
 	if c.checkServerVersionGreaterThanOrEqual(version1_12_0) != nil {
+		c.mutex.RLock()
 		issue.HTMLURL = fmt.Sprintf("%s/%s/issues/%d", c.url, issue.Repository.FullName, issue.Index)
+		c.mutex.RUnlock()
 	}
 }

gitea/user_app.go

@@ -27,12 +27,15 @@ type ListAccessTokensOptions struct {
 
 // ListAccessTokens lists all the access tokens of user
 func (c *Client) ListAccessTokens(opts ListAccessTokensOptions) ([]*AccessToken, *Response, error) {
-	if len(c.username) == 0 {
+	c.mutex.RLock()
+	username := c.username
+	c.mutex.RUnlock()
+	if len(username) == 0 {
 		return nil, nil, fmt.Errorf("\"username\" not set: only BasicAuth allowed")
 	}
 	opts.setDefaults()
 	tokens := make([]*AccessToken, 0, opts.PageSize)
-	resp, err := c.getParsedResponse("GET", fmt.Sprintf("/users/%s/tokens?%s", c.username, opts.getURLQuery().Encode()), jsonHeader, nil, &tokens)
+	resp, err := c.getParsedResponse("GET", fmt.Sprintf("/users/%s/tokens?%s", username, opts.getURLQuery().Encode()), jsonHeader, nil, &tokens)
 	return tokens, resp, err
 }
 
@@ -43,7 +46,10 @@ type CreateAccessTokenOption struct {
 
 // CreateAccessToken create one access token with options
 func (c *Client) CreateAccessToken(opt CreateAccessTokenOption) (*AccessToken, *Response, error) {
-	if len(c.username) == 0 {
+	c.mutex.RLock()
+	username := c.username
+	c.mutex.RUnlock()
+	if len(username) == 0 {
 		return nil, nil, fmt.Errorf("\"username\" not set: only BasicAuth allowed")
 	}
 	body, err := json.Marshal(&opt)
@@ -51,13 +57,16 @@ func (c *Client) CreateAccessToken(opt CreateAccessTokenOption) (*AccessToken, *
 		return nil, nil, err
 	}
 	t := new(AccessToken)
-	resp, err := c.getParsedResponse("POST", fmt.Sprintf("/users/%s/tokens", c.username), jsonHeader, bytes.NewReader(body), t)
+	resp, err := c.getParsedResponse("POST", fmt.Sprintf("/users/%s/tokens", username), jsonHeader, bytes.NewReader(body), t)
 	return t, resp, err
 }
 
 // DeleteAccessToken delete token, identified by ID and if not available by name
 func (c *Client) DeleteAccessToken(value interface{}) (*Response, error) {
-	if len(c.username) == 0 {
+	c.mutex.RLock()
+	username := c.username
+	c.mutex.RUnlock()
+	if len(username) == 0 {
 		return nil, fmt.Errorf("\"username\" not set: only BasicAuth allowed")
 	}
 
@@ -75,6 +84,6 @@ func (c *Client) DeleteAccessToken(value interface{}) (*Response, error) {
 		return nil, fmt.Errorf("only string and int64 supported")
 	}
 
-	_, resp, err := c.getResponse("DELETE", fmt.Sprintf("/users/%s/tokens/%s", c.username, token), jsonHeader, nil)
+	_, resp, err := c.getResponse("DELETE", fmt.Sprintf("/users/%s/tokens/%s", username, token), jsonHeader, nil)
 	return resp, err
 }

gitea/version.go

@@ -31,7 +31,10 @@ func (c *Client) CheckServerVersionConstraint(constraint string) error {
 		return err
 	}
 	if !check.Check(c.serverVersion) {
-		return fmt.Errorf("gitea server at %s does not satisfy version constraint %s", c.url, constraint)
+		c.mutex.RLock()
+		url := c.url
+		c.mutex.RUnlock()
+		return fmt.Errorf("gitea server at %s does not satisfy version constraint %s", url, constraint)
 	}
 	return nil
 }
@@ -51,7 +54,10 @@ func (c *Client) checkServerVersionGreaterThanOrEqual(v *version.Version) error
 	}
 
 	if !c.serverVersion.GreaterThanOrEqual(v) {
-		return fmt.Errorf("gitea server at %s is older than %s", c.url, v.Original())
+		c.mutex.RLock()
+		url := c.url
+		c.mutex.RUnlock()
+		return fmt.Errorf("gitea server at %s is older than %s", url, v.Original())
 	}
 	return nil
 }