8000 Fix: Some suggested changes related to ODP by mnoman09 · Pull Request #325 · optimizely/csharp-sdk · GitHub
[go: up one dir, main page]

Skip to content

Fix: Some suggested changes related to ODP #325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading 8000
Diff view
Diff view
4 changes: 2 additions & 2 deletions OptimizelySDK/Odp/Enums.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public enum OdpUserKeyType
/// </summary>
public enum OdpSegmentOption
{
IgnoreCache = 0,
ResetCache = 1,
IGNORE_CACHE = 0,
RESET_CACHE = 1,
}
}
11 changes: 3 additions & 8 deletions OptimizelySDK/Odp/OdpEventManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ public void UpdateSettings(OdpConfig odpConfig)
{
return;
}

Flush();
_odpConfig = odpConfig;

if (_autoStart)
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion OptimizelySDK/Odp/OdpManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
19 changes: 2 additions & 17 deletions OptimizelySDK/Odp/OdpSegmentApiManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions OptimizelySDK/Odp/OdpSegmentManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
}
Expand Down
63 changes: 40 additions & 23 deletions OptimizelySDK/Optimizely.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
{
6D40 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);
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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)
Expand Down
31 changes: 21 additions & 10 deletions OptimizelySDK/OptimizelyUserContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}
}

Expand All @@ -157,7 +171,7 @@ public bool IsQualifiedFor(string segment)
{
lock (mutex)
{
return QualifiedSegments.Contains(segment);
return QualifiedSegments?.Contains(segment) ?? false;
}
}

Expand All @@ -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;
}

Expand Down
0