Fixed targeting assignment precedence.

src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFeatureVariantAssigner.cs

@@ -65,71 +65,128 @@ public ValueTask<FeatureVariant> AssignVariantAsync(FeatureVariantAssignmentCont
                     nameof(variantAssignmentContext));
             }
 
-            FeatureVariant variant = null;
+            var lookup = new Dictionary<FeatureVariant, TargetingFilterSettings>();
 
-            double cumulativePercentage = 0;
+            //
+            // Check users
+            foreach (FeatureVariant v in featureDefinition.Variants)
+            {
+                TargetingFilterSettings targetingSettings = v.AssignmentParameters.Get<TargetingFilterSettings>();
+
+                //
+                // Put in lookup table to avoid repeatedly creating targeting settings
+                lookup[v] = targetingSettings;
+
+                if (targetingSettings == null &&
+                    v.Default)
+                {
+                    //
+                    // Valid to omit audience for default variant
+                    continue;
+                }
+
+                if (!TargetingEvaluator.TryValidateSettings(targetingSettings, out string paramName, out string reason))
+                {
+                    throw new ArgumentException(reason, paramName);
+                }
+
+                //
+                // Check if the user is being targeted directly
+                if (targetingSettings.Audience.Users != null &&
+                    TargetingEvaluator.IsTargeted(
+                        targetingContext,
+                        targetingSettings.Audience.Users,
+                        _options.IgnoreCase))
+                {
+                    return new ValueTask<FeatureVariant>(v);
+                }
+            }
 
             var cumulativeGroups = new Dictionary<string, double>(
                 _options.IgnoreCase ? StringComparer.OrdinalIgnoreCase :
                                       StringComparer.Ordinal);
 
+            //
+            // Check Groups
             foreach (FeatureVariant v in featureDefinition.Variants)
             {
-                TargetingFilterSettings targetingSettings = v.AssignmentParameters.Get<TargetingFilterSettings>();
+                TargetingFilterSettings targetingSettings = lookup[v];
 
-                if (targetingSettings == null)
+                if (targetingSettings == null ||
+                    targetingSettings.Audience.Groups == null)
                 {
-                    if (v.Default)
-                    {
-                        //
-                        // Valid to omit audience for default variant
-                        continue;
-                    }
+                    continue;
                 }
 
-                if (!TargetingEvaluator.TryValidateSettings(targetingSettings, out string paramName, out string reason))
+                AccumulateGroups(targetingSettings.Audience.Groups, cumulativeGroups);
+
+                if (TargetingEvaluator.IsTargeted(
+                    targetingContext,
+                    targetingSettings.Audience.Groups,
+                    _options.IgnoreCase,
+                    featureDefinition.Name))
                 {
-                    throw new ArgumentException(reason, paramName);
+                    return new ValueTask<FeatureVariant>(v);
                 }
+            }
+
+            double cumulativePercentage = 0;
 
-                AccumulateAudience(targetingSettings.Audience, cumulativeGroups, ref cumulativePercentage);
+            //
+            // Check default rollout percentage
+            foreach (FeatureVariant v in featureDefinition.Variants)
+            {
+                TargetingFilterSettings targetingSettings = lookup[v];
 
-                if (TargetingEvaluator.IsTargeted(targetingSettings, targetingContext, _options.IgnoreCase, featureDefinition.Name))
+                if (targetingSettings == null)
                 {
-                    variant = v;
+                    continue;
+                }
+
+                AccumulateDefaultRollout(targetingSettings.Audience, ref cumulativePercentage);
 
-                    break;
+                if (TargetingEvaluator.IsTargeted(
+                    targetingContext,
+                    targetingSettings.Audience.DefaultRolloutPercentage,
+                    _options.IgnoreCase,
+                    featureDefinition.Name))
+                {
+                    return new ValueTask<FeatureVariant>(v);
                 }
             }
 
-            return new ValueTask<FeatureVariant>(variant);
+            return new ValueTask<FeatureVariant>((FeatureVariant)null);
         }
 
         /// <summary>
-        /// Accumulates percentages for groups and the default rollout for an audience.
+        /// Accumulates percentages for groups of an audience.
         /// </summary>
-        /// <param name="audience">The audience that will have its percentages updated based on currently accumulated percentages</param>
-        /// <param name="cumulativeDefaultPercentage">The current cumulative default rollout percentage</param>
+        /// <param name="groups">The groups that will have their percentages updated based on currently accumulated percentages</param>
         /// <param name="cumulativeGroups">The current cumulative rollout percentage for each group</param>
