Skip to content

Commit 5a0d450

Browse files
yasirfolio3yasirfolio3
authored
[FSSDK-8984] fix: Updates SendOdpEvent to return error instead of bool. (#365)
* Updates SendOdpEvent to return error instead of bool.
1 parent 49b754c commit 5a0d450

File tree

7 files changed

+56
-47
lines changed

7 files changed

+56
-47
lines changed

‎pkg/client/client.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,8 @@ func (o *OptimizelyClient) fetchQualifiedSegments(userContext *OptimizelyUserCon
287287
}
288288

289289
// SendOdpEvent sends an event to the ODP server.
290-
func (o *OptimizelyClient) SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) bool {
290+
func (o *OptimizelyClient) SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) (err error) {
291291

292-
var err error
293292
defer func() {
294293
if r := recover(); r != nil {
295294
switch t := r.(type) {
@@ -312,8 +311,9 @@ func (o *OptimizelyClient) SendOdpEvent(eventType, action string, identifiers ma
312311
}
313312

314313
if len(identifiers) == 0 {
315-
o.logger.Error("ODP events must have at least one key-value pair in identifiers", err)
316-
return false
314+
err = errors.New("ODP events must have at least one key-value pair in identifiers")
315+
o.logger.Error("received an error while sending ODP event", err)
316+
return err
317317
}
318318

319319
return o.OdpManager.SendOdpEvent(eventType, action, identifiers, data)

‎pkg/client/client_test.go

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,12 @@ func (m *MockODPManager) IdentifyUser(userID string) {
193193
m.Called(userID)
194194
}
195195

196-
func (m *MockODPManager) SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) bool {
197-
return m.Called(eventType, action, identifiers, data).Get(0).(bool)
196+
func (m *MockODPManager) SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) error {
197+
err := m.Called(eventType, action, identifiers, data).Get(0)
198+
if err == nil {
199+
return nil
200+
}
201+
return err.(error)
198202
}
199203

200204
func (m *MockODPManager) Update(apiKey, apiHost string, segmentsToCheck []string) {
@@ -208,7 +212,7 @@ func TestSendODPEventWhenODPDisabled(t *testing.T) {
208212
var disableOdp = true
209213
optimizelyClient, err := factory.Client(WithSegmentsCacheSize(segmentsCacheSize), WithSegmentsCacheTimeout(segmentsCacheTimeout), WithOdpDisabled(disableOdp))
210214
assert.NoError(t, err)
211-
success := optimizelyClient.SendOdpEvent("123", "456", map[string]string{
215+
err = optimizelyClient.SendOdpEvent("123", "456", map[string]string{
212216
"abc": "123",
213217
}, map[string]interface{}{
214218
"abc": nil,
@@ -217,7 +221,7 @@ func TestSendODPEventWhenODPDisabled(t *testing.T) {
217221
"data_source": true,
218222
"data_source_version": 6.78,
219223
})
220-
assert.False(t, success)
224+
assert.Error(t, err)
221225
}
222226

223227
func TestSendODPEventEmptyType(t *testing.T) {
@@ -234,12 +238,12 @@ func TestSendODPEventEmptyType(t *testing.T) {
234238
"data_source_version": 6.78,
235239
}
236240
mockOdpManager := &MockODPManager{}
237-
mockOdpManager.On("SendOdpEvent", eventType, action, identifiers, data).Return(true)
241+
mockOdpManager.On("SendOdpEvent", eventType, action, identifiers, data).Return(nil)
238242
optimizelyClient := OptimizelyClient{
239243
OdpManager: mockOdpManager,
240244
}
241-
success := optimizelyClient.SendOdpEvent("", action, identifiers, data)
242-
assert.True(t, success)
245+
err := optimizelyClient.SendOdpEvent("", action, identifiers, data)
246+
assert.NoError(t, err)
243247
mockOdpManager.AssertExpectations(t)
244248
}
245249

@@ -256,8 +260,8 @@ func TestSendODPEventEmptyIdentifiers(t *testing.T) {
256260
optimizelyClient := OptimizelyClient{
257261
logger: logging.GetLogger("", ""),
258262
}
259-
success := optimizelyClient.SendOdpEvent("", action, identifiers, data)
260-
assert.False(t, success)
263+
err := optimizelyClient.SendOdpEvent("", action, identifiers, data)
264+
assert.Error(t, err)
261265
}
262266

263267
func TestSendODPEventNilIdentifiers(t *testing.T) {
@@ -272,18 +276,18 @@ func TestSendODPEventNilIdentifiers(t *testing.T) {
272276
optimizelyClient := OptimizelyClient{
273277
logger: logging.GetLogger("", ""),
274278
}
275-
success := optimizelyClient.SendOdpEvent("", action, nil, data)
276-
assert.False(t, success)
279+
err := optimizelyClient.SendOdpEvent("", action, nil, data)
280+
assert.Error(t, err)
277281
}
278282

279283
func TestSendODPEvent(t *testing.T) {
280284
mockOdpManager := &MockODPManager{}
281-
mockOdpManager.On("SendOdpEvent", "123", "", map[string]string{"identifier": "123"}, mock.Anything).Return(true)
285+
mockOdpManager.On("SendOdpEvent", "123", "", map[string]string{"identifier": "123"}, mock.Anything).Return(nil)
282286
optimizelyClient := OptimizelyClient{
283287
OdpManager: mockOdpManager,
284288
}
285-
success := optimizelyClient.SendOdpEvent("123", "", map[string]string{"identifier": "123"}, nil)
286-
assert.True(t, success)
289+
err := optimizelyClient.SendOdpEvent("123", "", map[string]string{"identifier": "123"}, nil)
290+
assert.NoError(t, err)
287291
mockOdpManager.AssertExpectations(t)
288292
}
289293

‎pkg/odp/event/event_manager.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type Manager interface {
4343
// odpConfig is required here since it can be updated anytime and ticker needs to be aware of latest changes
4444
Start(ctx context.Context, odpConfig config.Config)
4545
IdentifyUser(apiKey, apiHost, userID string)
46-
ProcessEvent(apiKey, apiHost string, odpEvent Event) bool
46+
ProcessEvent(apiKey, apiHost string, odpEvent Event) error
4747
FlushEvents(apiKey, apiHost string)
4848
}
4949

@@ -163,33 +163,34 @@ func (bm *BatchEventManager) IdentifyUser(apiKey, apiHost, userID string) {
163163
Action: utils.OdpActionIdentified,
164164
Identifiers: identifiers,
165165
}
166-
bm.ProcessEvent(apiKey, apiHost, odpEvent)
166+
_ = bm.ProcessEvent(apiKey, apiHost, odpEvent)
167167
}
168168

169169
// ProcessEvent takes the given odp event and queues it up to be dispatched.
170170
// A dispatch happens when we flush the events, which can happen on a set interval or
171171
// when the specified batch size is reached.
172-
func (bm *BatchEventManager) ProcessEvent(apiKey, apiHost string, odpEvent Event) bool {
172+
func (bm *BatchEventManager) ProcessEvent(apiKey, apiHost string, odpEvent Event) (err error) {
173173
if !bm.IsOdpServiceIntegrated(apiKey, apiHost) {
174174
bm.logger.Debug(utils.OdpNotIntegrated)
175-
return false
175+
return errors.New(utils.OdpNotIntegrated)
176176
}
177177

178178
if !utils.IsValidOdpData(odpEvent.Data) {
179179
bm.logger.Error(utils.OdpInvalidData, errors.New("invalid event data"))
180-
return false
180+
return errors.New(utils.OdpInvalidData)
181181
}
182182

183183
if bm.eventQueue.Size() >= bm.maxQueueSize {
184-
bm.logger.Warning("maxQueueSize has been met. Discarding event")
185-
return false
184+
err = errors.New("ODP EventQueue is full")
185+
bm.logger.Error("maxQueueSize has been met. Discarding event", err)
186+
return err
186187
}
187188

188189
bm.addCommonData(&odpEvent)
189190
bm.eventQueue.Add(odpEvent)
190191

191192
if bm.eventQueue.Size() < bm.batchSize {
192-
return true
193+
return nil
193194
}
194195

195196
if bm.processing.TryAcquire(1) {
@@ -202,7 +203,7 @@ func (bm *BatchEventManager) ProcessEvent(apiKey, apiHost string, odpEvent Event
202203
}()
203204
}
204205

205-
return true
206+
return nil
206207
}
207208

208209
// StartTicker starts new ticker for flushing events

‎pkg/odp/event/event_manager_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func (e *EventManagerTestSuite) TestIdentifyUserWhenODPIntegrated() {
202202

203203
func (e *EventManagerTestSuite) TestProcessEventWithInvalidODPConfig() {
204204
em := NewBatchEventManager(WithAPIManager(&MockEventAPIManager{}))
205-
e.False(em.ProcessEvent("", "", Event{}))
205+
e.Error(em.ProcessEvent("", "", Event{}))
206206
e.Equal(0, em.eventQueue.Size())
207207
}
208208

@@ -220,7 +220,7 @@ func (e *EventManagerTestSuite) TestProcessEventWithValidEventData() {
220220
},
221221
}
222222

223-
e.True(e.eventManager.ProcessEvent("1", "2", tmpEvent))
223+
e.NoError(e.eventManager.ProcessEvent("1", "2", tmpEvent))
224224
e.Equal(1, e.eventManager.eventQueue.Size())
225225
}
226226

@@ -234,20 +234,20 @@ func (e *EventManagerTestSuite) TestProcessEventWithInvalidEventData() {
234234
"key12": []string{},
235235
},
236236
}
237-
e.False(e.eventManager.ProcessEvent("a", "b", tmpEvent))
237+
e.Error(e.eventManager.ProcessEvent("a", "b", tmpEvent))
238238
e.Equal(0, e.eventManager.eventQueue.Size())
239239
}
240240

241241
func (e *EventManagerTestSuite) TestProcessEventDiscardsEventExceedingMaxQueueSize() {
242242
e.eventManager.maxQueueSize = 1
243243
e.eventManager.eventQueue.Add(Event{})
244-
e.False(e.eventManager.ProcessEvent("a", "b", Event{}))
244+
e.Error(e.eventManager.ProcessEvent("a", "b", Event{}))
245245
e.Equal(1, e.eventManager.eventQueue.Size())
246246
}
247247

248248
func (e *EventManagerTestSuite) TestProcessEventWithBatchSizeNotReached() {
249249
em := NewBatchEventManager(WithAPIManager(&MockEventAPIManager{}))
250-
e.True(em.ProcessEvent("a", "b", Event{}))
250+
e.NoError(em.ProcessEvent("a", "b", Event{}))
251251
e.Equal(1, em.eventQueue.Size())
252252
e.Equal(0, e.eventAPIManager.timesSendEventsCalled)
253253
}
@@ -257,7 +257,7 @@ func (e *EventManagerTestSuite) TestProcessEventWithBatchSizeReached() {
257257
em := NewBatchEventManager(WithAPIManager(apiManager), WithFlushInterval(0))
258258
e.Equal(0, em.eventQueue.Size())
259259
apiManager.wg.Add(1)
260-
e.True(em.ProcessEvent("a", "b", Event{}))
260+
e.NoError(em.ProcessEvent("a", "b", Event{}))
261261
// Wait for event fire through go routine
262262
apiManager.wg.Wait()
263263
e.Equal(0, em.eventQueue.Size())
@@ -272,7 +272,7 @@ func (e *EventManagerTestSuite) TestProcessEventsExceedingBatchSize() {
272272
e.Equal(2, em.eventQueue.Size())
273273
// Three batch events should be fired
274274
apiManager.wg.Add(3)
275-
e.True(em.ProcessEvent("a", "b", Event{}))
275+
e.NoError(em.ProcessEvent("a", "b", Event{}))
276276
// Wait for event fire through go routine
277277
apiManager.wg.Wait()
278278
// Since all events fired successfully, queue should be empty
@@ -293,7 +293,7 @@ func (e *EventManagerTestSuite) TestProcessEventFirstEventFailsWithRetries() {
293293
// Total 2 events in queue which make 2 batches
294294
// first batch will be retried thrice, second one wont be fired since first failed thrice
295295
apiManager.wg.Add(maxRetries)
296-
e.True(em.ProcessEvent("a", "b", Event{}))
296+
e.NoError(em.ProcessEvent("a", "b", Event{}))
297297
// Wait for three retries
298298
apiManager.wg.Wait()
299299
// Since all events failed, queue should contain all events
@@ -311,7 +311,7 @@ func (e *EventManagerTestSuite) TestProcessEventFirstEventFailsWithRetryNotAllow
311311
apiManager.errResponses = []error{tmpError}
312312
// first batch will not be retried, second one wont be fired since first failed
313313
apiManager.wg.Add(1)
314-
e.True(em.ProcessEvent("a", "b", Event{}))
314+
e.NoError(em.ProcessEvent("a", "b", Event{}))
315315
// Wait for three retries
316316
apiManager.wg.Wait()
317317
// Since first batch of 2 events failed with no retry allowed, queue should only contain 1 event
@@ -332,7 +332,7 @@ func (e *EventManagerTestSuite) TestProcessEventSecondEventFailsWithRetriesLater
332332
// Total 2 events in queue which make 2 batches
333333
// first batch will be successfully dispatched, second will be retried thrice
334334
apiManager.wg.Add(4)
335-
e.True(em.ProcessEvent("a", "b", Event{}))
335+
e.NoError(em.ProcessEvent("a", "b", Event{}))
336336
// Wait for events to fire
337337
apiManager.wg.Wait()
338338
// Since second batch of 1 event failed, queue should be contain 1 event
@@ -344,7 +344,7 @@ func (e *EventManagerTestSuite) TestProcessEventSecondEventFailsWithRetriesLater
344344
time.Sleep(200 * time.Millisecond)
345345

346346
apiManager.wg.Add(2)
347-
e.True(em.ProcessEvent("a", "b", Event{}))
347+
e.NoError(em.ProcessEvent("a", "b", Event{}))
348348
// Wait for events to fire
349349
apiManager.wg.Wait()
350350
// Queue should be empty since remaining event was sent now
@@ -366,7 +366,7 @@ func (e *EventManagerTestSuite) TestProcessEventFirstEventPassesWithRetries() {
366366
// Total 2 events in queue which make 2 batches
367367
// first batch will be retried once, second will be successful immediately
368368
apiManager.wg.Add(3)
369-
e.True(em.ProcessEvent("a", "b", Event{}))
369+
e.NoError(em.ProcessEvent("a", "b", Event{}))
370370
// Wait for events to fire
371371
apiManager.wg.Wait()
372372
// Since all events were successful, queue should be empty

‎pkg/odp/odp_manager.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type OMOptionFunc func(em *DefaultOdpManager)
3636
type Manager interface {
3737
FetchQualifiedSegments(userID string, options []segment.OptimizelySegmentOption) (segments []string, err error)
3838
IdentifyUser(userID string)
39-
SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) bool
39+
SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) (err error)
4040
Update(apiKey, apiHost string, segmentsToCheck []string)
4141
}
4242

@@ -145,10 +145,10 @@ func (om *DefaultOdpManager) IdentifyUser(userID string) {
145145
}
146146

147147
// SendOdpEvent sends an event to the ODP server.
148-
func (om *DefaultOdpManager) SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) bool {
148+
func (om *DefaultOdpManager) SendOdpEvent(eventType, action string, identifiers map[string]string, data map[string]interface{}) (err error) {
149149
if !om.enabled {
150150
om.logger.Debug(utils.OdpNotEnabled)
151-
return false
151+
return errors.New(utils.OdpNotEnabled)
152152
}
153153
if identifiers == nil {
154154
identifiers = map[string]string{}

‎pkg/odp/odp_manager_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,12 @@ func (m *MockEventManager) IdentifyUser(apiKey, apiHost, userID string) {
4545
m.Called(apiKey, apiHost, userID)
4646
}
4747

48-
func (m *MockEventManager) ProcessEvent(apiKey, apiHost string, odpEvent event.Event) bool {
49-
return m.Called(apiKey, apiHost, odpEvent).Get(0).(bool)
48+
func (m *MockEventManager) ProcessEvent(apiKey, apiHost string, odpEvent event.Event) error {
49+
err := m.Called(apiKey, apiHost, odpEvent).Get(0)
50+
if err == nil {
51+
return nil
52+
}
53+
return err.(error)
5054
}
5155

5256
func (m *MockEventManager) FlushEvents(apiKey, apiHost string) {
@@ -209,8 +213,8 @@ func (o *ODPManagerTestSuite) TestSendOdpEvent() {
209213
}}
210214
o.config.On("GetAPIKey").Return("")
211215
o.config.On("GetAPIHost").Return("")
212-
o.eventManager.On("ProcessEvent", "", "", userEvent).Return(true)
213-
o.True(o.odpManager.SendOdpEvent(userEvent.Type, userEvent.Action, userEvent.Identifiers, userEvent.Data))
216+
o.eventManager.On("ProcessEvent", "", "", userEvent).Return(nil)
217+
o.NoError(o.odpManager.SendOdpEvent(userEvent.Type, userEvent.Action, userEvent.Identifiers, userEvent.Data))
214218
o.segmentManager.AssertExpectations(o.T())
215219
}
216220

‎pkg/odp/utils/errors.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const IdentityOdpDisabled = "ODP identify event is not dispatched (ODP disabled)
3333
const IdentityOdpNotIntegrated = "ODP identify event is not dispatched (ODP not integrated)"
3434

3535
// OdpNotIntegrated error string when odp is not integrated
36-
const OdpNotIntegrated = "ODP not integrated"
36+
const OdpNotIntegrated = "ODP is not integrated"
3737

3838
// OdpEventFailed error string when odp event failed with provided reason
3939
const OdpEventFailed = "ODP event send failed (%s)"

0 commit comments

Comments
 (0)