Skip to content

Commit 94fcdc1

Browse files
kylejuliandevkylejuliandev
authored
fix: Address issues when evaluating the context in the InMemoryProvider (#615)
* Address issues when evaluating the context in the InMemoryProvider Signed-off-by: Kyle Julian <[email protected]> * Address Gemini review comments Signed-off-by: Kyle Julian <[email protected]> * Improve unit test coverage Signed-off-by: Kyle Julian <[email protected]> --------- Signed-off-by: Kyle Julian <[email protected]>
1 parent a8d12ef commit 94fcdc1

File tree

2 files changed

+144
-33
lines changed

2 files changed

+144
-33
lines changed

‎src/OpenFeature/Providers/Memory/Flag.cs‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,41 +36,48 @@ public Flag(Dictionary<string, T> variants, string defaultVariant, Func<Evaluati
3636

3737
internal ResolutionDetails<T> Evaluate(string flagKey, T _, EvaluationContext? evaluationContext)
3838
{
39-
T? value;
4039
if (this._contextEvaluator == null)
4140
{
42-
if (this._variants.TryGetValue(this._defaultVariant, out value))
43-
{
44-
return new ResolutionDetails<T>(
45-
flagKey,
46-
value,
47-
variant: this._defaultVariant,
48-
reason: Reason.Static,
49-
flagMetadata: this._flagMetadata
50-
);
51-
}
52-
else
53-
{
54-
throw new GeneralException($"variant {this._defaultVariant} not found");
55-
}
41+
return this.EvaluateDefaultVariant(flagKey);
5642
}
57-
else
43+
44+
string variant;
45+
try
46+
{
47+
variant = this._contextEvaluator.Invoke(evaluationContext ?? EvaluationContext.Empty);
48+
}
49+
catch (Exception)
5850
{
59-
var variant = this._contextEvaluator.Invoke(evaluationContext ?? EvaluationContext.Empty);
60-
if (!this._variants.TryGetValue(variant, out value))
61-
{
62-
throw new GeneralException($"variant {variant} not found");
63-
}
64-
else
65-
{
66-
return new ResolutionDetails<T>(
67-
flagKey,
68-
value,
69-
variant: variant,
70-
reason: Reason.TargetingMatch,
71-
flagMetadata: this._flagMetadata
72-
);
73-
}
51+
return this.EvaluateDefaultVariant(flagKey, Reason.Default);
7452
}
53+
54+
if (!this._variants.TryGetValue(variant, out var value))
55+
{
56+
return this.EvaluateDefaultVariant(flagKey, Reason.Default);
57+
}
58+
59+
return new ResolutionDetails<T>(
60+
flagKey,
61+
value,
62+
variant: variant,
63+
reason: Reason.TargetingMatch,
64+
flagMetadata: this._flagMetadata
65+
);
66+
}
67+
68+
private ResolutionDetails<T> EvaluateDefaultVariant(string flagKey, string reason = Reason.Static)
69+
{
70+
if (this._variants.TryGetValue(this._defaultVariant, out var value))
71+
{
72+
return new ResolutionDetails<T>(
73+
flagKey,
74+
value,
75+
variant: this._defaultVariant,
76+
reason: reason,
77+
flagMetadata: this._flagMetadata
78+
);
79+
}
80+
81+
throw new GeneralException($"variant {this._defaultVariant} not found");
7582
}
7683
}

‎test/OpenFeature.Tests/Providers/Memory/InMemoryProviderTests.cs‎

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ public InMemoryProviderTests()
9898
return "missing";
9999
}
100100
)
101+
},
102+
{
103+
"evaluator-throws-flag", new Flag<bool>(
104+
variants: new Dictionary<string, bool>(){
105+
{ "on", true },
106+
{ "off", false }
107+
},
108+
defaultVariant: "on",
109+
(context) => {
110+
throw new Exception("Cannot evaluate flag at the moment.");
111+
}
112+
)
101113
}
102114
});
103115

@@ -113,6 +125,18 @@ public async Task GetBoolean_ShouldEvaluateWithReasonAndVariant()
113125
Assert.Equal("on", details.Variant);
114126
}
115127

128+
[Fact]
129+
public async Task GetBoolean_WithNoEvaluationContext_ShouldEvaluateWithReasonAndVariant()
130+
{
131+
// Act
132+
ResolutionDetails<bool> details = await this.commonProvider.ResolveBooleanValueAsync("boolean-flag", false);
133+
134+
// Assert
135+
Assert.True(details.Value);
136+
Assert.Equal(Reason.Static, details.Reason);
137+
Assert.Equal("on", details.Variant);
138+
}
139+
116140
[Fact]
117141
public async Task GetString_ShouldEvaluateWithReasonAndVariant()
118142
{
@@ -122,6 +146,18 @@ public async Task GetString_ShouldEvaluateWithReasonAndVariant()
122146
Assert.Equal("greeting", details.Variant);
123147
}
124148

