Skip to content

Commit c68eea9

Browse files
mikechu-optimizelymsohailhussain
mikechu-optimizely
and
msohailhussain
authored
feat: Fix NotificationCenter Issue for ODPManager (#324)
* PR changes * More PR changes * proposed changes * Understanding & additional updates * WIP Refactors & test fixes * WIP correcting tests... * Refactors * WIP OdpEventManagerTest resolutions * Remove NotificationCenter from NotificationCenterRegistry on Dispose * Fix Disposed = true location * Modify NCR Dispose * Mods to NCR and NC disposal * Mods to NCR and NC disposal * Fix OdpEventManager & tests * Remove double lock in NotificationCenterRegistry * Fix final test corrections * Pass logger into NotificationCenterRegistry * Last PR change request * Remove TODO comments Co-authored-by: msohailhussain <[email protected]>
1 parent 3dde070 commit c68eea9

16 files changed

+461
-276
lines changed

‎OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
<Compile Include="..\OptimizelySDK\IOptimizely.cs">
1111
<Link>IOptimizely.cs</Link>
1212
</Compile>
13+
<Compile Include="..\OptimizelySDK\Notifications\NotificationCenterRegistry.cs">
14+
<Link>Notifications\NotificationCenterRegistry.cs</Link>
15+
</Compile>
1316
<Compile Include="..\OptimizelySDK\Odp\Constants.cs">
1417
<Link>Odp\Constants.cs</Link>
1518
</Compile>
@@ -40,9 +43,6 @@
4043
<Compile Include="..\OptimizelySDK\Odp\Entity\OdpEvent.cs">
4144
<Link>Odp\Entity\OdpEvent.cs</Link>
4245
</Compile>
43-
<Compile Include="..\OptimizelySDK\Odp\Entity\OptimizelySdkSettings.cs">
44-
<Link>Odp\Entity\OptimizelySdkSettings.cs</Link>
45-
</Compile>
4646
<Compile Include="..\OptimizelySDK\Odp\Entity\Response.cs">
4747
<Link>Odp\Entity\Response.cs</Link>
4848
</Compile>

‎OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs‎

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022, Optimizely
2+
* Copyright 2022-2023, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -155,12 +155,14 @@ public void Setup()
155155
[Test]
156156
public void ShouldLogAndDiscardEventsWhenEventManagerNotRunning()
157157
{
158-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
158+
var eventManager = new OdpEventManager.Builder().
159159
WithOdpEventApiManager(_mockApiManager.Object).
160160
WithLogger(_mockLogger.Object).
161-
Build(startImmediately: false);
161+
WithAutoStart(false).
162+
Build();
163+
eventManager.UpdateSettings(_odpConfig);
162164

163-
// since we've not called start() then...
165+
// since we've not called Start() then...
164166
eventManager.SendEvent(_testEvents[0]);
165167

166168
// ...we should get a notice after trying to send an event
@@ -173,11 +175,13 @@ public void ShouldLogAndDiscardEventsWhenEventManagerNotRunning()
173175
public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady()
174176
{
175177
var mockOdpConfig = new Mock<OdpConfig>(API_KEY, API_HOST, _emptySegmentsToCheck);
176-
mockOdpConfig.Setup(o => o.IsReady()).Returns(false);
177-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object).
178+
mockOdpConfig.Setup(o => o.IsReady()).Returns(false); // stay not ready
179+
var eventManager = new OdpEventManager.Builder().
178180
WithOdpEventApiManager(_mockApiManager.Object).
179181
WithLogger(_mockLogger.Object).
180-
Build(startImmediately: false); // doing it manually in Act next
182+
WithAutoStart(false). // start manually in Act
183+
Build();
184+
eventManager.UpdateSettings(mockOdpConfig.Object);
181185

182186
eventManager.Start(); // Log when Start() called
183187
eventManager.SendEvent(_testEvents[0]); // Log when enqueue attempted
@@ -191,30 +195,33 @@ public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady()
191195
public void ShouldLogWhenOdpNotIntegratedAndIdentifyUserCalled()
192196
{
193197
var mockOdpConfig = new Mock<OdpConfig>(API_KEY, API_HOST, _emptySegmentsToCheck);
194-
mockOdpConfig.Setup(o => o.IsReady()).Returns(false);
195-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object).
198+
mockOdpConfig.Setup(o => o.IsReady()).Returns(false); // never ready
199+
var eventManager = new OdpEventManager.Builder().
196200
WithOdpEventApiManager(_mockApiManager.Object).
197201
WithLogger(_mockLogger.Object).
198-
Build();
202+
Build(); // assumed AutoStart true; Logs 1x here
203+
eventManager.UpdateSettings(mockOdpConfig.Object); // auto-start after update; Logs 1x here
199204

200-
eventManager.IdentifyUser(FS_USER_ID);
205+
eventManager.IdentifyUser(FS_USER_ID); // Logs 1x here too
201206

202207
_mockLogger.Verify(
203208
l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE),
204-
Times.Exactly(2)); // during Start() and SendEvent()
209+
Times.Exactly(3)); // during Start() and SendEvent()
205210
}
206211

