From b315d5d0365f45026565ca581de1da8708cfcdb1 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 18 Oct 2024 14:58:51 -0400 Subject: [PATCH 01/28] ci: fail if triggered from Draft PR --- .github/workflows/csharp.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 4303e0e2..8a6f6426 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -7,9 +7,20 @@ on: branches: [ master ] jobs: - lintCodebase: + failOnDraftPullRequest: + name: Fail If Draft Pull Request + if: github.event.pull_request.draft == true runs-on: ubuntu-latest + steps: + - name: Fails Draft pull request. + run: | + echo "Draft pull requests should not trigger CI. Please mark the pull request as Ready for review to continue." + exit 1 + + lintCodebase: name: Lint Codebase + needs: [ failOnDraftPullRequest ] + runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v3 From b5658b1d7dd6d61942ceb7571f3a9f510c0f2a69 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 18 Oct 2024 14:59:10 -0400 Subject: [PATCH 02/28] test: cover new cases --- .../OptimizelyUserContextTest.cs | 163 +++++++++++++++++- 1 file changed, 158 insertions(+), 5 deletions(-) diff --git a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs index 49fd91a4..bd41baab 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs +++ b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021, 2022-2023 Optimizely and contributors + * Copyright 2020-2021, 2022-2024 Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ using System; using System.Collections.Generic; -using System.Threading; +using System.Linq; using Castle.Core.Internal; using Moq; using NUnit.Framework; @@ -61,6 +61,22 @@ public void SetUp() Optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object); } + + private Mock MakeUserProfileServiceMock() + { + var projectConfig = DatafileProjectConfig.Create(TestData.Datafile, LoggerMock.Object, + ErrorHandlerMock.Object); + var experiment = projectConfig.Experiments[8]; + var variation = experiment.Variations[0]; + var decision = new Decision(variation.Id); + var userProfile = new UserProfile(UserID, new Dictionary + { + { experiment.Id, decision }, + }); + var userProfileServiceMock = new Mock(); + userProfileServiceMock.Setup(up => up.Lookup(UserID)).Returns(userProfile.ToMap()); + return userProfileServiceMock; + } [Test] public void OptimizelyUserContextWithAttributes() @@ -193,7 +209,7 @@ public void SetAttributeToOverrideAttribute() Assert.AreEqual(user.GetAttributes()["k1"], true); } - #region decide + #region Decide [Test] public void TestDecide() @@ -409,10 +425,111 @@ public void DecideWhenConfigIsNull() Assert.IsTrue(TestData.CompareObjects(decision, decisionExpected)); } - #endregion decide + [Test] + public void SeparateDecideShouldHaveSameNumberOfUpsSaveOnlyOneLookup() + { + var experimentFlagKey = "double_single_variable_feature"; + var rolloutFlagKey = "boolean_single_variable_feature"; + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var user = optimizely.CreateUserContext(UserID); + var expectedUserProfileExperiment = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + }); + var expectedUserProfileRollout = new UserProfile(UserID, new Dictionary + { + { "999", new Decision("99") }, + { "99999", new Decision("999") }, + }); + + user.Decide(experimentFlagKey); + user.Decide(rolloutFlagKey); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Exactly(2)); + Assert.AreEqual(saveArgsCollector[0], expectedUserProfileExperiment.ToMap()); + Assert.AreEqual(saveArgsCollector[1], expectedUserProfileRollout.ToMap()); + } + + [Test] + public void DecideWithUpsShouldOnlyLookupSaveOnce() + { + var flagKeyFromTestDataJson = "double_single_variable_feature"; + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var user = optimizely.CreateUserContext(UserID); + var expectedUserProfile = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + }); + + user.Decide(flagKeyFromTestDataJson); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + } + + #endregion Decide + + #region DecideForKeys - #region decideAll + [Test] + public void DecideForKeysWithUpsShouldOnlyLookupSaveOnceWithMultipleFlags() + { + var flagKeys = new[] { "double_single_variable_feature", "boolean_feature" }; + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var userContext = optimizely.CreateUserContext(UserID); + var expectedUserProfile = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + }); + userContext.DecideForKeys(flagKeys); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + } + [Test] public void DecideForKeysWithOneFlag() { @@ -442,6 +559,42 @@ public void DecideForKeysWithOneFlag() new string[0]); Assert.IsTrue(TestData.CompareObjects(decision, expDecision)); } + + #endregion DecideForKeys + + #region DecideAll + + [Test] + public void DecideAllWithUpsShouldOnlyLookupSaveOnce() + { + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var user = optimizely.CreateUserContext(UserID); + var expectedUserProfile = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + { "122241", new Decision("122242") }, + { "122235", new Decision("122236") }, + { "188880", new Decision("188881") }, + }); + + user.DecideAll(); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + } [Test] public void DecideAllTwoFlag() From 74d98f705fef3c282d7782ba3b92eddea7a624b3 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Fri, 18 Oct 2024 17:51:53 -0400 Subject: [PATCH 03/28] feat: WIP --- OptimizelySDK/Bucketing/DecisionService.cs | 188 +++++++++++++++++---- OptimizelySDK/Optimizely.cs | 58 ++++++- 2 files changed, 202 insertions(+), 44 deletions(-) diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index e6088d7e..c820fefc 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -1,5 +1,5 @@ /* -* Copyright 2017-2022, Optimizely +* Copyright 2017-2022, 2024 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; +using System.Linq; using OptimizelySDK.Entity; using OptimizelySDK.ErrorHandler; using OptimizelySDK.Logger; @@ -706,9 +707,9 @@ OptimizelyDecideOption[] options /// /// Get the variation the user is bucketed into for the FeatureFlag /// - /// The feature flag the user wants to access. - /// User Identifier - /// The user's attributes. This should be filtered to just attributes in the Datafile. + /// The feature flag the user wants to access. + /// The user context. + /// The project config. /// null if the user is not bucketed into any variation or the FeatureDecision entity if the user is /// successfully bucketed. public virtual Result GetVariationForFeature(FeatureFlag featureFlag, @@ -719,53 +720,168 @@ public virtual Result GetVariationForFeature(FeatureFlag featur new OptimizelyDecideOption[] { }); } - /// - /// Get the variation the user is bucketed into for the FeatureFlag - /// - /// The feature flag the user wants to access. - /// User Identifier - /// The user's attributes. This should be filtered to just attributes in the Datafile. - /// The user's attributes. This should be filtered to just attributes in the Datafile. - /// An array of decision options. - /// null if the user is not bucketed into any variation or the FeatureDecision entity if the user is - /// successfully bucketed. - public virtual Result GetVariationForFeature(FeatureFlag featureFlag, + private class UserProfileTracker + { + public UserProfile UserProfile { get; set; } + public bool ProfileUpdated { get; set; } + + public UserProfileTracker(UserProfile userProfile, bool profileUpdated) + { + UserProfile = userProfile; + ProfileUpdated = profileUpdated; + } + } + + void SaveUserProfile(UserProfile userProfile) + { + if (UserProfileService == null) + { + return; + } + + try + { + UserProfileService.Save(userProfile.ToMap()); + Logger.Log(LogLevel.INFO, + $"Saved user profile of user \"{userProfile.UserId}\"."); + } + catch (Exception exception) + { + Logger.Log(LogLevel.WARN, + $"Failed to save user profile of user \"{userProfile.UserId}\"."); + ErrorHandler.HandleError(new Exceptions.OptimizelyRuntimeException(exception.Message)); + } + } + + private UserProfile GetUserProfile(String userId, DecisionReasons reasons) + { + UserProfile userProfile = null; + + try + { + var userProfileMap = UserProfileService.Lookup(userId); + if (userProfileMap == null) + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + "We were unable to get a user profile map from the UserProfileService.")); + } + else if (UserProfileUtil.IsValidUserProfileMap(userProfileMap)) + { + userProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); + } + else + { + Logger.Log(LogLevel.WARN, + reasons.AddInfo("The UserProfileService returned an invalid map.")); + } + } + catch (Exception exception) + { + Logger.Log(LogLevel.ERROR, reasons.AddInfo(exception.Message)); + ErrorHandler.HandleError( + new Exceptions.OptimizelyRuntimeException(exception.Message)); + } + + if (userProfile == null) + { + userProfile = new UserProfile(userId, new Dictionary()); + } + + return userProfile; + } + + public virtual List> GetVariationsForFeatureList( + List featureFlags, OptimizelyUserContext user, - ProjectConfig config, + ProjectConfig projectConfig, UserAttributes filteredAttributes, OptimizelyDecideOption[] options ) { - var reasons = new DecisionReasons(); - var userId = user.GetUserId(); - // Check if the feature flag has an experiment and the user is bucketed into that experiment. - var decisionResult = GetVariationForFeatureExperiment(featureFlag, user, - filteredAttributes, config, options); - reasons += decisionResult.DecisionReasons; + var upsReasons = new DecisionReasons(); - if (decisionResult.ResultObject != null) + var ignoreUPS = options.Contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); + UserProfileTracker userProfileTracker = null; + + if (UserProfileService != null && !ignoreUPS) { - return Result.NewResult(decisionResult.ResultObject, reasons); + var userProfile = GetUserProfile(user.GetUserId(), upsReasons); + userProfileTracker = new UserProfileTracker(userProfile, false); } - // Check if the feature flag has rollout and the the user is bucketed into one of its rules. - decisionResult = GetVariationForFeatureRollout(featureFlag, user, config); - reasons += decisionResult.DecisionReasons; + var userId = user.GetUserId(); + var decisions = new List>(); - if (decisionResult.ResultObject != null) + foreach (var featureFlag in featureFlags) { + var reasons = new DecisionReasons(); + reasons += upsReasons; + + // Check if the feature flag has an experiment and the user is bucketed into that experiment. + var decisionResult = GetVariationForFeatureExperiment(featureFlag, user, + filteredAttributes, projectConfig, options); + reasons += decisionResult.DecisionReasons; + + if (decisionResult.ResultObject != null) + { + decisions.Add( + Result.NewResult(decisionResult.ResultObject, reasons)); + continue; + } + + // Check if the feature flag has rollout and the the user is bucketed into one of its rules. + decisionResult = GetVariationForFeatureRollout(featureFlag, user, projectConfig); + reasons += decisionResult.DecisionReasons; + + if (decisionResult.ResultObject != null) + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"The user \"{userId}\" is bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); + decisions.Add( + Result.NewResult(decisionResult.ResultObject, reasons)); + continue; + } + Logger.Log(LogLevel.INFO, reasons.AddInfo( - $"The user \"{userId}\" is bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); - return Result.NewResult(decisionResult.ResultObject, reasons); + $"The user \"{userId}\" is not bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); + decisions.Add(Result.NewResult( + new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT), + reasons)); } - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - $"The user \"{userId}\" is not bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); - return Result.NewResult( - new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT), reasons); - ; + if (UserProfileService != null && !ignoreUPS && userProfileTracker?.ProfileUpdated == true) + { + SaveUserProfile(userProfileTracker.UserProfile); + } + + return decisions; + } + + /// + /// Get the variation the user is bucketed into for the FeatureFlag + /// + /// The feature flag the user wants to access. + /// The user context. + /// The project config. + /// The user's attributes. This should be filtered to just attributes in the Datafile. + /// An array of decision options. + /// null if the user is not bucketed into any variation or the FeatureDecision entity if the user is + /// successfully bucketed. + public virtual Result GetVariationForFeature(FeatureFlag featureFlag, + OptimizelyUserContext user, + ProjectConfig config, + UserAttributes filteredAttributes, + OptimizelyDecideOption[] options + ) + { + return GetVariationsForFeatureList(new List { featureFlag }, + user, + config, + filteredAttributes, + options).First(); } /// diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 9da300f8..4bc3b568 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -869,6 +869,8 @@ internal OptimizelyDecision Decide(OptimizelyUserContext user, OptimizelyDecideOption[] options ) { + return DecideForKeys(user, new[] { key }, options)[key]; + var config = ProjectConfigManager?.GetConfig(); if (config == null) @@ -1024,7 +1026,7 @@ OptimizelyDecideOption[] options if (projectConfig == null) { Logger.Log(LogLevel.ERROR, - "Optimizely instance is not valid, failing isFeatureEnabled call."); + "Optimizely instance is not valid, failing DecideAll call."); return decisionMap; } @@ -1041,23 +1043,61 @@ OptimizelyDecideOption[] options { var decisionDictionary = new Dictionary(); - var projectConfig = ProjectConfigManager?.GetConfig(); - if (projectConfig == null) + if (keys.Length == 0) { - Logger.Log(LogLevel.ERROR, - "Optimizely instance is not valid, failing isFeatureEnabled call."); return decisionDictionary; } - if (keys.Length == 0) + var projectConfig = ProjectConfigManager?.GetConfig(); + if (projectConfig == null) { + Logger.Log(LogLevel.ERROR, + "Optimizely instance is not valid, failing DecideForKeys call."); return decisionDictionary; } var allOptions = GetAllOptions(options); + var flagDecisions = new Dictionary(); + var decisionReasons = new Dictionary(); + + var flagsWithoutForcedDecisions = new List(); + foreach (var key in keys) { + var flag = projectConfig.GetFeatureFlagFromKey(key); + if (flag.Key == null) + { + decisionDictionary.Add(key, + OptimizelyDecision.NewErrorDecision(key, user, + DecisionMessage.Reason(DecisionMessage.FLAG_KEY_INVALID, key), + ErrorHandler, Logger)); + continue; + } + + var decisionContext = new OptimizelyDecisionContext(flag.Key); + var forcedDecisionVariation = + DecisionService.ValidatedForcedDecision(decisionContext, projectConfig, user); + decisionReasons.Add(key, forcedDecisionVariation.DecisionReasons); + + if (forcedDecisionVariation.ResultObject != null) + { + var experiment = projectConfig.GetExperimentFromKey(flag.Key); + var featureDecision = Result.NewResult( + new FeatureDecision(experiment, forcedDecisionVariation.ResultObject, + FeatureDecision.DECISION_SOURCE_FEATURE_TEST), + forcedDecisionVariation.DecisionReasons); + flagDecisions.Add(key, featureDecision.ResultObject); + } + else + { + flagsWithoutForcedDecisions.Add(flag); + } + + var decisionsList = DecisionService.GetVariationsForFeatureList( + flagsWithoutForcedDecisions, user, projectConfig, user.GetAttributes(), + options); + var decision = Decide(user, key, options); if (!allOptions.Contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.Enabled) @@ -1347,7 +1387,8 @@ List segmentOptions if (config == null) { - Logger.Log(LogLevel.ERROR, "Datafile has invalid format. Failing 'FetchQualifiedSegments'."); + Logger.Log(LogLevel.ERROR, + "Datafile has invalid format. Failing 'FetchQualifiedSegments'."); return null; } @@ -1378,7 +1419,8 @@ internal void IdentifyUser(string userId) /// Dictionary for identifiers. The caller must provide at least one key-value pair. /// Type of event (defaults to `fullstack`) /// Optional event data in a key-value pair format - public void SendOdpEvent(string action, Dictionary identifiers, string type = Constants.ODP_EVENT_TYPE, + public void SendOdpEvent(string action, Dictionary identifiers, + string type = Constants.ODP_EVENT_TYPE, Dictionary data = null ) { From a8662667d5ab1e69ec25c34994d0d4976a4da5a5 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 21 Oct 2024 16:34:07 -0400 Subject: [PATCH 04/28] feat: WIP implementing solution. Tests are still in progress too. --- OptimizelySDK.sln.DotSettings | 5 + OptimizelySDK/Bucketing/DecisionService.cs | 204 +++++++----- OptimizelySDK/Optimizely.cs | 346 +++++++++++---------- 3 files changed, 310 insertions(+), 245 deletions(-) diff --git a/OptimizelySDK.sln.DotSettings b/OptimizelySDK.sln.DotSettings index 3ccf7ffc..8ee6e5a4 100644 --- a/OptimizelySDK.sln.DotSettings +++ b/OptimizelySDK.sln.DotSettings @@ -43,10 +43,15 @@ <Policy Inspect="True" Prefix="I" Suffix="" Style="AaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /> <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> + <Policy><Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"><ElementKinds><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Enum members"><ElementKinds><Kind Name="ENUM_MEMBER" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AaBb"><ExtraRule Prefix="" Suffix="" Style="AaBb" /></Policy></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Local constants"><ElementKinds><Kind Name="LOCAL_CONSTANT" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Interfaces"><ElementKinds><Kind Name="INTERFACE" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="I" Suffix="" Style="AaBb" /></Policy> True True True True + True True True True diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index c820fefc..4dba482c 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -43,7 +43,7 @@ public class DecisionService private Bucketer Bucketer; private IErrorHandler ErrorHandler; private UserProfileService UserProfileService; - private ILogger Logger; + private static ILogger Logger; /// /// Associative array of user IDs to an associative array @@ -85,9 +85,9 @@ public DecisionService(Bucketer bucketer, IErrorHandler errorHandler, /// /// Get a Variation of an Experiment for a user to be allocated into. /// - /// The Experiment the user will be bucketed into. - /// Optimizely user context. - /// Project config. + /// The Experiment the user will be bucketed into. + /// Optimizely user context. + /// Project config. /// The Variation the user is allocated into. public virtual Result GetVariation(Experiment experiment, OptimizelyUserContext user, @@ -100,11 +100,11 @@ ProjectConfig config /// /// Get a Variation of an Experiment for a user to be allocated into. /// - /// The Experiment the user will be bucketed into. - /// optimizely user context. - /// Project Config. - /// An array of decision options. - /// The Variation the user is allocated into. + /// The Experiment the user will be bucketed into. + /// Optimizely user context. + /// Project Config. + /// An array of decision options. + /// public virtual Result GetVariation(Experiment experiment, OptimizelyUserContext user, ProjectConfig config, @@ -112,12 +112,60 @@ OptimizelyDecideOption[] options ) { var reasons = new DecisionReasons(); - var userId = user.GetUserId(); + + var ignoreUps = options.Contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); + UserProfileTracker userProfileTracker = null; + + if (UserProfileService != null && !ignoreUps) + { + var userProfile = GetUserProfile(user.GetUserId(), reasons); + userProfileTracker = new UserProfileTracker(userProfile, false); + } + + var response = GetVariation(experiment, user, config, options, userProfileTracker, + reasons); + + if (UserProfileService != null && !ignoreUps && + userProfileTracker?.ProfileUpdated == true) + { + SaveUserProfile(userProfileTracker.UserProfile); + } + + return response; + } + + /// + /// Get a Variation of an Experiment for a user to be allocated into. + /// + /// The Experiment the user will be bucketed into. + /// Optimizely user context. + /// Project Config. + /// An array of decision options. + /// A UserProfileTracker object. + /// Set of reasons for the decision. + /// The Variation the user is allocated into. + public virtual Result GetVariation(Experiment experiment, + OptimizelyUserContext user, + ProjectConfig config, + OptimizelyDecideOption[] options, + UserProfileTracker userProfileTracker, + DecisionReasons reasons = null + ) + { + if (reasons == null) + { + reasons = new DecisionReasons(); + } + if (!ExperimentUtils.IsExperimentActive(experiment, Logger)) { + var message = reasons.AddInfo($"Experiment {experiment.Key} is not running."); + Logger.Log(LogLevel.INFO, message); return Result.NullResult(reasons); } + var userId = user.GetUserId(); + // check if a forced variation is set var decisionVariationResult = GetForcedVariation(experiment.Key, userId, config); reasons += decisionVariationResult.DecisionReasons; @@ -137,76 +185,41 @@ OptimizelyDecideOption[] options return decisionVariationResult; } - // fetch the user profile map from the user profile service - var ignoreUPS = Array.Exists(options, - option => option == OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); - - UserProfile userProfile = null; - if (!ignoreUPS && UserProfileService != null) + if (userProfileTracker != null) { - try - { - var userProfileMap = UserProfileService.Lookup(user.GetUserId()); - if (userProfileMap != null && - UserProfileUtil.IsValidUserProfileMap(userProfileMap)) - { - userProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); - decisionVariationResult = - GetStoredVariation(experiment, userProfile, config); - reasons += decisionVariationResult.DecisionReasons; - if (decisionVariationResult.ResultObject != null) - { - return decisionVariationResult.SetReasons(reasons); - } - } - else if (userProfileMap == null) - { - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - "We were unable to get a user profile map from the UserProfileService.")); - } - else - { - Logger.Log(LogLevel.ERROR, - reasons.AddInfo("The UserProfileService returned an invalid map.")); - } - } - catch (Exception exception) + decisionVariationResult = + GetStoredVariation(experiment, userProfileTracker.UserProfile, config); + reasons += decisionVariationResult.DecisionReasons; + variation = decisionVariationResult.ResultObject; + if (variation != null) { - Logger.Log(LogLevel.ERROR, reasons.AddInfo(exception.Message)); - ErrorHandler.HandleError( - new Exceptions.OptimizelyRuntimeException(exception.Message)); + return decisionVariationResult; } } - var filteredAttributes = user.GetAttributes(); - var doesUserMeetAudienceConditionsResult = - ExperimentUtils.DoesUserMeetAudienceConditions(config, experiment, user, - LOGGING_KEY_TYPE_EXPERIMENT, experiment.Key, Logger); - reasons += doesUserMeetAudienceConditionsResult.DecisionReasons; - if (doesUserMeetAudienceConditionsResult.ResultObject) + var decisionMeetAudience = ExperimentUtils.DoesUserMeetAudienceConditions(config, + experiment, user, + LOGGING_KEY_TYPE_EXPERIMENT, experiment.Key, Logger); + reasons += decisionMeetAudience.DecisionReasons; + if (decisionMeetAudience.ResultObject) { - // Get Bucketing ID from user attributes. - var bucketingIdResult = GetBucketingId(userId, filteredAttributes); + var bucketingIdResult = GetBucketingId(userId, user.GetAttributes()); reasons += bucketingIdResult.DecisionReasons; decisionVariationResult = Bucketer.Bucket(config, experiment, bucketingIdResult.ResultObject, userId); reasons += decisionVariationResult.DecisionReasons; + variation = decisionVariationResult.ResultObject; - if (decisionVariationResult.ResultObject?.Key != null) + if (variation != null) { - if (UserProfileService != null && !ignoreUPS) + if (userProfileTracker != null) { - var bucketerUserProfile = userProfile ?? - new UserProfile(userId, - new Dictionary()); - SaveVariation(experiment, decisionVariationResult.ResultObject, - bucketerUserProfile); + userProfileTracker.UpdateUserProfile(experiment, variation); } else { - Logger.Log(LogLevel.INFO, + Logger.Log(LogLevel.DEBUG, "This decision will not be saved since the UserProfileService is null."); } } @@ -720,18 +733,6 @@ public virtual Result GetVariationForFeature(FeatureFlag featur new OptimizelyDecideOption[] { }); } - private class UserProfileTracker - { - public UserProfile UserProfile { get; set; } - public bool ProfileUpdated { get; set; } - - public UserProfileTracker(UserProfile userProfile, bool profileUpdated) - { - UserProfile = userProfile; - ProfileUpdated = profileUpdated; - } - } - void SaveUserProfile(UserProfile userProfile) { if (UserProfileService == null) @@ -791,6 +792,40 @@ private UserProfile GetUserProfile(String userId, DecisionReasons reasons) return userProfile; } + public class UserProfileTracker + { + public UserProfile UserProfile { get; set; } + public bool ProfileUpdated { get; set; } + + public UserProfileTracker(UserProfile userProfile, bool profileUpdated) + { + UserProfile = userProfile; + ProfileUpdated = profileUpdated; + } + + public void UpdateUserProfile(Experiment experiment, Variation variation) + { + var experimentId = experiment.Id; + var variationId = variation.Id; + Decision decision; + if (UserProfile.ExperimentBucketMap.ContainsKey(experimentId)) + { + decision = UserProfile.ExperimentBucketMap[experimentId]; + decision.VariationId = variationId; + } + else + { + decision = new Decision(variationId); + } + + UserProfile.ExperimentBucketMap[experimentId] = decision; + ProfileUpdated = true; + + Logger.Log(LogLevel.INFO, + $"Updated variation \"{variationId}\" of experiment \"{experimentId}\" for user \"{UserProfile.UserId}\"."); + } + } + public virtual List> GetVariationsForFeatureList( List featureFlags, OptimizelyUserContext user, @@ -834,22 +869,23 @@ OptimizelyDecideOption[] options decisionResult = GetVariationForFeatureRollout(featureFlag, user, projectConfig); reasons += decisionResult.DecisionReasons; - if (decisionResult.ResultObject != null) + if (decisionResult.ResultObject == null) + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"The user \"{userId}\" is not bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); + decisions.Add(Result.NewResult( + new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT), + reasons)); + } + else { Logger.Log(LogLevel.INFO, reasons.AddInfo( $"The user \"{userId}\" is bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); decisions.Add( Result.NewResult(decisionResult.ResultObject, reasons)); - continue; } - - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - $"The user \"{userId}\" is not bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); - decisions.Add(Result.NewResult( - new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT), - reasons)); } if (UserProfileService != null && !ignoreUPS && userProfileTracker?.ProfileUpdated == true) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 4bc3b568..a84466e4 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -855,12 +855,30 @@ private OptimizelyUserContext CreateUserContextCopy(string userId, ); } + public FeatureDecision GetForcedDecision(string flagKey, DecisionReasons decisionReasons, + ProjectConfig projectConfig, OptimizelyUserContext user + ) + { + var context = new OptimizelyDecisionContext(flagKey); + var forcedDecisionVariation = + DecisionService.ValidatedForcedDecision(context, projectConfig, user); + decisionReasons += forcedDecisionVariation.DecisionReasons; + if (forcedDecisionVariation.ResultObject != null) + { + return new FeatureDecision(null, forcedDecisionVariation.ResultObject, + FeatureDecision.DECISION_SOURCE_FEATURE_TEST); + } + + return null; + } + /// /// Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context, which contains all data required to deliver the flag. ///
    ///
  • If the SDK finds an error, it’ll return a decision with null for variationKey. The decision will include an error message in reasons. ///
