From 424206f30ada846c5265c461b4e12d173a024f25 Mon Sep 17 00:00:00 2001 From: Amer Jusupovic <32405726+amerjusupovic@users.noreply.github.com> Date: Wed, 13 Mar 2024 09:57:13 -0700 Subject: [PATCH 1/2] Fix bug with variant configuration value for feature management (#531) * WIP fix configuration value to accept any type * fix configuration value case for all valid values * add more testing, clarify null value scenario * remove special case, match configuration pattern for null * fix test to match changes --- .../FeatureManagementKeyValueAdapter.cs | 5 +- .../FeatureManagement/FeatureVariant.cs | 3 +- .../FeatureManagementTests.cs | 136 ++++++++++++++---- 3 files changed, 110 insertions(+), 34 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs index dbdacbf0..7b15f881 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs @@ -93,9 +93,10 @@ public Task>> ProcessKeyValue(Configura keyValues.Add(new KeyValuePair($"{variantsPath}:{FeatureManagementConstants.Name}", featureVariant.Name)); - if (featureVariant.ConfigurationValue != null) + foreach (KeyValuePair kvp in new JsonFlattener().FlattenJson(featureVariant.ConfigurationValue)) { - keyValues.Add(new KeyValuePair($"{variantsPath}:{FeatureManagementConstants.ConfigurationValue}", featureVariant.ConfigurationValue)); + keyValues.Add(new KeyValuePair($"{variantsPath}:{FeatureManagementConstants.ConfigurationValue}" + + (string.IsNullOrEmpty(kvp.Key) ? "" : $":{kvp.Key}"), kvp.Value)); } if (featureVariant.ConfigurationReference != null) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureVariant.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureVariant.cs index d3baa00d..c590f56b 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureVariant.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureVariant.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // +using System.Text.Json; using System.Text.Json.Serialization; namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement @@ -11,7 +12,7 @@ internal class FeatureVariant public string Name { get; set; } [JsonPropertyName("configuration_value")] - public string ConfigurationValue { get; set; } + public JsonElement ConfigurationValue { get; set; } [JsonPropertyName("configuration_reference")] public string ConfigurationReference { get; set; } diff --git a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs index 852f053b..8c2d2bbb 100644 --- a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs +++ b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs @@ -188,13 +188,13 @@ public class FeatureManagementTests eTag: new ETag("0a76e3d7-7ec1-4e37-883c-9ea6d0d89e63"), contentType: "text"); - private ConfigurationSetting _variantsKv = ConfigurationModelFactory.ConfigurationSetting( - key: FeatureManagementConstants.FeatureFlagMarker + "VariantsFeature", + private ConfigurationSetting _variantsKv1 = ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "VariantsFeature1", value: @" { - ""id"": ""VariantsFeature"", + ""id"": ""VariantsFeature1"", ""description"": """", - ""display_name"": ""Variants Feature"", + ""display_name"": ""Variants Feature 1"", ""enabled"": true, ""conditions"": { ""client_filters"": [ @@ -265,6 +265,54 @@ public class FeatureManagementTests contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")); + private ConfigurationSetting _variantsKv2 = ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "VariantsFeature2", + value: @" + { + ""id"": ""VariantsFeature2"", + ""description"": """", + ""display_name"": ""Variants Feature 2"", + ""enabled"": false, + ""conditions"": { + ""client_filters"": [ + ] + }, + ""variants"": [ + { + ""name"": ""ObjectVariant"", + ""configuration_value"": { + ""Key1"": ""Value1"", + ""Key2"": { + ""InsideKey2"": ""Value2"" + } + } + }, + { + ""name"": ""NumberVariant"", + ""configuration_value"": 100 + }, + { + ""name"": ""NullVariant"", + ""configuration_value"": null + }, + { + ""name"": ""MissingValueVariant"" + }, + { + ""name"": ""BooleanVariant"", + ""configuration_value"": true + } + ], + ""allocation"": { + ""default_when_disabled"": ""ObjectVariant"", + ""default_when_enabled"": ""ObjectVariant"" + } + } + ", + label: default, + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")); + private ConfigurationSetting _telemetryKv = ConfigurationModelFactory.ConfigurationSetting( key: FeatureManagementConstants.FeatureFlagMarker + "TelemetryFeature", value: @" @@ -1266,7 +1314,8 @@ public void WithVariants() { var featureFlags = new List() { - _variantsKv + _variantsKv1, + _variantsKv2 }; var mockResponse = new Mock(); @@ -1283,32 +1332,57 @@ public void WithVariants() }) .Build(); - Assert.Equal("AlwaysOn", config["FeatureManagement:VariantsFeature:EnabledFor:0:Name"]); - Assert.Equal("Big", config["FeatureManagement:VariantsFeature:Variants:0:Name"]); - Assert.Equal("600px", config["FeatureManagement:VariantsFeature:Variants:0:ConfigurationValue"]); - Assert.Equal("Small", config["FeatureManagement:VariantsFeature:Variants:1:Name"]); - Assert.Equal("ShoppingCart:Small", config["FeatureManagement:VariantsFeature:Variants:1:ConfigurationReference"]); - Assert.Equal("Disabled", config["FeatureManagement:VariantsFeature:Variants:1:StatusOverride"]); - Assert.Equal("Small", config["FeatureManagement:VariantsFeature:Allocation:DefaultWhenDisabled"]); - Assert.Equal("Small", config["FeatureManagement:VariantsFeature:Allocation:DefaultWhenEnabled"]); - Assert.Equal("Big", config["FeatureManagement:VariantsFeature:Allocation:User:0:Variant"]); - Assert.Equal("Marsha", config["FeatureManagement:VariantsFeature:Allocation:User:0:Users:0"]); - Assert.Equal("John", config["FeatureManagement:VariantsFeature:Allocation:User:0:Users:1"]); - Assert.Equal("Small", config["FeatureManagement:VariantsFeature:Allocation:User:1:Variant"]); - Assert.Equal("Alice", config["FeatureManagement:VariantsFeature:Allocation:User:1:Users:0"]); - Assert.Equal("Bob", config["FeatureManagement:VariantsFeature:Allocation:User:1:Users:1"]); - Assert.Equal("Big", config["FeatureManagement:VariantsFeature:Allocation:Group:0:Variant"]); - Assert.Equal("Ring1", config["FeatureManagement:VariantsFeature:Allocation:Group:0:Groups:0"]); - Assert.Equal("Small", config["FeatureManagement:VariantsFeature:Allocation:Group:1:Variant"]); - Assert.Equal("Ring2", config["FeatureManagement:VariantsFeature:Allocation:Group:1:Groups:0"]); - Assert.Equal("Ring3", config["FeatureManagement:VariantsFeature:Allocation:Group:1:Groups:1"]); - Assert.Equal("Big", config["FeatureManagement:VariantsFeature:Allocation:Percentile:0:Variant"]); - Assert.Equal("0", config["FeatureManagement:VariantsFeature:Allocation:Percentile:0:From"]); - Assert.Equal("50", config["FeatureManagement:VariantsFeature:Allocation:Percentile:0:To"]); - Assert.Equal("Small", config["FeatureManagement:VariantsFeature:Allocation:Percentile:1:Variant"]); - Assert.Equal("50", config["FeatureManagement:VariantsFeature:Allocation:Percentile:1:From"]); - Assert.Equal("100", config["FeatureManagement:VariantsFeature:Allocation:Percentile:1:To"]); - Assert.Equal("13992821", config["FeatureManagement:VariantsFeature:Allocation:Seed"]); + Assert.Equal("AlwaysOn", config["FeatureManagement:VariantsFeature1:EnabledFor:0:Name"]); + Assert.Equal("Big", config["FeatureManagement:VariantsFeature1:Variants:0:Name"]); + Assert.Equal("600px", config["FeatureManagement:VariantsFeature1:Variants:0:ConfigurationValue"]); + Assert.Equal("Small", config["FeatureManagement:VariantsFeature1:Variants:1:Name"]); + Assert.Equal("ShoppingCart:Small", config["FeatureManagement:VariantsFeature1:Variants:1:ConfigurationReference"]); + Assert.Equal("Disabled", config["FeatureManagement:VariantsFeature1:Variants:1:StatusOverride"]); + Assert.Equal("Small", config["FeatureManagement:VariantsFeature1:Allocation:DefaultWhenDisabled"]); + Assert.Equal("Small", config["FeatureManagement:VariantsFeature1:Allocation:DefaultWhenEnabled"]); + Assert.Equal("Big", config["FeatureManagement:VariantsFeature1:Allocation:User:0:Variant"]); + Assert.Equal("Marsha", config["FeatureManagement:VariantsFeature1:Allocation:User:0:Users:0"]); + Assert.Equal("John", config["FeatureManagement:VariantsFeature1:Allocation:User:0:Users:1"]); + Assert.Equal("Small", config["FeatureManagement:VariantsFeature1:Allocation:User:1:Variant"]); + Assert.Equal("Alice", config["FeatureManagement:VariantsFeature1:Allocation:User:1:Users:0"]); + Assert.Equal("Bob", config["FeatureManagement:VariantsFeature1:Allocation:User:1:Users:1"]); + Assert.Equal("Big", config["FeatureManagement:VariantsFeature1:Allocation:Group:0:Variant"]); + Assert.Equal("Ring1", config["FeatureManagement:VariantsFeature1:Allocation:Group:0:Groups:0"]); + Assert.Equal("Small", config["FeatureManagement:VariantsFeature1:Allocation:Group:1:Variant"]); + Assert.Equal("Ring2", config["FeatureManagement:VariantsFeature1:Allocation:Group:1:Groups:0"]); + Assert.Equal("Ring3", config["FeatureManagement:VariantsFeature1:Allocation:Group:1:Groups:1"]); + Assert.Equal("Big", config["FeatureManagement:VariantsFeature1:Allocation:Percentile:0:Variant"]); + Assert.Equal("0", config["FeatureManagement:VariantsFeature1:Allocation:Percentile:0:From"]); + Assert.Equal("50", config["FeatureManagement:VariantsFeature1:Allocation:Percentile:0:To"]); + Assert.Equal("Small", config["FeatureManagement:VariantsFeature1:Allocation:Percentile:1:Variant"]); + Assert.Equal("50", config["FeatureManagement:VariantsFeature1:Allocation:Percentile:1:From"]); + Assert.Equal("100", config["FeatureManagement:VariantsFeature1:Allocation:Percentile:1:To"]); + Assert.Equal("13992821", config["FeatureManagement:VariantsFeature1:Allocation:Seed"]); + + Assert.Equal("Disabled", config["FeatureManagement:VariantsFeature2:Status"]); + Assert.Equal("ObjectVariant", config["FeatureManagement:VariantsFeature2:Variants:0:Name"]); + Assert.Equal("Value1", config["FeatureManagement:VariantsFeature2:Variants:0:ConfigurationValue:Key1"]); + Assert.Equal("Value2", config["FeatureManagement:VariantsFeature2:Variants:0:ConfigurationValue:Key2:InsideKey2"]); + Assert.Equal("NumberVariant", config["FeatureManagement:VariantsFeature2:Variants:1:Name"]); + Assert.Equal("100", config["FeatureManagement:VariantsFeature2:Variants:1:ConfigurationValue"]); + Assert.Equal("NullVariant", config["FeatureManagement:VariantsFeature2:Variants:2:Name"]); + Assert.Equal("", config["FeatureManagement:VariantsFeature2:Variants:2:ConfigurationValue"]); + Assert.True(config + .GetSection("FeatureManagement:VariantsFeature2:Variants:2") + .AsEnumerable() + .ToDictionary(x => x.Key, x => x.Value) + .ContainsKey("FeatureManagement:VariantsFeature2:Variants:2:ConfigurationValue")); + Assert.Equal("MissingValueVariant", config["FeatureManagement:VariantsFeature2:Variants:3:Name"]); + Assert.Null(config["FeatureManagement:VariantsFeature2:Variants:3:ConfigurationValue"]); + Assert.False(config + .GetSection("FeatureManagement:VariantsFeature2:Variants:3") + .AsEnumerable() + .ToDictionary(x => x.Key, x => x.Value) + .ContainsKey("FeatureManagement:VariantsFeature2:Variants:3:ConfigurationValue")); + Assert.Equal("BooleanVariant", config["FeatureManagement:VariantsFeature2:Variants:4:Name"]); + Assert.Equal("True", config["FeatureManagement:VariantsFeature2:Variants:4:ConfigurationValue"]); + Assert.Equal("ObjectVariant", config["FeatureManagement:VariantsFeature2:Allocation:DefaultWhenDisabled"]); + Assert.Equal("ObjectVariant", config["FeatureManagement:VariantsFeature2:Allocation:DefaultWhenEnabled"]); } [Fact] From f1af43cbb59b73a9891539c568841c01c2f7ce94 Mon Sep 17 00:00:00 2001 From: Amer Jusupovic <32405726+amerjusupovic@users.noreply.github.com> Date: Thu, 14 Mar 2024 10:48:52 -0700 Subject: [PATCH 2/2] update package versions for release (#533) --- .../Microsoft.Azure.AppConfiguration.AspNetCore.csproj | 2 +- .../Microsoft.Azure.AppConfiguration.Functions.Worker.csproj | 2 +- ...rosoft.Extensions.Configuration.AzureAppConfiguration.csproj | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj b/src/Microsoft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj index 21100235..c8b8b0a9 100644 --- a/src/Microsoft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj +++ b/src/Microsoft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj @@ -21,7 +21,7 @@ - 8.0.0-preview + 8.0.0-preview.2 diff --git a/src/Microsoft.Azure.AppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj b/src/Microsoft.Azure.AppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj index cc9e2561..55c32e2d 100644 --- a/src/Microsoft.Azure.AppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj +++ b/src/Microsoft.Azure.AppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj @@ -24,7 +24,7 @@ - 8.0.0-preview + 8.0.0-preview.2 diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj index 121e3618..d5b85ce4 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj @@ -34,7 +34,7 @@ - 8.0.0-preview + 8.0.0-preview.2