Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkgs/sdk/client/contract-tests/TestService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class Webapp
"inline-context-all",
"anonymous-redaction",
"client-prereq-events",
"client-prereq-cycle-detection",
"client-per-context-summaries"
};

Expand Down
31 changes: 26 additions & 5 deletions pkgs/sdk/client/src/LdClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ EvaluationDetail<T> VariationInternal<T>(string featureKey, LdValue defaultJson,
}

EvaluationDetail<T> EvaluateInternal<T>(string featureKey, LdValue defaultJson, LdValue.Converter<T> converter,
bool checkType, EventFactory eventFactory)
bool checkType, EventFactory eventFactory, HashSet<string> visited = null)
{
T defaultValue = converter.ToType(defaultJson);

Expand Down Expand Up @@ -791,12 +791,33 @@ EvaluationDetail<T> errorResult(EvaluationErrorKind kind) =>
}

// Prerequisites are evaluated directly via EvaluateInternal to avoid triggering hooks.
if (flag.Prerequisites != null)
//
// `visited` tracks the chain of prerequisite dependencies from the top-level evaluation
// to (but not including) the current flag. It is allocated lazily: variation calls on
// prereq-less flags allocate no set. Once created it is shared for the rest of the walk
// via add-before-recurse / remove-after-recurse in a try/finally, so an exception below
// cannot leave a stale ancestor entry visible to a sibling branch.
if (flag.Prerequisites != null && flag.Prerequisites.Count > 0)
{
foreach (var prerequisiteKey in flag.Prerequisites)
var ancestors = visited ?? new HashSet<string>();
ancestors.Add(featureKey);
try
{
foreach (var prerequisiteKey in flag.Prerequisites)
{
if (ancestors.Contains(prerequisiteKey))
{
// Cyclic edge: skip descent, continue with remaining prerequisites at this level.
// The requested flag's value and reason (below) are unaffected.
continue;
}
EvaluateInternal(prerequisiteKey, LdValue.Null, LdValue.Convert.Json, false,
_eventFactoryWithReasons, ancestors);
}
}
finally
{
EvaluateInternal(prerequisiteKey, LdValue.Null, LdValue.Convert.Json, false,
_eventFactoryWithReasons);
ancestors.Remove(featureKey);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,133 @@ public void VariationSendsFeatureEventForPrerequisites()
}
}

// Cycle-detection tests exercise the ancestor-set cycle guard added to EvaluateInternal.
// Each test constructs a cyclic prereq graph, evaluates one flag on the cycle, and asserts
// (a) the requested flag returns its cached value unchanged and (b) the recorded evaluation
// events match exactly one entry per cycle-safe descent.

[Fact]
public void VariationSkipsSelfLoopPrerequisiteAndReturnsCachedValue()
{
var flagA = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagA").Build();
_testData.Update(_testData.Flag("flagA").PreconfiguredFlag(flagA));

using (LdClient client = MakeClient(user))
{
Assert.True(client.BoolVariation("flagA"));
// Only flagA emits a feature event; the self-prereq is cycle-skipped.
Assert.Collection(eventProcessor.Events,
e => CheckIdentifyEvent(e, user),
e => CheckEvaluationEvent(e, "flagA")
);
}
}

[Fact]
public void VariationHandlesTwoCycleEvaluatingA()
{
var flagA = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagB").Build();
var flagB = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagA").Build();
_testData.Update(_testData.Flag("flagA").PreconfiguredFlag(flagA));
_testData.Update(_testData.Flag("flagB").PreconfiguredFlag(flagB));

using (LdClient client = MakeClient(user))
{
Assert.True(client.BoolVariation("flagA"));
// A -> B -> [A skipped]. Events deepest-first: B (as prereq of A), then A.
Assert.Collection(eventProcessor.Events,
e => CheckIdentifyEvent(e, user),
e => CheckEvaluationEvent(e, "flagB"),
e => CheckEvaluationEvent(e, "flagA")
);
}
}

[Fact]
public void VariationHandlesTwoCycleEvaluatingB()
{
var flagA = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagB").Build();
var flagB = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagA").Build();
_testData.Update(_testData.Flag("flagA").PreconfiguredFlag(flagA));
_testData.Update(_testData.Flag("flagB").PreconfiguredFlag(flagB));

using (LdClient client = MakeClient(user))
{
Assert.True(client.BoolVariation("flagB"));
// Symmetric: same graph, entry from B. Events: A (as prereq of B), then B.
Assert.Collection(eventProcessor.Events,
e => CheckIdentifyEvent(e, user),
e => CheckEvaluationEvent(e, "flagA"),
e => CheckEvaluationEvent(e, "flagB")
);
}
}

[Fact]
public void VariationHandlesThreeCycle()
{
var flagA = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagB").Build();
var flagB = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagC").Build();
var flagC = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagA").Build();
_testData.Update(_testData.Flag("flagA").PreconfiguredFlag(flagA));
_testData.Update(_testData.Flag("flagB").PreconfiguredFlag(flagB));
_testData.Update(_testData.Flag("flagC").PreconfiguredFlag(flagC));

using (LdClient client = MakeClient(user))
{
Assert.True(client.BoolVariation("flagA"));
// A -> B -> C -> [A skipped]. Events emitted deepest-first: C, B, A.
Assert.Collection(eventProcessor.Events,
e => CheckIdentifyEvent(e, user),
e => CheckEvaluationEvent(e, "flagC"),
e => CheckEvaluationEvent(e, "flagB"),
e => CheckEvaluationEvent(e, "flagA")
);
}
}

[Fact]
public void VariationEmitsSharedDescendantOncePerPathInDiamond()
{
// Diamond: A -> [B, C], B -> [D], C -> [D]. Not a cycle. Ancestor-set (current-path)
// semantics let D be reached on each of the two independent paths, so D emits twice.
// A naive "visited across the whole walk" implementation would drop the second event.
var flagA = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagB", "flagC").Build();
var flagB = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagD").Build();
var flagC = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Prerequisites("flagD").Build();
var flagD = new FeatureFlagBuilder().Value(LdValue.Of(true)).Variation(1).Version(1)
.TrackEvents(false).TrackReason(false).Build();
_testData.Update(_testData.Flag("flagA").PreconfiguredFlag(flagA));
_testData.Update(_testData.Flag("flagB").PreconfiguredFlag(flagB));
_testData.Update(_testData.Flag("flagC").PreconfiguredFlag(flagC));
_testData.Update(_testData.Flag("flagD").PreconfiguredFlag(flagD));

using (LdClient client = MakeClient(user))
{
Assert.True(client.BoolVariation("flagA"));
// Events (deepest-first per path): D (via B), B, D (via C), C, A. D appears twice.
Assert.Collection(eventProcessor.Events,
e => CheckIdentifyEvent(e, user),
e => CheckEvaluationEvent(e, "flagD"),
e => CheckEvaluationEvent(e, "flagB"),
e => CheckEvaluationEvent(e, "flagD"),
e => CheckEvaluationEvent(e, "flagC"),
e => CheckEvaluationEvent(e, "flagA")
);
}
}

private void CheckIdentifyEvent(object e, Context c)
{
IdentifyEvent ie = Assert.IsType<IdentifyEvent>(e);
Expand Down
Loading