From 05a38f19c2bbcd10805cb240c1d68028ad62900f Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:50:15 +0000 Subject: [PATCH 1/2] Fix instance name in BuildClient function --- httpclient/httpclient_client.go | 2 +- httpclient/httpclient_request.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/httpclient/httpclient_client.go b/httpclient/httpclient_client.go index decf3d8..a8f834b 100644 --- a/httpclient/httpclient_client.go +++ b/httpclient/httpclient_client.go @@ -144,7 +144,7 @@ func BuildClient(config Config) (*Client, error) { // Create a new HTTP client with the provided configuration. client := &Client{ - InstanceName: config.Environment.APIType, + InstanceName: config.Environment.InstanceName, APIHandler: apiHandler, AuthMethod: authMethod, httpClient: &http.Client{Timeout: config.CustomTimeout}, diff --git a/httpclient/httpclient_request.go b/httpclient/httpclient_request.go index 9e8f028..74702dd 100644 --- a/httpclient/httpclient_request.go +++ b/httpclient/httpclient_request.go @@ -320,7 +320,7 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l } // Return an error with the status code and its description - return resp, fmt.Errorf("Error status code: %d - %s", resp.StatusCode, statusDescription) + return resp, fmt.Errorf("error status code: %d - %s", resp.StatusCode, statusDescription) } } // TODO refactor to remove repition. From 419c1dae428d23b3c88853beeefd974b6b821c69 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Fri, 9 Feb 2024 12:23:38 +0000 Subject: [PATCH 2/2] Update httpclient_request.go and httpclient_auth_token_management.go --- .../httpclient_auth_token_management.go | 10 +- httpclient/httpclient_client.go | 148 +++++++++--------- httpclient/httpclient_request.go | 4 +- 3 files changed, 83 insertions(+), 79 deletions(-) diff --git a/httpclient/httpclient_auth_token_management.go b/httpclient/httpclient_auth_token_management.go index ba45101..cb7986b 100644 --- a/httpclient/httpclient_auth_token_management.go +++ b/httpclient/httpclient_auth_token_management.go @@ -28,7 +28,7 @@ func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) { } } else if c.AuthMethod == "oauth" { c.Logger.Info("Credential Match", zap.String("AuthMethod", c.AuthMethod)) - if err := c.ObtainOAuthToken(c.config.Auth, log); err != nil { + if err := c.ObtainOAuthToken(c.clientConfig.Auth, log); err != nil { return false, c.Logger.Error("Failed to obtain OAuth token", zap.Error(err)) } } else { @@ -36,12 +36,12 @@ func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) { } } - if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod { + if time.Until(c.Expiry) < c.clientConfig.ClientOptions.TokenRefreshBufferPeriod { var err error if c.BearerTokenAuthCredentials.Username != "" && c.BearerTokenAuthCredentials.Password != "" { err = c.RefreshToken(log) } else if c.OAuthCredentials.ClientID != "" && c.OAuthCredentials.ClientSecret != "" { - err = c.ObtainOAuthToken(c.config.Auth, log) + err = c.ObtainOAuthToken(c.clientConfig.Auth, log) } else { return false, c.Logger.Error("Unknown auth method", zap.String("authMethod", c.AuthMethod)) } @@ -51,10 +51,10 @@ func (c *Client) ValidAuthTokenCheck(log logger.Logger) (bool, error) { } } - if time.Until(c.Expiry) < c.config.TokenRefreshBufferPeriod { + if time.Until(c.Expiry) < c.clientConfig.ClientOptions.TokenRefreshBufferPeriod { return false, c.Logger.Error( "Token lifetime setting less than buffer", - zap.Duration("buffer_period", c.config.TokenRefreshBufferPeriod), + zap.Duration("buffer_period", c.clientConfig.ClientOptions.TokenRefreshBufferPeriod), zap.Duration("time_until_expiry", time.Until(c.Expiry)), ) } diff --git a/httpclient/httpclient_client.go b/httpclient/httpclient_client.go index a8f834b..10ea5fe 100644 --- a/httpclient/httpclient_client.go +++ b/httpclient/httpclient_client.go @@ -16,12 +16,48 @@ import ( "go.uber.org/zap" ) +// Client represents an HTTP client to interact with a specific API. +type Client struct { + APIHandler APIHandler // APIHandler interface used to define which API handler to use + InstanceName string // Website Instance name without the root domain + AuthMethod string // Specifies the authentication method: "bearer" or "oauth" + Token string // Authentication Token + OverrideBaseDomain string // Base domain override used when the default in the api handler isn't suitable + OAuthCredentials OAuthCredentials // ClientID / Client Secret + BearerTokenAuthCredentials BearerTokenAuthCredentials // Username and Password for Basic Authentication + Expiry time.Time // Expiry time set for the auth token + httpClient *http.Client + tokenLock sync.Mutex + clientConfig ClientConfig + Logger logger.Logger + ConcurrencyMgr *ConcurrencyManager + PerfMetrics PerformanceMetrics +} + // Config holds configuration options for the HTTP Client. -type Config struct { - // Required - Auth AuthConfig // User can either supply these values manually or pass from LoadAuthConfig/Env vars - Environment EnvironmentConfig // User can either supply these values manually or pass from LoadAuthConfig/Env vars - // Optional +type ClientConfig struct { + Auth AuthConfig // User can either supply these values manually or pass from LoadAuthConfig/Env vars + Environment EnvironmentConfig // User can either supply these values manually or pass from LoadAuthConfig/Env vars + ClientOptions ClientOptions // Optional configuration options for the HTTP Client +} + +// EnvironmentConfig represents the structure to read authentication details from a JSON configuration file. +type EnvironmentConfig struct { + InstanceName string `json:"InstanceName,omitempty"` + OverrideBaseDomain string `json:"OverrideBaseDomain,omitempty"` + APIType string `json:"APIType,omitempty"` +} + +// AuthConfig represents the structure to read authentication details from a JSON configuration file. +type AuthConfig struct { + Username string `json:"Username,omitempty"` + Password string `json:"Password,omitempty"` + ClientID string `json:"ClientID,omitempty"` + ClientSecret string `json:"ClientSecret,omitempty"` +} + +// ClientOptions holds optional configuration options for the HTTP Client. +type ClientOptions struct { LogLevel logger.LogLevel // Field for defining tiered logging level. MaxRetryAttempts int // Config item defines the max number of retry request attempts for retryable HTTP methods. EnableDynamicRateLimiting bool @@ -42,49 +78,16 @@ type PerformanceMetrics struct { lock sync.Mutex } -// EnvironmentConfig represents the structure to read authentication details from a JSON configuration file. -type EnvironmentConfig struct { - InstanceName string `json:"instanceName,omitempty"` - OverrideBaseDomain string `json:"overrideBaseDomain,omitempty"` - APIType string `json:"apiType,omitempty"` -} - -// AuthConfig represents the structure to read authentication details from a JSON configuration file. -type AuthConfig struct { - Username string `json:"username,omitempty"` - Password string `json:"password,omitempty"` - ClientID string `json:"clientID,omitempty"` - ClientSecret string `json:"clientSecret,omitempty"` -} - -// Client represents an HTTP client to interact with a specific API. -type Client struct { - APIHandler APIHandler // APIHandler interface used to define which API handler to use - InstanceName string // Website Instance name without the root domain - AuthMethod string // Specifies the authentication method: "bearer" or "oauth" - Token string // Authentication Token - OverrideBaseDomain string // Base domain override used when the default in the api handler isn't suitable - OAuthCredentials OAuthCredentials // ClientID / Client Secret - BearerTokenAuthCredentials BearerTokenAuthCredentials // Username and Password for Basic Authentication - Expiry time.Time // Expiry time set for the auth token - httpClient *http.Client - tokenLock sync.Mutex - config Config - Logger logger.Logger - ConcurrencyMgr *ConcurrencyManager - PerfMetrics PerformanceMetrics -} - // BuildClient creates a new HTTP client with the provided configuration. -func BuildClient(config Config) (*Client, error) { +func BuildClient(config ClientConfig) (*Client, error) { // Initialize the zap logger. - log := logger.BuildLogger(config.LogLevel) + log := logger.BuildLogger(config.ClientOptions.LogLevel) // Set the logger's level based on the provided configuration. - log.SetLevel(config.LogLevel) + log.SetLevel(config.ClientOptions.LogLevel) - if config.LogLevel < logger.LogLevelDebug || config.LogLevel > logger.LogLevelFatal { - return nil, log.Error("Invalid LogLevel setting", zap.Int("Provided LogLevel", int(config.LogLevel))) + if config.ClientOptions.LogLevel < logger.LogLevelDebug || config.ClientOptions.LogLevel > logger.LogLevelFatal { + return nil, log.Error("Invalid LogLevel setting", zap.Int("Provided LogLevel", int(config.ClientOptions.LogLevel))) } // Use the APIType from the config to determine which API handler to load @@ -100,38 +103,38 @@ func BuildClient(config Config) (*Client, error) { return nil, log.Error("InstanceName cannot be empty") } - if config.MaxRetryAttempts < 0 { - config.MaxRetryAttempts = DefaultMaxRetryAttempts + if config.ClientOptions.MaxRetryAttempts < 0 { + config.ClientOptions.MaxRetryAttempts = DefaultMaxRetryAttempts log.Info("MaxRetryAttempts was negative, set to default value", zap.Int("MaxRetryAttempts", DefaultMaxRetryAttempts)) } - if config.MaxConcurrentRequests <= 0 { - config.MaxConcurrentRequests = DefaultMaxConcurrentRequests + if config.ClientOptions.MaxConcurrentRequests <= 0 { + config.ClientOptions.MaxConcurrentRequests = DefaultMaxConcurrentRequests log.Info("MaxConcurrentRequests was negative or zero, set to default value", zap.Int("MaxConcurrentRequests", DefaultMaxConcurrentRequests)) } - if config.TokenRefreshBufferPeriod < 0 { - config.TokenRefreshBufferPeriod = DefaultTokenBufferPeriod + if config.ClientOptions.TokenRefreshBufferPeriod < 0 { + config.ClientOptions.TokenRefreshBufferPeriod = DefaultTokenBufferPeriod log.Info("TokenRefreshBufferPeriod was negative, set to default value", zap.Duration("TokenRefreshBufferPeriod", DefaultTokenBufferPeriod)) } - if config.TotalRetryDuration <= 0 { - config.TotalRetryDuration = DefaultTotalRetryDuration + if config.ClientOptions.TotalRetryDuration <= 0 { + config.ClientOptions.TotalRetryDuration = DefaultTotalRetryDuration log.Info("TotalRetryDuration was negative or zero, set to default value", zap.Duration("TotalRetryDuration", DefaultTotalRetryDuration)) } - if config.TokenRefreshBufferPeriod == 0 { - config.TokenRefreshBufferPeriod = DefaultTokenBufferPeriod + if config.ClientOptions.TokenRefreshBufferPeriod == 0 { + config.ClientOptions.TokenRefreshBufferPeriod = DefaultTokenBufferPeriod log.Info("TokenRefreshBufferPeriod not set, set to default value", zap.Duration("TokenRefreshBufferPeriod", DefaultTokenBufferPeriod)) } - if config.TotalRetryDuration == 0 { - config.TotalRetryDuration = DefaultTotalRetryDuration + if config.ClientOptions.TotalRetryDuration == 0 { + config.ClientOptions.TotalRetryDuration = DefaultTotalRetryDuration log.Info("TotalRetryDuration not set, set to default value", zap.Duration("TotalRetryDuration", DefaultTotalRetryDuration)) } - if config.CustomTimeout == 0 { - config.CustomTimeout = DefaultTimeout + if config.ClientOptions.CustomTimeout == 0 { + config.ClientOptions.CustomTimeout = DefaultTimeout log.Info("CustomTimeout not set, set to default value", zap.Duration("CustomTimeout", DefaultTimeout)) } @@ -144,14 +147,15 @@ func BuildClient(config Config) (*Client, error) { // Create a new HTTP client with the provided configuration. client := &Client{ - InstanceName: config.Environment.InstanceName, - APIHandler: apiHandler, - AuthMethod: authMethod, - httpClient: &http.Client{Timeout: config.CustomTimeout}, - config: config, - Logger: log, - ConcurrencyMgr: NewConcurrencyManager(config.MaxConcurrentRequests, log, true), - PerfMetrics: PerformanceMetrics{}, + APIHandler: apiHandler, + InstanceName: config.Environment.InstanceName, + AuthMethod: authMethod, + OverrideBaseDomain: config.Environment.OverrideBaseDomain, + httpClient: &http.Client{Timeout: config.ClientOptions.CustomTimeout}, + clientConfig: config, + Logger: log, + ConcurrencyMgr: NewConcurrencyManager(config.ClientOptions.MaxConcurrentRequests, log, true), + PerfMetrics: PerformanceMetrics{}, } // Log the client's configuration. @@ -160,13 +164,13 @@ func BuildClient(config Config) (*Client, error) { zap.String("Instance Name", client.InstanceName), zap.String("OverrideBaseDomain", config.Environment.OverrideBaseDomain), zap.String("AuthMethod", authMethod), - zap.Int("MaxRetryAttempts", config.MaxRetryAttempts), - zap.Int("MaxConcurrentRequests", config.MaxConcurrentRequests), - zap.Bool("EnableDynamicRateLimiting", config.EnableDynamicRateLimiting), - zap.Duration("TokenRefreshBufferPeriod", config.TokenRefreshBufferPeriod), - zap.Duration("TotalRetryDuration", config.TotalRetryDuration), - zap.Duration("CustomTimeout", config.CustomTimeout), - zap.String("LogLevel", config.LogLevel.String()), + zap.Int("MaxRetryAttempts", config.ClientOptions.MaxRetryAttempts), + zap.Int("MaxConcurrentRequests", config.ClientOptions.MaxConcurrentRequests), + zap.Bool("EnableDynamicRateLimiting", config.ClientOptions.EnableDynamicRateLimiting), + zap.Duration("TokenRefreshBufferPeriod", config.ClientOptions.TokenRefreshBufferPeriod), + zap.Duration("TotalRetryDuration", config.ClientOptions.TotalRetryDuration), + zap.Duration("CustomTimeout", config.ClientOptions.CustomTimeout), + zap.String("LogLevel", config.ClientOptions.LogLevel.String()), ) return client, nil diff --git a/httpclient/httpclient_request.go b/httpclient/httpclient_request.go index 74702dd..7578ff2 100644 --- a/httpclient/httpclient_request.go +++ b/httpclient/httpclient_request.go @@ -131,11 +131,11 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}, log l if retryableHTTPMethods[method] { // Define a deadline for total retries based on http client TotalRetryDuration config - totalRetryDeadline := time.Now().Add(c.config.TotalRetryDuration) + totalRetryDeadline := time.Now().Add(c.clientConfig.ClientOptions.TotalRetryDuration) i := 0 for { // Check if we've reached the maximum number of retries or if our total retry time has exceeded - if i > c.config.MaxRetryAttempts || time.Now().After(totalRetryDeadline) { + if i > c.clientConfig.ClientOptions.MaxRetryAttempts || time.Now().After(totalRetryDeadline) { return nil, fmt.Errorf("max retry attempts reached or total retry duration exceeded") }