Adds exclude logic to Targeting and Audience, adds unit test, updates example to include exclude

[rossgrambo]

In response to #123.

Adds Exclusion to Audience

Audiences have historically been a list of users, groups, and rollout percentages. This change adds an additional field in Audience called Exclusion. This field holds a list of Users and Groups. These users and groups are evaluated before the rest of the Audience. If there is a match, the feature flag immediately returns false.

If the targeting filter has sibling filters, they will still be evaluated after a false result.

Example

appsettings.json

{
  "Name": "Targeting",
  "Parameters": {
    "Audience": {
      "Users": [
        "Jeff",
        "Anne"
      ],
      "Groups": [
        {
          "Name": "Management",
          "RolloutPercentage": 100
        },
        {
          "Name": "TeamMembers",
          "RolloutPercentage": 45
        }
      ],
      "DefaultRolloutPercentage": 20,
      "Exclusion": {
        "Users": [
          "Anne",
          "Phil"
        ],
        "Groups": [
          "Contractor"
        ]
      }
    }
  }
}

Evaluation

.IsEnabledAsync(betaFeature, new TargetingContext { UserId = "Anne" }}) // returns false
.IsEnabledAsync(betaFeature, new TargetingContext { UserId = "Phil", Groups = new List<string>() { "Management" }}}) // returns false
.IsEnabledAsync(betaFeature, new TargetingContext { UserId = "Bill", Groups = new List<string>() { "Contractor" }}}) // returns false
.IsEnabledAsync(betaFeature, new TargetingContext { UserId = "Bill", Groups = new List<string>() { "Management", "Contractor" }}}) // returns false

[rossgrambo]

I should call out I went with the wording "Exclude" rather than "Except". Using Except/Exception in the code was not very clear. If we still want the field named Except, we can do that.

[zhenlan]

I should call out I went with the wording "Exclude" rather than "Except". Using Except/Exception in the code was not very clear. If we still want the field named Except, we can do that.

I like it. I was thinking about it too. "Exclude" is a verb, a little bit weird as a property name. Do we like to use a noun, "Exclusion", instead?

[jimmyca15]

I should call out I went with the wording "Exclude" rather than "Except". Using Except/Exception in the code was not very clear. If we still want the field named Except, we can do that.

I like it. I was thinking about it too. "Exclude" is a verb, a little bit weird as a property name. Do we like to use a noun, "Exclusion", instead?

Also offering that symmetry with existing name in the configuration, enabledFor, would be excluded

[zhenlan]

Also offering that symmetry with existing name in the configuration, enabledFor, would be excluded

Honestly speaking, "except" works better with "enabledFor". I think that's the reason we picked the "except" in the first place. I am just concerned we may over complicate the design. If we choose to use "Exclusion" for the AppConfig schema, we might just use the same everywhere else. It will make the tooling work easier.

[jimmyca15]

@zhenlan

If we choose to use "Exclusion" for the AppConfig schema, we might just use the same everywhere else.

You lost me here. What are you referring to as the AppConfig schema? and what are you referring to as everywhere else?

From my thinking, AppConfig schema plays no role in the decision here. This name will only show up in the targeting configuration which is AppConfig agnostic.

[rossgrambo]

If we choose to use "Exclusion" for the AppConfig schema, we might just use the same everywhere else.
I believe this is the only occurrence of the word. We're defining what the field name in an Audience should be. And then that will be reflected in both the backend and featuremanagement schemas. It will also likely be reflected in the portal UI.

Agreed- Except is very clear. I think Exclude or ExcludedFor are still pretty readable. Exclusion makes it sound like a class or something I need more context to understand- but I'm likely just overthinking it.

I also prefer ExcludedFor over Excluded as I think it hints that the contents are just users/groups. And strictly matches EnabledFor.

All of that said- I think I'll move forward with Exclusion unless people feel strongly against it. *update: We felt strongly against it.

[rossgrambo]

Synced with Jimmy- We aligned on Except.

In this context, we're just defining an Audience (not a feature flag). Except is clearly calling out that the audience is as defined- except for these users/groups.

Also calling out that this field is the only place this idea of Except/Exclude/Exclusion exists. It's on the Audience object- so it will be seen in both the backend and FeatureManagement schemas, but there's no other uses of this word yet.

Not going to merge until we hear sign-off from you as well @zhenlan , but for now I'll go ahead and make the code changes towards "Except". *Update - We've aligned back on Exclusion

[rossgrambo]

Changed the naming to Exclusion!