Adjusts the configuration provider to no longer check top level for feature flags (#261)

* Adjusts the configuration provider to no longer check top level for feature flags

* Adding missing using and removes unused else

* Remove unused string

* Add null check to constructor

Author: rossgrambo

src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 //
 using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Primitives;
 using System;
 using System.Collections.Concurrent;
@@ -23,14 +24,17 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider
 
         private const string FeatureFiltersSectionName = "EnabledFor";
         private const string RequirementTypeKeyword = "RequirementType";
+        private const string FeatureManagementSectionName = "FeatureManagement";
         private readonly IConfiguration _configuration;
         private readonly ConcurrentDictionary<string, FeatureDefinition> _definitions;
         private IDisposable _changeSubscription;
+        private readonly ILogger _logger;
         private int _stale = 0;
 
-        public ConfigurationFeatureDefinitionProvider(IConfiguration configuration)
+        public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory)
         {
             _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
+            _logger = loggerFactory?.CreateLogger<ConfigurationFeatureDefinitionProvider>() ?? throw new ArgumentNullException(nameof(loggerFactory));
             _definitions = new ConcurrentDictionary<string, FeatureDefinition>();
 
             _changeSubscription = ChangeToken.OnChange(
@@ -199,18 +203,18 @@ We support
 
         private IEnumerable<IConfigurationSection> GetFeatureDefinitionSections()
         {
-            const string FeatureManagementSectionName = "FeatureManagement";
+            //
+            // Look for feature definitions under the "FeatureManagement" section
+            IConfigurationSection featureManagementConfigurationSection = _configuration.GetSection(FeatureManagementSectionName);
 
-            if (_configuration.GetChildren().Any(s => s.Key.Equals(FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase)))
+            if (featureManagementConfigurationSection.Exists())
             {
-                //
-                // Look for feature definitions under the "FeatureManagement" section
-                return _configuration.GetSection(FeatureManagementSectionName).GetChildren();
-            }
-            else
-            {
-                return _configuration.GetChildren();
+                return featureManagementConfigurationSection.GetChildren();
             }
+
+            _logger.LogDebug($"No configuration section named '{FeatureManagementSectionName}' was found.");
+
+            return Enumerable.Empty<IConfigurationSection>();
         }
     }
 }

src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs

@@ -4,6 +4,7 @@
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Logging;
 using System;
 
 namespace Microsoft.FeatureManagement
@@ -48,7 +49,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec
                 throw new ArgumentNullException(nameof(configuration));
             }
 
-            services.AddSingleton<IFeatureDefinitionProvider>(new ConfigurationFeatureDefinitionProvider(configuration));
+            services.AddSingleton<IFeatureDefinitionProvider>(sp => new ConfigurationFeatureDefinitionProvider(configuration, sp.GetRequiredService<ILoggerFactory>()));
 
             return services.AddFeatureManagement();
         }

tests/Tests.FeatureManagement/FeatureManagement.cs

@@ -12,9 +12,11 @@
 using Microsoft.FeatureManagement.FeatureFilters;
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Net;
 using System.Net.Http;
+using System.Text;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -71,6 +73,30 @@ public async Task ReadsConfiguration()
             Assert.True(called);
         }
 
+        [Fact]
+        public async Task ReadsOnlyFeatureManagementSection()
+        {
+            MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes("{\"AllowedHosts\": \"*\"}"));
+            IConfiguration config = new ConfigurationBuilder().AddJsonStream(stream).Build();
+
+            var services = new ServiceCollection();
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureFilter<TestFilter>();
+
+            ServiceProvider serviceProvider = services.BuildServiceProvider();
+
+            IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
+
+            await foreach (string featureName in featureManager.GetFeatureNamesAsync())
+            {
+                // Fail, as no features should be found
+                Assert.True(false);
+            }
+        }
+
         [Fact]
         public async Task Integrates()
         {

tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj

@@ -16,7 +16,7 @@
   <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.1'">
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
     <PackageReference Include="Microsoft.AspNetCore" Version="2.1.7" />
-    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="2.1.1" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="3.1.32" />
     <PackageReference Include="Microsoft.AspNetCore.TestHost" Version="2.1.1" />
     <PackageReference Include="Microsoft.AspNetCore.Mvc" Version="2.1.3" />
   </ItemGroup>