Merge pull request #2 from deploymenttheory/dev

Dev

Author: ShocOne

internal/apihandlers/jamfpro/jamfpro_api_handler.go

@@ -49,7 +49,9 @@ import (
 
 	_ "embed"
 
-	"github.com/deploymenttheory/go-api-http-client/internal/httpclient"
+	"github.com/deploymenttheory/go-api-http-client/internal/errors"
+	"github.com/deploymenttheory/go-api-http-client/internal/logger"
+	"go.uber.org/zap"
 )
 
 // Endpoint constants represent the URL suffixes used for Jamf API token interactions.
@@ -104,13 +106,39 @@ type EndpointConfig struct {
 
 // JamfAPIHandler implements the APIHandler interface for the Jamf Pro API.
 type JamfAPIHandler struct {
-	logger             httpclient.Logger // logger is used to output logs for the API handling processes.
-	OverrideBaseDomain string            // OverrideBaseDomain is used to override the base domain for URL construction.
-	InstanceName       string            // InstanceName is the name of the Jamf instance.
+	OverrideBaseDomain string // OverrideBaseDomain is used to override the base domain for URL construction.
+	InstanceName       string // InstanceName is the name of the Jamf instance.
+}
+
+type Logger interface {
+	Debug(msg string, keysAndValues ...interface{})
+	Info(msg string, keysAndValues ...interface{})
+	Warn(msg string, keysAndValues ...interface{})
+	Error(msg string, keysAndValues ...interface{})
 }
 
 // Functions
 
+func (j *JamfAPIHandler) GetDefaultBaseDomain() string {
+	return DefaultBaseDomain
+}
+
+func (j *JamfAPIHandler) GetOAuthTokenEndpoint() string {
+	return OAuthTokenEndpoint
+}
+
+func (j *JamfAPIHandler) GetBearerTokenEndpoint() string {
+	return BearerTokenEndpoint
+}
+
+func (j *JamfAPIHandler) GetTokenRefreshEndpoint() string {
+	return TokenRefreshEndpoint
+}
+
+func (j *JamfAPIHandler) GetTokenInvalidateEndpoint() string {
+	return TokenInvalidateEndpoint
+}
+
 // GetBaseDomain returns the appropriate base domain for URL construction.
 // It uses OverrideBaseDomain if set, otherwise falls back to DefaultBaseDomain.
 func (j *JamfAPIHandler) GetBaseDomain() string {
@@ -120,19 +148,19 @@ func (j *JamfAPIHandler) GetBaseDomain() string {
 	return DefaultBaseDomain
 }
 
-// ConstructAPIResourceEndpoint returns the full URL for a Jamf API resource endpoint path.
-func (j *JamfAPIHandler) ConstructAPIResourceEndpoint(endpointPath string) string {
+// ConstructAPIResourceEndpoint constructs the full URL for a Jamf API resource endpoint path and logs the URL.
+func (j *JamfAPIHandler) ConstructAPIResourceEndpoint(endpointPath string, log logger.Logger) string {
 	baseDomain := j.GetBaseDomain()
 	url := fmt.Sprintf("https://%s%s%s", j.InstanceName, baseDomain, endpointPath)
-	j.logger.Info("Request will be made to API   URL:", "URL", url)
+	log.Info("Constructed API resource endpoint URL", zap.String("URL", url))
 	return url
 }
 
-// ConstructAPIAuthEndpoint returns the full URL for a Jamf API auth endpoint path.
-func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string) string {
+// ConstructAPIAuthEndpoint constructs the full URL for a Jamf API auth endpoint path and logs the URL.
+func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string, log logger.Logger) string {
 	baseDomain := j.GetBaseDomain()
 	url := fmt.Sprintf("https://%s%s%s", j.InstanceName, baseDomain, endpointPath)
-	j.logger.Info("Request will be made to API authentication URL:", "URL", url)
+	log.Info("Constructed API authentication URL", zap.String("URL", url))
 	return url
 }
 
@@ -144,36 +172,36 @@ func (j *JamfAPIHandler) ConstructAPIAuthEndpoint(endpointPath string) string {
 // - For url endpoints starting with "/api", it defaults to "application/json" for the JamfPro API.
 // If the endpoint does not match any of the predefined patterns, "application/json" is used as a fallback.
 // This method logs the decision process at various stages for debugging purposes.
-func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string) string {
+func (u *JamfAPIHandler) GetContentTypeHeader(endpoint string, log logger.Logger) string {
 	// Dynamic lookup from configuration should be the first priority
 	for key, config := range configMap {
 		if strings.HasPrefix(endpoint, key) {
 			if config.ContentType != nil {
-				u.logger.Debug("Content-Type for endpoint found in configMap", "endpoint", endpoint, "content_type", *config.ContentType)
+				log.Debug("Content-Type for endpoint found in configMap", zap.String("endpoint", endpoint), zap.String("content_type", *config.ContentType))
 				return *config.ContentType
 			}
-			u.logger.Debug("Content-Type for endpoint is nil in configMap, handling as special case", "endpoint", endpoint)
+			log.Debug("Content-Type for endpoint is nil in configMap, handling as special case", zap.String("endpoint", endpoint))
 			// If a nil ContentType is an expected case, do not set Content-Type header.
 			return "" // Return empty to indicate no Content-Type should be set.
 		}
 	}
 
 	// If no specific configuration is found, then check for standard URL patterns.
 	if strings.Contains(endpoint, "/JSSResource") {
-		u.logger.Debug("Content-Type for endpoint defaulting to XML for Classic API", "endpoint", endpoint)
+		log.Debug("Content-Type for endpoint defaulting to XML for Classic API", zap.String("endpoint", endpoint))
 		return "application/xml" // Classic API uses XML
 	} else if strings.Contains(endpoint, "/api") {
-		u.logger.Debug("Content-Type for endpoint defaulting to JSON for JamfPro API", "endpoint", endpoint)
+		log.Debug("Content-Type for endpoint defaulting to JSON for JamfPro API", zap.String("endpoint", endpoint))
 		return "application/json" // JamfPro API uses JSON
 	}
 
 	// Fallback to JSON if no other match is found.
-	u.logger.Debug("Content-Type for endpoint not found in configMap or standard patterns, using default JSON", "endpoint", endpoint)
+	log.Debug("Content-Type for endpoint not found in configMap or standard patterns, using default JSON", zap.String("endpoint", endpoint))
 	return "application/json"
 }
 
 // MarshalRequest encodes the request body according to the endpoint for the API.
-func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoint string) ([]byte, error) {
+func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoint string, log logger.Logger) ([]byte, error) {
 	var (
 		data []byte
 		err  error
@@ -195,47 +223,47 @@ func (u *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin
 		}
 
 		if method == "POST" || method == "PUT" {
-			u.logger.Trace("XML Request Body:", "Body", string(data))
+			log.Debug("XML Request Body", zap.String("Body", string(data)))
 		}
 
 	case "json":
 		data, err = json.Marshal(body)
 		if err != nil {
-			u.logger.Error("Failed marshaling JSON request", "error", err)
+			log.Error("Failed marshaling JSON request", zap.Error(err))
 			return nil, err
 		}
 
 		if method == "POST" || method == "PUT" || method == "PATCH" {
-			u.logger.Debug("JSON Request Body:", string(data))
+			log.Debug("JSON Request Body", zap.String("Body", string(data)))
 		}
 	}
 
 	return data, nil
 }
 
 // UnmarshalResponse decodes the response body from XML or JSON format depending on the Content-Type header.
-func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}) error {
+func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{}, log logger.Logger) error {
 	// Handle DELETE method
 	if resp.Request.Method == "DELETE" {
 		if resp.StatusCode >= 200 && resp.StatusCode < 300 {
 			return nil
 		} else {
-			return fmt.Errorf("DELETE request failed with status code: %d", resp.StatusCode)
+			return log.Error("DELETE request failed", zap.Int("Status Code", resp.StatusCode))
 		}
 	}
 
 	bodyBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
-		u.logger.Error("Failed reading response body", "error", err)
+		log.Error("Failed reading response body", zap.Error(err))
 		return err
 	}
 
 	// Log the raw response body and headers
