From 7c37c816b1214f3e39cdba6a01715d7db5e771ca Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Mon, 21 Oct 2024 17:36:40 -0700 Subject: [PATCH 1/7] feat: support prerequisite relationships in AllFlagsState --- pkgs/sdk/server/src/FeatureFlagsState.cs | 44 ++++++++++++++++++------ pkgs/sdk/server/src/LdClient.cs | 15 +++++--- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/pkgs/sdk/server/src/FeatureFlagsState.cs b/pkgs/sdk/server/src/FeatureFlagsState.cs index 7d5ea093..b285355d 100644 --- a/pkgs/sdk/server/src/FeatureFlagsState.cs +++ b/pkgs/sdk/server/src/FeatureFlagsState.cs @@ -156,6 +156,7 @@ public FeatureFlagsState Build() /// /// true if valid, false if invalid (default is valid) /// the same builder + [Obsolete("Unused, construct a FeatureFlagState with valid/invalid state directly")] public FeatureFlagsStateBuilder Valid(bool valid) { _valid = valid; @@ -167,8 +168,9 @@ public FeatureFlagsStateBuilder Valid(bool valid) /// /// the flag key /// the evaluation result + /// the direct prerequisites evaluated for this flag /// - public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail result) + public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail result, List prerequisites) { return AddFlag(flagKey, result.Value, @@ -177,13 +179,14 @@ public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail prerequisites) { bool flagIsTracked = flagTrackEvents || flagDebugEventsUntilDate != null; var flag = new FlagState @@ -194,7 +197,8 @@ internal FeatureFlagsStateBuilder AddFlag(string flagKey, LdValue value, int? va Reason = trackReason || (_withReasons && (!_detailsOnlyIfTracked || flagIsTracked)) ? reason : (EvaluationReason?)null, DebugEventsUntilDate = flagDebugEventsUntilDate, TrackEvents = flagTrackEvents, - TrackReason = trackReason + TrackReason = trackReason, + Prerequisites = prerequisites }; _flags[flagKey] = flag; return this; @@ -211,24 +215,27 @@ internal struct FlagState internal UnixMillisecondTime? DebugEventsUntilDate { get; set; } internal EvaluationReason? Reason { get; set; } + internal List Prerequisites { get; set; } + public override bool Equals(object other) { if (other is FlagState o) { return Variation == o.Variation && - Version == o.Version && - TrackEvents == o.TrackEvents && - TrackReason == o.TrackReason && - DebugEventsUntilDate.Equals(o.DebugEventsUntilDate) && - Object.Equals(Reason, o.Reason); + Version == o.Version && + TrackEvents == o.TrackEvents && + TrackReason == o.TrackReason && + DebugEventsUntilDate.Equals(o.DebugEventsUntilDate) && + Object.Equals(Reason, o.Reason) && + Prerequisites == o.Prerequisites; } return false; } public override int GetHashCode() { - return new HashCodeBuilder().With(Variation).With(Version).With(TrackEvents).With(TrackReason). - With(DebugEventsUntilDate).With(Reason).Value; + return new HashCodeBuilder().With(Variation).With(Version).With(TrackEvents).With(TrackReason) + .With(DebugEventsUntilDate).With(Reason).With(Prerequisites).Value; } } @@ -271,6 +278,14 @@ public override void Write(Utf8JsonWriter w, FeatureFlagsState state, JsonSerial w.WritePropertyName("reason"); EvaluationReasonConverter.WriteJsonValue(meta.Reason.Value, w); } + if (meta.Prerequisites.Count > 0) { + w.WriteStartArray("prerequisites"); + foreach (var p in meta.Prerequisites) + { + w.WriteStringValue(p); + } + w.WriteEndArray(); + } w.WriteEndObject(); } w.WriteEndObject(); @@ -318,6 +333,13 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon flag.Reason = reader.TokenType == JsonTokenType.Null ? (EvaluationReason?)null : EvaluationReasonConverter.ReadJsonValue(ref reader); break; + case "prerequisites": + flag.Prerequisites = new List(); + for (var prereqs = RequireArray(ref reader); prereqs.Next(ref reader);) + { + flag.Prerequisites.Add(reader.GetString()); + } + break; } } flags[subKey] = flag; diff --git a/pkgs/sdk/server/src/LdClient.cs b/pkgs/sdk/server/src/LdClient.cs index cbff359a..a6b2339c 100644 --- a/pkgs/sdk/server/src/LdClient.cs +++ b/pkgs/sdk/server/src/LdClient.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Security.Cryptography; using LaunchDarkly.Logging; @@ -371,8 +372,7 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[ var builder = new FeatureFlagsStateBuilder(options); var clientSideOnly = FlagsStateOption.HasOption(options, FlagsStateOption.ClientSideOnly); - var withReasons = FlagsStateOption.HasOption(options, FlagsStateOption.WithReasons); - var detailsOnlyIfTracked = FlagsStateOption.HasOption(options, FlagsStateOption.DetailsOnlyForTrackedFlags); + KeyedItems flags; try { @@ -397,6 +397,11 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[ { EvaluatorTypes.EvalResult result = _evaluator.Evaluate(flag, context); bool inExperiment = EventFactory.IsExperiment(flag, result.Result.Reason); + + var directPrerequisites = result.PrerequisiteEvals.Where( + e => e.PrerequisiteOfFlagKey == flag.Key) + .Select(p => p.PrerequisiteFlag.Key).ToList(); + builder.AddFlag( flag.Key, result.Result.Value, @@ -405,8 +410,8 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[ flag.Version, flag.TrackEvents || inExperiment, inExperiment, - flag.DebugEventsUntilDate - ); + flag.DebugEventsUntilDate, + directPrerequisites); } catch (Exception e) { @@ -414,7 +419,7 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[ string.Format("Exception caught for feature flag \"{0}\" when evaluating all flags", flag.Key), e); EvaluationReason reason = EvaluationReason.ErrorReason(EvaluationErrorKind.Exception); - builder.AddFlag(flag.Key, new EvaluationDetail(LdValue.Null, null, reason)); + builder.AddFlag(flag.Key, new EvaluationDetail(LdValue.Null, null, reason), new List()); } } return builder.Build(); From 598adcbcd764527ea5d62fc79fc3c0b4253be026 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 22 Oct 2024 12:12:02 -0700 Subject: [PATCH 2/7] updating equality comparison of FlagState --- pkgs/sdk/server/src/FeatureFlagsState.cs | 55 ++++++++++++++----- .../src/Internal/Evaluation/EvaluatorTypes.cs | 1 + pkgs/sdk/server/test/FeatureFlagsStateTest.cs | 23 +++++--- 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/pkgs/sdk/server/src/FeatureFlagsState.cs b/pkgs/sdk/server/src/FeatureFlagsState.cs index b285355d..e7603726 100644 --- a/pkgs/sdk/server/src/FeatureFlagsState.cs +++ b/pkgs/sdk/server/src/FeatureFlagsState.cs @@ -168,6 +168,18 @@ public FeatureFlagsStateBuilder Valid(bool valid) /// /// the flag key /// the evaluation result + /// + public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail result) + { + return AddFlag(flagKey, result, new List()); + } + + + /// + /// Adds the result of a flag evaluation, including direct prerequisites. + /// + /// the flag key + /// the evaluation result /// the direct prerequisites evaluated for this flag /// public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail result, List prerequisites) @@ -205,7 +217,7 @@ internal FeatureFlagsStateBuilder AddFlag(string flagKey, LdValue value, int? va } } - internal struct FlagState + internal struct FlagState : IEquatable { internal LdValue Value { get; set; } internal int? Variation { get; set; } @@ -217,19 +229,29 @@ internal struct FlagState internal List Prerequisites { get; set; } - public override bool Equals(object other) + public bool Equals(FlagState o) { - if (other is FlagState o) - { - return Variation == o.Variation && - Version == o.Version && - TrackEvents == o.TrackEvents && - TrackReason == o.TrackReason && - DebugEventsUntilDate.Equals(o.DebugEventsUntilDate) && - Object.Equals(Reason, o.Reason) && - Prerequisites == o.Prerequisites; - } - return false; + return Variation == o.Variation && + Version == o.Version && + TrackEvents == o.TrackEvents && + TrackReason == o.TrackReason && + DebugEventsUntilDate.Equals(o.DebugEventsUntilDate) && + Object.Equals(Reason, o.Reason) && + Prerequisites.SequenceEqual(o.Prerequisites); + } + public override bool Equals(object obj) + { + return obj is FlagState other && Equals(other); + } + + public static bool operator ==(FlagState lhs, FlagState rhs) + { + return lhs.Equals(rhs); + } + + public static bool operator !=(FlagState lhs, FlagState rhs) + { + return !(lhs == rhs); } public override int GetHashCode() @@ -310,7 +332,10 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon for (var flagsObj = RequireObject(ref reader); flagsObj.Next(ref reader);) { var subKey = flagsObj.Name; - var flag = flags.ContainsKey(subKey) ? flags[subKey] : new FlagState(); + var flag = flags.ContainsKey(subKey) + ? flags[subKey] + : new FlagState() { Prerequisites = new List() }; + for (var metaObj = RequireObject(ref reader); metaObj.Next(ref reader);) { switch (metaObj.Name) @@ -347,7 +372,7 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon break; default: - var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState(); + var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState(){Prerequisites = new List()}; flagForValue.Value = LdValueConverter.ReadJsonValue(ref reader); flags[key] = flagForValue; break; diff --git a/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs b/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs index 561d81e8..dbff2bef 100644 --- a/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs +++ b/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs @@ -21,6 +21,7 @@ internal EvalResult(EvaluationDetail result, IList Result; diff --git a/pkgs/sdk/server/test/FeatureFlagsStateTest.cs b/pkgs/sdk/server/test/FeatureFlagsStateTest.cs index 685a323b..0015e325 100644 --- a/pkgs/sdk/server/test/FeatureFlagsStateTest.cs +++ b/pkgs/sdk/server/test/FeatureFlagsStateTest.cs @@ -71,9 +71,12 @@ public void CanConvertToValuesMap() public void CanSerializeToJson() { var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons) - .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null) - .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000)) - .AddFlag("key3", LdValue.Null, null, EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), 300, false, false, null) + .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, new List()) + .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000), new List()) + .AddFlag("key3", LdValue.Null, null, EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), 300, false, false, null, new List() + { + "key1", "key2" + }) .Build(); var expectedString = @"{""key1"":""value1"",""key2"":""value2"",""key3"":null, @@ -83,7 +86,7 @@ public void CanSerializeToJson() },""key2"":{ ""variation"":1,""version"":200,""reason"":{""kind"":""FALLTHROUGH""},""trackEvents"":true,""debugEventsUntilDate"":1000 },""key3"":{ - ""version"":300,""reason"":{""kind"":""ERROR"",""errorKind"":""MALFORMED_FLAG""} + ""version"":300,""reason"":{""kind"":""ERROR"",""errorKind"":""MALFORMED_FLAG""},""prerequisites"":[""key1"",""key2""] } }, ""$valid"":true @@ -96,14 +99,20 @@ public void CanSerializeToJson() public void CanDeserializeFromJson() { var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons) - .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null) - .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000)) + .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, new List()) + .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000), new List{"key1"}) .Build(); var jsonString = LdJsonSerialization.SerializeObject(state); var state1 = LdJsonSerialization.DeserializeObject(jsonString); + var jsonString2 = LdJsonSerialization.SerializeObject(state1); + + // Ensure a roundtrip state -> json -> json is equal. + Assert.Equal(jsonString, jsonString2); + + // Ensure a roundtrip state -> json -> state is equal. Assert.Equal(state, state1); } } -} \ No newline at end of file +} From 3937e3fc81e52e8a6d52ede948690277b7442d46 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 22 Oct 2024 12:21:13 -0700 Subject: [PATCH 3/7] more tests --- pkgs/sdk/server/test/FeatureFlagsStateTest.cs | 58 ++++++++++++++++--- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/pkgs/sdk/server/test/FeatureFlagsStateTest.cs b/pkgs/sdk/server/test/FeatureFlagsStateTest.cs index 0015e325..6d763953 100644 --- a/pkgs/sdk/server/test/FeatureFlagsStateTest.cs +++ b/pkgs/sdk/server/test/FeatureFlagsStateTest.cs @@ -71,12 +71,13 @@ public void CanConvertToValuesMap() public void CanSerializeToJson() { var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons) - .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, new List()) - .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000), new List()) - .AddFlag("key3", LdValue.Null, null, EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), 300, false, false, null, new List() - { - "key1", "key2" - }) + .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, + new List()) + .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, + UnixMillisecondTime.OfMillis(1000), new List()) + .AddFlag("key3", LdValue.Null, null, EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), + 300, false, false, null, new List() + ) .Build(); var expectedString = @"{""key1"":""value1"",""key2"":""value2"",""key3"":null, @@ -86,7 +87,7 @@ public void CanSerializeToJson() },""key2"":{ ""variation"":1,""version"":200,""reason"":{""kind"":""FALLTHROUGH""},""trackEvents"":true,""debugEventsUntilDate"":1000 },""key3"":{ - ""version"":300,""reason"":{""kind"":""ERROR"",""errorKind"":""MALFORMED_FLAG""},""prerequisites"":[""key1"",""key2""] + ""version"":300,""reason"":{""kind"":""ERROR"",""errorKind"":""MALFORMED_FLAG""} } }, ""$valid"":true @@ -95,12 +96,48 @@ public void CanSerializeToJson() JsonAssertions.AssertJsonEqual(expectedString, actualString); } + [Fact] + public void CanSerializeFlagPrerequisites() + { + var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons) + .AddFlag("prereq1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, + new List()) + .AddFlag("prereq2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, + UnixMillisecondTime.OfMillis(1000), new List()) + .AddFlag("toplevel", LdValue.Null, null, + EvaluationReason.ErrorReason(EvaluationErrorKind.MalformedFlag), 300, false, false, null, + new List + { + "prereq1", "prereq2" + }) + .Build(); + + + var expectedString = @"{""prereq1"":""value1"",""prereq2"":""value2"",""toplevel"":null, + ""$flagsState"":{ + ""prereq1"":{ + ""variation"":0,""version"":100,""reason"":{""kind"":""OFF""} + },""prereq2"":{ + ""variation"":1,""version"":200,""reason"":{""kind"":""FALLTHROUGH""},""trackEvents"":true,""debugEventsUntilDate"":1000 + },""toplevel"":{ + ""version"":300,""reason"":{""kind"":""ERROR"",""errorKind"":""MALFORMED_FLAG""},""prerequisites"":[""prereq1"",""prereq2""] + } + }, + ""$valid"":true + }"; + var actualString = LdJsonSerialization.SerializeObject(state); + JsonAssertions.AssertJsonEqual(expectedString, actualString); + } + + [Fact] public void CanDeserializeFromJson() { var state = FeatureFlagsState.Builder(FlagsStateOption.WithReasons) - .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, new List()) - .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, UnixMillisecondTime.OfMillis(1000), new List{"key1"}) + .AddFlag("key1", LdValue.Of("value1"), 0, EvaluationReason.OffReason, 100, false, false, null, + new List()) + .AddFlag("key2", LdValue.Of("value2"), 1, EvaluationReason.FallthroughReason, 200, true, false, + UnixMillisecondTime.OfMillis(1000), new List { "key1" }) .Build(); var jsonString = LdJsonSerialization.SerializeObject(state); @@ -115,4 +152,7 @@ public void CanDeserializeFromJson() Assert.Equal(state, state1); } } + + + } From 7402975aa26202cd7ac75cc23e9994509301af27 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 22 Oct 2024 14:50:11 -0700 Subject: [PATCH 4/7] more tests, contract tests --- pkgs/sdk/server/contract-tests/TestService.cs | 3 +- .../src/Internal/Evaluation/EvaluatorTypes.cs | 7 +- pkgs/sdk/server/src/LdClient.cs | 4 +- .../Evaluation/EvaluatorPrerequisitesTest.cs | 10 +- .../test/Internal/Model/FeatureFlagBuilder.cs | 5 + .../sdk/server/test/LdClientEvaluationTest.cs | 189 ++++++++++++++++++ 6 files changed, 206 insertions(+), 12 deletions(-) diff --git a/pkgs/sdk/server/contract-tests/TestService.cs b/pkgs/sdk/server/contract-tests/TestService.cs index d6e7da05..50a008f0 100644 --- a/pkgs/sdk/server/contract-tests/TestService.cs +++ b/pkgs/sdk/server/contract-tests/TestService.cs @@ -37,7 +37,8 @@ public class Webapp "tags", "inline-context", "anonymous-redaction", - "evaluation-hooks" + "evaluation-hooks", + "client-prereq-events" }; public readonly Handler Handler; diff --git a/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs b/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs index dbff2bef..3deb788d 100644 --- a/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs +++ b/pkgs/sdk/server/src/Internal/Evaluation/EvaluatorTypes.cs @@ -21,15 +21,14 @@ internal EvalResult(EvaluationDetail result, IList Result; - internal PrerequisiteEvalRecord(FeatureFlag prerequisiteFlag, string prerequisiteOfFlagKey, + internal PrerequisiteEvalRecord(FeatureFlag prerequisiteFlag, string flagKey, EvaluationDetail result) { PrerequisiteFlag = prerequisiteFlag; - PrerequisiteOfFlagKey = prerequisiteOfFlagKey; + FlagKey = flagKey; Result = result; } } diff --git a/pkgs/sdk/server/src/LdClient.cs b/pkgs/sdk/server/src/LdClient.cs index a6b2339c..ecc32f28 100644 --- a/pkgs/sdk/server/src/LdClient.cs +++ b/pkgs/sdk/server/src/LdClient.cs @@ -399,7 +399,7 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[ bool inExperiment = EventFactory.IsExperiment(flag, result.Result.Reason); var directPrerequisites = result.PrerequisiteEvals.Where( - e => e.PrerequisiteOfFlagKey == flag.Key) + e => e.FlagKey == flag.Key) .Select(p => p.PrerequisiteFlag.Key).ToList(); builder.AddFlag( @@ -482,7 +482,7 @@ public FeatureFlagsState AllFlagsState(Context context, params FlagsStateOption[ foreach (var prereqEvent in evalResult.PrerequisiteEvals) { _eventProcessor.RecordEvaluationEvent(eventFactory.NewPrerequisiteEvaluationEvent( - prereqEvent.PrerequisiteFlag, context, prereqEvent.Result, prereqEvent.PrerequisiteOfFlagKey)); + prereqEvent.PrerequisiteFlag, context, prereqEvent.Result, prereqEvent.FlagKey)); } } var evalDetail = evalResult.Result; diff --git a/pkgs/sdk/server/test/Internal/Evaluation/EvaluatorPrerequisitesTest.cs b/pkgs/sdk/server/test/Internal/Evaluation/EvaluatorPrerequisitesTest.cs index bc953375..913b43df 100644 --- a/pkgs/sdk/server/test/Internal/Evaluation/EvaluatorPrerequisitesTest.cs +++ b/pkgs/sdk/server/test/Internal/Evaluation/EvaluatorPrerequisitesTest.cs @@ -64,7 +64,7 @@ public void FlagReturnsOffVariationAndEventIfPrerequisiteIsOff() Assert.Equal(f1.Key, e.PrerequisiteFlag.Key); Assert.Equal(LdValue.Of("go"), e.Result.Value); Assert.Equal(f1.Version, e.PrerequisiteFlag.Version); - Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey); + Assert.Equal(f0.Key, e.FlagKey); }); } @@ -99,7 +99,7 @@ public void FlagReturnsOffVariationAndEventIfPrerequisiteIsNotMet() Assert.Equal(f1.Key, e.PrerequisiteFlag.Key); Assert.Equal(LdValue.Of("nogo"), e.Result.Value); Assert.Equal(f1.Version, e.PrerequisiteFlag.Version); - Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey); + Assert.Equal(f0.Key, e.FlagKey); }); } @@ -133,7 +133,7 @@ public void FlagReturnsFallthroughVariationAndEventIfPrerequisiteIsMetAndThereAr Assert.Equal(f1.Key, e.PrerequisiteFlag.Key); Assert.Equal(LdValue.Of("go"), e.Result.Value); Assert.Equal(f1.Version, e.PrerequisiteFlag.Version); - Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey); + Assert.Equal(f0.Key, e.FlagKey); }); } @@ -176,14 +176,14 @@ public void MultipleLevelsOfPrerequisitesProduceMultipleEvents() Assert.Equal(f2.Key, e.PrerequisiteFlag.Key); Assert.Equal(LdValue.Of("go"), e.Result.Value); Assert.Equal(f2.Version, e.PrerequisiteFlag.Version); - Assert.Equal(f1.Key, e.PrerequisiteOfFlagKey); + Assert.Equal(f1.Key, e.FlagKey); }, e => { Assert.Equal(f1.Key, e.PrerequisiteFlag.Key); Assert.Equal(LdValue.Of("go"), e.Result.Value); Assert.Equal(f1.Version, e.PrerequisiteFlag.Version); - Assert.Equal(f0.Key, e.PrerequisiteOfFlagKey); + Assert.Equal(f0.Key, e.FlagKey); }); } diff --git a/pkgs/sdk/server/test/Internal/Model/FeatureFlagBuilder.cs b/pkgs/sdk/server/test/Internal/Model/FeatureFlagBuilder.cs index 20c60fe0..c51975a5 100644 --- a/pkgs/sdk/server/test/Internal/Model/FeatureFlagBuilder.cs +++ b/pkgs/sdk/server/test/Internal/Model/FeatureFlagBuilder.cs @@ -202,6 +202,11 @@ internal FeatureFlagBuilder OffWithValue(LdValue value) return On(false).OffVariation(0).Variations(value); } + internal FeatureFlagBuilder OnWithValue(LdValue value) + { + return On(true).OffVariation(0).FallthroughVariation(0).Variations(value); + } + internal FeatureFlagBuilder BooleanWithClauses(params Clause[] clauses) { return On(true).OffVariation(0) diff --git a/pkgs/sdk/server/test/LdClientEvaluationTest.cs b/pkgs/sdk/server/test/LdClientEvaluationTest.cs index 3fea194a..b63c074e 100644 --- a/pkgs/sdk/server/test/LdClientEvaluationTest.cs +++ b/pkgs/sdk/server/test/LdClientEvaluationTest.cs @@ -389,6 +389,195 @@ public void ExceptionWhenEvaluatingFlagInAllFlagsIsHandledCorrectly() AssertLogMessageRegex(true, Logging.LogLevel.Error, Evaluator.ErrorMessageForTesting); } + + + [Fact] + public void AllFlagsStateCanExposePrerequisiteRelationshipsWhenPrereqIsNotVisibleToClients() + { + var prereq1 = new FeatureFlagBuilder("prereq1") + .OnWithValue(LdValue.Of(true)).ClientSide(false).Build(); + + var prereq2 = new FeatureFlagBuilder("prereq2") + .OnWithValue(LdValue.Of(true)).ClientSide(false).Build(); + + var toplevel = new FeatureFlagBuilder("toplevel") + .Prerequisites(new Prerequisite("prereq1", 0), new Prerequisite("prereq2", 0)) + .Variations(LdValue.Of(false), LdValue.Of(true)) + .On(true) + .ClientSide(true) + .OffVariation(0) + .FallthroughVariation(1) + .Build(); + + testData.UsePreconfiguredFlag(prereq1); + testData.UsePreconfiguredFlag(prereq2); + testData.UsePreconfiguredFlag(toplevel); + + var state = client.AllFlagsState(context, FlagsStateOption.ClientSideOnly); + Assert.True(state.Valid); + + + var expectedString = @"{""toplevel"":true, + ""$flagsState"":{ + ""toplevel"":{ + ""variation"":1,""version"":1,""prerequisites"":[ + ""prereq1"",""prereq2"" + ] + } + }, + ""$valid"":true + }"; + + var actualString = LdJsonSerialization.SerializeObject(state); + + JsonAssertions.AssertJsonEqual(expectedString, actualString); + } + + [Fact] + public void AllFlagsStateCanExposePrerequisiteRelationshipsInEvaluationOrderShortCircuit() + { + var prereq1 = new FeatureFlagBuilder("prereq1") + .OffWithValue(LdValue.Of(false)).Build(); + + var prereq2 = new FeatureFlagBuilder("prereq2") + .OnWithValue(LdValue.Of(true)).Build(); + + var toplevel = new FeatureFlagBuilder("toplevel") + .Prerequisites(new Prerequisite("prereq1", 0), new Prerequisite("prereq2", 0)) + .Variations(LdValue.Of(false), LdValue.Of(true)) + .On(true) + .OffVariation(0) + .FallthroughVariation(1) + .Build(); + + testData.UsePreconfiguredFlag(prereq1); + testData.UsePreconfiguredFlag(prereq2); + testData.UsePreconfiguredFlag(toplevel); + + var state = client.AllFlagsState(context); + Assert.True(state.Valid); + + + var expectedString = @"{""prereq1"":false,""prereq2"":true,""toplevel"":false, + ""$flagsState"":{ + ""prereq1"":{ + ""variation"":0, + ""version"":1 + },""prereq2"":{ + ""variation"":0, + ""version"":1 + },""toplevel"":{ + ""variation"":0,""version"":1,""prerequisites"":[ + ""prereq1"" + ] + } + }, + ""$valid"":true + }"; + + var actualString = LdJsonSerialization.SerializeObject(state); + + JsonAssertions.AssertJsonEqual(expectedString, actualString); + } + + [Fact] + public void AllFlagsStateCanExposePrerequisiteRelationshipsInEvaluationOrderBothOn() + { + var prereq1 = new FeatureFlagBuilder("prereq1") + .OnWithValue(LdValue.Of(true)).Build(); + + var prereq2 = new FeatureFlagBuilder("prereq2") + .OnWithValue(LdValue.Of(true)).Build(); + + var toplevel = new FeatureFlagBuilder("toplevel") + .Prerequisites(new Prerequisite("prereq1", 0), new Prerequisite("prereq2", 0)) + .Variations(LdValue.Of(false), LdValue.Of(true)) + .On(true) + .OffVariation(0) + .FallthroughVariation(1) + .Build(); + + testData.UsePreconfiguredFlag(prereq1); + testData.UsePreconfiguredFlag(prereq2); + testData.UsePreconfiguredFlag(toplevel); + + var state = client.AllFlagsState(context); + Assert.True(state.Valid); + + + var expectedString = @"{""prereq1"":true,""prereq2"":true,""toplevel"":true, + ""$flagsState"":{ + ""prereq1"":{ + ""variation"":0, + ""version"":1 + },""prereq2"":{ + ""variation"":0, + ""version"":1 + },""toplevel"":{ + ""variation"":1,""version"":1,""prerequisites"":[ + ""prereq1"",""prereq2"" + ] + } + }, + ""$valid"":true + }"; + + var actualString = LdJsonSerialization.SerializeObject(state); + + JsonAssertions.AssertJsonEqual(expectedString, actualString); + } + + + [Fact] + public void AllFlagsStateCanExposePrerequisiteRelationshipsInEvaluationOrderBothOnSwapped() + { + // Same as previous test, but the order of prerequisites in the toplevel flag is swapped. This is to + // ensure we're not sorting the prerequisite list. + + var prereq1 = new FeatureFlagBuilder("prereq1") + .OnWithValue(LdValue.Of(true)).Build(); + + var prereq2 = new FeatureFlagBuilder("prereq2") + .OnWithValue(LdValue.Of(true)).Build(); + + var toplevel = new FeatureFlagBuilder("toplevel") + .Prerequisites(new Prerequisite("prereq2", 0), new Prerequisite("prereq1", 0)) // swapped + .Variations(LdValue.Of(false), LdValue.Of(true)) + .On(true) + .OffVariation(0) + .FallthroughVariation(1) + .Build(); + + testData.UsePreconfiguredFlag(prereq1); + testData.UsePreconfiguredFlag(prereq2); + testData.UsePreconfiguredFlag(toplevel); + + var state = client.AllFlagsState(context); + Assert.True(state.Valid); + + + var expectedString = @"{""prereq1"":true,""prereq2"":true,""toplevel"":true, + ""$flagsState"":{ + ""prereq1"":{ + ""variation"":0, + ""version"":1 + },""prereq2"":{ + ""variation"":0, + ""version"":1 + },""toplevel"":{ + ""variation"":1,""version"":1,""prerequisites"":[ + ""prereq2"",""prereq1"" + ] + } + }, + ""$valid"":true + }"; + + var actualString = LdJsonSerialization.SerializeObject(state); + + JsonAssertions.AssertJsonEqual(expectedString, actualString); + } + [Theory] [InlineData(MigrationStage.Off)] [InlineData(MigrationStage.DualWrite)] From 9f5f76fd6f346b1889f51aca1f7932bb627de215 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 23 Oct 2024 11:10:08 -0700 Subject: [PATCH 5/7] don't allocate prereq list by default --- pkgs/sdk/server/src/FeatureFlagsState.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pkgs/sdk/server/src/FeatureFlagsState.cs b/pkgs/sdk/server/src/FeatureFlagsState.cs index e7603726..5942aaf3 100644 --- a/pkgs/sdk/server/src/FeatureFlagsState.cs +++ b/pkgs/sdk/server/src/FeatureFlagsState.cs @@ -332,9 +332,14 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon for (var flagsObj = RequireObject(ref reader); flagsObj.Next(ref reader);) { var subKey = flagsObj.Name; + var flag = flags.ContainsKey(subKey) ? flags[subKey] - : new FlagState() { Prerequisites = new List() }; + : new FlagState + { + // Most flags have no prerequisites, don't allocate unless we need to. + Prerequisites = new List(0) + }; for (var metaObj = RequireObject(ref reader); metaObj.Next(ref reader);) { @@ -359,7 +364,6 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon EvaluationReasonConverter.ReadJsonValue(ref reader); break; case "prerequisites": - flag.Prerequisites = new List(); for (var prereqs = RequireArray(ref reader); prereqs.Next(ref reader);) { flag.Prerequisites.Add(reader.GetString()); @@ -372,7 +376,11 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon break; default: - var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState(){Prerequisites = new List()}; + var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState + { + // Most flags have no prerequisites, don't allocate unless we need to. + Prerequisites = new List(0) + }; flagForValue.Value = LdValueConverter.ReadJsonValue(ref reader); flags[key] = flagForValue; break; From f64bfa8b4de480f40cd70b69f0983637bfd7ae41 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 23 Oct 2024 12:55:55 -0700 Subject: [PATCH 6/7] empty doc tags --- pkgs/sdk/server/src/FeatureFlagsState.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkgs/sdk/server/src/FeatureFlagsState.cs b/pkgs/sdk/server/src/FeatureFlagsState.cs index 5942aaf3..8eb65fba 100644 --- a/pkgs/sdk/server/src/FeatureFlagsState.cs +++ b/pkgs/sdk/server/src/FeatureFlagsState.cs @@ -168,7 +168,7 @@ public FeatureFlagsStateBuilder Valid(bool valid) /// /// the flag key /// the evaluation result - /// + /// the same builder public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail result) { return AddFlag(flagKey, result, new List()); @@ -181,7 +181,7 @@ public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetailthe flag key /// the evaluation result /// the direct prerequisites evaluated for this flag - /// + /// the same builder public FeatureFlagsStateBuilder AddFlag(string flagKey, EvaluationDetail result, List prerequisites) { return AddFlag(flagKey, @@ -229,6 +229,7 @@ internal struct FlagState : IEquatable internal List Prerequisites { get; set; } + public bool Equals(FlagState o) { return Variation == o.Variation && @@ -319,6 +320,7 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon { var valid = true; var flags = new Dictionary(); + for (var topLevelObj = RequireObject(ref reader); topLevelObj.Next(ref reader);) { var key = topLevelObj.Name; From 4c300ca83aa3dd946c11554923cd230f95744f22 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 23 Oct 2024 15:47:28 -0700 Subject: [PATCH 7/7] make prerequisites an IReadOnlyList, and add a long comment explaining merging behavior --- pkgs/sdk/server/src/FeatureFlagsState.cs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pkgs/sdk/server/src/FeatureFlagsState.cs b/pkgs/sdk/server/src/FeatureFlagsState.cs index 8eb65fba..44c4ec06 100644 --- a/pkgs/sdk/server/src/FeatureFlagsState.cs +++ b/pkgs/sdk/server/src/FeatureFlagsState.cs @@ -227,7 +227,7 @@ internal struct FlagState : IEquatable internal UnixMillisecondTime? DebugEventsUntilDate { get; set; } internal EvaluationReason? Reason { get; set; } - internal List Prerequisites { get; set; } + internal IReadOnlyList Prerequisites { get; set; } public bool Equals(FlagState o) @@ -339,7 +339,7 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon ? flags[subKey] : new FlagState { - // Most flags have no prerequisites, don't allocate unless we need to. + // Most flags have no prerequisites, don't allocate capacity unless we need to. Prerequisites = new List(0) }; @@ -366,10 +366,25 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon EvaluationReasonConverter.ReadJsonValue(ref reader); break; case "prerequisites": + // Note: there is an assumption in this code that a given flag key could already + // have been seen before: specifically in the "values" section of the data + // (where it's a simple map of flag key -> evaluated value), but *also* if we + // have duplicate flag keys under the $flagState key. + // + // The first case is expected, but the second is not. LaunchDarkly SaaS / SDKs + // should never generate JSON that has duplicate keys. If this did happen, + // we don't want to 'merge' prerequisites in an arbitrary order: it's important + // that they remain the order they were serialized originally. + // + // Therefore, the behavior here is that the last seen value for a key will 'win' + // and overwrite any previous value. + var prereqList = new List(); for (var prereqs = RequireArray(ref reader); prereqs.Next(ref reader);) { - flag.Prerequisites.Add(reader.GetString()); + prereqList.Add(reader.GetString()); + } + flag.Prerequisites = prereqList; break; } } @@ -380,7 +395,7 @@ public override FeatureFlagsState Read(ref Utf8JsonReader reader, Type typeToCon default: var flagForValue = flags.ContainsKey(key) ? flags[key] : new FlagState { - // Most flags have no prerequisites, don't allocate unless we need to. + // Most flags have no prerequisites, don't allocate capacity unless we need to. Prerequisites = new List(0) }; flagForValue.Value = LdValueConverter.ReadJsonValue(ref reader);