-        private static void AccumulateAudience(Audience audience, Dictionary<string, double> cumulativeGroups, ref double cumulativeDefaultPercentage)
+        private static void AccumulateGroups(IEnumerable<GroupRollout> groups, Dictionary<string, double> cumulativeGroups)
         {
-            if (audience.Groups != null)
+            foreach (GroupRollout gr in groups)
             {
-                foreach (GroupRollout gr in audience.Groups)
-                {
-                    double percentage = gr.RolloutPercentage;
+                double percentage = gr.RolloutPercentage;
 
-                    if (cumulativeGroups.TryGetValue(gr.Name, out double p))
-                    {
-                        percentage += p;
-                    }
+                if (cumulativeGroups.TryGetValue(gr.Name, out double p))
+                {
+                    percentage += p;
+                }
 
-                    cumulativeGroups[gr.Name] = percentage;
+                cumulativeGroups[gr.Name] = percentage;
 
-                    gr.RolloutPercentage = percentage;
-                }
+                gr.RolloutPercentage = percentage;
             }
+        }
 
+        /// <summary>
+        /// Accumulates percentages for the  default rollout of an audience.
+        /// </summary>
+        /// <param name="audience">The audience that will have its percentages updated based on currently accumulated percentages</param>
+        /// <param name="cumulativeDefaultPercentage">The current cumulative default rollout percentage</param>
+        private static void AccumulateDefaultRollout(Audience audience, ref double cumulativeDefaultPercentage)
+        {
             cumulativeDefaultPercentage = cumulativeDefaultPercentage + audience.DefaultRolloutPercentage;
 
             audience.DefaultRolloutPercentage = cumulativeDefaultPercentage;

src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs

@@ -50,7 +50,7 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, ITargeti
 
             TargetingFilterSettings settings = context.Parameters.Get<TargetingFilterSettings>() ?? new TargetingFilterSettings();
 
-            return Task.FromResult(TargetingEvaluator.IsTargeted(settings, targetingContext, _options.IgnoreCase, context.FeatureFlagName));
+            return Task.FromResult(TargetingEvaluator.IsTargeted(targetingContext, settings, _options.IgnoreCase, context.FeatureFlagName));
         }
     }
 }

src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs

@@ -17,7 +17,10 @@ private static StringComparison GetComparisonType(bool ignoreCase) =>
                 StringComparison.OrdinalIgnoreCase :
                 StringComparison.Ordinal;
 