207212
[Test]
208213
public void ShouldLogWhenOdpNotIntegratedAndStartCalled()
209214
{
210215
var mockOdpConfig = new Mock<OdpConfig>(API_KEY, API_HOST, _emptySegmentsToCheck);
211-
mockOdpConfig.Setup(o => o.IsReady()).Returns(false);
212-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object).
216+
mockOdpConfig.Setup(o => o.IsReady()).Returns(false); // since never ready
217+
var eventManager = new OdpEventManager.Builder().
213218
WithOdpEventApiManager(_mockApiManager.Object).
214219
WithLogger(_mockLogger.Object).
215-
Build(startImmediately: false); // doing it manually in Act next
220+
WithAutoStart(false). // doing it manually in Act next
221+
Build();
222+
eventManager.UpdateSettings(mockOdpConfig.Object);
216223

217-
eventManager.Start();
224+
eventManager.Start(); // Log 1x here too
218225

219226
_mockLogger.Verify(l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE),
220227
Times.Once);
@@ -250,10 +257,11 @@ public void ShouldDiscardEventsWithInvalidData()
250257
"key-3", new DateTime()
251258
},
252259
});
253-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
260+
var eventManager = new OdpEventManager.Builder().
254261
WithOdpEventApiManager(_mockApiManager.Object).
255262
WithLogger(_mockLogger.Object).
256263
Build();
264+
eventManager.UpdateSettings(_odpConfig);
257265

258266
eventManager.SendEvent(eventWithAnArray);
259267
eventManager.SendEvent(eventWithADate);
@@ -271,16 +279,17 @@ public void ShouldAddAdditionalInformationToEachEvent()
271279
_mockApiManager.Setup(api => api.SendEvents(It.IsAny<string>(), It.IsAny<string>(),
272280
Capture.In(eventsCollector))).
273281
Callback(() => cde.Signal());
274-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
282+
var eventManager = new OdpEventManager.Builder().
275283
WithOdpEventApiManager(_mockApiManager.Object).
276284
WithLogger(_mockLogger.Object).
277285
WithEventQueue(new BlockingCollection<object>(10)). // max capacity of 10
278286
WithBatchSize(10).
279287
WithFlushInterval(TimeSpan.FromMilliseconds(100)).
280288
Build();
289+
eventManager.UpdateSettings(_odpConfig);
281290

282291
eventManager.SendEvent(_testEvents[0]);
283-
cde.Wait();
292+
cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS);
284293

285294
var eventsSentToApi = eventsCollector.FirstOrDefault();
286295
var actualEvent = eventsSentToApi?.FirstOrDefault();
@@ -304,13 +313,14 @@ public void ShouldAddAdditionalInformationToEachEvent()
304313
[Test]
305314
public void ShouldAttemptToFlushAnEmptyQueueAtFlushInterval()
306315
{
307-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
316+
var eventManager = new OdpEventManager.Builder().
308317
WithOdpEventApiManager(_mockApiManager.Object).
309318
WithLogger(_mockLogger.Object).
310319
WithEventQueue(new BlockingCollection<object>(10)).
311320
WithBatchSize(10).
312321
WithFlushInterval(TimeSpan.FromMilliseconds(100)).
313322
Build();
323+
eventManager.UpdateSettings(_odpConfig);
314324

315325
// do not add events to the queue, but allow for
316326
// at least 3 flush intervals executions
@@ -328,13 +338,14 @@ public void ShouldDispatchEventsInCorrectNumberOfBatches()
328338
a.SendEvents(It.IsAny<string>(), It.IsAny<string>(),
329339
It.IsAny<List<OdpEvent>>())).
330340
Returns(false);
331-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
341+
var eventManager = new OdpEventManager.Builder().
332342
WithOdpEventApiManager(_mockApiManager.Object).
333343
WithLogger(_mockLogger.Object).
334344
WithEventQueue(new BlockingCollection<object>(10)).
335345
WithBatchSize(10).
336346
WithFlushInterval(TimeSpan.FromMilliseconds(500)).
337347
Build();
348+
eventManager.UpdateSettings(_odpConfig);
338349

