Skip to content

Commit

Permalink
Fixed null reference issue, inherited IDisposable, and added Experime…
Browse files Browse the repository at this point in the history
…ntAssignment tracking to EvalFeature
  • Loading branch information
Norhaven committed Mar 30, 2024
1 parent ea1f340 commit 513e36b
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 21 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>() call.
Expand Down
61 changes: 61 additions & 0 deletions GrowthBook.Tests/CustomTests/RegressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<FeatureRule> { new() { Coverage = 1d } } };

var staticFeatures = new Dictionary<string, Feature>
{
[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<FeatureRule> { new() { Force = forcedResult } } };

var staticFeatures = new Dictionary<string, Feature>
{
[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()
{
Expand Down
45 changes: 26 additions & 19 deletions GrowthBook/GrowthBook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -251,6 +251,8 @@ public FeatureResult EvalFeature(string featureId)

var result = RunExperiment(experiment, featureId);

TryAssignExperimentResult(experiment, result);

if (!result.InExperiment || result.Passthrough)
{
continue;
Expand Down Expand Up @@ -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<Experiment, ExperimentResult> 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;
}
Expand Down Expand Up @@ -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<Experiment, ExperimentResult> 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.
Expand Down
2 changes: 1 addition & 1 deletion GrowthBook/GrowthBook.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<RepositoryUrl>https://github.com/growthbook/growthbook-csharp.git</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<PackageTags>GrowthBook,A/B test,feature toggle,flag</PackageTags>
<Version>1.0.2</Version>
<Version>1.0.3</Version>
<Title>GrowthBook C# SDK</Title>
<PackageProjectUrl>https://www.growthbook.io/</PackageProjectUrl>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion GrowthBook/IGrowthBook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace GrowthBook
/// <summary>
/// Providing operations to interact with feature flags.
/// </summary>
public interface IGrowthBook
public interface IGrowthBook : IDisposable
{
/// <summary>
/// Checks to see if a feature is on.
Expand Down

0 comments on commit 513e36b

Please sign in to comment.