-        public static bool IsTargeted(TargetingFilterSettings settings, ITargetingContext targetingContext, bool ignoreCase, string hint)
+        /// <summary>
+        /// Checks if a provided targeting context should be targeted given targeting settings.
+        /// </summary>
+        public static bool IsTargeted(ITargetingContext targetingContext, TargetingFilterSettings settings, bool ignoreCase, string hint)
         {
             if (settings == null)
             {
@@ -36,29 +39,95 @@ public static bool IsTargeted(TargetingFilterSettings settings, ITargetingContex
 
             //
             // Check if the user is being targeted directly
+            if (settings.Audience.Users != null &&
+                IsTargeted(
+                    targetingContext,
+                    settings.Audience.Users,
+                    ignoreCase))
+            {
+                return true;
+            }
+
+            //
+            // Check if the user is in a group that is being targeted
+            if (settings.Audience.Groups != null &&
+                IsTargeted(
+                    targetingContext,
+                    settings.Audience.Groups,
+                    ignoreCase,
+                    hint))
+            {
+                return true;
+            }
+
+            //
+            // Check if the user is being targeted by a default rollout percentage
+            return IsTargeted(
+                targetingContext,
+                settings.Audience.DefaultRolloutPercentage,
+                ignoreCase,
+                hint);
+        }
+
+        /// <summary>
+        /// Determines if a targeting context is targeted by presence in a list of users
+        /// </summary>
+        public static bool IsTargeted(
+            ITargetingContext targetingContext,
+            IEnumerable<string> users,
+            bool ignoreCase)
+        {
+            if (targetingContext == null)
+            {
+                throw new ArgumentNullException(nameof(targetingContext));
+            }
+
+            if (users == null)
+            {
+                throw new ArgumentNullException(nameof(users));
+            }
+
             if (targetingContext.UserId != null &&
-                settings.Audience.Users != null &&
-                settings.Audience.Users.Any(user => targetingContext.UserId.Equals(user, GetComparisonType(ignoreCase))))
+                users.Any(user => targetingContext.UserId.Equals(user, GetComparisonType(ignoreCase))))
             {
                 return true;
             }
 
+            return false;
+        }
+
+        /// <summary>
+        /// Determine if a targeting context is targeted by presence in a group
+        /// </summary>
+        public static bool IsTargeted(
+            ITargetingContext targetingContext,
+            IEnumerable<GroupRollout> groups,
+            bool ignoreCase,
+            string hint)
+        {
+            if (targetingContext == null)
+            {
+                throw new ArgumentNullException(nameof(targetingContext));
+            }
+
+            if (groups == null)
+            {
+                throw new ArgumentNullException(nameof(groups));
+            }
+
             string userId = ignoreCase ?
                 targetingContext.UserId.ToLower() :
                 targetingContext.UserId;
 
-            //
-            // Check if the user is in a group that is being targeted
-            if (targetingContext.Groups != null &&
-                settings.Audience.Groups != null)
+            if (targetingContext.Groups != null)
             {
-                IEnumerable<string> groups = ignoreCase ?
+                IEnumerable<string> normalizedGroups = ignoreCase ?
                     targetingContext.Groups.Select(g => g.ToLower()) :
                     targetingContext.Groups;
 
-                foreach (string group in groups)
+                foreach (string group in normalizedGroups)
                 {
-                    GroupRollout groupRollout = settings.Audience.Groups.FirstOrDefault(g => g.Name.Equals(group, GetComparisonType(ignoreCase)));
+                    GroupRollout groupRollout = groups.FirstOrDefault(g => g.Name.Equals(group, GetComparisonType(ignoreCase)));
 
                     if (groupRollout != null)
                     {
@@ -72,11 +141,30 @@ public static bool IsTargeted(TargetingFilterSettings settings, ITargetingContex
                 }
             }
 
-            //
-            // Check if the user is being targeted by a default rollout percentage
+            return false;
+        }
+
+        /// <summary>
+        /// Determines if a targeting context is targeted by presence in a default rollout percentage.
+        /// </summary>
+        public static bool IsTargeted(
+            ITargetingContext targetingContext,
+            double defaultRolloutPercentage,
+            bool ignoreCase,
+            string hint)
+        {
+            if (targetingContext == null)
+            {
+                throw new ArgumentNullException(nameof(targetingContext));
+            }
+
+            string userId = ignoreCase ?
+                targetingContext.UserId.ToLower() :
+                targetingContext.UserId;
+
             string defaultContextId = $"{userId}\n{hint}";
 
-            return IsTargeted(defaultContextId, settings.Audience.DefaultRolloutPercentage);
+            return IsTargeted(defaultContextId, defaultRolloutPercentage);
         }
 
         /// <summary>

tests/Tests.FeatureManagement/FeatureManagement.cs

@@ -486,6 +486,67 @@ public async Task VariantTargeting()
                 CancellationToken.None));
         }
 
+        [Fact]
+        public async Task TargetingAssignmentPrecedence()
+        {
+            IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();
+
+            var services = new ServiceCollection();
+
+            services
+                .Configure<FeatureManagementOptions>(options =>
+                {
+                    options.IgnoreMissingFeatureFilters = true;
+                });
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureVariantAssigner<ContextualTargetingFeatureVariantAssigner>();
+
+            ServiceProvider serviceProvider = services.BuildServiceProvider();
+
+            IDynamicFeatureManager variantManager = serviceProvider.GetRequiredService<IDynamicFeatureManager>();
+
+            //
+            // Assigned variant by default rollout due to no higher precedence match
+            Assert.Equal("def", await variantManager.GetVariantAsync<string, ITargetingContext>(
+                Features.PrecedenceTestingFeature,
+                new TargetingContext
+                {
+                    UserId = "Patty"
+                },
+                CancellationToken.None));
+
+            //
+            // Assigned variant by group due to higher precedence than default rollout
+            Assert.Equal("ghi", await variantManager.GetVariantAsync<string, ITargetingContext>(
+                Features.PrecedenceTestingFeature,
+                new TargetingContext
+                {
+                    UserId = "Patty",
+                    Groups = new string[]
+                    {
+                        "Ring0"
+                    }
+                },
+                CancellationToken.None));
+
+            //
+            // Assigned variant by user name to higher precedence than default rollout, and group match
+            Assert.Equal("jkl", await variantManager.GetVariantAsync<string, ITargetingContext>(
+                Features.PrecedenceTestingFeature,
+                new TargetingContext
+                {
+                    UserId = "Jeff",
+                    Groups = new string[]
+                    {
+                        "Ring0"
+                    }
+                },
+                CancellationToken.None));
+        }
+
         [Fact]
         public async Task AccumulatesAudience()
         {

tests/Tests.FeatureManagement/Features.cs

@@ -13,5 +13,6 @@ static class Features
         public const string VariantFeature = "VariantFeature";
         public const string ContextualVariantFeature = "ContextualVariantFeature";
         public const string ContextualVariantTargetingFeature = "ContextualVariantTargetingFeature";
+        public const string PrecedenceTestingFeature = "PrecedenceTestingFeature";
     }
 }