Fix: Some suggested changes related to ODP (#325)

* Some suggested changes to fix null pointer exception

* - Removed batchsize support from ODP event manager
- Made some general fixes

* Flush previous event

* reverting this change

Co-authored-by: mnoman09 <[email protected]>

Author: mnoman09

OptimizelySDK/Odp/Enums.cs

@@ -32,7 +32,7 @@ public enum OdpUserKeyType
     /// </summary>
     public enum OdpSegmentOption
     {
-        IgnoreCache = 0,
-        ResetCache = 1,
+        IGNORE_CACHE = 0,
+        RESET_CACHE = 1,
     }
 }

OptimizelySDK/Odp/OdpEventManager.cs

@@ -358,7 +358,7 @@ public void UpdateSettings(OdpConfig odpConfig)
             {
                 return;
             }
-
+            Flush();
             _odpConfig = odpConfig;
 
             if (_autoStart)
@@ -480,20 +480,15 @@ 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;
                 manager._logger = _logger ?? new NoOpLogger();
                 manager._errorHandler = _errorHandler ?? new NoOpErrorHandler();
                 manager._autoStart = _autoStart ?? true;
 
-                manager._batchSize = manager._flushInterval == TimeSpan.Zero ?
-                    1 :
-                    Constants.DEFAULT_BATCH_SIZE;
-
                 manager._validOdpDataTypes = new List<string>()
                 {
                     "Char",

OptimizelySDK/Odp/OdpManager.cs

@@ -93,7 +93,7 @@ public string[] FetchQualifiedSegments(string userId, List<OdpSegmentOption> opt
                 return null;
             }
 
-            return SegmentManager.FetchQualifiedSegments(userId, options).ToArray();
+            return SegmentManager.FetchQualifiedSegments(userId, options)?.ToArray();
         }
 
         /// <summary>

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}
@@ -210,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;

OptimizelySDK/Odp/OdpSegmentManager.cs

@@ -101,12 +101,12 @@ public List<string> FetchQualifiedSegments(string fsUserId,
             List<string> 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)
@@ -126,7 +126,7 @@ public List<string> 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);
             }

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
                 }
@@ -219,29 +219,28 @@ public Optimizely(ProjectConfigManager configManager,
             InitializeComponents(eventDispatcher, logger, errorHandler, userProfileService,
                 notificationCenter, eventProcessor, defaultDecideOptions, odpManager);
 
-            if (ProjectConfigManager.CachedProjectConfig == null)
+            var projectConfig = ProjectConfigManager.CachedProjectConfig;
+            
+            if (ProjectConfigManager.CachedProjectConfig != null)
             {
-                return;
+                // in case Project config is instantly available
+                OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp,
+                    projectConfig.Segments.ToList());
             }
-
-            var projectConfig = ProjectConfigManager.CachedProjectConfig;
-
-            // in case if notification is lost.
-            OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp, projectConfig.HostForOdp,
-                projectConfig.Segments.ToList());
-
             if (ProjectConfigManager.SdkKey != null)
             {
-                NotificationCenterRegistry.
-                    GetNotificationCenter(ProjectConfigManager.SdkKey, logger)?.
-                    AddNotification(NotificationCenter.NotificationType.OptimizelyConfigUpdate,
-                        () =>
-                        {
-                            OdpManager?.UpdateSettings(projectConfig.PublicKeyForOdp,
-                                projectConfig.HostForOdp,
-                                projectConfig.Segments.ToList());
-                        });
+                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);
@@ -457,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<string, object>
@@ -586,7 +585,7 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId,
             bool featureEnabled = false;
             var sourceInfo = new Dictionary<string, string>();
             var decision = DecisionService.
-                GetVariationForFeature(featureFlag, CreateUserContext(userId, userAttributes),
+                GetVariationForFeature(featureFlag, CreateUserContextCopy(userId, userAttributes),
                     config).
                 ResultObject;
             var variation = decision?.Variation;
@@ -706,7 +705,7 @@ public virtual T GetFeatureVariableValueForType<T>(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;
 
@@ -892,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<string, string>
+            {
+                {
+                    USER_ID, userId
+                },
+            };
+
+            if (!ValidateStringInputs(inputValues))
+                return null;
+            
+
+            return new OptimizelyUserContext(this, userId, userAttributes, null, null, ErrorHandler, Logger, shouldIdentifyUser: false);
+        }
+
         /// <summary>
         /// Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context, which contains all data required to deliver the flag.
         /// <ul>
@@ -1276,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)

OptimizelySDK/OptimizelyUserContext.cs

@@ -126,10 +126,13 @@ public virtual string GetUserId()
         /// <returns>List of qualified segments</returns>
         public List<string> GetQualifiedSegments()
         {
-            List<string> qualifiedSegmentsCopy;
+            List<string> qualifiedSegmentsCopy = null;
             lock (mutex)
             {
-                qualifiedSegmentsCopy = new List<string>(QualifiedSegments);
+                if (QualifiedSegments != null)
+                {
+                    qualifiedSegmentsCopy = new List<string>(QualifiedSegments);
+                }
             }
 
             return qualifiedSegmentsCopy;
@@ -143,8 +146,19 @@ public void SetQualifiedSegments(List<string> qualifiedSegments)
         {
             lock (mutex)
             {
-                QualifiedSegments.Clear();
-                QualifiedSegments.AddRange(qualifiedSegments);
+                if (qualifiedSegments == null)
+                {
+                    QualifiedSegments = null;
+                }
+                else if (QualifiedSegments == null)
+                {
+                    QualifiedSegments = new List<string>(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<OdpSegmentOption> segmentOptions = null)
 
             var success = segments != null;
 
-            if (success)
-            {
-                SetQualifiedSegments(segments.ToList());
-            }
-
+            SetQualifiedSegments(segments?.ToList());
+           
             return success;
         }