From 513e36b1ea9fbca25d4bc6a15662e7c96fa7a314 Mon Sep 17 00:00:00 2001 From: Chris Hannon Date: Sat, 30 Mar 2024 16:00:44 -0600 Subject: [PATCH] Fixed null reference issue, inherited IDisposable, and added ExperimentAssignment tracking to EvalFeature --- CHANGELOG.md | 6 ++ .../CustomTests/RegressionTests.cs | 61 +++++++++++++++++++ GrowthBook/GrowthBook.cs | 45 ++++++++------ GrowthBook/GrowthBook.csproj | 2 +- GrowthBook/IGrowthBook.cs | 2 +- 5 files changed, 95 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 299b04f..c35cf0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.0.3] + +- Fixed null reference exception when forcing a rule with a tracking callback set and null tracking data. +- Experiment assignments are included in EvalFeature as well as Run. +- IGrowthBook interface fixed to inherit IDisposable. + ## [1.0.2] - Fixed issue with incorrect logic in GrowthBook.GetFeatureResult() call. diff --git a/GrowthBook.Tests/CustomTests/RegressionTests.cs b/GrowthBook.Tests/CustomTests/RegressionTests.cs index 79f62e9..9b48a89 100644 --- a/GrowthBook.Tests/CustomTests/RegressionTests.cs +++ b/GrowthBook.Tests/CustomTests/RegressionTests.cs @@ -4,13 +4,74 @@ using System.Text; using System.Threading.Tasks; using FluentAssertions; +using Microsoft.AspNetCore.Mvc; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using Xunit; namespace GrowthBook.Tests.CustomTests; public class RegressionTests : UnitTest { + [Fact] + public void ExperimentWillResultInAssignmentIfDifferentFromPreviousOrMissing() + { + const string FeatureName = "test-assignment"; + + var feature = new Feature { DefaultValue = false, Rules = new List { new() { Coverage = 1d } } }; + + var staticFeatures = new Dictionary + { + [FeatureName] = feature + }; + + var context = new Context + { + Features = staticFeatures + }; + + var growthBook = new GrowthBook(context); + + var result = growthBook.EvalFeature(FeatureName); + var result2 = growthBook.EvalFeature(FeatureName); + + var assignmentsByExperimentKey = growthBook.GetAllResults(); + + assignmentsByExperimentKey.Should().NotBeNull("because it must always be a valid collection instance"); + assignmentsByExperimentKey.Count.Should().Be(1, "because the second experiment was not different"); + } + + [Fact] + public void GetFeatureValueShouldReturnForcedValueEvenWhenTracksIsNull() + { + const string FeatureName = "test-tracks-default-value"; + var forcedResult = JToken.FromObject(true); + + var feature = new Feature { DefaultValue = false, Rules = new List { new() { Force = forcedResult } } }; + + var staticFeatures = new Dictionary + { + [FeatureName] = feature + }; + + var trackingWasCalled = false; + + var context = new Context + { + Features = staticFeatures, + TrackingCallback = (features, tracking) => trackingWasCalled = true + }; + + var growthBook = new GrowthBook(context); + + var result = growthBook.EvalFeature(FeatureName); + + result.Should().NotBeNull("because a result must be created"); + result.Value.Should().NotBeNull("because the result value was forced"); + result.Value.ToString().Should().Be(forcedResult.ToString(), "because that was the forced value"); + trackingWasCalled.Should().BeFalse("because no tracking data was present"); + } + [Fact] public void GetFeatureValueShouldOnlyReturnFallbackValueWhenTheFeatureResultValueIsNull() { diff --git a/GrowthBook/GrowthBook.cs b/GrowthBook/GrowthBook.cs index d988b50..e4bf7a4 100644 --- a/GrowthBook/GrowthBook.cs +++ b/GrowthBook/GrowthBook.cs @@ -215,7 +215,7 @@ public FeatureResult EvalFeature(string featureId) continue; } - if (_trackingCallback != null && rule.Tracks.Any()) + if (_trackingCallback != null && rule.Tracks?.Any() == true) { foreach (var trackData in rule.Tracks) { @@ -251,6 +251,8 @@ public FeatureResult EvalFeature(string featureId) var result = RunExperiment(experiment, featureId); + TryAssignExperimentResult(experiment, result); + if (!result.InExperiment || result.Passthrough) { continue; @@ -281,24 +283,7 @@ public ExperimentResult Run(Experiment experiment) { ExperimentResult result = RunExperiment(experiment, null); - if (!_assigned.TryGetValue(experiment.Key, out ExperimentAssignment prev) - || prev.Result.InExperiment != result.InExperiment - || prev.Result.VariationId != result.VariationId) - { - _assigned.Add(experiment.Key, new ExperimentAssignment { Experiment = experiment, Result = result }); - - foreach (Action callback in _subscriptions) - { - try - { - callback?.Invoke(experiment, result); - } - catch (Exception ex) - { - _logger.LogError(ex, $"Encountered exception during subscription callback for experiment with key '{experiment.Key}'"); - } - } - } + TryAssignExperimentResult(experiment, result); return result; } @@ -327,6 +312,28 @@ public async Task LoadFeatures(GrowthBookRetrievalOptions options = null, Cancel } } + private void TryAssignExperimentResult(Experiment experiment, ExperimentResult result) + { + if (!_assigned.TryGetValue(experiment.Key, out ExperimentAssignment prev) + || prev.Result.InExperiment != result.InExperiment + || prev.Result.VariationId != result.VariationId) + { + _assigned.Add(experiment.Key, new ExperimentAssignment { Experiment = experiment, Result = result }); + + foreach (Action callback in _subscriptions) + { + try + { + callback?.Invoke(experiment, result); + } + catch (Exception ex) + { + _logger.LogError(ex, $"Encountered exception during subscription callback for experiment with key '{experiment.Key}'"); + } + } + } + } + private ExperimentResult RunExperiment(Experiment experiment, string featureId) { // 1. Abort if there aren't enough variations present. diff --git a/GrowthBook/GrowthBook.csproj b/GrowthBook/GrowthBook.csproj index b100ab4..ec2655d 100644 --- a/GrowthBook/GrowthBook.csproj +++ b/GrowthBook/GrowthBook.csproj @@ -11,7 +11,7 @@ https://github.com/growthbook/growthbook-csharp.git git GrowthBook,A/B test,feature toggle,flag - 1.0.2 + 1.0.3 GrowthBook C# SDK https://www.growthbook.io/ diff --git a/GrowthBook/IGrowthBook.cs b/GrowthBook/IGrowthBook.cs index ebb5b34..90ad0f5 100644 --- a/GrowthBook/IGrowthBook.cs +++ b/GrowthBook/IGrowthBook.cs @@ -8,7 +8,7 @@ namespace GrowthBook /// /// Providing operations to interact with feature flags. /// - public interface IGrowthBook + public interface IGrowthBook : IDisposable { /// /// Checks to see if a feature is on.