Fixed JSON tests for existing and sticky bucket

growthbook · Dec 16, 2024 · 0abda2d · 0abda2d
1 parent 8018c77
commit 0abda2d
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 30 deletions.
diff --git a/GrowthBook.Tests/StandardTests/GrowthBookTests/StickyBucketTests.cs b/GrowthBook.Tests/StandardTests/GrowthBookTests/StickyBucketTests.cs
@@ -3,6 +3,9 @@
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
+using FluentAssertions;
+using GrowthBook.Services;
+using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using Xunit;
 
@@ -24,14 +27,47 @@ public class StickyBucketTestCase
         [TestPropertyIndex(4)]
         public JToken ExpectedResult { get; set; }
         [TestPropertyIndex(5)]
-        public StickyAssignmentsDocument[] ExpectedAssignmentDocs { get; set; } = [];
+        public Dictionary<string, StickyAssignmentsDocument> ExpectedAssignmentDocs { get; set; } = [];
     }
 
     [Theory]
     [MemberData(nameof(GetMappedTestsInCategory), typeof(StickyBucketTestCase))]
     public void Run(StickyBucketTestCase testCase)
     {
+        var service = new InMemoryStickyBucketService();
+
+        testCase.Context.StickyBucketService = service;
+        testCase.Context.StickyBucketAssignmentDocs = testCase.PreExistingAssignmentDocs.ToDictionary(x => x.FormattedAttribute);
+
+        // NOTE: Existing sticky bucket JSON tests in the JS SDK load this into the service up front
+        //       but I wonder if that's correct because without that any assignment doc that exists
+        //       other than those will not be stored and some of these test cases will fail.
+
+        foreach (var document in testCase.PreExistingAssignmentDocs)
+        {
+            service.SaveAssignments(document);
+        }
+
         var gb = new GrowthBook(testCase.Context);
-#warning Complete this.
+
+        var result = gb.EvalFeature(testCase.FeatureName);
+
+        var actualResult = JToken.Parse(JsonConvert.SerializeObject(result.ExperimentResult));
+
+        if (testCase.ExpectedResult is JObject obj)
+        {
+            foreach (var property in obj.Properties())
+            {
+                actualResult[property.Name].ToString().Should().Be(property.Value.ToString());
+            }
+        }
+        else
+        {
+            actualResult.ToString().Should().Be(testCase.ExpectedResult.ToString());
+        }
+
+        var storedDocuments = service.GetAllAssignments(testCase.ExpectedAssignmentDocs.Keys);
+
+        storedDocuments.Should().BeEquivalentTo(testCase.ExpectedAssignmentDocs, "because those should have been stored correctly");
     }
 }
diff --git a/GrowthBook/GrowthBook.cs b/GrowthBook/GrowthBook.cs
@@ -486,7 +486,7 @@ private ExperimentResult RunExperiment(Experiment experiment, string featureId)
 
                 if (hasFallback)
                 {
-                    (_, hashValue) = Attributes.GetHashAttributeAndValue(experiment.FallbackAttribute);
+                    (hashAttribute, hashValue) = Attributes.GetHashAttributeAndValue(experiment.FallbackAttribute);
                 }
                 else
                 {
@@ -548,24 +548,24 @@ private ExperimentResult RunExperiment(Experiment experiment, string featureId)
                         _logger.LogDebug("Aborting experiment, associated conditions have prohibited participation");
                         return GetExperimentResult(experiment, featureId: featureId);
                     }
+                }
 
-                    if (experiment.ParentConditions != null)
+                if (experiment.ParentConditions != null)
+                {
+                    foreach (var parentCondition in experiment.ParentConditions)
                     {
-                        foreach (var parentCondition in experiment.ParentConditions)
-                        {
-                            var parentResult = EvaluateFeature(featureId);
+                        var parentResult = EvaluateFeature(parentCondition.Id);
 
-                            if (parentResult.Source == FeatureResult.SourceId.CyclicPrerequisite)
-                            {
-                                return GetExperimentResult(experiment, featureId: featureId);
-                            }
+                        if (parentResult.Source == FeatureResult.SourceId.CyclicPrerequisite)
+                        {
+                            return GetExperimentResult(experiment, featureId: featureId);
+                        }
 
-                            var evaluationObject = new JObject { ["value"] = parentResult.Value };
+                        var evaluationObject = new JObject { ["value"] = parentResult.Value };
 
-                            if (!_conditionEvaluator.EvalCondition(evaluationObject, parentCondition.Condition ?? new JObject(), _savedGroups))
-                            {
-                                return GetExperimentResult(experiment, featureId: featureId);
-                            }
+                        if (!_conditionEvaluator.EvalCondition(evaluationObject, parentCondition.Condition ?? new JObject(), _savedGroups))
+                        {
+                            return GetExperimentResult(experiment, featureId: featureId);
                         }
                     }
                 }
@@ -753,7 +753,11 @@ private ExperimentResult GetExperimentResult(Experiment experiment, int variatio
                 HashUsed = hashUsed,
                 HashValue = hashValue,
                 Value = experiment.Variations is null ? null : experiment.Variations[variationIndex],
-                VariationId = variationIndex
+                VariationId = variationIndex,
+                Name = meta?.Name,
+                Passthrough = meta?.Passthrough ?? false,
+                Bucket = bucketHash ?? 0d,
+                StickyBucketUsed = wasStickyBucketUsed
             };
 
             result.Name = meta?.Name;

diff --git a/GrowthBook/Services/IStickyBucketService.cs b/GrowthBook/Services/IStickyBucketService.cs
@@ -8,6 +8,6 @@ public interface IStickyBucketService
     {
         StickyAssignmentsDocument GetAssignments(string attributeName, string attributeValue);
         void SaveAssignments(StickyAssignmentsDocument document);
-        IDictionary<string, StickyAssignmentsDocument> GetAllAssignments(IDictionary<string, string> attributes);
+        IDictionary<string, StickyAssignmentsDocument> GetAllAssignments(IEnumerable<string> attributes);
     }
 }
