Merge pull request #257 from deploymenttheory/dev-jl

More tidyup operations

Author: thejoeker12

httpclient/methods.go

@@ -20,7 +20,7 @@ import "net/http"
 */
 
 // IsIdempotentHTTPMethod checks if the given HTTP method is idempotent.
-func IsIdempotentHTTPMethod(method string) bool {
+func isIdempotentHTTPMethod(method string) bool {
 	methodsMap := map[string]bool{
 		http.MethodGet:     true,
 		http.MethodPut:     true,

httpclient/multipartrequest.go

@@ -112,7 +112,6 @@ func (c *Client) DoMultiPartRequest(method, endpoint string, files map[string][]
 		return nil, err
 	}
 
-	// sugar the request details
 	c.Sugar.Info("Created HTTP Multipart request", zap.String("method", method), zap.String("url", url), zap.String("content_type", contentType))
 
 	(*c.Integration).PrepRequestParamsAndAuth(req)

httpclient/request.go

@@ -59,21 +59,15 @@ import (
 //   - The decision to retry requests is based on the idempotency of the HTTP method and the client's retry configuration,
 //     including maximum retry attempts and total retry duration.
 func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*http.Response, error) {
-	if !c.config.RetryEligiableRequests {
-		return c.requestNoRetries(method, endpoint, body, out)
-	}
 
-	if IsIdempotentHTTPMethod(method) {
-		return c.requestWithRetries(method, endpoint, body, out)
-	} else if !IsIdempotentHTTPMethod(method) {
+	if !c.config.RetryEligiableRequests || !isIdempotentHTTPMethod(method) {
 		return c.requestNoRetries(method, endpoint, body, out)
 	}
 
-	return nil, fmt.Errorf("unsupported http method: %s", method)
+	return c.requestWithRetries(method, endpoint, body, out)
 }
 
-// region comment
-// executeRequestWithRetries executes an HTTP request using the specified method, endpoint, request body, and output variable.
+// requestWithRetries executes an HTTP request using the specified method, endpoint, request body, and output variable.
 // It is designed for idempotent HTTP methods (GET, PUT, DELETE), where the request can be safely retried in case of
 // transient errors or rate limiting. The function implements a retry mechanism that respects the client's configuration
 // for maximum retry attempts and total retry duration. Each retry attempt uses exponential backoff with jitter to avoid
@@ -102,7 +96,6 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt
 // - The function respects the client's concurrency token, acquiring and releasing it as needed to ensure safe concurrent
 // operations.
 // - The retry mechanism employs exponential backoff with jitter to mitigate the impact of retries on the server.
-// endregion
 func (c *Client) requestWithRetries(method, endpoint string, body, out interface{}) (*http.Response, error) {
 	var resp *http.Response
 	var err error
@@ -188,7 +181,7 @@ func (c *Client) requestWithRetries(method, endpoint string, body, out interface
 	return resp, response.HandleAPIErrorResponse(resp, c.Sugar)
 }
 
-// executeRequest executes an HTTP request using the specified method, endpoint, and request body without implementing
+// requestNoRetries executes an HTTP request using the specified method, endpoint, and request body without implementing
 // retry logic. It is primarily designed for non-idempotent HTTP methods like POST and PATCH, where the request should
 // not be automatically retried within this function due to the potential side effects of re-submitting the same data.
 // Parameters:
@@ -225,11 +218,9 @@ func (c *Client) requestNoRetries(method, endpoint string, body, out interface{}
 		return nil, err
 	}
 
-	c.Sugar.Debug("LOGHERE")
 	c.Sugar.Debugf("Status Code: %v", resp.StatusCode)
 
 	if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusBadRequest {
-		c.Sugar.Debug("FLAG 1 - Request Considered Successful")
 		if resp.StatusCode == http.StatusPermanentRedirect || resp.StatusCode == http.StatusTemporaryRedirect {
 			c.Sugar.Warn("Redirect response received", zap.Int("status_code", resp.StatusCode), zap.String("location", resp.Header.Get("Location")))
 		}
@@ -238,15 +229,12 @@ func (c *Client) requestNoRetries(method, endpoint string, body, out interface{}
 		return resp, response.HandleAPISuccessResponse(resp, out, c.Sugar)
 	}
 
-	c.Sugar.Debug("FLAG 2 - Request Considered NOT Successful")
-
 	return nil, response.HandleAPIErrorResponse(resp, c.Sugar)
 }
 
-// TODO function comment
+// request is a base leve private function which the contextual functions above utilise to make requests // TODO improve this comment probably.
 func (c *Client) request(ctx context.Context, method, endpoint string, body interface{}) (*http.Response, error) {
 
-	// TODO Concurrency - Refactor or remove this
 	if c.config.EnableConcurrencyManagement {
 		_, requestID, err := c.Concurrency.AcquireConcurrencyPermit(ctx)
 		if err != nil {

httpclient/utility.go

@@ -84,8 +84,6 @@ func validatePassword(password string) error {
 	return nil
 }
 
-// environment variable mapping helpers
-
 // getEnvAsString reads an environment variable as a string, with a fallback default value.
 func getEnvAsString(name string, defaultVal string) string {
 	if value, exists := os.LookupEnv(name); exists {