Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions pkg/client/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"errors"
"time"

"github.com/optimizely/go-sdk/v2/pkg/cache"
"github.com/optimizely/go-sdk/v2/pkg/cmab"
"github.com/optimizely/go-sdk/v2/pkg/config"
"github.com/optimizely/go-sdk/v2/pkg/decide"
Expand All @@ -37,6 +38,28 @@ import (
"github.com/optimizely/go-sdk/v2/pkg/utils"
)

// CmabConfig holds CMAB configuration options exposed at the client level.
// This provides a stable public API while allowing internal cmab.Config to change.
type CmabConfig struct {
CacheSize int
CacheTTL time.Duration
HTTPTimeout time.Duration
Cache cache.CacheWithRemove // Custom cache implementation (Redis, etc.)
}

// toCmabConfig converts client-level CmabConfig to internal cmab.Config
func (c *CmabConfig) toCmabConfig() *cmab.Config {
if c == nil {
return nil
}
return &cmab.Config{
CacheSize: c.CacheSize,
CacheTTL: c.CacheTTL,
HTTPTimeout: c.HTTPTimeout,
Cache: c.Cache,
}
}

// OptimizelyFactory is used to customize and construct an instance of the OptimizelyClient.
type OptimizelyFactory struct {
SDKKey string
Expand All @@ -54,7 +77,7 @@ type OptimizelyFactory struct {
overrideStore decision.ExperimentOverrideStore
userProfileService decision.UserProfileService
notificationCenter notification.Center
cmabConfig *cmab.Config
cmabConfig *CmabConfig

// ODP
segmentsCacheSize int
Expand Down Expand Up @@ -163,7 +186,7 @@ func (f *OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClie
}
// Add CMAB config option if provided
if f.cmabConfig != nil {
experimentServiceOptions = append(experimentServiceOptions, decision.WithCmabConfig(f.cmabConfig))
experimentServiceOptions = append(experimentServiceOptions, decision.WithCmabConfig(f.cmabConfig.toCmabConfig()))
}
compositeExperimentService := decision.NewCompositeExperimentService(f.SDKKey, experimentServiceOptions...)
compositeService := decision.NewCompositeService(f.SDKKey, decision.WithCompositeExperimentService(compositeExperimentService))
Expand Down Expand Up @@ -327,7 +350,7 @@ func WithTracer(tracer tracing.Tracer) OptionFunc {
}

// WithCmabConfig sets the CMAB configuration options
func WithCmabConfig(cmabConfig *cmab.Config) OptionFunc {
func WithCmabConfig(cmabConfig *CmabConfig) OptionFunc {
return func(f *OptimizelyFactory) {
f.cmabConfig = cmabConfig
}
Expand Down
27 changes: 7 additions & 20 deletions pkg/client/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/stretchr/testify/mock"

"github.com/optimizely/go-sdk/v2/pkg/cache"
"github.com/optimizely/go-sdk/v2/pkg/cmab"
"github.com/optimizely/go-sdk/v2/pkg/config"
"github.com/optimizely/go-sdk/v2/pkg/decide"
"github.com/optimizely/go-sdk/v2/pkg/decision"
Expand Down Expand Up @@ -461,13 +460,10 @@ func TestStaticClientError(t *testing.T) {

func TestFactoryWithCmabConfig(t *testing.T) {
factory := OptimizelyFactory{}
cmabConfig := cmab.Config{
cmabConfig := CmabConfig{
CacheSize: 100,
CacheTTL: time.Minute,
HTTPTimeout: 30 * time.Second,
RetryConfig: &cmab.RetryConfig{
MaxRetries: 5,
},
}

// Test the option function
Expand All @@ -477,19 +473,14 @@ func TestFactoryWithCmabConfig(t *testing.T) {
assert.Equal(t, 100, factory.cmabConfig.CacheSize)
assert.Equal(t, time.Minute, factory.cmabConfig.CacheTTL)
assert.Equal(t, 30*time.Second, factory.cmabConfig.HTTPTimeout)
assert.NotNil(t, factory.cmabConfig.RetryConfig)
assert.Equal(t, 5, factory.cmabConfig.RetryConfig.MaxRetries)
}

func TestFactoryCmabConfigPassedToDecisionService(t *testing.T) {
// Test that CMAB config is correctly passed to decision service when creating client
cmabConfig := cmab.Config{
cmabConfig := CmabConfig{
CacheSize: 200,
CacheTTL: 2 * time.Minute,
HTTPTimeout: 20 * time.Second,
RetryConfig: &cmab.RetryConfig{
MaxRetries: 3,
},
}

factory := OptimizelyFactory{
Expand All @@ -501,7 +492,6 @@ func TestFactoryCmabConfigPassedToDecisionService(t *testing.T) {
assert.Equal(t, &cmabConfig, factory.cmabConfig)
assert.Equal(t, 200, factory.cmabConfig.CacheSize)
assert.Equal(t, 2*time.Minute, factory.cmabConfig.CacheTTL)
assert.NotNil(t, factory.cmabConfig.RetryConfig)
}

func TestFactoryOptionFunctions(t *testing.T) {
Expand All @@ -512,19 +502,19 @@ func TestFactoryOptionFunctions(t *testing.T) {
WithSegmentsCacheSize(100)(factory)
WithSegmentsCacheTimeout(5 * time.Second)(factory)
WithOdpDisabled(true)(factory)
WithCmabConfig(&cmab.Config{CacheSize: 50})(factory)
WithCmabConfig(&CmabConfig{CacheSize: 50})(factory)

// Verify options were set
assert.Equal(t, "test_token", factory.DatafileAccessToken)
assert.Equal(t, 100, factory.segmentsCacheSize)
assert.Equal(t, 5*time.Second, factory.segmentsCacheTimeout)
assert.True(t, factory.odpDisabled)
assert.Equal(t, &cmab.Config{CacheSize: 50}, factory.cmabConfig)
assert.Equal(t, &CmabConfig{CacheSize: 50}, factory.cmabConfig)
}

func TestWithCmabConfigOption(t *testing.T) {
factory := &OptimizelyFactory{}
testConfig := cmab.Config{
testConfig := CmabConfig{
CacheSize: 200,
CacheTTL: 2 * time.Minute,
}
Expand All @@ -534,13 +524,10 @@ func TestWithCmabConfigOption(t *testing.T) {

func TestClientWithCmabConfig(t *testing.T) {
// Test client creation with non-empty CMAB config (tests reflect.DeepEqual path)
cmabConfig := cmab.Config{
cmabConfig := CmabConfig{
CacheSize: 200,
CacheTTL: 5 * time.Minute,
HTTPTimeout: 30 * time.Second,
RetryConfig: &cmab.RetryConfig{
MaxRetries: 5,
},
}

factory := OptimizelyFactory{
Expand All @@ -559,7 +546,7 @@ func TestClientWithCmabConfig(t *testing.T) {

func TestClientWithEmptyCmabConfig(t *testing.T) {
// Test client creation with empty CMAB config (tests reflect.DeepEqual returns true)
emptyCmabConfig := cmab.Config{}
emptyCmabConfig := CmabConfig{}

factory := OptimizelyFactory{
SDKKey: "test_sdk_key",
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmab/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var CMABPredictionEndpoint = "https://prediction.cmab.optimizely.com/predict/%s"

const (
// DefaultMaxRetries is the default number of retries for CMAB requests
DefaultMaxRetries = 3
DefaultMaxRetries = 1
// DefaultInitialBackoff is the default initial backoff duration
DefaultInitialBackoff = 100 * time.Millisecond
// DefaultMaxBackoff is the default maximum backoff duration
Expand Down
4 changes: 0 additions & 4 deletions pkg/cmab/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ type Config struct {
CacheSize int
CacheTTL time.Duration
HTTPTimeout time.Duration
RetryConfig *RetryConfig
Cache cache.CacheWithRemove // Custom cache implementation (Redis, etc.)
}

Expand All @@ -48,8 +47,5 @@ func NewDefaultConfig() Config {
CacheSize: DefaultCacheSize,
CacheTTL: DefaultCacheTTL,
HTTPTimeout: DefaultHTTPTimeout,
RetryConfig: &RetryConfig{
MaxRetries: DefaultMaxRetries,
},
}
}
2 changes: 0 additions & 2 deletions pkg/cmab/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ func TestNewDefaultConfig(t *testing.T) {
assert.Equal(t, DefaultCacheSize, config.CacheSize)
assert.Equal(t, DefaultCacheTTL, config.CacheTTL)
assert.Equal(t, DefaultHTTPTimeout, config.HTTPTimeout)
assert.NotNil(t, config.RetryConfig)
assert.Equal(t, DefaultMaxRetries, config.RetryConfig.MaxRetries)
assert.Nil(t, config.Cache) // Should be nil by default
}

Expand Down
1 change: 0 additions & 1 deletion pkg/cmab/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,6 @@ func (s *CmabServiceTestSuite) TestGetDecisionApiError() {
s.mockClient.AssertExpectations(s.T())
}


// TestLockStripingDistribution verifies that different user/rule combinations
// use different locks to allow for better concurrency
func TestLockStripingDistribution(t *testing.T) {
Expand Down
5 changes: 0 additions & 5 deletions pkg/decision/composite_experiment_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,6 @@ func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentServiceWithCmab
CacheSize: 200,
CacheTTL: 5 * time.Minute,
HTTPTimeout: 30 * time.Second,
RetryConfig: &cmab.RetryConfig{
MaxRetries: 5,
},
}

compositeExperimentService := NewCompositeExperimentService("test-sdk-key",
Expand All @@ -266,8 +263,6 @@ func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentServiceWithCmab
s.Equal(200, compositeExperimentService.cmabConfig.CacheSize) // From config
s.Equal(5*time.Minute, compositeExperimentService.cmabConfig.CacheTTL) // From config
s.Equal(30*time.Second, compositeExperimentService.cmabConfig.HTTPTimeout) // From config
s.NotNil(compositeExperimentService.cmabConfig.RetryConfig)
s.Equal(5, compositeExperimentService.cmabConfig.RetryConfig.MaxRetries) // From config

// Verify service order
s.Equal(3, len(compositeExperimentService.experimentServices))
Expand Down
38 changes: 7 additions & 31 deletions pkg/decision/experiment_cmab_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,13 @@ func NewExperimentCmabService(sdkKey string, config *cmab.Config) *ExperimentCma
var cacheSize int
var cacheTTL time.Duration
var httpTimeout time.Duration
var retryConfig *cmab.RetryConfig
var customCache cache.CacheWithRemove

if config == nil {
// Use all defaults
cacheSize = cmab.DefaultCacheSize
cacheTTL = cmab.DefaultCacheTTL
httpTimeout = cmab.DefaultHTTPTimeout
retryConfig = &cmab.RetryConfig{
MaxRetries: cmab.DefaultMaxRetries,
InitialBackoff: cmab.DefaultInitialBackoff,
MaxBackoff: cmab.DefaultMaxBackoff,
BackoffMultiplier: cmab.DefaultBackoffMultiplier,
}
} else {
// Config is not nil, use defaults for zero values
customCache = config.Cache
Expand All @@ -82,31 +75,14 @@ func NewExperimentCmabService(sdkKey string, config *cmab.Config) *ExperimentCma
if httpTimeout == 0 {
httpTimeout = cmab.DefaultHTTPTimeout
}
}

// Handle retry config
if config.RetryConfig == nil {
retryConfig = &cmab.RetryConfig{
MaxRetries: cmab.DefaultMaxRetries,
InitialBackoff: cmab.DefaultInitialBackoff,
MaxBackoff: cmab.DefaultMaxBackoff,
BackoffMultiplier: cmab.DefaultBackoffMultiplier,
}
} else {
retryConfig = config.RetryConfig
// Apply defaults for zero values in provided RetryConfig
if retryConfig.MaxRetries == 0 {
retryConfig.MaxRetries = cmab.DefaultMaxRetries
}
if retryConfig.InitialBackoff == 0 {
retryConfig.InitialBackoff = cmab.DefaultInitialBackoff
}
if retryConfig.MaxBackoff == 0 {
retryConfig.MaxBackoff = cmab.DefaultMaxBackoff
}
if retryConfig.BackoffMultiplier == 0 {
retryConfig.BackoffMultiplier = cmab.DefaultBackoffMultiplier
}
}
// Always use hardcoded retry config (not exposed in public API)
retryConfig := &cmab.RetryConfig{
MaxRetries: cmab.DefaultMaxRetries,
InitialBackoff: cmab.DefaultInitialBackoff,
MaxBackoff: cmab.DefaultMaxBackoff,
BackoffMultiplier: cmab.DefaultBackoffMultiplier,
}

// Use custom cache if provided, otherwise create default LRU cache
Expand Down