From 28fea57dd82b249d832118f0fa74566406d62226 Mon Sep 17 00:00:00 2001 From: Roelof Blom Date: Wed, 7 Feb 2024 08:54:24 +0100 Subject: [PATCH 1/7] Fixes #230 Signed-off-by: Roelof Blom --- src/OpenFeature/Model/EvaluationContext.cs | 18 ++++++++- .../Model/EvaluationContextBuilder.cs | 32 +++++++++++++++- .../OpenFeatureEvaluationContextTests.cs | 37 ++++++++++++++++++- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/src/OpenFeature/Model/EvaluationContext.cs b/src/OpenFeature/Model/EvaluationContext.cs index e8a94bc9..dcbff291 100644 --- a/src/OpenFeature/Model/EvaluationContext.cs +++ b/src/OpenFeature/Model/EvaluationContext.cs @@ -17,8 +17,18 @@ public sealed class EvaluationContext /// Internal constructor used by the builder. /// /// The content of the context. - internal EvaluationContext(Structure content) + internal EvaluationContext(Structure content) : this(string.Empty, content) { + } + + /// + /// Internal constructor used by the builder. + /// + /// The targeting key + /// The content of the context. + internal EvaluationContext(string targetingKey, Structure content) + { + this.TargetingKey = targetingKey; this._structure = content; } @@ -28,6 +38,7 @@ internal EvaluationContext(Structure content) private EvaluationContext() { this._structure = Structure.Empty; + this.TargetingKey = string.Empty; } /// @@ -83,6 +94,11 @@ public IImmutableDictionary AsDictionary() /// public int Count => this._structure.Count; + /// + /// Returns the targeting key for the context. + /// + public string TargetingKey { get; } + /// /// Return an enumerator for all values /// diff --git a/src/OpenFeature/Model/EvaluationContextBuilder.cs b/src/OpenFeature/Model/EvaluationContextBuilder.cs index 57afa5cf..ef4e3ab9 100644 --- a/src/OpenFeature/Model/EvaluationContextBuilder.cs +++ b/src/OpenFeature/Model/EvaluationContextBuilder.cs @@ -14,11 +14,24 @@ public sealed class EvaluationContextBuilder { private readonly StructureBuilder _attributes = Structure.Builder(); + internal string TargetingKey { get; private set; } + /// /// Internal to only allow direct creation by . /// internal EvaluationContextBuilder() { } + /// + /// Set the targeting key for the context. + /// + /// The targeting key + /// This builder + public EvaluationContextBuilder SetTargetingKey(string targetingKey) + { + this.TargetingKey = targetingKey; + return this; + } + /// /// Set the key to the given . /// @@ -125,6 +138,23 @@ public EvaluationContextBuilder Set(string key, DateTime value) /// This builder public EvaluationContextBuilder Merge(EvaluationContext context) { + string newTargetingKey = ""; + + if (TargetingKey != null && TargetingKey.Trim() != string.Empty) + { + newTargetingKey = TargetingKey; + } + + if (context.TargetingKey != null && context.TargetingKey.Trim() != string.Empty) + { + newTargetingKey = context.TargetingKey; + } + + if (newTargetingKey != null && newTargetingKey.Trim() != string.Empty) + { + this.TargetingKey = newTargetingKey; + } + foreach (var kvp in context) { this.Set(kvp.Key, kvp.Value); @@ -139,7 +169,7 @@ public EvaluationContextBuilder Merge(EvaluationContext context) /// An immutable public EvaluationContext Build() { - return new EvaluationContext(this._attributes.Build()); + return new EvaluationContext(this.TargetingKey, this._attributes.Build()); } } } diff --git a/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs b/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs index a9906cf4..0b8ee097 100644 --- a/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs @@ -16,7 +16,6 @@ public void Should_Merge_Two_Contexts() .Set("key1", "value1"); var contextBuilder2 = new EvaluationContextBuilder() .Set("key2", "value2"); - var context1 = contextBuilder1.Merge(contextBuilder2.Build()).Build(); Assert.Equal(2, context1.Count); @@ -24,6 +23,35 @@ public void Should_Merge_Two_Contexts() Assert.Equal("value2", context1.GetValue("key2").AsString); } + [Fact] + public void Should_Change_TargetingKey_From_OverridingContext() + { + var contextBuilder1 = new EvaluationContextBuilder() + .Set("key1", "value1") + .SetTargetingKey("targeting_key"); + var contextBuilder2 = new EvaluationContextBuilder() + .Set("key2", "value2") + .SetTargetingKey("overriding_key"); + + var mergeContext = contextBuilder1.Merge(contextBuilder2.Build()).Build(); + + Assert.Equal("overriding_key", mergeContext.TargetingKey); + } + + [Fact] + public void Should_Retain_TargetingKey_When_OverridingContext_TargetingKey_Value_IsEmpty() + { + var contextBuilder1 = new EvaluationContextBuilder() + .Set("key1", "value1") + .SetTargetingKey("targeting_key"); + var contextBuilder2 = new EvaluationContextBuilder() + .Set("key2", "value2"); + + var mergeContext = contextBuilder1.Merge(contextBuilder2.Build()).Build(); + + Assert.Equal("targeting_key", mergeContext.TargetingKey); + } + [Fact] [Specification("3.2.2", "Evaluation context MUST be merged in the order: API (global; lowest precedence) - client - invocation - before hooks (highest precedence), with duplicate values being overwritten.")] public void Should_Merge_TwoContexts_And_Override_Duplicates_With_RightHand_Context() @@ -51,6 +79,8 @@ public void EvaluationContext_Should_All_Types() var now = fixture.Create(); var structure = fixture.Create(); var contextBuilder = new EvaluationContextBuilder() + .SetTargetingKey("targeting_key") + .Set("targeting_key", "userId") .Set("key1", "value") .Set("key2", 1) .Set("key3", true) @@ -60,6 +90,11 @@ public void EvaluationContext_Should_All_Types() var context = contextBuilder.Build(); + context.TargetingKey.Should().Be("targeting_key"); + var targetingKeyValue = context.GetValue(context.TargetingKey); + targetingKeyValue.IsString.Should().BeTrue(); + targetingKeyValue.AsString.Should().Be("userId"); + var value1 = context.GetValue("key1"); value1.IsString.Should().BeTrue(); value1.AsString.Should().Be("value"); From 50b6c23e7c0f161b8b43ae5aefe5c4f8a91bffc8 Mon Sep 17 00:00:00 2001 From: Roelof Blom Date: Wed, 7 Feb 2024 17:29:41 +0100 Subject: [PATCH 2/7] Improve code coverage Signed-off-by: Roelof Blom --- src/OpenFeature/Model/EvaluationContext.cs | 8 -------- .../Steps/EvaluationStepDefinitions.cs | 13 ++++++------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/OpenFeature/Model/EvaluationContext.cs b/src/OpenFeature/Model/EvaluationContext.cs index dcbff291..6db585a1 100644 --- a/src/OpenFeature/Model/EvaluationContext.cs +++ b/src/OpenFeature/Model/EvaluationContext.cs @@ -13,14 +13,6 @@ public sealed class EvaluationContext { private readonly Structure _structure; - /// - /// Internal constructor used by the builder. - /// - /// The content of the context. - internal EvaluationContext(Structure content) : this(string.Empty, content) - { - } - /// /// Internal constructor used by the builder. /// diff --git a/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs b/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs index d2cd483d..a2d4e785 100644 --- a/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs +++ b/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs @@ -200,12 +200,11 @@ public void Giventhevariantshouldbeandthereasonshouldbe(string expectedVariant, [When(@"context contains keys ""(.*)"", ""(.*)"", ""(.*)"", ""(.*)"" with values ""(.*)"", ""(.*)"", (.*), ""(.*)""")] public void Whencontextcontainskeyswithvalues(string field1, string field2, string field3, string field4, string value1, string value2, int value3, string value4) { - var attributes = ImmutableDictionary.CreateBuilder(); - attributes.Add(field1, new Value(value1)); - attributes.Add(field2, new Value(value2)); - attributes.Add(field3, new Value(value3)); - attributes.Add(field4, new Value(bool.Parse(value4))); - this.context = new EvaluationContext(new Structure(attributes)); + this.context = new EvaluationContextBuilder() + .Set(field1, value1) + .Set(field2, value2) + .Set(field3, value3) + .Set(field4, value4).Build(); } [When(@"a flag with key ""(.*)"" is evaluated with default value ""(.*)""")] @@ -225,7 +224,7 @@ public void Thentheresolvedstringresponseshouldbe(string expected) [Then(@"the resolved flag value is ""(.*)"" when the context is empty")] public void Giventheresolvedflagvalueiswhenthecontextisempty(string expected) { - string emptyContextValue = client.GetStringValue(contextAwareFlagKey, contextAwareDefaultValue, new EvaluationContext(new Structure(ImmutableDictionary.Empty))).Result; + string emptyContextValue = client.GetStringValue(contextAwareFlagKey, contextAwareDefaultValue, new EvaluationContextBuilder().Build()).Result; Assert.Equal(expected, emptyContextValue); } From 2ccb3bea24befb4d8b92a580763f2fffb011bb8c Mon Sep 17 00:00:00 2001 From: Roelof Blom Date: Wed, 7 Feb 2024 23:38:35 +0100 Subject: [PATCH 3/7] Update src/OpenFeature/Model/EvaluationContextBuilder.cs Co-authored-by: Austin Drenski Signed-off-by: Roelof Blom --- src/OpenFeature/Model/EvaluationContextBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenFeature/Model/EvaluationContextBuilder.cs b/src/OpenFeature/Model/EvaluationContextBuilder.cs index ef4e3ab9..fb8a779d 100644 --- a/src/OpenFeature/Model/EvaluationContextBuilder.cs +++ b/src/OpenFeature/Model/EvaluationContextBuilder.cs @@ -140,7 +140,7 @@ public EvaluationContextBuilder Merge(EvaluationContext context) { string newTargetingKey = ""; - if (TargetingKey != null && TargetingKey.Trim() != string.Empty) + if (!string.IsNullOrWhitespace(TargetingKey)) { newTargetingKey = TargetingKey; } From f2f36a67d3d24103fb6a1ecc31c568ea8446b06e Mon Sep 17 00:00:00 2001 From: Roelof Blom Date: Wed, 7 Feb 2024 23:38:50 +0100 Subject: [PATCH 4/7] Update src/OpenFeature/Model/EvaluationContextBuilder.cs Co-authored-by: Austin Drenski Signed-off-by: Roelof Blom --- src/OpenFeature/Model/EvaluationContextBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenFeature/Model/EvaluationContextBuilder.cs b/src/OpenFeature/Model/EvaluationContextBuilder.cs index fb8a779d..2a561033 100644 --- a/src/OpenFeature/Model/EvaluationContextBuilder.cs +++ b/src/OpenFeature/Model/EvaluationContextBuilder.cs @@ -145,7 +145,7 @@ public EvaluationContextBuilder Merge(EvaluationContext context) newTargetingKey = TargetingKey; } - if (context.TargetingKey != null && context.TargetingKey.Trim() != string.Empty) + if (!string.IsNullOrWhitespace(context.TargetingKey)) { newTargetingKey = context.TargetingKey; } From 3fb5a4436ad13e7e9d4930b882e670885b4293da Mon Sep 17 00:00:00 2001 From: Roelof Blom Date: Wed, 7 Feb 2024 23:38:58 +0100 Subject: [PATCH 5/7] Update src/OpenFeature/Model/EvaluationContextBuilder.cs Co-authored-by: Austin Drenski Signed-off-by: Roelof Blom --- src/OpenFeature/Model/EvaluationContextBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenFeature/Model/EvaluationContextBuilder.cs b/src/OpenFeature/Model/EvaluationContextBuilder.cs index 2a561033..d86ad44b 100644 --- a/src/OpenFeature/Model/EvaluationContextBuilder.cs +++ b/src/OpenFeature/Model/EvaluationContextBuilder.cs @@ -150,7 +150,7 @@ public EvaluationContextBuilder Merge(EvaluationContext context) newTargetingKey = context.TargetingKey; } - if (newTargetingKey != null && newTargetingKey.Trim() != string.Empty) + if (!string.IsNullOrWhitespace(newTargetingKey)) { this.TargetingKey = newTargetingKey; } From 82060c3cd07a354756fdc0af13f29fafe4332f65 Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Wed, 7 Feb 2024 21:41:20 -0500 Subject: [PATCH 6/7] Fix case typos (oops) Signed-off-by: Austin Drenski --- src/OpenFeature/Model/EvaluationContextBuilder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenFeature/Model/EvaluationContextBuilder.cs b/src/OpenFeature/Model/EvaluationContextBuilder.cs index d86ad44b..89174cf6 100644 --- a/src/OpenFeature/Model/EvaluationContextBuilder.cs +++ b/src/OpenFeature/Model/EvaluationContextBuilder.cs @@ -140,17 +140,17 @@ public EvaluationContextBuilder Merge(EvaluationContext context) { string newTargetingKey = ""; - if (!string.IsNullOrWhitespace(TargetingKey)) + if (!string.IsNullOrWhiteSpace(TargetingKey)) { newTargetingKey = TargetingKey; } - if (!string.IsNullOrWhitespace(context.TargetingKey)) + if (!string.IsNullOrWhiteSpace(context.TargetingKey)) { newTargetingKey = context.TargetingKey; } - if (!string.IsNullOrWhitespace(newTargetingKey)) + if (!string.IsNullOrWhiteSpace(newTargetingKey)) { this.TargetingKey = newTargetingKey; } From cea58b63fffa868ca69f6ad22523e8f1ff6d2514 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Sat, 10 Feb 2024 15:58:16 -0500 Subject: [PATCH 7/7] Update test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs Signed-off-by: Todd Baert --- test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs b/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs index 7dabfa3b..9aaf5fce 100644 --- a/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs +++ b/test/OpenFeature.E2ETests/Steps/EvaluationStepDefinitions.cs @@ -204,7 +204,7 @@ public void Whencontextcontainskeyswithvalues(string field1, string field2, stri .Set(field1, value1) .Set(field2, value2) .Set(field3, value3) - .Set(field4, value4).Build(); + .Set(field4, bool.Parse(value4)).Build(); } [When(@"a flag with key ""(.*)"" is evaluated with default value ""(.*)""")]