Fix session manager bug

[zhiyuanliang-ms]

Why this PR?

#580

[apetukhov]

I also think that if there is no value in the session manager, then it should be written to what the feature manager returns, i.e. the default value.

FeatureManagement-Dotnet/src/Microsoft.FeatureManagement/FeatureManager.cs

Lines 275 to 278 in 57f5f93

	foreach (ISessionManager sessionManager in _sessionManagers)
	{
	await sessionManager.SetAsync(feature, enabled).ConfigureAwait(false);
	}

[zhiyuanliang-ms]

The current logic is that if feature definition is not null, then feature manager will do the feature evaluation and the result will be set to session manager.

Did I miss something?

[apetukhov]

It doesn't matter whether a feature definition exists; if the feature manager returns a result and doesn't throw an exception, the same result must be set in the session manager.

Test case

1. Check the boolean feature x in the feature manager.
2. There is no definition for feature x.
3. There is no previously saved result in the session manager.
4. The default result "false" stored in the session manager
5. The feature manager returns the default result "false"

Step 4 is currently missing.

[zhiyuanliang-ms]

It doesn't matter whether a feature definition exists; if the feature manager returns a result and doesn't throw an exception, the same result must be set in the session manager.

I believe this is the current behavior (when feature definition is not null).

We introduced the bug in v4. The bug is when feature definition is null, the result won't be set to session manage, I explained it in this comment

[apetukhov]

[Fact]
        public async Task SetResultInSessionManagerWhenFeatureDefinitionIsNull()
        {
            IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();

            var services = new ServiceCollection();

            ISessionManager sessionManager = new TestSessionManager();

            services
                .AddSingleton(config)
                .AddSingleton<ISessionManager>(sessionManager)
                .AddFeatureManagement();

            ServiceProvider serviceProvider = services.BuildServiceProvider();

            IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

           // session manager is clean here
            var result = await featureManager.IsEnabledAsync("UnexistedFeature");

           // session manager should store the result after evaluation
             Assert.AreEqual(result, await sessionManager.GetAsync("UnexistedFeature"));
        }

[zhiyuanliang-ms]

I see. Updated. Thanks!