Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the single-value parameters (on Feature Filters) be serialized as arrays ? #27

Closed
celer1tas opened this issue Nov 28, 2019 · 3 comments

Comments

@celer1tas
Copy link

celer1tas commented Nov 28, 2019

In Azure App Configuration, when defining a feature with a feature filter that has one parameter then that parameter is serialized as single-value.

public class BrowserPreviewFeatureFilterParameters
{
        public IList<string> Allowed { get; set; } = new List<string>();
        public IList<string> Blocked { get; set; } = new List<string>();
}

In the below example if we use the above model for deserialization then the parameter "Allowed" is deserialized but "Blocked" is not.
image

Observations:

  • This means that if someone defines the same parameter twice in Azure portal then the developer must deserialize field as an array, if the parameter is defined only once then the developer needs to deserialize the value using a completely different model:
public class BrowserPreviewFeatureFilterParameters
{
        public IList<string> Allowed { get; set; } = new List<string>();
        public string Blocked { get; set; } ;
}

ISSUE: The developer has to implement a JSONConverter to handle both scenarios, which is not nice.
Is this something that can be improved from Azure App Configuration portal UI ?

@celer1tas celer1tas changed the title Single-value parameters (on Feature Filters) are invisible to FeatureFilterEvaluationContext Should the single-value parameters (on Feature Filters) be serialized as arrays ? Nov 28, 2019
@celer1tas
Copy link
Author

celer1tas commented Nov 29, 2019

Is there an easy/recommended way to avoid the above issue when configuring the "AllowedBrowsers" parameter in Azure App Configuration UI, as provided in this example: appsettings.json ?

"Home": true,
    "Beta": {
      "EnabledFor": [
        {
          "Name": "Browser",
          "Parameters": { // This json object maps to a strongly typed configuration class
            "AllowedBrowsers": [ "Chrome", "Edge" ]
          }
        }
      ]
    }

To obtain the same array of values ["Chrome", "Edge"] as per above example of appsettings.json, the customer has to configure the parameter "AllowedBrowsers" twice in Azure App Configuration UI.

  • AllowedBrowsers=Crome
  • AllowedBrowsers=Edge

@jimmyca15
Copy link
Member

Related to Azure/AppConfiguration#147. The only way to avoid this before we revamp this UI to have more intuitive support for arrays is to suffix the array element with it's position in the array.

E.g.
Allowed:0 Chrome
Allowed:1 Edge

It is on our to-do list to make this more user friendly. At this point it's very hard to work with arrays like this since managing indexes is a lot of overhead.

@jimmyca15
Copy link
Member

Arrays are supported now. Azure/AppConfiguration#147

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants