From 8fb9933d5ce1c47ee0217b1275850ca181d35668 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 15 May 2025 06:27:30 -0700 Subject: [PATCH 1/4] Move CMAB functionality from pkg/decision to pkg/cmab and update event factory for CMAB impression events --- .../cmab_client.go => cmab/client.go} | 125 ++++++- .../client_test.go} | 4 +- .../cmab_service.go => cmab/service.go} | 19 +- .../service_test.go} | 309 +++++++++++++++++- pkg/{decision/cmab.go => cmab/types.go} | 14 +- pkg/event/events.go | 8 +- pkg/event/factory.go | 65 +++- pkg/event/factory_test.go | 103 +++++- 8 files changed, 625 insertions(+), 22 deletions(-) rename pkg/{decision/cmab_client.go => cmab/client.go} (73%) rename pkg/{decision/cmab_client_test.go => cmab/client_test.go} (99%) rename pkg/{decision/cmab_service.go => cmab/service.go} (95%) rename pkg/{decision/cmab_service_test.go => cmab/service_test.go} (70%) rename pkg/{decision/cmab.go => cmab/types.go} (88%) diff --git a/pkg/decision/cmab_client.go b/pkg/cmab/client.go similarity index 73% rename from pkg/decision/cmab_client.go rename to pkg/cmab/client.go index 49cc51f0..0be7b9ee 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 CMAB client implementation +package cmab import ( "bytes" @@ -27,6 +27,9 @@ import ( "net/http" "time" + "github.com/optimizely/go-sdk/v2/pkg/config" + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/optimizely/go-sdk/v2/pkg/event" "github.com/optimizely/go-sdk/v2/pkg/logging" ) @@ -88,16 +91,20 @@ type RetryConfig struct { // DefaultCmabClient implements the CmabClient interface type DefaultCmabClient struct { - httpClient *http.Client - retryConfig *RetryConfig - logger logging.OptimizelyLogProducer + httpClient *http.Client + retryConfig *RetryConfig + logger logging.OptimizelyLogProducer + eventProcessor event.Processor + projectConfig config.ProjectConfig } // CmabClientOptions defines options for creating a CMAB client type CmabClientOptions struct { - HTTPClient *http.Client - RetryConfig *RetryConfig - Logger logging.OptimizelyLogProducer + HTTPClient *http.Client + RetryConfig *RetryConfig + Logger logging.OptimizelyLogProducer + EventProcessor event.Processor + ProjectConfig config.ProjectConfig } // NewDefaultCmabClient creates a new instance of DefaultCmabClient @@ -119,9 +126,11 @@ func NewDefaultCmabClient(options CmabClientOptions) *DefaultCmabClient { } return &DefaultCmabClient{ - httpClient: httpClient, - retryConfig: retryConfig, - logger: logger, + httpClient: httpClient, + retryConfig: retryConfig, + logger: logger, + eventProcessor: options.EventProcessor, + projectConfig: options.ProjectConfig, } } @@ -266,3 +275,97 @@ func (c *DefaultCmabClient) doFetch(ctx context.Context, url string, bodyBytes [ func (c *DefaultCmabClient) validateResponse(response CMABResponse) bool { return len(response.Predictions) > 0 && response.Predictions[0].VariationID != "" } + +// EventProcessor is an interface for processing events +type EventProcessor interface { + Process(userEvent interface{}) bool +} + +// LogImpression logs a CMAB impression event +func (c *DefaultCmabClient) LogImpression( + ctx context.Context, + eventProcessor EventProcessor, + projectConfig config.ProjectConfig, + ruleID string, + userID string, + variationKey string, + variationID string, + attributes map[string]interface{}, + cmabUUID string, +) error { + // If no context is provided, create a background context + if ctx == nil { + ctx = context.Background() + } + + // Instead of directly creating an event, we'll delegate this to the client + // that has access to the event package + return fmt.Errorf("CMAB impression logging not implemented") +} + +// TrackCMABDecision tracks a CMAB decision event +func (c *DefaultCmabClient) TrackCMABDecision( + ruleID string, + userID string, + variationID string, + variationKey string, + attributes map[string]interface{}, + cmabUUID string, +) { + if c.eventProcessor == nil || c.projectConfig == nil { + c.logger.Debug("Event processor or project config not available, not tracking impression") + return + } + + // Get experiment from project config + experiment, err := c.projectConfig.GetExperimentByID(ruleID) + if err != nil { + c.logger.Error("Error getting experiment", err) + return + } + + // Create variation object + variation := entities.Variation{ + ID: variationID, + Key: variationKey, + } + + // Create user context + userContext := entities.UserContext{ + ID: userID, + Attributes: attributes, + } + + // Look for associated feature flag (if any) + flagKey := "" + featureList := c.projectConfig.GetFeatureList() + for _, feature := range featureList { + for _, featureExperiment := range feature.FeatureExperiments { + if featureExperiment.ID == ruleID { + flagKey = feature.Key + break + } + } + if flagKey != "" { + break + } + } + + // Create user event with CMAB impression + userEvent, shouldDispatch := event.CreateCMABImpressionUserEvent( + c.projectConfig, + experiment, + &variation, + userContext, + flagKey, + experiment.Key, // ruleKey + "experiment", // ruleType + true, + cmabUUID, + ) + + // Process the event if it should be dispatched + if shouldDispatch { + c.eventProcessor.ProcessEvent(userEvent) + } +} diff --git a/pkg/decision/cmab_client_test.go b/pkg/cmab/client_test.go similarity index 99% rename from pkg/decision/cmab_client_test.go rename to pkg/cmab/client_test.go index a84a6b20..6a72e822 100644 --- a/pkg/decision/cmab_client_test.go +++ b/pkg/cmab/client_test.go @@ -14,8 +14,8 @@ * limitations under the License. * ***************************************************************************/ -// Package decision // -package decision +// Package cmab // +package cmab import ( "context" diff --git a/pkg/decision/cmab_service.go b/pkg/cmab/service.go similarity index 95% rename from pkg/decision/cmab_service.go rename to pkg/cmab/service.go index 995170ba..02ff7aa6 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 provides CMAB decision service implementation +package cmab import ( "encoding/json" @@ -186,6 +186,21 @@ 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)) + + // Get variation key from variation ID + // This might require additional logic depending on your implementation + variationKey := variationID // Simplified - you may need to look up the key + + // Track the decision event + s.cmabClient.TrackCMABDecision( + ruleID, + userID, + variationID, + variationKey, + attributes, + cmabUUID, + ) + return CmabDecision{ VariationID: variationID, CmabUUID: cmabUUID, diff --git a/pkg/decision/cmab_service_test.go b/pkg/cmab/service_test.go similarity index 70% rename from pkg/decision/cmab_service_test.go rename to pkg/cmab/service_test.go index 08333ef1..2804eeb0 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" @@ -285,6 +285,15 @@ func (s *CmabServiceTestSuite) TestGetDecision() { expectedVariationID := "variant-1" s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + mock.Anything, // variationID + mock.Anything, // variationKey + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + // Setup cache save s.mockCache.On("Save", cacheKey, mock.Anything).Return() @@ -374,6 +383,15 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithIgnoreCache() { expectedVariationID := "variant-1" s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + mock.Anything, // variationID + mock.Anything, // variationKey + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + // Test with IgnoreCMABCache option options := &decide.Options{ IgnoreCMABCache: true, @@ -426,6 +444,15 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithResetCache() { expectedVariationID := "variant-1" s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + mock.Anything, // variationID + mock.Anything, // variationKey + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + // Setup cache save s.mockCache.On("Save", cacheKey, mock.Anything).Return() @@ -475,6 +502,15 @@ func (s *CmabServiceTestSuite) TestGetDecisionWithInvalidateUserCache() { expectedVariationID := "variant-1" s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + mock.Anything, // variationID + mock.Anything, // variationKey + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + // Setup cache save s.mockCache.On("Save", cacheKey, mock.Anything).Return() @@ -617,6 +653,16 @@ func (s *CmabServiceTestSuite) TestOnlyFilteredAttributesPassedToClient() { return true }), mock.Anything).Return(expectedVariationID, nil) + // set up track + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + mock.Anything, // variationID + mock.Anything, // variationKey + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + // Setup cache save s.mockCache.On("Save", cacheKey, mock.Anything).Return() @@ -675,6 +721,15 @@ func (s *CmabServiceTestSuite) TestCacheInvalidatedWhenAttributesChange() { expectedVariationID := "new-variant" s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + expectedVariationID, // variationID + expectedVariationID, // variationKey (assuming it's the same as ID in your test) + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + // Setup cache save for the new decision s.mockCache.On("Save", cacheKey, mock.Anything).Return() @@ -696,6 +751,15 @@ func (s *CmabServiceTestSuite) TestCacheInvalidatedWhenAttributesChange() { s.mockCache.AssertCalled(s.T(), "Save", cacheKey, mock.MatchedBy(func(value CmabCacheValue) bool { return value.VariationID == expectedVariationID && value.AttributesHash != oldAttributesHash })) + + // Verify TrackCMABDecision was called + s.mockClient.AssertCalled(s.T(), "TrackCMABDecision", + s.testRuleID, + s.testUserID, + expectedVariationID, + expectedVariationID, + mock.Anything, + mock.Anything) } func (s *CmabServiceTestSuite) TestGetAttributesJSON() { @@ -734,3 +798,244 @@ func (s *CmabServiceTestSuite) TestNewDefaultCmabService() { func TestCmabServiceTestSuite(t *testing.T) { suite.Run(t, new(CmabServiceTestSuite)) } + +func (m *MockCmabClient) TrackCMABDecision( + ruleID string, + userID string, + variationID string, + variationKey string, + attributes map[string]interface{}, + cmabUUID string, +) { + m.Called(ruleID, userID, variationID, variationKey, attributes, cmabUUID) +} + +// Test that TrackCMABDecision is called when a decision is successfully fetched +func (s *CmabServiceTestSuite) TestTrackCMABDecisionOnSuccessfulFetch() { + // Setup mock experiment with CMAB configuration + experiment := entities.Experiment{ + ID: s.testRuleID, + Key: "experiment-key", // Add experiment key + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1", "attr2"}, + }, + } + + // 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) + + // Create user context + userContext := entities.UserContext{ + ID: s.testUserID, + Attributes: s.testAttributes, + } + + // Setup cache key + cacheKey := s.cmabService.getCacheKey(s.testUserID, s.testRuleID) + + // Setup cache lookup - return nil to simulate cache miss + s.mockCache.On("Lookup", cacheKey).Return(nil) + + // Setup mock API response + expectedVariationID := "variant-1" + s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + + // Setup tracking expectation + s.mockClient.On("TrackCMABDecision", + s.testRuleID, // ruleID + s.testUserID, // userID + mock.Anything, // variationID + mock.Anything, // variationKey + mock.Anything, // attributes + mock.Anything, // cmabUUID + ).Return(nil) + + // Setup cache save + s.mockCache.On("Save", cacheKey, mock.Anything).Return() + + // Test with no options + decision, err := s.cmabService.GetDecision(s.mockConfig, userContext, s.testRuleID, nil) + s.NoError(err) + s.Equal(expectedVariationID, decision.VariationID) + s.NotEmpty(decision.CmabUUID) + + // Verify tracking was called + s.mockClient.AssertCalled(s.T(), "TrackCMABDecision", + s.testRuleID, + s.testUserID, + expectedVariationID, + expectedVariationID, // Simplified - in real implementation this might be different + mock.Anything, + mock.Anything) +} + +// Test that TrackCMABDecision is not called when there's an error fetching a decision +func (s *CmabServiceTestSuite) TestNoTrackingOnFetchError() { + // Setup mock experiment with CMAB configuration + experiment := entities.Experiment{ + ID: s.testRuleID, + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1", "attr2"}, + }, + } + + // 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) + + // Create user context + userContext := entities.UserContext{ + ID: s.testUserID, + Attributes: s.testAttributes, + } + + // Setup cache key + cacheKey := s.cmabService.getCacheKey(s.testUserID, s.testRuleID) + + // Setup cache miss + s.mockCache.On("Lookup", cacheKey).Return(nil) + + // Setup mock API error + expectedError := errors.New("API error") + s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return("", expectedError) + + // Test error handling + decision, err := s.cmabService.GetDecision(s.mockConfig, userContext, s.testRuleID, nil) + s.Error(err) + s.Equal("", decision.VariationID) // Should be empty + + // Verify tracking was NOT called + s.mockClient.AssertNotCalled(s.T(), "TrackCMABDecision") +} + +// Test that TrackCMABDecision is called with the correct parameters +func (s *CmabServiceTestSuite) TestTrackCMABDecisionParameters() { + // Setup mock experiment with CMAB configuration + experiment := entities.Experiment{ + ID: s.testRuleID, + Key: "experiment-key", + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1", "attr2"}, + }, + } + + // 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) + + // Create user context + userContext := entities.UserContext{ + ID: s.testUserID, + Attributes: s.testAttributes, + } + + // Setup cache key + cacheKey := s.cmabService.getCacheKey(s.testUserID, s.testRuleID) + + // Setup cache lookup - return nil to simulate cache miss + s.mockCache.On("Lookup", cacheKey).Return(nil) + + // Setup mock API response + expectedVariationID := "variant-1" + s.mockClient.On("FetchDecision", s.testRuleID, s.testUserID, mock.Anything, mock.Anything).Return(expectedVariationID, nil) + + // Setup tracking expectation with parameter verification + s.mockClient.On("TrackCMABDecision", + s.testRuleID, + s.testUserID, + expectedVariationID, + expectedVariationID, // Simplified - in real implementation this might be different + mock.MatchedBy(func(attrs map[string]interface{}) bool { + // Verify filtered attributes are passed to tracking + if len(attrs) != 2 { + return false + } + if attrs["age"] != 30 { + return false + } + if attrs["location"] != "San Francisco" { + return false + } + return true + }), + mock.MatchedBy(func(uuid string) bool { + // Verify UUID is not empty + return uuid != "" + })).Return() + + // Setup cache save + s.mockCache.On("Save", cacheKey, mock.Anything).Return() + + // Test with no options + decision, err := s.cmabService.GetDecision(s.mockConfig, userContext, s.testRuleID, nil) + s.NoError(err) + s.Equal(expectedVariationID, decision.VariationID) + s.NotEmpty(decision.CmabUUID) + + // Verify tracking was called with correct parameters + s.mockClient.AssertCalled(s.T(), "TrackCMABDecision", + s.testRuleID, + s.testUserID, + expectedVariationID, + expectedVariationID, // Simplified - in real implementation this might be different + mock.MatchedBy(func(attrs map[string]interface{}) bool { + return attrs["age"] == 30 && attrs["location"] == "San Francisco" + }), + decision.CmabUUID) // Should match the UUID in the decision +} + +// Test that TrackCMABDecision is not called when using cached decisions +func (s *CmabServiceTestSuite) TestNoTrackingOnCachedDecisions() { + // Setup mock experiment with CMAB configuration + experiment := entities.Experiment{ + ID: s.testRuleID, + Cmab: &entities.Cmab{ + AttributeIds: []string{"attr1", "attr2"}, + }, + } + + // 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) + + // Create user context + userContext := entities.UserContext{ + ID: s.testUserID, + Attributes: s.testAttributes, + } + + // Setup cache key + cacheKey := s.cmabService.getCacheKey(s.testUserID, s.testRuleID) + + // Calculate attributes hash using murmur3 as in your implementation + attributesJSON, _ := s.cmabService.getAttributesJSON(s.testAttributes) + hasher := murmur3.SeedNew32(1) + hasher.Write([]byte(attributesJSON)) + attributesHash := strconv.FormatUint(uint64(hasher.Sum32()), 10) + + // Setup cache hit with matching attributes hash + cachedValue := CmabCacheValue{ + AttributesHash: attributesHash, + VariationID: "cached-variant", + CmabUUID: "cached-uuid", + } + s.mockCache.On("Lookup", cacheKey).Return(cachedValue) + + // Test with cache hit + decision, err := s.cmabService.GetDecision(s.mockConfig, userContext, s.testRuleID, nil) + s.NoError(err) + s.Equal("cached-variant", decision.VariationID) + s.Equal("cached-uuid", decision.CmabUUID) + + // Verify API was not called + s.mockClient.AssertNotCalled(s.T(), "FetchDecision") + + // Verify tracking was NOT called for cached decisions + // This is a design decision - you might want to track cached decisions in some implementations + s.mockClient.AssertNotCalled(s.T(), "TrackCMABDecision") +} diff --git a/pkg/decision/cmab.go b/pkg/cmab/types.go similarity index 88% rename from pkg/decision/cmab.go rename to pkg/cmab/types.go index 97d4ec81..4b30ead2 100644 --- a/pkg/decision/cmab.go +++ b/pkg/cmab/types.go @@ -14,8 +14,8 @@ * limitations under the License. * ***************************************************************************/ -// Package decision provides CMAB decision service interfaces and types -package decision +// Package cmab provides CMAB decision service interfaces and types +package cmab import ( "github.com/optimizely/go-sdk/v2/pkg/config" @@ -57,4 +57,14 @@ type CmabClient interface { attributes map[string]interface{}, cmabUUID string, ) (string, error) + + // TrackCMABDecision tracks a CMAB decision event + TrackCMABDecision( + ruleID string, + userID string, + variationID string, + variationKey string, + attributes map[string]interface{}, + cmabUUID string, + ) } diff --git a/pkg/event/events.go b/pkg/event/events.go index 88fd8e82..f6eeeea1 100644 --- a/pkg/event/events.go +++ b/pkg/event/events.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2020, Optimizely, Inc. and contributors * + * 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. * @@ -127,3 +127,9 @@ type SnapshotEvent struct { Revenue *int64 `json:"revenue,omitempty"` Value *float64 `json:"value,omitempty"` } + +// CMABImpressionEvent represents an impression event for CMAB decisions +type CMABImpressionEvent struct { + ImpressionEvent + CmabUUID string `json:"cmab_uuid"` +} diff --git a/pkg/event/factory.go b/pkg/event/factory.go index c1536971..e50fb7c8 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022 Optimizely, Inc. and contributors * + * Copyright 2019-2020,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. * @@ -133,6 +133,69 @@ func createImpressionVisitor(userEvent UserEvent) Visitor { return visitor } +// CreateCMABImpressionEvent creates a CMAB impression event +func CreateCMABImpressionEvent( + configObj config.ProjectConfig, + experiment entities.Experiment, + variation *entities.Variation, + attributes map[string]interface{}, + flagKey, ruleKey, ruleType string, + enabled bool, + cmabUUID string, +) CMABImpressionEvent { + baseImpression := createImpressionEvent( + configObj, + experiment, + variation, + attributes, + flagKey, + ruleKey, + ruleType, + enabled, + ) + + return CMABImpressionEvent{ + ImpressionEvent: baseImpression, + CmabUUID: cmabUUID, + } +} + +// CreateCMABImpressionUserEvent creates a UserEvent containing a CMAB impression +func CreateCMABImpressionUserEvent( + projectConfig config.ProjectConfig, + experiment entities.Experiment, + variation *entities.Variation, + userContext entities.UserContext, + flagKey, ruleKey, ruleType string, + enabled bool, + cmabUUID string, +) (UserEvent, bool) { + if (ruleType == decisionPkg.Rollout || variation == nil) && !projectConfig.SendFlagDecisions() { + return UserEvent{}, false + } + + impression := CreateCMABImpressionEvent( + projectConfig, + experiment, + variation, + userContext.Attributes, + flagKey, + ruleKey, + ruleType, + enabled, + cmabUUID, + ) + + userEvent := UserEvent{} + userEvent.Timestamp = makeTimestamp() + userEvent.VisitorID = userContext.ID + userEvent.UUID = cmabUUID // Use the CMAB UUID instead of generating a new one + userEvent.Impression = &impression.ImpressionEvent // Use the embedded ImpressionEvent + userEvent.EventContext = CreateEventContext(projectConfig) + + return userEvent, true +} + // create a conversion event func createConversionEvent(projectConfig config.ProjectConfig, event entities.Event, attributes, eventTags map[string]interface{}) ConversionEvent { conversion := ConversionEvent{} diff --git a/pkg/event/factory_test.go b/pkg/event/factory_test.go index ea4ed82d..44a1aa3f 100644 --- a/pkg/event/factory_test.go +++ b/pkg/event/factory_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019,2022 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. * @@ -19,6 +19,7 @@ package event import ( "context" + "fmt" "math/rand" "testing" "time" @@ -241,3 +242,103 @@ func TestCreateImpressionUserEvent(t *testing.T) { } } } + +func TestCreateCMABImpressionUserEvent(t *testing.T) { + // Setup + config := TestConfig{} + + experiment := entities.Experiment{ + ID: "exp123", + Key: "exp_key", + } + + variation := entities.Variation{ + ID: "var123", + Key: "var_key", + } + + userContext := entities.UserContext{ + ID: "user123", + Attributes: map[string]interface{}{ + "attr1": "value1", + }, + } + + flagKey := "flag_key" + ruleKey := "rule_key" + ruleType := "experiment" + enabled := true + cmabUUID := "uuid123" + + // Call function + userEvent, shouldDispatch := CreateCMABImpressionUserEvent( + config, + experiment, + &variation, + userContext, + flagKey, + ruleKey, + ruleType, + enabled, + cmabUUID, + ) + + // Assertions + assert.True(t, shouldDispatch) + + // Check that it's an impression event (Impression field is set) + assert.NotNil(t, userEvent.Impression) + assert.Nil(t, userEvent.Conversion) + + // Check impression event fields + impressionEvent := userEvent.Impression + assert.Equal(t, experiment.ID, impressionEvent.ExperimentID) + assert.Equal(t, variation.ID, impressionEvent.VariationID) + assert.Equal(t, flagKey, impressionEvent.Metadata.FlagKey) + assert.Equal(t, ruleKey, impressionEvent.Metadata.RuleKey) + assert.Equal(t, ruleType, impressionEvent.Metadata.RuleType) + assert.Equal(t, enabled, impressionEvent.Metadata.Enabled) + + fmt.Printf("CMAB UUID: %s\n", cmabUUID) + fmt.Printf("UserEvent UUID: %s\n", userEvent.UUID) + + // Check that the UserEvent.UUID is set to the CMAB UUID + assert.Equal(t, cmabUUID, userEvent.UUID) +} + +func TestCreateCMABImpressionUserEventWithNilVariation(t *testing.T) { + // Setup + config := TestConfig{} + + experiment := entities.Experiment{ + ID: "exp123", + Key: "exp_key", + } + + userContext := entities.UserContext{ + ID: "user123", + } + + flagKey := "flag_key" + ruleKey := "rule_key" + ruleType := "experiment" + enabled := true + cmabUUID := "uuid123" + + // Call function with nil variation + userEvent, shouldDispatch := CreateCMABImpressionUserEvent( + config, + experiment, + nil, + userContext, + flagKey, + ruleKey, + ruleType, + enabled, + cmabUUID, + ) + + // Assertions + assert.False(t, shouldDispatch) + assert.Equal(t, UserEvent{}, userEvent) +} From a5b197cf9458bf8fd5f042b784f7783c295aeee0 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 15 May 2025 06:42:38 -0700 Subject: [PATCH 2/4] remove unnecessary context --- pkg/cmab/client.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/cmab/client.go b/pkg/cmab/client.go index 0be7b9ee..3ea2bc13 100644 --- a/pkg/cmab/client.go +++ b/pkg/cmab/client.go @@ -293,11 +293,6 @@ func (c *DefaultCmabClient) LogImpression( attributes map[string]interface{}, cmabUUID string, ) error { - // If no context is provided, create a background context - if ctx == nil { - ctx = context.Background() - } - // Instead of directly creating an event, we'll delegate this to the client // that has access to the event package return fmt.Errorf("CMAB impression logging not implemented") From 8f9771e7f38138f717161093880111c97f702922 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Mon, 19 May 2025 02:43:15 -0700 Subject: [PATCH 3/4] remove cmab term from types inside cmab package to avoid lint stuttering --- pkg/cmab/client.go | 50 ++++++++++++++++++++-------------------- pkg/cmab/client_test.go | 44 +++++++++++++++++------------------ pkg/cmab/service.go | 30 ++++++++++++------------ pkg/cmab/service_test.go | 12 +++++----- pkg/cmab/types.go | 18 +++++++-------- 5 files changed, 77 insertions(+), 77 deletions(-) diff --git a/pkg/cmab/client.go b/pkg/cmab/client.go index 3ea2bc13..c26d8881 100644 --- a/pkg/cmab/client.go +++ b/pkg/cmab/client.go @@ -47,34 +47,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 @@ -98,8 +98,8 @@ type DefaultCmabClient struct { projectConfig config.ProjectConfig } -// 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 @@ -108,7 +108,7 @@ type CmabClientOptions struct { } // 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{ @@ -151,9 +151,9 @@ func (c *DefaultCmabClient) FetchDecision( 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", @@ -161,8 +161,8 @@ func (c *DefaultCmabClient) FetchDecision( } // Create the request body - requestBody := CMABRequest{ - Instances: []CMABInstance{ + requestBody := Request{ + Instances: []Instance{ { VisitorID: userID, ExperimentID: ruleID, @@ -257,7 +257,7 @@ func (c *DefaultCmabClient) doFetch(ctx context.Context, url string, bodyBytes [ } // Parse response - var cmabResponse CMABResponse + var cmabResponse Response if err := json.Unmarshal(respBody, &cmabResponse); err != nil { return "", fmt.Errorf("failed to unmarshal CMAB response: %w", err) } @@ -272,7 +272,7 @@ func (c *DefaultCmabClient) doFetch(ctx context.Context, url string, bodyBytes [ } // validateResponse validates the CMAB response -func (c *DefaultCmabClient) validateResponse(response CMABResponse) bool { +func (c *DefaultCmabClient) validateResponse(response Response) bool { return len(response.Predictions) > 0 && response.Predictions[0].VariationID != "" } diff --git a/pkg/cmab/client_test.go b/pkg/cmab/client_test.go index 6a72e822..ab1d75c4 100644 --- a/pkg/cmab/client_test.go +++ b/pkg/cmab/client_test.go @@ -65,7 +65,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 +80,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 +109,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 +121,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, }, @@ -167,7 +167,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 +187,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 +199,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, }, @@ -247,7 +247,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, }, @@ -292,7 +292,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, }, @@ -356,7 +356,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, }, @@ -393,7 +393,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 }, @@ -442,8 +442,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 +454,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, }, @@ -504,7 +504,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 +541,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, }, @@ -610,7 +610,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, }, @@ -649,8 +649,8 @@ func TestDefaultCmabClient_FetchDecision_ContextCancellation(t *testing.T) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - response := CMABResponse{ - Predictions: []CMABPrediction{ + response := Response{ + Predictions: []Prediction{ { VariationID: "var123", }, @@ -661,7 +661,7 @@ func TestDefaultCmabClient_FetchDecision_ContextCancellation(t *testing.T) { defer server.Close() // Create client with custom endpoint - client := NewDefaultCmabClient(CmabClientOptions{ + client := NewDefaultCmabClient(ClientOptions{ HTTPClient: &http.Client{ Timeout: 5 * time.Second, }, diff --git a/pkg/cmab/service.go b/pkg/cmab/service.go index 02ff7aa6..e1aa9979 100644 --- a/pkg/cmab/service.go +++ b/pkg/cmab/service.go @@ -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, @@ -143,7 +143,7 @@ func (s *DefaultCmabService) GetDecision( } // 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{} @@ -201,7 +201,7 @@ func (s *DefaultCmabService) fetchDecisionWithRetry( cmabUUID, ) - return CmabDecision{ + return Decision{ VariationID: variationID, CmabUUID: cmabUUID, Reasons: reasons, @@ -214,7 +214,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/cmab/service_test.go b/pkg/cmab/service_test.go index 2804eeb0..ea30c810 100644 --- a/pkg/cmab/service_test.go +++ b/pkg/cmab/service_test.go @@ -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, @@ -339,7 +339,7 @@ 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", @@ -705,7 +705,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", @@ -748,7 +748,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 })) @@ -789,7 +789,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) @@ -1019,7 +1019,7 @@ func (s *CmabServiceTestSuite) TestNoTrackingOnCachedDecisions() { 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", diff --git a/pkg/cmab/types.go b/pkg/cmab/types.go index 4b30ead2..b2c1b35c 100644 --- a/pkg/cmab/types.go +++ b/pkg/cmab/types.go @@ -23,33 +23,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, From cf1ab2811aaa34731976d2ff37ecfd3319d0a91f Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Mon, 19 May 2025 06:18:27 -0700 Subject: [PATCH 4/4] add tests --- pkg/cmab/client_test.go | 498 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 497 insertions(+), 1 deletion(-) diff --git a/pkg/cmab/client_test.go b/pkg/cmab/client_test.go index ab1d75c4..3cd9aa5f 100644 --- a/pkg/cmab/client_test.go +++ b/pkg/cmab/client_test.go @@ -28,14 +28,197 @@ import ( "testing" "time" + "github.com/optimizely/go-sdk/v2/pkg/entities" + "github.com/optimizely/go-sdk/v2/pkg/event" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// Create a simple test implementation that satisfies the interface +type testProjectConfig struct { + // Map to store attributes by key for GetAttributeByKey + attributesByKey map[string]entities.Attribute + + // Functions that can be overridden for testing + getExperimentByIDFn func(id string) (entities.Experiment, error) + getFeatureListFn func() []entities.Feature +} + +// NewTestProjectConfig creates a new test project config +func NewTestProjectConfig() *testProjectConfig { + return &testProjectConfig{ + attributesByKey: make(map[string]entities.Attribute), + getExperimentByIDFn: func(id string) (entities.Experiment, error) { + if id == "rule-id" { + return entities.Experiment{ID: id, Key: "rule-key"}, nil + } + return entities.Experiment{}, fmt.Errorf("experiment not found") + }, + getFeatureListFn: func() []entities.Feature { + return []entities.Feature{ + { + Key: "feature-key", + FeatureExperiments: []entities.Experiment{ + {ID: "rule-id"}, + }, + }, + } + }, + } +} + +// GetExperimentByID returns a mock experiment +func (c *testProjectConfig) GetExperimentByID(id string) (entities.Experiment, error) { + if c.getExperimentByIDFn != nil { + return c.getExperimentByIDFn(id) + } + return entities.Experiment{}, fmt.Errorf("experiment not found") +} + +// GetFeatureList returns mock features +func (c *testProjectConfig) GetFeatureList() []entities.Feature { + if c.getFeatureListFn != nil { + return c.getFeatureListFn() + } + return []entities.Feature{} +} + +// GetAttributeByKey returns a mock attribute +func (c *testProjectConfig) GetAttributeByKey(key string) (entities.Attribute, error) { + if attr, ok := c.attributesByKey[key]; ok { + return attr, nil + } + return entities.Attribute{Key: key}, nil +} + +// Implement all other required methods with minimal functionality +func (c *testProjectConfig) GetProjectID() string { return "mock-project" } +func (c *testProjectConfig) GetRevision() string { return "1" } +func (c *testProjectConfig) GetAccountID() string { return "mock-account" } +func (c *testProjectConfig) GetAnonymizeIP() bool { return false } +func (c *testProjectConfig) GetAttributeID(key string) string { return "attr-id" } +func (c *testProjectConfig) GetAttributeKeyByID(id string) (string, error) { + return "attr-key", nil +} +func (c *testProjectConfig) GetAttributeByID(id string) (entities.Attribute, error) { + return entities.Attribute{ID: id, Key: "attr-key"}, nil +} +func (c *testProjectConfig) GetAudienceByID(id string) (entities.Audience, error) { + return entities.Audience{ID: id}, nil +} +func (c *testProjectConfig) GetAudienceMap() map[string]entities.Audience { + return map[string]entities.Audience{"audience-id": {ID: "audience-id"}} +} +func (c *testProjectConfig) GetBotFiltering() bool { return false } +func (c *testProjectConfig) GetClientName() string { return "mock-client" } +func (c *testProjectConfig) GetClientVersion() string { return "1.0.0" } +func (c *testProjectConfig) GetDatafile() string { return "{}" } +func (c *testProjectConfig) GetEventByKey(key string) (entities.Event, error) { + return entities.Event{Key: key}, nil +} +func (c *testProjectConfig) GetFeatureByID(featureID string) (entities.Feature, error) { + return entities.Feature{ID: featureID}, nil +} +func (c *testProjectConfig) GetFeatureByKey(featureKey string) (entities.Feature, error) { + return entities.Feature{Key: featureKey}, nil +} +func (c *testProjectConfig) GetExperimentByKey(key string) (entities.Experiment, error) { + return entities.Experiment{Key: key}, nil +} +func (c *testProjectConfig) GetExperimentFeatureMap() map[string][]entities.Feature { + return map[string][]entities.Feature{} +} +func (c *testProjectConfig) GetFlagVariationsMap() map[string][]entities.Variation { + return map[string][]entities.Variation{} +} +func (c *testProjectConfig) GetGroupByID(id string) (entities.Group, error) { + return entities.Group{ID: id}, nil +} +func (c *testProjectConfig) GetRolloutByID(id string) (entities.Rollout, error) { + return entities.Rollout{ID: id}, nil +} +func (c *testProjectConfig) GetRolloutIDFromExperimentID(experimentID string) string { + return "rollout-id" +} +func (c *testProjectConfig) GetVariationFromKey(experimentKey, variationKey string) (entities.Variation, error) { + return entities.Variation{Key: variationKey}, nil +} +func (c *testProjectConfig) GetVariationFromID(experimentID, variationID string) (entities.Variation, error) { + return entities.Variation{ID: variationID}, nil +} +func (c *testProjectConfig) GetSdkKey() string { return "mock-sdk-key" } +func (c *testProjectConfig) GetEnvironmentKey() string { return "mock-env-key" } +func (c *testProjectConfig) SendFlagDecisions() bool { return false } + +// GetAttributes returns all attributes +func (c *testProjectConfig) GetAttributes() []entities.Attribute { + return []entities.Attribute{ + {ID: "attr-id", Key: "attr-key"}, + } +} + +// GetAudienceList returns all audiences +func (c *testProjectConfig) GetAudienceList() []entities.Audience { + return []entities.Audience{ + {ID: "audience-id", Name: "Test Audience"}, + } +} + +// GetEvents returns all events +func (c *testProjectConfig) GetEvents() []entities.Event { + return []entities.Event{ + {ID: "event-id", Key: "test_event"}, + } +} + +// GetHostForODP returns the host for ODP +func (c *testProjectConfig) GetHostForODP() string { + return "https://odp.example.com" +} + +// GetPublicKeyForODP returns the public key for ODP +func (c *testProjectConfig) GetPublicKeyForODP() string { + return "mock-public-key" +} + +// GetExperimentList returns all experiments +func (c *testProjectConfig) GetExperimentList() []entities.Experiment { + return []entities.Experiment{ + {ID: "rule-id", Key: "rule-key"}, + } +} + +// GetSegmentList returns all segments +func (c *testProjectConfig) GetSegmentList() []string { + return []string{"segment1", "segment2"} +} + +// GetIntegrationList returns all integrations +func (c *testProjectConfig) GetIntegrationList() []entities.Integration { + return []entities.Integration{} +} + +// GetRolloutList returns all rollouts +func (c *testProjectConfig) GetRolloutList() []entities.Rollout { + return []entities.Rollout{ + {ID: "rollout-id"}, + } +} + +// GetVariableByKey returns a variable by key +func (c *testProjectConfig) GetVariableByKey(featureKey string, variableKey string) (entities.Variable, error) { + return entities.Variable{ + ID: "var-id", + Key: variableKey, + Type: "string", + }, nil +} + // Mock logger for testing type mockLogger struct { debugFn func(message string) warningFn func(message string) + errorFn func(message string, err interface{}) } func (m *mockLogger) Debug(message string) { @@ -53,7 +236,11 @@ func (m *mockLogger) Warning(message string) { } // Update the Error method to match the expected interface -func (m *mockLogger) Error(message string, err interface{}) {} +func (m *mockLogger) Error(message string, err interface{}) { + if m.errorFn != nil { + m.errorFn(message, err) + } +} func TestDefaultCmabClient_FetchDecision(t *testing.T) { // Setup test server @@ -688,3 +875,312 @@ func TestDefaultCmabClient_FetchDecision_ContextCancellation(t *testing.T) { assert.Contains(t, err.Error(), "context") assert.Contains(t, err.Error(), "deadline exceeded") } + +func TestLogImpression(t *testing.T) { + // Create a client with minimal options + client := NewDefaultCmabClient(ClientOptions{}) + + // Test the unimplemented method + err := client.LogImpression( + context.Background(), + nil, + nil, + "rule-id", + "user-id", + "variation-key", + "variation-id", + nil, + "cmab-uuid", + ) + + // Verify we get the expected error + assert.Error(t, err) + assert.Contains(t, err.Error(), "not implemented") +} + +// Mock event processor for testing +type mockEventProcessor struct { + processEventFn func(userEvent event.UserEvent) bool +} + +func (m *mockEventProcessor) Process(userEvent interface{}) bool { + if m.processEventFn != nil { + if userEventTyped, ok := userEvent.(event.UserEvent); ok { + return m.processEventFn(userEventTyped) + } + } + return true +} + +func (m *mockEventProcessor) ProcessEvent(userEvent event.UserEvent) bool { + if m.processEventFn != nil { + return m.processEventFn(userEvent) + } + return true +} + +func (m *mockEventProcessor) OnEventDispatch(callback func(event.LogEvent)) (int, error) { + return 1, nil // Return a dummy notification ID and no error +} + +func (m *mockEventProcessor) RemoveOnEventDispatch(id int) error { + return nil // Return no error +} + +// Mock project config for testing +type mockProjectConfig struct { + getExperimentByIDFn func(id string) (entities.Experiment, error) + getFeatureListFn func() []entities.Feature +} + +func (m *mockProjectConfig) GetExperimentByID(id string) (entities.Experiment, error) { + if m.getExperimentByIDFn != nil { + return m.getExperimentByIDFn(id) + } + return entities.Experiment{}, fmt.Errorf("experiment not found") +} + +func (m *mockProjectConfig) GetAttributeByKey(key string) (entities.Attribute, error) { + return entities.Attribute{}, nil +} + +func (m *mockProjectConfig) GetAttributeKeyByID(id string) (string, error) { + return "attr-key", nil +} + +func (m *mockProjectConfig) GetFeatureList() []entities.Feature { + if m.getFeatureListFn != nil { + return m.getFeatureListFn() + } + return []entities.Feature{} +} + +// Implement other required methods of the ProjectConfig interface with empty implementations +func (m *mockProjectConfig) GetProjectID() string { return "mock-project" } +func (m *mockProjectConfig) GetRevision() string { return "1" } +func (m *mockProjectConfig) GetAccountID() string { return "mock-account" } +func (m *mockProjectConfig) GetAnonymizeIP() bool { return false } +func (m *mockProjectConfig) GetAttributeID(key string) string { return "" } +func (m *mockProjectConfig) GetAttributeByID(id string) (entities.Attribute, error) { + return entities.Attribute{}, nil +} +func (m *mockProjectConfig) GetAudienceByID(id string) (entities.Audience, error) { + return entities.Audience{}, nil +} +func (m *mockProjectConfig) GetAudienceMap() map[string]entities.Audience { return nil } +func (m *mockProjectConfig) GetBotFiltering() bool { return false } +func (m *mockProjectConfig) GetClientName() string { return "mock-client" } +func (m *mockProjectConfig) GetClientVersion() string { return "1.0.0" } +func (m *mockProjectConfig) GetDatafile() string { return "" } +func (m *mockProjectConfig) GetEventByKey(key string) (entities.Event, error) { + return entities.Event{}, nil +} +func (m *mockProjectConfig) GetFeatureByID(featureID string) (entities.Feature, error) { + return entities.Feature{}, nil +} +func (m *mockProjectConfig) GetFeatureByKey(featureKey string) (entities.Feature, error) { + return entities.Feature{}, nil +} +func (m *mockProjectConfig) GetExperimentByKey(key string) (entities.Experiment, error) { + return entities.Experiment{}, nil +} +func (m *mockProjectConfig) GetExperimentFeatureMap() map[string][]entities.Feature { return nil } +func (m *mockProjectConfig) GetFlagVariationsMap() map[string][]entities.Variation { return nil } +func (m *mockProjectConfig) GetGroupByID(id string) (entities.Group, error) { + return entities.Group{}, nil +} +func (m *mockProjectConfig) GetRolloutByID(id string) (entities.Rollout, error) { + return entities.Rollout{}, nil +} +func (m *mockProjectConfig) GetRolloutIDFromExperimentID(experimentID string) string { return "" } +func (m *mockProjectConfig) GetVariationFromKey(experimentKey, variationKey string) (entities.Variation, error) { + return entities.Variation{}, nil +} +func (m *mockProjectConfig) GetVariationFromID(experimentID, variationID string) (entities.Variation, error) { + return entities.Variation{}, nil +} +func (m *mockProjectConfig) GetSdkKey() string { return "mock-sdk-key" } +func (m *mockProjectConfig) GetEnvironmentKey() string { return "mock-env-key" } +func (m *mockProjectConfig) SendFlagDecisions() bool { return false } + +// GetAttributes returns all attributes +func (m *mockProjectConfig) GetAttributes() []entities.Attribute { + return []entities.Attribute{ + {ID: "attr-id", Key: "attr-key"}, + } +} + +// GetAudienceList returns all audiences +func (m *mockProjectConfig) GetAudienceList() []entities.Audience { + return []entities.Audience{ + {ID: "audience-id", Name: "Test Audience"}, + } +} + +// GetEvents returns all events +func (m *mockProjectConfig) GetEvents() []entities.Event { + return []entities.Event{ + {ID: "event-id", Key: "test_event"}, + } +} + +// GetHostForODP returns the host for ODP +func (m *mockProjectConfig) GetHostForODP() string { + return "https://odp.example.com" +} + +// GetPublicKeyForODP returns the public key for ODP +func (m *mockProjectConfig) GetPublicKeyForODP() string { + return "mock-public-key" +} + +// GetExperimentList returns all experiments +func (m *mockProjectConfig) GetExperimentList() []entities.Experiment { + return []entities.Experiment{ + {ID: "rule-id", Key: "rule-key"}, + } +} + +// GetSegmentList returns all segments +func (m *mockProjectConfig) GetSegmentList() []string { + return []string{"segment1", "segment2"} +} + +// GetIntegrationList returns all integrations +func (m *mockProjectConfig) GetIntegrationList() []entities.Integration { + return []entities.Integration{} +} + +// GetRolloutList returns all rollouts +func (m *mockProjectConfig) GetRolloutList() []entities.Rollout { + return []entities.Rollout{ + {ID: "rollout-id"}, + } +} + +// GetVariableByKey returns a variable by key +func (m *mockProjectConfig) GetVariableByKey(featureKey string, variableKey string) (entities.Variable, error) { + return entities.Variable{ + ID: "var-id", + Key: variableKey, + Type: "string", + }, nil +} + +func TestTrackCMABDecision(t *testing.T) { + // Test 1: With nil event processor and project config + t.Run("with nil dependencies", func(t *testing.T) { + debugMessages := []string{} + mockLogger := &mockLogger{ + debugFn: func(message string) { + debugMessages = append(debugMessages, message) + }, + } + + client := NewDefaultCmabClient(ClientOptions{ + Logger: mockLogger, + }) + + client.TrackCMABDecision( + "rule-id", + "user-id", + "variation-id", + "variation-key", + nil, + "cmab-uuid", + ) + + // Verify log message was produced + assert.Len(t, debugMessages, 1) + assert.Contains(t, debugMessages[0], "Event processor or project config not available") + }) + + // Test 2: With error getting experiment + t.Run("with error getting experiment", func(t *testing.T) { + errorMessages := []string{} + mockLogger := &mockLogger{ + errorFn: func(message string, err interface{}) { + errorMessages = append(errorMessages, message) + }, + } + + // Create a test config with a custom GetExperimentByID function + testConfig := NewTestProjectConfig() + testConfig.getExperimentByIDFn = func(id string) (entities.Experiment, error) { + return entities.Experiment{}, fmt.Errorf("experiment not found") + } + + client := NewDefaultCmabClient(ClientOptions{ + Logger: mockLogger, + ProjectConfig: testConfig, + }) + + client.TrackCMABDecision( + "rule-id", + "user-id", + "variation-id", + "variation-key", + nil, + "cmab-uuid", + ) + + // This test will pass even without checking logs because we're testing that + // the function doesn't panic when experiment is not found + }) + + // Test 3: With valid event processor and project config + t.Run("with valid dependencies", func(t *testing.T) { + processedEvents := []event.UserEvent{} + mockEventProcessor := &mockEventProcessor{ + processEventFn: func(userEvent event.UserEvent) bool { + processedEvents = append(processedEvents, userEvent) + return true + }, + } + + // Setup mock experiment + mockExperiment := entities.Experiment{ + ID: "rule-id", + Key: "rule-key", + } + + // Setup mock feature + mockFeature := entities.Feature{ + Key: "feature-key", + FeatureExperiments: []entities.Experiment{ + { + ID: "rule-id", + }, + }, + } + + mockProjectConfig := &mockProjectConfig{ + getExperimentByIDFn: func(id string) (entities.Experiment, error) { + if id == "rule-id" { + return mockExperiment, nil + } + return entities.Experiment{}, fmt.Errorf("experiment not found") + }, + getFeatureListFn: func() []entities.Feature { + return []entities.Feature{mockFeature} + }, + } + + client := NewDefaultCmabClient(ClientOptions{ + EventProcessor: mockEventProcessor, + ProjectConfig: mockProjectConfig, + }) + + client.TrackCMABDecision( + "rule-id", + "user-id", + "variation-id", + "variation-key", + map[string]interface{}{"attr1": "value1"}, + "cmab-uuid", + ) + + // Verify the event processor was called + assert.Len(t, processedEvents, 1, "Expected one event to be processed") + }) +}