-	u.logger.Trace("Raw HTTP Response:", string(bodyBytes))
-	u.logger.Debug("Unmarshaling response", "status", resp.Status)
+	log.Debug("Raw HTTP Response", zap.String("Body", string(bodyBytes)))
+	log.Debug("Unmarshaling response", zap.String("status", resp.Status))
 
 	// Log headers when in debug mode
-	u.logger.Debug("HTTP Response Headers:", resp.Header)
+	log.Debug("HTTP Response Headers", zap.Any("Headers", resp.Header))
 
 	// Check the Content-Type and Content-Disposition headers
 	contentType := resp.Header.Get("Content-Type")
@@ -249,30 +277,38 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{})
 	// If content type is HTML, extract the error message
 	if strings.Contains(contentType, "text/html") {
 		errMsg := ExtractErrorMessageFromHTML(string(bodyBytes))
-		u.logger.Warn("Received HTML content", "error_message", errMsg, "status_code", resp.StatusCode)
-		return &APIError{
+		log.Warn("Received HTML content", zap.String("error_message", errMsg), zap.Int("status_code", resp.StatusCode))
+		return &errors.APIError{
 			StatusCode: resp.StatusCode,
 			Message:    errMsg,
 		}
 	}
 
 	// Check for non-success status codes before attempting to unmarshal
-	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		// Parse the error details from the response body for JSON content type
-		if strings.Contains(contentType, "application/json") {
+if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+	// Parse the error details from the response body for JSON content type
+	if strings.Contains(contentType, "application/json") {
 			description, err := ParseJSONErrorResponse(bodyBytes)
 			if err != nil {
-				u.logger.Error("Failed to parse JSON error response", "error", err)
-				return fmt.Errorf("received non-success status code: %d and failed to parse error response", resp.StatusCode)
+					// Log the error using the structured logger and return the error
+					return nil, c.Logger.Error("Failed to parse JSON error response",
+							zap.Error(err),
+							zap.Int("status_code", resp.StatusCode),
+					)
 			}
-			return fmt.Errorf("received non-success status code: %d, error: %s", resp.StatusCode, description)
-		}
-
-		// If the response is not JSON or another error occurs, return a generic error message
-		u.logger.Error("Received non-success status code", "status_code", resp.StatusCode)
-		return fmt.Errorf("received non-success status code: %d", resp.StatusCode)
+			// Log the error with description using the structured logger and return the error
+			return nil, c.Logger.Error("Received non-success status code with JSON response",
+					zap.Int("status_code", resp.StatusCode),
+					zap.String("error_description", description),
+			)
 	}
 
+	// If the response is not JSON or another error occurs, log a generic error message and return an error
+	return nil, c.Logger.Error("Received non-success status code without JSON response",
+			zap.Int("status_code", resp.StatusCode),
+	)
+}
+
 	// Determine whether the content type is JSON or XML and unmarshal accordingly
 	switch {
 	case strings.Contains(contentType, "application/json"):
@@ -285,20 +321,23 @@ func (u *JamfAPIHandler) UnmarshalResponse(resp *http.Response, out interface{})
 	}
 
 	// Handle any errors that occurred during unmarshaling
