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.05k stars 114 forks source link

Add feature management schema for main branch #362

Closed zhiyuanliang-ms closed 8 months ago

zhiyuanliang-ms commented 9 months ago

358

Before release 3.2.0. We need to specify the schema we supported. The schemas specify what the "FeatureManagement" section should look like, which is actually a missing piece of this repo's documentation.

This PR is associated with this PR: https://github.com/Azure/AppConfiguration/pull/871. The Microsoft Feature Management configuration schema mentioned in the README is placed in the Azure App Config repo.

The Json schema (file) names need to be discussed.

I found this website: https://www.jsonschemavalidator.net/ which can validate the json schema.

zhiyuanliang-ms commented 9 months ago

@jimmyca15

https://github.com/microsoft/FeatureManagement-Dotnet/blob/2.6.1/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs#L115

/*

We support

myFeature: {
  enabledFor: [ "myFeatureFilter1", "myFeatureFilter2" ]
},
myDisabledFeature: {
  enabledFor: [  ]
},
myFeature2: {
  enabledFor: "myFeatureFilter1;myFeatureFilter2"
},
myDisabledFeature2: {
  enabledFor: ""
},
myFeature3: "myFeatureFilter1;myFeatureFilter2",
myDisabledFeature3: "",
myAlwaysEnabledFeature: true,
myAlwaysDisabledFeature: false // removing this line would be the same as setting it to false
myAlwaysEnabledFeature2: {
  enabledFor: true
},
myAlwaysDisabledFeature2: {
  enabledFor: false
},
myAllRequiredFilterFeature: {
    requirementType: "all"
    enabledFor: [ "myFeatureFilter1", "myFeatureFilter2" ],
},
*/

These are all valid/invalid usages we mentioned in the comment in ConfigurationFeatureDefinitionProvider.cs.

First, I've tested these usages (which was told as valid in the comment, but actually invalid):

myFeature3: "myFeatureFilter1;myFeatureFilter2",
myFeature2: {
  enabledFor: "myFeatureFilter1;myFeatureFilter2"
}
myFeature: {
  enabledFor: [ "myFeatureFilter1", "myFeatureFilter2" ]
}

They cannot be recognized by our current library. I think we should remove them in the comments.

The following usages can be supported by the current library, but we only mentioned it in the comments not in the README or anywhere else:

myAlwaysEnabledFeature2: {
  enabledFor: true
},
myAlwaysDisabledFeature2: {
  enabledFor: false
},

I will add this usage in the schema but will not mention it in the README

zhiyuanliang-ms commented 9 months ago

@jimmyca15 The schema names need to be decided. Do you have any idea about the name?