From dd028bf49609c8ae38dcae12840e69c82ae60f6f Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Nov 2022 14:30:03 -0500 Subject: [PATCH 01/18] Initial ODP Manager --- OptimizelySDK/Odp/OdpManager.cs | 62 ++++++++++++++++++++++++++ OptimizelySDK/Odp/OdpSegmentManager.cs | 10 +++++ OptimizelySDK/OptimizelySDK.csproj | 2 +- 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 OptimizelySDK/Odp/OdpManager.cs diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs new file mode 100644 index 00000000..adbde889 --- /dev/null +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -0,0 +1,62 @@ +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; + +namespace OptimizelySDK.Odp +{ + public class OdpManager + { + private volatile OdpConfig _odpConfig; + + public OdpSegmentManager SegmentManager { get; } + + public OdpEventManager EventManager { get; } + + public OdpManager(OdpConfig odpConfig, OdpSegmentManager segmentManager, + OdpEventManager eventManager + ) + { + _odpConfig = odpConfig; + SegmentManager = segmentManager; + EventManager = eventManager; + + EventManager.Start(); + } + + public bool UpdateSettings(string apiHost, string apiKey, List segmentsToCheck) + { + var newConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); + if (_odpConfig.Equals(newConfig)) + { + return false; + } + + _odpConfig = newConfig; + + EventManager.UpdateSettings(_odpConfig); + + SegmentManager.ResetCache(); + SegmentManager.UpdateSettings(_odpConfig); + + return true; + } + + public void Close() { + EventManager.Stop(); + } + } +} diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index e6bead9d..655d7a9d 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -142,5 +142,15 @@ private static string GetCacheKey(string userKey, string userValue) { return $"{userKey}-$-{userValue}"; } + + public void UpdateSettings(OdpConfig odpConfig) + { + _odpConfig.Update(odpConfig.ApiKey, odpConfig.ApiHost, odpConfig.SegmentsToCheck); + } + + public void ResetCache() + { + _segmentsCache.Reset(); + } } } diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index c67269d2..372384c7 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -102,7 +102,7 @@ - + From a317407280e55174217190a20b342a367336f812 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Nov 2022 17:15:24 -0500 Subject: [PATCH 02/18] Outline unit tests --- .../OdpTests/OdpManagerTest.cs | 83 +++++++++++++++++++ .../OptimizelySDK.Tests.csproj | 1 + OptimizelySDK/Odp/OdpManager.cs | 17 ++-- 3 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs new file mode 100644 index 00000000..6ba06576 --- /dev/null +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -0,0 +1,83 @@ +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using Castle.Core.Logging; +using Moq; +using NUnit.Framework; +using OptimizelySDK.Odp; + +namespace OptimizelySDK.Tests.OdpTests +{ + [TestFixture] + public class OdpManagerTest + { + private OdpConfig _odpConfig; + private Mock _mockLogger; + private Mock _mockOdpEventManager; + private Mock _mockSegmentApiManager; + + [SetUp] + public void Setup() { } + + [Test] + public void ShouldStartEventManagerWhenOdpManagerIsInitialized() { } + + [Test] + public void ShouldStopEventManagerWhenCloseIsCalled() { } + + [Test] + public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() { } + + [Test] + public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() { } + + [Test] + public void ShouldHandleSettingsNoChange() { } + + [Test] + public void ShouldUpdateSettingsWithReset() { } + + [Test] + public void ShouldGetEventManager() { } + + [Test] + public void ShouldGetSegmentManager() { } + + [Test] + public void ShouldFetchQualifiedSegments() { } + + [Test] + public void ShouldDisableOdpThroughConfiguration() { } + + [Test] + public void ShouldIdentifyUserWhenDatafileNotReady() { } + + [Test] + public void ShouldIdentifyUserWhenOdpIsIntegrated() { } + + [Test] + public void ShouldNotIdentifyUserWhenOdpNotIntegrated() { } + + [Test] + public void ShouldNotIdentifyUserWhenOdpDisabled() { } + + [Test] + public void ShouldSendEventWhenOdpIsIntegrated() { } + + [Test] + public void ShouldNotSendEventOdpNotIntegrated() { } + } +} diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index 867c217c..a71d34f3 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -80,6 +80,7 @@ + diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index adbde889..9a6ae338 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -14,6 +14,7 @@ * limitations under the License. */ +using OptimizelySDK.Logger; using System.Collections.Generic; namespace OptimizelySDK.Odp @@ -26,13 +27,16 @@ public class OdpManager public OdpEventManager EventManager { get; } + private ILogger _logger; + public OdpManager(OdpConfig odpConfig, OdpSegmentManager segmentManager, - OdpEventManager eventManager + OdpEventManager eventManager, ILogger logger = null ) { _odpConfig = odpConfig; SegmentManager = segmentManager; EventManager = eventManager; + _logger = logger; EventManager.Start(); } @@ -46,16 +50,17 @@ public bool UpdateSettings(string apiHost, string apiKey, List segmentsT } _odpConfig = newConfig; - + EventManager.UpdateSettings(_odpConfig); - + SegmentManager.ResetCache(); SegmentManager.UpdateSettings(_odpConfig); - + return true; } - - public void Close() { + + public void Close() + { EventManager.Stop(); } } From 443164c0d60e6e46dd921ee75dc84eb6c5079a08 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 28 Nov 2022 11:16:27 -0500 Subject: [PATCH 03/18] WIP Unit tests + changes to CUD --- .../OdpTests/OdpManagerTest.cs | 121 +++++++++++++++--- OptimizelySDK/Odp/IOdpEventManager.cs | 10 ++ OptimizelySDK/Odp/IOdpSegmentManager.cs | 4 + OptimizelySDK/Odp/OdpConfig.cs | 9 ++ OptimizelySDK/Odp/OdpManager.cs | 12 +- 5 files changed, 131 insertions(+), 25 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 6ba06576..579ffc94 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -14,70 +14,153 @@ * limitations under the License. */ -using Castle.Core.Logging; using Moq; using NUnit.Framework; +using OptimizelySDK.Logger; using OptimizelySDK.Odp; +using System.Collections.Generic; +using System.Linq; namespace OptimizelySDK.Tests.OdpTests { [TestFixture] public class OdpManagerTest { + private const string API_KEY = "JUs7AFak3aP1K3y"; + private const string API_HOST = "https://odp-api.example.com"; + private const string UPDATED_API_KEY = "D1fF3rEn7kEy"; + private const string UPDATED_ODP_ENDPOINT = "https://an-updated-odp-endpoint.example.com"; + + private readonly List _updatedSegmentsToCheck = new List + { + "updated-segment-1", + "updated-segment-2", + }; + + private readonly List _emptySegmentsToCheck = new List(0); + private OdpConfig _odpConfig; private Mock _mockLogger; private Mock _mockOdpEventManager; - private Mock _mockSegmentApiManager; + private Mock _mockSegmentManager; [SetUp] - public void Setup() { } + public void Setup() + { + _odpConfig = new OdpConfig(API_KEY, API_HOST, _emptySegmentsToCheck); + _mockLogger = new Mock(); + _mockOdpEventManager = new Mock(); + _mockSegmentManager = new Mock(); + } [Test] - public void ShouldStartEventManagerWhenOdpManagerIsInitialized() { } + public void ShouldStartEventManagerWhenOdpManagerIsInitialized() + { + _mockOdpEventManager.Setup(e => e.Start()); - [Test] - public void ShouldStopEventManagerWhenCloseIsCalled() { } + _ = new OdpManager(_odpConfig, _mockSegmentManager.Object, + _mockOdpEventManager.Object, _mockLogger.Object); + + _mockOdpEventManager.Verify(e => e.Start(), Times.Once); + } [Test] - public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() { } + public void ShouldStopEventManagerWhenCloseIsCalled() + { + _mockOdpEventManager.Setup(e => e.Stop()); + var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, + _mockOdpEventManager.Object, _mockLogger.Object); + + manager.Close(); + + _mockOdpEventManager.Verify(e => e.Stop(), Times.Once); + } [Test] - public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() { } + public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() + { + var eventManagerParameterCollector = new List(); + _mockOdpEventManager.Setup(e => + e.UpdateSettings(Capture.In(eventManagerParameterCollector))); + var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, + _mockOdpEventManager.Object, _mockLogger.Object); + + var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, + _updatedSegmentsToCheck); + + Assert.IsTrue(wasUpdated); + var configPassedToOdpEventManager = eventManagerParameterCollector.FirstOrDefault(); + Assert.AreEqual(UPDATED_API_KEY, configPassedToOdpEventManager?.ApiKey); + Assert.AreEqual(UPDATED_ODP_ENDPOINT, configPassedToOdpEventManager.ApiHost); + Assert.AreEqual(_updatedSegmentsToCheck, configPassedToOdpEventManager.SegmentsToCheck); + } [Test] - public void ShouldHandleSettingsNoChange() { } + public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() + { + var segmentManagerParameterCollector = new List(); + _mockSegmentManager.Setup(s => + s.UpdateSettings(Capture.In(segmentManagerParameterCollector))); + var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, + _mockOdpEventManager.Object, _mockLogger.Object); + + var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, + _updatedSegmentsToCheck); + + Assert.IsTrue(wasUpdated); + var configPassedToSegmentManager = segmentManagerParameterCollector.FirstOrDefault(); + Assert.AreEqual(UPDATED_API_KEY, configPassedToSegmentManager?.ApiKey); + Assert.AreEqual(UPDATED_ODP_ENDPOINT, configPassedToSegmentManager?.ApiHost); + Assert.AreEqual(_updatedSegmentsToCheck, configPassedToSegmentManager.SegmentsToCheck); + } [Test] + public void ShouldHandleOdpConfigSettingsNoChange() + { + _mockSegmentManager.Setup(s => s.UpdateSettings(It.IsAny())); + _mockOdpEventManager.Setup(e => e.UpdateSettings(It.IsAny())); + var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, + _mockOdpEventManager.Object, _mockLogger.Object); + + var wasUpdated = manager.UpdateSettings(_odpConfig.ApiKey, _odpConfig.ApiHost, + _odpConfig.SegmentsToCheck); + + Assert.IsFalse(wasUpdated); + _mockSegmentManager.Verify(s => s.UpdateSettings(It.IsAny()), Times.Never); + _mockOdpEventManager.Verify(e => e.UpdateSettings(It.IsAny()), Times.Never); + } + + [Ignore, Test] public void ShouldUpdateSettingsWithReset() { } - [Test] + [Ignore, Test] public void ShouldGetEventManager() { } - [Test] + [Ignore, Test] public void ShouldGetSegmentManager() { } - [Test] + [Ignore, Test] public void ShouldFetchQualifiedSegments() { } - [Test] + [Ignore, Test] public void ShouldDisableOdpThroughConfiguration() { } - [Test] + [Ignore, Test] public void ShouldIdentifyUserWhenDatafileNotReady() { } - [Test] + [Ignore, Test] public void ShouldIdentifyUserWhenOdpIsIntegrated() { } - [Test] + [Ignore, Test] public void ShouldNotIdentifyUserWhenOdpNotIntegrated() { } - [Test] + [Ignore, Test] public void ShouldNotIdentifyUserWhenOdpDisabled() { } - [Test] + [Ignore, Test] public void ShouldSendEventWhenOdpIsIntegrated() { } - [Test] + [Ignore, Test] public void ShouldNotSendEventOdpNotIntegrated() { } } } diff --git a/OptimizelySDK/Odp/IOdpEventManager.cs b/OptimizelySDK/Odp/IOdpEventManager.cs index dabf7a99..f2cf96fb 100644 --- a/OptimizelySDK/Odp/IOdpEventManager.cs +++ b/OptimizelySDK/Odp/IOdpEventManager.cs @@ -61,5 +61,15 @@ public interface IOdpEventManager /// /// Configuration object containing new values void UpdateSettings(OdpConfig odpConfig); + + /// + /// Indicates the ODP Event Manager has been stopped and disposed + /// + bool Disposed { get; } + + /// + /// Indicates the ODP Event Manager instance is in a running state + /// + bool IsStarted { get; } } } diff --git a/OptimizelySDK/Odp/IOdpSegmentManager.cs b/OptimizelySDK/Odp/IOdpSegmentManager.cs index 0a316e24..863b5e6e 100644 --- a/OptimizelySDK/Odp/IOdpSegmentManager.cs +++ b/OptimizelySDK/Odp/IOdpSegmentManager.cs @@ -31,5 +31,9 @@ public interface IOdpSegmentManager /// An array of OptimizelySegmentOption used to ignore and/or reset the cache. /// Qualified segments for the user from the cache or the ODP server if the cache is empty. List FetchQualifiedSegments(string fsUserId, List options = null); + + void UpdateSettings(OdpConfig odpConfig); + + void ResetCache(); } } diff --git a/OptimizelySDK/Odp/OdpConfig.cs b/OptimizelySDK/Odp/OdpConfig.cs index 30127033..2f87c548 100644 --- a/OptimizelySDK/Odp/OdpConfig.cs +++ b/OptimizelySDK/Odp/OdpConfig.cs @@ -14,6 +14,7 @@ * limitations under the License. */ +using System; using System.Collections.Generic; namespace OptimizelySDK.Odp @@ -117,5 +118,13 @@ public bool HasSegments() { return SegmentsToCheck?.Count > 0; } + + public bool Equals(OdpConfig toCompare) + { + return ApiKey.Equals(toCompare.ApiKey, StringComparison.OrdinalIgnoreCase) && + ApiHost.Equals(toCompare.ApiHost, StringComparison.OrdinalIgnoreCase) && + SegmentsToCheck.TrueForAll( + segment => toCompare.SegmentsToCheck.Contains(segment)); + } } } diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 9a6ae338..bfa03bb5 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -23,25 +23,25 @@ public class OdpManager { private volatile OdpConfig _odpConfig; - public OdpSegmentManager SegmentManager { get; } + public IOdpSegmentManager SegmentManager { get; } - public OdpEventManager EventManager { get; } + public IOdpEventManager EventManager { get; } private ILogger _logger; - public OdpManager(OdpConfig odpConfig, OdpSegmentManager segmentManager, - OdpEventManager eventManager, ILogger logger = null + public OdpManager(OdpConfig odpConfig, IOdpSegmentManager segmentManager, + IOdpEventManager eventManager, ILogger logger = null ) { _odpConfig = odpConfig; SegmentManager = segmentManager; EventManager = eventManager; - _logger = logger; + _logger = logger ?? new NoOpLogger(); EventManager.Start(); } - public bool UpdateSettings(string apiHost, string apiKey, List segmentsToCheck) + public bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck) { var newConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); if (_odpConfig.Equals(newConfig)) From faea784e7cb5e5d84877121ba7a6445d11f4a02d Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 28 Nov 2022 15:32:09 -0500 Subject: [PATCH 04/18] Implement fetch, identify, and send events with Builder pattern --- .../OdpTests/OdpManagerTest.cs | 83 ++++++-- OptimizelySDK/Odp/IOdpManager.cs | 35 ++++ OptimizelySDK/Odp/OdpConfig.cs | 5 + OptimizelySDK/Odp/OdpManager.cs | 192 ++++++++++++++++-- OptimizelySDK/OptimizelySDK.csproj | 1 + 5 files changed, 282 insertions(+), 34 deletions(-) create mode 100644 OptimizelySDK/Odp/IOdpManager.cs diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 579ffc94..46fde0e2 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -58,8 +58,11 @@ public void ShouldStartEventManagerWhenOdpManagerIsInitialized() { _mockOdpEventManager.Setup(e => e.Start()); - _ = new OdpManager(_odpConfig, _mockSegmentManager.Object, - _mockOdpEventManager.Object, _mockLogger.Object); + _ = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); _mockOdpEventManager.Verify(e => e.Start(), Times.Once); } @@ -68,8 +71,11 @@ public void ShouldStartEventManagerWhenOdpManagerIsInitialized() public void ShouldStopEventManagerWhenCloseIsCalled() { _mockOdpEventManager.Setup(e => e.Stop()); - var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, - _mockOdpEventManager.Object, _mockLogger.Object); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); manager.Close(); @@ -82,8 +88,11 @@ public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() var eventManagerParameterCollector = new List(); _mockOdpEventManager.Setup(e => e.UpdateSettings(Capture.In(eventManagerParameterCollector))); - var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, - _mockOdpEventManager.Object, _mockLogger.Object); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -101,8 +110,11 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() var segmentManagerParameterCollector = new List(); _mockSegmentManager.Setup(s => s.UpdateSettings(Capture.In(segmentManagerParameterCollector))); - var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, - _mockOdpEventManager.Object, _mockLogger.Object); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -110,7 +122,7 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() Assert.IsTrue(wasUpdated); var configPassedToSegmentManager = segmentManagerParameterCollector.FirstOrDefault(); Assert.AreEqual(UPDATED_API_KEY, configPassedToSegmentManager?.ApiKey); - Assert.AreEqual(UPDATED_ODP_ENDPOINT, configPassedToSegmentManager?.ApiHost); + Assert.AreEqual(UPDATED_ODP_ENDPOINT, configPassedToSegmentManager.ApiHost); Assert.AreEqual(_updatedSegmentsToCheck, configPassedToSegmentManager.SegmentsToCheck); } @@ -119,8 +131,11 @@ public void ShouldHandleOdpConfigSettingsNoChange() { _mockSegmentManager.Setup(s => s.UpdateSettings(It.IsAny())); _mockOdpEventManager.Setup(e => e.UpdateSettings(It.IsAny())); - var manager = new OdpManager(_odpConfig, _mockSegmentManager.Object, - _mockOdpEventManager.Object, _mockLogger.Object); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); var wasUpdated = manager.UpdateSettings(_odpConfig.ApiKey, _odpConfig.ApiHost, _odpConfig.SegmentsToCheck); @@ -130,17 +145,47 @@ public void ShouldHandleOdpConfigSettingsNoChange() _mockOdpEventManager.Verify(e => e.UpdateSettings(It.IsAny()), Times.Never); } - [Ignore, Test] - public void ShouldUpdateSettingsWithReset() { } + [Test] + public void ShouldUpdateSettingsWithReset() + { + _mockSegmentManager.Setup(s => + s.UpdateSettings(It.IsAny())); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); - [Ignore, Test] - public void ShouldGetEventManager() { } + var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, + _updatedSegmentsToCheck); - [Ignore, Test] - public void ShouldGetSegmentManager() { } + Assert.IsTrue(wasUpdated); + _mockSegmentManager.Verify(s => s.ResetCache(), Times.Once); + } - [Ignore, Test] - public void ShouldFetchQualifiedSegments() { } + [Test] + public void ShouldGetEventManager() + { + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); + + Assert.IsNotNull(manager.EventManager); + } + + [Test] + public void ShouldGetSegmentManager() + { + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithSegmentManager(_mockSegmentManager.Object). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); + + Assert.IsNotNull(manager.SegmentManager); + } [Ignore, Test] public void ShouldDisableOdpThroughConfiguration() { } diff --git a/OptimizelySDK/Odp/IOdpManager.cs b/OptimizelySDK/Odp/IOdpManager.cs new file mode 100644 index 00000000..22d0beda --- /dev/null +++ b/OptimizelySDK/Odp/IOdpManager.cs @@ -0,0 +1,35 @@ +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; + +namespace OptimizelySDK.Odp +{ + public interface IOdpManager + { + bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck); + + List FetchQualifiedSegments(string userId, List options); + + void IdentifyUser(string userId); + + void SendEvent(string type, string action, Dictionary identifiers, + Dictionary data + ); + + void Close(); + } +} diff --git a/OptimizelySDK/Odp/OdpConfig.cs b/OptimizelySDK/Odp/OdpConfig.cs index 2f87c548..99750e73 100644 --- a/OptimizelySDK/Odp/OdpConfig.cs +++ b/OptimizelySDK/Odp/OdpConfig.cs @@ -119,6 +119,11 @@ public bool HasSegments() return SegmentsToCheck?.Count > 0; } + /// + /// Determine equality between two OdpConfig objects + /// + /// OdpConfig object to compare current instance against + /// True if equal otherwise False public bool Equals(OdpConfig toCompare) { return ApiKey.Equals(toCompare.ApiKey, StringComparison.OrdinalIgnoreCase) && diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index bfa03bb5..0627943b 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -14,33 +14,26 @@ * limitations under the License. */ +using OptimizelySDK.ErrorHandler; using OptimizelySDK.Logger; +using OptimizelySDK.Odp.Entity; +using System; using System.Collections.Generic; namespace OptimizelySDK.Odp { - public class OdpManager + public class OdpManager : IOdpManager { + private bool _enabled; + private volatile OdpConfig _odpConfig; - public IOdpSegmentManager SegmentManager { get; } + public IOdpSegmentManager SegmentManager { get; private set; } - public IOdpEventManager EventManager { get; } + public IOdpEventManager EventManager { get; private set; } private ILogger _logger; - public OdpManager(OdpConfig odpConfig, IOdpSegmentManager segmentManager, - IOdpEventManager eventManager, ILogger logger = null - ) - { - _odpConfig = odpConfig; - SegmentManager = segmentManager; - EventManager = eventManager; - _logger = logger ?? new NoOpLogger(); - - EventManager.Start(); - } - public bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck) { var newConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); @@ -51,6 +44,7 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT _odpConfig = newConfig; + EventManager.Flush(); EventManager.UpdateSettings(_odpConfig); SegmentManager.ResetCache(); @@ -59,9 +53,177 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT return true; } + public List FetchQualifiedSegments(string userId, List options) + { + if (!_enabled || SegmentManager == null) + { + _logger.Log(LogLevel.ERROR, Constants.ODP_NOT_ENABLED_MESSAGE); + return null; + } + + return SegmentManager.FetchQualifiedSegments(userId, options); + } + + public void IdentifyUser(string userId) + { + if (!_enabled || EventManager == null) + { + _logger.Log(LogLevel.DEBUG, "ODP identify event not dispatched (ODP disabled)."); + return; + } + + if (!EventManager.IsStarted) + { + _logger.Log(LogLevel.DEBUG, + "ODP identify event not dispatched (ODP not integrated)."); + return; + } + + EventManager.IdentifyUser(userId); + } + + public void SendEvent(string type, string action, Dictionary identifiers, + Dictionary data + ) + { + if (!_enabled || EventManager == null) + { + _logger.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."); + return; + } + + if (!EventManager.IsStarted) + { + _logger.Log(LogLevel.DEBUG, + "ODP event not dispatched (ODP not integrated)."); + return; + } + + EventManager.SendEvent(new OdpEvent(type, action, identifiers, data)); + } + public void Close() { + if (!_enabled || EventManager == null) + { + return; + } + EventManager.Stop(); } + + /// + /// Builder pattern to create an instances of OdpManager + /// + public class Builder + { + private OdpConfig _odpConfig; + private IOdpEventManager _eventManager; + private IOdpSegmentManager _segmentManager; + private int _cacheSize; + private int _cacheTimeoutSeconds; + private ILogger _logger; + private IErrorHandler _errorHandler; + private ICache> _cache; + + public Builder WithSegmentManager(IOdpSegmentManager segmentManager) + { + _segmentManager = segmentManager; + return this; + } + + public Builder WithEventManager(IOdpEventManager eventManager) + { + _eventManager = eventManager; + return this; + } + + public Builder WithOdpConfig(OdpConfig odpConfig) + { + _odpConfig = odpConfig; + return this; + } + + public Builder WithCacheSize(int cacheSize) + { + _cacheSize = cacheSize; + return this; + } + + public Builder WithCacheTimeout(int seconds) + { + _cacheTimeoutSeconds = seconds; + return this; + } + + public Builder WithLogger(ILogger logger = null) + { + _logger = logger; + return this; + } + + public Builder WithErrorHandler(IErrorHandler errorHandler = null) + { + _errorHandler = errorHandler; + return this; + } + + public Builder WithCacheImplementation(ICache> cache) + { + _cache = cache; + return this; + } + + /// + /// Build OdpManager instance using collected parameters + /// + /// Should mark as enabled upon initialization + /// OdpManager instance + public OdpManager Build(bool asEnabled = true) + { + var manager = new OdpManager + { + _odpConfig = _odpConfig, + _logger = _logger ?? new NoOpLogger(), + _enabled = asEnabled, + }; + + _errorHandler = _errorHandler ?? new NoOpErrorHandler(); + + if (_eventManager == null) + { + var eventApiManager = new OdpEventApiManager(_logger, _errorHandler); + + manager.EventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + WithOdpEventApiManager(eventApiManager). + WithLogger(_logger). + WithErrorHandler(_errorHandler). + Build(); + } + else + { + manager.EventManager = _eventManager; + } + + if (_segmentManager == null) + { + var cacheTimeout = TimeSpan.FromSeconds(_cacheTimeoutSeconds <= 0 ? + Constants.DEFAULT_CACHE_SECONDS : + _cacheTimeoutSeconds); + var apiManager = new OdpSegmentApiManager(_logger, _errorHandler); + + manager.SegmentManager = new OdpSegmentManager(_odpConfig, apiManager, + _cacheSize, cacheTimeout, _logger, _cache); + } + else + { + manager.SegmentManager = _segmentManager; + } + + manager.EventManager.Start(); + + return manager; + } + } } } diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 372384c7..b667a2fb 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -101,6 +101,7 @@ + From 191a6ec594fb400eeacd1bd8da528874fa5ba17a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 28 Nov 2022 17:02:08 -0500 Subject: [PATCH 05/18] WIP Fixing final unit test --- .../OdpTests/OdpManagerTest.cs | 156 ++++++++++++++++-- 1 file changed, 142 insertions(+), 14 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 46fde0e2..db9e3f59 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -18,6 +18,7 @@ using NUnit.Framework; using OptimizelySDK.Logger; using OptimizelySDK.Odp; +using OptimizelySDK.Odp.Entity; using System.Collections.Generic; using System.Linq; @@ -30,6 +31,9 @@ public class OdpManagerTest private const string API_HOST = "https://odp-api.example.com"; private const string UPDATED_API_KEY = "D1fF3rEn7kEy"; private const string UPDATED_ODP_ENDPOINT = "https://an-updated-odp-endpoint.example.com"; + private const string TEST_EVENT_TYPE = "event-type"; + private const string TEST_EVENT_ACTION = "event-action"; + private const string VALID_FS_USER_ID = "valid-test-fs-user-id"; private readonly List _updatedSegmentsToCheck = new List { @@ -37,6 +41,30 @@ public class OdpManagerTest "updated-segment-2", }; + private readonly Dictionary _testEventIdentifiers = + new Dictionary + { + { + "fs_user_id", "id-key-1" + }, + }; + + private readonly Dictionary _testEventData = new Dictionary + { + { + "key-1", "value-1" + }, + { + "key-2", null + }, + { + "key-3", 3.3 + }, + { + "key-4", true + }, + }; + private readonly List _emptySegmentsToCheck = new List(0); private OdpConfig _odpConfig; @@ -163,6 +191,39 @@ public void ShouldUpdateSettingsWithReset() _mockSegmentManager.Verify(s => s.ResetCache(), Times.Once); } + [Test] + public void ShouldDisableOdpThroughConfiguration() + { + _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); + _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); + + manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, + _testEventData); + + _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Never); + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), Times.Never); + + manager.UpdateSettings(string.Empty, string.Empty, _emptySegmentsToCheck); + + manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, + _testEventData); + manager.Close(); + + // still only once + _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), Times.Never); + } + [Test] public void ShouldGetEventManager() { @@ -187,25 +248,92 @@ public void ShouldGetSegmentManager() Assert.IsNotNull(manager.SegmentManager); } - [Ignore, Test] - public void ShouldDisableOdpThroughConfiguration() { } + [Test] + public void ShouldIdentifyUserWhenOdpIsIntegrated() + { + _mockOdpEventManager.Setup(e => e.IdentifyUser(It.IsAny())); + _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); - [Ignore, Test] - public void ShouldIdentifyUserWhenDatafileNotReady() { } + manager.IdentifyUser(VALID_FS_USER_ID); + manager.Close(); - [Ignore, Test] - public void ShouldIdentifyUserWhenOdpIsIntegrated() { } + _mockLogger.Verify(l => l.Log(It.IsAny(), It.IsAny()), Times.Never); + _mockOdpEventManager.Verify(e => e.IdentifyUser(It.IsAny()), Times.Once); + } - [Ignore, Test] - public void ShouldNotIdentifyUserWhenOdpNotIntegrated() { } + [Test] + public void ShouldNotIdentifyUserWhenOdpNotIntegrated() + { + _mockOdpEventManager.Setup(e => e.IdentifyUser(It.IsAny())); + _mockOdpEventManager.Setup(e => e.IsStarted).Returns(false); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); - [Ignore, Test] - public void ShouldNotIdentifyUserWhenOdpDisabled() { } + manager.IdentifyUser(VALID_FS_USER_ID); + manager.Close(); - [Ignore, Test] - public void ShouldSendEventWhenOdpIsIntegrated() { } + _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, + "ODP identify event not dispatched (ODP not integrated).")); + _mockOdpEventManager.Verify(e => e.IdentifyUser(It.IsAny()), Times.Never); + } - [Ignore, Test] - public void ShouldNotSendEventOdpNotIntegrated() { } + [Test] + public void ShouldNotIdentifyUserWhenOdpDisabled() + { + _mockOdpEventManager.Setup(e => e.IdentifyUser(It.IsAny())); + _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(false); + + manager.IdentifyUser(VALID_FS_USER_ID); + manager.Close(); + + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP identify event not dispatched (ODP disabled).")); + _mockOdpEventManager.Verify(e => e.IdentifyUser(It.IsAny()), Times.Never); + } + + [Test] + public void ShouldSendEventWhenOdpIsIntegrated() + { + _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); + _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); + var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + WithEventManager(_mockOdpEventManager.Object). + WithLogger(_mockLogger.Object). + Build(); + + manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, + _testEventData); + manager.Close(); + + _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); + } + + [Test] + public void ShouldNotSendEventOdpNotIntegrated() + { + var odpConfig = new OdpConfig(string.Empty, string.Empty, _emptySegmentsToCheck); + _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); + var manager = new OdpManager.Builder().WithOdpConfig(odpConfig). + WithLogger(_mockLogger.Object). + Build(false); // do not enable + + manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, + _testEventData); + manager.Close(); + + _mockLogger.Verify(l => + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); + _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Never); + } } } From f6276bc9a88aaed5c8e45ea740737af9aae3057a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 29 Nov 2022 14:22:40 -0500 Subject: [PATCH 06/18] Finish unit tests --- OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs | 6 ++++-- OptimizelySDK/Odp/OdpManager.cs | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index db9e3f59..7fcfeef5 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -210,14 +210,16 @@ public void ShouldDisableOdpThroughConfiguration() _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), Times.Never); + _mockOdpEventManager.ResetCalls(); + _mockLogger.ResetCalls(); + manager.UpdateSettings(string.Empty, string.Empty, _emptySegmentsToCheck); manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); manager.Close(); - // still only once - _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); + _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Never); _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); _mockLogger.Verify(l => diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 0627943b..57d7cb7d 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -55,7 +55,7 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT public List FetchQualifiedSegments(string userId, List options) { - if (!_enabled || SegmentManager == null) + if (!_enabled || SegmentManager == null || !_odpConfig.IsReady()) { _logger.Log(LogLevel.ERROR, Constants.ODP_NOT_ENABLED_MESSAGE); return null; @@ -66,7 +66,7 @@ public List FetchQualifiedSegments(string userId, List public void IdentifyUser(string userId) { - if (!_enabled || EventManager == null) + if (!_enabled || EventManager == null || !_odpConfig.IsReady()) { _logger.Log(LogLevel.DEBUG, "ODP identify event not dispatched (ODP disabled)."); return; @@ -86,7 +86,7 @@ public void SendEvent(string type, string action, Dictionary ide Dictionary data ) { - if (!_enabled || EventManager == null) + if (!_enabled || EventManager == null || !_odpConfig.IsReady()) { _logger.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."); return; From 4f629fad36044438eef70274444faf13afeb0b01 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 29 Nov 2022 14:48:14 -0500 Subject: [PATCH 07/18] Add documentation --- OptimizelySDK/Odp/IOdpManager.cs | 30 ++++++++++++++++ OptimizelySDK/Odp/IOdpSegmentManager.cs | 7 ++++ OptimizelySDK/Odp/OdpManager.cs | 47 ++++++++++++++++++++++++- OptimizelySDK/Odp/OdpSegmentManager.cs | 7 ++++ 4 files changed, 90 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/IOdpManager.cs b/OptimizelySDK/Odp/IOdpManager.cs index 22d0beda..3b4716ba 100644 --- a/OptimizelySDK/Odp/IOdpManager.cs +++ b/OptimizelySDK/Odp/IOdpManager.cs @@ -18,18 +18,48 @@ namespace OptimizelySDK.Odp { + /// + /// Interface describing orchestration of segment manager, event manager, and ODP config + /// public interface IOdpManager { + /// + /// Update the settings being used for ODP configuration and reset/restart dependent processes + /// + /// Public API key from ODP + /// Host portion of the URL of ODP + /// Audience segments to consider + /// True if settings were update otherwise False bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck); + /// + /// Attempts to fetch and return a list of a user's qualified segments. + /// + /// FS User ID + /// Options used during segment cache handling + /// Qualified segments for the user from the cache or the ODP server List FetchQualifiedSegments(string userId, List options); + /// + /// Send identification event to ODP for a given full-stack User ID + /// + /// User ID to send void IdentifyUser(string userId); + /// + /// Add event to queue for sending to ODP + /// + /// Type of event (typically `fullstack` from server-side SDK events) + /// Subcategory of the event type + /// Key-value map of user identifiers + /// Event data in a key-value pair format void SendEvent(string type, string action, Dictionary identifiers, Dictionary data ); + /// + /// Sends signal to stop Event Manager and clean up ODP Manager use + /// void Close(); } } diff --git a/OptimizelySDK/Odp/IOdpSegmentManager.cs b/OptimizelySDK/Odp/IOdpSegmentManager.cs index 863b5e6e..3f7b729a 100644 --- a/OptimizelySDK/Odp/IOdpSegmentManager.cs +++ b/OptimizelySDK/Odp/IOdpSegmentManager.cs @@ -32,8 +32,15 @@ public interface IOdpSegmentManager /// Qualified segments for the user from the cache or the ODP server if the cache is empty. List FetchQualifiedSegments(string fsUserId, List options = null); + /// + /// Update the ODP configuration settings being used by the Segment Manager + /// + /// New ODP Configuration to apply void UpdateSettings(OdpConfig odpConfig); + /// + /// Reset/clear the segments cache + /// void ResetCache(); } } diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 57d7cb7d..04a38d1f 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -22,18 +22,43 @@ namespace OptimizelySDK.Odp { + /// + /// Concrete implementation to orchestrate segment manager, event manager, and ODP config + /// public class OdpManager : IOdpManager { + /// + /// Denotes if ODP Manager is meant to be handling ODP communication + /// private bool _enabled; + /// + /// Configuration used to communicate with ODP + /// private volatile OdpConfig _odpConfig; + /// + /// Manager used to handle audience segment membership + /// public IOdpSegmentManager SegmentManager { get; private set; } + /// + /// Manager used to send events to ODP + /// public IOdpEventManager EventManager { get; private set; } + /// + /// Logger used to record messages that occur within the ODP client + /// private ILogger _logger; + /// + /// Update the settings being used for ODP configuration and reset/restart dependent processes + /// + /// Public API key from ODP + /// Host portion of the URL of ODP + /// Audience segments to consider + /// True if settings were update otherwise False public bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck) { var newConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); @@ -53,6 +78,12 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT return true; } + /// + /// Attempts to fetch and return a list of a user's qualified segments. + /// + /// FS User ID + /// Options used during segment cache handling + /// Qualified segments for the user from the cache or the ODP server public List FetchQualifiedSegments(string userId, List options) { if (!_enabled || SegmentManager == null || !_odpConfig.IsReady()) @@ -64,6 +95,10 @@ public List FetchQualifiedSegments(string userId, List return SegmentManager.FetchQualifiedSegments(userId, options); } + /// + /// Send identification event to ODP for a given full-stack User ID + /// + /// User ID to send public void IdentifyUser(string userId) { if (!_enabled || EventManager == null || !_odpConfig.IsReady()) @@ -82,6 +117,13 @@ public void IdentifyUser(string userId) EventManager.IdentifyUser(userId); } + /// + /// Add event to queue for sending to ODP + /// + /// Type of event (typically `fullstack` from server-side SDK events) + /// Subcategory of the event type + /// Key-value map of user identifiers + /// Event data in a key-value pair format public void SendEvent(string type, string action, Dictionary identifiers, Dictionary data ) @@ -102,6 +144,9 @@ Dictionary data EventManager.SendEvent(new OdpEvent(type, action, identifiers, data)); } + /// + /// Sends signal to stop Event Manager and clean up ODP Manager use + /// public void Close() { if (!_enabled || EventManager == null) @@ -219,7 +264,7 @@ public OdpManager Build(bool asEnabled = true) { manager.SegmentManager = _segmentManager; } - + manager.EventManager.Start(); return manager; diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 655d7a9d..89d8fbca 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -143,11 +143,18 @@ private static string GetCacheKey(string userKey, string userValue) return $"{userKey}-$-{userValue}"; } + /// + /// Update the ODP configuration settings being used by the Segment Manager + /// + /// New ODP Configuration to apply public void UpdateSettings(OdpConfig odpConfig) { _odpConfig.Update(odpConfig.ApiKey, odpConfig.ApiHost, odpConfig.SegmentsToCheck); } + /// + /// Reset/clear the segments cache + /// public void ResetCache() { _segmentsCache.Reset(); From bbf8ac1dbf706016dade327c7c76879aa922d12b Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 29 Nov 2022 14:55:40 -0500 Subject: [PATCH 08/18] Lint fixes --- OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 7fcfeef5..c0c09770 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -199,22 +199,23 @@ public void ShouldDisableOdpThroughConfiguration() var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). - Build(); + Build(); manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); - + _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Never); _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), Times.Never); + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), + Times.Never); _mockOdpEventManager.ResetCalls(); _mockLogger.ResetCalls(); manager.UpdateSettings(string.Empty, string.Empty, _emptySegmentsToCheck); - + manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); manager.Close(); @@ -223,7 +224,8 @@ public void ShouldDisableOdpThroughConfiguration() _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), Times.Never); + l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), + Times.Never); } [Test] From 4a54b4c80d8764a8e9fb7fdfad6c0855442a6158 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 1 Dec 2022 11:40:38 -0500 Subject: [PATCH 09/18] Pull request code revisions + additional unit tests --- OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs | 110 ++++++++++++++++++ .../OdpTests/OdpManagerTest.cs | 14 +-- .../OptimizelySDK.Tests.csproj | 1 + OptimizelySDK/Odp/IOdpManager.cs | 2 +- OptimizelySDK/Odp/OdpConfig.cs | 33 ++++-- OptimizelySDK/Odp/OdpManager.cs | 34 +++++- 6 files changed, 172 insertions(+), 22 deletions(-) create mode 100644 OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs diff --git a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs new file mode 100644 index 00000000..d37492d5 --- /dev/null +++ b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs @@ -0,0 +1,110 @@ +/* + * Copyright 2022 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using Moq; +using NUnit.Framework; +using OptimizelySDK.Logger; +using OptimizelySDK.Odp; +using OptimizelySDK.Odp.Entity; +using System.CodeDom; +using System.Collections.Generic; +using System.Linq; + +namespace OptimizelySDK.Tests.OdpTests +{ + [TestFixture] + public class OdpConfigTest + { + private const string API_KEY = "UrAp1k3Y"; + private const string API_HOST = "https://not-real-odp-host.example.com"; + + private static readonly List segmentsToCheck = new List + { + "UPPER-CASE-AUDIENCE", + "lower-case-audience", + }; + + private readonly OdpConfig _goodOdpConfig = + new OdpConfig(API_KEY, API_HOST, segmentsToCheck); + + [Test] + public void ShouldNotEqualWithNullInApiKey() + { + var nullKeyConfig = + new OdpConfig(null, API_HOST, segmentsToCheck); + + Assert.IsFalse(_goodOdpConfig.Equals(nullKeyConfig)); + Assert.IsFalse(nullKeyConfig.Equals(_goodOdpConfig)); + } + + [Test] + public void ShouldNotEqualWithNullInApiHost() + { + var nullHostConfig = + new OdpConfig(API_KEY, null, segmentsToCheck); + + Assert.IsFalse(_goodOdpConfig.Equals(nullHostConfig)); + Assert.IsFalse(nullHostConfig.Equals(_goodOdpConfig)); + } + + [Test] + public void ShouldNotEqualWithNullSegmentsCollection() + { + var nullSegmentsConfig = + new OdpConfig(API_KEY, API_HOST, null); + + Assert.IsFalse(_goodOdpConfig.Equals(nullSegmentsConfig)); + Assert.IsFalse(nullSegmentsConfig.Equals(_goodOdpConfig)); + } + + [Test] + public void ShouldNotEqualWithSegmentsWithNull() + { + var segmentsWithANullValue = + new OdpConfig(API_KEY, API_HOST, new List + { + "good-value", + null, + }); + + Assert.IsFalse(_goodOdpConfig.Equals(segmentsWithANullValue)); + Assert.IsFalse(segmentsWithANullValue.Equals(_goodOdpConfig)); + } + + [Test] + public void ShouldEqualDespiteCaseDifferenceInApiHost() + { + var apiHostUpperCasedConfig = new OdpConfig(API_KEY, API_HOST.ToUpper(), segmentsToCheck); + + Assert.IsTrue(_goodOdpConfig.Equals(apiHostUpperCasedConfig)); + Assert.IsTrue(apiHostUpperCasedConfig.Equals(_goodOdpConfig)); + } + + [Test] + public void ShouldEqualDespiteCaseDifferenceInSegments() + { + var wrongCaseSegmentsToCheck = new List + { + "upper-case-audience", + "LOWER-CASE-AUDIENCE", + }; + var wrongCaseConfig = new OdpConfig(API_KEY, API_HOST, wrongCaseSegmentsToCheck); + + Assert.IsTrue(_goodOdpConfig.Equals(wrongCaseConfig)); + Assert.IsTrue(wrongCaseConfig.Equals(_goodOdpConfig)); + } + } +} diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index c0c09770..4551eefa 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -105,7 +105,7 @@ public void ShouldStopEventManagerWhenCloseIsCalled() WithLogger(_mockLogger.Object). Build(); - manager.Close(); + manager.Dispose(); _mockOdpEventManager.Verify(e => e.Stop(), Times.Once); } @@ -218,7 +218,7 @@ public void ShouldDisableOdpThroughConfiguration() manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); - manager.Close(); + manager.Dispose(); _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Never); _mockLogger.Verify(l => @@ -263,7 +263,7 @@ public void ShouldIdentifyUserWhenOdpIsIntegrated() Build(); manager.IdentifyUser(VALID_FS_USER_ID); - manager.Close(); + manager.Dispose(); _mockLogger.Verify(l => l.Log(It.IsAny(), It.IsAny()), Times.Never); _mockOdpEventManager.Verify(e => e.IdentifyUser(It.IsAny()), Times.Once); @@ -280,7 +280,7 @@ public void ShouldNotIdentifyUserWhenOdpNotIntegrated() Build(); manager.IdentifyUser(VALID_FS_USER_ID); - manager.Close(); + manager.Dispose(); _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP identify event not dispatched (ODP not integrated).")); @@ -298,7 +298,7 @@ public void ShouldNotIdentifyUserWhenOdpDisabled() Build(false); manager.IdentifyUser(VALID_FS_USER_ID); - manager.Close(); + manager.Dispose(); _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP identify event not dispatched (ODP disabled).")); @@ -317,7 +317,7 @@ public void ShouldSendEventWhenOdpIsIntegrated() manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); - manager.Close(); + manager.Dispose(); _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); } @@ -333,7 +333,7 @@ public void ShouldNotSendEventOdpNotIntegrated() manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); - manager.Close(); + manager.Dispose(); _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index a71d34f3..fa50e575 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -80,6 +80,7 @@ + diff --git a/OptimizelySDK/Odp/IOdpManager.cs b/OptimizelySDK/Odp/IOdpManager.cs index 3b4716ba..8e4f2ec7 100644 --- a/OptimizelySDK/Odp/IOdpManager.cs +++ b/OptimizelySDK/Odp/IOdpManager.cs @@ -60,6 +60,6 @@ Dictionary data /// /// Sends signal to stop Event Manager and clean up ODP Manager use /// - void Close(); + void Dispose(); } } diff --git a/OptimizelySDK/Odp/OdpConfig.cs b/OptimizelySDK/Odp/OdpConfig.cs index 99750e73..d1354255 100644 --- a/OptimizelySDK/Odp/OdpConfig.cs +++ b/OptimizelySDK/Odp/OdpConfig.cs @@ -16,10 +16,11 @@ using System; using System.Collections.Generic; +using System.Linq; namespace OptimizelySDK.Odp { - public class OdpConfig + public class OdpConfig : IEquatable { /// /// Public API key for the ODP account from which the audience segments will be fetched (optional). @@ -120,16 +121,32 @@ public bool HasSegments() } /// - /// Determine equality between two OdpConfig objects + /// Determine equality between two OdpConfig objects based on case-insensitive value comparisons /// - /// OdpConfig object to compare current instance against + /// OdpConfig object to compare current instance against /// True if equal otherwise False - public bool Equals(OdpConfig toCompare) + public bool Equals(OdpConfig otherConfig) { - return ApiKey.Equals(toCompare.ApiKey, StringComparison.OrdinalIgnoreCase) && - ApiHost.Equals(toCompare.ApiHost, StringComparison.OrdinalIgnoreCase) && - SegmentsToCheck.TrueForAll( - segment => toCompare.SegmentsToCheck.Contains(segment)); + // less expensive equality checks first + if (otherConfig == null || + !string.Equals(ApiKey, otherConfig.ApiKey, + StringComparison.Ordinal) || // case-matters + !string.Equals(ApiHost, otherConfig.ApiHost, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + if (SegmentsToCheck == null || + otherConfig.SegmentsToCheck == null || + SegmentsToCheck.Count != otherConfig.SegmentsToCheck.Count) + { + return false; + } + + return SegmentsToCheck.TrueForAll( + segment => + otherConfig.SegmentsToCheck.Contains(segment, + StringComparer.OrdinalIgnoreCase)); } } } diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 04a38d1f..b6654052 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -25,7 +25,7 @@ namespace OptimizelySDK.Odp /// /// Concrete implementation to orchestrate segment manager, event manager, and ODP config /// - public class OdpManager : IOdpManager + public class OdpManager : IOdpManager, IDisposable { /// /// Denotes if ODP Manager is meant to be handling ODP communication @@ -69,6 +69,10 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT _odpConfig = newConfig; + // flush old events using old odp publicKey (if exists) before updating odp key. + // NOTE: It should be rare but possible that odp public key is changed for the same datafile (sdkKey). + // - Try to send all old events with the previous public key. + // - If it fails to flush all the old events here (network error), remaining events will be discarded. EventManager.Flush(); EventManager.UpdateSettings(_odpConfig); @@ -86,7 +90,7 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT /// Qualified segments for the user from the cache or the ODP server public List FetchQualifiedSegments(string userId, List options) { - if (!_enabled || SegmentManager == null || !_odpConfig.IsReady()) + if (SegmentManagerOrConfigNotReady()) { _logger.Log(LogLevel.ERROR, Constants.ODP_NOT_ENABLED_MESSAGE); return null; @@ -101,7 +105,7 @@ public List FetchQualifiedSegments(string userId, List /// User ID to send public void IdentifyUser(string userId) { - if (!_enabled || EventManager == null || !_odpConfig.IsReady()) + if (EventManagerOrConfigNotReady()) { _logger.Log(LogLevel.DEBUG, "ODP identify event not dispatched (ODP disabled)."); return; @@ -128,7 +132,7 @@ public void SendEvent(string type, string action, Dictionary ide Dictionary data ) { - if (!_enabled || EventManager == null || !_odpConfig.IsReady()) + if (EventManagerOrConfigNotReady()) { _logger.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."); return; @@ -144,12 +148,30 @@ Dictionary data EventManager.SendEvent(new OdpEvent(type, action, identifiers, data)); } + /// + /// Determines if the EventManager is ready to be used + /// + /// True if EventManager can process events otherwise False + private bool EventManagerOrConfigNotReady() + { + return EventManager == null || !_enabled || !_odpConfig.IsReady(); + } + + /// + /// Determines if the SegmentManager is ready to be used + /// + /// True if SegmentManager can fetch audience segments otherwise False + private bool SegmentManagerOrConfigNotReady() + { + return SegmentManager == null || !_enabled || !_odpConfig.IsReady(); + } + /// /// Sends signal to stop Event Manager and clean up ODP Manager use /// - public void Close() + public void Dispose() { - if (!_enabled || EventManager == null) + if (EventManager == null || !_enabled) { return; } From 09956942b1341cc212a040cd4c0d9318963627f0 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 1 Dec 2022 11:44:51 -0500 Subject: [PATCH 10/18] Lint fixes --- OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs index d37492d5..c8eb2125 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs @@ -87,7 +87,8 @@ public void ShouldNotEqualWithSegmentsWithNull() [Test] public void ShouldEqualDespiteCaseDifferenceInApiHost() { - var apiHostUpperCasedConfig = new OdpConfig(API_KEY, API_HOST.ToUpper(), segmentsToCheck); + var apiHostUpperCasedConfig = + new OdpConfig(API_KEY, API_HOST.ToUpper(), segmentsToCheck); Assert.IsTrue(_goodOdpConfig.Equals(apiHostUpperCasedConfig)); Assert.IsTrue(apiHostUpperCasedConfig.Equals(_goodOdpConfig)); @@ -102,7 +103,7 @@ public void ShouldEqualDespiteCaseDifferenceInSegments() "LOWER-CASE-AUDIENCE", }; var wrongCaseConfig = new OdpConfig(API_KEY, API_HOST, wrongCaseSegmentsToCheck); - + Assert.IsTrue(_goodOdpConfig.Equals(wrongCaseConfig)); Assert.IsTrue(wrongCaseConfig.Equals(_goodOdpConfig)); } From 88b6684eae6436b7022065194c3a8d3fa9eac27e Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 1 Dec 2022 11:49:03 -0500 Subject: [PATCH 11/18] Change to use string array over List --- OptimizelySDK/Odp/IOdpManager.cs | 2 +- OptimizelySDK/Odp/OdpManager.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK/Odp/IOdpManager.cs b/OptimizelySDK/Odp/IOdpManager.cs index 8e4f2ec7..b65d2687 100644 --- a/OptimizelySDK/Odp/IOdpManager.cs +++ b/OptimizelySDK/Odp/IOdpManager.cs @@ -38,7 +38,7 @@ public interface IOdpManager /// FS User ID /// Options used during segment cache handling /// Qualified segments for the user from the cache or the ODP server - List FetchQualifiedSegments(string userId, List options); + string[] FetchQualifiedSegments(string userId, List options); /// /// Send identification event to ODP for a given full-stack User ID diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index b6654052..6fb2c166 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -88,7 +88,7 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT /// FS User ID /// Options used during segment cache handling /// Qualified segments for the user from the cache or the ODP server - public List FetchQualifiedSegments(string userId, List options) + public string[] FetchQualifiedSegments(string userId, List options) { if (SegmentManagerOrConfigNotReady()) { @@ -96,7 +96,7 @@ public List FetchQualifiedSegments(string userId, List return null; } - return SegmentManager.FetchQualifiedSegments(userId, options); + return SegmentManager.FetchQualifiedSegments(userId, options).ToArray(); } /// From 29514ff1efbe018a4875bfb1f0c797c3cc748a88 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 1 Dec 2022 11:59:50 -0500 Subject: [PATCH 12/18] Clean usings --- OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs index c8eb2125..f8f9833d 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs @@ -14,14 +14,9 @@ * limitations under the License. */ -using Moq; using NUnit.Framework; -using OptimizelySDK.Logger; using OptimizelySDK.Odp; -using OptimizelySDK.Odp.Entity; -using System.CodeDom; using System.Collections.Generic; -using System.Linq; namespace OptimizelySDK.Tests.OdpTests { From 22b04ae236e806c6f6289e6110d7236d303dc2dc Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 1 Dec 2022 12:04:43 -0500 Subject: [PATCH 13/18] Add new OdpConfig test --- OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs index f8f9833d..b0eaa0a2 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs @@ -79,6 +79,18 @@ public void ShouldNotEqualWithSegmentsWithNull() Assert.IsFalse(segmentsWithANullValue.Equals(_goodOdpConfig)); } + [Test] + public void ShouldNotEqualIfCaseDifferenceInApiKey() + { + const string caseDifferenceInFirstLetterOfApiKey = "urAp1k3Y"; + ; + var apiKeyCaseDifferentConfig = + new OdpConfig(caseDifferenceInFirstLetterOfApiKey, API_HOST, segmentsToCheck); + + Assert.IsFalse(_goodOdpConfig.Equals(apiKeyCaseDifferentConfig)); + Assert.IsFalse(apiKeyCaseDifferentConfig.Equals(_goodOdpConfig)); + } + [Test] public void ShouldEqualDespiteCaseDifferenceInApiHost() { From dcafcee3e07a78595f681fd4040501d4b0d590ed Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 1 Dec 2022 16:02:58 -0500 Subject: [PATCH 14/18] Small refactors --- OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs | 6 ++-- OptimizelySDK/Odp/OdpManager.cs | 36 +++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs index b0eaa0a2..a948e661 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpConfigTest.cs @@ -82,10 +82,10 @@ public void ShouldNotEqualWithSegmentsWithNull() [Test] public void ShouldNotEqualIfCaseDifferenceInApiKey() { - const string caseDifferenceInFirstLetterOfApiKey = "urAp1k3Y"; - ; + const string CASE_DIFFERENCE_IN_FIRST_LETTER_OF_API_KEY = "urAp1k3Y"; var apiKeyCaseDifferentConfig = - new OdpConfig(caseDifferenceInFirstLetterOfApiKey, API_HOST, segmentsToCheck); + new OdpConfig(CASE_DIFFERENCE_IN_FIRST_LETTER_OF_API_KEY, API_HOST, + segmentsToCheck); Assert.IsFalse(_goodOdpConfig.Equals(apiKeyCaseDifferentConfig)); Assert.IsFalse(apiKeyCaseDifferentConfig.Equals(_goodOdpConfig)); diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 6fb2c166..5624b98d 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -148,24 +148,6 @@ Dictionary data EventManager.SendEvent(new OdpEvent(type, action, identifiers, data)); } - /// - /// Determines if the EventManager is ready to be used - /// - /// True if EventManager can process events otherwise False - private bool EventManagerOrConfigNotReady() - { - return EventManager == null || !_enabled || !_odpConfig.IsReady(); - } - - /// - /// Determines if the SegmentManager is ready to be used - /// - /// True if SegmentManager can fetch audience segments otherwise False - private bool SegmentManagerOrConfigNotReady() - { - return SegmentManager == null || !_enabled || !_odpConfig.IsReady(); - } - /// /// Sends signal to stop Event Manager and clean up ODP Manager use /// @@ -292,5 +274,23 @@ public OdpManager Build(bool asEnabled = true) return manager; } } + + /// + /// Determines if the EventManager is ready to be used + /// + /// True if EventManager can process events otherwise False + private bool EventManagerOrConfigNotReady() + { + return EventManager == null || !_enabled || !_odpConfig.IsReady(); + } + + /// + /// Determines if the SegmentManager is ready to be used + /// + /// True if SegmentManager can fetch audience segments otherwise False + private bool SegmentManagerOrConfigNotReady() + { + return SegmentManager == null || !_enabled || !_odpConfig.IsReady(); + } } } From 8747556454e3b2ab1f8e579c056aa7910400b34c Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 2 Dec 2022 13:46:48 -0500 Subject: [PATCH 15/18] Pull request code review changes --- .../OdpTests/OdpEventManagerTests.cs | 12 ++----- .../OdpTests/OdpManagerTest.cs | 30 ++-------------- OptimizelySDK/Odp/OdpConfig.cs | 21 ----------- OptimizelySDK/Odp/OdpEventManager.cs | 7 +++- OptimizelySDK/Odp/OdpManager.cs | 35 +++++++++---------- OptimizelySDK/Odp/OdpSegmentManager.cs | 4 +-- 6 files changed, 30 insertions(+), 79 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index b1bef6f0..0fea8a82 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -480,9 +480,6 @@ public void ShouldPrepareCorrectPayloadForIdentifyUser() [Test] public void ShouldApplyUpdatedOdpConfigurationWhenAvailable() { - var apiKeyCollector = new List(); - var apiHostCollector = new List(); - var segmentsToCheckCollector = new List>(); var apiKey = "testing-api-key"; var apiHost = "https://some.other.example.com"; var segmentsToCheck = new List @@ -490,20 +487,15 @@ public void ShouldApplyUpdatedOdpConfigurationWhenAvailable() "empty-cart", "1-item-cart", }; - var mockOdpConfig = new Mock(API_KEY, API_HOST, segmentsToCheck); - mockOdpConfig.Setup(m => m.Update(Capture.In(apiKeyCollector), - Capture.In(apiHostCollector), Capture.In(segmentsToCheckCollector))); var differentOdpConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object). + var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(); eventManager.UpdateSettings(differentOdpConfig); - Assert.AreEqual(apiKey, apiKeyCollector[0]); - Assert.AreEqual(apiHost, apiHostCollector[0]); - Assert.AreEqual(segmentsToCheck, segmentsToCheckCollector[0]); + Assert.IsFalse(_odpConfig.Equals(eventManager._readOdpConfigForTesting())); } private static OdpEvent MakeEvent(int id) => diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 4551eefa..2f943a0c 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -206,10 +206,7 @@ public void ShouldDisableOdpThroughConfiguration() _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Never); - _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), - Times.Never); + l.Log(LogLevel.ERROR, "ODP event not dispatched (ODP disabled)."), Times.Never); _mockOdpEventManager.ResetCalls(); _mockLogger.ResetCalls(); @@ -222,10 +219,7 @@ public void ShouldDisableOdpThroughConfiguration() _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Never); _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); - _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP not integrated)."), - Times.Never); + l.Log(LogLevel.ERROR, "ODP event not dispatched (ODP disabled)."), Times.Once); } [Test] @@ -269,24 +263,6 @@ public void ShouldIdentifyUserWhenOdpIsIntegrated() _mockOdpEventManager.Verify(e => e.IdentifyUser(It.IsAny()), Times.Once); } - [Test] - public void ShouldNotIdentifyUserWhenOdpNotIntegrated() - { - _mockOdpEventManager.Setup(e => e.IdentifyUser(It.IsAny())); - _mockOdpEventManager.Setup(e => e.IsStarted).Returns(false); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). - WithEventManager(_mockOdpEventManager.Object). - WithLogger(_mockLogger.Object). - Build(); - - manager.IdentifyUser(VALID_FS_USER_ID); - manager.Dispose(); - - _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, - "ODP identify event not dispatched (ODP not integrated).")); - _mockOdpEventManager.Verify(e => e.IdentifyUser(It.IsAny()), Times.Never); - } - [Test] public void ShouldNotIdentifyUserWhenOdpDisabled() { @@ -336,7 +312,7 @@ public void ShouldNotSendEventOdpNotIntegrated() manager.Dispose(); _mockLogger.Verify(l => - l.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."), Times.Once); + l.Log(LogLevel.ERROR, "ODP event not dispatched (ODP disabled)."), Times.Once); _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Never); } } diff --git a/OptimizelySDK/Odp/OdpConfig.cs b/OptimizelySDK/Odp/OdpConfig.cs index d1354255..de81a740 100644 --- a/OptimizelySDK/Odp/OdpConfig.cs +++ b/OptimizelySDK/Odp/OdpConfig.cs @@ -81,27 +81,6 @@ public OdpConfig(string apiKey, string apiHost, List segmentsToCheck) SegmentsToCheck = segmentsToCheck ?? new List(0); } - /// - /// Update the ODP configuration details - /// - /// Public API key for the ODP account - /// Host of ODP audience segments API - /// Audience segments - /// true if configuration was updated successfully otherwise false - public virtual bool Update(string apiKey, string apiHost, List segmentsToCheck) - { - if (ApiKey == apiKey && ApiHost == apiHost && SegmentsToCheck == segmentsToCheck) - { - return false; - } - - ApiKey = apiKey; - ApiHost = apiHost; - SegmentsToCheck = segmentsToCheck; - - return true; - } - /// /// Determines if ODP configuration has the minimum amount of information /// diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 45f208ae..860ee8f0 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -349,7 +349,7 @@ public void IdentifyUser(string userId) /// Configuration object containing new values public void UpdateSettings(OdpConfig odpConfig) { - _odpConfig.Update(odpConfig.ApiKey, odpConfig.ApiHost, odpConfig.SegmentsToCheck); + _odpConfig = odpConfig; } /// @@ -518,5 +518,10 @@ public OdpEventManager Build(bool startImmediately = true) return manager; } } + + public OdpConfig _readOdpConfigForTesting() + { + return _odpConfig; + } } } diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 5624b98d..261f37dc 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -111,13 +111,6 @@ public void IdentifyUser(string userId) return; } - if (!EventManager.IsStarted) - { - _logger.Log(LogLevel.DEBUG, - "ODP identify event not dispatched (ODP not integrated)."); - return; - } - EventManager.IdentifyUser(userId); } @@ -134,14 +127,7 @@ Dictionary data { if (EventManagerOrConfigNotReady()) { - _logger.Log(LogLevel.DEBUG, "ODP event not dispatched (ODP disabled)."); - return; - } - - if (!EventManager.IsStarted) - { - _logger.Log(LogLevel.DEBUG, - "ODP event not dispatched (ODP not integrated)."); + _logger.Log(LogLevel.ERROR, "ODP event not dispatched (ODP disabled)."); return; } @@ -230,20 +216,33 @@ public Builder WithCacheImplementation(ICache> cache) /// OdpManager instance public OdpManager Build(bool asEnabled = true) { + _logger = _logger ?? new DefaultLogger(); + _errorHandler = _errorHandler ?? new NoOpErrorHandler(); + + if (_odpConfig == null) + { + _logger.Log(LogLevel.ERROR, "ODP Config via WithOdpConfig() required."); + return null; + } + var manager = new OdpManager { _odpConfig = _odpConfig, - _logger = _logger ?? new NoOpLogger(), + _logger = _logger, _enabled = asEnabled, }; - _errorHandler = _errorHandler ?? new NoOpErrorHandler(); + if (!manager._enabled) + { + return manager; + } if (_eventManager == null) { var eventApiManager = new OdpEventApiManager(_logger, _errorHandler); - manager.EventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + manager.EventManager = new OdpEventManager.Builder(). + WithOdpConfig(_odpConfig). WithOdpEventApiManager(eventApiManager). WithLogger(_logger). WithErrorHandler(_errorHandler). diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 89d8fbca..12376286 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -40,7 +40,7 @@ public class OdpSegmentManager : IOdpSegmentManager /// /// ODP configuration containing the connection parameters /// - private readonly OdpConfig _odpConfig; + private OdpConfig _odpConfig; /// /// Cached segments @@ -149,7 +149,7 @@ private static string GetCacheKey(string userKey, string userValue) /// New ODP Configuration to apply public void UpdateSettings(OdpConfig odpConfig) { - _odpConfig.Update(odpConfig.ApiKey, odpConfig.ApiHost, odpConfig.SegmentsToCheck); + _odpConfig = odpConfig; } /// From dfaa27b24faea51b2e901d8a68da1dbca135ea1f Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 6 Dec 2022 14:08:06 -0500 Subject: [PATCH 16/18] Code review changes --- OptimizelySDK/Odp/OdpConfig.cs | 50 ++++++--------------------------- OptimizelySDK/Odp/OdpManager.cs | 14 ++------- 2 files changed, 10 insertions(+), 54 deletions(-) diff --git a/OptimizelySDK/Odp/OdpConfig.cs b/OptimizelySDK/Odp/OdpConfig.cs index de81a740..07dc0925 100644 --- a/OptimizelySDK/Odp/OdpConfig.cs +++ b/OptimizelySDK/Odp/OdpConfig.cs @@ -25,59 +25,25 @@ public class OdpConfig : IEquatable /// /// Public API key for the ODP account from which the audience segments will be fetched (optional). /// - private volatile string _apiKey; - - public string ApiKey - { - get - { - return _apiKey; - } - private set - { - _apiKey = value; - } - } + public string ApiKey { get; private set; } /// /// Host of ODP audience segments API. /// - private volatile string _apiHost; - - public string ApiHost - { - get - { - return _apiHost; - } - private set - { - _apiHost = value; - } - } + public string ApiHost { get; private set; } /// /// All ODP segments used in the current datafile (associated with apiHost/apiKey). /// - private volatile List _segmentsToCheck; - - public List SegmentsToCheck - { - get - { - return _segmentsToCheck; - } + public List SegmentsToCheck { get; private set; } - private set - { - _segmentsToCheck = value; - } - } - public OdpConfig(string apiKey, string apiHost, List segmentsToCheck) + public OdpConfig(string apiKey = null, string apiHost = null, + List segmentsToCheck = null + ) { - ApiKey = apiKey; - ApiHost = apiHost; + ApiKey = apiKey ?? string.Empty; + ApiHost = apiHost ?? string.Empty; SegmentsToCheck = segmentsToCheck ?? new List(0); } diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 261f37dc..4600b650 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -35,7 +35,7 @@ public class OdpManager : IOdpManager, IDisposable /// /// Configuration used to communicate with ODP /// - private volatile OdpConfig _odpConfig; + private OdpConfig _odpConfig; /// /// Manager used to handle audience segment membership @@ -69,11 +69,6 @@ public bool UpdateSettings(string apiKey, string apiHost, List segmentsT _odpConfig = newConfig; - // flush old events using old odp publicKey (if exists) before updating odp key. - // NOTE: It should be rare but possible that odp public key is changed for the same datafile (sdkKey). - // - Try to send all old events with the previous public key. - // - If it fails to flush all the old events here (network error), remaining events will be discarded. - EventManager.Flush(); EventManager.UpdateSettings(_odpConfig); SegmentManager.ResetCache(); @@ -218,12 +213,7 @@ public OdpManager Build(bool asEnabled = true) { _logger = _logger ?? new DefaultLogger(); _errorHandler = _errorHandler ?? new NoOpErrorHandler(); - - if (_odpConfig == null) - { - _logger.Log(LogLevel.ERROR, "ODP Config via WithOdpConfig() required."); - return null; - } + _odpConfig = _odpConfig ?? new OdpConfig(); var manager = new OdpManager { From c528dce762c757ba6ef37249fe81c14ca26e6232 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 6 Dec 2022 16:26:46 -0500 Subject: [PATCH 17/18] Mark _odpConfig as volatile --- OptimizelySDK/Odp/OdpEventManager.cs | 2 +- OptimizelySDK/Odp/OdpSegmentManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 860ee8f0..070f4d8b 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -32,7 +32,7 @@ namespace OptimizelySDK.Odp /// public class OdpEventManager : IOdpEventManager, IDisposable { - private OdpConfig _odpConfig; + private volatile OdpConfig _odpConfig; private IOdpEventApiManager _odpEventApiManager; private int _batchSize; private TimeSpan _flushInterval; diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 12376286..c155a755 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -40,7 +40,7 @@ public class OdpSegmentManager : IOdpSegmentManager /// /// ODP configuration containing the connection parameters /// - private OdpConfig _odpConfig; + private volatile OdpConfig _odpConfig; /// /// Cached segments From a5b28e687f7c54fc32f26e7c4d03ccf6efde3d35 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 7 Dec 2022 10:44:05 -0500 Subject: [PATCH 18/18] Return `volatile` denotation --- OptimizelySDK/Odp/OdpManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 4600b650..0a57ca19 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -35,7 +35,7 @@ public class OdpManager : IOdpManager, IDisposable /// /// Configuration used to communicate with ODP /// - private OdpConfig _odpConfig; + private volatile OdpConfig _odpConfig; /// /// Manager used to handle audience segment membership