/// + /// User context to be used to make decision. /// A flag key for which a decision will be made. /// A list of options for decision-making. /// A decision result. @@ -869,8 +887,6 @@ internal OptimizelyDecision Decide(OptimizelyUserContext user, OptimizelyDecideOption[] options ) { - return DecideForKeys(user, new[] { key }, options)[key]; - var config = ProjectConfigManager?.GetConfig(); if (config == null) @@ -879,141 +895,11 @@ OptimizelyDecideOption[] options ErrorHandler, Logger); } - if (key == null) - { - return OptimizelyDecision.NewErrorDecision(key, - user, - DecisionMessage.Reason(DecisionMessage.FLAG_KEY_INVALID, key), - ErrorHandler, Logger); - } - - var flag = config.GetFeatureFlagFromKey(key); - if (flag.Key == null) - { - return OptimizelyDecision.NewErrorDecision(key, - user, - DecisionMessage.Reason(DecisionMessage.FLAG_KEY_INVALID, key), - ErrorHandler, Logger); - } - - var userId = user?.GetUserId(); - var userAttributes = user?.GetAttributes(); - var decisionEventDispatched = false; - var allOptions = GetAllOptions(options); - var decisionReasons = new DecisionReasons(); - FeatureDecision decision = null; - - var decisionContext = new OptimizelyDecisionContext(flag.Key); - var forcedDecisionVariation = - DecisionService.ValidatedForcedDecision(decisionContext, config, user); - decisionReasons += forcedDecisionVariation.DecisionReasons; - - if (forcedDecisionVariation.ResultObject != null) - { - decision = new FeatureDecision(null, forcedDecisionVariation.ResultObject, - FeatureDecision.DECISION_SOURCE_FEATURE_TEST); - } - else - { - var flagDecisionResult = DecisionService.GetVariationForFeature( - flag, - user, - config, - userAttributes, - allOptions - ); - decisionReasons += flagDecisionResult.DecisionReasons; - decision = flagDecisionResult.ResultObject; - } - - var featureEnabled = false; - - if (decision?.Variation != null) - { - featureEnabled = decision.Variation.FeatureEnabled.GetValueOrDefault(); - } + var filteredOptions = GetAllOptions(options). + Where(opt => opt != OptimizelyDecideOption.ENABLED_FLAGS_ONLY). + ToArray(); - if (featureEnabled) - { - Logger.Log(LogLevel.INFO, - "Feature \"" + key + "\" is enabled for user \"" + userId + "\""); - } - else - { - Logger.Log(LogLevel.INFO, - "Feature \"" + key + "\" is not enabled for user \"" + userId + "\""); - } - - var variableMap = new Dictionary(); - if (flag?.Variables != null && - !allOptions.Contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) - { - foreach (var featureVariable in flag?.Variables) - { - var variableValue = featureVariable.DefaultValue; - if (featureEnabled) - { - var featureVariableUsageInstance = - decision?.Variation.GetFeatureVariableUsageFromId(featureVariable.Id); - if (featureVariableUsageInstance != null) - { - variableValue = featureVariableUsageInstance.Value; - } - } - - var typeCastedValue = - GetTypeCastedVariableValue(variableValue, featureVariable.Type); - - if (typeCastedValue is OptimizelyJSON) - { - typeCastedValue = ((OptimizelyJSON)typeCastedValue).ToDictionary(); - } - - variableMap.Add(featureVariable.Key, typeCastedValue); - } - } - - var optimizelyJSON = new OptimizelyJSON(variableMap, ErrorHandler, Logger); - - var decisionSource = decision?.Source ?? FeatureDecision.DECISION_SOURCE_ROLLOUT; - if (!allOptions.Contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) - { - decisionEventDispatched = SendImpressionEvent(decision?.Experiment, - decision?.Variation, userId, userAttributes, config, key, decisionSource, - featureEnabled); - } - - var reasonsToReport = decisionReasons - .ToReport(allOptions.Contains(OptimizelyDecideOption.INCLUDE_REASONS)) - .ToArray(); - var variationKey = decision?.Variation?.Key; - - // TODO: add ruleKey values when available later. use a copy of experimentKey until then. - var ruleKey = decision?.Experiment?.Key; - - var decisionInfo = new Dictionary - { - { "flagKey", key }, - { "enabled", featureEnabled }, - { "variables", variableMap }, - { "variationKey", variationKey }, - { "ruleKey", ruleKey }, - { "reasons", reasonsToReport }, - { "decisionEventDispatched", decisionEventDispatched }, - }; - - NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, - DecisionNotificationTypes.FLAG, userId, - userAttributes ?? new UserAttributes(), decisionInfo); - - return new OptimizelyDecision( - variationKey, - featureEnabled, - optimizelyJSON, - ruleKey, - key, - user, - reasonsToReport); + return DecideForKeys(user, new[] { key }, filteredOptions, true)[key]; } internal Dictionary DecideAll(OptimizelyUserContext user, @@ -1038,16 +924,12 @@ OptimizelyDecideOption[] options internal Dictionary DecideForKeys(OptimizelyUserContext user, string[] keys, - OptimizelyDecideOption[] options + OptimizelyDecideOption[] options, + bool ignoreDefaultOptions = false ) { var decisionDictionary = new Dictionary(); - if (keys.Length == 0) - { - return decisionDictionary; - } - var projectConfig = ProjectConfigManager?.GetConfig(); if (projectConfig == null) { @@ -1056,13 +938,20 @@ OptimizelyDecideOption[] options return decisionDictionary; } - var allOptions = GetAllOptions(options); + if (keys.Length == 0) + { + return decisionDictionary; + } + + var allOptions = ignoreDefaultOptions ? options : GetAllOptions(options); var flagDecisions = new Dictionary(); - var decisionReasons = new Dictionary(); + var decisionReasonsMap = new Dictionary(); var flagsWithoutForcedDecisions = new List(); + var validKeys = new List(); + foreach (var key in keys) { var flag = projectConfig.GetFeatureFlagFromKey(key); @@ -1075,40 +964,175 @@ OptimizelyDecideOption[] options continue; } - var decisionContext = new OptimizelyDecisionContext(flag.Key); - var forcedDecisionVariation = - DecisionService.ValidatedForcedDecision(decisionContext, projectConfig, user); - decisionReasons.Add(key, forcedDecisionVariation.DecisionReasons); + validKeys.Add(key); + + var decisionReasons = new DecisionReasons(); + var forcedDecision = GetForcedDecision(key, decisionReasons, projectConfig, user); + decisionReasonsMap.Add(key, decisionReasons); - if (forcedDecisionVariation.ResultObject != null) + if (forcedDecision != null) { - var experiment = projectConfig.GetExperimentFromKey(flag.Key); - var featureDecision = Result.NewResult( - new FeatureDecision(experiment, forcedDecisionVariation.ResultObject, - FeatureDecision.DECISION_SOURCE_FEATURE_TEST), - forcedDecisionVariation.DecisionReasons); - flagDecisions.Add(key, featureDecision.ResultObject); + flagDecisions.Add(key, forcedDecision); } else { flagsWithoutForcedDecisions.Add(flag); } + } - var decisionsList = DecisionService.GetVariationsForFeatureList( - flagsWithoutForcedDecisions, user, projectConfig, user.GetAttributes(), - options); + var decisionsList = DecisionService.GetVariationsForFeatureList( + flagsWithoutForcedDecisions, user, projectConfig, user.GetAttributes(), + allOptions); - var decision = Decide(user, key, options); + for (var i = 0; i < decisionsList.Count; i += 1) + { + var decision = decisionsList[i]; + var flagKey = flagsWithoutForcedDecisions[i].Key; + flagDecisions.Add(flagKey, decision.ResultObject); + decisionReasonsMap[flagKey] += decision.DecisionReasons; + } + + foreach (var key in validKeys) + { + var flagDecision = flagDecisions[key]; + var decisionReasons = decisionReasonsMap[key]; + + var optimizelyDecision = CreateOptimizelyDecision(user, key, flagDecision, + decisionReasons, allOptions.ToList(), projectConfig); if (!allOptions.Contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || - decision.Enabled) + optimizelyDecision.Enabled) { - decisionDictionary.Add(key, decision); + decisionDictionary.Add(key, optimizelyDecision); } } return decisionDictionary; } + private OptimizelyDecision CreateOptimizelyDecision( + OptimizelyUserContext user, + string flagKey, + FeatureDecision flagDecision, + DecisionReasons decisionReasons, + List allOptions, + ProjectConfig projectConfig + ) + { + var userId = user.GetUserId(); + + var flagEnabled = false; + if (flagDecision.Variation != null) + { + if (flagDecision.Variation.IsFeatureEnabled) + { + flagEnabled = true; + } + } + + Logger.Log(LogLevel.INFO, + $"Feature \"{flagKey}\" is enabled for user \"{userId}\"? {flagEnabled}"); + + var variableMap = new Dictionary(); + if (!allOptions.Contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) + { + var decisionVariables = GetDecisionVariableMap( + projectConfig.GetFeatureFlagFromKey(flagKey), + flagDecision.Variation, + flagEnabled); + variableMap = decisionVariables.ResultObject; + decisionReasons += decisionVariables.DecisionReasons; + } + + var optimizelyJson = new OptimizelyJSON(variableMap, ErrorHandler, Logger); + + var decisionSource = FeatureDecision.DECISION_SOURCE_ROLLOUT; + if (flagDecision.Source != null) + { + decisionSource = flagDecision.Source; + } + + var reasonsToReport = decisionReasons.ToReport().ToArray(); + var variationKey = flagDecision.Variation?.Key; + // TODO: add ruleKey values when available later. use a copy of experimentKey until then. + // add to event metadata as well (currently set to experimentKey) + var ruleKey = flagDecision.Experiment?.Key; + + var decisionEventDispatched = false; + if (!allOptions.Contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) + { + decisionEventDispatched = SendImpressionEvent( + flagDecision.Experiment, + flagDecision.Variation, + userId, + user.GetAttributes(), + projectConfig, + flagKey, + decisionSource, + flagEnabled); + } + + var decisionInfo = new Dictionary + { + { "featureKey", flagKey }, + { "featureEnabled", flagEnabled }, + { "variableValues", variableMap }, + { "variationKey", variationKey }, + { "ruleKey", ruleKey }, + { "reasons", reasonsToReport }, + { "decisionEventDispatched", decisionEventDispatched }, + }; + + // var decisionNotificationType = + // projectConfig.IsFeatureExperiment(flagDecision.Experiment?.Id) ? + // DecisionNotificationTypes.FEATURE_TEST : + // DecisionNotificationTypes.AB_TEST; + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, + userId, + user.GetAttributes(), decisionInfo); + + return new OptimizelyDecision( + variationKey, + flagEnabled, + optimizelyJson, + ruleKey, + flagKey, + user, + reasonsToReport); + } + + private Result> GetDecisionVariableMap(FeatureFlag flag, Variation variation, bool featureEnabled) + { + var reasons = new DecisionReasons(); + var valuesMap = new Dictionary(); + + foreach (var variable in flag.Variables) + { + var value = variable.DefaultValue; + if (featureEnabled) + { + var instance = variation.GetFeatureVariableUsageFromId(variable.Id); + if (instance != null) + { + value = instance.Value; + } + } + + var convertedValue = GetTypeCastedVariableValue(value, variable.Type); + if (convertedValue == null) + { + reasons.AddError(DecisionMessage.Reason(DecisionMessage.VARIABLE_VALUE_INVALID, variable.Key)); + } + else if (convertedValue is OptimizelyJSON optimizelyJson) + { + convertedValue = optimizelyJson.ToDictionary(); + } + + valuesMap[variable.Key] = convertedValue; + } + + return Result>.NewResult(valuesMap, reasons); + } + private OptimizelyDecideOption[] GetAllOptions(OptimizelyDecideOption[] options) { var copiedOptions = DefaultDecideOptions; From 80bed0ae7fe07ab2c781f6d96b460fcdfdba3efb Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 21 Oct 2024 16:39:30 -0400 Subject: [PATCH 05/28] doc: add comment block --- OptimizelySDK/Optimizely.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index a84466e4..41ad272a 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -855,6 +855,14 @@ private OptimizelyUserContext CreateUserContextCopy(string userId, ); } + /// + /// Get forced decision for the given flag key. + /// + /// Flag key + /// A collection of decision reasons + /// The project config + /// The user context + /// Feature decision public FeatureDecision GetForcedDecision(string flagKey, DecisionReasons decisionReasons, ProjectConfig projectConfig, OptimizelyUserContext user ) @@ -1443,8 +1451,7 @@ internal void IdentifyUser(string userId) /// Dictionary for identifiers. The caller must provide at least one key-value pair. /// Type of event (defaults to `fullstack`) /// Optional event data in a key-value pair format - public void SendOdpEvent(string action, Dictionary identifiers, - string type = Constants.ODP_EVENT_TYPE, + public void SendOdpEvent(string action, Dictionary identifiers, string type = Constants.ODP_EVENT_TYPE, Dictionary data = null ) { From 075e51154f29fa897a942e18e91457f3cff2a55f Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 21 Oct 2024 16:41:21 -0400 Subject: [PATCH 06/28] chore: remove commented code --- OptimizelySDK/Optimizely.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 41ad272a..b76a2a97 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1089,14 +1089,9 @@ ProjectConfig projectConfig { "reasons", reasonsToReport }, { "decisionEventDispatched", decisionEventDispatched }, }; - - // var decisionNotificationType = - // projectConfig.IsFeatureExperiment(flagDecision.Experiment?.Id) ? - // DecisionNotificationTypes.FEATURE_TEST : - // DecisionNotificationTypes.AB_TEST; - NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, - userId, - user.GetAttributes(), decisionInfo); + + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, + userId, user.GetAttributes(), decisionInfo); return new OptimizelyDecision( variationKey, @@ -1419,8 +1414,7 @@ List segmentOptions if (config == null) { - Logger.Log(LogLevel.ERROR, - "Datafile has invalid format. Failing 'FetchQualifiedSegments'."); + Logger.Log(LogLevel.ERROR,"Datafile has invalid format. Failing 'FetchQualifiedSegments'."); return null; } From 344f451cda5eb8d1290f4bbc5965d5945be9ebd6 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Mon, 21 Oct 2024 16:52:00 -0400 Subject: [PATCH 07/28] test: fix via code under test --- OptimizelySDK/Optimizely.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index b76a2a97..7e32766c 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1081,9 +1081,9 @@ ProjectConfig projectConfig var decisionInfo = new Dictionary { - { "featureKey", flagKey }, - { "featureEnabled", flagEnabled }, - { "variableValues", variableMap }, + { "flagKey", flagKey }, + { "enabled", flagEnabled }, + { "variables", variableMap }, { "variationKey", variationKey }, { "ruleKey", ruleKey }, { "reasons", reasonsToReport }, @@ -1091,7 +1091,7 @@ ProjectConfig projectConfig }; NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, - userId, user.GetAttributes(), decisionInfo); + DecisionNotificationTypes.FLAG, userId, user.GetAttributes(), decisionInfo); return new OptimizelyDecision( variationKey, @@ -1414,7 +1414,7 @@ List segmentOptions if (config == null) { - Logger.Log(LogLevel.ERROR,"Datafile has invalid format. Failing 'FetchQualifiedSegments'."); + Logger.Log(LogLevel.ERROR, "Datafile has invalid format. Failing 'FetchQualifiedSegments'."); return null; } From c0ce71fd4af768d61c30003d2d3210cce24f7cab Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 09:47:14 -0400 Subject: [PATCH 08/28] fix: match updates to java PR --- OptimizelySDK/Bucketing/DecisionService.cs | 135 ++++++++++----------- OptimizelySDK/Optimizely.cs | 39 ++---- 2 files changed, 76 insertions(+), 98 deletions(-) diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 4dba482c..c1c0b2ef 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -41,8 +41,8 @@ public class DecisionService public const string LOGGING_KEY_TYPE_RULE = "rule"; private Bucketer Bucketer; - private IErrorHandler ErrorHandler; - private UserProfileService UserProfileService; + private static IErrorHandler ErrorHandler; + private static UserProfileService UserProfileService; private static ILogger Logger; /// @@ -118,8 +118,8 @@ OptimizelyDecideOption[] options if (UserProfileService != null && !ignoreUps) { - var userProfile = GetUserProfile(user.GetUserId(), reasons); - userProfileTracker = new UserProfileTracker(userProfile, false); + userProfileTracker = new UserProfileTracker(user.GetUserId()); + userProfileTracker.LoadUserProfile(reasons); } var response = GetVariation(experiment, user, config, options, userProfileTracker, @@ -128,7 +128,7 @@ OptimizelyDecideOption[] options if (UserProfileService != null && !ignoreUps && userProfileTracker?.ProfileUpdated == true) { - SaveUserProfile(userProfileTracker.UserProfile); + userProfileTracker.SaveUserProfile(); } return response; @@ -166,7 +166,6 @@ public virtual Result GetVariation(Experiment experiment, var userId = user.GetUserId(); - // check if a forced variation is set var decisionVariationResult = GetForcedVariation(experiment.Key, userId, config); reasons += decisionVariationResult.DecisionReasons; var variation = decisionVariationResult.ResultObject; @@ -733,75 +732,52 @@ public virtual Result GetVariationForFeature(FeatureFlag featur new OptimizelyDecideOption[] { }); } - void SaveUserProfile(UserProfile userProfile) + public class UserProfileTracker { - if (UserProfileService == null) - { - return; - } + public UserProfile UserProfile { get; private set; } + public bool ProfileUpdated { get; private set; } + private string UserId { get; set; } - try + public UserProfileTracker(string userId) { - UserProfileService.Save(userProfile.ToMap()); - Logger.Log(LogLevel.INFO, - $"Saved user profile of user \"{userProfile.UserId}\"."); + UserId = userId; + ProfileUpdated = false; + UserProfile = null; } - catch (Exception exception) + + public void LoadUserProfile(DecisionReasons reasons) { - Logger.Log(LogLevel.WARN, - $"Failed to save user profile of user \"{userProfile.UserId}\"."); - ErrorHandler.HandleError(new Exceptions.OptimizelyRuntimeException(exception.Message)); - } - } - - private UserProfile GetUserProfile(String userId, DecisionReasons reasons) - { - UserProfile userProfile = null; - - try - { - var userProfileMap = UserProfileService.Lookup(userId); - if (userProfileMap == null) + try { - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - "We were unable to get a user profile map from the UserProfileService.")); + var userProfileMap = UserProfileService.Lookup(UserId); + if (userProfileMap == null) + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + "We were unable to get a user profile map from the UserProfileService.")); + } + else if (UserProfileUtil.IsValidUserProfileMap(userProfileMap)) + { + UserProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); + } + else + { + Logger.Log(LogLevel.WARN, + reasons.AddInfo("The UserProfileService returned an invalid map.")); + } } - else if (UserProfileUtil.IsValidUserProfileMap(userProfileMap)) + catch (Exception exception) { - userProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); + Logger.Log(LogLevel.ERROR, reasons.AddInfo(exception.Message)); + ErrorHandler.HandleError( + new Exceptions.OptimizelyRuntimeException(exception.Message)); } - else + + if (UserProfile == null) { - Logger.Log(LogLevel.WARN, - reasons.AddInfo("The UserProfileService returned an invalid map.")); + UserProfile = new UserProfile(UserId, new Dictionary()); } } - catch (Exception exception) - { - Logger.Log(LogLevel.ERROR, reasons.AddInfo(exception.Message)); - ErrorHandler.HandleError( - new Exceptions.OptimizelyRuntimeException(exception.Message)); - } - - if (userProfile == null) - { - userProfile = new UserProfile(userId, new Dictionary()); - } - - return userProfile; - } - - public class UserProfileTracker - { - public UserProfile UserProfile { get; set; } - public bool ProfileUpdated { get; set; } - - public UserProfileTracker(UserProfile userProfile, bool profileUpdated) - { - UserProfile = userProfile; - ProfileUpdated = profileUpdated; - } public void UpdateUserProfile(Experiment experiment, Variation variation) { @@ -824,6 +800,27 @@ public void UpdateUserProfile(Experiment experiment, Variation variation) Logger.Log(LogLevel.INFO, $"Updated variation \"{variationId}\" of experiment \"{experimentId}\" for user \"{UserProfile.UserId}\"."); } + + public void SaveUserProfile() + { + if (!ProfileUpdated) + { + return; + } + + try + { + UserProfileService.Save(UserProfile.ToMap()); + Logger.Log(LogLevel.INFO, + $"Saved user profile of user \"{UserProfile.UserId}\"."); + } + catch (Exception exception) + { + Logger.Log(LogLevel.WARN, + $"Failed to save user profile of user \"{UserProfile.UserId}\"."); + ErrorHandler.HandleError(new Exceptions.OptimizelyRuntimeException(exception.Message)); + } + } } public virtual List> GetVariationsForFeatureList( @@ -836,13 +833,13 @@ OptimizelyDecideOption[] options { var upsReasons = new DecisionReasons(); - var ignoreUPS = options.Contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); + var ignoreUps = options.Contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); UserProfileTracker userProfileTracker = null; - if (UserProfileService != null && !ignoreUPS) + if (UserProfileService != null && !ignoreUps) { - var userProfile = GetUserProfile(user.GetUserId(), upsReasons); - userProfileTracker = new UserProfileTracker(userProfile, false); + userProfileTracker = new UserProfileTracker(user.GetUserId()); + userProfileTracker.LoadUserProfile(upsReasons); } var userId = user.GetUserId(); @@ -888,9 +885,9 @@ OptimizelyDecideOption[] options } } - if (UserProfileService != null && !ignoreUPS && userProfileTracker?.ProfileUpdated == true) + if (UserProfileService != null && !ignoreUps && userProfileTracker?.ProfileUpdated == true) { - SaveUserProfile(userProfileTracker.UserProfile); + userProfileTracker.SaveUserProfile(); } return decisions; diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 7e32766c..b9631333 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -855,31 +855,6 @@ private OptimizelyUserContext CreateUserContextCopy(string userId, ); } - /// - /// Get forced decision for the given flag key. - /// - /// Flag key - /// A collection of decision reasons - /// The project config - /// The user context - /// Feature decision - public FeatureDecision GetForcedDecision(string flagKey, DecisionReasons decisionReasons, - ProjectConfig projectConfig, OptimizelyUserContext user - ) - { - var context = new OptimizelyDecisionContext(flagKey); - var forcedDecisionVariation = - DecisionService.ValidatedForcedDecision(context, projectConfig, user); - decisionReasons += forcedDecisionVariation.DecisionReasons; - if (forcedDecisionVariation.ResultObject != null) - { - return new FeatureDecision(null, forcedDecisionVariation.ResultObject, - FeatureDecision.DECISION_SOURCE_FEATURE_TEST); - } - - return null; - } - /// /// Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context, which contains all data required to deliver the flag. ///
    @@ -975,12 +950,18 @@ internal Dictionary DecideForKeys(OptimizelyUserCont validKeys.Add(key); var decisionReasons = new DecisionReasons(); - var forcedDecision = GetForcedDecision(key, decisionReasons, projectConfig, user); decisionReasonsMap.Add(key, decisionReasons); - - if (forcedDecision != null) + + var optimizelyDecisionContext = new OptimizelyDecisionContext(key); + var forcedDecisionVariation = + DecisionService.ValidatedForcedDecision(optimizelyDecisionContext, projectConfig, user); + decisionReasons += forcedDecisionVariation.DecisionReasons; + + if (forcedDecisionVariation.ResultObject != null) { - flagDecisions.Add(key, forcedDecision); + flagDecisions.Add(key, new FeatureDecision(null, + forcedDecisionVariation.ResultObject, + FeatureDecision.DECISION_SOURCE_FEATURE_TEST)); } else { From 3fab122f25eeeb68408aab6d4f3dbf5f9a0c2a5b Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 11:08:32 -0400 Subject: [PATCH 09/28] fix: code from test results --- OptimizelySDK.Tests/DecisionServiceTest.cs | 8 ++-- OptimizelySDK/Bucketing/DecisionService.cs | 50 +++++++++++----------- OptimizelySDK/Optimizely.cs | 2 +- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index 633847ae..82096156 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -856,7 +856,7 @@ public void TestGetVariationForFeatureRolloutWhenRolloutIsNotInDataFile() ds.GetVariationForFeatureExperiment(It.IsAny(), It.IsAny(), It.IsAny(), ProjectConfig, - new OptimizelyDecideOption[] { })). + new OptimizelyDecideOption[] { }, null)). Returns(null); var optlyObject = new Optimizely(TestData.Datafile, new ValidEventDispatcher(), LoggerMock.Object); @@ -1201,7 +1201,7 @@ public void TestGetVariationForFeatureWhenTheUserIsBucketedIntoFeatureExperiment DecisionServiceMock.Setup(ds => ds.GetVariationForFeatureExperiment( It.IsAny(), It.IsAny(), It.IsAny(), ProjectConfig, - It.IsAny())). + It.IsAny(), null)). Returns(expectedDecision); OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns("user1"); @@ -1228,7 +1228,7 @@ public void DecisionServiceMock.Setup(ds => ds.GetVariationForFeatureExperiment( It.IsAny(), It.IsAny(), It.IsAny(), ProjectConfig, - It.IsAny())). + It.IsAny(), null)). Returns(Result.NullResult(null)); DecisionServiceMock.Setup(ds => ds.GetVariationForFeatureRollout( It.IsAny(), It.IsAny(), @@ -1262,7 +1262,7 @@ public void ds.GetVariationForFeatureExperiment(It.IsAny(), It.IsAny(), It.IsAny(), ProjectConfig, - new OptimizelyDecideOption[] { })). + new OptimizelyDecideOption[] { }, null)). Returns(Result.NullResult(null)); DecisionServiceMock. Setup(ds => ds.GetVariationForFeatureRollout(It.IsAny(), diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index c1c0b2ef..aac103d4 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -166,33 +166,32 @@ public virtual Result GetVariation(Experiment experiment, var userId = user.GetUserId(); - var decisionVariationResult = GetForcedVariation(experiment.Key, userId, config); - reasons += decisionVariationResult.DecisionReasons; - var variation = decisionVariationResult.ResultObject; + var decisionVariation = GetForcedVariation(experiment.Key, userId, config); + reasons += decisionVariation.DecisionReasons; + var variation = decisionVariation.ResultObject; if (variation == null) { - decisionVariationResult = GetWhitelistedVariation(experiment, user.GetUserId()); - reasons += decisionVariationResult.DecisionReasons; - - variation = decisionVariationResult.ResultObject; + decisionVariation = GetWhitelistedVariation(experiment, user.GetUserId()); + reasons += decisionVariation.DecisionReasons; + variation = decisionVariation.ResultObject; } if (variation != null) { - decisionVariationResult.SetReasons(reasons); - return decisionVariationResult; + decisionVariation.SetReasons(reasons); + return decisionVariation; } if (userProfileTracker != null) { - decisionVariationResult = + decisionVariation = GetStoredVariation(experiment, userProfileTracker.UserProfile, config); - reasons += decisionVariationResult.DecisionReasons; - variation = decisionVariationResult.ResultObject; + reasons += decisionVariation.DecisionReasons; + variation = decisionVariation.ResultObject; if (variation != null) { - return decisionVariationResult; + return decisionVariation; } } @@ -202,13 +201,11 @@ public virtual Result GetVariation(Experiment experiment, reasons += decisionMeetAudience.DecisionReasons; if (decisionMeetAudience.ResultObject) { - var bucketingIdResult = GetBucketingId(userId, user.GetAttributes()); - reasons += bucketingIdResult.DecisionReasons; + var bucketingId = GetBucketingId(userId, user.GetAttributes()).ResultObject; - decisionVariationResult = Bucketer.Bucket(config, experiment, - bucketingIdResult.ResultObject, userId); - reasons += decisionVariationResult.DecisionReasons; - variation = decisionVariationResult.ResultObject; + decisionVariation = Bucketer.Bucket(config, experiment, bucketingId, userId); + reasons += decisionVariation.DecisionReasons; + variation = decisionVariation.ResultObject; if (variation != null) { @@ -218,12 +215,12 @@ public virtual Result GetVariation(Experiment experiment, } else { - Logger.Log(LogLevel.DEBUG, + Logger.Log(LogLevel.INFO, "This decision will not be saved since the UserProfileService is null."); } } - return decisionVariationResult.SetReasons(reasons); + return decisionVariation.SetReasons(reasons); } Logger.Log(LogLevel.INFO, @@ -651,7 +648,8 @@ public virtual Result GetVariationForFeatureExperiment( OptimizelyUserContext user, UserAttributes filteredAttributes, ProjectConfig config, - OptimizelyDecideOption[] options + OptimizelyDecideOption[] options, + UserProfileTracker userProfileTracker = null ) { var reasons = new DecisionReasons(); @@ -692,7 +690,7 @@ OptimizelyDecideOption[] options } else { - var decisionResponse = GetVariation(experiment, user, config, options); + var decisionResponse = GetVariation(experiment, user, config, options, userProfileTracker); reasons += decisionResponse?.DecisionReasons; decisionVariation = decisionResponse.ResultObject; @@ -736,7 +734,7 @@ public class UserProfileTracker { public UserProfile UserProfile { get; private set; } public bool ProfileUpdated { get; private set; } - private string UserId { get; set; } + private string UserId { get; } public UserProfileTracker(string userId) { @@ -798,7 +796,7 @@ public void UpdateUserProfile(Experiment experiment, Variation variation) ProfileUpdated = true; Logger.Log(LogLevel.INFO, - $"Updated variation \"{variationId}\" of experiment \"{experimentId}\" for user \"{UserProfile.UserId}\"."); + $"Saved variation \"{variationId}\" of experiment \"{experimentId}\" for user \"{UserProfile.UserId}\"."); } public void SaveUserProfile() @@ -852,7 +850,7 @@ OptimizelyDecideOption[] options // Check if the feature flag has an experiment and the user is bucketed into that experiment. var decisionResult = GetVariationForFeatureExperiment(featureFlag, user, - filteredAttributes, projectConfig, options); + filteredAttributes, projectConfig, options, userProfileTracker); reasons += decisionResult.DecisionReasons; if (decisionResult.ResultObject != null) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index b9631333..9f0b4a6e 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -900,7 +900,7 @@ OptimizelyDecideOption[] options } var allFlags = projectConfig.FeatureFlags; - var allFlagKeys = allFlags.Select(v => v.Key).ToArray(); + var allFlagKeys = allFlags.Select(v => v.Key).ToArray(); return DecideForKeys(user, allFlagKeys, options); } From bfdc2e09fefb5a3a683733d62be69d003ed9fb7e Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 11:22:31 -0400 Subject: [PATCH 10/28] test: correct test --- OptimizelySDK.Tests/OptimizelyUserContextTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs index bd41baab..cb0ee9d2 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs +++ b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs @@ -426,7 +426,7 @@ public void DecideWhenConfigIsNull() } [Test] - public void SeparateDecideShouldHaveSameNumberOfUpsSaveOnlyOneLookup() + public void SeparateDecideShouldHaveSameNumberOfUpsSaveAndLookup() { var experimentFlagKey = "double_single_variable_feature"; var rolloutFlagKey = "boolean_single_variable_feature"; @@ -457,7 +457,7 @@ public void SeparateDecideShouldHaveSameNumberOfUpsSaveOnlyOneLookup() LoggerMock.Verify( l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), Times.Never); - userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Exactly(2)); userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), Times.Exactly(2)); Assert.AreEqual(saveArgsCollector[0], expectedUserProfileExperiment.ToMap()); From a36280a58467621d01f38a06979aaaa48f7184fd Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 12:31:20 -0400 Subject: [PATCH 11/28] tests: updated --- OptimizelySDK.Tests/DecisionServiceTest.cs | 9 ++++--- .../OptimizelyUserContextTest.cs | 27 ++++++++++--------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index 82096156..862d12f5 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -150,15 +150,18 @@ public void TestGetVariationLogsErrorWhenUserProfileMapItsNull() var decisionService = new DecisionService(BucketerMock.Object, ErrorHandlerMock.Object, UserProfileServiceMock.Object, LoggerMock.Object); var options = new OptimizelyDecideOption[] { OptimizelyDecideOption.INCLUDE_REASONS }; + var optlyObject = new Optimizely(TestData.Datafile, new ValidEventDispatcher(), + LoggerMock.Object); + OptimizelyUserContextMock = new Mock(optlyObject, + WhitelistedUserId, new UserAttributes(), ErrorHandlerMock.Object, + LoggerMock.Object); OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(GenericUserId); var variationResult = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, options); Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[0], - "We were unable to get a user profile map from the UserProfileService."); - Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[1], "Audiences for experiment \"etag3\" collectively evaluated to FALSE"); - Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[2], + Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[1], "User \"genericUserId\" does not meet conditions to be in experiment \"etag3\"."); } diff --git a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs index cb0ee9d2..66e974c9 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs +++ b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs @@ -428,27 +428,27 @@ public void DecideWhenConfigIsNull() [Test] public void SeparateDecideShouldHaveSameNumberOfUpsSaveAndLookup() { - var experimentFlagKey = "double_single_variable_feature"; - var rolloutFlagKey = "boolean_single_variable_feature"; + var flag1 = "double_single_variable_feature"; + var flag2 = "integer_single_variable_feature"; var userProfileServiceMock = MakeUserProfileServiceMock(); var saveArgsCollector = new List>(); userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); var user = optimizely.CreateUserContext(UserID); - var expectedUserProfileExperiment = new UserProfile(UserID, new Dictionary + var flag1UserProfile = new UserProfile(UserID, new Dictionary { { "224", new Decision("280") }, { "122238", new Decision("122240") }, - }); - var expectedUserProfileRollout = new UserProfile(UserID, new Dictionary + }); + var flag2UserProfile = new UserProfile(UserID, new Dictionary { - { "999", new Decision("99") }, - { "99999", new Decision("999") }, + { "224", new Decision("280") }, + { "122241", new Decision("122242") }, }); - user.Decide(experimentFlagKey); - user.Decide(rolloutFlagKey); + user.Decide(flag1); + user.Decide(flag2); LoggerMock.Verify( l => l.Log(LogLevel.INFO, @@ -460,8 +460,8 @@ public void SeparateDecideShouldHaveSameNumberOfUpsSaveAndLookup() userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Exactly(2)); userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), Times.Exactly(2)); - Assert.AreEqual(saveArgsCollector[0], expectedUserProfileExperiment.ToMap()); - Assert.AreEqual(saveArgsCollector[1], expectedUserProfileRollout.ToMap()); + Assert.AreEqual(saveArgsCollector[0], flag1UserProfile.ToMap()); + Assert.AreEqual(saveArgsCollector[1], flag2UserProfile.ToMap()); } [Test] @@ -513,6 +513,8 @@ public void DecideForKeysWithUpsShouldOnlyLookupSaveOnceWithMultipleFlags() { { "224", new Decision("280") }, { "122238", new Decision("122240") }, + { "7723330021", new Decision(null) }, + { "7718750065", new Decision(null) }, }); userContext.DecideForKeys(flagKeys); @@ -579,7 +581,8 @@ public void DecideAllWithUpsShouldOnlyLookupSaveOnce() { "122238", new Decision("122240") }, { "122241", new Decision("122242") }, { "122235", new Decision("122236") }, - { "188880", new Decision("188881") }, + { "7723330021", new Decision(null) }, + { "7718750065", new Decision(null) }, }); user.DecideAll(); From c23e898223924bf7d5edfeb5e10de68b7969b265 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 12:31:43 -0400 Subject: [PATCH 12/28] refactor: rename to match Java ref imp --- OptimizelySDK/Optimizely.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 9f0b4a6e..5c678f99 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -878,11 +878,11 @@ OptimizelyDecideOption[] options ErrorHandler, Logger); } - var filteredOptions = GetAllOptions(options). + var allOptions = GetAllOptions(options). Where(opt => opt != OptimizelyDecideOption.ENABLED_FLAGS_ONLY). ToArray(); - return DecideForKeys(user, new[] { key }, filteredOptions, true)[key]; + return DecideForKeys(user, new[] { key }, allOptions, true)[key]; } internal Dictionary DecideAll(OptimizelyUserContext user, From 03037e2f2059c1e1535acb9fee9a434e75b59b86 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 12:42:54 -0400 Subject: [PATCH 13/28] refactor: extract UserProfileTracker --- OptimizelySDK/Bucketing/DecisionService.cs | 116 +++--------------- OptimizelySDK/Bucketing/UserProfileTracker.cs | 108 ++++++++++++++++ OptimizelySDK/OptimizelySDK.csproj | 1 + 3 files changed, 124 insertions(+), 101 deletions(-) create mode 100644 OptimizelySDK/Bucketing/UserProfileTracker.cs diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index aac103d4..7167779e 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -41,9 +41,9 @@ public class DecisionService public const string LOGGING_KEY_TYPE_RULE = "rule"; private Bucketer Bucketer; - private static IErrorHandler ErrorHandler; + private readonly IErrorHandler ErrorHandler; private static UserProfileService UserProfileService; - private static ILogger Logger; + private readonly ILogger Logger; /// /// Associative array of user IDs to an associative array @@ -118,7 +118,8 @@ OptimizelyDecideOption[] options if (UserProfileService != null && !ignoreUps) { - userProfileTracker = new UserProfileTracker(user.GetUserId()); + userProfileTracker = new UserProfileTracker(UserProfileService, user.GetUserId(), + Logger, ErrorHandler); userProfileTracker.LoadUserProfile(reasons); } @@ -690,7 +691,8 @@ public virtual Result GetVariationForFeatureExperiment( } else { - var decisionResponse = GetVariation(experiment, user, config, options, userProfileTracker); + var decisionResponse = GetVariation(experiment, user, config, options, + userProfileTracker); reasons += decisionResponse?.DecisionReasons; decisionVariation = decisionResponse.ResultObject; @@ -730,97 +732,6 @@ public virtual Result GetVariationForFeature(FeatureFlag featur new OptimizelyDecideOption[] { }); } - public class UserProfileTracker - { - public UserProfile UserProfile { get; private set; } - public bool ProfileUpdated { get; private set; } - private string UserId { get; } - - public UserProfileTracker(string userId) - { - UserId = userId; - ProfileUpdated = false; - UserProfile = null; - } - - public void LoadUserProfile(DecisionReasons reasons) - { - try - { - var userProfileMap = UserProfileService.Lookup(UserId); - if (userProfileMap == null) - { - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - "We were unable to get a user profile map from the UserProfileService.")); - } - else if (UserProfileUtil.IsValidUserProfileMap(userProfileMap)) - { - UserProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); - } - else - { - Logger.Log(LogLevel.WARN, - reasons.AddInfo("The UserProfileService returned an invalid map.")); - } - } - catch (Exception exception) - { - Logger.Log(LogLevel.ERROR, reasons.AddInfo(exception.Message)); - ErrorHandler.HandleError( - new Exceptions.OptimizelyRuntimeException(exception.Message)); - } - - if (UserProfile == null) - { - UserProfile = new UserProfile(UserId, new Dictionary()); - } - } - - public void UpdateUserProfile(Experiment experiment, Variation variation) - { - var experimentId = experiment.Id; - var variationId = variation.Id; - Decision decision; - if (UserProfile.ExperimentBucketMap.ContainsKey(experimentId)) - { - decision = UserProfile.ExperimentBucketMap[experimentId]; - decision.VariationId = variationId; - } - else - { - decision = new Decision(variationId); - } - - UserProfile.ExperimentBucketMap[experimentId] = decision; - ProfileUpdated = true; - - Logger.Log(LogLevel.INFO, - $"Saved variation \"{variationId}\" of experiment \"{experimentId}\" for user \"{UserProfile.UserId}\"."); - } - - public void SaveUserProfile() - { - if (!ProfileUpdated) - { - return; - } - - try - { - UserProfileService.Save(UserProfile.ToMap()); - Logger.Log(LogLevel.INFO, - $"Saved user profile of user \"{UserProfile.UserId}\"."); - } - catch (Exception exception) - { - Logger.Log(LogLevel.WARN, - $"Failed to save user profile of user \"{UserProfile.UserId}\"."); - ErrorHandler.HandleError(new Exceptions.OptimizelyRuntimeException(exception.Message)); - } - } - } - public virtual List> GetVariationsForFeatureList( List featureFlags, OptimizelyUserContext user, @@ -836,7 +747,8 @@ OptimizelyDecideOption[] options if (UserProfileService != null && !ignoreUps) { - userProfileTracker = new UserProfileTracker(user.GetUserId()); + userProfileTracker = new UserProfileTracker(UserProfileService, user.GetUserId(), + Logger, ErrorHandler); userProfileTracker.LoadUserProfile(upsReasons); } @@ -883,7 +795,8 @@ OptimizelyDecideOption[] options } } - if (UserProfileService != null && !ignoreUps && userProfileTracker?.ProfileUpdated == true) + if (UserProfileService != null && !ignoreUps && + userProfileTracker?.ProfileUpdated == true) { userProfileTracker.SaveUserProfile(); } @@ -909,10 +822,11 @@ OptimizelyDecideOption[] options ) { return GetVariationsForFeatureList(new List { featureFlag }, - user, - config, - filteredAttributes, - options).First(); + user, + config, + filteredAttributes, + options). + First(); } /// diff --git a/OptimizelySDK/Bucketing/UserProfileTracker.cs b/OptimizelySDK/Bucketing/UserProfileTracker.cs new file mode 100644 index 00000000..226cca48 --- /dev/null +++ b/OptimizelySDK/Bucketing/UserProfileTracker.cs @@ -0,0 +1,108 @@ +using System; +using System.Collections.Generic; +using OptimizelySDK.Entity; +using OptimizelySDK.ErrorHandler; +using OptimizelySDK.Logger; +using OptimizelySDK.OptimizelyDecisions; + +namespace OptimizelySDK.Bucketing +{ + public class UserProfileTracker + { + public UserProfile UserProfile { get; private set; } + public bool ProfileUpdated { get; private set; } + + private readonly UserProfileService _userProfileService; + private readonly string _userId; + private readonly ILogger _logger; + private readonly IErrorHandler _errorHandler; + + public UserProfileTracker(UserProfileService userProfileService, string userId, ILogger logger, IErrorHandler errorHandler) + { + _userProfileService = userProfileService; + _userId = userId; + _logger = logger; + _errorHandler = errorHandler; + ProfileUpdated = false; + UserProfile = null; + } + + public void LoadUserProfile(DecisionReasons reasons) + { + try + { + var userProfileMap = _userProfileService.Lookup(_userId); + if (userProfileMap == null) + { + _logger.Log(LogLevel.INFO, + reasons.AddInfo( + "We were unable to get a user profile map from the UserProfileService.")); + } + else if (UserProfileUtil.IsValidUserProfileMap(userProfileMap)) + { + UserProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); + } + else + { + _logger.Log(LogLevel.WARN, + reasons.AddInfo("The UserProfileService returned an invalid map.")); + } + } + catch (Exception exception) + { + _logger.Log(LogLevel.ERROR, reasons.AddInfo(exception.Message)); + _errorHandler.HandleError( + new Exceptions.OptimizelyRuntimeException(exception.Message)); + } + + if (UserProfile == null) + { + UserProfile = new UserProfile(_userId, new Dictionary()); + } + } + + public void UpdateUserProfile(Experiment experiment, Variation variation) + { + var experimentId = experiment.Id; + var variationId = variation.Id; + Decision decision; + if (UserProfile.ExperimentBucketMap.ContainsKey(experimentId)) + { + decision = UserProfile.ExperimentBucketMap[experimentId]; + decision.VariationId = variationId; + } + else + { + decision = new Decision(variationId); + } + + UserProfile.ExperimentBucketMap[experimentId] = decision; + ProfileUpdated = true; + + _logger.Log(LogLevel.INFO, + $"Saved variation \"{variationId}\" of experiment \"{experimentId}\" for user \"{UserProfile.UserId}\"."); + } + + public void SaveUserProfile() + { + if (!ProfileUpdated) + { + return; + } + + try + { + _userProfileService.Save(UserProfile.ToMap()); + _logger.Log(LogLevel.INFO, + $"Saved user profile of user \"{UserProfile.UserId}\"."); + } + catch (Exception exception) + { + _logger.Log(LogLevel.WARN, + $"Failed to save user profile of user \"{UserProfile.UserId}\"."); + _errorHandler.HandleError( + new Exceptions.OptimizelyRuntimeException(exception.Message)); + } + } + } +} diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 1812a2ad..5f041ac1 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -75,6 +75,7 @@ + From 2ea917eba74c23779f4f1ed852890bd20d341157 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 16:22:12 -0400 Subject: [PATCH 14/28] test: fixes --- OptimizelySDK.Tests/DecisionServiceTest.cs | 34 +++++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index 862d12f5..02e2138d 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -1,6 +1,6 @@ /** * - * Copyright 2017-2021, Optimizely and contributors + * Copyright 2017-2021, 2024 Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -65,8 +65,8 @@ public void SetUp() DecisionService = new DecisionService(new Bucketer(LoggerMock.Object), ErrorHandlerMock.Object, null, LoggerMock.Object); DecisionServiceMock = new Mock(BucketerMock.Object, - ErrorHandlerMock.Object, null, LoggerMock.Object) - { CallBase = true }; + ErrorHandlerMock.Object, null, LoggerMock.Object) + { CallBase = true }; DecisionReasons = new DecisionReasons(); VariationWithKeyControl = @@ -294,6 +294,9 @@ public void TestBucketReturnsVariationStoredInUserProfile() { var experiment = ProjectConfig.Experiments[6]; var variation = experiment.Variations[0]; + var variationResult = Result.NewResult( + experiment.Variations[0], + DecisionReasons); var decision = new Decision(variation.Id); var userProfile = new UserProfile(UserProfileId, new Dictionary @@ -303,8 +306,10 @@ public void TestBucketReturnsVariationStoredInUserProfile() UserProfileServiceMock.Setup(_ => _.Lookup(UserProfileId)).Returns(userProfile.ToMap()); - var decisionService = new DecisionService(BucketerMock.Object, ErrorHandlerMock.Object, - UserProfileServiceMock.Object, LoggerMock.Object); + BucketerMock. + Setup(bm => bm.Bucket(ProjectConfig, experiment, It.IsAny(), + It.IsAny())). + Returns(variationResult); var optlyObject = new Optimizely(TestData.Datafile, new ValidEventDispatcher(), LoggerMock.Object); @@ -313,6 +318,8 @@ public void TestBucketReturnsVariationStoredInUserProfile() LoggerMock.Object); OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId); + var decisionService = new DecisionService(BucketerMock.Object, ErrorHandlerMock.Object, + UserProfileServiceMock.Object, LoggerMock.Object); var actualVariation = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig); @@ -739,7 +746,8 @@ public void TestGetVariationForFeatureExperimentGivenNonMutexGroupAndUserIsBucke DecisionServiceMock.Setup(ds => ds.GetVariation( ProjectConfig.GetExperimentFromKey("test_experiment_multivariate"), OptimizelyUserContextMock.Object, ProjectConfig, - It.IsAny())). + It.IsAny(), It.IsAny(), + It.IsAny())). Returns(variation); var featureFlag = ProjectConfig.GetFeatureFlagFromKey("multi_variate_feature"); @@ -792,13 +800,18 @@ public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserIsBucketed [Test] public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserNotBucketed() { - var mutexExperiment = ProjectConfig.GetExperimentFromKey("group_experiment_1"); + var optlyObject = new Optimizely(TestData.Datafile, new ValidEventDispatcher(), + LoggerMock.Object); + OptimizelyUserContextMock = new Mock(optlyObject, + WhitelistedUserId, new UserAttributes(), ErrorHandlerMock.Object, + LoggerMock.Object); OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns("user1"); DecisionServiceMock. Setup(ds => ds.GetVariation(It.IsAny(), It.IsAny(), ProjectConfig, - It.IsAny())). + It.IsAny(), It.IsAny(), + It.IsAny())). Returns(Result.NullResult(null)); var featureFlag = ProjectConfig.GetFeatureFlagFromKey("boolean_feature"); @@ -1312,6 +1325,11 @@ public void TestGetVariationForFeatureWhenTheUserIsBuckedtedInBothExperimentAndR WhitelistedUserId, userAttributes, ErrorHandlerMock.Object, LoggerMock.Object); OptimizelyUserContextMock.Setup(ouc => ouc.GetUserId()).Returns(UserProfileId); + BucketerMock. + Setup(bm => bm.Bucket(ProjectConfig, experiment, It.IsAny(), + It.IsAny())). + Returns(variation); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, It.IsAny())). From ecda765713d2f298c526a10ad794c46a61f61836 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 17:31:09 -0400 Subject: [PATCH 15/28] fix: tests & code under tests --- .../OptimizelyUserContextTest.cs | 19 ++++++++++--------- OptimizelySDK/Optimizely.cs | 3 ++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs index 66e974c9..76d0d8b8 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs +++ b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs @@ -61,7 +61,7 @@ public void SetUp() Optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object); } - + private Mock MakeUserProfileServiceMock() { var projectConfig = DatafileProjectConfig.Create(TestData.Datafile, LoggerMock.Object, @@ -440,7 +440,7 @@ public void SeparateDecideShouldHaveSameNumberOfUpsSaveAndLookup() { { "224", new Decision("280") }, { "122238", new Decision("122240") }, - }); + }); var flag2UserProfile = new UserProfile(UserID, new Dictionary { { "224", new Decision("280") }, @@ -531,7 +531,7 @@ public void DecideForKeysWithUpsShouldOnlyLookupSaveOnceWithMultipleFlags() Times.Once); Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); } - + [Test] public void DecideForKeysWithOneFlag() { @@ -561,11 +561,11 @@ public void DecideForKeysWithOneFlag() new string[0]); Assert.IsTrue(TestData.CompareObjects(decision, expDecision)); } - + #endregion DecideForKeys - + #region DecideAll - + [Test] public void DecideAllWithUpsShouldOnlyLookupSaveOnce() { @@ -595,8 +595,9 @@ public void DecideAllWithUpsShouldOnlyLookupSaveOnce() l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), Times.Never); userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); - userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), Times.Once); - Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); } [Test] @@ -806,7 +807,7 @@ public void DecideAllAllFlags() null, flagKey10, user, - new string[0]); + new[] { "Variable value for key \"any_key\" is invalid or wrong type." }); Assert.IsTrue(TestData.CompareObjects(decisions[flagKey10], expDecision10)); } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 5c678f99..775b9a2d 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1040,7 +1040,8 @@ ProjectConfig projectConfig decisionSource = flagDecision.Source; } - var reasonsToReport = decisionReasons.ToReport().ToArray(); + var includeReasons= allOptions.Contains(OptimizelyDecideOption.INCLUDE_REASONS); + var reasonsToReport = decisionReasons.ToReport(includeReasons).ToArray(); var variationKey = flagDecision.Variation?.Key; // TODO: add ruleKey values when available later. use a copy of experimentKey until then. // add to event metadata as well (currently set to experimentKey) From e0124c24f43d27a2480c5ad3f06e4b7fe0efea74 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 17:36:41 -0400 Subject: [PATCH 16/28] ci: also kick off CI if a PR turns to ready for review --- .github/workflows/csharp.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 8a6f6426..5fe0dc38 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -5,6 +5,8 @@ on: branches: [ master ] pull_request: branches: [ master ] + pull_request_target: + types: [ready_for_review] jobs: failOnDraftPullRequest: From 097e90c005cfe160dcb133d565d6ff5f1059bdf0 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 17:43:06 -0400 Subject: [PATCH 17/28] ci: try to ready for review trigger --- .github/workflows/csharp.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 5fe0dc38..2f13b031 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -5,8 +5,7 @@ on: branches: [ master ] pull_request: branches: [ master ] - pull_request_target: - types: [ready_for_review] + types: [ opened, synchronize, reopened, ready_for_review ] jobs: failOnDraftPullRequest: From dbf7eb17c1a36c72ff56cc706cdb82114ac12a9a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 17:44:54 -0400 Subject: [PATCH 18/28] ci: for now just manually kick it off --- .github/workflows/csharp.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 2f13b031..8a6f6426 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -5,7 +5,6 @@ on: branches: [ master ] pull_request: branches: [ master ] - types: [ opened, synchronize, reopened, ready_for_review ] jobs: failOnDraftPullRequest: From 5d0584fbe9283c4ebfa11bd957412e34445d1c2a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 22 Oct 2024 17:48:35 -0400 Subject: [PATCH 19/28] ci: remove Fail If Draft Pull Request --- .github/workflows/csharp.yml | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 8a6f6426..7228976e 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -5,21 +5,12 @@ on: branches: [ master ] pull_request: branches: [ master ] + types: [ opened, synchronize, reopened, ready_for_review ] -jobs: - failOnDraftPullRequest: - name: Fail If Draft Pull Request - if: github.event.pull_request.draft == true - runs-on: ubuntu-latest - steps: - - name: Fails Draft pull request. - run: | - echo "Draft pull requests should not trigger CI. Please mark the pull request as Ready for review to continue." - exit 1 - +jobs: lintCodebase: - name: Lint Codebase - needs: [ failOnDraftPullRequest ] + name: Lint Codebase if Not Draft + if: github.event.pull_request.draft == false runs-on: ubuntu-latest steps: - name: Checkout code From 99d59c1cf9b2f3675bdaa1b8b79a9fb7a19ae080 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Oct 2024 09:55:02 -0400 Subject: [PATCH 20/28] fix: lints & copyright dates --- OptimizelySDK.Tests/DecisionServiceTest.cs | 2 +- OptimizelySDK/Bucketing/DecisionService.cs | 15 +++------ OptimizelySDK/Optimizely.cs | 38 +++++++++------------- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index 02e2138d..d1bf15af 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -66,7 +66,7 @@ public void SetUp() ErrorHandlerMock.Object, null, LoggerMock.Object); DecisionServiceMock = new Mock(BucketerMock.Object, ErrorHandlerMock.Object, null, LoggerMock.Object) - { CallBase = true }; + { CallBase = true }; DecisionReasons = new DecisionReasons(); VariationWithKeyControl = diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 7167779e..f001da69 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -264,8 +264,7 @@ ProjectConfig config if (experimentToVariationMap.ContainsKey(experimentId) == false) { Logger.Log(LogLevel.DEBUG, - $@"No experiment ""{experimentKey}"" mapped to user ""{userId - }"" in the forced variation map."); + $@"No experiment ""{experimentKey}"" mapped to user ""{userId}"" in the forced variation map."); return Result.NullResult(reasons); } @@ -274,8 +273,7 @@ ProjectConfig config if (string.IsNullOrEmpty(variationId)) { Logger.Log(LogLevel.DEBUG, - $@"No variation mapped to experiment ""{experimentKey - }"" in the forced variation map."); + $@"No variation mapped to experiment ""{experimentKey}"" in the forced variation map."); return Result.NullResult(reasons); } @@ -288,8 +286,7 @@ ProjectConfig config } Logger.Log(LogLevel.DEBUG, - reasons.AddInfo($@"Variation ""{variationKey}"" is mapped to experiment ""{ - experimentKey}"" and user ""{userId}"" in the forced variation map")); + reasons.AddInfo($@"Variation ""{variationKey}"" is mapped to experiment ""{experimentKey}"" and user ""{userId}"" in the forced variation map")); var variation = config.GetVariationFromKey(experimentKey, variationKey); @@ -333,8 +330,7 @@ ProjectConfig config } Logger.Log(LogLevel.DEBUG, - $@"Variation mapped to experiment ""{experimentKey - }"" has been removed for user ""{userId}""."); + $@"Variation mapped to experiment ""{experimentKey}"" has been removed for user ""{userId}""."); return true; } @@ -356,8 +352,7 @@ ProjectConfig config ForcedVariationMap[userId][experimentId] = variationId; Logger.Log(LogLevel.DEBUG, - $@"Set variation ""{variationId}"" for experiment ""{experimentId}"" and user ""{ - userId}"" in the forced variation map."); + $@"Set variation ""{variationId}"" for experiment ""{experimentId}"" and user ""{userId}"" in the forced variation map."); return true; } diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 775b9a2d..b1766985 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017-2023, Optimizely + * Copyright 2017-2024, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use file except in compliance with the License. @@ -573,8 +573,7 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, else { Logger.Log(LogLevel.INFO, - $@"The user ""{userId}"" is not being experimented on feature ""{featureKey - }""."); + $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}""."); } } @@ -624,8 +623,7 @@ public virtual T GetFeatureVariableValueForType(string featureKey, string var if (config == null) { Logger.Log(LogLevel.ERROR, - $@"Datafile has invalid format. Failing '{ - FeatureVariable.GetFeatureVariableTypeName(variableType)}'."); + $@"Datafile has invalid format. Failing '{FeatureVariable.GetFeatureVariableTypeName(variableType)}'."); return default; } @@ -649,15 +647,13 @@ public virtual T GetFeatureVariableValueForType(string featureKey, string var if (featureVariable == null) { Logger.Log(LogLevel.ERROR, - $@"No feature variable was found for key ""{variableKey}"" in feature flag ""{ - featureKey}""."); + $@"No feature variable was found for key ""{variableKey}"" in feature flag ""{featureKey}""."); return default; } else if (featureVariable.Type != variableType) { Logger.Log(LogLevel.ERROR, - $@"Variable is of type ""{featureVariable.Type - }"", but you requested it as type ""{variableType}""."); + $@"Variable is of type ""{featureVariable.Type}"", but you requested it as type ""{variableType}""."); return default; } @@ -681,28 +677,24 @@ public virtual T GetFeatureVariableValueForType(string featureKey, string var { variableValue = featureVariableUsageInstance.Value; Logger.Log(LogLevel.INFO, - $@"Got variable value ""{variableValue}"" for variable ""{variableKey - }"" of feature flag ""{featureKey}""."); + $@"Got variable value ""{variableValue}"" for variable ""{variableKey}"" of feature flag ""{featureKey}""."); } else { Logger.Log(LogLevel.INFO, - $@"Feature ""{featureKey}"" is not enabled for user {userId - }. Returning the default variable value ""{variableValue}""."); + $@"Feature ""{featureKey}"" is not enabled for user {userId}. Returning the default variable value ""{variableValue}""."); } } else { Logger.Log(LogLevel.INFO, - $@"Variable ""{variableKey}"" is not used in variation ""{variation.Key - }"", returning default value ""{variableValue}""."); + $@"Variable ""{variableKey}"" is not used in variation ""{variation.Key}"", returning default value ""{variableValue}""."); } } else { Logger.Log(LogLevel.INFO, - $@"User ""{userId}"" is not in any variation for feature flag ""{featureKey - }"", returning default value ""{variableValue}""."); + $@"User ""{userId}"" is not in any variation for feature flag ""{featureKey}"", returning default value ""{variableValue}""."); } var sourceInfo = new Dictionary(); @@ -951,12 +943,12 @@ internal Dictionary DecideForKeys(OptimizelyUserCont var decisionReasons = new DecisionReasons(); decisionReasonsMap.Add(key, decisionReasons); - + var optimizelyDecisionContext = new OptimizelyDecisionContext(key); var forcedDecisionVariation = DecisionService.ValidatedForcedDecision(optimizelyDecisionContext, projectConfig, user); decisionReasons += forcedDecisionVariation.DecisionReasons; - + if (forcedDecisionVariation.ResultObject != null) { flagDecisions.Add(key, new FeatureDecision(null, @@ -1040,7 +1032,7 @@ ProjectConfig projectConfig decisionSource = flagDecision.Source; } - var includeReasons= allOptions.Contains(OptimizelyDecideOption.INCLUDE_REASONS); + var includeReasons = allOptions.Contains(OptimizelyDecideOption.INCLUDE_REASONS); var reasonsToReport = decisionReasons.ToReport(includeReasons).ToArray(); var variationKey = flagDecision.Variation?.Key; // TODO: add ruleKey values when available later. use a copy of experimentKey until then. @@ -1071,8 +1063,8 @@ ProjectConfig projectConfig { "reasons", reasonsToReport }, { "decisionEventDispatched", decisionEventDispatched }, }; - - NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, + + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Decision, DecisionNotificationTypes.FLAG, userId, user.GetAttributes(), decisionInfo); return new OptimizelyDecision( @@ -1114,7 +1106,7 @@ private Result> GetDecisionVariableMap(FeatureFlag fl valuesMap[variable.Key] = convertedValue; } - + return Result>.NewResult(valuesMap, reasons); } From b439dcbf34e208643b4b21f15c18f833ada050ad Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Oct 2024 09:55:32 -0400 Subject: [PATCH 21/28] ci: bump action versions --- .github/workflows/csharp.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 7228976e..6c746648 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -7,19 +7,19 @@ on: branches: [ master ] types: [ opened, synchronize, reopened, ready_for_review ] -jobs: +jobs: lintCodebase: name: Lint Codebase if Not Draft if: github.event.pull_request.draft == false runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: # Full git history is needed to get a proper list of changed files fetch-depth: 0 - name: Run Super-Linter - uses: github/super-linter@v4 + uses: github/super-linter@v7.1.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} VALIDATE_ALL_CODEBASE: false @@ -39,7 +39,7 @@ jobs: CURRENT_BRANCH: ${{ github.head_ref || github.ref_name }} steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v1 - name: Setup NuGet @@ -67,7 +67,7 @@ jobs: CURRENT_BRANCH: ${{ github.head_ref || github.ref_name }} steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup .NET uses: actions/setup-dotnet@v2 with: @@ -90,7 +90,7 @@ jobs: CURRENT_BRANCH: ${{ github.head_ref || github.ref_name }} steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup .NET uses: actions/setup-dotnet@v2 with: From 5225f7eb5e373f40b104ec4f078b6d9b6a497faf Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Oct 2024 09:55:49 -0400 Subject: [PATCH 22/28] chore: csproj auto-edit --- OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj index 329b8b72..9eb81c62 100644 --- a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj +++ b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj @@ -160,4 +160,7 @@ + + + From 62022c3dd2851675cdcc728702dd5f0f2503342a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Oct 2024 09:58:15 -0400 Subject: [PATCH 23/28] fix: super-linter version --- .github/workflows/csharp.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 6c746648..b55407af 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -19,7 +19,7 @@ jobs: # Full git history is needed to get a proper list of changed files fetch-depth: 0 - name: Run Super-Linter - uses: github/super-linter@v7.1.0 + uses: github/super-linter@v7 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} VALIDATE_ALL_CODEBASE: false From 81aae4ddd4b686eb9823345c8451f3f561d11b72 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Oct 2024 10:44:28 -0400 Subject: [PATCH 24/28] fix: add missing refs to UserProfileTracker.cs --- OptimizelySDK.Net35/OptimizelySDK.Net35.csproj | 5 ++++- OptimizelySDK.Net40/OptimizelySDK.Net40.csproj | 5 ++++- .../OptimizelySDK.NetStandard16.csproj | 1 + .../OptimizelySDK.NetStandard20.csproj | 3 +++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj b/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj index 8daf74ce..a4495471 100644 --- a/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj +++ b/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj @@ -209,6 +209,9 @@ Bucketing\UserProfile.cs + + Bucketing\UserProfileTracker.cs + Bucketing\ExperimentUtils @@ -356,4 +359,4 @@ --> - \ No newline at end of file + diff --git a/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj b/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj index a0f98b77..05785575 100644 --- a/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj +++ b/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj @@ -208,6 +208,9 @@ Bucketing\UserProfile.cs + + Bucketing\UserProfileTracker.cs + Bucketing\ExperimentUtils @@ -366,4 +369,4 @@ - \ No newline at end of file + diff --git a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj index 9eb81c62..0c327dc4 100644 --- a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj +++ b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj @@ -72,6 +72,7 @@ + diff --git a/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj b/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj index 6d7ea638..b7114653 100644 --- a/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj +++ b/OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj @@ -133,6 +133,9 @@ Bucketing\UserProfile.cs + + Bucketing\UserProfileTracker.cs + Bucketing\UserProfileService.cs From eced9ed0b20d23e63596224ac21230aafe39df72 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Wed, 23 Oct 2024 12:25:18 -0400 Subject: [PATCH 25/28] revert: accidental csproj references --- OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj index 0c327dc4..b17f79e7 100644 --- a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj +++ b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj @@ -161,7 +161,4 @@ - - - From 60bcddf0e79c443245ddac73b0165ec0486da552 Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 24 Oct 2024 14:14:44 -0400 Subject: [PATCH 26/28] ci: lots of updates to how we're releasing --- .github/workflows/csharp_release.yml | 145 +++++++++++++++------------ 1 file changed, 81 insertions(+), 64 deletions(-) diff --git a/.github/workflows/csharp_release.yml b/.github/workflows/csharp_release.yml index 8f989fc6..369b1915 100644 --- a/.github/workflows/csharp_release.yml +++ b/.github/workflows/csharp_release.yml @@ -9,26 +9,23 @@ jobs: name: Set Variables runs-on: ubuntu-latest env: - # ⚠️ IMPORTANT: tag should always start with integer & will be used verbatim to string end TAG: ${{ github.event.release.tag_name }} steps: - - name: Set semantic version variable + - name: Extract semantic version from tag id: set_version run: | - SEMANTIC_VERSION=$(echo "$TAG" | grep -Po "(?<=^|[^0-9])([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?(-[a-zA-Z]+[0-9]*)?)") + # Remove the "v" prefix if it exists and extract the semantic version number + SEMANTIC_VERSION=$(echo "${TAG}" | grep -Po "(?<=^|[^0-9])([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?(-[a-zA-Z]+[0-9]*)?)") + SEMANTIC_VERSION=${SEMANTIC_VERSION#"v"} if [ -z "${SEMANTIC_VERSION}" ]; then - echo "Tag did not start with a semantic version number (e.g., #.#.#; #.#.#.#; #.#.#.#-beta)" + echo "Error: Tag '${TAG}' does not start with a valid semantic version number (e.g., #.#.#; #.#.#.#; #.#.#.#-beta)" exit 1 fi + echo "Extracted semantic version: ${SEMANTIC_VERSION}" echo "semantic_version=${SEMANTIC_VERSION}" >> $GITHUB_OUTPUT - - name: Output tag & semantic version - id: outputs - run: | - echo "$TAG" - echo ${{ steps.set_version.outputs.semantic_version }} outputs: tag: $TAG - semanticVersion: ${{ steps.set_version.outputs.semantic_version }} + semanticVersion: ${{ steps.set_version.outputs.semantic_version }} buildFrameworkVersions: name: Build Framework versions @@ -48,9 +45,9 @@ jobs: - name: Build and strongly name assemblies run: msbuild /p:SignAssembly=true /p:AssemblyOriginatorKeyFile=$(pwd)/keypair.snk /p:Configuration=Release ./OptimizelySDK.NETFramework.sln - name: Upload Framework artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: - name: nuget-files + name: unsigned-dlls if-no-files-found: error path: ./**/bin/Release/**/Optimizely*.dll @@ -70,9 +67,9 @@ jobs: - name: Build and strongly name assemblies run: dotnet build OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj /p:SignAssembly=true /p:AssemblyOriginatorKeyFile=$(pwd)/keypair.snk -c Release - name: Upload Standard 1.6 artifact - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: - name: nuget-files + name: unsigned-dlls if-no-files-found: error path: ./**/bin/Release/**/Optimizely*.dll @@ -92,21 +89,69 @@ jobs: - name: Build and strongly name Standard 2.0 project run: dotnet build OptimizelySDK.NetStandard20/OptimizelySDK.NetStandard20.csproj /p:SignAssembly=true /p:AssemblyOriginatorKeyFile=$(pwd)/keypair.snk -c Release - name: Build and strongly name assemblies - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: - name: nuget-files + name: unsigned-dlls if-no-files-found: error path: ./**/bin/Release/**/Optimizely*.dll - pack: - name: Sign & pack NuGet package + sign: + name: Send DLLs for signing needs: [ variables, buildFrameworkVersions, buildStandard16, buildStandard20 ] runs-on: ubuntu-latest + env: + # TODO: Replace actual values + SIGNING_SERVER_PRIVATE_KEY: ${{ secrets.SIGNING_SERVER_PRIVATE_KEY }} + SIGNING_SERVER_HOST: ${{ secrets.SIGNING_SERVER_HOST }} + SIGNING_SERVER_UPLOAD_PATH: /path/to/UPLOAD/directory + SIGNING_SERVER_DOWNLOAD_PATH: /path/to/DOWNLOAD/directory + steps: + # TODO: Remove this when we're ready to automate + - name: Temporarily halt progress + run: exit 1 + - name: Download the unsigned files + uses: actions/download-artifact@v4 + with: + name: unsigned-dlls + path: ./unsigned-dlls + - name: Setup SSH + uses: shimataro/ssh-key-action@v2 + with: + key: $SIGNING_SERVER_PRIVATE_KEY + - name: Send files to signing server + run: scp -r ./unsigned-dlls $SIGNING_SERVER_HOST:$SIGNING_SERVER_UPLOAD_PATH + - name: Wait for artifact to be published + run: | + for i in {1..60}; do + # Replace with actual path + if ssh $SIGNING_SERVER_HOST "ls $SIGNING_SERVER_DOWNLOAD_PATH"; then + exit 0 + fi + sleep 10 + done + exit 1 + - name: Download signed files + run: | + mkdir ./signed-dlls + scp -r $SIGNING_SERVER_HOST:$SIGNING_SERVER_DOWNLOAD_PATH ./signed-dlls + - name: Delete signed files from server + run: ssh $SIGNING_SERVER_HOST "rm -rf $SIGNING_SERVER_DOWNLOAD_PATH/*" + - name: Upload signed files + uses: actions/upload-artifact@v4 + with: + name: signed-dlls + if-no-files-found: error + path: ./signed-dlls + + pack: + name: Pack NuGet package + needs: [ variables, sign ] + runs-on: ubuntu-latest env: VERSION: ${{ needs.variables.outputs.semanticVersion }} steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ needs.variables.outputs.tag }} - name: Install mono @@ -114,55 +159,25 @@ jobs: sudo apt update sudo apt install -y mono-devel - name: Download NuGet files - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v4 with: - name: nuget-files - path: ./nuget-files + name: signed-dlls + path: ./signed-dlls - name: Organize files run: | - pushd ./nuget-files + pushd ./signed-files # Move all dlls to the root directory - find . -type f -name "*.dll" -exec mv {} . \; + find . -type f -name "*.dll" -exec mv {} . popd # Create directories mkdir -p nuget/lib/net35/ nuget/lib/net40/ nuget/lib/net45/ nuget/lib/netstandard1.6/ nuget/lib/netstandard2.0/ pushd ./nuget # Move files to directories - mv ../nuget-files/OptimizelySDK.Net35.dll lib/net35/ - mv ../nuget-files/OptimizelySDK.Net40.dll lib/net40/ - mv ../nuget-files/OptimizelySDK.dll lib/net45/ - mv ../nuget-files/OptimizelySDK.NetStandard16.dll lib/netstandard1.6/ - mv ../nuget-files/OptimizelySDK.NetStandard20.dll lib/netstandard2.0/ - popd - - name: Setup signing prerequisites - env: - CERTIFICATE_P12: ${{ secrets.CERTIFICATE_P12 }} - CERTIFICATE_PASSWORD: ${{ secrets.CERTIFICATE_PASSWORD }} - run: | - pushd ./nuget - echo $CERTIFICATE_P12 | base64 --decode > authenticode.pfx - openssl pkcs12 -in authenticode.pfx -nocerts -nodes -legacy -out key.pem -password env:CERTIFICATE_PASSWORD - openssl rsa -in key.pem -outform PVK -pvk-none -out authenticode.pvk - openssl pkcs12 -in authenticode.pfx -nokeys -nodes -legacy -out cert.pem -password env:CERTIFICATE_PASSWORD - openssl crl2pkcs7 -nocrl -certfile cert.pem -outform DER -out authenticode.spc - popd - - name: Sign the DLLs - run: | - pushd ./nuget - find . -type f -name "*.dll" -print0 | while IFS= read -r -d '' file; do - echo "Signing ${file}" - signcode \ - -spc ./authenticode.spc \ - -v ./authenticode.pvk \ - -a sha1 -$ commercial \ - -n "Optimizely, Inc" \ - -i "https://www.optimizely.com/" \ - -t "http://timestamp.digicert.com" \ - -tr 10 \ - ${file} - rm ${file}.bak - done - rm *.spc *.pem *.pvk *.pfx + mv ../signed-dlls/OptimizelySDK.Net35.dll lib/net35/ + mv ../signed-dlls/OptimizelySDK.Net40.dll lib/net40/ + mv ../signed-dlls/OptimizelySDK.dll lib/net45/ + mv ../signed-dlls/OptimizelySDK.NetStandard16.dll lib/netstandard1.6/ + mv ../signed-dlls/OptimizelySDK.NetStandard20.dll lib/netstandard2.0/ popd - name: Create nuspec # Uses env.VERSION in OptimizelySDK.nuspec.template @@ -175,27 +190,29 @@ jobs: nuget pack OptimizelySDK.nuspec popd - name: Upload nupkg artifact - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: nuget-package if-no-files-found: error path: ./nuget/Optimizely.SDK.${{ env.VERSION }}.nupkg publish: - name: Publish package to NuGet + name: Publish package to NuGet after reviewing the artifact needs: [ variables, pack ] runs-on: ubuntu-latest + # Review the `nuget-package` artifact ensuring the dlls are + # organized and signed before approving. + environment: 'i-reviewed-nuget-package-artifact' env: VERSION: ${{ needs.variables.outputs.semanticVersion }} steps: - name: Download NuGet files - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v4 with: name: nuget-package path: ./nuget - name: Setup .NET - uses: actions/setup-dotnet@v3 + uses: actions/setup-dotnet@v4 - name: Publish NuGet package - # Unset secrets.NUGET_API_KEY to simulate dry run run: | dotnet nuget push ./nuget/Optimizely.SDK.${{ env.VERSION }}.nupkg --source https://api.nuget.org/v3/index.json --api-key ${{ secrets.NUGET_API_KEY }} From 3dc6f0a7480c1d21b4056da47e6b066801250e8e Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Thu, 24 Oct 2024 14:46:21 -0400 Subject: [PATCH 27/28] fix: lint & dir name --- .github/workflows/csharp_release.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/csharp_release.yml b/.github/workflows/csharp_release.yml index 369b1915..cd80b0b2 100644 --- a/.github/workflows/csharp_release.yml +++ b/.github/workflows/csharp_release.yml @@ -25,7 +25,7 @@ jobs: echo "semantic_version=${SEMANTIC_VERSION}" >> $GITHUB_OUTPUT outputs: tag: $TAG - semanticVersion: ${{ steps.set_version.outputs.semantic_version }} + semanticVersion: ${{ steps.set_version.outputs.semantic_version }} buildFrameworkVersions: name: Build Framework versions @@ -99,7 +99,7 @@ jobs: name: Send DLLs for signing needs: [ variables, buildFrameworkVersions, buildStandard16, buildStandard20 ] runs-on: ubuntu-latest - env: + env: # TODO: Replace actual values SIGNING_SERVER_PRIVATE_KEY: ${{ secrets.SIGNING_SERVER_PRIVATE_KEY }} SIGNING_SERVER_HOST: ${{ secrets.SIGNING_SERVER_HOST }} @@ -165,7 +165,7 @@ jobs: path: ./signed-dlls - name: Organize files run: | - pushd ./signed-files + pushd ./signed-dlls # Move all dlls to the root directory find . -type f -name "*.dll" -exec mv {} . popd @@ -202,7 +202,7 @@ jobs: runs-on: ubuntu-latest # Review the `nuget-package` artifact ensuring the dlls are # organized and signed before approving. - environment: 'i-reviewed-nuget-package-artifact' + environment: 'i-reviewed-nuget-package-artifact' env: VERSION: ${{ needs.variables.outputs.semanticVersion }} steps: From a6abb608d78a963062a9d31a272271af2d7e814a Mon Sep 17 00:00:00 2001 From: Mike Chu Date: Tue, 5 Nov 2024 17:32:48 -0500 Subject: [PATCH 28/28] revert: modifiers on DecisionService & test changes --- OptimizelySDK.Tests/DecisionServiceTest.cs | 6 +++++- OptimizelySDK/Bucketing/DecisionService.cs | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index d1bf15af..8fbedf23 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -160,8 +160,12 @@ public void TestGetVariationLogsErrorWhenUserProfileMapItsNull() var variationResult = decisionService.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, options); Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[0], - "Audiences for experiment \"etag3\" collectively evaluated to FALSE"); + "We were unable to get a user profile map from the UserProfileService."); Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[1], + "No previously activated variation of experiment \"etag3\" for user \"genericUserId\" found in user profile."); + Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[2], + "Audiences for experiment \"etag3\" collectively evaluated to FALSE"); + Assert.AreEqual(variationResult.DecisionReasons.ToReport(true)[3], "User \"genericUserId\" does not meet conditions to be in experiment \"etag3\"."); } diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index f001da69..1e364b29 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -41,9 +41,9 @@ public class DecisionService public const string LOGGING_KEY_TYPE_RULE = "rule"; private Bucketer Bucketer; - private readonly IErrorHandler ErrorHandler; - private static UserProfileService UserProfileService; - private readonly ILogger Logger; + private IErrorHandler ErrorHandler; + private UserProfileService UserProfileService; + private ILogger Logger; /// /// Associative array of user IDs to an associative array