Merge branch 'master' into yasir/datafilerevisionlog

Author: yasirfolio3

optimizely/client/client_test.go

@@ -92,7 +92,11 @@ func (m *PanickingDecisionService) GetFeatureDecision(decisionContext decision.F
 	panic("I'm panicking")
 }
 
-func (m *PanickingDecisionService) OnDecision(callback func(notification.DecisionNotification)) {
+func (m *PanickingDecisionService) OnDecision(callback func(notification.DecisionNotification)) (int, error) {
+	panic("I'm panicking")
+}
+
+func (m *PanickingDecisionService) RemoveOnDecision(id int) error {
 	panic("I'm panicking")
 }
 

optimizely/decision/composite_service.go

@@ -88,15 +88,27 @@ func (s CompositeService) GetFeatureDecision(featureDecisionContext FeatureDecis
 }
 
 // OnDecision registers a handler for Decision notifications
-func (s CompositeService) OnDecision(callback func(notification.DecisionNotification)) {
+func (s CompositeService) OnDecision(callback func(notification.DecisionNotification)) (int, error) {
 	handler := func(payload interface{}) {
 		if decisionNotification, ok := payload.(notification.DecisionNotification); ok {
 			callback(decisionNotification)
 		} else {
 			csLogger.Warning(fmt.Sprintf("Unable to convert notification payload %v into DecisionNotification", payload))
 		}
 	}
-	if _, err := s.notificationCenter.AddHandler(notification.Decision, handler); err != nil {
+	id, err := s.notificationCenter.AddHandler(notification.Decision, handler)
+	if err != nil {
 		csLogger.Warning("Problem with adding notification handler")
+		return 0, err
 	}
+	return id, nil
+}
+
+// RemoveOnDecision removes handler for Decision notification with given id
+func (s CompositeService) RemoveOnDecision(id int) error {
+	if err := s.notificationCenter.RemoveHandler(id, notification.Decision); err != nil {
+		csLogger.Warning("Problem with removing notification handler")
+		return err
+	}
+	return nil
 }

optimizely/decision/composite_service_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/optimizely/go-sdk/optimizely/entities"
+	"github.com/optimizely/go-sdk/optimizely/notification"
 )
 
 func TestGetFeatureDecision(t *testing.T) {
@@ -54,3 +55,45 @@ func TestGetFeatureDecision(t *testing.T) {
 	assert.Equal(t, expectedFeatureDecision, featureDecision)
 	testFeatureDecisionService.AssertExpectations(t)
 }
+
+func TestOnDecision(t *testing.T) {
+
+	mockProjectConfig := new(mockProjectConfig)
+	decisionContext := FeatureDecisionContext{
+		Feature:       &testFeat3333,
+		ProjectConfig: mockProjectConfig,
+	}
+
+	userContext := entities.UserContext{
+		ID: "test_user",
+	}
+
+	expectedFeatureDecision := FeatureDecision{
+		Experiment: testExp1111,
+		Variation:  &testExp1111Var2222,
+	}
+
+	testFeatureDecisionService := new(MockFeatureDecisionService)
+	testFeatureDecisionService.On("GetDecision", decisionContext, userContext).Return(expectedFeatureDecision, nil)
+
+	notificationCenter := notification.NewNotificationCenter()
+	decisionService := &CompositeService{
+		featureDecisionServices: []FeatureService{testFeatureDecisionService},
+		notificationCenter:      notificationCenter,
+	}
+
+	var numberOfCalls = 0
+	callback := func(notification notification.DecisionNotification) {
+		numberOfCalls++
+	}
+	id, _ := decisionService.OnDecision(callback)
+
+	assert.NotEqual(t, id, 0)
+	decisionService.GetFeatureDecision(decisionContext, userContext)
+	assert.Equal(t, numberOfCalls, 1)
+
+	err := decisionService.RemoveOnDecision(id)
+	assert.Nil(t, err)
+	decisionService.GetFeatureDecision(decisionContext, userContext)
+	assert.Equal(t, numberOfCalls, 1)
+}

optimizely/decision/interface.go

@@ -25,7 +25,8 @@ import (
 // Service interface is used to make a decision for a given feature or experiment
 type Service interface {
 	GetFeatureDecision(FeatureDecisionContext, entities.UserContext) (FeatureDecision, error)
-	OnDecision(func(notification.DecisionNotification))
+	OnDecision(func(notification.DecisionNotification)) (int, error)
+	RemoveOnDecision(id int) error
 }
 
 // ExperimentService can make a decision about an experiment

optimizely/notification/center.go

@@ -22,6 +22,7 @@ import "fmt"
 // Center handles all notification listeners. It keeps track of the Manager for each type of notification.
 type Center interface {
 	AddHandler(Type, func(interface{})) (int, error)
+	RemoveHandler(int, Type) error
 	Send(Type, interface{}) error
 }
 
@@ -43,12 +44,22 @@ func NewNotificationCenter() *DefaultCenter {
 // AddHandler adds a handler for the given notification type
 func (c *DefaultCenter) AddHandler(notificationType Type, handler func(interface{})) (int, error) {
 	if manager, ok := c.managerMap[notificationType]; ok {
-		return manager.AddHandler(handler)
+		return manager.Add(handler)
 	}
 
 	return -1, fmt.Errorf("no notification manager found for type %s", notificationType)
 }
 
+// RemoveHandler removes a handler for the given id and notification type
+func (c *DefaultCenter) RemoveHandler(id int, notificationType Type) error {
+	if manager, ok := c.managerMap[notificationType]; ok {
+		manager.Remove(id)
+		return nil
+	}
+
+	return fmt.Errorf("no notification manager found for type %s", notificationType)
+}
+
 // Send sends the given notification payload to all listeners of type
 func (c *DefaultCenter) Send(notificationType Type, notification interface{}) error {
 	if manager, ok := c.managerMap[notificationType]; ok {

optimizely/notification/center_test.go

@@ -3,6 +3,8 @@ package notification
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/optimizely/go-sdk/optimizely/entities"
 	"github.com/stretchr/testify/mock"
 )
@@ -32,10 +34,24 @@ func TestNotificationCenter(t *testing.T) {
 	mockReceiver.On("handleNotification", testDecisionNotification)
 	mockReceiver2.On("handleNotification", testDecisionNotification)
 	notificationCenter := NewNotificationCenter()
-	notificationCenter.AddHandler(Decision, mockReceiver.handleNotification)
-	notificationCenter.AddHandler(Decision, mockReceiver2.handleNotification)
+	id1, err1 := notificationCenter.AddHandler(Decision, mockReceiver.handleNotification)
+	id2, err2 := notificationCenter.AddHandler(Decision, mockReceiver2.handleNotification)
 	notificationCenter.Send(Decision, testDecisionNotification)
 
 	mockReceiver.AssertExpectations(t)
 	mockReceiver2.AssertExpectations(t)
+	assert.Nil(t, err1)
+	assert.Nil(t, err2)
+
+	notificationCenter.RemoveHandler(id1, Decision)
+	notificationCenter.Send(Decision, testDecisionNotification)
+
+	mockReceiver.AssertNumberOfCalls(t, "handleNotification", 1)
+	mockReceiver2.AssertNumberOfCalls(t, "handleNotification", 2)
+
+	notificationCenter.RemoveHandler(id2, Decision)
+	notificationCenter.Send(Decision, testDecisionNotification)
+
+	mockReceiver.AssertNumberOfCalls(t, "handleNotification", 1)
+	mockReceiver2.AssertNumberOfCalls(t, "handleNotification", 2)
 }

optimizely/notification/manager.go

@@ -23,7 +23,8 @@ import (
 
 // Manager is a generic interface for managing notifications of a particular type
 type Manager interface {
-	AddHandler(func(interface{})) (int, error)
+	Add(func(interface{})) (int, error)
+	Remove(id int)
 	Send(message interface{})
 }
 
@@ -40,13 +41,21 @@ func NewAtomicManager() *AtomicManager {
 	}
 }
 
-// AddHandler adds the given handler
-func (am *AtomicManager) AddHandler(newHandler func(interface{})) (int, error) {
+// Add adds the given handler
+func (am *AtomicManager) Add(newHandler func(interface{})) (int, error) {
 	atomic.AddUint32(&am.counter, 1)
 	am.handlers[am.counter] = newHandler
 	return int(am.counter), nil
 }
 
+// Remove removes handler with the given id
+func (am *AtomicManager) Remove(id int) {
+	handlerID := uint32(id)
+	if _, ok := am.handlers[handlerID]; ok {
+		delete(am.handlers, handlerID)
+	}
+}
+
 // Send sends the notification to the registered handlers
 func (am *AtomicManager) Send(notification interface{}) {
 	for _, handler := range am.handlers {

optimizely/notification/manager_test.go

@@ -29,12 +29,21 @@ func TestAtomicManager(t *testing.T) {
 	mockReceiver.On("handleBetter", payload)
 
 	atomicManager := NewAtomicManager()
-	result, _ := atomicManager.AddHandler(mockReceiver.handle)
-	assert.Equal(t, 1, result)
+	result1, _ := atomicManager.Add(mockReceiver.handle)
+	assert.Equal(t, 1, result1)
 
-	result, _ = atomicManager.AddHandler(mockReceiver.handleBetter)
-	assert.Equal(t, 2, result)
+	result2, _ := atomicManager.Add(mockReceiver.handleBetter)
+	assert.Equal(t, 2, result2)
 
 	atomicManager.Send(payload)
 	mockReceiver.AssertExpectations(t)
+
+	atomicManager.Remove(result1)
+	atomicManager.Send(payload)
+	mockReceiver.AssertNumberOfCalls(t, "handle", 1)
+	mockReceiver.AssertNumberOfCalls(t, "handleBetter", 2)
+
+	atomicManager.Remove(result2)
+	atomicManager.Send(payload)
+	mockReceiver.AssertNumberOfCalls(t, "handleBetter", 2)
 }