diff --git a/GrowthBook/Services/InMemoryStickyBucketService.cs b/GrowthBook/Services/InMemoryStickyBucketService.cs
@@ -9,10 +9,10 @@ public class InMemoryStickyBucketService : IStickyBucketService
     {
         private readonly IDictionary<string, StickyAssignmentsDocument> _cachedDocuments = new Dictionary<string, StickyAssignmentsDocument>();
 
-        public IDictionary<string, StickyAssignmentsDocument> GetAllAssignments(IDictionary<string, string> attributes)
+        public IDictionary<string, StickyAssignmentsDocument> GetAllAssignments(IEnumerable<string> attributes)
         {
-            var assignments = from pair in attributes
-                              let existingDoc = _cachedDocuments.TryGetValue(pair.Key, out var doc) ? doc : null
+            var assignments = from name in attributes
+                              let existingDoc = _cachedDocuments.TryGetValue(name, out var doc) ? doc : null
                               where existingDoc != null
                               select (Attribute: existingDoc.FormattedAttribute, Document: existingDoc);
 

diff --git a/GrowthBook/StickyAssignmentsDocument.cs b/GrowthBook/StickyAssignmentsDocument.cs
@@ -12,15 +12,15 @@ public class StickyAssignmentsDocument
 
         public string AttributeName { get; set; }
         public string AttributeValue { get; set; }
-        public IDictionary<string, string> StickyAssignments { get; set; }
+        public IDictionary<string, string> Assignments { get; set; } = new Dictionary<string, string>();
 
         public StickyAssignmentsDocument() { }
 
         public StickyAssignmentsDocument(string attributeName, string attributeValue, IDictionary<string, string> stickyAssignments = default)
         {
             AttributeName = attributeName;
             AttributeValue = attributeValue;
-            StickyAssignments = stickyAssignments ?? new Dictionary<string, string>();
+            Assignments = stickyAssignments ?? new Dictionary<string, string>();
         }
     }
 }
diff --git a/GrowthBook/Utilities/ExperimentUtilities.cs b/GrowthBook/Utilities/ExperimentUtilities.cs
@@ -253,15 +253,14 @@ private static Regex GetUrlRegex(UrlPattern pattern)
         public static (StickyAssignmentsDocument Document, bool IsChanged) GenerateStickyBucketAssignment(IStickyBucketService stickyBucketService, string attributeName, string attributeValue, IDictionary<string, string> assignments)
         {
             var existingDocument = stickyBucketService is null ? new StickyAssignmentsDocument(attributeName, attributeValue) : stickyBucketService.GetAssignments(attributeName, attributeValue);
-            var newAssignments = new Dictionary<string, string>(existingDocument.StickyAssignments);
+            var newAssignments = new Dictionary<string, string>(existingDocument?.Assignments ?? new Dictionary<string, string>());
 
             newAssignments.MergeWith(new[] { assignments });
 
             var isChanged = JsonConvert.SerializeObject(existingDocument) != JsonConvert.SerializeObject(newAssignments);
+            var document = new StickyAssignmentsDocument(attributeName, attributeValue, newAssignments);
 
-            existingDocument.StickyAssignments = newAssignments;
-
-            return (existingDocument, isChanged);
+            return (document, isChanged);
         }
 
         public static StickyBucketVariation GetStickyBucketVariation(Experiment experiment, int bucketVersion, int minBucketVersion, IList<VariationMeta> meta, JObject attributes, IDictionary<string, StickyAssignmentsDocument> document)
@@ -304,7 +303,7 @@ private static IDictionary<string, string> GetStickyBucketAssignments(JObject at
             (var hashAttributeWithoutFallback, var hashValueWithoutFallback) = attributes.GetHashAttributeAndValue(hashAttribute, default);
             var hashKey = new StickyAssignmentsDocument(hashAttributeWithoutFallback, hashValueWithoutFallback);
 
-            (var hashAttributeWithFallback, var hashValueWithFallback) = attributes.GetHashAttributeAndValue(default, fallbackAttribute);
+            (var hashAttributeWithFallback, var hashValueWithFallback) = attributes.GetHashAttributeAndValue(fallbackAttribute, default);
             var fallbackKey = new StickyAssignmentsDocument(hashAttributeWithFallback, hashValueWithFallback);
 
             var pendingAssignments = new List<IDictionary<string, string>>();
@@ -313,12 +312,12 @@ private static IDictionary<string, string> GetStickyBucketAssignments(JObject at
 
             if (fallbackKey.HasValue && stickyAssignmentDocs.TryGetValue(fallbackKey.FormattedAttribute, out var fallbackDocument))
             {
-                pendingAssignments.Add(fallbackDocument.StickyAssignments);
+                pendingAssignments.Add(fallbackDocument.Assignments);
             }
 
             if (stickyAssignmentDocs.TryGetValue(hashKey.FormattedAttribute, out var document))
             {
-                pendingAssignments.Add(document.StickyAssignments);
+                pendingAssignments.Add(document.Assignments);
             }
 
             return mergedAssignments.MergeWith(pendingAssignments);