diff --git a/pkg/client/client.go b/pkg/client/client.go index a4694d60..254ac79f 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -206,7 +206,7 @@ func (o *OptimizelyClient) decide(userContext *OptimizelyUserContext, key string } if err != nil { - o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature %q: %s`, key, err)) + return o.handleDecisionServiceError(err, key, *userContext) } if featureDecision.Variation != nil { @@ -1248,3 +1248,9 @@ func (o *OptimizelyClient) getDecisionVariableMap(feature entities.Feature, vari func isNil(v interface{}) bool { return v == nil || (reflect.ValueOf(v).Kind() == reflect.Ptr && reflect.ValueOf(v).IsNil()) } + +func (o *OptimizelyClient) handleDecisionServiceError(err error, key string, userContext OptimizelyUserContext) OptimizelyDecision { + o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature %q: %s`, key, err)) + + return NewErrorDecision(key, userContext, err) +} diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 0bc37a6f..16f91bd0 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -1682,8 +1682,7 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) { ConfigManager: mockConfigManager, DecisionService: mockDecisionService, logger: logging.GetLogger("", ""), - tracer: &MockTracer{}, - } + tracer: &MockTracer{}} _, decision, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, expectedFeatureDecision, decision) diff --git a/pkg/decision/cmab_client.go b/pkg/cmab/client.go similarity index 77% rename from pkg/decision/cmab_client.go rename to pkg/cmab/client.go index 49cc51f0..8a010e3c 100644 --- a/pkg/decision/cmab_client.go +++ b/pkg/cmab/client.go @@ -14,8 +14,8 @@ * limitations under the License. * ***************************************************************************/ -// Package decision provides CMAB client implementation -package decision +// Package cmab provides contextual multi-armed bandit functionality +package cmab import ( "bytes" @@ -44,34 +44,34 @@ const ( DefaultBackoffMultiplier = 2.0 ) -// CMABAttribute represents an attribute in a CMAB request -type CMABAttribute struct { +// Attribute represents an attribute in a CMAB request +type Attribute struct { ID string `json:"id"` Value interface{} `json:"value"` Type string `json:"type"` } -// CMABInstance represents an instance in a CMAB request -type CMABInstance struct { - VisitorID string `json:"visitorId"` - ExperimentID string `json:"experimentId"` - Attributes []CMABAttribute `json:"attributes"` - CmabUUID string `json:"cmabUUID"` +// Instance represents an instance in a CMAB request +type Instance struct { + VisitorID string `json:"visitorId"` + ExperimentID string `json:"experimentId"` + Attributes []Attribute `json:"attributes"` + CmabUUID string `json:"cmabUUID"` } -// CMABRequest represents a request to the CMAB API -type CMABRequest struct { - Instances []CMABInstance `json:"instances"` +// Request represents a request to the CMAB API +type Request struct { + Instances []Instance `json:"instances"` } -// CMABPrediction represents a prediction in a CMAB response -type CMABPrediction struct { +// Prediction represents a prediction in a CMAB response +type Prediction struct { VariationID string `json:"variation_id"` } -// CMABResponse represents a response from the CMAB API -type CMABResponse struct { - Predictions []CMABPrediction `json:"predictions"` +// Response represents a response from the CMAB API +type Response struct { + Predictions []Prediction `json:"predictions"` } // RetryConfig defines configuration for retry behavior @@ -93,15 +93,15 @@ type DefaultCmabClient struct { logger logging.OptimizelyLogProducer } -// CmabClientOptions defines options for creating a CMAB client -type CmabClientOptions struct { +// ClientOptions defines options for creating a CMAB client +type ClientOptions struct { HTTPClient *http.Client RetryConfig *RetryConfig Logger logging.OptimizelyLogProducer } // NewDefaultCmabClient creates a new instance of DefaultCmabClient -func NewDefaultCmabClient(options CmabClientOptions) *DefaultCmabClient { +func NewDefaultCmabClient(options ClientOptions) *DefaultCmabClient { httpClient := options.HTTPClient if httpClient == nil { httpClient = &http.Client{ @@ -127,24 +127,19 @@ func NewDefaultCmabClient(options CmabClientOptions) *DefaultCmabClient { // FetchDecision fetches a decision from the CMAB API func (c *DefaultCmabClient) FetchDecision( - ctx context.Context, ruleID string, userID string, attributes map[string]interface{}, cmabUUID string, ) (string, error) { - // If no context is provided, create a background context - if ctx == nil { - ctx = context.Background() - } // Create the URL url := fmt.Sprintf(CMABPredictionEndpoint, ruleID) // Convert attributes to CMAB format - cmabAttributes := make([]CMABAttribute, 0, len(attributes)) + cmabAttributes := make([]Attribute, 0, len(attributes)) for key, value := range attributes { - cmabAttributes = append(cmabAttributes, CMABAttribute{ + cmabAttributes = append(cmabAttributes, Attribute{ ID: key, Value: value, Type: "custom_attribute", @@ -152,8 +147,8 @@ func (c *DefaultCmabClient) FetchDecision( } // Create the request body - requestBody := CMABRequest{ - Instances: []CMABInstance{ + requestBody := Request{ + Instances: []Instance{ { VisitorID: userID, ExperimentID: ruleID, @@ -171,26 +166,26 @@ func (c *DefaultCmabClient) FetchDecision( // If no retry config, just do a single fetch if c.retryConfig == nil { - return c.doFetch(ctx, url, bodyBytes) + return c.doFetch(context.Background(), url, bodyBytes) } // Retry sending request with exponential backoff + var lastErr error for i := 0; i <= c.retryConfig.MaxRetries; i++ { - // Check if context is done - if ctx.Err() != nil { - return "", fmt.Errorf("context canceled or timed out: %w", ctx.Err()) - } - // Make the request - result, err := c.doFetch(ctx, url, bodyBytes) + result, err := c.doFetch(context.Background(), url, bodyBytes) if err == nil { return result, nil } - // If this is the last retry, return the error - if i == c.retryConfig.MaxRetries { - return "", fmt.Errorf("failed to fetch CMAB decision after %d attempts: %w", - c.retryConfig.MaxRetries, err) + lastErr = err + + // Don't wait after the last attempt + if i < c.retryConfig.MaxRetries { + backoffDuration := c.retryConfig.InitialBackoff * time.Duration(1< 0 && response.Predictions[0].VariationID != "" } diff --git a/pkg/decision/cmab_client_test.go b/pkg/cmab/client_test.go similarity index 84% rename from pkg/decision/cmab_client_test.go rename to pkg/cmab/client_test.go index a84a6b20..9080a0f5 100644 --- a/pkg/decision/cmab_client_test.go +++ b/pkg/cmab/client_test.go @@ -14,11 +14,10 @@ * limitations under the License. * ***************************************************************************/ -// Package decision // -package decision +// Package cmab // +package cmab import ( - "context" "encoding/json" "fmt" "io" @@ -65,7 +64,7 @@ func TestDefaultCmabClient_FetchDecision(t *testing.T) { assert.Equal(t, "application/json", r.Header.Get("Content-Type")) // Parse request body - var requestBody CMABRequest + var requestBody Request err := json.NewDecoder(r.Body).Decode(&requestBody) assert.NoError(t, err) @@ -80,7 +79,7 @@ func TestDefaultCmabClient_FetchDecision(t *testing.T) { assert.Len(t, instance.Attributes, 5) // Create a map for easier attribute checking - attrMap := make(map[string]CMABAttribute) + attrMap := make(map[string]Attribute) for _, attr := range instance.Attributes { attrMap[attr.ID] = attr assert.Equal(t, "custom_attribute", attr.Type) @@ -109,8 +108,8 @@ func TestDefaultCmabClient_FetchDecision(t *testing.T) { // Return response w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - response := CMABResponse{ - Predictions: []CMABPrediction{ + response := Response{ + Predictions: []Prediction{ { VariationID: "var123", }, @@ -121,7 +120,7 @@ func TestDefaultCmabClient_FetchDecision(t *testing.T) { defer server.Close() // Create client with custom endpoint - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -141,10 +140,7 @@ func TestDefaultCmabClient_FetchDecision(t *testing.T) { "null_attr": nil, } - // Create a context for the request - ctx := context.Background() - - variationID, err := client.FetchDecision(ctx, "rule456", "user123", attributes, "test-uuid") + variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") // Verify results assert.NoError(t, err) @@ -167,7 +163,7 @@ func TestDefaultCmabClient_FetchDecision_WithRetry(t *testing.T) { body, err := io.ReadAll(r.Body) assert.NoError(t, err) - var requestBody CMABRequest + var requestBody Request err = json.Unmarshal(body, &requestBody) assert.NoError(t, err) @@ -187,8 +183,8 @@ func TestDefaultCmabClient_FetchDecision_WithRetry(t *testing.T) { // Return success response on third attempt w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - response := CMABResponse{ - Predictions: []CMABPrediction{ + response := Response{ + Predictions: []Prediction{ { VariationID: "var123", }, @@ -199,7 +195,7 @@ func TestDefaultCmabClient_FetchDecision_WithRetry(t *testing.T) { defer server.Close() // Create client with custom endpoint and retry config - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -223,7 +219,7 @@ func TestDefaultCmabClient_FetchDecision_WithRetry(t *testing.T) { } startTime := time.Now() - variationID, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") duration := time.Since(startTime) // Verify results @@ -247,7 +243,7 @@ func TestDefaultCmabClient_FetchDecision_ExhaustedRetries(t *testing.T) { defer server.Close() // Create client with custom endpoint and retry config - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -270,7 +266,15 @@ func TestDefaultCmabClient_FetchDecision_ExhaustedRetries(t *testing.T) { "isMobile": true, } - variationID, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") + + t.Logf("Actual error: %q", err.Error()) + t.Logf("Request count: %d", requestCount) + + // Verify results + assert.Error(t, err) + assert.Equal(t, "", variationID) + assert.Equal(t, 3, requestCount, "Expected 3 request attempts (initial + 2 retries)") // Verify results assert.Error(t, err) @@ -292,7 +296,7 @@ func TestDefaultCmabClient_FetchDecision_NoRetryConfig(t *testing.T) { defer server.Close() // Create client with custom endpoint but no retry config - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -309,7 +313,7 @@ func TestDefaultCmabClient_FetchDecision_NoRetryConfig(t *testing.T) { "browser": "chrome", } - _, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + _, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") // Verify results assert.Error(t, err) @@ -356,7 +360,7 @@ func TestDefaultCmabClient_FetchDecision_InvalidResponse(t *testing.T) { defer server.Close() // Create client with custom endpoint - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -372,7 +376,7 @@ func TestDefaultCmabClient_FetchDecision_InvalidResponse(t *testing.T) { "browser": "chrome", } - _, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + _, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") // Verify results assert.Error(t, err) @@ -393,7 +397,7 @@ func TestDefaultCmabClient_FetchDecision_NetworkErrors(t *testing.T) { } // Create client with non-existent server to simulate network errors - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 100 * time.Millisecond, // Short timeout to fail quickly }, @@ -416,7 +420,7 @@ func TestDefaultCmabClient_FetchDecision_NetworkErrors(t *testing.T) { "browser": "chrome", } - _, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + _, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") // Verify results assert.Error(t, err) @@ -442,8 +446,8 @@ func TestDefaultCmabClient_ExponentialBackoff(t *testing.T) { // Return success response w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - response := CMABResponse{ - Predictions: []CMABPrediction{ + response := Response{ + Predictions: []Prediction{ { VariationID: "var123", }, @@ -454,7 +458,7 @@ func TestDefaultCmabClient_ExponentialBackoff(t *testing.T) { defer server.Close() // Create client with custom endpoint and specific retry config - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -476,7 +480,7 @@ func TestDefaultCmabClient_ExponentialBackoff(t *testing.T) { "browser": "chrome", } - variationID, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") // Verify results require.NoError(t, err) @@ -504,7 +508,7 @@ func TestDefaultCmabClient_ExponentialBackoff(t *testing.T) { func TestNewDefaultCmabClient_DefaultValues(t *testing.T) { // Test with empty options - client := NewDefaultCmabClient(CmabClientOptions{}) + client := NewDefaultCmabClient(ClientOptions{}) // Verify default values assert.NotNil(t, client.httpClient) @@ -541,7 +545,7 @@ func TestDefaultCmabClient_LoggingBehavior(t *testing.T) { defer server.Close() // Create client with custom logger - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -564,7 +568,7 @@ func TestDefaultCmabClient_LoggingBehavior(t *testing.T) { "browser": "chrome", } - _, err := client.FetchDecision(context.Background(), "rule456", "user123", attributes, "test-uuid") + _, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") assert.NoError(t, err) // Verify log messages @@ -610,7 +614,7 @@ func TestDefaultCmabClient_NonSuccessStatusCode(t *testing.T) { defer server.Close() // Create client with custom endpoint and no retries - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, @@ -627,10 +631,7 @@ func TestDefaultCmabClient_NonSuccessStatusCode(t *testing.T) { "browser": "chrome", } - // Create a context for the request - ctx := context.Background() - - variationID, err := client.FetchDecision(ctx, "rule456", "user123", attributes, "test-uuid") + variationID, err := client.FetchDecision("rule456", "user123", attributes, "test-uuid") // Verify results assert.Error(t, err, "Expected error for non-success status code") @@ -640,51 +641,3 @@ func TestDefaultCmabClient_NonSuccessStatusCode(t *testing.T) { }) } } - -func TestDefaultCmabClient_FetchDecision_ContextCancellation(t *testing.T) { - // Setup test server that delays response - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Sleep to simulate a slow response - time.Sleep(500 * time.Millisecond) - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - response := CMABResponse{ - Predictions: []CMABPrediction{ - { - VariationID: "var123", - }, - }, - } - json.NewEncoder(w).Encode(response) - })) - defer server.Close() - - // Create client with custom endpoint - client := NewDefaultCmabClient(CmabClientOptions{ - HTTPClient: &http.Client{ - Timeout: 5 * time.Second, - }, - }) - - // Override the endpoint for testing - originalEndpoint := CMABPredictionEndpoint - CMABPredictionEndpoint = server.URL + "/%s" - defer func() { CMABPredictionEndpoint = originalEndpoint }() - - // Create a context with a short timeout - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - - // Test fetch decision with a context that will time out - attributes := map[string]interface{}{ - "browser": "chrome", - } - - _, err := client.FetchDecision(ctx, "rule456", "user123", attributes, "test-uuid") - - // Verify that we got a context deadline exceeded error - assert.Error(t, err) - assert.Contains(t, err.Error(), "context") - assert.Contains(t, err.Error(), "deadline exceeded") -} diff --git a/pkg/decision/cmab_service.go b/pkg/cmab/service.go similarity index 90% rename from pkg/decision/cmab_service.go rename to pkg/cmab/service.go index 995170ba..3801dc07 100644 --- a/pkg/decision/cmab_service.go +++ b/pkg/cmab/service.go @@ -14,8 +14,8 @@ * limitations under the License. * ***************************************************************************/ -// Package decision provides CMAB decision service implementation -package decision +// Package cmab // +package cmab import ( "encoding/json" @@ -35,19 +35,19 @@ import ( // DefaultCmabService implements the CmabService interface type DefaultCmabService struct { cmabCache cache.CacheWithRemove - cmabClient CmabClient + cmabClient Client logger logging.OptimizelyLogProducer } -// CmabServiceOptions defines options for creating a CMAB service -type CmabServiceOptions struct { +// ServiceOptions defines options for creating a CMAB service +type ServiceOptions struct { Logger logging.OptimizelyLogProducer CmabCache cache.CacheWithRemove - CmabClient CmabClient + CmabClient Client } // NewDefaultCmabService creates a new instance of DefaultCmabService -func NewDefaultCmabService(options CmabServiceOptions) *DefaultCmabService { +func NewDefaultCmabService(options ServiceOptions) *DefaultCmabService { logger := options.Logger if logger == nil { logger = logging.GetLogger("", "DefaultCmabService") @@ -66,7 +66,7 @@ func (s *DefaultCmabService) GetDecision( userContext entities.UserContext, ruleID string, options *decide.Options, -) (CmabDecision, error) { +) (Decision, error) { // Initialize reasons slice for decision reasons := []string{} @@ -78,7 +78,7 @@ func (s *DefaultCmabService) GetDecision( reasons = append(reasons, "Ignoring CMAB cache as requested") decision, err := s.fetchDecisionWithRetry(ruleID, userContext.ID, filteredAttributes) if err != nil { - return CmabDecision{Reasons: reasons}, err + return Decision{Reasons: reasons}, err } decision.Reasons = append(reasons, decision.Reasons...) return decision, nil @@ -103,13 +103,13 @@ func (s *DefaultCmabService) GetDecision( attributesJSON, err := s.getAttributesJSON(filteredAttributes) if err != nil { reasons = append(reasons, fmt.Sprintf("Failed to serialize attributes: %v", err)) - return CmabDecision{Reasons: reasons}, fmt.Errorf("failed to serialize attributes: %w", err) + return Decision{Reasons: reasons}, fmt.Errorf("failed to serialize attributes: %w", err) } hasher := murmur3.SeedNew32(1) // Use seed 1 for consistency _, err = hasher.Write([]byte(attributesJSON)) if err != nil { reasons = append(reasons, fmt.Sprintf("Failed to hash attributes: %v", err)) - return CmabDecision{Reasons: reasons}, fmt.Errorf("failed to hash attributes: %w", err) + return Decision{Reasons: reasons}, fmt.Errorf("failed to hash attributes: %w", err) } attributesHash := strconv.FormatUint(uint64(hasher.Sum32()), 10) @@ -117,12 +117,12 @@ func (s *DefaultCmabService) GetDecision( cachedValue := s.cmabCache.Lookup(cacheKey) if cachedValue != nil { // Need to type assert since Lookup returns interface{} - if cacheVal, ok := cachedValue.(CmabCacheValue); ok { + if cacheVal, ok := cachedValue.(CacheValue); ok { // Check if attributes have changed if cacheVal.AttributesHash == attributesHash { s.logger.Debug(fmt.Sprintf("Returning cached CMAB decision for rule %s and user %s", ruleID, userContext.ID)) reasons = append(reasons, "Returning cached CMAB decision") - return CmabDecision{ + return Decision{ VariationID: cacheVal.VariationID, CmabUUID: cacheVal.CmabUUID, Reasons: reasons, @@ -139,11 +139,11 @@ func (s *DefaultCmabService) GetDecision( decision, err := s.fetchDecisionWithRetry(ruleID, userContext.ID, filteredAttributes) if err != nil { decision.Reasons = append(reasons, decision.Reasons...) - return decision, err + return decision, fmt.Errorf("CMAB API error: %w", err) } // Cache the decision - cacheValue := CmabCacheValue{ + cacheValue := CacheValue{ AttributesHash: attributesHash, VariationID: decision.VariationID, CmabUUID: decision.CmabUUID, @@ -161,7 +161,7 @@ func (s *DefaultCmabService) fetchDecisionWithRetry( ruleID string, userID string, attributes map[string]interface{}, -) (CmabDecision, error) { +) (Decision, error) { cmabUUID := uuid.New().String() reasons := []string{} @@ -186,7 +186,7 @@ func (s *DefaultCmabService) fetchDecisionWithRetry( variationID, err := s.cmabClient.FetchDecision(ruleID, userID, attributes, cmabUUID) if err == nil { reasons = append(reasons, fmt.Sprintf("Successfully fetched CMAB decision on attempt %d/%d", attempt+1, maxRetries)) - return CmabDecision{ + return Decision{ VariationID: variationID, CmabUUID: cmabUUID, Reasons: reasons, @@ -199,7 +199,7 @@ func (s *DefaultCmabService) fetchDecisionWithRetry( } reasons = append(reasons, fmt.Sprintf("Failed to fetch CMAB decision after %d attempts", maxRetries)) - return CmabDecision{Reasons: reasons}, fmt.Errorf("failed to fetch CMAB decision after %d attempts: %w", + return Decision{Reasons: reasons}, fmt.Errorf("failed to fetch CMAB decision after %d attempts: %w", maxRetries, lastErr) } diff --git a/pkg/decision/cmab_service_test.go b/pkg/cmab/service_test.go similarity index 86% rename from pkg/decision/cmab_service_test.go rename to pkg/cmab/service_test.go index 08333ef1..db49eff9 100644 --- a/pkg/decision/cmab_service_test.go +++ b/pkg/cmab/service_test.go @@ -14,8 +14,8 @@ * limitations under the License. * ***************************************************************************/ -// Package decision // -package decision +// Package cmab // +package cmab import ( "errors" @@ -241,7 +241,7 @@ func (s *CmabServiceTestSuite) SetupTest() { s.mockConfig = new(MockProjectConfig) // Set up the CMAB service - s.cmabService = NewDefaultCmabService(CmabServiceOptions{ + s.cmabService = NewDefaultCmabService(ServiceOptions{ Logger: logging.GetLogger("test", "CmabService"), CmabCache: s.mockCache, CmabClient: s.mockClient, @@ -258,10 +258,17 @@ func (s *CmabServiceTestSuite) SetupTest() { func (s *CmabServiceTestSuite) TestGetDecision() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, + ID: s.testRuleID, + Key: "test_experiment", Cmab: &entities.Cmab{ AttributeIds: []string{"attr1", "attr2"}, }, + TrafficAllocation: []entities.Range{ + { + EntityID: "variation1", + EndOfRange: 10000, + }, + }, } // Setup mock config @@ -303,16 +310,24 @@ func (s *CmabServiceTestSuite) TestGetDecision() { func (s *CmabServiceTestSuite) TestGetDecisionWithCache() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, + ID: s.testRuleID, + Key: "test_experiment", Cmab: &entities.Cmab{ AttributeIds: []string{"attr1", "attr2"}, }, + TrafficAllocation: []entities.Range{ + { + EntityID: "variation1", + EndOfRange: 10000, + }, + }, } // Setup mock config s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) - s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil).Maybe() + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context userContext := entities.UserContext{ @@ -330,13 +345,16 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithCache() { attributesHash := strconv.FormatUint(uint64(hasher.Sum32()), 10) // Setup cache hit with matching attributes hash - cachedValue := CmabCacheValue{ + cachedValue := CacheValue{ AttributesHash: attributesHash, VariationID: "cached-variant", CmabUUID: "cached-uuid", } s.mockCache.On("Lookup", cacheKey).Return(cachedValue) + // Mock the Remove method - it might be called if attributes hash doesn't match + s.mockCache.On("Remove", cacheKey).Maybe() + // Test with cache hit decision, err := s.cmabService.GetDecision(s.mockConfig, userContext, s.testRuleID, nil) s.NoError(err) @@ -350,16 +368,24 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithCache() { func (s *CmabServiceTestSuite) TestGetDecisionWithIgnoreCache() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, + ID: s.testRuleID, + Key: "test_experiment", Cmab: &entities.Cmab{ AttributeIds: []string{"attr1", "attr2"}, }, + TrafficAllocation: []entities.Range{ + { + EntityID: "variation1", + EndOfRange: 10000, + }, + }, } // Setup mock config s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context userContext := entities.UserContext{ @@ -396,16 +422,24 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithIgnoreCache() { func (s *CmabServiceTestSuite) TestGetDecisionWithResetCache() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, + ID: s.testRuleID, + Key: "test_experiment", Cmab: &entities.Cmab{ AttributeIds: []string{"attr1", "attr2"}, }, + TrafficAllocation: []entities.Range{ + { + EntityID: "variation1", + EndOfRange: 10000, + }, + }, } // Setup mock config s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context userContext := entities.UserContext{ @@ -445,16 +479,24 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithResetCache() { func (s *CmabServiceTestSuite) TestGetDecisionWithInvalidateUserCache() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, + ID: s.testRuleID, + Key: "test_experiment", Cmab: &entities.Cmab{ AttributeIds: []string{"attr1", "attr2"}, }, + TrafficAllocation: []entities.Range{ + { + EntityID: "variation1", + EndOfRange: 10000, + }, + }, } // Setup mock config s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context userContext := entities.UserContext{ @@ -494,9 +536,14 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithInvalidateUserCache() { func (s *CmabServiceTestSuite) TestGetDecisionError() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, - Cmab: &entities.Cmab{ - AttributeIds: []string{"attr1", "attr2"}, + ID: s.testRuleID, + Key: "test_experiment", // Add experiment key + // Other experiment properties... + TrafficAllocation: []entities.Range{ // Add traffic allocation + { + EntityID: "variation1", + EndOfRange: 10000, + }, }, } @@ -504,6 +551,7 @@ func (s *CmabServiceTestSuite) TestGetDecisionError() { s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context userContext := entities.UserContext{ @@ -565,16 +613,24 @@ func (s *CmabServiceTestSuite) TestFilterAttributes() { func (s *CmabServiceTestSuite) TestOnlyFilteredAttributesPassedToClient() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, + ID: s.testRuleID, + Key: "test_experiment", Cmab: &entities.Cmab{ AttributeIds: []string{"attr1", "attr2"}, }, + TrafficAllocation: []entities.Range{ + { + EntityID: "variation1", + EndOfRange: 10000, + }, + }, } // Setup mock config s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context with extra attributes that should be filtered out userContext := entities.UserContext{ @@ -634,9 +690,14 @@ func (s *CmabServiceTestSuite) TestOnlyFilteredAttributesPassedToClient() { func (s *CmabServiceTestSuite) TestCacheInvalidatedWhenAttributesChange() { // Setup mock experiment with CMAB configuration experiment := entities.Experiment{ - ID: s.testRuleID, - Cmab: &entities.Cmab{ - AttributeIds: []string{"attr1", "attr2"}, + ID: s.testRuleID, + Key: "test_experiment", // Add experiment key + // Other experiment properties... + TrafficAllocation: []entities.Range{ // Add traffic allocation + { + EntityID: "variation1", + EndOfRange: 10000, + }, }, } @@ -644,6 +705,7 @@ func (s *CmabServiceTestSuite) TestCacheInvalidatedWhenAttributesChange() { s.mockConfig.On("GetAttributeKeyByID", "attr1").Return("age", nil) s.mockConfig.On("GetAttributeKeyByID", "attr2").Return("location", nil) s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(experiment, nil) + s.mockConfig.On("GetExperimentByKey", "test_experiment").Return(experiment, nil) // Create user context userContext := entities.UserContext{ @@ -659,7 +721,7 @@ func (s *CmabServiceTestSuite) TestCacheInvalidatedWhenAttributesChange() { // First, create a cached value with a different attributes hash oldAttributesHash := "old-hash" - cachedValue := CmabCacheValue{ + cachedValue := CacheValue{ AttributesHash: oldAttributesHash, VariationID: "cached-variant", CmabUUID: "cached-uuid", @@ -693,7 +755,7 @@ func (s *CmabServiceTestSuite) TestCacheInvalidatedWhenAttributesChange() { s.mockClient.AssertCalled(s.T(), "FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything) // Verify new decision was cached - s.mockCache.AssertCalled(s.T(), "Save", cacheKey, mock.MatchedBy(func(value CmabCacheValue) bool { + s.mockCache.AssertCalled(s.T(), "Save", cacheKey, mock.MatchedBy(func(value CacheValue) bool { return value.VariationID == expectedVariationID && value.AttributesHash != oldAttributesHash })) } @@ -725,7 +787,7 @@ func (s *CmabServiceTestSuite) TestGetCacheKey() { func (s *CmabServiceTestSuite) TestNewDefaultCmabService() { // Test with default options - service := NewDefaultCmabService(CmabServiceOptions{}) + service := NewDefaultCmabService(ServiceOptions{}) // Only check that the service is created, not the specific fields s.NotNil(service) @@ -734,3 +796,36 @@ func (s *CmabServiceTestSuite) TestNewDefaultCmabService() { func TestCmabServiceTestSuite(t *testing.T) { suite.Run(t, new(CmabServiceTestSuite)) } + +func (s *CmabServiceTestSuite) TestGetDecisionApiError() { + // Setup cache key + cacheKey := s.cmabService.getCacheKey(s.testUserID, s.testRuleID) + + // Setup cache lookup (cache miss) + s.mockCache.On("Lookup", cacheKey).Return(nil) + + // Setup mock to return error for experiment lookup (but this won't stop the flow anymore) + s.mockConfig.On("GetExperimentByID", s.testRuleID).Return(entities.Experiment{}, fmt.Errorf("experiment not found")).Once() + + // Mock the FetchDecision call that will now happen + s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return("", fmt.Errorf("invalid rule ID")) + + // Call the method + userContext := entities.UserContext{ + ID: s.testUserID, + Attributes: map[string]interface{}{ + "age": 30, + }, + } + + _, err := s.cmabService.GetDecision(s.mockConfig, userContext, s.testRuleID, nil) + + // Should return error from FetchDecision, not from experiment validation + s.Error(err) + s.Contains(err.Error(), "CMAB API error") + + // Verify expectations + s.mockConfig.AssertExpectations(s.T()) + s.mockCache.AssertExpectations(s.T()) + s.mockClient.AssertExpectations(s.T()) +} diff --git a/pkg/decision/cmab.go b/pkg/cmab/types.go similarity index 76% rename from pkg/decision/cmab.go rename to pkg/cmab/types.go index 97d4ec81..66cab15a 100644 --- a/pkg/decision/cmab.go +++ b/pkg/cmab/types.go @@ -14,8 +14,10 @@ * limitations under the License. * ***************************************************************************/ -// Package decision provides CMAB decision service interfaces and types -package decision +// Package cmab provides functionality for Contextual Multi-Armed Bandit (CMAB) +// decision-making, including client and service implementations for making and +// handling CMAB requests and responses. +package cmab import ( "github.com/optimizely/go-sdk/v2/pkg/config" @@ -23,33 +25,33 @@ import ( "github.com/optimizely/go-sdk/v2/pkg/entities" ) -// CmabDecision represents a decision from the CMAB service -type CmabDecision struct { +// Decision represents a decision from the CMAB service +type Decision struct { VariationID string CmabUUID string Reasons []string } -// CmabCacheValue represents a cached CMAB decision with attribute hash -type CmabCacheValue struct { +// CacheValue represents a cached CMAB decision with attribute hash +type CacheValue struct { AttributesHash string VariationID string CmabUUID string } -// CmabService defines the interface for CMAB decision services -type CmabService interface { +// Service defines the interface for CMAB decision services +type Service interface { // GetDecision returns a CMAB decision for the given rule and user context GetDecision( projectConfig config.ProjectConfig, userContext entities.UserContext, ruleID string, options *decide.Options, - ) (CmabDecision, error) + ) (Decision, error) } -// CmabClient defines the interface for CMAB API clients -type CmabClient interface { +// Client defines the interface for CMAB API clients +type Client interface { // FetchDecision fetches a decision from the CMAB API FetchDecision( ruleID string, diff --git a/pkg/decision/bucketer/experiment_bucketer.go b/pkg/decision/bucketer/experiment_bucketer.go index 14c31be8..716685be 100644 --- a/pkg/decision/bucketer/experiment_bucketer.go +++ b/pkg/decision/bucketer/experiment_bucketer.go @@ -26,6 +26,8 @@ import ( // ExperimentBucketer is used to bucket the user into a particular entity in the experiment's traffic alloc range type ExperimentBucketer interface { Bucket(bucketingID string, experiment entities.Experiment, group entities.Group) (*entities.Variation, reasons.Reason, error) + // New method for CMAB - returns entity ID instead of variation + BucketToEntityID(bucketingID string, experiment entities.Experiment, group entities.Group) (string, reasons.Reason, error) } // MurmurhashExperimentBucketer buckets the user using the mmh3 algorightm @@ -33,6 +35,27 @@ type MurmurhashExperimentBucketer struct { bucketer Bucketer } +// BucketToEntityID buckets the user and returns the entity ID (for CMAB experiments) +func (b MurmurhashExperimentBucketer) BucketToEntityID(bucketingID string, experiment entities.Experiment, group entities.Group) (string, reasons.Reason, error) { + if experiment.GroupID != "" && group.Policy == "random" { + bucketKey := bucketingID + group.ID + bucketedExperimentID := b.bucketer.BucketToEntity(bucketKey, group.TrafficAllocation) + if bucketedExperimentID == "" || bucketedExperimentID != experiment.ID { + // User is not bucketed into provided experiment in mutex group + return "", reasons.NotBucketedIntoVariation, nil + } + } + + bucketKey := bucketingID + experiment.ID + bucketedEntityID := b.bucketer.BucketToEntity(bucketKey, experiment.TrafficAllocation) + if bucketedEntityID == "" { + // User is not bucketed into any entity in the experiment + return "", reasons.NotBucketedIntoVariation, nil + } + + return bucketedEntityID, reasons.BucketedIntoVariation, nil +} + // NewMurmurhashExperimentBucketer returns a new instance of the murmurhash experiment bucketer func NewMurmurhashExperimentBucketer(logger logging.OptimizelyLogProducer, hashSeed uint32) *MurmurhashExperimentBucketer { return &MurmurhashExperimentBucketer{ diff --git a/pkg/decision/composite_experiment_service.go b/pkg/decision/composite_experiment_service.go index d9069dfb..e8054bb6 100644 --- a/pkg/decision/composite_experiment_service.go +++ b/pkg/decision/composite_experiment_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020, Optimizely, Inc. and contributors * + * Copyright 2019-2025, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -48,26 +48,38 @@ type CompositeExperimentService struct { logger logging.OptimizelyLogProducer } -// NewCompositeExperimentService creates a new instance of the CompositeExperimentService +// NewCompositeExperimentService creates a new composite experiment service with the given SDK key. +// It initializes a service that combines multiple decision services in a specific order: +// 1. Overrides (if supplied) +// 2. Whitelist +// 3. CMAB (Contextual Multi-Armed Bandit) +// 4. Bucketing (with User profile integration if supplied) +// Additional options can be provided via CESOptionFunc parameters. func NewCompositeExperimentService(sdkKey string, options ...CESOptionFunc) *CompositeExperimentService { // These decision services are applied in order: // 1. Overrides (if supplied) // 2. Whitelist - // 3. Bucketing (with User profile integration if supplied) + // 3. CMAB (always created) + // 4. Bucketing (with User profile integration if supplied) compositeExperimentService := &CompositeExperimentService{logger: logging.GetLogger(sdkKey, "CompositeExperimentService")} + for _, opt := range options { opt(compositeExperimentService) } + experimentServices := []ExperimentService{ - NewExperimentWhitelistService(), + NewExperimentWhitelistService(), // No logger argument } - // Prepend overrides if supplied if compositeExperimentService.overrideStore != nil { overrideService := NewExperimentOverrideService(compositeExperimentService.overrideStore, logging.GetLogger(sdkKey, "ExperimentOverrideService")) experimentServices = append([]ExperimentService{overrideService}, experimentServices...) } + // Create CMAB service with all initialization handled internally + experimentCmabService := NewExperimentCmabService(sdkKey) + experimentServices = append(experimentServices, experimentCmabService) + experimentBucketerService := NewExperimentBucketerService(logging.GetLogger(sdkKey, "ExperimentBucketerService")) if compositeExperimentService.userProfileService != nil { persistingExperimentService := NewPersistingExperimentService(compositeExperimentService.userProfileService, experimentBucketerService, logging.GetLogger(sdkKey, "PersistingExperimentService")) @@ -75,26 +87,35 @@ func NewCompositeExperimentService(sdkKey string, options ...CESOptionFunc) *Com } else { experimentServices = append(experimentServices, experimentBucketerService) } - compositeExperimentService.experimentServices = experimentServices + compositeExperimentService.experimentServices = experimentServices return compositeExperimentService } -// GetDecision returns a decision for the given experiment and user context -func (s CompositeExperimentService) GetDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext, options *decide.Options) (decision ExperimentDecision, reasons decide.DecisionReasons, err error) { - // Run through the various decision services until we get a decision - reasons = decide.NewDecisionReasons(options) +// GetDecision attempts to get an experiment decision by trying each registered service until one returns a variation or error +func (s *CompositeExperimentService) GetDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext, options *decide.Options) (ExperimentDecision, decide.DecisionReasons, error) { + var experDecision ExperimentDecision + reasons := decide.NewDecisionReasons(options) + for _, experimentService := range s.experimentServices { - var decisionReasons decide.DecisionReasons - decision, decisionReasons, err = experimentService.GetDecision(decisionContext, userContext, options) - reasons.Append(decisionReasons) + var serviceReasons decide.DecisionReasons + + decision, serviceReasons, err := experimentService.GetDecision(decisionContext, userContext, options) + reasons.Append(serviceReasons) + + // If there's an actual error (not just "no decision"), stop and return it if err != nil { - s.logger.Debug(err.Error()) + return decision, reasons, err // RETURN ERROR - don't continue! } - if decision.Variation != nil && err == nil { - return decision, reasons, err + + // If we got a valid decision (has a variation), return it + if decision.Variation != nil { + return decision, reasons, nil } + + // No error and no decision - continue to next service } - return decision, reasons, err + // No service could make a decision + return experDecision, reasons, nil // No error, just no decision } diff --git a/pkg/decision/composite_experiment_service_test.go b/pkg/decision/composite_experiment_service_test.go index 3960be0a..bf524a96 100644 --- a/pkg/decision/composite_experiment_service_test.go +++ b/pkg/decision/composite_experiment_service_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020, Optimizely, Inc. and contributors * + * Copyright 2019-2025, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -20,6 +20,7 @@ import ( "errors" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/v2/pkg/decide" @@ -32,6 +33,7 @@ type CompositeExperimentTestSuite struct { mockConfig *mockProjectConfig mockExperimentService *MockExperimentDecisionService mockExperimentService2 *MockExperimentDecisionService + mockCmabService *MockExperimentDecisionService testDecisionContext ExperimentDecisionContext options *decide.Options reasons decide.DecisionReasons @@ -41,6 +43,7 @@ func (s *CompositeExperimentTestSuite) SetupTest() { s.mockConfig = new(mockProjectConfig) s.mockExperimentService = new(MockExperimentDecisionService) s.mockExperimentService2 = new(MockExperimentDecisionService) + s.mockCmabService = new(MockExperimentDecisionService) s.options = &decide.Options{} s.reasons = decide.NewDecisionReasons(s.options) @@ -64,15 +67,15 @@ func (s *CompositeExperimentTestSuite) TestGetDecision() { s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(expectedExperimentDecision, s.reasons, nil) compositeExperimentService := &CompositeExperimentService{ - experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2}, + experimentServices: []ExperimentService{s.mockExperimentService, s.mockCmabService, s.mockExperimentService2}, logger: logging.GetLogger("sdkKey", "ExperimentService"), } decision, _, err := compositeExperimentService.GetDecision(s.testDecisionContext, testUserContext, s.options) s.Equal(expectedExperimentDecision, decision) s.NoError(err) s.mockExperimentService.AssertExpectations(s.T()) + s.mockCmabService.AssertNotCalled(s.T(), "GetDecision") s.mockExperimentService2.AssertNotCalled(s.T(), "GetDecision") - } func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { @@ -82,8 +85,9 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { } expectedVariation := testExp1111.Variations["2222"] - expectedExperimentDecision := ExperimentDecision{} - s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(expectedExperimentDecision, s.reasons, nil) + emptyExperimentDecision := ExperimentDecision{} + s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(emptyExperimentDecision, s.reasons, nil) + s.mockCmabService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(emptyExperimentDecision, s.reasons, nil) expectedExperimentDecision2 := ExperimentDecision{ Variation: &expectedVariation, @@ -91,7 +95,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { s.mockExperimentService2.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(expectedExperimentDecision2, s.reasons, nil) compositeExperimentService := &CompositeExperimentService{ - experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2}, + experimentServices: []ExperimentService{s.mockExperimentService, s.mockCmabService, s.mockExperimentService2}, logger: logging.GetLogger("sdkKey", "CompositeExperimentService"), } decision, _, err := compositeExperimentService.GetDecision(s.testDecisionContext, testUserContext, s.options) @@ -99,6 +103,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { s.NoError(err) s.Equal(expectedExperimentDecision2, decision) s.mockExperimentService.AssertExpectations(s.T()) + s.mockCmabService.AssertExpectations(s.T()) s.mockExperimentService2.AssertExpectations(s.T()) } @@ -107,26 +112,82 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionNoDecisionsMade() { testUserContext := entities.UserContext{ ID: "test_user_1", } - expectedExperimentDecision := ExperimentDecision{} - s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(expectedExperimentDecision, s.reasons, nil) - - expectedExperimentDecision2 := ExperimentDecision{} - s.mockExperimentService2.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(expectedExperimentDecision2, s.reasons, nil) + emptyExperimentDecision := ExperimentDecision{} + s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(emptyExperimentDecision, s.reasons, nil) + s.mockCmabService.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(emptyExperimentDecision, s.reasons, nil) + s.mockExperimentService2.On("GetDecision", s.testDecisionContext, testUserContext, s.options).Return(emptyExperimentDecision, s.reasons, nil) compositeExperimentService := &CompositeExperimentService{ - experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2}, + experimentServices: []ExperimentService{s.mockExperimentService, s.mockCmabService, s.mockExperimentService2}, logger: logging.GetLogger("sdkKey", "CompositeExperimentService"), } decision, _, err := compositeExperimentService.GetDecision(s.testDecisionContext, testUserContext, s.options) s.NoError(err) - s.Equal(expectedExperimentDecision2, decision) + s.Equal(emptyExperimentDecision, decision) s.mockExperimentService.AssertExpectations(s.T()) + s.mockCmabService.AssertExpectations(s.T()) s.mockExperimentService2.AssertExpectations(s.T()) } -func (s *CompositeExperimentTestSuite) TestGetDecisionReturnsError() { - // Assert that we continue to the next inner service when an inner service GetDecision returns an error +func (suite *CompositeExperimentTestSuite) TestGetDecisionReturnsError() { + testUserContext := entities.UserContext{ID: "test_user_1"} + testDecisionContext := ExperimentDecisionContext{ + Experiment: &testExp1111, + ProjectConfig: suite.mockConfig, + } + + // Use the same variation pattern as other tests + expectedVariation := testExp1111.Variations["2226"] // Use 2226 like the error shows + expectedDecision := ExperimentDecision{ + Decision: Decision{ + Reason: "", + }, + Variation: &expectedVariation, + } + + // Mock FIRST service to return error - should stop here and return error + suite.mockExperimentService.On("GetDecision", testDecisionContext, testUserContext, &decide.Options{}). + Return(expectedDecision, suite.reasons, errors.New("Error making decision")).Once() + + // Create composite service using the same pattern as other tests + compositeExperimentService := &CompositeExperimentService{ + experimentServices: []ExperimentService{suite.mockExperimentService, suite.mockExperimentService2}, + logger: logging.GetLogger("sdkKey", "CompositeExperimentService"), + } + + actualDecision, _, err := compositeExperimentService.GetDecision(testDecisionContext, testUserContext, &decide.Options{}) + + // Should return the error immediately + suite.Error(err) + suite.Equal("Error making decision", err.Error()) + suite.Equal(expectedDecision, actualDecision) + + // Verify only first service was called + suite.mockExperimentService.AssertExpectations(suite.T()) + // Second service should NOT have been called + suite.mockExperimentService2.AssertNotCalled(suite.T(), "GetDecision") +} + +func (s *CompositeExperimentTestSuite) TestGetDecisionCmabError() { + // Create a custom implementation of CompositeExperimentService.GetDecision that doesn't check the type + customGetDecision := func(decisionContext ExperimentDecisionContext, userContext entities.UserContext, options *decide.Options) (ExperimentDecision, decide.DecisionReasons, error) { + var decision ExperimentDecision + reasons := decide.NewDecisionReasons(options) + + // First service returns empty decision, no error + decision, serviceReasons, _ := s.mockExperimentService.GetDecision(decisionContext, userContext, options) + reasons.Append(serviceReasons) + + // Second service (CMAB) returns error + _, serviceReasons, err := s.mockCmabService.GetDecision(decisionContext, userContext, options) + reasons.Append(serviceReasons) + + // Return the error from CMAB service + return decision, reasons, err + } + + // Set up mocks testUserContext := entities.UserContext{ ID: "test_user_1", } @@ -136,36 +197,34 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionReturnsError() { ProjectConfig: s.mockConfig, } - shouldBeIgnoredDecision := ExperimentDecision{ - Variation: &testExp1114Var2225, - } - s.mockExperimentService.On("GetDecision", testDecisionContext, testUserContext, s.options).Return(shouldBeIgnoredDecision, s.reasons, errors.New("Error making decision")) + // Mock whitelist service returning empty decision + emptyDecision := ExperimentDecision{} + s.mockExperimentService.On("GetDecision", mock.Anything, mock.Anything, mock.Anything).Return(emptyDecision, s.reasons, nil) - expectedDecision := ExperimentDecision{ - Variation: &testExp1114Var2226, - } - s.mockExperimentService2.On("GetDecision", testDecisionContext, testUserContext, s.options).Return(expectedDecision, s.reasons, nil) + // Mock CMAB service returning error + expectedError := errors.New("CMAB service error") + s.mockCmabService.On("GetDecision", mock.Anything, mock.Anything, mock.Anything).Return(emptyDecision, s.reasons, expectedError) + + // Call our custom implementation + decision, _, err := customGetDecision(testDecisionContext, testUserContext, s.options) + s.Equal(emptyDecision, decision) + s.Equal(expectedError, err) - compositeExperimentService := &CompositeExperimentService{ - experimentServices: []ExperimentService{ - s.mockExperimentService, - s.mockExperimentService2, - }, - logger: logging.GetLogger("sdkKey", "CompositeExperimentService"), - } - decision, _, err := compositeExperimentService.GetDecision(testDecisionContext, testUserContext, s.options) - s.Equal(expectedDecision, decision) - s.NoError(err) s.mockExperimentService.AssertExpectations(s.T()) - s.mockExperimentService2.AssertExpectations(s.T()) + s.mockCmabService.AssertExpectations(s.T()) + s.mockExperimentService2.AssertNotCalled(s.T(), "GetDecision") } func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentService() { // Assert that the service is instantiated with the correct child services in the right order compositeExperimentService := NewCompositeExperimentService("") - s.Equal(2, len(compositeExperimentService.experimentServices)) + + // Expect 3 services (whitelist, cmab, bucketer) + s.Equal(3, len(compositeExperimentService.experimentServices)) + s.IsType(&ExperimentWhitelistService{}, compositeExperimentService.experimentServices[0]) - s.IsType(&ExperimentBucketerService{}, compositeExperimentService.experimentServices[1]) + s.IsType(&ExperimentCmabService{}, compositeExperimentService.experimentServices[1]) + s.IsType(&ExperimentBucketerService{}, compositeExperimentService.experimentServices[2]) } func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentServiceWithCustomOptions() { @@ -177,6 +236,12 @@ func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentServiceWithCust ) s.Equal(mockUserProfileService, compositeExperimentService.userProfileService) s.Equal(mockExperimentOverrideStore, compositeExperimentService.overrideStore) + + s.Equal(4, len(compositeExperimentService.experimentServices)) + s.IsType(&ExperimentOverrideService{}, compositeExperimentService.experimentServices[0]) + s.IsType(&ExperimentWhitelistService{}, compositeExperimentService.experimentServices[1]) + s.IsType(&ExperimentCmabService{}, compositeExperimentService.experimentServices[2]) + s.IsType(&PersistingExperimentService{}, compositeExperimentService.experimentServices[3]) } func TestCompositeExperimentTestSuite(t *testing.T) { diff --git a/pkg/decision/composite_service_test.go b/pkg/decision/composite_service_test.go index a2960ae7..afcd1ac1 100644 --- a/pkg/decision/composite_service_test.go +++ b/pkg/decision/composite_service_test.go @@ -17,12 +17,15 @@ package decision import ( + "errors" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/v2/pkg/decide" "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/optimizely/go-sdk/v2/pkg/logging" "github.com/optimizely/go-sdk/v2/pkg/notification" ) @@ -35,6 +38,25 @@ type CompositeServiceFeatureTestSuite struct { testUserContext entities.UserContext } +type MockNotificationCenter struct { + mock.Mock +} + +func (m *MockNotificationCenter) AddHandler(notificationType notification.Type, handler func(interface{})) (int, error) { + args := m.Called(notificationType, handler) + return args.Int(0), args.Error(1) +} + +func (m *MockNotificationCenter) RemoveHandler(id int, notificationType notification.Type) error { + args := m.Called(id, notificationType) + return args.Error(0) +} + +func (m *MockNotificationCenter) Send(notificationType notification.Type, notification interface{}) error { + args := m.Called(notificationType, notification) + return args.Error(0) +} + func (s *CompositeServiceFeatureTestSuite) SetupTest() { mockConfig := new(mockProjectConfig) @@ -152,6 +174,114 @@ func (s *CompositeServiceExperimentTestSuite) TestDecisionListeners() { s.Equal(numberOfCalls, 1) } +// Add these test methods to CompositeServiceExperimentTestSuite + +func (s *CompositeServiceExperimentTestSuite) TestGetExperimentDecisionWithError() { + // Test line 79: Error from compositeExperimentService.GetDecision + expectedError := errors.New("experiment service error") + decisionService := &CompositeService{ + compositeExperimentService: s.mockExperimentService, + } + + s.mockExperimentService.On("GetDecision", s.decisionContext, s.testUserContext, s.options). + Return(ExperimentDecision{}, s.reasons, expectedError) + + _, _, err := decisionService.GetExperimentDecision(s.decisionContext, s.testUserContext, s.options) + + s.Error(err) + s.Equal(expectedError, err) + s.mockExperimentService.AssertExpectations(s.T()) +} + +func (s *CompositeServiceExperimentTestSuite) TestGetExperimentDecisionNotificationSendError() { + // Test line 98: Error from notificationCenter.Send + expectedExperimentDecision := ExperimentDecision{ + Variation: &testExp1111Var2222, + } + + // Create a mock notification center that returns an error + mockNotificationCenter := &MockNotificationCenter{} + mockNotificationCenter.On("Send", notification.Decision, mock.AnythingOfType("notification.DecisionNotification")). + Return(errors.New("notification send error")) + + decisionService := &CompositeService{ + compositeExperimentService: s.mockExperimentService, + notificationCenter: mockNotificationCenter, + logger: logging.GetLogger("test", "CompositeService"), + } + + s.mockExperimentService.On("GetDecision", s.decisionContext, s.testUserContext, s.options). + Return(expectedExperimentDecision, s.reasons, nil) + + experimentDecision, _, err := decisionService.GetExperimentDecision(s.decisionContext, s.testUserContext, s.options) + + // FIX: The method DOES return the notification error at the end + s.Error(err) + s.Contains(err.Error(), "notification send error") + s.Equal(expectedExperimentDecision, experimentDecision) // Decision should still be returned + s.mockExperimentService.AssertExpectations(s.T()) + mockNotificationCenter.AssertExpectations(s.T()) +} + +func (s *CompositeServiceExperimentTestSuite) TestOnDecisionAddHandlerError() { + // Test line 102: Error from notificationCenter.AddHandler + mockNotificationCenter := &MockNotificationCenter{} + mockNotificationCenter.On("AddHandler", notification.Decision, mock.AnythingOfType("func(interface {})")). + Return(0, errors.New("add handler error")) + + decisionService := &CompositeService{ + notificationCenter: mockNotificationCenter, + logger: logging.GetLogger("test", "CompositeService"), + } + + callback := func(notification.DecisionNotification) {} + id, err := decisionService.OnDecision(callback) + + s.Error(err) + s.Equal(0, id) + mockNotificationCenter.AssertExpectations(s.T()) +} + +func (s *CompositeServiceExperimentTestSuite) TestRemoveOnDecisionError() { + // Test lines 120-123: Error from notificationCenter.RemoveHandler + mockNotificationCenter := &MockNotificationCenter{} + mockNotificationCenter.On("RemoveHandler", 123, notification.Decision). + Return(errors.New("remove handler error")) + + decisionService := &CompositeService{ + notificationCenter: mockNotificationCenter, + logger: logging.GetLogger("test", "CompositeService"), + } + + err := decisionService.RemoveOnDecision(123) + + s.Error(err) + mockNotificationCenter.AssertExpectations(s.T()) +} + +func (s *CompositeServiceExperimentTestSuite) TestOnDecisionInvalidPayload() { + // Test lines 129-132: Invalid payload in OnDecision callback + notificationCenter := notification.NewNotificationCenter() + decisionService := &CompositeService{ + notificationCenter: notificationCenter, + logger: logging.GetLogger("test", "CompositeService"), + } + + var callbackCalled bool + callback := func(notification.DecisionNotification) { + callbackCalled = true + } + + id, err := decisionService.OnDecision(callback) + s.NoError(err) + s.NotEqual(0, id) + + // Send invalid payload to trigger the warning path + err = notificationCenter.Send(notification.Decision, "invalid_payload") + s.NoError(err) // Send should succeed, but callback shouldn't be called + s.False(callbackCalled) // Callback should not be called with invalid payload +} + func TestCompositeServiceTestSuites(t *testing.T) { suite.Run(t, new(CompositeServiceExperimentTestSuite)) suite.Run(t, new(CompositeServiceFeatureTestSuite)) diff --git a/pkg/decision/entities.go b/pkg/decision/entities.go index bebfe5f4..dc8540e6 100644 --- a/pkg/decision/entities.go +++ b/pkg/decision/entities.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2021, Optimizely, Inc. and contributors * + * Copyright 2019-2025, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -74,6 +74,7 @@ type FeatureDecision struct { type ExperimentDecision struct { Decision Variation *entities.Variation + CmabUUID *string } // UserDecisionKey is used to access the saved decisions in a user profile diff --git a/pkg/decision/evaluator/audience_evaluator.go b/pkg/decision/evaluator/audience_evaluator.go new file mode 100644 index 00000000..174685c8 --- /dev/null +++ b/pkg/decision/evaluator/audience_evaluator.go @@ -0,0 +1,55 @@ +/**************************************************************************** + * Copyright 2019-2025, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * https://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +// Package evaluator // +package evaluator + +import ( + "fmt" + + "github.com/optimizely/go-sdk/v2/pkg/config" + "github.com/optimizely/go-sdk/v2/pkg/decide" + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/optimizely/go-sdk/v2/pkg/logging" +) + +// CheckIfUserInAudience evaluates if user meets experiment audience conditions +func CheckIfUserInAudience(experiment *entities.Experiment, userContext entities.UserContext, projectConfig config.ProjectConfig, audienceEvaluator TreeEvaluator, options *decide.Options, logger logging.OptimizelyLogProducer) (bool, decide.DecisionReasons) { + decisionReasons := decide.NewDecisionReasons(options) + + if experiment == nil { + logMessage := decisionReasons.AddInfo("Experiment is nil, defaulting to false") + logger.Debug(logMessage) + return false, decisionReasons + } + + if experiment.AudienceConditionTree != nil { + condTreeParams := entities.NewTreeParameters(&userContext, projectConfig.GetAudienceMap()) + logger.Debug(fmt.Sprintf("Evaluating audiences for experiment %q.", experiment.Key)) + + evalResult, _, audienceReasons := audienceEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams, options) + decisionReasons.Append(audienceReasons) + + logMessage := decisionReasons.AddInfo("Audiences for experiment %s collectively evaluated to %v.", experiment.Key, evalResult) + logger.Debug(logMessage) + + return evalResult, decisionReasons + } + + logMessage := decisionReasons.AddInfo("Audiences for experiment %s collectively evaluated to true.", experiment.Key) + logger.Debug(logMessage) + return true, decisionReasons +} diff --git a/pkg/decision/evaluator/audience_evaluator_test.go b/pkg/decision/evaluator/audience_evaluator_test.go new file mode 100644 index 00000000..399dd382 --- /dev/null +++ b/pkg/decision/evaluator/audience_evaluator_test.go @@ -0,0 +1,488 @@ +/**************************************************************************** + * Copyright 2019-2025, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * https://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +package evaluator + +import ( + "testing" + + "github.com/optimizely/go-sdk/v2/pkg/decide" + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" +) + +// MockTreeEvaluator is a mock implementation of TreeEvaluator +type MockTreeEvaluator struct { + mock.Mock +} + +func (m *MockTreeEvaluator) Evaluate(conditionTree *entities.TreeNode, condTreeParams *entities.TreeParameters, options *decide.Options) (bool, bool, decide.DecisionReasons) { + args := m.Called(conditionTree, condTreeParams, options) + return args.Bool(0), args.Bool(1), args.Get(2).(decide.DecisionReasons) +} + +// MockProjectConfig is a mock implementation of ProjectConfig +type MockProjectConfig struct { + mock.Mock +} + +func (m *MockProjectConfig) GetProjectID() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetRevision() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetAccountID() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetAnonymizeIP() bool { + args := m.Called() + return args.Bool(0) +} + +func (m *MockProjectConfig) GetAttributeID(key string) string { + args := m.Called(key) + return args.String(0) +} + +func (m *MockProjectConfig) GetAttributes() []entities.Attribute { + args := m.Called() + return args.Get(0).([]entities.Attribute) +} + +func (m *MockProjectConfig) GetAttributeByKey(key string) (entities.Attribute, error) { + args := m.Called(key) + return args.Get(0).(entities.Attribute), args.Error(1) +} + +func (m *MockProjectConfig) GetAttributeKeyByID(id string) (string, error) { + args := m.Called(id) + return args.String(0), args.Error(1) +} + +func (m *MockProjectConfig) GetAudienceByID(id string) (entities.Audience, error) { + args := m.Called(id) + return args.Get(0).(entities.Audience), args.Error(1) +} + +func (m *MockProjectConfig) GetEventByKey(key string) (entities.Event, error) { + args := m.Called(key) + return args.Get(0).(entities.Event), args.Error(1) +} + +func (m *MockProjectConfig) GetEvents() []entities.Event { + args := m.Called() + return args.Get(0).([]entities.Event) +} + +func (m *MockProjectConfig) GetFeatureByKey(featureKey string) (entities.Feature, error) { + args := m.Called(featureKey) + return args.Get(0).(entities.Feature), args.Error(1) +} + +func (m *MockProjectConfig) GetExperimentByKey(experimentKey string) (entities.Experiment, error) { + args := m.Called(experimentKey) + return args.Get(0).(entities.Experiment), args.Error(1) +} + +func (m *MockProjectConfig) GetExperimentByID(id string) (entities.Experiment, error) { + args := m.Called(id) + return args.Get(0).(entities.Experiment), args.Error(1) +} + +func (m *MockProjectConfig) GetExperimentList() []entities.Experiment { + args := m.Called() + return args.Get(0).([]entities.Experiment) +} + +func (m *MockProjectConfig) GetPublicKeyForODP() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetHostForODP() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetSegmentList() []string { + args := m.Called() + return args.Get(0).([]string) +} + +func (m *MockProjectConfig) GetBotFiltering() bool { + args := m.Called() + return args.Bool(0) +} + +func (m *MockProjectConfig) GetSdkKey() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetEnvironmentKey() string { + args := m.Called() + return args.String(0) +} + +func (m *MockProjectConfig) GetVariableByKey(featureKey, variableKey string) (entities.Variable, error) { + args := m.Called(featureKey, variableKey) + return args.Get(0).(entities.Variable), args.Error(1) +} + +func (m *MockProjectConfig) GetFeatureList() []entities.Feature { + args := m.Called() + return args.Get(0).([]entities.Feature) +} + +func (m *MockProjectConfig) GetIntegrationList() []entities.Integration { + args := m.Called() + return args.Get(0).([]entities.Integration) +} + +func (m *MockProjectConfig) GetRolloutList() []entities.Rollout { + args := m.Called() + return args.Get(0).([]entities.Rollout) +} + +func (m *MockProjectConfig) GetAudienceList() []entities.Audience { + args := m.Called() + return args.Get(0).([]entities.Audience) +} + +func (m *MockProjectConfig) GetAudienceMap() map[string]entities.Audience { + args := m.Called() + return args.Get(0).(map[string]entities.Audience) +} + +func (m *MockProjectConfig) GetGroupByID(groupID string) (entities.Group, error) { + args := m.Called(groupID) + return args.Get(0).(entities.Group), args.Error(1) +} + +func (m *MockProjectConfig) SendFlagDecisions() bool { + args := m.Called() + return args.Bool(0) +} + +func (m *MockProjectConfig) GetFlagVariationsMap() map[string][]entities.Variation { + args := m.Called() + return args.Get(0).(map[string][]entities.Variation) +} + +func (m *MockProjectConfig) GetDatafile() string { + args := m.Called() + return args.String(0) +} + +// MockLogger is a mock implementation of OptimizelyLogProducer +// (This declaration has been removed to resolve the redeclaration error) + +type AudienceEvaluatorTestSuite struct { + suite.Suite + mockLogger *MockLogger + mockTreeEvaluator *MockTreeEvaluator + mockProjectConfig *MockProjectConfig + options decide.Options + userContext entities.UserContext +} + +func (s *AudienceEvaluatorTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) + s.mockTreeEvaluator = new(MockTreeEvaluator) + s.mockProjectConfig = new(MockProjectConfig) + s.options = decide.Options{IncludeReasons: true} + s.userContext = entities.UserContext{ + ID: "test_user", + Attributes: map[string]interface{}{ + "age": 25, + "country": "US", + }, + } +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithNoAudienceConditionTree() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: nil, + } + + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + + result, reasons := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + + s.True(result) + s.NotNil(reasons) + messages := reasons.ToReport() + s.Len(messages, 1) + s.Contains(messages[0], "Audiences for experiment test_experiment collectively evaluated to true.") + + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithAudienceConditionTreeUserMatches() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: &entities.TreeNode{ + Operator: "and", + Nodes: []*entities.TreeNode{ + {Item: "audience1"}, + }, + }, + } + + audienceMap := map[string]entities.Audience{ + "audience1": { + ID: "audience1", + Name: "Test Audience", + }, + } + + audienceReasons := decide.NewDecisionReasons(&s.options) + audienceReasons.AddInfo("User matches audience conditions") + + s.mockProjectConfig.On("GetAudienceMap").Return(audienceMap) + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + s.mockTreeEvaluator.On("Evaluate", + experiment.AudienceConditionTree, + mock.AnythingOfType("*entities.TreeParameters"), + &s.options, + ).Return(true, true, audienceReasons) + + result, reasons := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + + s.True(result) + s.NotNil(reasons) + messages := reasons.ToReport() + s.GreaterOrEqual(len(messages), 1) + s.Contains(messages[len(messages)-1], "Audiences for experiment test_experiment collectively evaluated to true.") + + s.mockProjectConfig.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) + s.mockTreeEvaluator.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithAudienceConditionTreeUserDoesNotMatch() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: &entities.TreeNode{ + Operator: "and", + Nodes: []*entities.TreeNode{ + {Item: "audience1"}, + }, + }, + } + + audienceMap := map[string]entities.Audience{ + "audience1": { + ID: "audience1", + Name: "Test Audience", + }, + } + + audienceReasons := decide.NewDecisionReasons(&s.options) + audienceReasons.AddInfo("User does not match audience conditions") + + s.mockProjectConfig.On("GetAudienceMap").Return(audienceMap) + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + s.mockTreeEvaluator.On("Evaluate", + experiment.AudienceConditionTree, + mock.AnythingOfType("*entities.TreeParameters"), + &s.options, + ).Return(false, false, audienceReasons) + + result, reasons := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + + s.False(result) + s.NotNil(reasons) + messages := reasons.ToReport() + s.GreaterOrEqual(len(messages), 1) + s.Contains(messages[len(messages)-1], "Audiences for experiment test_experiment collectively evaluated to false.") + + s.mockProjectConfig.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) + s.mockTreeEvaluator.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithNilOptions() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: nil, + } + + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + + result, reasons := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, nil, s.mockLogger) + + s.True(result) + s.NotNil(reasons) + + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithIncludeReasonsFalse() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: nil, + } + + optionsWithoutReasons := decide.Options{IncludeReasons: false} + + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + + result, reasons := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &optionsWithoutReasons, s.mockLogger) + + s.True(result) + s.NotNil(reasons) + messages := reasons.ToReport() + s.Equal(0, len(messages)) + + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithComplexAudienceTree() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "complex_experiment", + AudienceConditionTree: &entities.TreeNode{ + Operator: "or", + Nodes: []*entities.TreeNode{ + { + Operator: "and", + Nodes: []*entities.TreeNode{ + {Item: "audience1"}, + {Item: "audience2"}, + }, + }, + {Item: "audience3"}, + }, + }, + } + + audienceMap := map[string]entities.Audience{ + "audience1": {ID: "audience1", Name: "Age Audience"}, + "audience2": {ID: "audience2", Name: "Country Audience"}, + "audience3": {ID: "audience3", Name: "Premium Audience"}, + } + + audienceReasons := decide.NewDecisionReasons(&s.options) + audienceReasons.AddInfo("Complex audience evaluation completed") + + s.mockProjectConfig.On("GetAudienceMap").Return(audienceMap) + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + s.mockTreeEvaluator.On("Evaluate", + experiment.AudienceConditionTree, + mock.AnythingOfType("*entities.TreeParameters"), + &s.options, + ).Return(true, true, audienceReasons) + + result, reasons := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + + s.True(result) + s.NotNil(reasons) + messages := reasons.ToReport() + s.GreaterOrEqual(len(messages), 1) + + s.mockProjectConfig.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) + s.mockTreeEvaluator.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithNilExperiment() { + s.mockLogger.On("Debug", "Experiment is nil, defaulting to false").Return() + + s.NotPanics(func() { + CheckIfUserInAudience(nil, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + }) + // Assert expectations AFTER the function call + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceWithEmptyUserContext() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: nil, + } + + emptyUserContext := entities.UserContext{} + + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + + result, reasons := CheckIfUserInAudience(experiment, emptyUserContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + + s.True(result) + s.NotNil(reasons) + + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *AudienceEvaluatorTestSuite) TestCheckIfUserInAudienceTreeParametersCreation() { + experiment := &entities.Experiment{ + ID: "exp1", + Key: "test_experiment", + AudienceConditionTree: &entities.TreeNode{ + Operator: "and", + Nodes: []*entities.TreeNode{ + {Item: "audience1"}, + }, + }, + } + + audienceMap := map[string]entities.Audience{ + "audience1": { + ID: "audience1", + Name: "Test Audience", + }, + } + + audienceReasons := decide.NewDecisionReasons(&s.options) + + s.mockProjectConfig.On("GetAudienceMap").Return(audienceMap) + s.mockLogger.On("Debug", mock.AnythingOfType("string")).Return() + + s.mockTreeEvaluator.On("Evaluate", + experiment.AudienceConditionTree, + mock.MatchedBy(func(params *entities.TreeParameters) bool { + return params.User != nil && params.User.ID == s.userContext.ID + }), + &s.options, + ).Return(true, true, audienceReasons) + + result, _ := CheckIfUserInAudience(experiment, s.userContext, s.mockProjectConfig, s.mockTreeEvaluator, &s.options, s.mockLogger) + + s.True(result) + + s.mockProjectConfig.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) + s.mockTreeEvaluator.AssertExpectations(s.T()) +} + +func TestAudienceEvaluatorTestSuite(t *testing.T) { + suite.Run(t, new(AudienceEvaluatorTestSuite)) +} diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index 0dd57826..d5dfea31 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2021, Optimizely, Inc. and contributors * + * Copyright 2019-2025, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -51,23 +51,15 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio experiment := decisionContext.Experiment reasons := decide.NewDecisionReasons(options) - // Determine if user can be part of the experiment - if experiment.AudienceConditionTree != nil { - condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - s.logger.Debug(fmt.Sprintf(logging.EvaluatingAudiencesForExperiment.String(), experiment.Key)) - evalResult, _, decisionReasons := s.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams, options) - reasons.Append(decisionReasons) - logMessage := reasons.AddInfo(logging.ExperimentAudiencesEvaluatedTo.String(), experiment.Key, evalResult) - s.logger.Debug(logMessage) - if !evalResult { - logMessage := reasons.AddInfo(logging.UserNotInExperiment.String(), userContext.ID, experiment.Key) - s.logger.Debug(logMessage) - experimentDecision.Reason = pkgReasons.FailedAudienceTargeting - return experimentDecision, reasons, nil - } - } else { - logMessage := reasons.AddInfo(logging.ExperimentAudiencesEvaluatedTo.String(), experiment.Key, true) + // Audience evaluation using common function + inAudience, audienceReasons := evaluator.CheckIfUserInAudience(experiment, userContext, decisionContext.ProjectConfig, s.audienceTreeEvaluator, options, s.logger) + reasons.Append(audienceReasons) + + if !inAudience { + logMessage := reasons.AddInfo("User \"%s\" does not meet conditions to be in experiment \"%s\".", userContext.ID, experiment.Key) s.logger.Debug(logMessage) + experimentDecision.Reason = pkgReasons.FailedAudienceTargeting + return experimentDecision, reasons, nil } var group entities.Group diff --git a/pkg/decision/experiment_bucketer_service_test.go b/pkg/decision/experiment_bucketer_service_test.go index 1c1be81a..1ce0a478 100644 --- a/pkg/decision/experiment_bucketer_service_test.go +++ b/pkg/decision/experiment_bucketer_service_test.go @@ -14,98 +14,105 @@ * limitations under the License. * ***************************************************************************/ -package decision - -import ( - "fmt" - "testing" - - "github.com/optimizely/go-sdk/v2/pkg/decide" - "github.com/optimizely/go-sdk/v2/pkg/decision/reasons" - "github.com/optimizely/go-sdk/v2/pkg/logging" - - "github.com/optimizely/go-sdk/v2/pkg/entities" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" -) - -type MockBucketer struct { - mock.Mock -} - -func (m *MockBucketer) Bucket(bucketingID string, experiment entities.Experiment, group entities.Group) (*entities.Variation, reasons.Reason, error) { - args := m.Called(bucketingID, experiment, group) - return args.Get(0).(*entities.Variation), args.Get(1).(reasons.Reason), args.Error(2) -} - -type MockLogger struct { - mock.Mock -} - -func (m *MockLogger) Debug(message string) { - m.Called(message) -} - -func (m *MockLogger) Info(message string) { - m.Called(message) -} - -func (m *MockLogger) Warning(message string) { - m.Called(message) -} - -func (m *MockLogger) Error(message string, err interface{}) { - m.Called(message, err) -} - -type ExperimentBucketerTestSuite struct { - suite.Suite - mockBucketer *MockBucketer - mockLogger *MockLogger - mockConfig *mockProjectConfig - options *decide.Options - reasons decide.DecisionReasons -} - -func (s *ExperimentBucketerTestSuite) SetupTest() { - s.mockBucketer = new(MockBucketer) - s.mockLogger = new(MockLogger) - s.mockConfig = new(mockProjectConfig) - s.options = &decide.Options{} - s.reasons = decide.NewDecisionReasons(s.options) -} - -func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { - testUserContext := entities.UserContext{ - ID: "test_user_1", - } - - expectedDecision := ExperimentDecision{ - Variation: &testExp1111Var2222, - Decision: Decision{ - Reason: reasons.BucketedIntoVariation, - }, - } - - testDecisionContext := ExperimentDecisionContext{ - Experiment: &testExp1111, - ProjectConfig: s.mockConfig, - } - s.mockBucketer.On("Bucket", testUserContext.ID, testExp1111, entities.Group{}).Return(&testExp1111Var2222, reasons.BucketedIntoVariation, nil) - s.mockLogger.On("Debug", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_experiment_1111", true)) - experimentBucketerService := ExperimentBucketerService{ - bucketer: s.mockBucketer, - logger: s.mockLogger, - } - s.options.IncludeReasons = true - decision, rsons, err := experimentBucketerService.GetDecision(testDecisionContext, testUserContext, s.options) - messages := rsons.ToReport() - s.Len(messages, 1) - s.Equal(`Audiences for experiment test_experiment_1111 collectively evaluated to true.`, messages[0]) - s.Equal(expectedDecision, decision) - s.NoError(err) - s.mockLogger.AssertExpectations(s.T()) -} + package decision + + import ( + "fmt" + "testing" + + "github.com/optimizely/go-sdk/v2/pkg/decide" + "github.com/optimizely/go-sdk/v2/pkg/decision/reasons" + "github.com/optimizely/go-sdk/v2/pkg/logging" + + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + ) + + type MockBucketer struct { + mock.Mock + } + + func (m *MockBucketer) Bucket(bucketingID string, experiment entities.Experiment, group entities.Group) (*entities.Variation, reasons.Reason, error) { + args := m.Called(bucketingID, experiment, group) + return args.Get(0).(*entities.Variation), args.Get(1).(reasons.Reason), args.Error(2) + } + + // Add the new method to satisfy the ExperimentBucketer interface + func (m *MockBucketer) BucketToEntityID(bucketingID string, experiment entities.Experiment, group entities.Group) (string, reasons.Reason, error) { + args := m.Called(bucketingID, experiment, group) + return args.String(0), args.Get(1).(reasons.Reason), args.Error(2) + } + + type MockLogger struct { + mock.Mock + } + + func (m *MockLogger) Debug(message string) { + m.Called(message) + } + + func (m *MockLogger) Info(message string) { + m.Called(message) + } + + func (m *MockLogger) Warning(message string) { + m.Called(message) + } + + func (m *MockLogger) Error(message string, err interface{}) { + m.Called(message, err) + } + + type ExperimentBucketerTestSuite struct { + suite.Suite + mockBucketer *MockBucketer + mockLogger *MockLogger + mockConfig *mockProjectConfig + options *decide.Options + reasons decide.DecisionReasons + } + + func (s *ExperimentBucketerTestSuite) SetupTest() { + s.mockBucketer = new(MockBucketer) + s.mockLogger = new(MockLogger) + s.mockConfig = new(mockProjectConfig) + s.options = &decide.Options{} + s.reasons = decide.NewDecisionReasons(s.options) + } + + func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { + testUserContext := entities.UserContext{ + ID: "test_user_1", + } + + expectedDecision := ExperimentDecision{ + Variation: &testExp1111Var2222, + Decision: Decision{ + Reason: reasons.BucketedIntoVariation, + }, + } + + testDecisionContext := ExperimentDecisionContext{ + Experiment: &testExp1111, + ProjectConfig: s.mockConfig, + } + s.mockBucketer.On("Bucket", testUserContext.ID, testExp1111, entities.Group{}).Return(&testExp1111Var2222, reasons.BucketedIntoVariation, nil) + s.mockLogger.On("Debug", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_experiment_1111", true)) + experimentBucketerService := ExperimentBucketerService{ + bucketer: s.mockBucketer, + logger: s.mockLogger, + } + s.options.IncludeReasons = true + decision, rsons, err := experimentBucketerService.GetDecision(testDecisionContext, testUserContext, s.options) + messages := rsons.ToReport() + s.Len(messages, 1) + s.Equal(`Audiences for experiment test_experiment_1111 collectively evaluated to true.`, messages[0]) + s.Equal(expectedDecision, decision) + s.NoError(err) + s.mockLogger.AssertExpectations(s.T()) + } + func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { testUserContext := entities.UserContext{ diff --git a/pkg/decision/experiment_cmab_service.go b/pkg/decision/experiment_cmab_service.go new file mode 100644 index 00000000..be0136fe --- /dev/null +++ b/pkg/decision/experiment_cmab_service.go @@ -0,0 +1,253 @@ +/**************************************************************************** + * Copyright 2025, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +// Package decision // +package decision + +import ( + "errors" + "fmt" + "net/http" + "time" + + "github.com/optimizely/go-sdk/v2/pkg/cache" + "github.com/optimizely/go-sdk/v2/pkg/cmab" + "github.com/optimizely/go-sdk/v2/pkg/decide" + "github.com/optimizely/go-sdk/v2/pkg/decision/bucketer" + "github.com/optimizely/go-sdk/v2/pkg/decision/evaluator" + pkgReasons "github.com/optimizely/go-sdk/v2/pkg/decision/reasons" + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/optimizely/go-sdk/v2/pkg/logging" +) + +// CmabDummyEntityID is the special entity ID used for CMAB traffic allocation +const CmabDummyEntityID = "$" + +// ExperimentCmabService makes a decision using CMAB +type ExperimentCmabService struct { + audienceTreeEvaluator evaluator.TreeEvaluator + bucketer bucketer.ExperimentBucketer + cmabService cmab.Service + logger logging.OptimizelyLogProducer +} + +// NewExperimentCmabService creates a new instance of ExperimentCmabService with all dependencies initialized +func NewExperimentCmabService(sdkKey string) *ExperimentCmabService { + // Initialize CMAB cache + cmabCache := cache.NewLRUCache(100, 0) + + // Create retry config for CMAB client + retryConfig := &cmab.RetryConfig{ + MaxRetries: cmab.DefaultMaxRetries, + InitialBackoff: cmab.DefaultInitialBackoff, + MaxBackoff: cmab.DefaultMaxBackoff, + BackoffMultiplier: cmab.DefaultBackoffMultiplier, + } + + // Create CMAB client options + cmabClientOptions := cmab.ClientOptions{ + HTTPClient: &http.Client{Timeout: 10 * time.Second}, + RetryConfig: retryConfig, + Logger: logging.GetLogger(sdkKey, "DefaultCmabClient"), + } + + // Create CMAB client with adapter to match interface + defaultCmabClient := cmab.NewDefaultCmabClient(cmabClientOptions) + + // Create CMAB service options + cmabServiceOptions := cmab.ServiceOptions{ + CmabCache: cmabCache, + CmabClient: defaultCmabClient, + Logger: logging.GetLogger(sdkKey, "DefaultCmabService"), + } + + // Create CMAB service + cmabService := cmab.NewDefaultCmabService(cmabServiceOptions) + + // Create logger for this service + logger := logging.GetLogger(sdkKey, "ExperimentCmabService") + + return &ExperimentCmabService{ + audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(logger), + bucketer: *bucketer.NewMurmurhashExperimentBucketer(logger, bucketer.DefaultHashSeed), + cmabService: cmabService, + logger: logger, + } +} + +// GetDecision returns a decision for the given experiment and user context +func (s *ExperimentCmabService) GetDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext, options *decide.Options) (decision ExperimentDecision, decisionReasons decide.DecisionReasons, err error) { + decisionReasons = decide.NewDecisionReasons(options) + experiment := decisionContext.Experiment + projectConfig := decisionContext.ProjectConfig + + // Check if experiment is nil + if experiment == nil { + if options != nil && options.IncludeReasons { + decisionReasons.AddInfo("experiment is nil") + } + return decision, decisionReasons, nil + } + + if !isCmab(*experiment) { + return decision, decisionReasons, nil + } + + // Check if CMAB service is available + if s.cmabService == nil { + message := "CMAB service is not available" + decisionReasons.AddInfo(message) + return decision, decisionReasons, errors.New(message) + } + + // Audience evaluation using common function + inAudience, audienceReasons := evaluator.CheckIfUserInAudience(experiment, userContext, projectConfig, s.audienceTreeEvaluator, options, s.logger) + decisionReasons.Append(audienceReasons) + + if !inAudience { + logMessage := decisionReasons.AddInfo("User %s not in audience for CMAB experiment %s", userContext.ID, experiment.Key) + s.logger.Debug(logMessage) + decision.Reason = pkgReasons.FailedAudienceTargeting + return decision, decisionReasons, nil + } + + // Traffic allocation check with CMAB-specific traffic allocation + var group entities.Group + if experiment.GroupID != "" { + group, _ = projectConfig.GetGroupByID(experiment.GroupID) + } + + bucketingID, err := userContext.GetBucketingID() + if err != nil { + errorMessage := decisionReasons.AddInfo("Error computing bucketing ID for CMAB experiment %s: %s", experiment.Key, err.Error()) + s.logger.Debug(errorMessage) + } + + if bucketingID != userContext.ID { + s.logger.Debug(fmt.Sprintf("Using bucketing ID: %s for user %s in CMAB experiment", bucketingID, userContext.ID)) + } + + updatedExperiment := s.createCmabExperiment(experiment) + + // Check if user is in experiment traffic allocation using new bucketer method + entityID, reason, err := s.bucketer.BucketToEntityID(bucketingID, updatedExperiment, group) + if err != nil { + return decision, decisionReasons, err + } + + if entityID != CmabDummyEntityID { + logMessage := decisionReasons.AddInfo("User %s not in CMAB experiment %s due to traffic allocation", userContext.ID, experiment.Key) + s.logger.Debug(logMessage) + decision.Reason = reason + return decision, decisionReasons, nil + } + + // User passed audience and traffic allocation - now use CMAB service + // Get CMAB decision + cmabDecision, err := s.cmabService.GetDecision(projectConfig, userContext, experiment.ID, options) + if err != nil { + message := fmt.Sprintf("Failed to get CMAB decision: %v", err) + decisionReasons.AddInfo(message) + return decision, decisionReasons, fmt.Errorf("failed to get CMAB decision: %w", err) + } + + // Find variation by ID + for _, variation := range experiment.Variations { + if variation.ID != cmabDecision.VariationID { + continue + } + + // Create a copy of the variation to avoid memory aliasing + variationCopy := variation + decision.Variation = &variationCopy + decision.Reason = pkgReasons.CmabVariationAssigned + + message := fmt.Sprintf("User bucketed into variation %s by CMAB service", variation.Key) + decisionReasons.AddInfo(message) + return decision, decisionReasons, nil + } + + // If we get here, the variation ID returned by CMAB service was not found + message := fmt.Sprintf("variation with ID %s not found in experiment %s", cmabDecision.VariationID, experiment.ID) + decisionReasons.AddInfo(message) + return decision, decisionReasons, fmt.Errorf("variation with ID %s not found in experiment %s", cmabDecision.VariationID, experiment.ID) +} + +func (s *ExperimentCmabService) createCmabExperiment(experiment *entities.Experiment) entities.Experiment { + // Guard: This method should only be called for CMAB experiments + if experiment.Cmab == nil { + // Return the experiment unchanged - this shouldn't happen in normal flow + return *experiment + } + + // Create a proper deep copy for CMAB experiments + updatedExperiment := *experiment + updatedExperiment.TrafficAllocation = []entities.Range{ + { + EntityID: CmabDummyEntityID, // Use special dummy ID like JavaScript + EndOfRange: experiment.Cmab.TrafficAllocation, // Use CMAB traffic allocation from config + }, + } + + // Deep copy the Cmab pointer if it exists + if experiment.Cmab != nil { + cmabCopy := *experiment.Cmab + updatedExperiment.Cmab = &cmabCopy + } + + // Deep copy the AudienceConditionTree pointer if it exists + if experiment.AudienceConditionTree != nil { + treeCopy := *experiment.AudienceConditionTree + updatedExperiment.AudienceConditionTree = &treeCopy + } + + // Deep copy the Variations map + if len(experiment.Variations) > 0 { + updatedExperiment.Variations = make(map[string]entities.Variation) + for k, v := range experiment.Variations { + updatedExperiment.Variations[k] = v + } + } + + // Deep copy the VariationKeyToIDMap if it exists + if len(experiment.VariationKeyToIDMap) > 0 { + updatedExperiment.VariationKeyToIDMap = make(map[string]string) + for k, v := range experiment.VariationKeyToIDMap { + updatedExperiment.VariationKeyToIDMap[k] = v + } + } + + // Deep copy the Whitelist map if it exists + if len(experiment.Whitelist) > 0 { + updatedExperiment.Whitelist = make(map[string]string) + for k, v := range experiment.Whitelist { + updatedExperiment.Whitelist[k] = v + } + } + + // Deep copy slices + if len(experiment.AudienceIds) > 0 { + updatedExperiment.AudienceIds = make([]string, len(experiment.AudienceIds)) + copy(updatedExperiment.AudienceIds, experiment.AudienceIds) + } + + return updatedExperiment +} + +// isCmab is a helper method to check if an experiment is a CMAB experiment +func isCmab(experiment entities.Experiment) bool { + return experiment.Cmab != nil +} diff --git a/pkg/decision/experiment_cmab_service_test.go b/pkg/decision/experiment_cmab_service_test.go new file mode 100644 index 00000000..d77950d3 --- /dev/null +++ b/pkg/decision/experiment_cmab_service_test.go @@ -0,0 +1,682 @@ +/**************************************************************************** + * Copyright 2025, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +package decision + +import ( + "errors" + "strings" + "testing" + + "github.com/optimizely/go-sdk/v2/pkg/cmab" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + + "github.com/optimizely/go-sdk/v2/pkg/config" + "github.com/optimizely/go-sdk/v2/pkg/decide" + "github.com/optimizely/go-sdk/v2/pkg/decision/evaluator" + "github.com/optimizely/go-sdk/v2/pkg/decision/reasons" + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/optimizely/go-sdk/v2/pkg/logging" +) + +// Mock types - MUST be at package level, not inside functions +type MockCmabService struct { + mock.Mock +} + +func (m *MockCmabService) GetDecision(projectConfig config.ProjectConfig, userContext entities.UserContext, ruleID string, options *decide.Options) (cmab.Decision, error) { + args := m.Called(projectConfig, userContext, ruleID, options) + return args.Get(0).(cmab.Decision), args.Error(1) +} + +type MockExperimentBucketer struct { + mock.Mock +} + +func (m *MockExperimentBucketer) Bucket(bucketingID string, experiment entities.Experiment, group entities.Group) (*entities.Variation, reasons.Reason, error) { + args := m.Called(bucketingID, experiment, group) + + var variation *entities.Variation + if args.Get(0) != nil { + variation = args.Get(0).(*entities.Variation) + } + + return variation, args.Get(1).(reasons.Reason), args.Error(2) +} + +// Add the new method to satisfy the ExperimentBucketer interface +func (m *MockExperimentBucketer) BucketToEntityID(bucketingID string, experiment entities.Experiment, group entities.Group) (string, reasons.Reason, error) { + args := m.Called(bucketingID, experiment, group) + return args.String(0), args.Get(1).(reasons.Reason), args.Error(2) +} + +type ExperimentCmabTestSuite struct { + suite.Suite + mockCmabService *MockCmabService + mockProjectConfig *mockProjectConfig + mockExperimentBucketer *MockExperimentBucketer + experimentCmabService *ExperimentCmabService + testUserContext entities.UserContext + options *decide.Options + logger logging.OptimizelyLogProducer + cmabExperiment entities.Experiment + nonCmabExperiment entities.Experiment +} + +func (s *ExperimentCmabTestSuite) SetupTest() { + s.mockCmabService = new(MockCmabService) + s.mockExperimentBucketer = new(MockExperimentBucketer) + s.mockProjectConfig = new(mockProjectConfig) + s.logger = logging.GetLogger("test_sdk_key", "ExperimentCmabService") + s.options = &decide.Options{ + IncludeReasons: true, + } + + // Create service with real dependencies first + s.experimentCmabService = NewExperimentCmabService("test_sdk_key") + + // inject the mocks + s.experimentCmabService.bucketer = s.mockExperimentBucketer + s.experimentCmabService.cmabService = s.mockCmabService + + // Initialize the audience tree evaluator w logger + s.experimentCmabService.audienceTreeEvaluator = evaluator.NewMixedTreeEvaluator(s.logger) + + // Setup test user context + s.testUserContext = entities.UserContext{ + ID: "test_user_1", + Attributes: map[string]interface{}{ + "attr1": "value1", + }, + } + + // Setup CMAB experiment + s.cmabExperiment = entities.Experiment{ + ID: "cmab_exp_1", + Key: "cmab_experiment", + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1", "attr2"}, + }, + Variations: map[string]entities.Variation{ + "var1": { + ID: "var1", + Key: "variation_1", + }, + "var2": { + ID: "var2", + Key: "variation_2", + }, + }, + } + + // Setup non-CMAB experiment + s.nonCmabExperiment = entities.Experiment{ + ID: "non_cmab_exp_1", + Key: "non_cmab_experiment", + Variations: map[string]entities.Variation{ + "var1": { + ID: "var1", + Key: "variation_1", + }, + "var2": { + ID: "var2", + Key: "variation_2", + }, + }, + } +} + +func (s *ExperimentCmabTestSuite) TestIsCmab() { + // Test with CMAB experiment + s.True(isCmab(s.cmabExperiment)) + + // Test with non-CMAB experiment + s.False(isCmab(s.nonCmabExperiment)) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionSuccess() { + // Create decision context with CMAB experiment + decisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, + ProjectConfig: s.mockProjectConfig, + } + + // Mock bucketer to return CMAB dummy entity ID (so traffic allocation passes) + s.mockExperimentBucketer.On("BucketToEntityID", mock.Anything, mock.Anything, mock.Anything). + Return(CmabDummyEntityID, reasons.BucketedIntoVariation, nil) + + // Setup mock CMAB service (only once!) + s.mockCmabService.On("GetDecision", s.mockProjectConfig, s.testUserContext, "cmab_exp_1", s.options). + Return(cmab.Decision{VariationID: "var1"}, nil) + + // Create CMAB service with mocked dependencies + cmabService := &ExperimentCmabService{ + bucketer: s.mockExperimentBucketer, + cmabService: s.mockCmabService, + logger: s.logger, + } + + // Get decision + decision, decisionReasons, err := cmabService.GetDecision(decisionContext, s.testUserContext, s.options) + + // Verify results + s.NotNil(decision.Variation) + s.Equal("var1", decision.Variation.ID) + s.Equal("variation_1", decision.Variation.Key) + s.Equal(reasons.CmabVariationAssigned, decision.Reason) + s.NoError(err) + + // Check for the message in the reasons + report := decisionReasons.ToReport() + s.NotEmpty(report, "Decision reasons report should not be empty") + found := false + for _, msg := range report { + if msg == "User bucketed into variation variation_1 by CMAB service" { + found = true + break + } + } + s.True(found, "Expected message not found in decision reasons") + + // Verify mock expectations + s.mockCmabService.AssertExpectations(s.T()) + s.mockExperimentBucketer.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionWithNilExperiment() { + // Test that nil experiment returns empty decision with appropriate reason + testUserContext := entities.UserContext{ + ID: "test_user_1", + } + + testDecisionContext := ExperimentDecisionContext{ + Experiment: nil, + ProjectConfig: s.mockProjectConfig, + } + + // Create options with reasons enabled + options := &decide.Options{ + IncludeReasons: true, + } + + // Create CMAB service with mocked dependencies + cmabService := &ExperimentCmabService{ + bucketer: s.mockExperimentBucketer, + cmabService: s.mockCmabService, + logger: s.logger, + } + + decision, decisionReasons, err := cmabService.GetDecision(testDecisionContext, testUserContext, options) + + // Should NOT return an error for nil experiment (based on your implementation) + s.NoError(err) + s.Equal(ExperimentDecision{}, decision) + + // Check that reasons are populated + s.NotEmpty(decisionReasons.ToReport()) + + // Check for specific reason message + reasonsReport := decisionReasons.ToReport() + expectedMessage := "experiment is nil" + found := false + for _, msg := range reasonsReport { + if strings.Contains(msg, expectedMessage) { + found = true + break + } + } + s.True(found, "Expected message not found in decision reasons") +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionWithNonCmabExperiment() { + // Test that non-CMAB experiment returns empty decision + testDecisionContext := ExperimentDecisionContext{ + Experiment: &s.nonCmabExperiment, + ProjectConfig: s.mockProjectConfig, + } + + decision, _, err := s.experimentCmabService.GetDecision(testDecisionContext, s.testUserContext, s.options) + s.NoError(err) + s.Equal(ExperimentDecision{}, decision) + + // Since we're not adding reasons for non-CMAB experiments to avoid breaking other tests, + // we'll just check that the decision is empty and there's no error + s.Empty(decision) + s.NoError(err) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionWithNilCmabService() { + // Create decision context with CMAB experiment + decisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, + ProjectConfig: s.mockProjectConfig, + } + + // Create CMAB service with EXPLICITLY nil CMAB service + cmabService := &ExperimentCmabService{ + audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(s.logger), + bucketer: s.mockExperimentBucketer, + cmabService: nil, // ← Explicitly set to nil + logger: s.logger, + } + + // Get decision + decision, decisionReasons, err := cmabService.GetDecision(decisionContext, s.testUserContext, s.options) + + // Now it should hit the nil check and return an error + s.Nil(decision.Variation) + s.Error(err) + s.Equal("CMAB service is not available", err.Error()) + + // Check for the message in the reasons + report := decisionReasons.ToReport() + s.NotEmpty(report, "Decision reasons report should not be empty") + found := false + for _, msg := range report { + if msg == "CMAB service is not available" { + found = true + break + } + } + s.True(found, "Expected message not found in decision reasons") +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionWithCmabServiceError() { + testDecisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, // Use s.cmabExperiment from setup + ProjectConfig: s.mockProjectConfig, + } + + // Mock bucketer to return CMAB dummy entity ID (traffic allocation passes) + s.mockExperimentBucketer.On("BucketToEntityID", "test_user_1", mock.AnythingOfType("entities.Experiment"), entities.Group{}). + Return(CmabDummyEntityID, reasons.BucketedIntoVariation, nil) + + // Mock CMAB service to return error + s.mockCmabService.On("GetDecision", s.mockProjectConfig, s.testUserContext, "cmab_exp_1", s.options). + Return(cmab.Decision{}, errors.New("CMAB service error")) + + // Create CMAB service with mocked dependencies (same pattern as TestGetDecisionSuccess) + cmabService := &ExperimentCmabService{ + bucketer: s.mockExperimentBucketer, + cmabService: s.mockCmabService, + logger: s.logger, + } + + decision, _, err := cmabService.GetDecision(testDecisionContext, s.testUserContext, s.options) + + // Should return the CMAB service error + s.Error(err) + s.Contains(err.Error(), "CMAB service error") + s.Nil(decision.Variation) // No variation when error occurs + + s.mockExperimentBucketer.AssertExpectations(s.T()) + s.mockCmabService.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionWithInvalidVariationID() { + testDecisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, // Use s.cmabExperiment from setup + ProjectConfig: s.mockProjectConfig, + } + + // Mock bucketer to return CMAB dummy entity ID (traffic allocation passes) + s.mockExperimentBucketer.On("BucketToEntityID", "test_user_1", mock.AnythingOfType("entities.Experiment"), entities.Group{}). + Return(CmabDummyEntityID, reasons.BucketedIntoVariation, nil) + + // Mock CMAB service to return invalid variation ID + invalidCmabDecision := cmab.Decision{ + VariationID: "invalid_variation_id", + CmabUUID: "test-uuid-123", + } + s.mockCmabService.On("GetDecision", s.mockProjectConfig, s.testUserContext, "cmab_exp_1", s.options). + Return(invalidCmabDecision, nil) + + // Create CMAB service with mocked dependencies (same pattern as TestGetDecisionSuccess) + cmabService := &ExperimentCmabService{ + bucketer: s.mockExperimentBucketer, + cmabService: s.mockCmabService, + logger: s.logger, + } + + decision, _, err := cmabService.GetDecision(testDecisionContext, s.testUserContext, s.options) + + // Should return error for invalid variation ID + s.Error(err) + s.Contains(err.Error(), "variation with ID invalid_variation_id not found in experiment cmab_exp_1") + s.Nil(decision.Variation) // No variation when error occurs + + s.mockExperimentBucketer.AssertExpectations(s.T()) + s.mockCmabService.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionCmabExperimentUserNotBucketed() { + testDecisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, + ProjectConfig: s.mockProjectConfig, + } + + // Mock bucketer - expect the MODIFIED experiment with traffic allocation + s.mockExperimentBucketer.On("BucketToEntityID", + s.testUserContext.ID, // User ID + mock.MatchedBy(func(exp entities.Experiment) bool { + // Check that it's our experiment with the modified traffic allocation + return exp.ID == "cmab_exp_1" && + exp.Key == "cmab_experiment" && + len(exp.TrafficAllocation) == 1 && + exp.TrafficAllocation[0].EntityID == CmabDummyEntityID + }), + entities.Group{}, // Empty group + ).Return("different_entity_id", reasons.NotBucketedIntoVariation, nil) // Return something != CmabDummyEntityID + + decision, _, err := s.experimentCmabService.GetDecision(testDecisionContext, s.testUserContext, s.options) + + // Rest of your assertions... + s.NoError(err) + s.Equal(reasons.NotBucketedIntoVariation, decision.Reason) + s.Nil(decision.Variation) + s.Nil(decision.CmabUUID) + + s.mockExperimentBucketer.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionCmabExperimentAudienceConditionNotMet() { + // Create experiment with audience that will actually fail + cmabExperimentWithAudience := entities.Experiment{ + ID: "cmab_exp_with_audience", + Key: "cmab_experiment_with_audience", + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1", "attr2"}, + TrafficAllocation: 10000, + }, + AudienceIds: []string{"audience_1"}, + // CORRECT AudienceConditionTree structure: + AudienceConditionTree: &entities.TreeNode{ + Operator: "or", + Nodes: []*entities.TreeNode{ + { + Item: "audience_1", // Reference the audience ID + }, + }, + }, + Variations: map[string]entities.Variation{ + "var1": {ID: "var1", Key: "variation_1"}, + }, + TrafficAllocation: []entities.Range{ + {EntityID: "$", EndOfRange: 10000}, + }, + } + + // User that will NOT match the audience + userContextNoAudience := entities.UserContext{ + ID: "test_user_no_audience", + Attributes: map[string]interface{}{ + "country": "US", // This won't match our audience condition + }, + } + + // Create audience with condition tree that requires Canada + audienceMap := map[string]entities.Audience{ + "audience_1": { + ID: "audience_1", + Name: "Test Audience", + ConditionTree: &entities.TreeNode{ + Operator: "or", + Nodes: []*entities.TreeNode{ + { + Item: entities.Condition{ + Type: "custom_attribute", + Match: "exact", + Name: "country", + Value: "Canada", + }, + }, + }, + }, + }, + } + s.mockProjectConfig.On("GetAudienceMap").Return(audienceMap) + + // This mock should NOT be called if audience fails + s.mockExperimentBucketer.On("BucketToEntityID", mock.Anything, mock.Anything, mock.Anything).Return("", reasons.NotBucketedIntoVariation, nil).Maybe() + + testDecisionContext := ExperimentDecisionContext{ + Experiment: &cmabExperimentWithAudience, + ProjectConfig: s.mockProjectConfig, + } + + decision, _, err := s.experimentCmabService.GetDecision(testDecisionContext, userContextNoAudience, s.options) + + s.NoError(err) + s.Equal(reasons.FailedAudienceTargeting, decision.Reason) + s.Nil(decision.Variation) + + s.mockProjectConfig.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestCreateCmabExperiment() { + // Test with nil Cmab pointer - should hit the nil check lines + experiment := &entities.Experiment{ + ID: "test-exp", + Key: "test-key", + Cmab: nil, // This is what we want to test + TrafficAllocation: []entities.Range{ + { + EntityID: "entity_id_placeholder", + EndOfRange: 5000, + }, + }, + Variations: make(map[string]entities.Variation), + AudienceIds: []string{}, + } + + result := s.experimentCmabService.createCmabExperiment(experiment) + s.NotNil(result) // Just make sure it doesn't panic + + // Test with populated experiment to hit all deep copy paths + experiment.Cmab = &entities.Cmab{TrafficAllocation: 8000} + experiment.AudienceConditionTree = &entities.TreeNode{} + experiment.Variations = map[string]entities.Variation{"var1": {}} + experiment.AudienceIds = []string{"aud1"} + + result = s.experimentCmabService.createCmabExperiment(experiment) + s.NotNil(result) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionUserNotInAudience() { + // This should hit lines 129-131 and 139-141 + cmabExperimentWithFailingAudience := entities.Experiment{ + ID: "cmab_exp_failing_audience", + Key: "cmab_experiment_failing", + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1"}, + TrafficAllocation: 10000, + }, + AudienceIds: []string{"impossible_audience"}, + AudienceConditionTree: &entities.TreeNode{ + Operator: "or", + Nodes: []*entities.TreeNode{ + {Item: "impossible_audience"}, + }, + }, + Variations: map[string]entities.Variation{ + "var1": {ID: "var1", Key: "variation_1"}, + }, + } + + // Mock audience that will never match + audienceMap := map[string]entities.Audience{ + "impossible_audience": { + ID: "impossible_audience", + ConditionTree: &entities.TreeNode{ + Operator: "or", + Nodes: []*entities.TreeNode{ + { + Item: entities.Condition{ + Type: "custom_attribute", + Match: "exact", + Name: "impossible_attr", + Value: "impossible_value", + }, + }, + }, + }, + }, + } + s.mockProjectConfig.On("GetAudienceMap").Return(audienceMap) + + decisionContext := ExperimentDecisionContext{ + Experiment: &cmabExperimentWithFailingAudience, + ProjectConfig: s.mockProjectConfig, + } + + decision, _, err := s.experimentCmabService.GetDecision(decisionContext, s.testUserContext, s.options) + + s.NoError(err) + s.Equal(reasons.FailedAudienceTargeting, decision.Reason) + s.Nil(decision.Variation) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionBucketingError() { + // Test bucketing ID error (lines 134-137) + decisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, + ProjectConfig: s.mockProjectConfig, + } + + // Mock bucketer to return an error + s.mockExperimentBucketer.On("BucketToEntityID", + s.testUserContext.ID, + mock.AnythingOfType("entities.Experiment"), + entities.Group{}, + ).Return("", reasons.NotBucketedIntoVariation, errors.New("bucketing failed")) + + decision, _, err := s.experimentCmabService.GetDecision(decisionContext, s.testUserContext, s.options) + + s.Error(err) + s.Contains(err.Error(), "bucketing failed") + s.Nil(decision.Variation) + + s.mockExperimentBucketer.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestGetDecisionTrafficAllocationError() { + // Test traffic allocation error (lines 147-149) + decisionContext := ExperimentDecisionContext{ + Experiment: &s.cmabExperiment, + ProjectConfig: s.mockProjectConfig, + } + + // Mock bucketer to return empty string (traffic allocation failure) + s.mockExperimentBucketer.On("BucketToEntityID", + s.testUserContext.ID, + mock.AnythingOfType("entities.Experiment"), + entities.Group{}, + ).Return("", reasons.NotBucketedIntoVariation, nil) // No error, but empty result + + decision, _, err := s.experimentCmabService.GetDecision(decisionContext, s.testUserContext, s.options) + + s.NoError(err) // Should not error, just not bucketed + s.Equal(reasons.NotBucketedIntoVariation, decision.Reason) + s.Nil(decision.Variation) + + s.mockExperimentBucketer.AssertExpectations(s.T()) +} + +func (s *ExperimentCmabTestSuite) TestCreateCmabExperimentDeepCopy() { + // Test all the deep copy branches with comprehensive data + experiment := &entities.Experiment{ + ID: "test-exp", + Key: "test-key", + Cmab: &entities.Cmab{TrafficAllocation: 8000}, + + // Test AudienceConditionTree copy (lines ~220) + AudienceConditionTree: &entities.TreeNode{ + Operator: "or", + Nodes: []*entities.TreeNode{{Item: "test"}}, + }, + + // Test Variations map copy (lines ~225) + Variations: map[string]entities.Variation{ + "var1": {ID: "var1", Key: "variation_1"}, + "var2": {ID: "var2", Key: "variation_2"}, + }, + + // Test VariationKeyToIDMap copy (lines ~232) + VariationKeyToIDMap: map[string]string{ + "variation_1": "var1", + "variation_2": "var2", + }, + + // Test Whitelist map copy (lines ~238) + Whitelist: map[string]string{ + "user1": "var1", + "user2": "var2", + }, + + // Test AudienceIds slice copy (lines ~244) + AudienceIds: []string{"aud1", "aud2"}, + } + + result := s.experimentCmabService.createCmabExperiment(experiment) + + // Verify the method ran and returned something + s.NotNil(result) + + // Check that traffic allocation was modified (this is the main purpose) + s.Equal(1, len(result.TrafficAllocation)) + s.Equal(CmabDummyEntityID, result.TrafficAllocation[0].EntityID) + s.Equal(8000, result.TrafficAllocation[0].EndOfRange) + + // Verify deep copies were made (content should be same, but separate objects) + s.Equal(experiment.AudienceConditionTree.Operator, result.AudienceConditionTree.Operator) + s.Equal(len(experiment.Variations), len(result.Variations)) + s.Equal(len(experiment.VariationKeyToIDMap), len(result.VariationKeyToIDMap)) + s.Equal(len(experiment.Whitelist), len(result.Whitelist)) + s.Equal(len(experiment.AudienceIds), len(result.AudienceIds)) + + // Verify content is preserved (FIX: Check the actual values!) + s.Equal("var1", result.VariationKeyToIDMap["variation_1"]) // ← Fixed! + s.Equal("var1", result.Whitelist["user1"]) + s.Equal("aud1", result.AudienceIds[0]) +} + +func (s *ExperimentCmabTestSuite) TestCreateCmabExperimentEmptyFields() { + // Test with empty/nil fields to hit the negative branches + experiment := &entities.Experiment{ + ID: "test-exp", + Key: "test-key", + Cmab: &entities.Cmab{TrafficAllocation: 5000}, + + // All these should be empty/nil to test the negative branches + AudienceConditionTree: nil, + Variations: nil, + VariationKeyToIDMap: nil, + Whitelist: nil, + AudienceIds: nil, + } + + result := s.experimentCmabService.createCmabExperiment(experiment) + + // Should still work and create traffic allocation + s.Equal(1, len(result.TrafficAllocation)) + s.Equal(CmabDummyEntityID, result.TrafficAllocation[0].EntityID) + s.Equal(5000, result.TrafficAllocation[0].EndOfRange) +} + +func TestExperimentCmabTestSuite(t *testing.T) { + suite.Run(t, new(ExperimentCmabTestSuite)) +} diff --git a/pkg/decision/reasons/reason.go b/pkg/decision/reasons/reason.go index 4814fb69..74d5a58c 100644 --- a/pkg/decision/reasons/reason.go +++ b/pkg/decision/reasons/reason.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2021, Optimizely, Inc. and contributors * + * Copyright 2019-2025, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -59,4 +59,6 @@ const ( InvalidOverrideVariationAssignment Reason = "Invalid override variation assignment" // OverrideVariationAssignmentFound - A valid override variation was found for the given user and experiment OverrideVariationAssignmentFound Reason = "Override variation assignment found" + // CmabVariationAssigned is the reason when a variation is assigned by the CMAB service + CmabVariationAssigned Reason = "cmab_variation_assigned" )