Refactor HTTP client and API handler***

***Adjust concurrency management based on metrics

Author: ShocOne

internal/apihandlers/jamfpro/jamfpro_api_handler.go

@@ -49,7 +49,6 @@ import (
 
 	_ "embed"
 
-	"github.com/deploymenttheory/go-api-http-client/internal/httpclient"
 	"github.com/deploymenttheory/go-api-http-client/internal/logger"
 	"go.uber.org/zap"
 )
@@ -258,7 +257,7 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{},
 	if strings.Contains(contentType, "text/html") {
 		errMsg := ExtractErrorMessageFromHTML(string(bodyBytes))
 		log.Warn("Received HTML content", zap.String("error_message", errMsg), zap.Int("status_code", resp.StatusCode))
-		return &httpclient.APIError{
+		return &APIError{
 			StatusCode: resp.StatusCode,
 			Message:    errMsg,
 		}

…ternal/httpclient/http_error_handling.go internal/errors/http_error_handling.gointernal/httpclient/http_error_handling.go renamed to internal/errors/http_error_handling.go internal/httpclient/http_error_handling.go renamed to internal/errors/http_error_handling.go

@@ -1,6 +1,6 @@
 // http_error_handling.go
 // This package provides utility functions and structures for handling and categorizing HTTP error responses.
-package httpclient
+package errors
 
 import (
 	"encoding/json"
@@ -25,7 +25,7 @@ type StructuredError struct {
 }
 
 // HandleAPIError handles error responses from the API, converting them into a structured error if possible.
-func (c *Client) HandleAPIError(resp *http.Response) error {
+func HandleAPIError(logger Logger, resp *http.Response) error {
 	var structuredErr StructuredError
 	err := json.NewDecoder(resp.Body).Decode(&structuredErr)
 	if err == nil && structuredErr.Error.Message != "" {

internal/httpclient/http_client.go

@@ -157,7 +157,7 @@ func BuildClient(config Config) (*Client, error) {
 	}
 
 	// Authenticate and check token validity.
-	_, err = client.ValidAuthTokenCheck()
+	_, err = client.ValidAuthTokenCheck(client.Logger)
 	if err != nil {
 		return nil, log.Error("Failed to validate or obtain auth token", zap.Error(err))
 	}

internal/httpclient/http_client_auth_token_management.go

@@ -2,9 +2,9 @@
 package httpclient
 
 import (
-	"fmt"
 	"time"
 
+	"github.com/deploymenttheory/go-api-http-client/internal/logger"
 	"go.uber.org/zap"
 )
 
@@ -14,72 +14,23 @@ type TokenResponse struct {
 	Expires time.Time `json:"expires"`
 }
 
-/*
 // ValidAuthTokenCheck checks if the current token is valid and not close to expiry.
 // If the token is invalid, it tries to refresh it.
 // It returns a boolean indicating the validity of the token and an error if there's a failure.
-func (c *Client) ValidAuthTokenCheck() (bool, error) {
-
+func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) {
 	if c.Token == "" {
+		c.Logger.Debug("No token found, attempting to obtain a new one")
 		if c.AuthMethod == "bearer" {
 			err := c.ObtainToken()
 			if err != nil {
-				return false, fmt.Errorf("failed to obtain bearer token: %w", err)
+				return false, c.Logger.Error("Failed to obtain bearer token", zap.Error(err))
 			}
 		} else if c.AuthMethod == "oauth" {
-			err := c.ObtainOAuthToken(c.config.Auth)
-			if err != nil {
-				return false, fmt.Errorf("failed to obtain OAuth token: %w", err)
-			}
-		} else {
-			return false, fmt.Errorf("no valid credentials provided. Unable to obtain a token")
-		}
-	}
-
-	// If token exists and is close to expiry or already expired
-	if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod {
-		var err error
-		if c.BearerTokenAuthCredentials.Username != "" && c.BearerTokenAuthCredentials.Password != "" {
-			err = c.RefreshToken()
-		} else if c.OAuthCredentials.ClientID != "" && c.OAuthCredentials.ClientSecret != "" {
-			err = c.ObtainOAuthToken(c.config.Auth)
-		} else {
-			return false, fmt.Errorf("unknown auth method: %s", c.AuthMethod)
-		}
-
-		if err != nil {
-			return false, fmt.Errorf("failed to refresh token: %w", err)
-		}
-	}
-
-	if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod {
-		return false, fmt.Errorf("token lifetime setting less than buffer. Buffer setting: %v, Time (seconds) until Exp: %v", c.config.TokenRefreshBufferPeriod, time.Until(c.Expiry))
-	}
-	return true, nil
-}
-*/
-
-// ValidAuthTokenCheck checks if the current token is valid and not close to expiry.
-// If the token is invalid, it tries to refresh it.
-// It returns a boolean indicating the validity of the token and an error if there's a failure.
-func (c *Client) ValidAuthTokenCheck() (bool, error) {
-	if c.Token == "" {
-		if c.AuthMethod == "bearer" {
-			err := c.ObtainToken()
-			if err != nil {
-				c.logger.Error("Failed to obtain bearer token", zap.Error(err))
-				return false, fmt.Errorf("failed to obtain bearer token: %w", err)
-			}
-		} else if c.AuthMethod == "oauth" {
-			err := c.ObtainOAuthToken(c.config.Auth)
-			if err != nil {
-				c.logger.Error("Failed to obtain OAuth token", zap.Error(err))
-				return false, fmt.Errorf("failed to obtain OAuth token: %w", err)
+			if err := c.ObtainOAuthToken(c.config.Auth); err != nil {
+				return false, c.Logger.Error("Failed to obtain OAuth token", zap.Error(err))
 			}
 		} else {
-			err := fmt.Errorf("no valid credentials provided. Unable to obtain a token")
-			c.logger.Error("No valid credentials provided", zap.Error(err))
-			return false, err
+			return false, c.Logger.Error("No valid credentials provided. Unable to obtain a token", zap.String("authMethod", c.AuthMethod))
 		}
 	}
 
@@ -90,21 +41,20 @@ func (c *Client) ValidAuthTokenCheck() (bool, error) {
 		} else if c.OAuthCredentials.ClientID != "" && c.OAuthCredentials.ClientSecret != "" {
 			err = c.ObtainOAuthToken(c.config.Auth)
 		} else {
-			err = fmt.Errorf("unknown auth method: %s", c.AuthMethod)
-			c.logger.Error("Unknown auth method", zap.String("auth_method", c.AuthMethod), zap.Error(err))
-			return false, err
+			return false, c.Logger.Error("Unknown auth method", zap.String("authMethod", c.AuthMethod))
 		}
 
 		if err != nil {
-			c.logger.Error("Failed to refresh token", zap.Error(err))
-			return false, fmt.Errorf("failed to refresh token: %w", err)
+			return false, c.Logger.Error("Failed to refresh token", zap.Error(err))
 		}
 	}
 
 	if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod {
-		err := fmt.Errorf("token lifetime setting less than buffer. Buffer setting: %v, Time (seconds) until Exp: %v", c.config.TokenRefreshBufferPeriod, time.Until(c.Expiry))
-		c.logger.Error("Token lifetime less than buffer", zap.Duration("buffer_period", c.config.TokenRefreshBufferPeriod), zap.Duration("time_until_expiry", time.Until(c.Expiry)), zap.Error(err))
-		return false, err
+		return false, c.Logger.Error(
+			"Token lifetime setting less than buffer",
+			zap.Duration("buffer_period", c.config.TokenRefreshBufferPeriod),
+			zap.Duration("time_until_expiry", time.Until(c.Expiry)),
+		)
 	}
 
 	return true, nil

internal/httpclient/http_concurrency_management.go

@@ -8,6 +8,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/deploymenttheory/go-api-http-client/internal/logger"
 	"github.com/google/uuid"
 	"go.uber.org/zap"
 )
@@ -23,7 +24,7 @@ const (
 // ConcurrencyManager controls the number of concurrent HTTP requests.
 type ConcurrencyManager struct {
 	sem                      chan struct{}
-	logger                   Logger
+	logger                   logger.Logger
 	debugMode                bool
 	AcquisitionTimes         []time.Duration
 	lock                     sync.Mutex
@@ -37,10 +38,7 @@ type requestIDKey struct{}
 // NewConcurrencyManager initializes a new ConcurrencyManager with the given concurrency limit, logger, and debug mode.
 // The ConcurrencyManager ensures no more than a certain number of concurrent requests are made.
 // It uses a semaphore to control concurrency.
-func NewConcurrencyManager(limit int, logger Logger, debugMode bool) *ConcurrencyManager {
-	if logger == nil {
-		logger = &defaultLogger{} // Assuming this is the default logger implementation
-	}
+func NewConcurrencyManager(limit int, logger logger.Logger, debugMode bool) *ConcurrencyManager {
 	return &ConcurrencyManager{
 		sem:              make(chan struct{}, limit),
 		logger:           logger,
@@ -209,7 +207,7 @@ func (c *Client) AdjustConcurrencyBasedOnMetrics() {
 	if newLimit != currentLimit {
 		c.ConcurrencyMgr.AdjustConcurrencyLimit(newLimit)
 
-		c.logger.Debug("Adjusted concurrency",
+		c.Logger.Debug("Adjusted concurrency",
 			zap.Int("OldLimit", currentLimit),
 			zap.Int("NewLimit", newLimit),
 			zap.String("Reason", "Based on average acquisition time"),

internal/httpclient/http_headers.go

@@ -2,10 +2,12 @@ package httpclient
 
 import (
 	"net/http"
+
+	"github.com/deploymenttheory/go-api-http-client/internal/logger"
 )
 
 // LogHTTPHeaders logs the HTTP headers of an HTTP request or response, with an option to hide sensitive information like the token in secure mode.
-func LogHTTPHeaders(logger Logger, headers http.Header, secureMode bool) {
+func LogHTTPHeaders(log logger.Logger, headers http.Header, secureMode bool) {
 	var keysAndValues []interface{}
 	if secureMode {
 		for key, values := range headers {
@@ -23,6 +25,6 @@ func LogHTTPHeaders(logger Logger, headers http.Header, secureMode bool) {
 
 	// Log the headers using the logger from the httpclient package
 	if len(keysAndValues) > 0 {
-		logger.Debug("HTTP Headers", keysAndValues...)
+		log.Debug("HTTP Headers", keysAndValues...)
 	}
 }