From 63e9afb828d0f0ff0523fa419d7d3ae34247b5e5 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 19 Jan 2023 18:46:03 +0500 Subject: [PATCH 1/4] Some suggested changes to fix null pointer exception --- .../OdpTests/OdpSegmentManagerTest.cs | 4 ++-- OptimizelySDK/Config/DatafileProjectConfig.cs | 3 +-- OptimizelySDK/Odp/Enums.cs | 4 ++-- OptimizelySDK/Odp/OdpManager.cs | 2 +- OptimizelySDK/Odp/OdpSegmentApiManager.cs | 17 +---------------- OptimizelySDK/Odp/OdpSegmentManager.cs | 6 +++--- 6 files changed, 10 insertions(+), 26 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index d6bd82fe4..545179608 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -185,7 +185,7 @@ public void ShouldIgnoreCache() manager.FetchQualifiedSegments(FS_USER_ID, new List { - OdpSegmentOption.IgnoreCache, + OdpSegmentOption.IGNORE_CACHE, }); _mockCache.Verify(c => c.Reset(), Times.Never); @@ -206,7 +206,7 @@ public void ShouldResetCache() manager.FetchQualifiedSegments(FS_USER_ID, new List { - OdpSegmentOption.ResetCache, + OdpSegmentOption.RESET_CACHE, }); _mockCache.Verify(c => c.Reset(), Times.Once); diff --git a/OptimizelySDK/Config/DatafileProjectConfig.cs b/OptimizelySDK/Config/DatafileProjectConfig.cs index 74a9c62cd..aeaef420e 100644 --- a/OptimizelySDK/Config/DatafileProjectConfig.cs +++ b/OptimizelySDK/Config/DatafileProjectConfig.cs @@ -404,8 +404,7 @@ private void Initialize() } } } - - var integration = Integrations.FirstOrDefault(i => i.Key.ToLower() == "odp"); + var integration = Integrations.FirstOrDefault(i => i.Key == "odp"); HostForOdp = integration?.Host; PublicKeyForOdp = integration?.PublicKey; diff --git a/OptimizelySDK/Odp/Enums.cs b/OptimizelySDK/Odp/Enums.cs index 7aeb27535..c69923f64 100644 --- a/OptimizelySDK/Odp/Enums.cs +++ b/OptimizelySDK/Odp/Enums.cs @@ -32,7 +32,7 @@ public enum OdpUserKeyType /// public enum OdpSegmentOption { - IgnoreCache = 0, - ResetCache = 1, + IGNORE_CACHE = 0, + RESET_CACHE = 1, } } diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 740c5a877..64d755ae5 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -93,7 +93,7 @@ public string[] FetchQualifiedSegments(string userId, List opt return null; } - return SegmentManager.FetchQualifiedSegments(userId, options).ToArray(); + return SegmentManager.FetchQualifiedSegments(userId, options)?.ToArray(); } /// diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index 95ea81da8..44d25350d 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -150,22 +150,7 @@ IEnumerable segmentsToCheck { return @"{ - ""query"": ""{ - query($userId: String, $audiences: [String]) { - { - customer({userKey}: $userId) { - audiences(subset: $audiences) { - edges { - node { - name - state - } - } - } - } - } - } - }"", + ""query"": ""query($userId: String, $audiences: [String]) {customer({userKey}: $userId) {audiences(subset: $audiences) {edges {node {name state}}}}}"", ""variables"" : { ""userId"": ""{userValue}"", ""audiences"": {audiences} diff --git a/OptimizelySDK/Odp/OdpSegmentManager.cs b/OptimizelySDK/Odp/OdpSegmentManager.cs index 162c02120..7785e9312 100644 --- a/OptimizelySDK/Odp/OdpSegmentManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentManager.cs @@ -113,12 +113,12 @@ public List FetchQualifiedSegments(string fsUserId, List qualifiedSegments; var cacheKey = GetCacheKey(OdpUserKeyType.FS_USER_ID.ToString().ToLower(), fsUserId); - if (options.Contains(OdpSegmentOption.ResetCache)) + if (options.Contains(OdpSegmentOption.RESET_CACHE)) { _segmentsCache.Reset(); } - if (!options.Contains(OdpSegmentOption.IgnoreCache)) + if (!options.Contains(OdpSegmentOption.IGNORE_CACHE)) { qualifiedSegments = _segmentsCache.Lookup(cacheKey); if (qualifiedSegments != null) @@ -138,7 +138,7 @@ public List FetchQualifiedSegments(string fsUserId, _odpConfig.SegmentsToCheck)?. ToList(); - if (qualifiedSegments != null && !options.Contains(OdpSegmentOption.IgnoreCache)) + if (qualifiedSegments != null && !options.Contains(OdpSegmentOption.IGNORE_CACHE)) { _segmentsCache.Save(cacheKey, qualifiedSegments); } From eae067af0d79407258c80a2a9a8b07bb3ee40371 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 20 Jan 2023 20:57:14 +0500 Subject: [PATCH 2/4] - Removed batchsize support from ODP event manager - Made some general fixes --- .../OdpTests/OdpSegmentManagerTest.cs | 4 +- OptimizelySDK/Config/DatafileProjectConfig.cs | 3 +- .../Config/HttpProjectConfigManager.cs | 2 +- .../Config/PollingProjectConfigManager.cs | 2 +- OptimizelySDK/Odp/OdpEventManager.cs | 14 +---- OptimizelySDK/Odp/OdpManager.cs | 1 - OptimizelySDK/Odp/OdpSegmentApiManager.cs | 2 +- OptimizelySDK/Optimizely.cs | 60 +++++++++++++------ OptimizelySDK/OptimizelyUserContext.cs | 31 ++++++---- 9 files changed, 72 insertions(+), 47 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs index 545179608..d6bd82fe4 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpSegmentManagerTest.cs @@ -185,7 +185,7 @@ public void ShouldIgnoreCache() manager.FetchQualifiedSegments(FS_USER_ID, new List { - OdpSegmentOption.IGNORE_CACHE, + OdpSegmentOption.IgnoreCache, }); _mockCache.Verify(c => c.Reset(), Times.Never); @@ -206,7 +206,7 @@ public void ShouldResetCache() manager.FetchQualifiedSegments(FS_USER_ID, new List { - OdpSegmentOption.RESET_CACHE, + OdpSegmentOption.ResetCache, }); _mockCache.Verify(c => c.Reset(), Times.Once); diff --git a/OptimizelySDK/Config/DatafileProjectConfig.cs b/OptimizelySDK/Config/DatafileProjectConfig.cs index aeaef420e..74a9c62cd 100644 --- a/OptimizelySDK/Config/DatafileProjectConfig.cs +++ b/OptimizelySDK/Config/DatafileProjectConfig.cs @@ -404,7 +404,8 @@ private void Initialize() } } } - var integration = Integrations.FirstOrDefault(i => i.Key == "odp"); + + var integration = Integrations.FirstOrDefault(i => i.Key.ToLower() == "odp"); HostForOdp = integration?.Host; PublicKeyForOdp = integration?.PublicKey; diff --git a/OptimizelySDK/Config/HttpProjectConfigManager.cs b/OptimizelySDK/Config/HttpProjectConfigManager.cs index 54f82a835..65cd71d82 100644 --- a/OptimizelySDK/Config/HttpProjectConfigManager.cs +++ b/OptimizelySDK/Config/HttpProjectConfigManager.cs @@ -56,7 +56,7 @@ string sdkKey /// /// SDK key in use for this project /// - public string SdkKey + public override string SdkKey { get { diff --git a/OptimizelySDK/Config/PollingProjectConfigManager.cs b/OptimizelySDK/Config/PollingProjectConfigManager.cs index 2725330a4..671e8c134 100644 --- a/OptimizelySDK/Config/PollingProjectConfigManager.cs +++ b/OptimizelySDK/Config/PollingProjectConfigManager.cs @@ -150,7 +150,7 @@ public ProjectConfig GetConfig() /// Access to current cached project configuration /// /// ProjectConfig instance - public ProjectConfig CachedProjectConfig + public virtual ProjectConfig CachedProjectConfig { get { diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 1aa870696..9a26195b7 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -418,7 +418,6 @@ public class Builder new BlockingCollection(Constants.DEFAULT_QUEUE_CAPACITY); private IOdpEventApiManager _odpEventApiManager; - private int _batchSize; private TimeSpan _flushInterval; private TimeSpan _timeoutInterval; private ILogger _logger; @@ -448,12 +447,6 @@ public Builder WithOdpEventApiManager(IOdpEventApiManager odpEventApiManager) return this; } - public Builder WithBatchSize(int batchSize) - { - _batchSize = batchSize; - return this; - } - public Builder WithFlushInterval(TimeSpan flushInterval) { _flushInterval = flushInterval; @@ -487,11 +480,8 @@ public OdpEventManager Build() var manager = new OdpEventManager(); manager._eventQueue = _eventQueue; manager._odpEventApiManager = _odpEventApiManager; - manager._batchSize = - _batchSize < 1 ? Constants.DEFAULT_BATCH_SIZE : _batchSize; - manager._flushInterval = _flushInterval <= TimeSpan.FromSeconds(0) ? - Constants.DEFAULT_FLUSH_INTERVAL : - _flushInterval; + manager._flushInterval = (_flushInterval != null && _flushInterval > TimeSpan.FromSeconds(0)) ? _flushInterval : Constants.DEFAULT_FLUSH_INTERVAL; + manager._batchSize = (_flushInterval != null && _flushInterval == TimeSpan.FromSeconds(0)) ? 1 : Constants.DEFAULT_BATCH_SIZE; manager._timeoutInterval = _timeoutInterval <= TimeSpan.FromSeconds(0) ? Constants.DEFAULT_TIMEOUT_INTERVAL : _timeoutInterval; diff --git a/OptimizelySDK/Odp/OdpManager.cs b/OptimizelySDK/Odp/OdpManager.cs index 64d755ae5..8c66d4db4 100644 --- a/OptimizelySDK/Odp/OdpManager.cs +++ b/OptimizelySDK/Odp/OdpManager.cs @@ -218,7 +218,6 @@ public OdpManager Build(bool asEnabled = true) var eventApiManager = new OdpEventApiManager(_logger, _errorHandler); manager.EventManager = new OdpEventManager.Builder(). - WithBatchSize(Constants.DEFAULT_MAX_CACHE_SIZE). WithTimeoutInterval( TimeSpan.FromMilliseconds(Constants.DEFAULT_CACHE_SECONDS)). WithOdpEventApiManager(eventApiManager). diff --git a/OptimizelySDK/Odp/OdpSegmentApiManager.cs b/OptimizelySDK/Odp/OdpSegmentApiManager.cs index 44d25350d..ec7fdae31 100644 --- a/OptimizelySDK/Odp/OdpSegmentApiManager.cs +++ b/OptimizelySDK/Odp/OdpSegmentApiManager.cs @@ -195,7 +195,7 @@ private string QuerySegments(string apiKey, string endpoint, string query) _logger.Log(LogLevel.ERROR, $"{AUDIENCE_FETCH_FAILURE_MESSAGE} ({Constants.NETWORK_ERROR_REASON})"); - return default; + return null; } return response.Content.ReadAsStringAsync().Result; diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 29fa46aa5..c384e440e 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -166,7 +166,7 @@ public Optimizely(string datafile, #if USE_ODP // 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, + OdpManager?.UpdateSettings(config.PublicKeyForOdp, config.HostForOdp, config.Segments.ToList()); #endif } @@ -220,21 +220,27 @@ public Optimizely(ProjectConfigManager configManager, notificationCenter, eventProcessor, defaultDecideOptions, odpManager); var projectConfig = ProjectConfigManager.CachedProjectConfig; - if (projectConfig != null) + + if (ProjectConfigManager.CachedProjectConfig != null) { - NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey, logger)?. - AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, - () => - { - OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, - projectConfig.HostForOdp, - projectConfig.Segments.ToList()); - }); - - // in case if notification is lost. + // in case Project config is instantly available OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp, projectConfig.Segments.ToList()); } + if (ProjectConfigManager.SdkKey != null) + { + NotificationCenterRegistry.GetNotificationCenter(configManager.SdkKey, logger)?. + AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate, + () => + { + projectConfig = ProjectConfigManager.CachedProjectConfig; + + OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, + projectConfig.HostForOdp, + projectConfig.Segments.ToList()); + }); + } + #else InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService, notificationCenter, eventProcessor, defaultDecideOptions); @@ -268,7 +274,7 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null, DefaultDecideOptions = defaultDecideOptions ?? new OptimizelyDecideOption[] { }; #if USE_ODP - OdpManager = odpManager ?? new OdpManager.Builder().Build(); + OdpManager = odpManager; #endif } @@ -450,7 +456,7 @@ private Variation GetVariation(string experimentKey, string userId, ProjectConfi return null; userAttributes = userAttributes ?? new UserAttributes(); - var userContext = CreateUserContext(userId, userAttributes); + var userContext = CreateUserContextCopy(userId, userAttributes); var variation = DecisionService.GetVariation(experiment, userContext, config)?. ResultObject; var decisionInfo = new Dictionary @@ -579,7 +585,7 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, bool featureEnabled = false; var sourceInfo = new Dictionary(); var decision = DecisionService. - GetVariationForFeature(featureFlag, CreateUserContext(userId, userAttributes), + GetVariationForFeature(featureFlag, CreateUserContextCopy(userId, userAttributes), config). ResultObject; var variation = decision?.Variation; @@ -699,7 +705,7 @@ public virtual T GetFeatureVariableValueForType(string featureKey, string var var featureEnabled = false; var variableValue = featureVariable.DefaultValue; var decision = DecisionService. - GetVariationForFeature(featureFlag, CreateUserContext(userId, userAttributes), + GetVariationForFeature(featureFlag, CreateUserContextCopy(userId, userAttributes), config). ResultObject; @@ -885,6 +891,24 @@ public OptimizelyUserContext CreateUserContext(string userId, return new OptimizelyUserContext(this, userId, userAttributes, ErrorHandler, Logger); } + private OptimizelyUserContext CreateUserContextCopy(string userId, + UserAttributes userAttributes = null + ) + { + var inputValues = new Dictionary + { + { + USER_ID, userId + }, + }; + + if (!ValidateStringInputs(inputValues)) + return null; + + + return new OptimizelyUserContext(this, userId, userAttributes, null, null, ErrorHandler, Logger, shouldIdentifyUser: false); + } + /// /// Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context, which contains all data required to deliver the flag. ///
    @@ -1269,7 +1293,7 @@ public OptimizelyJSON GetAllFeatureVariables(string featureKey, string userId, var featureEnabled = false; var decisionResult = DecisionService.GetVariationForFeature(featureFlag, - CreateUserContext(userId, userAttributes), config); + CreateUserContextCopy(userId, userAttributes), config); var variation = decisionResult.ResultObject?.Variation; if (variation != null) @@ -1392,7 +1416,7 @@ internal string[] FetchQualifiedSegments(string userId, List segmentOptions ) { - return OdpManager?.FetchQualifiedSegments(userId, segmentOptions) ?? new string[0]; + return OdpManager?.FetchQualifiedSegments(userId, segmentOptions); } /// diff --git a/OptimizelySDK/OptimizelyUserContext.cs b/OptimizelySDK/OptimizelyUserContext.cs index e88d00bca..9fc247084 100644 --- a/OptimizelySDK/OptimizelyUserContext.cs +++ b/OptimizelySDK/OptimizelyUserContext.cs @@ -126,10 +126,13 @@ public virtual string GetUserId() /// List of qualified segments public List GetQualifiedSegments() { - List qualifiedSegmentsCopy; + List qualifiedSegmentsCopy = null; lock (mutex) { - qualifiedSegmentsCopy = new List(QualifiedSegments); + if (QualifiedSegments != null) + { + qualifiedSegmentsCopy = new List(QualifiedSegments); + } } return qualifiedSegmentsCopy; @@ -143,8 +146,19 @@ public void SetQualifiedSegments(List qualifiedSegments) { lock (mutex) { - QualifiedSegments.Clear(); - QualifiedSegments.AddRange(qualifiedSegments); + if (qualifiedSegments == null) + { + QualifiedSegments = null; + } + else if (QualifiedSegments == null) + { + QualifiedSegments = new List(qualifiedSegments); + } + else + { + QualifiedSegments.Clear(); + QualifiedSegments.AddRange(qualifiedSegments); + } } } @@ -157,7 +171,7 @@ public bool IsQualifiedFor(string segment) { lock (mutex) { - return QualifiedSegments.Contains(segment); + return QualifiedSegments?.Contains(segment) ?? false; } } @@ -173,11 +187,8 @@ public bool FetchQualifiedSegments(List segmentOptions = null) var success = segments != null; - if (success) - { - SetQualifiedSegments(segments.ToList()); - } - + SetQualifiedSegments(segments?.ToList()); + return success; } From 12821a1ecf7587fcdb1e0c6e61a877a9f7b2ae72 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 23 Jan 2023 21:22:34 +0500 Subject: [PATCH 3/4] Flush previous event --- OptimizelySDK/Odp/OdpEventManager.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index f77b5686b..a897ff8b0 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -358,7 +358,7 @@ public void UpdateSettings(OdpConfig odpConfig) { return; } - + Flush(); _odpConfig = odpConfig; if (_autoStart) @@ -480,9 +480,8 @@ public OdpEventManager Build() var manager = new OdpEventManager(); manager._eventQueue = _eventQueue; manager._odpEventApiManager = _odpEventApiManager; - manager._flushInterval = _flushInterval < TimeSpan.Zero ? - Constants.DEFAULT_FLUSH_INTERVAL : - _flushInterval; + manager._flushInterval = (_flushInterval != null && _flushInterval > TimeSpan.Zero) ? _flushInterval : Constants.DEFAULT_FLUSH_INTERVAL; + manager._batchSize = (_flushInterval != null && _flushInterval == TimeSpan.Zero) ? 1 : Constants.DEFAULT_BATCH_SIZE; manager._timeoutInterval = _timeoutInterval <= TimeSpan.Zero ? Constants.DEFAULT_TIMEOUT_INTERVAL : _timeoutInterval; @@ -490,10 +489,6 @@ public OdpEventManager Build() manager._errorHandler = _errorHandler ?? new NoOpErrorHandler(); manager._autoStart = _autoStart ?? true; - manager._batchSize = manager._flushInterval == TimeSpan.Zero ? - 1 : - Constants.DEFAULT_BATCH_SIZE; - manager._validOdpDataTypes = new List() { "Char", From f0ea6394f1a4559e253591beda9c78e678be0f87 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 23 Jan 2023 22:19:45 +0500 Subject: [PATCH 4/4] reverting this change --- OptimizelySDK/Optimizely.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index a9abf81b7..8ce84f9b0 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -274,7 +274,7 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null, DefaultDecideOptions = defaultDecideOptions ?? new OptimizelyDecideOption[] { }; #if USE_ODP - OdpManager = odpManager ?? new OdpManager.Builder().Build(); + OdpManager = odpManager ?? new NoOpOdpManager(); #endif }