diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 44b2bcc3..4fd365a7 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -24,11 +24,12 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP // provider to be marked for caching as well. private readonly IConfiguration _configuration; private IEnumerable _dotnetFeatureDefinitionSections; - private IEnumerable _microsoftFeatureDefinitionSections; + private ConcurrentDictionary> _microsoftFeatureDefinitionsByName; private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; private int _initialized = 0; + private readonly object _initLock = new object(); private readonly Func> _getFeatureDefinitionFunc; const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; @@ -93,9 +94,11 @@ public Task GetFeatureDefinitionAsync(string featureName) if (Interlocked.Exchange(ref _stale, 0) != 0) { + Console.WriteLine("Feature definitions are stale, reloading..."); + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); - _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + GetMicrosoftFeatureDefinitions(); _definitions.Clear(); } @@ -118,37 +121,48 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() if (Interlocked.Exchange(ref _stale, 0) != 0) { + Console.WriteLine("Feature definitions are stale, reloading..."); + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); - _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + GetMicrosoftFeatureDefinitions(); _definitions.Clear(); } - foreach (IConfigurationSection featureSection in _microsoftFeatureDefinitionSections) - { - string featureName = featureSection[MicrosoftFeatureManagementFields.Id]; + // Use hashset to avoid duplicate feature names + var returnedFeatures = new HashSet(StringComparer.OrdinalIgnoreCase); - if (string.IsNullOrEmpty(featureName)) + if (_microsoftFeatureDefinitionsByName != null) + { + foreach (KeyValuePair> kvp in _microsoftFeatureDefinitionsByName) { - continue; - } + string featureName = kvp.Key; - // - // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, _getFeatureDefinitionFunc).Result; + if (returnedFeatures.Contains(featureName)) + { + continue; + } - if (definition != null) - { - yield return definition; + // + // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned + FeatureDefinition definition = _definitions.GetOrAdd(featureName, _getFeatureDefinitionFunc).Result; + + if (definition != null) + { + returnedFeatures.Add(featureName); + + yield return definition; + } } } + // Return .NET schema features for any features not already returned foreach (IConfigurationSection featureSection in _dotnetFeatureDefinitionSections) { string featureName = featureSection.Key; - if (string.IsNullOrEmpty(featureName)) + if (string.IsNullOrEmpty(featureName) || returnedFeatures.Contains(featureName)) { continue; } @@ -159,6 +173,8 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() if (definition != null) { + returnedFeatures.Add(featureName); + yield return definition; } } @@ -168,11 +184,17 @@ private void EnsureInit() { if (_initialized == 0) { - _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + lock (_initLock) + { + if (_initialized == 0) + { + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); - _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + GetMicrosoftFeatureDefinitions(); - _initialized = 1; + _initialized = 1; + } + } } } @@ -192,16 +214,12 @@ private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName) private FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName) { - IConfigurationSection microsoftFeatureDefinitionConfiguration = _microsoftFeatureDefinitionSections - .LastOrDefault(section => - string.Equals(section[MicrosoftFeatureManagementFields.Id], featureName, StringComparison.OrdinalIgnoreCase)); - - if (microsoftFeatureDefinitionConfiguration == null) + if (_microsoftFeatureDefinitionsByName.TryGetValue(featureName, out List featureSections)) { - return null; + return MergeMicrosoftSchemaFeatureDefinitions(featureName, featureSections); } - return ParseMicrosoftSchemaFeatureDefinition(microsoftFeatureDefinitionConfiguration); + return null; } private IEnumerable GetDotnetFeatureDefinitionSections() @@ -227,16 +245,16 @@ private IEnumerable GetDotnetFeatureDefinitionSections() return Enumerable.Empty(); } - private IEnumerable GetMicrosoftFeatureDefinitionSections() + private void GetMicrosoftFeatureDefinitions() { - var featureDefinitionSections = new List(); + var featureDefinitionsByName = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); - FindFeatureFlags(_configuration, featureDefinitionSections); + FindFeatureFlags(_configuration, featureDefinitionsByName); - return featureDefinitionSections; + _microsoftFeatureDefinitionsByName = featureDefinitionsByName; } - private void FindFeatureFlags(IConfiguration configuration, List featureDefinitionSections) + private void FindFeatureFlags(IConfiguration configuration, ConcurrentDictionary> featureDefinitionsByName) { if (!(configuration is IConfigurationRoot configurationRoot) || configurationRoot.Providers.Any(provider => @@ -248,7 +266,7 @@ private void FindFeatureFlags(IConfiguration configuration, List featureSections, ConcurrentDictionary> featureDefinitionsByName) + { + foreach (IConfigurationSection section in featureSections) + { + string featureName = section[MicrosoftFeatureManagementFields.Id]; + + if (!string.IsNullOrEmpty(featureName)) + { + featureDefinitionsByName.AddOrUpdate( + featureName, + new List { section }, + (key, existingList) => + { + existingList.Add(section); + return existingList; + }); + } + } + } + + private FeatureDefinition MergeMicrosoftSchemaFeatureDefinitions(string featureName, List configurationSections) + { + if (!configurationSections.Any()) + { + return null; + } + + FeatureDefinition baseDefinition = ParseMicrosoftSchemaFeatureDefinition(configurationSections.First()); + + if (configurationSections.Count == 1) + { + return baseDefinition; + } + + // Merge subsequent definitions, last definition wins in case of conflicts. + var mergedEnabledFor = new List(baseDefinition.EnabledFor ?? Enumerable.Empty()); + + var mergedVariants = new List(baseDefinition.Variants ?? Enumerable.Empty()); + + var mergedTelemetryMetadata = new Dictionary(); + + if (baseDefinition.Telemetry?.Metadata != null) + { + foreach (var kvp in baseDefinition.Telemetry.Metadata) + { + mergedTelemetryMetadata[kvp.Key] = kvp.Value; + } + } + + RequirementType mergedRequirementType = baseDefinition.RequirementType; + + FeatureStatus mergedStatus = baseDefinition.Status; + + Allocation mergedAllocation = baseDefinition.Allocation; + + bool mergedTelemetryEnabled = baseDefinition.Telemetry?.Enabled ?? false; + + foreach (IConfigurationSection section in configurationSections.Skip(1)) + { + FeatureDefinition additionalDefinition = ParseMicrosoftSchemaFeatureDefinition(section); + + if (additionalDefinition == null) + { + continue; + } + + if (additionalDefinition.EnabledFor != null) + { + foreach (FeatureFilterConfiguration filter in additionalDefinition.EnabledFor) + { + int existingIndex = mergedEnabledFor.FindIndex(f => string.Equals(f.Name, filter.Name, StringComparison.OrdinalIgnoreCase)); + + if (existingIndex >= 0) + { + // Replace the previous feature filter definition + mergedEnabledFor[existingIndex] = filter; + } + else + { + mergedEnabledFor.Add(filter); + } + } + } + + if (additionalDefinition.Variants != null) + { + foreach (VariantDefinition variant in additionalDefinition.Variants) + { + int existingIndex = mergedVariants.FindIndex(v => string.Equals(v.Name, variant.Name, StringComparison.OrdinalIgnoreCase)); + + if (existingIndex >= 0) + { + // Replace the previous variant definition + mergedVariants[existingIndex] = variant; + } + else + { + mergedVariants.Add(variant); + } + } + } + + if (additionalDefinition.Allocation != null) + { + if (mergedAllocation == null) + { + mergedAllocation = additionalDefinition.Allocation; + } + else + { + // Merge allocation properties, only override if not null + if (!string.IsNullOrEmpty(additionalDefinition.Allocation.DefaultWhenEnabled)) + { + mergedAllocation.DefaultWhenEnabled = additionalDefinition.Allocation.DefaultWhenEnabled; + } + + if (!string.IsNullOrEmpty(additionalDefinition.Allocation.DefaultWhenDisabled)) + { + mergedAllocation.DefaultWhenDisabled = additionalDefinition.Allocation.DefaultWhenDisabled; + } + + if (!string.IsNullOrEmpty(additionalDefinition.Allocation.Seed)) + { + mergedAllocation.Seed = additionalDefinition.Allocation.Seed; + } + + if (additionalDefinition.Allocation.User != null) + { + var mergedUserAllocations = new List(mergedAllocation.User ?? Enumerable.Empty()); + + foreach (UserAllocation userAllocation in additionalDefinition.Allocation.User) + { + int existingIndex = mergedUserAllocations.FindIndex(u => string.Equals(u.Variant, userAllocation.Variant, StringComparison.OrdinalIgnoreCase)); + + if (existingIndex >= 0) + { + mergedUserAllocations[existingIndex] = userAllocation; + } + else + { + mergedUserAllocations.Add(userAllocation); + } + } + + mergedAllocation.User = mergedUserAllocations; + } + + if (additionalDefinition.Allocation.Group != null) + { + var mergedGroupAllocations = new List(mergedAllocation.Group ?? Enumerable.Empty()); + + foreach (GroupAllocation groupAllocation in additionalDefinition.Allocation.Group) + { + int existingIndex = mergedGroupAllocations.FindIndex(g => string.Equals(g.Variant, groupAllocation.Variant, StringComparison.OrdinalIgnoreCase)); + + if (existingIndex >= 0) + { + mergedGroupAllocations[existingIndex] = groupAllocation; + } + else + { + mergedGroupAllocations.Add(groupAllocation); + } + } + + mergedAllocation.Group = mergedGroupAllocations; + } + + // Merge Percentile allocations - replace existing allocations with same variant name + if (additionalDefinition.Allocation.Percentile != null) + { + var mergedPercentileAllocations = new List(mergedAllocation.Percentile ?? Enumerable.Empty()); + + foreach (PercentileAllocation percentileAllocation in additionalDefinition.Allocation.Percentile) + { + int existingIndex = mergedPercentileAllocations.FindIndex(p => string.Equals(p.Variant, percentileAllocation.Variant, StringComparison.OrdinalIgnoreCase)); + + if (existingIndex >= 0) + { + mergedPercentileAllocations[existingIndex] = percentileAllocation; + } + else + { + mergedPercentileAllocations.Add(percentileAllocation); + } + } + + mergedAllocation.Percentile = mergedPercentileAllocations; + } + } + } + + if (additionalDefinition.Telemetry?.Metadata != null) + { + foreach (KeyValuePair kvp in additionalDefinition.Telemetry.Metadata) + { + mergedTelemetryMetadata[kvp.Key] = kvp.Value; + } + } + + mergedRequirementType = additionalDefinition.RequirementType; + mergedStatus = additionalDefinition.Status; + + if (additionalDefinition.Telemetry != null) + { + mergedTelemetryEnabled = additionalDefinition.Telemetry.Enabled; + } + } + + return new FeatureDefinition() + { + Name = featureName, + EnabledFor = mergedEnabledFor, + RequirementType = mergedRequirementType, + Status = mergedStatus, + Allocation = mergedAllocation, + Variants = mergedVariants, + Telemetry = new TelemetryConfiguration + { + Enabled = mergedTelemetryEnabled, + Metadata = mergedTelemetryMetadata + } + }; + } + private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) { /*