From 1a43162236b6aaf84f5dbfd3bcbc51abf3308a86 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 6 Jan 2023 14:49:28 -0500 Subject: [PATCH 01/19] PR changes --- .../Odp/Entity/OptimizelySdkSettings.cs | 51 ------------------- OptimizelySDK/Odp/OdpManager.cs | 24 ++------- OptimizelySDK/Optimizely.cs | 44 +++++++++------- OptimizelySDK/OptimizelySDK.csproj | 1 - 4 files changed, 30 insertions(+), 90 deletions(-) delete mode 100644 OptimizelySDK/Odp/Entity/OptimizelySdkSettings.cs diff --git a/OptimizelySDK/Odp/Entity/OptimizelySdkSettings.cs b/OptimizelySDK/Odp/Entity/OptimizelySdkSettings.cs deleted file mode 100644 index 8e38d002..00000000 --- a/OptimizelySDK/Odp/Entity/OptimizelySdkSettings.cs +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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. - */ - -namespace OptimizelySDK.Odp.Entity -{ - public class OptimizelySdkSettings - { - /// - /// The maximum size of audience segments cache - cache is disabled if this is set to zero - /// - public int SegmentsCacheSize { get; set; } - - /// - /// The timeout in seconds of audience segments cache - timeout is disabled if this is set to zero - /// - public int SegmentsCacheTimeout { get; set; } - - /// - /// ODP features are disabled if this is set to true. - /// - public bool DisableOdp { get; set; } - - /// - /// Optimizely SDK Settings - /// - /// The maximum size of audience segments cache (optional. default = 100). Set to zero to disable caching - /// The timeout in seconds of audience segments cache (optional. default = 600). Set to zero to disable timeout - /// Set this flag to true (default = false) to disable ODP features - public OptimizelySdkSettings(int segmentsCacheSize = 100, int segmentsCacheTimeout = 600, - bool disableOdp = false - ) - { - SegmentsCacheSize = segmentsCacheSize; - SegmentsCacheTimeout = segmentsCacheTimeout; - DisableOdp = disableOdp; - } - } -} diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index ddaf79e0..e3a6118c 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -152,8 +152,6 @@ 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; @@ -176,18 +174,6 @@ public Builder WithOdpConfig(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; @@ -235,8 +221,8 @@ public OdpManager Build(bool asEnabled = true) manager.EventManager = new OdpEventManager.Builder(). WithOdpConfig(_odpConfig). - WithBatchSize(_cacheSize). - WithTimeoutInterval(TimeSpan.FromMilliseconds(_cacheTimeoutSeconds)). + WithBatchSize(Constants.DEFAULT_MAX_CACHE_SIZE). + WithTimeoutInterval(TimeSpan.FromMilliseconds(Constants.DEFAULT_CACHE_SECONDS)). WithOdpEventApiManager(eventApiManager). WithLogger(_logger). WithErrorHandler(_errorHandler). @@ -251,13 +237,11 @@ public OdpManager Build(bool asEnabled = true) if (_segmentManager == null) { - var cacheTimeout = TimeSpan.FromSeconds(_cacheTimeoutSeconds <= 0 ? - Constants.DEFAULT_CACHE_SECONDS : - _cacheTimeoutSeconds); + var cacheTimeout = TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS); var apiManager = new OdpSegmentApiManager(_logger, _errorHandler); manager.SegmentManager = new OdpSegmentManager(_odpConfig, apiManager, - _cacheSize, cacheTimeout, _logger, _cache); + Constants.DEFAULT_MAX_CACHE_SIZE, cacheTimeout, _logger, _cache); } else { diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 7504b822..0c184a31 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -38,7 +38,6 @@ #if USE_ODP using OptimizelySDK.Odp; -using OptimizelySDK.Odp.Entity; #endif namespace OptimizelySDK @@ -75,8 +74,6 @@ public class Optimizely : IOptimizely, IDisposable #if USE_ODP private OdpManager OdpManager; - - private OptimizelySdkSettings SdkSettings; #endif /// @@ -138,7 +135,7 @@ public static String SDK_TYPE /// boolean representing whether JSON schema validation needs to be performed /// EventProcessor /// Default Decide options - /// Optional settings for SDK configuration + /// Optional ODP Manager public Optimizely(string datafile, IEventDispatcher eventDispatcher = null, ILogger logger = null, @@ -148,21 +145,26 @@ public Optimizely(string datafile, EventProcessor eventProcessor = null, OptimizelyDecideOption[] defaultDecideOptions = null #if USE_ODP - , OptimizelySdkSettings sdkSettings = null + , OdpManager odpManager = null #endif ) { try { +#if USE_ODP + InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, + null, eventProcessor, defaultDecideOptions, odpManager); +#else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, - null, eventProcessor, defaultDecideOptions); + null, eventProcessor, defaultDecideOptions); +#endif if (ValidateInputs(datafile, skipJsonValidation)) { var config = DatafileProjectConfig.Create(datafile, Logger, ErrorHandler); ProjectConfigManager = new FallbackProjectConfigManager(config); #if USE_ODP - SetupOdp(config, sdkSettings); + SetupOdp(config); #endif } else @@ -194,7 +196,7 @@ public Optimizely(string datafile, /// User profile service. /// EventProcessor /// Default Decide options - /// Optional settings for SDK configuration + /// Optional ODP Manager public Optimizely(ProjectConfigManager configManager, NotificationCenter notificationCenter = null, IEventDispatcher eventDispatcher = null, @@ -204,16 +206,20 @@ public Optimizely(ProjectConfigManager configManager, EventProcessor eventProcessor = null, OptimizelyDecideOption[] defaultDecideOptions = null #if USE_ODP - , OptimizelySdkSettings sdkSettings = null + , OdpManager odpManager = null #endif ) { ProjectConfigManager = configManager; +#if USE_ODP + InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, + notificationCenter, eventProcessor, defaultDecideOptions, odpManager); + + SetupOdp(ProjectConfigManager.CachedProjectConfig); +#else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions); -#if USE_ODP - SetupOdp(ProjectConfigManager.CachedProjectConfig, sdkSettings); #endif } @@ -224,6 +230,9 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null, NotificationCenter notificationCenter = null, EventProcessor eventProcessor = null, OptimizelyDecideOption[] defaultDecideOptions = null +#if USE_ODP + , OdpManager odpManager = null +#endif ) { Logger = logger ?? new NoOpLogger(); @@ -240,28 +249,27 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null, Logger); DefaultDecideOptions = defaultDecideOptions ?? new OptimizelyDecideOption[] { }; +#if USE_ODP + OdpManager = odpManager ?? new OdpManager.Builder().Build(); +#endif } #if USE_ODP - private void SetupOdp(ProjectConfig config, OptimizelySdkSettings sdkSettings = null) + private void SetupOdp(ProjectConfig config) { - if (config == null || sdkSettings?.DisableOdp == true) + if (config == null) { return; } - SdkSettings = sdkSettings ?? new OptimizelySdkSettings(); - var odpConfig = new OdpConfig(config.PublicKeyForOdp, config.HostForOdp, config.Segments.ToList()); OdpManager = new OdpManager.Builder(). WithOdpConfig(odpConfig). - WithCacheSize(SdkSettings.SegmentsCacheSize). - WithCacheTimeout(SdkSettings.SegmentsCacheTimeout). WithLogger(Logger). WithErrorHandler(ErrorHandler). - Build(!SdkSettings.DisableOdp); + Build(); NotificationCenter.AddNotification( NotificationCenter.NotificationType.OptimizelyConfigUpdate, diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 15a6fcfb..b667a2fb 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -99,7 +99,6 @@ - From ebd61f679fe8c833ba93b0f710ad2c942bd25ca2 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 6 Jan 2023 16:33:40 -0500 Subject: [PATCH 02/19] More PR changes (as I understand them) --- .../OdpTests/OdpSegmentManagerTest.cs | 32 +++++++++---------- OptimizelySDK/Odp/OdpManager.cs | 8 ++--- OptimizelySDK/Odp/OdpSegmentManager.cs | 23 +++++++++++-- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index c8abe650..8b45bc48 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -69,8 +69,8 @@ public void ShouldFetchSegmentsOnCacheMiss() _mockApiManager.Setup(a => a.FetchSegments(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())). Returns(segmentsToCheck.ToArray()); - var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, + _mockCache.Object, _mockLogger.Object); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -98,8 +98,8 @@ public void ShouldFetchSegmentsSuccessOnCacheHit() _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))).Returns(segmentsToCheck); _mockApiManager.Setup(a => a.FetchSegments(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, + _mockCache.Object, _mockLogger.Object); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -124,8 +124,8 @@ public void ShouldHandleFetchSegmentsWithError() _mockApiManager.Setup(a => a.FetchSegments(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())). Returns(null as string[]); - var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, + _mockCache.Object, _mockLogger.Object); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -143,8 +143,8 @@ public void ShouldHandleFetchSegmentsWithError() public void ShouldLogAndReturnAnEmptySetWhenNoSegmentsToCheck() { var odpConfig = new OdpConfig(API_KEY, API_HOST, new List(0)); - var manager = new OdpSegmentManager(odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, odpConfig, + _mockCache.Object, _mockLogger.Object); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -160,8 +160,8 @@ public void ShouldLogAndReturnNullWhenOdpConfigNotReady() { var mockOdpConfig = new Mock(API_KEY, API_HOST, new List(0)); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); - var manager = new OdpSegmentManager(mockOdpConfig.Object, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, mockOdpConfig.Object, + _mockCache.Object, _mockLogger.Object); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -174,8 +174,8 @@ public void ShouldLogAndReturnNullWhenOdpConfigNotReady() [Test] public void ShouldIgnoreCache() { - var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, + _mockCache.Object, _mockLogger.Object); manager.FetchQualifiedSegments(FS_USER_ID, new List { @@ -194,8 +194,8 @@ public void ShouldIgnoreCache() [Test] public void ShouldResetCache() { - var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, + _mockCache.Object, _mockLogger.Object); manager.FetchQualifiedSegments(FS_USER_ID, new List { @@ -216,8 +216,8 @@ public void ShouldMakeValidCacheKey() { var keyCollector = new List(); _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))); - var manager = new OdpSegmentManager(_odpConfig, _mockApiManager.Object, - Constants.DEFAULT_MAX_CACHE_SIZE, null, _mockLogger.Object, _mockCache.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, + _mockCache.Object, _mockLogger.Object); manager.FetchQualifiedSegments(FS_USER_ID); diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index e3a6118c..49388b86 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -222,7 +222,8 @@ public OdpManager Build(bool asEnabled = true) manager.EventManager = new OdpEventManager.Builder(). WithOdpConfig(_odpConfig). WithBatchSize(Constants.DEFAULT_MAX_CACHE_SIZE). - WithTimeoutInterval(TimeSpan.FromMilliseconds(Constants.DEFAULT_CACHE_SECONDS)). + WithTimeoutInterval( + TimeSpan.FromMilliseconds(Constants.DEFAULT_CACHE_SECONDS)). WithOdpEventApiManager(eventApiManager). WithLogger(_logger). WithErrorHandler(_errorHandler). @@ -237,11 +238,10 @@ public OdpManager Build(bool asEnabled = true) if (_segmentManager == null) { - var cacheTimeout = TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS); var apiManager = new OdpSegmentApiManager(_logger, _errorHandler); - manager.SegmentManager = new OdpSegmentManager(_odpConfig, apiManager, - Constants.DEFAULT_MAX_CACHE_SIZE, cacheTimeout, _logger, _cache); + manager.SegmentManager = + new OdpSegmentManager(apiManager, _odpConfig, _cache, _logger); } else { diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index c155a755..3fa10511 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -47,15 +47,32 @@ public class OdpSegmentManager : IOdpSegmentManager /// private readonly ICache> _segmentsCache; - public OdpSegmentManager(OdpConfig odpConfig, IOdpSegmentApiManager apiManager, - int? cacheSize = null, TimeSpan? itemTimeout = null, - ILogger logger = null, ICache> cache = null + public OdpSegmentManager(IOdpSegmentApiManager apiManager, + OdpConfig odpConfig, + ICache> cache = null, + ILogger logger = null ) { _apiManager = apiManager; _odpConfig = odpConfig; _logger = logger ?? new DefaultLogger(); + _segmentsCache = + cache ?? new LruCache>(Constants.DEFAULT_MAX_CACHE_SIZE, + TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS), logger); + } + + public OdpSegmentManager(IOdpSegmentApiManager apiManager, + int? cacheSize = null, + TimeSpan? itemTimeout = null, + ICache> cache = null, + ILogger logger = null + ) + { + _apiManager = apiManager; + _odpConfig = new OdpConfig(); + _logger = logger ?? new DefaultLogger(); + itemTimeout = itemTimeout ?? TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS); if (itemTimeout < TimeSpan.Zero) { From 41e01d80e62436f6100b3215ecd53b61168c4693 Mon Sep 17 00:00:00 2001 From: msohailhussain Date: Mon, 9 Jan 2023 16:47:01 -0800 Subject: [PATCH 03/19] proposed changes --- .../Config/FallbackProjectConfigManager.cs | 9 +++++ .../Config/HttpProjectConfigManager.cs | 21 ++++++++-- .../Config/PollingProjectConfigManager.cs | 1 + OptimizelySDK/Config/ProjectConfigManager.cs | 3 ++ .../Notifications/NotificationRegistry.cs | 34 ++++++++++++++++ OptimizelySDK/Odp/LruCache.cs | 1 + OptimizelySDK/Odp/OdpEventManager.cs | 7 ---- OptimizelySDK/Odp/OdpManager.cs | 30 ++++++++------ OptimizelySDK/Odp/OdpSegmentManager.cs | 5 +-- OptimizelySDK/Optimizely.cs | 39 +++++++++---------- OptimizelySDK/OptimizelySDK.csproj | 1 + 11 files changed, 104 insertions(+), 47 deletions(-) create mode 100644 OptimizelySDK/Notifications/NotificationRegistry.cs diff --git a/OptimizelySDK/Config/FallbackProjectConfigManager.cs b/OptimizelySDK/Config/FallbackProjectConfigManager.cs index b1044e45..5529dd81 100644 --- a/OptimizelySDK/Config/FallbackProjectConfigManager.cs +++ b/OptimizelySDK/Config/FallbackProjectConfigManager.cs @@ -45,6 +45,15 @@ public ProjectConfig GetConfig() return ProjectConfig; } + public string SdkKey + { + get + { + return null; + } + + } + /// /// Access to current cached project configuration /// diff --git a/OptimizelySDK/Config/HttpProjectConfigManager.cs b/OptimizelySDK/Config/HttpProjectConfigManager.cs index 8b909755..b95bdae7 100644 --- a/OptimizelySDK/Config/HttpProjectConfigManager.cs +++ b/OptimizelySDK/Config/HttpProjectConfigManager.cs @@ -29,18 +29,30 @@ public class HttpProjectConfigManager : PollingProjectConfigManager private string Url; private string LastModifiedSince = string.Empty; private string DatafileAccessToken = string.Empty; - private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, bool autoUpdate, ILogger logger, IErrorHandler errorHandler) + private string _sdkKey = string.Empty; + + private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string sdkKey) : base(period, blockingTimeout, autoUpdate, logger, errorHandler) { Url = url; + _sdkKey = sdkKey; } - private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string datafileAccessToken) - : this(period, url, blockingTimeout, autoUpdate, logger, errorHandler) + private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string datafileAccessToken, string sdkKey + ) + : this(period, url, blockingTimeout, autoUpdate, logger, errorHandler, sdkKey) { DatafileAccessToken = datafileAccessToken; } + public string SdkKey + { + get + { + return _sdkKey; + } + } + public Task OnReady() { return CompletableConfigManager.Task; @@ -337,7 +349,7 @@ public HttpProjectConfigManager Build(bool defer) } - configManager = new HttpProjectConfigManager(Period, Url, BlockingTimeoutSpan, AutoUpdate, Logger, ErrorHandler, DatafileAccessToken); + configManager = new HttpProjectConfigManager(Period, Url, BlockingTimeoutSpan, AutoUpdate, Logger, ErrorHandler, DatafileAccessToken, SdkKey); if (Datafile != null) { @@ -354,6 +366,7 @@ public HttpProjectConfigManager Build(bool defer) configManager.NotifyOnProjectConfigUpdate += () => { NotificationCenter?.SendNotifications(NotificationCenter.NotificationType.OptimizelyConfigUpdate); + NotificationRegistry.GetNotificationCenter(SdkKey).SendNotifications(NotificationCenter.NotificationType.OptimizelyConfigUpdate); }; diff --git a/OptimizelySDK/Config/PollingProjectConfigManager.cs b/OptimizelySDK/Config/PollingProjectConfigManager.cs index e21319fa..0b618ace 100644 --- a/OptimizelySDK/Config/PollingProjectConfigManager.cs +++ b/OptimizelySDK/Config/PollingProjectConfigManager.cs @@ -46,6 +46,7 @@ public abstract class PollingProjectConfigManager : ProjectConfigManager, protected IErrorHandler ErrorHandler { get; set; } protected TimeSpan BlockingTimeout; + public virtual string SdkKey { get; } protected TaskCompletionSource CompletableConfigManager = new TaskCompletionSource(); diff --git a/OptimizelySDK/Config/ProjectConfigManager.cs b/OptimizelySDK/Config/ProjectConfigManager.cs index 7fe9753e..4bc38ab3 100644 --- a/OptimizelySDK/Config/ProjectConfigManager.cs +++ b/OptimizelySDK/Config/ProjectConfigManager.cs @@ -27,6 +27,9 @@ public interface ProjectConfigManager /// ProjectConfig instance ProjectConfig GetConfig(); + + string SdkKey { get; } + /// /// Access to current cached project configuration /// diff --git a/OptimizelySDK/Notifications/NotificationRegistry.cs b/OptimizelySDK/Notifications/NotificationRegistry.cs new file mode 100644 index 00000000..8ec6cd1d --- /dev/null +++ b/OptimizelySDK/Notifications/NotificationRegistry.cs @@ -0,0 +1,34 @@ +using OptimizelySDK.Logger; +using System; +using System.Collections.Generic; +namespace OptimizelySDK.Notifications +{ + internal class NotificationRegistry + { + private static object _mutex = new object(); + private static Dictionary _notificationCenters; + + private NotificationRegistry() + { + } + + public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger logger = null) + { + NotificationCenter notificationCenter; + lock(_mutex) + { + if (_notificationCenters.ContainsKey(sdkKey)) + { + notificationCenter = _notificationCenters[sdkKey]; + } + else + { + notificationCenter = new NotificationCenter(logger); + _notificationCenters[sdkKey] = notificationCenter; + } + } + return notificationCenter; + } + } +} + diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index a475c117..1a6282d2 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -61,6 +61,7 @@ public LruCache(int? maxSize = null, ILogger logger = null ) { + //TODO: Please add a condition to check minimum value of maxSize as well. _mutex = new object(); _maxSize = Math.Max(0, maxSize ?? Constants.DEFAULT_MAX_CACHE_SIZE); diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 5fe3a3c7..a5cf5626 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -402,7 +402,6 @@ public class Builder private BlockingCollection _eventQueue = new BlockingCollection(Constants.DEFAULT_QUEUE_CAPACITY); - private OdpConfig _odpConfig; private IOdpEventApiManager _odpEventApiManager; private int _batchSize; private TimeSpan _flushInterval; @@ -416,11 +415,6 @@ public Builder WithEventQueue(BlockingCollection eventQueue) return this; } - public Builder WithOdpConfig(OdpConfig odpConfig) - { - _odpConfig = odpConfig; - return this; - } public Builder WithOdpEventApiManager(IOdpEventApiManager odpEventApiManager) { @@ -467,7 +461,6 @@ public OdpEventManager Build(bool startImmediately = true) { var manager = new OdpEventManager(); manager._eventQueue = _eventQueue; - manager._odpConfig = _odpConfig; manager._odpEventApiManager = _odpEventApiManager; manager._batchSize = _batchSize < 1 ? Constants.DEFAULT_BATCH_SIZE : _batchSize; diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 49388b86..5e07f6ec 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -155,7 +155,8 @@ public class Builder private ILogger _logger; private IErrorHandler _errorHandler; private ICache> _cache; - + private int? _maxSize; + private TimeSpan? _itemTimeout; public Builder WithSegmentManager(IOdpSegmentManager segmentManager) { _segmentManager = segmentManager; @@ -168,12 +169,6 @@ public Builder WithEventManager(IOdpEventManager eventManager) return this; } - public Builder WithOdpConfig(OdpConfig odpConfig) - { - _odpConfig = odpConfig; - return this; - } - public Builder WithLogger(ILogger logger = null) { _logger = logger; @@ -186,12 +181,21 @@ public Builder WithErrorHandler(IErrorHandler errorHandler = null) return this; } - public Builder WithCacheImplementation(ICache> cache) + public Builder WithCache(ICache> cache) { _cache = cache; return this; } + public Builder WithCache(int? maxSize = null, + TimeSpan? itemTimeout = null) + { + _maxSize = maxSize; + _itemTimeout = itemTimeout; + + return this; + } + /// /// Build OdpManager instance using collected parameters /// @@ -201,11 +205,9 @@ public OdpManager Build(bool asEnabled = true) { _logger = _logger ?? new DefaultLogger(); _errorHandler = _errorHandler ?? new NoOpErrorHandler(); - _odpConfig = _odpConfig ?? new OdpConfig(); var manager = new OdpManager { - _odpConfig = _odpConfig, _logger = _logger, _enabled = asEnabled, }; @@ -220,7 +222,6 @@ public OdpManager Build(bool asEnabled = true) var eventApiManager = new OdpEventApiManager(_logger, _errorHandler); manager.EventManager = new OdpEventManager.Builder(). - WithOdpConfig(_odpConfig). WithBatchSize(Constants.DEFAULT_MAX_CACHE_SIZE). WithTimeoutInterval( TimeSpan.FromMilliseconds(Constants.DEFAULT_CACHE_SECONDS)). @@ -241,7 +242,7 @@ public OdpManager Build(bool asEnabled = true) var apiManager = new OdpSegmentApiManager(_logger, _errorHandler); manager.SegmentManager = - new OdpSegmentManager(apiManager, _odpConfig, _cache, _logger); + new OdpSegmentManager(apiManager, this.GetCache(), _logger); } else { @@ -250,6 +251,11 @@ public OdpManager Build(bool asEnabled = true) return manager; } + + private ICache> GetCache() + { + return _cache ?? new LruCache>(_maxSize, _itemTimeout, _logger); + } } /// diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 3fa10511..162c0212 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -48,13 +48,11 @@ public class OdpSegmentManager : IOdpSegmentManager private readonly ICache> _segmentsCache; public OdpSegmentManager(IOdpSegmentApiManager apiManager, - OdpConfig odpConfig, ICache> cache = null, ILogger logger = null ) { _apiManager = apiManager; - _odpConfig = odpConfig; _logger = logger ?? new DefaultLogger(); _segmentsCache = @@ -70,7 +68,6 @@ public OdpSegmentManager(IOdpSegmentApiManager apiManager, ) { _apiManager = apiManager; - _odpConfig = new OdpConfig(); _logger = logger ?? new DefaultLogger(); itemTimeout = itemTimeout ?? TimeSpan.FromSeconds(Constants.DEFAULT_CACHE_SECONDS); @@ -98,7 +95,7 @@ public List FetchQualifiedSegments(string fsUserId, List options = null ) { - if (!_odpConfig.IsReady()) + if (_odpConfig == null || !_odpConfig.IsReady()) { _logger.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE); return null; diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 0c184a31..1c710002 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -164,7 +164,10 @@ public Optimizely(string datafile, var config = DatafileProjectConfig.Create(datafile, Logger, ErrorHandler); ProjectConfigManager = new FallbackProjectConfigManager(config); #if USE_ODP - SetupOdp(config); + // Don't need to setup notification for update settings when DF is changed, because this is for hardcoded datafile. + // Not supposed to change using this method. + OdpManager.UpdateSettings(config.PublicKeyForOdp, config.HostForOdp, + config.Segments.ToList()); #endif } else @@ -215,8 +218,8 @@ public Optimizely(ProjectConfigManager configManager, #if USE_ODP InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions, odpManager); + SetupOdp(configManager.SdkKey); - SetupOdp(ProjectConfigManager.CachedProjectConfig); #else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions); @@ -255,27 +258,23 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null, } #if USE_ODP - private void SetupOdp(ProjectConfig config) + private void SetupOdp(string sdkKey) { - if (config == null) - { - return; - } - - var odpConfig = new OdpConfig(config.PublicKeyForOdp, config.HostForOdp, - config.Segments.ToList()); - OdpManager = new OdpManager.Builder(). - WithOdpConfig(odpConfig). - WithLogger(Logger). - WithErrorHandler(ErrorHandler). - Build(); - NotificationCenter.AddNotification( - NotificationCenter.NotificationType.OptimizelyConfigUpdate, - () => OdpManager?.UpdateSettings(config.PublicKeyForOdp, config.HostForOdp, - config.Segments.ToList()) - ); + NotificationRegistry.GetNotificationCenter(sdkKey). + AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, + () => + { + var projectConfig = this.ProjectConfigManager.CachedProjectConfig; + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, + projectConfig.Segments.ToList()); + }); + + // in case if notification is lost. + var projectConfig = this.ProjectConfigManager.CachedProjectConfig; + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, + projectConfig.Segments.ToList()); } #endif diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index b667a2fb..55851693 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -195,6 +195,7 @@ + From 7118ae40ad0065c01c559186eb22217c63578bee Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 10 Jan 2023 08:52:22 -0500 Subject: [PATCH 04/19] Understanding & additional updates --- .../Config/FallbackProjectConfigManager.cs | 6 +- .../Config/HttpProjectConfigManager.cs | 140 ++++++++++++------ OptimizelySDK/Config/ProjectConfigManager.cs | 6 +- .../Notifications/NotificationRegistry.cs | 28 +++- OptimizelySDK/Odp/LruCache.cs | 15 +- OptimizelySDK/Odp/OdpManager.cs | 11 +- OptimizelySDK/Optimizely.cs | 45 +++--- 7 files changed, 159 insertions(+), 92 deletions(-) diff --git a/OptimizelySDK/Config/FallbackProjectConfigManager.cs b/OptimizelySDK/Config/FallbackProjectConfigManager.cs index 5529dd81..fcd68709 100644 --- a/OptimizelySDK/Config/FallbackProjectConfigManager.cs +++ b/OptimizelySDK/Config/FallbackProjectConfigManager.cs @@ -1,5 +1,5 @@ /* - * Copyright 2019, 2022 Optimizely + * Copyright 2019, 2022-2023 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -45,13 +45,15 @@ public ProjectConfig GetConfig() return ProjectConfig; } + /// + /// SDK Key for Fallback is not used and always null + /// public string SdkKey { get { return null; } - } /// diff --git a/OptimizelySDK/Config/HttpProjectConfigManager.cs b/OptimizelySDK/Config/HttpProjectConfigManager.cs index b95bdae7..0323f6c0 100644 --- a/OptimizelySDK/Config/HttpProjectConfigManager.cs +++ b/OptimizelySDK/Config/HttpProjectConfigManager.cs @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021, Optimizely + * Copyright 2019-2021, 2023 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,20 +31,27 @@ public class HttpProjectConfigManager : PollingProjectConfigManager private string DatafileAccessToken = string.Empty; private string _sdkKey = string.Empty; - private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string sdkKey) + private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, + bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string sdkKey + ) : base(period, blockingTimeout, autoUpdate, logger, errorHandler) { Url = url; _sdkKey = sdkKey; } - private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string datafileAccessToken, string sdkKey - ) + private HttpProjectConfigManager(TimeSpan period, string url, TimeSpan blockingTimeout, + bool autoUpdate, ILogger logger, IErrorHandler errorHandler, string datafileAccessToken, + string sdkKey + ) : this(period, url, blockingTimeout, autoUpdate, logger, errorHandler, sdkKey) { DatafileAccessToken = datafileAccessToken; } + /// + /// SDK key in use for this project + /// public string SdkKey { get @@ -71,21 +78,26 @@ public HttpClient() public HttpClient(System.Net.Http.HttpClient httpClient) : this() { - if (httpClient != null) { + if (httpClient != null) + { Client = httpClient; } } public static System.Net.Http.HttpClientHandler GetHttpClientHandler() { - var handler = new System.Net.Http.HttpClientHandler() { - AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate + var handler = new System.Net.Http.HttpClientHandler() + { + AutomaticDecompression = System.Net.DecompressionMethods.GZip | + System.Net.DecompressionMethods.Deflate }; return handler; } - public virtual Task SendAsync(System.Net.Http.HttpRequestMessage httpRequestMessage) + public virtual Task SendAsync( + System.Net.Http.HttpRequestMessage httpRequestMessage + ) { return Client.SendAsync(httpRequestMessage); } @@ -96,12 +108,13 @@ public static System.Net.Http.HttpClientHandler GetHttpClientHandler() static HttpProjectConfigManager() { Client = new HttpClient(); - } + } private string GetRemoteDatafileResponse() { Logger.Log(LogLevel.DEBUG, $"Making datafile request to url \"{Url}\""); - var request = new System.Net.Http.HttpRequestMessage { + var request = new System.Net.Http.HttpRequestMessage + { RequestUri = new Uri(Url), Method = System.Net.Http.HttpMethod.Get, }; @@ -110,16 +123,20 @@ private string GetRemoteDatafileResponse() if (!string.IsNullOrEmpty(LastModifiedSince)) request.Headers.Add("If-Modified-Since", LastModifiedSince); - if (!string.IsNullOrEmpty(DatafileAccessToken)) { - request.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", DatafileAccessToken); + if (!string.IsNullOrEmpty(DatafileAccessToken)) + { + request.Headers.Authorization = + new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", + DatafileAccessToken); } - var httpResponse = Client.SendAsync(request); + var httpResponse = Client.SendAsync(request); httpResponse.Wait(); // Return from here if datafile is not modified. var result = httpResponse.Result; - if (!result.IsSuccessStatusCode) { + if (!result.IsSuccessStatusCode) + { Logger.Log(LogLevel.ERROR, $"Error fetching datafile \"{result.StatusCode}\""); return null; } @@ -134,9 +151,9 @@ private string GetRemoteDatafileResponse() var content = result.Content.ReadAsStringAsync(); content.Wait(); - return content.Result; + return content.Result; } -#elif NET40 +#elif NET40 private string GetRemoteDatafileResponse() { var request = (System.Net.HttpWebRequest)System.Net.WebRequest.Create(Url); @@ -176,19 +193,24 @@ protected override ProjectConfig Poll() return DatafileProjectConfig.Create(datafile, Logger, ErrorHandler); } - + public class Builder { private const long MAX_MILLISECONDS_LIMIT = 4294967294; private readonly TimeSpan DEFAULT_PERIOD = TimeSpan.FromMinutes(5); private readonly TimeSpan DEFAULT_BLOCKINGOUT_PERIOD = TimeSpan.FromSeconds(15); - private readonly string DEFAULT_FORMAT = "https://cdn.optimizely.com/datafiles/{0}.json"; - private readonly string DEFAULT_AUTHENTICATED_DATAFILE_FORMAT = "https://config.optimizely.com/datafiles/auth/{0}.json"; + + private readonly string DEFAULT_FORMAT = + "https://cdn.optimizely.com/datafiles/{0}.json"; + + private readonly string DEFAULT_AUTHENTICATED_DATAFILE_FORMAT = + "https://config.optimizely.com/datafiles/auth/{0}.json"; + private string Datafile; - private string DatafileAccessToken; + private string DatafileAccessToken; private string SdkKey; private string Url; - private string Format; + private string Format; private ILogger Logger; private IErrorHandler ErrorHandler; private TimeSpan Period; @@ -209,6 +231,7 @@ public Builder WithBlockingTimeoutPeriod(TimeSpan blockingTimeoutSpan) return this; } + public Builder WithDatafile(string datafile) { Datafile = datafile; @@ -253,7 +276,7 @@ public Builder WithFormat(string format) return this; } - + public Builder WithLogger(ILogger logger) { Logger = logger; @@ -275,7 +298,7 @@ public Builder WithAutoUpdate(bool autoUpdate) return this; } - public Builder WithStartByDefault(bool startByDefault=true) + public Builder WithStartByDefault(bool startByDefault = true) { StartByDefault = startByDefault; @@ -315,41 +338,62 @@ public HttpProjectConfigManager Build(bool defer) if (ErrorHandler == null) ErrorHandler = new DefaultErrorHandler(Logger, false); - if (string.IsNullOrEmpty(Format)) { - - if (string.IsNullOrEmpty(DatafileAccessToken)) { + if (string.IsNullOrEmpty(Format)) + { + if (string.IsNullOrEmpty(DatafileAccessToken)) + { Format = DEFAULT_FORMAT; - } else { + } + else + { Format = DEFAULT_AUTHENTICATED_DATAFILE_FORMAT; } } - if (string.IsNullOrEmpty(Url)) { - if (string.IsNullOrEmpty(SdkKey)) { + if (string.IsNullOrEmpty(Url)) + { + if (string.IsNullOrEmpty(SdkKey)) + { ErrorHandler.HandleError(new Exception("SdkKey cannot be null")); } + Url = string.Format(Format, SdkKey); } - if (IsPollingIntervalProvided && (Period.TotalMilliseconds <= 0 || Period.TotalMilliseconds > MAX_MILLISECONDS_LIMIT)) { - Logger.Log(LogLevel.DEBUG, $"Polling interval is not valid for periodic calls, using default period {DEFAULT_PERIOD.TotalMilliseconds}ms"); + if (IsPollingIntervalProvided && (Period.TotalMilliseconds <= 0 || + Period.TotalMilliseconds > + MAX_MILLISECONDS_LIMIT)) + { + Logger.Log(LogLevel.DEBUG, + $"Polling interval is not valid for periodic calls, using default period {DEFAULT_PERIOD.TotalMilliseconds}ms"); Period = DEFAULT_PERIOD; - } else if(!IsPollingIntervalProvided) { - Logger.Log(LogLevel.DEBUG, $"No polling interval provided, using default period {DEFAULT_PERIOD.TotalMilliseconds}ms"); + } + else if (!IsPollingIntervalProvided) + { + Logger.Log(LogLevel.DEBUG, + $"No polling interval provided, using default period {DEFAULT_PERIOD.TotalMilliseconds}ms"); Period = DEFAULT_PERIOD; } - - if (IsBlockingTimeoutProvided && (BlockingTimeoutSpan.TotalMilliseconds <= 0 || BlockingTimeoutSpan.TotalMilliseconds > MAX_MILLISECONDS_LIMIT)) { - Logger.Log(LogLevel.DEBUG, $"Blocking timeout is not valid, using default blocking timeout {DEFAULT_BLOCKINGOUT_PERIOD.TotalMilliseconds}ms"); + + if (IsBlockingTimeoutProvided && (BlockingTimeoutSpan.TotalMilliseconds <= 0 || + BlockingTimeoutSpan.TotalMilliseconds > + MAX_MILLISECONDS_LIMIT)) + { + Logger.Log(LogLevel.DEBUG, + $"Blocking timeout is not valid, using default blocking timeout {DEFAULT_BLOCKINGOUT_PERIOD.TotalMilliseconds}ms"); BlockingTimeoutSpan = DEFAULT_BLOCKINGOUT_PERIOD; - } else if(!IsBlockingTimeoutProvided) { - Logger.Log(LogLevel.DEBUG, $"No Blocking timeout provided, using default blocking timeout {DEFAULT_BLOCKINGOUT_PERIOD.TotalMilliseconds}ms"); + } + else if (!IsBlockingTimeoutProvided) + { + Logger.Log(LogLevel.DEBUG, + $"No Blocking timeout provided, using default blocking timeout {DEFAULT_BLOCKINGOUT_PERIOD.TotalMilliseconds}ms"); BlockingTimeoutSpan = DEFAULT_BLOCKINGOUT_PERIOD; } - - configManager = new HttpProjectConfigManager(Period, Url, BlockingTimeoutSpan, AutoUpdate, Logger, ErrorHandler, DatafileAccessToken, SdkKey); + + configManager = new HttpProjectConfigManager(Period, Url, BlockingTimeoutSpan, + AutoUpdate, Logger, ErrorHandler, DatafileAccessToken, SdkKey); if (Datafile != null) { @@ -363,12 +407,16 @@ public HttpProjectConfigManager Build(bool defer) Logger.Log(LogLevel.WARN, "Error parsing fallback datafile." + ex.Message); } } - - configManager.NotifyOnProjectConfigUpdate += () => { - NotificationCenter?.SendNotifications(NotificationCenter.NotificationType.OptimizelyConfigUpdate); - NotificationRegistry.GetNotificationCenter(SdkKey).SendNotifications(NotificationCenter.NotificationType.OptimizelyConfigUpdate); + + configManager.NotifyOnProjectConfigUpdate += () => + { + NotificationCenter?.SendNotifications(NotificationCenter.NotificationType. + OptimizelyConfigUpdate); + NotificationRegistry.GetNotificationCenter(SdkKey). + SendNotifications( + NotificationCenter.NotificationType.OptimizelyConfigUpdate); }; - + if (StartByDefault) configManager.Start(); @@ -376,7 +424,7 @@ public HttpProjectConfigManager Build(bool defer) // Optionally block until config is available. if (!defer) configManager.GetConfig(); - + return configManager; } } diff --git a/OptimizelySDK/Config/ProjectConfigManager.cs b/OptimizelySDK/Config/ProjectConfigManager.cs index 4bc38ab3..bdd4e839 100644 --- a/OptimizelySDK/Config/ProjectConfigManager.cs +++ b/OptimizelySDK/Config/ProjectConfigManager.cs @@ -1,5 +1,5 @@ /* - * Copyright 2019, 2022 Optimizely + * Copyright 2019, 2022-2023 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +27,9 @@ public interface ProjectConfigManager /// ProjectConfig instance ProjectConfig GetConfig(); - + /// + /// SDK key in use for this project + /// string SdkKey { get; } /// diff --git a/OptimizelySDK/Notifications/NotificationRegistry.cs b/OptimizelySDK/Notifications/NotificationRegistry.cs index 8ec6cd1d..a9735e03 100644 --- a/OptimizelySDK/Notifications/NotificationRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationRegistry.cs @@ -1,6 +1,22 @@ -using OptimizelySDK.Logger; -using System; +/* + * Copyright 2023, 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 OptimizelySDK.Logger; using System.Collections.Generic; + namespace OptimizelySDK.Notifications { internal class NotificationRegistry @@ -8,14 +24,12 @@ internal class NotificationRegistry private static object _mutex = new object(); private static Dictionary _notificationCenters; - private NotificationRegistry() - { - } + private NotificationRegistry() { } public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger logger = null) { NotificationCenter notificationCenter; - lock(_mutex) + lock (_mutex) { if (_notificationCenters.ContainsKey(sdkKey)) { @@ -27,8 +41,8 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo _notificationCenters[sdkKey] = notificationCenter; } } + return notificationCenter; } } } - diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index 1a6282d2..ee7fe148 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -1,5 +1,5 @@ /* - * Copyright 2022, Optimizely + * Copyright 2022-2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,8 +22,7 @@ namespace OptimizelySDK.Odp { - public class LruCache : ICache - where T : class + public class LruCache : ICache where T : class { /// /// The maximum number of elements that should be stored @@ -61,9 +60,17 @@ public LruCache(int? maxSize = null, ILogger logger = null ) { - //TODO: Please add a condition to check minimum value of maxSize as well. _mutex = new object(); + // TODO: Please add a condition to check minimum value of maxSize as well. + // @msohailhussain What's the minimum value for maxSize? + // Thinking... + // maxSize ==> ____ + // null ==> 10,000 (DEFAULT_MAX_CACHE_SIZE) + // -1 ==> 0 + // 1 ==> 1 + // 100 ==> 100 + // 20,000 ==> 20,000 _maxSize = Math.Max(0, maxSize ?? Constants.DEFAULT_MAX_CACHE_SIZE); _logger = logger ?? new DefaultLogger(); diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 5e07f6ec..bcf25277 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -1,5 +1,5 @@ /* - * Copyright 2022 Optimizely + * Copyright 2022-2023 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -157,6 +157,7 @@ public class Builder private ICache> _cache; private int? _maxSize; private TimeSpan? _itemTimeout; + public Builder WithSegmentManager(IOdpSegmentManager segmentManager) { _segmentManager = segmentManager; @@ -188,11 +189,12 @@ public Builder WithCache(ICache> cache) } public Builder WithCache(int? maxSize = null, - TimeSpan? itemTimeout = null) + TimeSpan? itemTimeout = null + ) { _maxSize = maxSize; _itemTimeout = itemTimeout; - + return this; } @@ -241,8 +243,7 @@ public OdpManager Build(bool asEnabled = true) { var apiManager = new OdpSegmentApiManager(_logger, _errorHandler); - manager.SegmentManager = - new OdpSegmentManager(apiManager, this.GetCache(), _logger); + manager.SegmentManager = new OdpSegmentManager(apiManager, GetCache(), _logger); } else { diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 1c710002..08d06802 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017-2022, Optimizely + * Copyright 2017-2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use file except in compliance with the License. @@ -164,10 +164,10 @@ public Optimizely(string datafile, var config = DatafileProjectConfig.Create(datafile, Logger, ErrorHandler); ProjectConfigManager = new FallbackProjectConfigManager(config); #if USE_ODP - // Don't need to setup notification for update settings when DF is changed, because this is for hardcoded datafile. - // Not supposed to change using this method. + // No need to setup notification for datafile updates. This constructor + // is for hardcoded datafile which should not be changed using this method. OdpManager.UpdateSettings(config.PublicKeyForOdp, config.HostForOdp, - config.Segments.ToList()); + config.Segments.ToList()); #endif } else @@ -218,7 +218,21 @@ public Optimizely(ProjectConfigManager configManager, #if USE_ODP InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions, odpManager); - SetupOdp(configManager.SdkKey); + + var projectConfig = ProjectConfigManager.CachedProjectConfig; + + NotificationRegistry.GetNotificationCenter(sdkKey). + AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, + () => + { + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, + projectConfig.HostForOdp, + projectConfig.Segments.ToList()); + }); + + // in case if notification is lost. + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, + projectConfig.Segments.ToList()); #else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, @@ -257,27 +271,6 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null, #endif } -#if USE_ODP - private void SetupOdp(string sdkKey) - { - - - NotificationRegistry.GetNotificationCenter(sdkKey). - AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, - () => - { - var projectConfig = this.ProjectConfigManager.CachedProjectConfig; - OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, - projectConfig.Segments.ToList()); - }); - - // in case if notification is lost. - var projectConfig = this.ProjectConfigManager.CachedProjectConfig; - OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, - projectConfig.Segments.ToList()); - } -#endif - /// /// Buckets visitor and sends impression event to Optimizely. /// From 7e643316999f9dcfe31d8633f7f89235d0d188eb Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 10 Jan 2023 11:58:43 -0500 Subject: [PATCH 05/19] WIP Refactors & test fixes --- .../OdpTests/OdpEventManagerTests.cs | 39 ++++++++++++------- .../OdpTests/OdpManagerTest.cs | 35 ++++++++--------- .../OdpTests/OdpSegmentManagerTest.cs | 39 +++++++++++-------- .../Notifications/NotificationRegistry.cs | 8 +++- OptimizelySDK/Odp/OdpManager.cs | 1 - OptimizelySDK/Optimizely.cs | 2 +- 6 files changed, 73 insertions(+), 51 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index 0fea8a82..00d9ce79 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -155,10 +155,11 @@ public void Setup() [Test] public void ShouldLogAndDiscardEventsWhenEventManagerNotRunning() { - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(startImmediately: false); + eventManager.UpdateSettings(_odpConfig); // since we've not called start() then... eventManager.SendEvent(_testEvents[0]); @@ -174,10 +175,11 @@ public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady() { var mockOdpConfig = new Mock(API_KEY, API_HOST, _emptySegmentsToCheck); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(startImmediately: false); // doing it manually in Act next + eventManager.UpdateSettings(mockOdpConfig.Object); eventManager.Start(); // Log when Start() called eventManager.SendEvent(_testEvents[0]); // Log when enqueue attempted @@ -192,10 +194,11 @@ public void ShouldLogWhenOdpNotIntegratedAndIdentifyUserCalled() { var mockOdpConfig = new Mock(API_KEY, API_HOST, _emptySegmentsToCheck); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(); + eventManager.UpdateSettings(mockOdpConfig.Object); eventManager.IdentifyUser(FS_USER_ID); @@ -209,10 +212,11 @@ public void ShouldLogWhenOdpNotIntegratedAndStartCalled() { var mockOdpConfig = new Mock(API_KEY, API_HOST, _emptySegmentsToCheck); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(mockOdpConfig.Object). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(startImmediately: false); // doing it manually in Act next + eventManager.UpdateSettings(mockOdpConfig.Object); eventManager.Start(); @@ -250,10 +254,11 @@ public void ShouldDiscardEventsWithInvalidData() "key-3", new DateTime() }, }); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(); + eventManager.UpdateSettings(_odpConfig); eventManager.SendEvent(eventWithAnArray); eventManager.SendEvent(eventWithADate); @@ -271,13 +276,14 @@ public void ShouldAddAdditionalInformationToEachEvent() _mockApiManager.Setup(api => api.SendEvents(It.IsAny(), It.IsAny(), Capture.In(eventsCollector))). Callback(() => cde.Signal()); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithEventQueue(new BlockingCollection(10)). // max capacity of 10 WithBatchSize(10). WithFlushInterval(TimeSpan.FromMilliseconds(100)). Build(); + eventManager.UpdateSettings(_odpConfig); eventManager.SendEvent(_testEvents[0]); cde.Wait(); @@ -304,13 +310,14 @@ public void ShouldAddAdditionalInformationToEachEvent() [Test] public void ShouldAttemptToFlushAnEmptyQueueAtFlushInterval() { - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithEventQueue(new BlockingCollection(10)). WithBatchSize(10). WithFlushInterval(TimeSpan.FromMilliseconds(100)). Build(); + eventManager.UpdateSettings(_odpConfig); // do not add events to the queue, but allow for // at least 3 flush intervals executions @@ -328,13 +335,14 @@ public void ShouldDispatchEventsInCorrectNumberOfBatches() a.SendEvents(It.IsAny(), It.IsAny(), It.IsAny>())). Returns(false); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithEventQueue(new BlockingCollection(10)). WithBatchSize(10). WithFlushInterval(TimeSpan.FromMilliseconds(500)). Build(); + eventManager.UpdateSettings(_odpConfig); for (int i = 0; i < 25; i++) { @@ -362,12 +370,13 @@ public void ShouldDispatchEventsWithCorrectPayload() Capture.In(eventCollector))). Callback(() => cde.Signal()). Returns(false); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithBatchSize(10). WithFlushInterval(TimeSpan.FromSeconds(1)). Build(); + eventManager.UpdateSettings(_odpConfig); _testEvents.ForEach(e => eventManager.SendEvent(e)); cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS); @@ -395,13 +404,14 @@ public void ShouldRetryFailedEvents() It.IsAny>())). Callback(() => cde.Signal()). Returns(true); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithEventQueue(new BlockingCollection(10)). WithBatchSize(2). WithFlushInterval(TimeSpan.FromMilliseconds(100)). Build(); + eventManager.UpdateSettings(_odpConfig); for (int i = 0; i < 4; i++) { @@ -419,13 +429,14 @@ public void ShouldRetryFailedEvents() [Test] public void ShouldFlushAllScheduledEventsBeforeStopping() { - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithEventQueue(new BlockingCollection(100)). WithBatchSize(2). // small batch size WithFlushInterval(TimeSpan.FromSeconds(2)). // long flush interval Build(); + eventManager.UpdateSettings(_odpConfig); for (int i = 0; i < 25; i++) { @@ -453,12 +464,13 @@ public void ShouldPrepareCorrectPayloadForIdentifyUser() _mockApiManager.Setup(api => api.SendEvents(It.IsAny(), It.IsAny(), Capture.In(eventsCollector))). Callback(() => cde.Signal()); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). WithEventQueue(new BlockingCollection(1)). WithBatchSize(1). Build(); + eventManager.UpdateSettings(_odpConfig); eventManager.IdentifyUser(USER_ID); cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS); @@ -488,10 +500,11 @@ public void ShouldApplyUpdatedOdpConfigurationWhenAvailable() "1-item-cart", }; var differentOdpConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); - var eventManager = new OdpEventManager.Builder().WithOdpConfig(_odpConfig). + var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). Build(); + eventManager.UpdateSettings(_odpConfig); eventManager.UpdateSettings(differentOdpConfig); diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 2f943a0c..24cd8559 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -67,7 +67,7 @@ public class OdpManagerTest private readonly List _emptySegmentsToCheck = new List(0); - private OdpConfig _odpConfig; + private Mock _mockLogger; private Mock _mockOdpEventManager; private Mock _mockSegmentManager; @@ -75,7 +75,6 @@ public class OdpManagerTest [SetUp] public void Setup() { - _odpConfig = new OdpConfig(API_KEY, API_HOST, _emptySegmentsToCheck); _mockLogger = new Mock(); _mockOdpEventManager = new Mock(); _mockSegmentManager = new Mock(); @@ -86,7 +85,7 @@ public void ShouldStartEventManagerWhenOdpManagerIsInitialized() { _mockOdpEventManager.Setup(e => e.Start()); - _ = new OdpManager.Builder().WithOdpConfig(_odpConfig). + _ = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -99,7 +98,7 @@ public void ShouldStartEventManagerWhenOdpManagerIsInitialized() public void ShouldStopEventManagerWhenCloseIsCalled() { _mockOdpEventManager.Setup(e => e.Stop()); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -116,7 +115,7 @@ public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() var eventManagerParameterCollector = new List(); _mockOdpEventManager.Setup(e => e.UpdateSettings(Capture.In(eventManagerParameterCollector))); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -138,7 +137,7 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() var segmentManagerParameterCollector = new List(); _mockSegmentManager.Setup(s => s.UpdateSettings(Capture.In(segmentManagerParameterCollector))); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -157,16 +156,17 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() [Test] public void ShouldHandleOdpConfigSettingsNoChange() { + var odpConfig = new OdpConfig(API_KEY, API_HOST, _emptySegmentsToCheck); _mockSegmentManager.Setup(s => s.UpdateSettings(It.IsAny())); _mockOdpEventManager.Setup(e => e.UpdateSettings(It.IsAny())); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); - var wasUpdated = manager.UpdateSettings(_odpConfig.ApiKey, _odpConfig.ApiHost, - _odpConfig.SegmentsToCheck); + var wasUpdated = manager.UpdateSettings(odpConfig.ApiKey, odpConfig.ApiHost, + odpConfig.SegmentsToCheck); Assert.IsFalse(wasUpdated); _mockSegmentManager.Verify(s => s.UpdateSettings(It.IsAny()), Times.Never); @@ -178,7 +178,7 @@ public void ShouldUpdateSettingsWithReset() { _mockSegmentManager.Setup(s => s.UpdateSettings(It.IsAny())); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -196,7 +196,7 @@ public void ShouldDisableOdpThroughConfiguration() { _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); @@ -225,7 +225,7 @@ public void ShouldDisableOdpThroughConfiguration() [Test] public void ShouldGetEventManager() { - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -237,7 +237,7 @@ public void ShouldGetEventManager() [Test] public void ShouldGetSegmentManager() { - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). @@ -251,7 +251,7 @@ public void ShouldIdentifyUserWhenOdpIsIntegrated() { _mockOdpEventManager.Setup(e => e.IdentifyUser(It.IsAny())); _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); @@ -268,7 +268,7 @@ public void ShouldNotIdentifyUserWhenOdpDisabled() { _mockOdpEventManager.Setup(e => e.IdentifyUser(It.IsAny())); _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(false); @@ -286,7 +286,7 @@ public void ShouldSendEventWhenOdpIsIntegrated() { _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); - var manager = new OdpManager.Builder().WithOdpConfig(_odpConfig). + var manager = new OdpManager.Builder(). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); @@ -301,9 +301,8 @@ public void ShouldSendEventWhenOdpIsIntegrated() [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). + var manager = new OdpManager.Builder(). WithLogger(_mockLogger.Object). Build(false); // do not enable diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index 8b45bc48..78d6b56e 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -69,8 +69,9 @@ public void ShouldFetchSegmentsOnCacheMiss() _mockApiManager.Setup(a => a.FetchSegments(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())). Returns(segmentsToCheck.ToArray()); - var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, + _mockLogger.Object); + manager.UpdateSettings(_odpConfig); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -98,8 +99,9 @@ public void ShouldFetchSegmentsSuccessOnCacheHit() _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))).Returns(segmentsToCheck); _mockApiManager.Setup(a => a.FetchSegments(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, + _mockLogger.Object); + manager.UpdateSettings(_odpConfig); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -124,8 +126,9 @@ public void ShouldHandleFetchSegmentsWithError() _mockApiManager.Setup(a => a.FetchSegments(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())). Returns(null as string[]); - var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, + _mockLogger.Object); + manager.UpdateSettings(_odpConfig); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -143,8 +146,9 @@ public void ShouldHandleFetchSegmentsWithError() public void ShouldLogAndReturnAnEmptySetWhenNoSegmentsToCheck() { var odpConfig = new OdpConfig(API_KEY, API_HOST, new List(0)); - var manager = new OdpSegmentManager(_mockApiManager.Object, odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, + _mockLogger.Object); + manager.UpdateSettings(odpConfig); var segments = manager.FetchQualifiedSegments(FS_USER_ID); @@ -160,9 +164,10 @@ public void ShouldLogAndReturnNullWhenOdpConfigNotReady() { var mockOdpConfig = new Mock(API_KEY, API_HOST, new List(0)); mockOdpConfig.Setup(o => o.IsReady()).Returns(false); - var manager = new OdpSegmentManager(_mockApiManager.Object, mockOdpConfig.Object, - _mockCache.Object, _mockLogger.Object); - + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, + _mockLogger.Object); + manager.UpdateSettings(mockOdpConfig.Object); + var segments = manager.FetchQualifiedSegments(FS_USER_ID); Assert.IsNull(segments); @@ -174,8 +179,8 @@ public void ShouldLogAndReturnNullWhenOdpConfigNotReady() [Test] public void ShouldIgnoreCache() { - var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, _mockLogger.Object); + manager.UpdateSettings(_odpConfig); manager.FetchQualifiedSegments(FS_USER_ID, new List { @@ -194,8 +199,8 @@ public void ShouldIgnoreCache() [Test] public void ShouldResetCache() { - var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, _mockLogger.Object); + manager.UpdateSettings(_odpConfig); manager.FetchQualifiedSegments(FS_USER_ID, new List { @@ -216,8 +221,8 @@ public void ShouldMakeValidCacheKey() { var keyCollector = new List(); _mockCache.Setup(c => c.Lookup(Capture.In(keyCollector))); - var manager = new OdpSegmentManager(_mockApiManager.Object, _odpConfig, - _mockCache.Object, _mockLogger.Object); + var manager = new OdpSegmentManager(_mockApiManager.Object, _mockCache.Object, _mockLogger.Object); + manager.UpdateSettings(_odpConfig); manager.FetchQualifiedSegments(FS_USER_ID); diff --git a/OptimizelySDK/Notifications/NotificationRegistry.cs b/OptimizelySDK/Notifications/NotificationRegistry.cs index a9735e03..235152a7 100644 --- a/OptimizelySDK/Notifications/NotificationRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationRegistry.cs @@ -21,11 +21,17 @@ namespace OptimizelySDK.Notifications { internal class NotificationRegistry { - private static object _mutex = new object(); + private static readonly object _mutex = new object(); private static Dictionary _notificationCenters; private NotificationRegistry() { } + /// + /// Thread-safe access to the NotificationCenter + /// + /// Retrieve NotificationCenter based on SDK key + /// Logger to record events + /// NotificationCenter instance per SDK key public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger logger = null) { NotificationCenter notificationCenter; diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index bcf25277..3f77acf1 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -149,7 +149,6 @@ public void Dispose() /// public class Builder { - private OdpConfig _odpConfig; private IOdpEventManager _eventManager; private IOdpSegmentManager _segmentManager; private ILogger _logger; diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 08d06802..530d8690 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -221,7 +221,7 @@ public Optimizely(ProjectConfigManager configManager, var projectConfig = ProjectConfigManager.CachedProjectConfig; - NotificationRegistry.GetNotificationCenter(sdkKey). + NotificationRegistry.GetNotificationCenter(configManager.SdkKey). AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, () => { From bf16dd93356136533148a551b5f9418d8ab4903b Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 10 Jan 2023 16:10:52 -0500 Subject: [PATCH 06/19] WIP correcting tests... --- .../OdpTests/OdpEventManagerTests.cs | 2 +- OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs | 14 ++++++++++++-- OptimizelySDK/Odp/OdpEventManager.cs | 2 +- OptimizelySDK/Odp/OdpManager.cs | 7 ++++--- OptimizelySDK/Optimizely.cs | 1 - 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index 00d9ce79..202f2831 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -286,7 +286,7 @@ public void ShouldAddAdditionalInformationToEachEvent() eventManager.UpdateSettings(_odpConfig); eventManager.SendEvent(_testEvents[0]); - cde.Wait(); + cde.Wait(MAX_COUNT_DOWN_EVENT_WAIT_MS); var eventsSentToApi = eventsCollector.FirstOrDefault(); var actualEvent = eventsSentToApi?.FirstOrDefault(); diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index 24cd8559..ad97bd52 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -67,7 +67,6 @@ public class OdpManagerTest private readonly List _emptySegmentsToCheck = new List(0); - private Mock _mockLogger; private Mock _mockOdpEventManager; private Mock _mockSegmentManager; @@ -103,7 +102,8 @@ public void ShouldStopEventManagerWhenCloseIsCalled() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); - + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); + manager.Dispose(); _mockOdpEventManager.Verify(e => e.Stop(), Times.Once); @@ -120,6 +120,7 @@ public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -142,6 +143,7 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -183,6 +185,7 @@ public void ShouldUpdateSettingsWithReset() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -200,6 +203,7 @@ public void ShouldDisableOdpThroughConfiguration() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); @@ -230,6 +234,7 @@ public void ShouldGetEventManager() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); Assert.IsNotNull(manager.EventManager); } @@ -242,6 +247,7 @@ public void ShouldGetSegmentManager() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); Assert.IsNotNull(manager.SegmentManager); } @@ -255,6 +261,7 @@ public void ShouldIdentifyUserWhenOdpIsIntegrated() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); manager.IdentifyUser(VALID_FS_USER_ID); manager.Dispose(); @@ -272,6 +279,7 @@ public void ShouldNotIdentifyUserWhenOdpDisabled() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(false); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); manager.IdentifyUser(VALID_FS_USER_ID); manager.Dispose(); @@ -290,6 +298,7 @@ public void ShouldSendEventWhenOdpIsIntegrated() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); @@ -305,6 +314,7 @@ public void ShouldNotSendEventOdpNotIntegrated() var manager = new OdpManager.Builder(). WithLogger(_mockLogger.Object). Build(false); // do not enable + manager.UpdateSettings(string.Empty, string.Empty, _emptySegmentsToCheck); manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index a5cf5626..a526e0da 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -107,7 +107,7 @@ private void DropQueue() /// public void Start() { - if (!_odpConfig.IsReady()) + if (_odpConfig == null || !_odpConfig.IsReady()) { _logger.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE); diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 3f77acf1..32020a3c 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -64,7 +64,7 @@ private OdpManager() { } public bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck) { var newConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); - if (_odpConfig.Equals(newConfig)) + if (_odpConfig == null || _odpConfig.Equals(newConfig)) { return false; } @@ -264,7 +264,7 @@ private ICache> GetCache() /// True if EventManager can process events otherwise False private bool EventManagerOrConfigNotReady() { - return EventManager == null || !_enabled || !_odpConfig.IsReady(); + return EventManager == null || !_enabled || _odpConfig == null || !_odpConfig.IsReady(); } /// @@ -273,7 +273,8 @@ private bool EventManagerOrConfigNotReady() /// True if SegmentManager can fetch audience segments otherwise False private bool SegmentManagerOrConfigNotReady() { - return SegmentManager == null || !_enabled || !_odpConfig.IsReady(); + return SegmentManager == null || !_enabled || _odpConfig == null || + !_odpConfig.IsReady(); } } } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 530d8690..6379b914 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -233,7 +233,6 @@ public Optimizely(ProjectConfigManager configManager, // in case if notification is lost. OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, projectConfig.Segments.ToList()); - #else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions); From 6b829a2685d132f8c9db643a31df1992ce949c00 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 11 Jan 2023 09:18:39 -0500 Subject: [PATCH 07/19] Refactors --- OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs | 2 +- OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs | 2 +- OptimizelySDK/Config/HttpProjectConfigManager.cs | 4 ++-- OptimizelySDK/Config/PollingProjectConfigManager.cs | 2 +- ...{NotificationRegistry.cs => NotificationCenterRegistry.cs} | 4 +--- OptimizelySDK/Odp/OdpEventManager.cs | 2 +- OptimizelySDK/Optimizely.cs | 2 +- OptimizelySDK/OptimizelySDK.csproj | 2 +- 8 files changed, 9 insertions(+), 11 deletions(-) rename OptimizelySDK/Notifications/{NotificationRegistry.cs => NotificationCenterRegistry.cs} (95%) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index 202f2831..0bb349d4 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -1,5 +1,5 @@ /* - * Copyright 2022, Optimizely + * Copyright 2022-2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index ad97bd52..f07b209b 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -1,5 +1,5 @@ /* - * Copyright 2022 Optimizely + * Copyright 2022-2023 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/OptimizelySDK/Config/HttpProjectConfigManager.cs b/OptimizelySDK/Config/HttpProjectConfigManager.cs index 0323f6c0..2ba52b3e 100644 --- a/OptimizelySDK/Config/HttpProjectConfigManager.cs +++ b/OptimizelySDK/Config/HttpProjectConfigManager.cs @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * 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, @@ -412,7 +412,7 @@ public HttpProjectConfigManager Build(bool defer) { NotificationCenter?.SendNotifications(NotificationCenter.NotificationType. OptimizelyConfigUpdate); - NotificationRegistry.GetNotificationCenter(SdkKey). + NotificationCenterRegistry.GetNotificationCenter(SdkKey). SendNotifications( NotificationCenter.NotificationType.OptimizelyConfigUpdate); }; diff --git a/OptimizelySDK/Config/PollingProjectConfigManager.cs b/OptimizelySDK/Config/PollingProjectConfigManager.cs index 0b618ace..b7022a3b 100644 --- a/OptimizelySDK/Config/PollingProjectConfigManager.cs +++ b/OptimizelySDK/Config/PollingProjectConfigManager.cs @@ -1,5 +1,5 @@ /* - * Copyright 2019-2020, 2022 Optimizely + * Copyright 2019-2020, 2022-2023 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/OptimizelySDK/Notifications/NotificationRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs similarity index 95% rename from OptimizelySDK/Notifications/NotificationRegistry.cs rename to OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 235152a7..2ad3075a 100644 --- a/OptimizelySDK/Notifications/NotificationRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -19,13 +19,11 @@ namespace OptimizelySDK.Notifications { - internal class NotificationRegistry + internal static class NotificationCenterRegistry { private static readonly object _mutex = new object(); private static Dictionary _notificationCenters; - private NotificationRegistry() { } - /// /// Thread-safe access to the NotificationCenter /// diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index a526e0da..fa738890 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -1,5 +1,5 @@ /* - * Copyright 2022, Optimizely + * Copyright 2022-2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 6379b914..c2bfe652 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -221,7 +221,7 @@ public Optimizely(ProjectConfigManager configManager, var projectConfig = ProjectConfigManager.CachedProjectConfig; - NotificationRegistry.GetNotificationCenter(configManager.SdkKey). + NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey). AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, () => { diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 55851693..b27a1f8a 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -195,7 +195,7 @@ - + From fead3e99026fdcd085c8189dd5951e24b13ef57a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 11 Jan 2023 16:52:31 -0500 Subject: [PATCH 08/19] WIP OdpEventManagerTest resolutions --- .../OdpTests/OdpEventManagerTests.cs | 16 ++++++++-------- OptimizelySDK/Odp/OdpEventManager.cs | 19 ++++++++++++++++--- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index 0bb349d4..9c457b05 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -161,7 +161,7 @@ public void ShouldLogAndDiscardEventsWhenEventManagerNotRunning() Build(startImmediately: false); eventManager.UpdateSettings(_odpConfig); - // since we've not called start() then... + // since we've not called Start() then... eventManager.SendEvent(_testEvents[0]); // ...we should get a notice after trying to send an event @@ -174,11 +174,11 @@ public void ShouldLogAndDiscardEventsWhenEventManagerNotRunning() public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady() { var mockOdpConfig = new Mock(API_KEY, API_HOST, _emptySegmentsToCheck); - mockOdpConfig.Setup(o => o.IsReady()).Returns(false); + mockOdpConfig.Setup(o => o.IsReady()).Returns(false); // stay not ready var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). - Build(startImmediately: false); // doing it manually in Act next + Build(startImmediately: false); // start manually in Act; Log once here eventManager.UpdateSettings(mockOdpConfig.Object); eventManager.Start(); // Log when Start() called @@ -186,7 +186,7 @@ public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady() _mockLogger.Verify( l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), - Times.Exactly(2)); + Times.Exactly(3)); } [Test] @@ -211,17 +211,17 @@ public void ShouldLogWhenOdpNotIntegratedAndIdentifyUserCalled() public void ShouldLogWhenOdpNotIntegratedAndStartCalled() { var mockOdpConfig = new Mock(API_KEY, API_HOST, _emptySegmentsToCheck); - mockOdpConfig.Setup(o => o.IsReady()).Returns(false); + mockOdpConfig.Setup(o => o.IsReady()).Returns(false); // since never ready var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). - Build(startImmediately: false); // doing it manually in Act next + Build(startImmediately: false); // doing it manually in Act next; Log once eventManager.UpdateSettings(mockOdpConfig.Object); - eventManager.Start(); + eventManager.Start(); // Log again _mockLogger.Verify(l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), - Times.Once); + Times.Exactly(2)); } [Test] diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index fa738890..be2e2989 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -266,7 +266,7 @@ public void Stop() /// Event to enqueue public void SendEvent(OdpEvent odpEvent) { - if (!_odpConfig.IsReady()) + if (_odpConfig == null || !_odpConfig.IsReady()) { _logger.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE); return; @@ -349,7 +349,21 @@ public void IdentifyUser(string userId) /// Configuration object containing new values public void UpdateSettings(OdpConfig odpConfig) { - _odpConfig = odpConfig; + if (odpConfig == null) + { + return; + } + + if (_odpConfig == null) + { + _odpConfig = odpConfig; + + Start(); + } + else + { + _odpConfig = odpConfig; + } } /// @@ -415,7 +429,6 @@ public Builder WithEventQueue(BlockingCollection eventQueue) return this; } - public Builder WithOdpEventApiManager(IOdpEventApiManager odpEventApiManager) { _odpEventApiManager = odpEventApiManager; From b8b5f0ab1be4b89e35ef61c840605267549febff Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 14:20:56 -0500 Subject: [PATCH 09/19] Remove NotificationCenter from NotificationCenterRegistry on Dispose --- .../Notifications/NotificationCenterRegistry.cs | 13 +++++++++++++ OptimizelySDK/Optimizely.cs | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 2ad3075a..11deb1a8 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -48,5 +48,18 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo return notificationCenter; } + + /// + /// Thread-safe removal of a NotificationCenter from the Registry + /// + /// SDK key identifying the target + /// true if the element is successfully found and removed; otherwise, false + public static bool RemoveNotificationCenter(string sdkKey) + { + lock (_mutex) + { + return _notificationCenters.Remove(sdkKey); + } + } } } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index c2bfe652..f71a9ca3 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1491,7 +1491,7 @@ public void Dispose() { if (Disposed) return; - Disposed = true; + NotificationCenterRegistry.RemoveNotificationCenter(ProjectConfigManager.SdkKey); (ProjectConfigManager as IDisposable)?.Dispose(); (EventProcessor as IDisposable)?.Dispose(); @@ -1501,6 +1501,7 @@ public void Dispose() #if USE_ODP OdpManager?.Dispose(); #endif + Disposed = true; } } } From 1144b0c99d7f5c2f64f726e35d46a77405bc3e7c Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 14:23:20 -0500 Subject: [PATCH 10/19] Fix Disposed = true location --- OptimizelySDK/Optimizely.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index f71a9ca3..dfb07cb9 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1491,6 +1491,8 @@ public void Dispose() { if (Disposed) return; + Disposed = true; + NotificationCenterRegistry.RemoveNotificationCenter(ProjectConfigManager.SdkKey); (ProjectConfigManager as IDisposable)?.Dispose(); @@ -1501,7 +1503,6 @@ public void Dispose() #if USE_ODP OdpManager?.Dispose(); #endif - Disposed = true; } } } From f64ca5558cfcaa4c3d080e8ddb2ed880901745f7 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 14:35:36 -0500 Subject: [PATCH 11/19] Modify NCR Dispose --- OptimizelySDK/Optimizely.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index dfb07cb9..960def3d 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1493,9 +1493,16 @@ public void Dispose() Disposed = true; - NotificationCenterRegistry.RemoveNotificationCenter(ProjectConfigManager.SdkKey); + if (ProjectConfigManager is IDisposable) + { + NotificationCenterRegistry.GetNotificationCenter(ProjectConfigManager.SdkKey). + RemoveNotification(NotificationCenter.NotificationId); + + NotificationCenterRegistry.RemoveNotificationCenter(ProjectConfigManager.SdkKey); + + (ProjectConfigManager as IDisposable)?.Dispose(); + } - (ProjectConfigManager as IDisposable)?.Dispose(); (EventProcessor as IDisposable)?.Dispose(); ProjectConfigManager = null; From 5b7b674503e8aa2014190a15e8d53706821dc1b3 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 14:42:00 -0500 Subject: [PATCH 12/19] Mods to NCR and NC disposal --- .../NotificationCenterRegistry.cs | 8 ++++--- OptimizelySDK/Odp/OdpEventManager.cs | 23 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 11deb1a8..4fcd242b 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -53,12 +53,14 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo /// Thread-safe removal of a NotificationCenter from the Registry /// /// SDK key identifying the target - /// true if the element is successfully found and removed; otherwise, false - public static bool RemoveNotificationCenter(string sdkKey) + public static void RemoveNotificationCenter(string sdkKey) { + var notificationCenterToRemove = GetNotificationCenter(sdkKey); + notificationCenterToRemove.ClearAllNotifications(); + lock (_mutex) { - return _notificationCenters.Remove(sdkKey); + _notificationCenters.Remove(sdkKey); } } } diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index be2e2989..a1db8fda 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -91,6 +91,11 @@ public class OdpEventManager : IOdpEventManager, IDisposable /// private Dictionary _commonData; + /// + /// Indicates if OdpEventManager should start upon Build() and UpdateSettings() + /// + private bool _autoStart; + /// /// Clear all entries from the queue /// @@ -422,6 +427,18 @@ public class Builder private TimeSpan _timeoutInterval; private ILogger _logger; private IErrorHandler _errorHandler; + private bool? _autoStart; + + /// + /// Indicates if OdpEventManager should start upon Build() and UpdateSettings() + /// + /// + /// + public Builder WithAutoStart(bool autoStart) + { + _autoStart = autoStart; + return this; + } public Builder WithEventQueue(BlockingCollection eventQueue) { @@ -468,9 +485,8 @@ public Builder WithErrorHandler(IErrorHandler errorHandler = null) /// /// Build OdpEventManager instance using collected parameters /// - /// Should start event processor upon initialization /// OdpEventProcessor instance - public OdpEventManager Build(bool startImmediately = true) + public OdpEventManager Build() { var manager = new OdpEventManager(); manager._eventQueue = _eventQueue; @@ -485,6 +501,7 @@ public OdpEventManager Build(bool startImmediately = true) _timeoutInterval; manager._logger = _logger ?? new NoOpLogger(); manager._errorHandler = _errorHandler ?? new NoOpErrorHandler(); + manager._autoStart = _autoStart ?? true; manager._validOdpDataTypes = new List() { @@ -516,7 +533,7 @@ public OdpEventManager Build(bool startImmediately = true) }, }; - if (startImmediately) + if (manager._autoStart) { manager.Start(); } From 59d8995cff402fe981a4dfe92480ac6c3746c19d Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 14:42:00 -0500 Subject: [PATCH 13/19] Mods to NCR and NC disposal --- .../NotificationCenterRegistry.cs | 8 ++++--- OptimizelySDK/Odp/OdpEventManager.cs | 23 ++++++++++++++++--- OptimizelySDK/Optimizely.cs | 14 +++++------ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 11deb1a8..4fcd242b 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -53,12 +53,14 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo /// Thread-safe removal of a NotificationCenter from the Registry /// /// SDK key identifying the target - /// true if the element is successfully found and removed; otherwise, false - public static bool RemoveNotificationCenter(string sdkKey) + public static void RemoveNotificationCenter(string sdkKey) { + var notificationCenterToRemove = GetNotificationCenter(sdkKey); + notificationCenterToRemove.ClearAllNotifications(); + lock (_mutex) { - return _notificationCenters.Remove(sdkKey); + _notificationCenters.Remove(sdkKey); } } } diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index be2e2989..a1db8fda 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -91,6 +91,11 @@ public class OdpEventManager : IOdpEventManager, IDisposable /// private Dictionary _commonData; + /// + /// Indicates if OdpEventManager should start upon Build() and UpdateSettings() + /// + private bool _autoStart; + /// /// Clear all entries from the queue /// @@ -422,6 +427,18 @@ public class Builder private TimeSpan _timeoutInterval; private ILogger _logger; private IErrorHandler _errorHandler; + private bool? _autoStart; + + /// + /// Indicates if OdpEventManager should start upon Build() and UpdateSettings() + /// + /// + /// + public Builder WithAutoStart(bool autoStart) + { + _autoStart = autoStart; + return this; + } public Builder WithEventQueue(BlockingCollection eventQueue) { @@ -468,9 +485,8 @@ public Builder WithErrorHandler(IErrorHandler errorHandler = null) /// /// Build OdpEventManager instance using collected parameters /// - /// Should start event processor upon initialization /// OdpEventProcessor instance - public OdpEventManager Build(bool startImmediately = true) + public OdpEventManager Build() { var manager = new OdpEventManager(); manager._eventQueue = _eventQueue; @@ -485,6 +501,7 @@ public OdpEventManager Build(bool startImmediately = true) _timeoutInterval; manager._logger = _logger ?? new NoOpLogger(); manager._errorHandler = _errorHandler ?? new NoOpErrorHandler(); + manager._autoStart = _autoStart ?? true; manager._validOdpDataTypes = new List() { @@ -516,7 +533,7 @@ public OdpEventManager Build(bool startImmediately = true) }, }; - if (startImmediately) + if (manager._autoStart) { manager.Start(); } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 960def3d..205b357a 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1489,24 +1489,22 @@ private object GetTypeCastedVariableValue(string value, string type) public void Dispose() { - if (Disposed) return; - + if (Disposed) + { + return; + } Disposed = true; if (ProjectConfigManager is IDisposable) { - NotificationCenterRegistry.GetNotificationCenter(ProjectConfigManager.SdkKey). - RemoveNotification(NotificationCenter.NotificationId); - NotificationCenterRegistry.RemoveNotificationCenter(ProjectConfigManager.SdkKey); (ProjectConfigManager as IDisposable)?.Dispose(); } - - (EventProcessor as IDisposable)?.Dispose(); - ProjectConfigManager = null; + (EventProcessor as IDisposable)?.Dispose(); + #if USE_ODP OdpManager?.Dispose(); #endif From 1f094220d59dbb03eda65d151f9bf9f7832ff32c Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 14:57:33 -0500 Subject: [PATCH 14/19] Fix OdpEventManager & tests --- .../OdpTests/OdpEventManagerTests.cs | 27 ++++++++++--------- OptimizelySDK/Odp/OdpEventManager.cs | 10 +++---- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index 9c457b05..2fd54753 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -158,7 +158,8 @@ public void ShouldLogAndDiscardEventsWhenEventManagerNotRunning() var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). - Build(startImmediately: false); + WithAutoStart(false). + Build(); eventManager.UpdateSettings(_odpConfig); // since we've not called Start() then... @@ -178,7 +179,8 @@ public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady() var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). - Build(startImmediately: false); // start manually in Act; Log once here + WithAutoStart(false). // start manually in Act + Build(); eventManager.UpdateSettings(mockOdpConfig.Object); eventManager.Start(); // Log when Start() called @@ -186,25 +188,25 @@ public void ShouldLogAndDiscardEventsWhenEventManagerConfigNotReady() _mockLogger.Verify( l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), - Times.Exactly(3)); + Times.Exactly(2)); } [Test] public void ShouldLogWhenOdpNotIntegratedAndIdentifyUserCalled() { var mockOdpConfig = new Mock(API_KEY, API_HOST, _emptySegmentsToCheck); - mockOdpConfig.Setup(o => o.IsReady()).Returns(false); + mockOdpConfig.Setup(o => o.IsReady()).Returns(false); // never ready var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). - Build(); - eventManager.UpdateSettings(mockOdpConfig.Object); + Build(); // assumed AutoStart true; Logs 1x here + eventManager.UpdateSettings(mockOdpConfig.Object); // auto-start after update; Logs 1x here - eventManager.IdentifyUser(FS_USER_ID); + eventManager.IdentifyUser(FS_USER_ID); // Logs 1x here too _mockLogger.Verify( l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), - Times.Exactly(2)); // during Start() and SendEvent() + Times.Exactly(3)); // during Start() and SendEvent() } [Test] @@ -215,13 +217,14 @@ public void ShouldLogWhenOdpNotIntegratedAndStartCalled() var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). WithLogger(_mockLogger.Object). - Build(startImmediately: false); // doing it manually in Act next; Log once - eventManager.UpdateSettings(mockOdpConfig.Object); + WithAutoStart(false). // doing it manually in Act next + Build(); + eventManager.UpdateSettings(mockOdpConfig.Object); - eventManager.Start(); // Log again + eventManager.Start(); // Log 1x here too _mockLogger.Verify(l => l.Log(LogLevel.WARN, Constants.ODP_NOT_INTEGRATED_MESSAGE), - Times.Exactly(2)); + Times.Once); } [Test] diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index a1db8fda..1aa87069 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -359,15 +359,11 @@ public void UpdateSettings(OdpConfig odpConfig) return; } - if (_odpConfig == null) - { - _odpConfig = odpConfig; + _odpConfig = odpConfig; - Start(); - } - else + if (_autoStart) { - _odpConfig = odpConfig; + Start(); } } From 91e6720d7bf04ba976b169fa2bf4d1749adc0e73 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 12 Jan 2023 15:36:04 -0500 Subject: [PATCH 15/19] Remove double lock in NotificationCenterRegistry --- .../Notifications/NotificationCenterRegistry.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 4fcd242b..3f58cc20 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -55,12 +55,13 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo /// SDK key identifying the target public static void RemoveNotificationCenter(string sdkKey) { - var notificationCenterToRemove = GetNotificationCenter(sdkKey); - notificationCenterToRemove.ClearAllNotifications(); - lock (_mutex) { - _notificationCenters.Remove(sdkKey); + if (_notificationCenters.TryGetValue(sdkKey, out NotificationCenter notificationCenter)) + { + notificationCenter.ClearAllNotifications(); + _notificationCenters.Remove(sdkKey); + } } } } From 5d6b79742b82443038b2e182255d1450c494e18b Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 13 Jan 2023 14:01:41 -0500 Subject: [PATCH 16/19] Fix final test corrections --- .../OptimizelySDK.NetStandard20.csproj | 6 ++-- .../OdpTests/OdpManagerTest.cs | 36 ++++++++++++------- .../Config/HttpProjectConfigManager.cs | 7 ++++ .../NotificationCenterRegistry.cs | 17 +++++++-- OptimizelySDK/Odp/OdpManager.cs | 7 +--- OptimizelySDK/Optimizely.cs | 34 ++++++++++-------- 6 files changed, 69 insertions(+), 38 deletions(-) diff --git a/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj b/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj index 67870037..66204977 100644 --- a/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj +++ b/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj @@ -10,6 +10,9 @@ IOptimizely.cs + + Notifications\NotificationCenterRegistry.cs + Odp\Constants.cs @@ -40,9 +43,6 @@ Odp\Entity\OdpEvent.cs - - Odp\Entity\OptimizelySdkSettings.cs - Odp\Entity\Response.cs diff --git a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs index f07b209b..d9d89c24 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpManagerTest.cs @@ -103,7 +103,7 @@ public void ShouldStopEventManagerWhenCloseIsCalled() WithLogger(_mockLogger.Object). Build(); manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); - + manager.Dispose(); _mockOdpEventManager.Verify(e => e.Stop(), Times.Once); @@ -120,7 +120,6 @@ public void ShouldUseNewSettingsInEventManagerWhenOdpConfigIsUpdated() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); - manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -143,7 +142,6 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); - manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); @@ -158,7 +156,6 @@ public void ShouldUseNewSettingsInSegmentManagerWhenOdpConfigIsUpdated() [Test] public void ShouldHandleOdpConfigSettingsNoChange() { - var odpConfig = new OdpConfig(API_KEY, API_HOST, _emptySegmentsToCheck); _mockSegmentManager.Setup(s => s.UpdateSettings(It.IsAny())); _mockOdpEventManager.Setup(e => e.UpdateSettings(It.IsAny())); var manager = new OdpManager.Builder(). @@ -166,9 +163,12 @@ public void ShouldHandleOdpConfigSettingsNoChange() WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); + manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); // initial set + _mockOdpEventManager.ResetCalls(); + _mockSegmentManager.ResetCalls(); - var wasUpdated = manager.UpdateSettings(odpConfig.ApiKey, odpConfig.ApiHost, - odpConfig.SegmentsToCheck); + // attempt to set with the same config + var wasUpdated = manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); Assert.IsFalse(wasUpdated); _mockSegmentManager.Verify(s => s.UpdateSettings(It.IsAny()), Times.Never); @@ -178,36 +178,44 @@ public void ShouldHandleOdpConfigSettingsNoChange() [Test] public void ShouldUpdateSettingsWithReset() { - _mockSegmentManager.Setup(s => - s.UpdateSettings(It.IsAny())); + _mockOdpEventManager.Setup(e => e.UpdateSettings(It.IsAny())); + _mockSegmentManager.Setup(s => s.ResetCache()); + _mockSegmentManager.Setup(s => s.UpdateSettings(It.IsAny())); var manager = new OdpManager.Builder(). WithSegmentManager(_mockSegmentManager.Object). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(); - manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); var wasUpdated = manager.UpdateSettings(UPDATED_API_KEY, UPDATED_ODP_ENDPOINT, _updatedSegmentsToCheck); Assert.IsTrue(wasUpdated); + _mockOdpEventManager.Verify(e=>e.UpdateSettings(It.IsAny()), Times.Once); _mockSegmentManager.Verify(s => s.ResetCache(), Times.Once); + _mockSegmentManager.Verify(s=>s.UpdateSettings(It.IsAny()), Times.Once); } [Test] public void ShouldDisableOdpThroughConfiguration() { - _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); + _mockOdpEventManager.Setup(e => e.Start()); _mockOdpEventManager.Setup(e => e.IsStarted).Returns(true); + _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); + _mockOdpEventManager.Setup(e => e.UpdateSettings(It.IsAny())); var manager = new OdpManager.Builder(). WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). - Build(); - manager.UpdateSettings(API_KEY, API_HOST, _emptySegmentsToCheck); + Build(); // auto-start event manager attempted, but no config + manager.UpdateSettings(API_KEY, API_HOST, + _emptySegmentsToCheck); // event manager config added + auto-start + // should send event manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); + _mockOdpEventManager.Verify(e => e.Start(), Times.Once); + _mockOdpEventManager.Verify(e => e.UpdateSettings(It.IsAny()), Times.Once); _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Once); _mockLogger.Verify(l => l.Log(LogLevel.ERROR, "ODP event not dispatched (ODP disabled)."), Times.Never); @@ -215,12 +223,13 @@ public void ShouldDisableOdpThroughConfiguration() _mockOdpEventManager.ResetCalls(); _mockLogger.ResetCalls(); + // remove config and try sending again manager.UpdateSettings(string.Empty, string.Empty, _emptySegmentsToCheck); - manager.SendEvent(TEST_EVENT_TYPE, TEST_EVENT_ACTION, _testEventIdentifiers, _testEventData); manager.Dispose(); + // should not try to send and provide a log message _mockOdpEventManager.Verify(e => e.SendEvent(It.IsAny()), Times.Never); _mockLogger.Verify(l => l.Log(LogLevel.ERROR, "ODP event not dispatched (ODP disabled)."), Times.Once); @@ -312,6 +321,7 @@ public void ShouldNotSendEventOdpNotIntegrated() { _mockOdpEventManager.Setup(e => e.SendEvent(It.IsAny())); var manager = new OdpManager.Builder(). + WithEventManager(_mockOdpEventManager.Object). WithLogger(_mockLogger.Object). Build(false); // do not enable manager.UpdateSettings(string.Empty, string.Empty, _emptySegmentsToCheck); diff --git a/OptimizelySDK/Config/HttpProjectConfigManager.cs b/OptimizelySDK/Config/HttpProjectConfigManager.cs index 2ba52b3e..9b0ee951 100644 --- a/OptimizelySDK/Config/HttpProjectConfigManager.cs +++ b/OptimizelySDK/Config/HttpProjectConfigManager.cs @@ -14,6 +14,10 @@ * limitations under the License. */ +#if !(NET35 || NET40 || NETSTANDARD1_6) +#define USE_ODP +#endif + using OptimizelySDK.ErrorHandler; using OptimizelySDK.Logger; using OptimizelySDK.Notifications; @@ -412,9 +416,12 @@ public HttpProjectConfigManager Build(bool defer) { NotificationCenter?.SendNotifications(NotificationCenter.NotificationType. OptimizelyConfigUpdate); + +#if USE_ODP NotificationCenterRegistry.GetNotificationCenter(SdkKey). SendNotifications( NotificationCenter.NotificationType.OptimizelyConfigUpdate); +#endif }; diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 3f58cc20..bd978f8a 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -22,7 +22,9 @@ namespace OptimizelySDK.Notifications internal static class NotificationCenterRegistry { private static readonly object _mutex = new object(); - private static Dictionary _notificationCenters; + + private static Dictionary _notificationCenters = + new Dictionary(); /// /// Thread-safe access to the NotificationCenter @@ -32,6 +34,11 @@ internal static class NotificationCenterRegistry /// NotificationCenter instance per SDK key public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger logger = null) { + if (sdkKey == null) + { + return default; + } + NotificationCenter notificationCenter; lock (_mutex) { @@ -55,9 +62,15 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo /// SDK key identifying the target public static void RemoveNotificationCenter(string sdkKey) { + if (sdkKey == null) + { + return; + } + lock (_mutex) { - if (_notificationCenters.TryGetValue(sdkKey, out NotificationCenter notificationCenter)) + if (_notificationCenters.TryGetValue(sdkKey, + out NotificationCenter notificationCenter)) { notificationCenter.ClearAllNotifications(); _notificationCenters.Remove(sdkKey); diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 32020a3c..740c5a87 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -64,7 +64,7 @@ private OdpManager() { } public bool UpdateSettings(string apiKey, string apiHost, List segmentsToCheck) { var newConfig = new OdpConfig(apiKey, apiHost, segmentsToCheck); - if (_odpConfig == null || _odpConfig.Equals(newConfig)) + if (_odpConfig != null && _odpConfig.Equals(newConfig)) { return false; } @@ -213,11 +213,6 @@ public OdpManager Build(bool asEnabled = true) _enabled = asEnabled, }; - if (!manager._enabled) - { - return manager; - } - if (_eventManager == null) { var eventApiManager = new OdpEventApiManager(_logger, _errorHandler); diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 205b357a..5c6cfa6f 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -220,19 +220,21 @@ public Optimizely(ProjectConfigManager configManager, notificationCenter, eventProcessor, defaultDecideOptions, odpManager); var projectConfig = ProjectConfigManager.CachedProjectConfig; - - NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey). - AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, - () => - { - OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, - projectConfig.HostForOdp, - projectConfig.Segments.ToList()); - }); - - // in case if notification is lost. - OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, - projectConfig.Segments.ToList()); + if (projectConfig != null) + { + NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey)?. + AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, + () => + { + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, + projectConfig.HostForOdp, + projectConfig.Segments.ToList()); + }); + + // in case if notification is lost. + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, + projectConfig.Segments.ToList()); + } #else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions); @@ -1493,18 +1495,22 @@ public void Dispose() { return; } + Disposed = true; if (ProjectConfigManager is IDisposable) { +#if USE_ODP NotificationCenterRegistry.RemoveNotificationCenter(ProjectConfigManager.SdkKey); +#endif (ProjectConfigManager as IDisposable)?.Dispose(); } + ProjectConfigManager = null; (EventProcessor as IDisposable)?.Dispose(); - + #if USE_ODP OdpManager?.Dispose(); #endif From f6891547b1fc6a3426260119963106f1d86a46b7 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 18 Jan 2023 11:35:35 -0500 Subject: [PATCH 17/19] Pass logger into NotificationCenterRegistry --- OptimizelySDK/Notifications/NotificationCenterRegistry.cs | 1 + OptimizelySDK/Optimizely.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index bd978f8a..8389926f 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -36,6 +36,7 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo { if (sdkKey == null) { + logger?.Log(LogLevel.INFO, "No SDK key provided to GetNotificationCenter"); return default; } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 5c6cfa6f..29fa46aa 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -222,7 +222,7 @@ public Optimizely(ProjectConfigManager configManager, var projectConfig = ProjectConfigManager.CachedProjectConfig; if (projectConfig != null) { - NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey)?. + NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey, logger)?. AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, () => { From 9dd3d6993f6a11d870f6e9d471f5f3b573f70cf9 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 18 Jan 2023 14:17:17 -0500 Subject: [PATCH 18/19] Last PR change request --- OptimizelySDK/Config/HttpProjectConfigManager.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK/Config/HttpProjectConfigManager.cs b/OptimizelySDK/Config/HttpProjectConfigManager.cs index 9b0ee951..54f82a83 100644 --- a/OptimizelySDK/Config/HttpProjectConfigManager.cs +++ b/OptimizelySDK/Config/HttpProjectConfigManager.cs @@ -414,14 +414,13 @@ public HttpProjectConfigManager Build(bool defer) configManager.NotifyOnProjectConfigUpdate += () => { - NotificationCenter?.SendNotifications(NotificationCenter.NotificationType. - OptimizelyConfigUpdate); - #if USE_ODP NotificationCenterRegistry.GetNotificationCenter(SdkKey). SendNotifications( NotificationCenter.NotificationType.OptimizelyConfigUpdate); #endif + NotificationCenter?.SendNotifications(NotificationCenter.NotificationType. + OptimizelyConfigUpdate); }; From 556cfbffaa8760ceaf20d6850267b92ecc43533f Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 18 Jan 2023 14:37:36 -0500 Subject: [PATCH 19/19] Remove TODO comments --- OptimizelySDK/Odp/LruCache.cs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/OptimizelySDK/Odp/LruCache.cs b/OptimizelySDK/Odp/LruCache.cs index ee7fe148..e6f3dd5c 100644 --- a/OptimizelySDK/Odp/LruCache.cs +++ b/OptimizelySDK/Odp/LruCache.cs @@ -61,16 +61,7 @@ public LruCache(int? maxSize = null, ) { _mutex = new object(); - - // TODO: Please add a condition to check minimum value of maxSize as well. - // @msohailhussain What's the minimum value for maxSize? - // Thinking... - // maxSize ==> ____ - // null ==> 10,000 (DEFAULT_MAX_CACHE_SIZE) - // -1 ==> 0 - // 1 ==> 1 - // 100 ==> 100 - // 20,000 ==> 20,000 + _maxSize = Math.Max(0, maxSize ?? Constants.DEFAULT_MAX_CACHE_SIZE); _logger = logger ?? new DefaultLogger();