diff --git a/.gitmodules b/.gitmodules index 0538c45..ac00ee5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "Flagsmith.EngineTest/EngineTestData"] path = Flagsmith.EngineTest/EngineTestData url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.10.0 + branch = v3.11.0 diff --git a/Flagsmith.Client.Test/Fixtures.cs b/Flagsmith.Client.Test/Fixtures.cs index e7f4f05..7756c62 100644 --- a/Flagsmith.Client.Test/Fixtures.cs +++ b/Flagsmith.Client.Test/Fixtures.cs @@ -82,6 +82,7 @@ internal class Fixtures 'id': 100, 'multivariate_feature_option': { 'id': 10, + 'key': 'variant_a_key', 'value': 'variant_a' }, 'mv_fs_value_uuid': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', diff --git a/Flagsmith.Client.Test/FlagsmithTest.cs b/Flagsmith.Client.Test/FlagsmithTest.cs index ac31c91..8a695a5 100644 --- a/Flagsmith.Client.Test/FlagsmithTest.cs +++ b/Flagsmith.Client.Test/FlagsmithTest.cs @@ -245,6 +245,92 @@ public async Task TestGetIdentityFlagsUsesLocalIdentityOverridesWhenAvailable() Assert.False(flag.Enabled); Assert.Equal("some-overridden-value", flag.Value); } + [Theory] + [InlineData("premium", true, "new-checkout")] + [InlineData("free", false, "old-checkout")] + public async Task TestGetIdentityFlagsServesFlagDependingOnAnotherFlagWhenLocallyEvaluated(string plan, bool expectedEnabled, string expectedValue) + { + // Given + var environmentDocument = @"{ + 'api_key': 'test_key', + 'name': 'Test Environment', + 'project': { + 'name': 'Test project', + 'organisation': {'feature_analytics': false, 'name': 'Test Org', 'id': 1, 'persist_trait_data': true, 'stop_serving_flags': false}, + 'id': 1, + 'hide_disabled_flags': false, + 'segments': [ + { + 'id': 1, + 'name': 'Premium plan', + 'rules': [{'type': 'ALL', 'rules': [], 'conditions': [{'operator': 'EQUAL', 'property_': 'plan', 'value': 'premium'}]}], + 'feature_states': [ + { + 'django_id': 3, + 'feature': {'name': 'beta_access', 'type': 'STANDARD', 'id': 1}, + 'feature_state_value': null, + 'enabled': true, + 'feature_segment': {'priority': 0}, + 'multivariate_feature_state_values': [] + } + ] + }, + { + 'id': 2, + 'name': 'Beta access', + 'rules': [{'type': 'ALL', 'rules': [], 'conditions': [{'operator': 'EQUAL', 'property_': '$.flags.beta_access.enabled', 'value': 'true'}]}], + 'feature_states': [ + { + 'django_id': 4, + 'feature': {'name': 'checkout', 'type': 'STANDARD', 'id': 2}, + 'feature_state_value': 'new-checkout', + 'enabled': true, + 'feature_segment': {'priority': 1}, + 'multivariate_feature_state_values': [] + } + ] + } + ] + }, + 'id': 1, + 'feature_states': [ + { + 'django_id': 1, + 'feature': {'name': 'beta_access', 'type': 'STANDARD', 'id': 1}, + 'feature_state_value': null, + 'enabled': false, + 'multivariate_feature_state_values': [] + }, + { + 'django_id': 2, + 'feature': {'name': 'checkout', 'type': 'STANDARD', 'id': 2}, + 'feature_state_value': 'old-checkout', + 'enabled': false, + 'multivariate_feature_state_values': [] + } + ], + 'identity_overrides': [] +}"; + var mockHttpClient = HttpMocker.MockHttpResponse(new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent(environmentDocument) + }); + var flagsmithClientTest = new FlagsmithClient(new FlagsmithConfiguration + { + EnvironmentKey = Fixtures.ApiKey, + HttpClient = mockHttpClient.Object, + EnableLocalEvaluation = true + }); + + // When + var flags = await flagsmithClientTest.GetIdentityFlags("identifier", new List { new Trait("plan", plan) }); + + // Then + var flag = await flags.GetFlag("checkout"); + Assert.Equal(expectedEnabled, flag.Enabled); + Assert.Equal(expectedValue, flag.Value); + } [Fact] public async Task TestRequestConnectionErrorRaisesFlagsmithApiError() { diff --git a/Flagsmith.Client.Test/MappersTest.cs b/Flagsmith.Client.Test/MappersTest.cs index 5edf365..3fee288 100644 --- a/Flagsmith.Client.Test/MappersTest.cs +++ b/Flagsmith.Client.Test/MappersTest.cs @@ -94,11 +94,13 @@ public void MapEnvironmentDocumentToContext_ProducesEvaluationContext() Assert.Equal(2, mvFeatureWithIds.Metadata.Id); // First variant: ID=100, should have priority 100 + Assert.Equal("variant_a_key", mvFeatureWithIds.Variants[0].Key); Assert.Equal("variant_a", mvFeatureWithIds.Variants[0].Value); Assert.Equal(30.0, mvFeatureWithIds.Variants[0].Weight); Assert.Equal(100, mvFeatureWithIds.Variants[0].Priority); // Second variant: ID=200, should have priority 200 + Assert.Null(mvFeatureWithIds.Variants[1].Key); Assert.Equal("variant_b", mvFeatureWithIds.Variants[1].Value); Assert.Equal(70.0, mvFeatureWithIds.Variants[1].Weight); Assert.Equal(200, mvFeatureWithIds.Variants[1].Priority); diff --git a/Flagsmith.Engine/Engine.FlagDependencies.cs b/Flagsmith.Engine/Engine.FlagDependencies.cs new file mode 100644 index 0000000..7ca5469 --- /dev/null +++ b/Flagsmith.Engine/Engine.FlagDependencies.cs @@ -0,0 +1,219 @@ +using System.Collections.Generic; +using System.Text; +using Newtonsoft.Json.Linq; + +namespace FlagsmithEngine +{ + public partial class Engine + { + private const string FlagsPathPrefix = "$.flags"; + + /// + /// The `$.flags` mapping read by segment conditions, resolving each flag the first time a condition reads it. + /// + private sealed class FlagDependencies + { + private readonly EvaluationContext _context; + // Allocated once a condition first reads a flag, sparing evaluations without Flag Dependency + private Dictionary> _resolved; + private HashSet _cyclic; + private List _resolving; + private Dictionary _segmentMatches; + private Dictionary> _segmentKeysByFeatureName; + + public FlagDependencies(EvaluationContext context) + { + _context = context; + } + + /// + /// Whether any segment condition has read a flag. + /// + public bool Used => _resolved != null; + + /// + /// Incremented whenever a dependency cycle is cut. A segment whose evaluation increments it read a flag that could not be resolved. + /// + private int CycleHits { get; set; } + + /// + /// Flags resolved for segment conditions, mapped by feature name. Flags in a dependency cycle are never resolved. + /// + public IReadOnlyDictionary> Resolved => _resolved; + + /// + /// Names of the features found to be in a dependency cycle. + /// + public IEnumerable Cyclic => _cyclic; + + /// + /// Get the flag result for a feature, or null when the feature is absent from the context or in a dependency cycle. + /// + public FlagResult Get(string featureName) + { + if (!Used) + { + _resolved = new Dictionary>(); + _cyclic = new HashSet(); + _resolving = new List(); + _segmentMatches = new Dictionary(); + } + if (!_resolved.TryGetValue(featureName, out var flag)) + { + Resolve(featureName); + _resolved.TryGetValue(featureName, out flag); + } + return flag; + } + + private void Resolve(string featureName) + { + var resolvingIndex = _resolving.IndexOf(featureName); + if (resolvingIndex >= 0) + { + _cyclic.UnionWith(_resolving.GetRange(resolvingIndex, _resolving.Count - resolvingIndex)); + CycleHits++; + return; + } + + FeatureContext featureContext = null; + if (!(_context.Features?.TryGetValue(featureName, out featureContext) ?? false)) + return; + + _resolving.Add(featureName); + var segmentOverride = GetSegmentOverride(featureName); + _resolving.RemoveAt(_resolving.Count - 1); + + if (_cyclic.Contains(featureName)) + return; + + _resolved[featureName] = segmentOverride is null + ? GetFlagResult(_context, featureContext, "DEFAULT") + : GetFlagResult(_context, segmentOverride.FeatureContext, GetTargetingMatchReason(segmentOverride.SegmentName)); + } + + private SegmentOverride GetSegmentOverride(string featureName) + { + SegmentOverride segmentOverride = null; + + if (!GetSegmentKeysByFeatureName().TryGetValue(featureName, out var segmentKeys)) + return segmentOverride; + + foreach (var segmentKey in segmentKeys) + { + if (!MatchesSegment(segmentKey)) + continue; + var segmentContext = _context.Segments[segmentKey]; + foreach (var overrideFeatureContext in segmentContext.Overrides) + { + if (overrideFeatureContext.Name != featureName || !WinsOver(overrideFeatureContext, segmentOverride?.FeatureContext)) + continue; + segmentOverride = new SegmentOverride + { + FeatureContext = overrideFeatureContext, + SegmentName = segmentContext.Name + }; + } + } + + return segmentOverride; + } + + /// + /// Whether the context matches a segment, evaluating each segment once. Returns false when the verdict is + /// unsound, having read a flag left unresolved by a dependency cycle. + /// + public bool TryMatchSegment(string segmentKey, out bool matches) + { + if (Used && _segmentMatches.TryGetValue(segmentKey, out matches)) + return true; + + var cycleHits = CycleHits; + matches = IsContextInSegment(_context, _context.Segments[segmentKey], this); + + if (CycleHits != cycleHits) + return false; + + if (Used) + _segmentMatches[segmentKey] = matches; + return true; + } + + private bool MatchesSegment(string segmentKey) => TryMatchSegment(segmentKey, out var matches) && matches; + + private Dictionary> GetSegmentKeysByFeatureName() + { + if (_segmentKeysByFeatureName != null) + return _segmentKeysByFeatureName; + + _segmentKeysByFeatureName = new Dictionary>(); + foreach (var segmentItem in _context.Segments ?? new Dictionary>()) + { + if (segmentItem.Value.Overrides is null) + continue; + foreach (var overrideFeatureContext in segmentItem.Value.Overrides) + { + if (!_segmentKeysByFeatureName.TryGetValue(overrideFeatureContext.Name, out var segmentKeys)) + _segmentKeysByFeatureName[overrideFeatureContext.Name] = segmentKeys = new List(); + if (!segmentKeys.Contains(segmentItem.Key)) + segmentKeys.Add(segmentItem.Key); + } + } + return _segmentKeysByFeatureName; + } + } + + /// + /// Get the feature name a JSONPath property reads from `$.flags`, e.g. `my_feature` from `$.flags.my_feature.enabled` or `$.flags['my_feature'].enabled`. + /// + private static bool TryGetFlagDependencyName(string property, out string featureName) + { + featureName = null; + if (!property.StartsWith(FlagsPathPrefix) || property.Length <= FlagsPathPrefix.Length) + return false; + + var position = FlagsPathPrefix.Length; + switch (property[position]) + { + case '.': + var nameStart = position + 1; + var nameEnd = property.IndexOfAny(new[] { '.', '[' }, nameStart); + if (nameEnd < 0) + nameEnd = property.Length; + featureName = property.Substring(nameStart, nameEnd - nameStart); + return featureName.Length > 0 && featureName != "*"; + case '[': + if (property.Length <= position + 1) + return false; + var quote = property[position + 1]; + if (quote != '\'' && quote != '"') + return false; + var name = new StringBuilder(); + for (var index = position + 2; index < property.Length; index++) + { + var character = property[index]; + if (character == '\\' && index + 1 < property.Length) + name.Append(property[++index]); + else if (character == quote) + { + featureName = name.ToString(); + return true; + } + else + name.Append(character); + } + return false; + default: + return false; + } + } + + private static JObject GetFlagsRoot(string featureName, FlagResult flag) + { + var flags = new JObject(); + if (flag != null) + flags[featureName] = JToken.FromObject(flag); + return new JObject { ["flags"] = flags }; + } + } +} diff --git a/Flagsmith.Engine/Engine.cs b/Flagsmith.Engine/Engine.cs index f2567bb..fca2793 100644 --- a/Flagsmith.Engine/Engine.cs +++ b/Flagsmith.Engine/Engine.cs @@ -17,13 +17,17 @@ public class SegmentOverride public string SegmentName { get; set; } } - public class Engine : IEngine + public partial class Engine : IEngine { public static Hashing Hashing = new Hashing(); public const double StrongestPriority = double.NegativeInfinity; public const double WeakestPriority = double.PositiveInfinity; + public const string CircularDependencyReason = "ERROR; code=CIRCULAR_DEPENDENCY"; + + private const string ControlVariant = "control"; + /// /// Get the evaluation result for a given context /// @@ -35,9 +39,20 @@ public EvaluationResult GetEvaluationResult< { context = GetEnrichedEvaluationContext(context); var result = new EvaluationResult(); - var segmentEvaluationResult = EvaluateSegments(context); + var flagDependencies = new FlagDependencies(context); + var segmentEvaluationResult = EvaluateSegments(context, flagDependencies); result.Flags = EvaluateFlags(context, segmentEvaluationResult.SegmentOverrides); result.Segments = segmentEvaluationResult.Segments; + + if (flagDependencies.Used) + { + foreach (var resolvedFlag in flagDependencies.Resolved) + result.Flags[resolvedFlag.Key] = resolvedFlag.Value; + foreach (var featureName in flagDependencies.Cyclic) + if (result.Flags.TryGetValue(featureName, out var cyclicFlag)) + cyclicFlag.Reason = CircularDependencyReason; + } + return result; } @@ -54,7 +69,7 @@ private EvaluationContext GetEnrichedEvaluat return context; } - private static (SegmentResult[] Segments, Dictionary> SegmentOverrides) EvaluateSegments(EvaluationContext context) + private static (SegmentResult[] Segments, Dictionary> SegmentOverrides) EvaluateSegments(EvaluationContext context, FlagDependencies flagDependencies) { var segmentOverrides = new Dictionary>(); @@ -67,7 +82,7 @@ private static (SegmentResult[] Segments, Dictionary @@ -81,13 +96,9 @@ private static (SegmentResult[] Segments, Dictionary existingPriority) - continue; - } + segmentOverrides.TryGetValue(segmentOverride.Name, out var incumbent); + if (!WinsOver(segmentOverride, incumbent?.FeatureContext)) + continue; segmentOverrides[segmentOverride.Name] = new SegmentOverride { FeatureContext = segmentOverride, @@ -99,6 +110,14 @@ private static (SegmentResult[] Segments, Dictionary + /// Whether a segment override takes precedence over the one held so far: the lowest priority wins, and the latest seen wins a tie. + /// + private static bool WinsOver(FeatureContext candidate, FeatureContext incumbent) + { + return incumbent is null || (candidate.Priority ?? WeakestPriority) <= (incumbent.Priority ?? WeakestPriority); + } + private static Dictionary> EvaluateFlags(EvaluationContext context, Dictionary> segmentOverrides) { var flags = new Dictionary>(); @@ -116,7 +135,7 @@ private static Dictionary> EvaluateFlags> EvaluateFlags(EvaluationContext<_, __> context, SegmentContext<_, __> segmentContext) + private static string GetTargetingMatchReason(string segmentName) => $"TARGETING_MATCH; segment={segmentName}"; + + private static bool IsContextInSegment<_, __>(EvaluationContext<_, __> context, SegmentContext<_, __> segmentContext, FlagDependencies<_, __> flagDependencies) { return ( segmentContext?.Rules != null && segmentContext.Rules.All(rule => ContextMatchesRule( context, rule, - segmentContext.Key + segmentContext.Key, + flagDependencies )) ); } - private static bool ContextMatchesRule<_, __>(EvaluationContext<_, __> context, SegmentRule rule, string segmentKey) + private static bool ContextMatchesRule<_, __>(EvaluationContext<_, __> context, SegmentRule rule, string segmentKey, FlagDependencies<_, __> flagDependencies) { bool matchesConditions; @@ -155,13 +177,13 @@ private static bool ContextMatchesRule<_, __>(EvaluationContext<_, __> context, switch (rule.Type) { case TypeEnum.All: - matchesConditions = rule.Conditions.All(condition => ContextMatchesCondition(context, condition, segmentKey)); + matchesConditions = rule.Conditions.All(condition => ContextMatchesCondition(context, condition, segmentKey, flagDependencies)); break; case TypeEnum.Any: - matchesConditions = rule.Conditions.Any(condition => ContextMatchesCondition(context, condition, segmentKey)); + matchesConditions = rule.Conditions.Any(condition => ContextMatchesCondition(context, condition, segmentKey, flagDependencies)); break; case TypeEnum.None: - matchesConditions = !rule.Conditions.Any(condition => ContextMatchesCondition(context, condition, segmentKey)); + matchesConditions = !rule.Conditions.Any(condition => ContextMatchesCondition(context, condition, segmentKey, flagDependencies)); break; default: matchesConditions = false; @@ -178,13 +200,13 @@ private static bool ContextMatchesRule<_, __>(EvaluationContext<_, __> context, switch (rule.Type) { case TypeEnum.All: - matchesSubRules = rule.Rules.All(r => ContextMatchesRule(context, r, segmentKey)); + matchesSubRules = rule.Rules.All(r => ContextMatchesRule(context, r, segmentKey, flagDependencies)); break; case TypeEnum.Any: - matchesSubRules = rule.Rules.Any(r => ContextMatchesRule(context, r, segmentKey)); + matchesSubRules = rule.Rules.Any(r => ContextMatchesRule(context, r, segmentKey, flagDependencies)); break; case TypeEnum.None: - matchesSubRules = !rule.Rules.Any(r => ContextMatchesRule(context, r, segmentKey)); + matchesSubRules = !rule.Rules.Any(r => ContextMatchesRule(context, r, segmentKey, flagDependencies)); break; default: matchesSubRules = false; @@ -193,9 +215,9 @@ private static bool ContextMatchesRule<_, __>(EvaluationContext<_, __> context, return matchesSubRules; } - private static bool ContextMatchesCondition<_, __>(EvaluationContext<_, __> context, Condition condition, string segmentKey) + private static bool ContextMatchesCondition<_, __>(EvaluationContext<_, __> context, Condition condition, string segmentKey, FlagDependencies<_, __> flagDependencies) { - var contextValue = GetContextValue(context, condition.Property); + var contextValue = GetContextValue(context, condition.Property, flagDependencies); switch (condition.Operator) { @@ -257,14 +279,17 @@ private static bool ContextMatchesCondition<_, __>(EvaluationContext<_, __> cont } } - private static object GetContextValue<_, __>(EvaluationContext<_, __> context, string property) + private static object GetContextValue<_, __>(EvaluationContext<_, __> context, string property, FlagDependencies<_, __> flagDependencies) { object value = null; if (!(context.Identity?.Traits?.TryGetValue(property, out value) ?? false)) { if (property.StartsWith("$.")) { - var jToken = JToken.FromObject(context).SelectToken(property); + var root = TryGetFlagDependencyName(property, out var featureName) + ? GetFlagsRoot(featureName, flagDependencies.Get(featureName)) + : JToken.FromObject(context); + var jToken = root.SelectToken(property); if (jToken is JValue jValue) { value = jValue.ToObject(); @@ -277,10 +302,12 @@ private static object GetContextValue<_, __>(EvaluationContext<_, __> context, s private static FlagResult GetFlagResult<_, FeatureMetadataT>(EvaluationContext<_, FeatureMetadataT> context, FeatureContext featureContext, String reason) { FlagResult flagResult = null; + string variantKey = null; var key = context?.Identity?.Key; - if (key != null && featureContext.Variants != null) + if (key != null && featureContext.Variants != null && featureContext.Variants.Any()) { + variantKey = ControlVariant; var percentageValue = Hashing.GetHashedPercentageForObjectIds(new List() { featureContext.Key, key }); var startPercentage = 0.0f; float limit; @@ -298,6 +325,7 @@ private static FlagResult GetFlagResult<_, FeatureMetadataT>(E Value = variant.Value, Metadata = featureContext.Metadata, Reason = FormattableString.Invariant($"SPLIT; weight={weight}"), + Variant = variant.Key, }; break; } @@ -313,7 +341,8 @@ private static FlagResult GetFlagResult<_, FeatureMetadataT>(E Enabled = featureContext.Enabled, Value = featureContext.Value, Metadata = featureContext.Metadata, - Reason = reason + Reason = reason, + Variant = variantKey, }; } diff --git a/Flagsmith.Engine/EvaluationContext/EvaluationContext.cs b/Flagsmith.Engine/EvaluationContext/EvaluationContext.cs index 1e20540..047e892 100644 --- a/Flagsmith.Engine/EvaluationContext/EvaluationContext.cs +++ b/Flagsmith.Engine/EvaluationContext/EvaluationContext.cs @@ -141,6 +141,13 @@ public partial class FeatureContext /// public partial class FeatureValue { + /// + /// A stable, human-readable identifier for the variant. Omitted when the variant has no key + /// set. + /// + [JsonProperty("key", Required = Required.DisallowNull, NullValueHandling = NullValueHandling.Ignore)] + public string Key { get; set; } + /// /// Priority of the feature flag variant. Lower values indicate a higher priority when /// multiple variants apply to the same context key. diff --git a/Flagsmith.Engine/EvaluationResult/EvaluationResult.cs b/Flagsmith.Engine/EvaluationResult/EvaluationResult.cs index 9a8181b..c8a3823 100644 --- a/Flagsmith.Engine/EvaluationResult/EvaluationResult.cs +++ b/Flagsmith.Engine/EvaluationResult/EvaluationResult.cs @@ -65,6 +65,14 @@ public partial class FlagResult /// [JsonProperty("value", Required = Required.AllowNull)] public object Value { get; set; } + + /// + /// A stable identifier of the multivariate variant the identity was bucketed into: the + /// variant's key, "control" for the control bucket, or null when no multivariate split + /// applied (a standard feature, an unkeyed variant, or evaluation without an identity). + /// + [JsonProperty("variant", Required = Required.AllowNull)] + public string Variant { get; set; } } public partial class SegmentResult diff --git a/Flagsmith.Engine/Feature/Models/MultivariateFeatureOptionModel.cs b/Flagsmith.Engine/Feature/Models/MultivariateFeatureOptionModel.cs index 6db248d..71fa927 100644 --- a/Flagsmith.Engine/Feature/Models/MultivariateFeatureOptionModel.cs +++ b/Flagsmith.Engine/Feature/Models/MultivariateFeatureOptionModel.cs @@ -10,5 +10,7 @@ public class MultivariateFeatureOptionModel public int Id { get; set; } [JsonProperty("value")] public object Value { get; set; } + [JsonProperty("key")] + public string Key { get; set; } } } diff --git a/Flagsmith.EngineTest/EngineTestData b/Flagsmith.EngineTest/EngineTestData index 2e710ea..28363b3 160000 --- a/Flagsmith.EngineTest/EngineTestData +++ b/Flagsmith.EngineTest/EngineTestData @@ -1 +1 @@ -Subproject commit 2e710ea7c6994110d17d1adb40b2ec2e90e7cf44 +Subproject commit 28363b388de2850c7c3f1b6e7a8bdd41d8da433c diff --git a/Flagsmith.FlagsmithClient/Mappers.cs b/Flagsmith.FlagsmithClient/Mappers.cs index 96a21be..4f0a276 100644 --- a/Flagsmith.FlagsmithClient/Mappers.cs +++ b/Flagsmith.FlagsmithClient/Mappers.cs @@ -144,6 +144,7 @@ private static Dictionary> MapEnvironmen feature.Variants = featureState.MultivariateFeatureStateValues .Select(mv => new FeatureValue { + Key = mv.MultivariateFeatureOption.Key, Value = mv.MultivariateFeatureOption.Value, Weight = mv.PercentageAllocation, Priority = mv.Id != 0 ? mv.Id : Array.IndexOf(sortedUUIDs, mv.MvFsValueUUID),