149+
[Fact]
150+
public async Task GetString_WithNoEvaluationContext_ShouldEvaluateWithReasonAndVariant()
151+
{
152+
// Act
153+
ResolutionDetails<string> details = await this.commonProvider.ResolveStringValueAsync("string-flag", "nope");
154+
155+
// Assert
156+
Assert.Equal("hi", details.Value);
157+
Assert.Equal(Reason.Static, details.Reason);
158+
Assert.Equal("greeting", details.Variant);
159+
}
160+
125161
[Fact]
126162
public async Task GetInt_ShouldEvaluateWithReasonAndVariant()
127163
{
@@ -131,6 +167,18 @@ public async Task GetInt_ShouldEvaluateWithReasonAndVariant()
131167
Assert.Equal("ten", details.Variant);
132168
}
133169

170+
[Fact]
171+
public async Task GetInt_WithNoEvaluationContext_ShouldEvaluateWithReasonAndVariant()
172+
{
173+
// Act
174+
ResolutionDetails<int> details = await this.commonProvider.ResolveIntegerValueAsync("integer-flag", 13);
175+
176+
// Assert
177+
Assert.Equal(10, details.Value);
178+
Assert.Equal(Reason.Static, details.Reason);
179+
Assert.Equal("ten", details.Variant);
180+
}
181+
134182
[Fact]
135183
public async Task GetDouble_ShouldEvaluateWithReasonAndVariant()
136184
{
@@ -140,6 +188,18 @@ public async Task GetDouble_ShouldEvaluateWithReasonAndVariant()
140188
Assert.Equal("half", details.Variant);
141189
}
142190

191+
[Fact]
192+
public async Task GetDouble_WithNoEvaluationContext_ShouldEvaluateWithReasonAndVariant()
193+
{
194+
// Arrange
195+
ResolutionDetails<double> details = await this.commonProvider.ResolveDoubleValueAsync("float-flag", 13);
196+
197+
// Assert
198+
Assert.Equal(0.5, details.Value);
199+
Assert.Equal(Reason.Static, details.Reason);
200+
Assert.Equal("half", details.Variant);
201+
}
202+
143203
[Fact]
144204
public async Task GetStruct_ShouldEvaluateWithReasonAndVariant()
145205
{
@@ -151,6 +211,20 @@ public async Task GetStruct_ShouldEvaluateWithReasonAndVariant()
151211
Assert.Equal("template", details.Variant);
152212
}
153213

214+
[Fact]
215+
public async Task GetStruct_WithNoEvaluationContext_ShouldEvaluateWithReasonAndVariant()
216+
{
217+
// Act
218+
ResolutionDetails<Value> details = await this.commonProvider.ResolveStructureValueAsync("object-flag", new Value());
219+
220+
// Assert
221+
Assert.Equal(true, details.Value.AsStructure?["showImages"].AsBoolean);
222+
Assert.Equal("Check out these pics!", details.Value.AsStructure?["title"].AsString);
223+
Assert.Equal(100, details.Value.AsStructure?["imagesPerPage"].AsInteger);
224+
Assert.Equal(Reason.Static, details.Reason);
225+
Assert.Equal("template", details.Variant);
226+
}
227+
154228
[Fact]
155229
public async Task GetString_ContextSensitive_ShouldEvaluateWithReasonAndVariant()
156230
{
@@ -161,6 +235,18 @@ public async Task GetString_ContextSensitive_ShouldEvaluateWithReasonAndVariant(
161235
Assert.Equal("internal", details.Variant);
162236
}
163237

238+
[Fact]
239+
public async Task GetString_ContextSensitive_WithNoEvaluationContext_ShouldEvaluateWithReasonAndVariant()
240+
{
241+
// Act
242+
ResolutionDetails<string> details = await this.commonProvider.ResolveStringValueAsync("context-aware", "nope");
243+
244+
// Assert
245+
Assert.Equal("EXTERNAL", details.Value);
246+
Assert.Equal(Reason.Default, details.Reason);
247+
Assert.Equal("external", details.Variant);
248+
}
249+
164250
[Fact]
165251
public async Task EmptyFlags_ShouldWork()
166252
{
@@ -198,9 +284,27 @@ public async Task MissingDefaultVariant_ShouldThrow()
198284
}
199285

200286
[Fact]
201-
public async Task MissingEvaluatedVariant_ShouldThrow()
287+
public async Task MissingEvaluatedVariant_ReturnsDefaultVariant()
202288
{
203-
await Assert.ThrowsAsync<GeneralException>(() => this.commonProvider.ResolveBooleanValueAsync("invalid-evaluator-flag", false, EvaluationContext.Empty));
289+
// Act
290+
var result = await this.commonProvider.ResolveBooleanValueAsync("invalid-evaluator-flag", false, EvaluationContext.Empty);
291+
292+
// Assert
293+
Assert.True(result.Value);
294+
Assert.Equal(Reason.Default, result.Reason);
295+
Assert.Equal("on", result.Variant);
296+
}
297+
298+
[Fact]
299+
public async Task ContextEvaluatorThrows_ReturnsDefaultVariant()
300+
{
301+
// Act
302+
var result = await this.commonProvider.ResolveBooleanValueAsync("evaluator-throws-flag", false, EvaluationContext.Empty);
303+
304+
// Assert
305+
Assert.True(result.Value);
306+
Assert.Equal(Reason.Default, result.Reason);
307+
Assert.Equal("on", result.Variant);
204308
}
205309

206310
[Fact]

0 commit comments

Comments
 (0)