microsoft / FeatureManagement-Dotnet

Microsoft.FeatureManagement provides standardized APIs for enabling feature flags within applications. Utilize this library to secure a consistent experience when developing applications that use patterns such as beta access, rollout, dark deployments, and more.
MIT License
1.06k stars 115 forks source link

Can't Set FeatureFilters In FeatureManager In .NET Framework Application #435

Closed wenkepaul closed 5 months ago

wenkepaul commented 7 months ago

I'm working on a .NET Framework 4.6.1 application which doesn't use IServiceCollection and C# 9, but we'd benefit from using this library. Because this framework targets .NET Standard 2.0 and 2.1, I was hoping to incorporate it into my application. However I'm unable to set FeatureFilters because this library uses's C# 9's init only setters. In order to support applications which don't use C# 9, I created PR 434. In addition to those changes, FeatureManagerSnapshot would also need to made public.

zhiyuanliang-ms commented 6 months ago

Are you experiencing a build error? We have this class which should help resolve this init accessor issue. Do you mean that it actually fails to work?

wenkepaul commented 6 months ago

Our .NET Framework application uses C# 7.3 and doesn't use IServiceCollection. So when I try to manually register FeatureManager with our DI container, the application fails to compile with the following code:

var featureManager = new FeatureManager {
    FeatureFilters = new List<IFeatureFilterMetadata>()
};
alexaka1 commented 6 months ago

@zhiyuanliang-ms For me in a 4.7.2 project that doesn't use the SDK style project, the IsExternalInit does not solve the build error.

zhiyuanliang-ms commented 6 months ago

@wenkepaul @alexaka1 What kind of build error do you have?

Did you have this build error:

error CS8370: Feature 'init-only setters' is not available in C# 7.3. Please use language version 9.0 or greater.

I created a .NET framwork 4.7.2 console app and added latest Microsoft.FeatureManagement 3.3.0 package to it.

Here are my Program.cs:

using System;
using System.Threading.Tasks;
using Microsoft.Extensions.Configuration;
using Microsoft.FeatureManagement;
using Microsoft.FeatureManagement.FeatureFilters;

namespace ConsoleApp1
{
    internal class Program
    {
        static async Task Main(string[] args)
        {
            IConfiguration configuration = new ConfigurationBuilder()
                .AddJsonFile("C:\\Users\\zhiyuanliang\\OneDrive - Microsoft\\Desktop\\ConsoleApp1\\appsettings.json")
                .Build();

            IFeatureDefinitionProvider featureDefinitionProvider = new ConfigurationFeatureDefinitionProvider(configuration);

            IFeatureManager featureManager = new FeatureManager(featureDefinitionProvider)
            {
                FeatureFilters = new IFeatureFilter[]
                {
                    new TimeWindowFilter()
                }
            };

            bool res = await featureManager.IsEnabledAsync("MyFeature");
            Console.WriteLine($"{res}");
        }
    }
}

The feature flag:

{
  "FeatureManagement": {
    "MyFeature": {
      "EnabledFor": [
        {
          "Name": "Microsoft.TimeWindow",
          "Parameters": {
            "Start": "2020-01-01T00:00:00Z",
            "End": "2030-12-31T23:59:59Z"
          }
        }
      ]
    }
  }
}

When I tried to build it through dotnet build command, it gave me the above build error. Then, I tried this solution.

I added <LangVersion>9.0</LangVersion> under the <PropertyGroup> section of .csproj file, then I can successfully build the app.

Could you try this solution? Please let me know whether it works for you.

alexaka1 commented 6 months ago

Yes that is error I am getting.

Yes, setting langversion explicitly to 9 results in a successful build. However, .net standard 2.0 officially only supports langversion 7.3 (supposedly because that's the highest .Net Framework supports). Now my project is 9, which gives the false sense that we can use version 9 features in Framework, which is not true and will result in runtime errors instead when someone tries to use newer features (like utf8 string literals)

Leaving the langversion at 9 because of missing optional ctor parameters seems overkill to me on my end. If the PR can be merged it would be greatly appriciated. I'm fine with preprocessing the optional parameters for standard and framework only, and for core you force them to use the init only setter.

zhiyuanliang-ms commented 6 months ago

@alexaka1

Instead of add optional parameters to constructor, we are considering replace all init with set.

will result in runtime errors instead when someone tries to use newer features (like utf8 string literals)

If this is true, we have to stop using init for netstandard.

alexaka1 commented 6 months ago

Init in itself won't cause runtime errors. But certain language features need runtime support. I saw a keynote from Mads Torgensen and he said that some of the language features are purely editor fancyness, like init only setters and nullable reference types, and others are runtime dependent like primary constructors and record structs.

alexaka1 commented 5 months ago

@zhiyuanliang-ms Would this be released as a minor version? Also thx for the merge! ❤️

zhiyuanliang-ms commented 5 months ago

@alexaka1

Would this be released as a minor version?

Yes. It will go with 3.4.0. I will release it this/next week.

zhiyuanliang-ms commented 5 months ago

@wenkepaul @alexaka1 Microsoft.FeatureManagement 3.4.0 has been released. I will close this issue. Feel free to reopen it if there is anything doesn't work for you.