339350
for (int i = 0; i < 25; i++)
340351
{
@@ -362,12 +373,13 @@ public void ShouldDispatchEventsWithCorrectPayload()
362373
Capture.In(eventCollector))).
363374
Callback(() => cde.Signal()).
364375
Returns(false);
365-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
376+
var eventManager = new OdpEventManager.Builder().
366377
WithOdpEventApiManager(_mockApiManager.Object).
367378
WithLogger(_mockLogger.Object).
368379
WithBatchSize(10).
369380
WithFlushInterval(TimeSpan.FromSeconds(1)).
370381
Build();
382+
eventManager.UpdateSettings(_odpConfig);
371383

372384
_testEvents.ForEach(e => eventManager.SendEvent(e));
373385
cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS);
@@ -395,13 +407,14 @@ public void ShouldRetryFailedEvents()
395407
It.IsAny<List<OdpEvent>>())).
396408
Callback(() => cde.Signal()).
397409
Returns(true);
398-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
410+
var eventManager = new OdpEventManager.Builder().
399411
WithOdpEventApiManager(_mockApiManager.Object).
400412
WithLogger(_mockLogger.Object).
401413
WithEventQueue(new BlockingCollection<object>(10)).
402414
WithBatchSize(2).
403415
WithFlushInterval(TimeSpan.FromMilliseconds(100)).
404416
Build();
417+
eventManager.UpdateSettings(_odpConfig);
405418

406419
for (int i = 0; i < 4; i++)
407420
{
@@ -419,13 +432,14 @@ public void ShouldRetryFailedEvents()
419432
[Test]
420433
public void ShouldFlushAllScheduledEventsBeforeStopping()
421434
{
422-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
435+
var eventManager = new OdpEventManager.Builder().
423436
WithOdpEventApiManager(_mockApiManager.Object).
424437
WithLogger(_mockLogger.Object).
425438
WithEventQueue(new BlockingCollection<object>(100)).
426439
WithBatchSize(2). // small batch size
427440
WithFlushInterval(TimeSpan.FromSeconds(2)). // long flush interval
428441
Build();
442+
eventManager.UpdateSettings(_odpConfig);
429443

430444
for (int i = 0; i < 25; i++)
431445
{
@@ -453,12 +467,13 @@ public void ShouldPrepareCorrectPayloadForIdentifyUser()
453467
_mockApiManager.Setup(api => api.SendEvents(It.IsAny<string>(), It.IsAny<string>(),
454468
Capture.In(eventsCollector))).
455469
Callback(() => cde.Signal());
456-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
470+
var eventManager = new OdpEventManager.Builder().
457471
WithOdpEventApiManager(_mockApiManager.Object).
458472
WithLogger(_mockLogger.Object).
459473
WithEventQueue(new BlockingCollection<object>(1)).
460474
WithBatchSize(1).
461475
Build();
476+
eventManager.UpdateSettings(_odpConfig);
462477

463478
eventManager.IdentifyUser(USER_ID);
464479
cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS);
@@ -488,10 +503,11 @@ public void ShouldApplyUpdatedOdpConfigurationWhenAvailable()
488503
"1-item-cart",
489504
};
490505
var differentOdpConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck);
491-
var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig).
506+
var eventManager = new OdpEventManager.Builder().
492507
WithOdpEventApiManager(_mockApiManager.Object).
493508
WithLogger(_mockLogger.Object).
494509
Build();
510+
eventManager.UpdateSettings(_odpConfig);
495511

496512
eventManager.UpdateSettings(differentOdpConfig);
497513

0 commit comments

Comments
 (0)