From 7bb349fd3d30a413977697af4ae644e1c57804e1 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 29 Jul 2024 13:53:15 -0400 Subject: [PATCH] feat: back targetingKey with internal map (#287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the internal dictionary for the `targetingKey`. This is non-breaking from a compiler perspective. It could result in some behavioral changes, but IMO they are largely desirable. Fixes: https://github.com/open-feature/dotnet-sdk/issues/235 --------- Signed-off-by: Todd Baert Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com> Signed-off-by: Artyom Tonoyan --- src/OpenFeature/Model/EvaluationContext.cs | 23 +++++--- .../Model/EvaluationContextBuilder.cs | 23 +------- .../OpenFeatureEvaluationContextTests.cs | 52 ++++++++++++++++++- 3 files changed, 70 insertions(+), 28 deletions(-) diff --git a/src/OpenFeature/Model/EvaluationContext.cs b/src/OpenFeature/Model/EvaluationContext.cs index 59b1fe20..304e4cd9 100644 --- a/src/OpenFeature/Model/EvaluationContext.cs +++ b/src/OpenFeature/Model/EvaluationContext.cs @@ -11,26 +11,30 @@ namespace OpenFeature.Model /// Evaluation context public sealed class EvaluationContext { + /// + /// The index for the "targeting key" property when the EvaluationContext is serialized or expressed as a dictionary. + /// + internal const string TargetingKeyIndex = "targetingKey"; + + private readonly Structure _structure; /// /// Internal constructor used by the builder. /// - /// The targeting key - /// The content of the context. - internal EvaluationContext(string? targetingKey, Structure content) + /// + internal EvaluationContext(Structure content) { - this.TargetingKey = targetingKey; this._structure = content; } + /// /// Private constructor for making an empty . /// private EvaluationContext() { this._structure = Structure.Empty; - this.TargetingKey = string.Empty; } /// @@ -89,7 +93,14 @@ public IImmutableDictionary AsDictionary() /// /// Returns the targeting key for the context. /// - public string? TargetingKey { get; } + public string? TargetingKey + { + get + { + this._structure.TryGetValue(TargetingKeyIndex, out Value? targetingKey); + return targetingKey?.AsString; + } + } /// /// Return an enumerator for all values diff --git a/src/OpenFeature/Model/EvaluationContextBuilder.cs b/src/OpenFeature/Model/EvaluationContextBuilder.cs index c672c401..30e2ffe0 100644 --- a/src/OpenFeature/Model/EvaluationContextBuilder.cs +++ b/src/OpenFeature/Model/EvaluationContextBuilder.cs @@ -14,8 +14,6 @@ public sealed class EvaluationContextBuilder { private readonly StructureBuilder _attributes = Structure.Builder(); - internal string? TargetingKey { get; private set; } - /// /// Internal to only allow direct creation by . /// @@ -28,7 +26,7 @@ internal EvaluationContextBuilder() { } /// This builder public EvaluationContextBuilder SetTargetingKey(string targetingKey) { - this.TargetingKey = targetingKey; + this._attributes.Set(EvaluationContext.TargetingKeyIndex, targetingKey); return this; } @@ -138,23 +136,6 @@ public EvaluationContextBuilder Set(string key, DateTime value) /// This builder public EvaluationContextBuilder Merge(EvaluationContext context) { - string? newTargetingKey = ""; - - if (!string.IsNullOrWhiteSpace(this.TargetingKey)) - { - newTargetingKey = this.TargetingKey; - } - - if (!string.IsNullOrWhiteSpace(context.TargetingKey)) - { - newTargetingKey = context.TargetingKey; - } - - if (!string.IsNullOrWhiteSpace(newTargetingKey)) - { - this.TargetingKey = newTargetingKey; - } - foreach (var kvp in context) { this.Set(kvp.Key, kvp.Value); @@ -169,7 +150,7 @@ public EvaluationContextBuilder Merge(EvaluationContext context) /// An immutable public EvaluationContext Build() { - return new EvaluationContext(this.TargetingKey, this._attributes.Build()); + return new EvaluationContext(this._attributes.Build()); } } } diff --git a/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs b/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs index 5329620f..826ac68e 100644 --- a/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureEvaluationContextTests.cs @@ -160,7 +160,7 @@ public void TryGetValue_WhenCalledWithExistingKey_ReturnsTrueAndExpectedValue() var key = "testKey"; var expectedValue = new Value("testValue"); var structure = new Structure(new Dictionary { { key, expectedValue } }); - var evaluationContext = new EvaluationContext("targetingKey", structure); + var evaluationContext = new EvaluationContext(structure); // Act var result = evaluationContext.TryGetValue(key, out var actualValue); @@ -169,5 +169,55 @@ public void TryGetValue_WhenCalledWithExistingKey_ReturnsTrueAndExpectedValue() Assert.True(result); Assert.Equal(expectedValue, actualValue); } + + [Fact] + public void GetValueOnTargetingKeySetWithTargetingKey_Equals_TargetingKey() + { + // Arrange + var value = "my_targeting_key"; + var evaluationContext = EvaluationContext.Builder().SetTargetingKey(value).Build(); + + // Act + var result = evaluationContext.TryGetValue(EvaluationContext.TargetingKeyIndex, out var actualFromStructure); + var actualFromTargetingKey = evaluationContext.TargetingKey; + + // Assert + Assert.True(result); + Assert.Equal(value, actualFromStructure?.AsString); + Assert.Equal(value, actualFromTargetingKey); + } + + [Fact] + public void GetValueOnTargetingKeySetWithStructure_Equals_TargetingKey() + { + // Arrange + var value = "my_targeting_key"; + var evaluationContext = EvaluationContext.Builder().Set(EvaluationContext.TargetingKeyIndex, new Value(value)).Build(); + + // Act + var result = evaluationContext.TryGetValue(EvaluationContext.TargetingKeyIndex, out var actualFromStructure); + var actualFromTargetingKey = evaluationContext.TargetingKey; + + // Assert + Assert.True(result); + Assert.Equal(value, actualFromStructure?.AsString); + Assert.Equal(value, actualFromTargetingKey); + } + + [Fact] + public void GetValueOnTargetingKeySetWithNonStringValue_Equals_Null() + { + // Arrange + var evaluationContext = EvaluationContext.Builder().Set(EvaluationContext.TargetingKeyIndex, new Value(1)).Build(); + + // Act + var result = evaluationContext.TryGetValue(EvaluationContext.TargetingKeyIndex, out var actualFromStructure); + var actualFromTargetingKey = evaluationContext.TargetingKey; + + // Assert + Assert.True(result); + Assert.Null(actualFromStructure?.AsString); + Assert.Null(actualFromTargetingKey); + } } }