-	if err != nil {
-		// If unmarshalling fails, check if the content might be HTML
-		if strings.Contains(string(bodyBytes), "<html>") {
+if err != nil {
+	// If unmarshalling fails, check if the content might be HTML
+	if strings.Contains(string(bodyBytes), "<html>") {
 			errMsg := ExtractErrorMessageFromHTML(string(bodyBytes))
-			u.logger.Warn("Received HTML content instead of expected format", "error_message", errMsg, "status_code", resp.StatusCode)
-			return fmt.Errorf(errMsg)
-		}
-
-		// Log the error and return it
-		u.logger.Error("Failed to unmarshal response", "error", err)
-		return fmt.Errorf("failed to unmarshal response: %v", err)
+			
+			// Log the warning and return an error using the structured logger
+			return nil, log.Warn("Received HTML content instead of expected format",
+					zap.String("error_message", errMsg),
+					zap.Int("status_code", resp.StatusCode),
+			)
 	}
 
-	return nil
+		// Log the error using the structured logger and return the error
+    return nil, log.Error("Failed to unmarshal response",
+        zap.Error(err),
+    )
+
 }
 
 // GetAcceptHeader constructs and returns a weighted Accept header string for HTTP requests.
@@ -327,7 +366,7 @@ func (u *JamfAPIHandler) GetAcceptHeader() string {
 }
 
 // MarshalMultipartFormData takes a map with form fields and file paths and returns the encoded body and content type.
-func (u *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string) ([]byte, string, error) {
+func (u *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, error) {
 	body := &bytes.Buffer{}
 	writer := multipart.NewWriter(body)
 

internal/httpclient/http_error_handling.go renamed to internal/errors/http_error_handling.go

@@ -1,19 +1,21 @@
 // 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"
 	"fmt"
 	"net/http"
 
+	"github.com/deploymenttheory/go-api-http-client/internal/logger"
 	"go.uber.org/zap"
 )
 
 // APIError represents a structured API error response.
 type APIError struct {
-	StatusCode int
-	Message    string
+	StatusCode int    // HTTP status code
+	Type       string // A brief identifier for the type of error (e.g., "RateLimit", "BadRequest", etc.)
+	Message    string // Human-readable message
 }
 
 // StructuredError represents a structured error response from the API.
@@ -24,19 +26,25 @@ type StructuredError struct {
 	} `json:"error"`
 }
 
+// Error returns a string representation of the APIError.
+func (e *APIError) Error() string {
+	return fmt.Sprintf("API Error (Type: %s, Code: %d): %s", e.Type, e.StatusCode, e.Message)
+}
+
 // 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(resp *http.Response, log logger.Logger) error {
 	var structuredErr StructuredError
 	err := json.NewDecoder(resp.Body).Decode(&structuredErr)
 	if err == nil && structuredErr.Error.Message != "" {
 		// Using structured logging to log the structured error details
-		c.logger.Warn("API returned structured error",
+		log.Warn("API returned structured error",
 			zap.String("status", resp.Status),
 			zap.String("error_code", structuredErr.Error.Code),
 			zap.String("error_message", structuredErr.Error.Message),
 		)
 		return &APIError{
 			StatusCode: resp.StatusCode,
+			Type:       structuredErr.Error.Code,
 			Message:    structuredErr.Error.Message,
 		}
 	}
@@ -46,29 +54,25 @@ func (c *Client) HandleAPIError(resp *http.Response) error {
 	if err != nil || errMsg == "" {
 		errMsg = fmt.Sprintf("Unexpected error with status code: %d", resp.StatusCode)
 		// Logging with structured fields
-		c.logger.Warn("Failed to decode API error message, using default error message",
+		log.Warn("Failed to decode API error message, using default error message",
 			zap.String("status", resp.Status),
 			zap.String("error_message", errMsg),
 		)
 	} else {
 		// Logging non-structured error as a warning with structured fields
-		c.logger.Warn("API returned non-structured error",
+		log.Warn("API returned non-structured error",
 			zap.String("status", resp.Status),
 			zap.String("error_message", errMsg),
 		)
 	}
 
 	return &APIError{
 		StatusCode: resp.StatusCode,
+		Type:       "UnexpectedError",
 		Message:    errMsg,
 	}
 }
 
-// Error returns a string representation of the APIError.
-func (e *APIError) Error() string {
-	return fmt.Sprintf("API Error (Code: %d): %s", e.StatusCode, e.Message)
-}
-
 // TranslateStatusCode provides a human-readable message for HTTP status codes.
 func TranslateStatusCode(statusCode int) string {
 	messages := map[int]string{