From 01598e2b6d54555f3817a615a60c8b2017d68ca2 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 7 Mar 2024 22:34:27 +0800 Subject: [PATCH 1/2] do validation during binding parameters --- .../Targeting/ContextualTargetingFilter.cs | 9 ++++++++- .../Targeting/TargetingEvaluator.cs | 7 +------ .../Targeting/TargetingFilter.cs | 10 +++++++++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs b/src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs index 34633749..0741bbfd 100644 --- a/src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs +++ b/src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs @@ -38,7 +38,14 @@ public ContextualTargetingFilter(IOptions options, I /// that can later be used in targeting. public object BindParameters(IConfiguration filterParameters) { - return filterParameters.Get() ?? new TargetingFilterSettings(); + TargetingFilterSettings settings = filterParameters.Get() ?? new TargetingFilterSettings(); + + if (!TargetingEvaluator.TryValidateSettings(settings, out string paramName, out string reason)) + { + throw new ArgumentException(reason, paramName); + } + + return settings; } /// diff --git a/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs b/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs index cc809e27..237c1791 100644 --- a/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs +++ b/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs @@ -35,11 +35,6 @@ public static bool IsTargeted(ITargetingContext targetingContext, TargetingFilte throw new ArgumentNullException(nameof(targetingContext)); } - if (!TryValidateSettings(settings, out string paramName, out string reason)) - { - throw new ArgumentException(reason, paramName); - } - if (settings.Audience.Exclusion != null) { // @@ -236,7 +231,7 @@ public static bool IsTargeted( /// The name of the invalid setting, if any. /// The reason that the setting is invalid. /// True if the provided settings are valid. False if the provided settings are invalid. - private static bool TryValidateSettings(TargetingFilterSettings targetingSettings, out string paramName, out string reason) + public static bool TryValidateSettings(TargetingFilterSettings targetingSettings, out string paramName, out string reason) { paramName = null; diff --git a/src/Microsoft.FeatureManagement/Targeting/TargetingFilter.cs b/src/Microsoft.FeatureManagement/Targeting/TargetingFilter.cs index d57e4b31..1ff48647 100644 --- a/src/Microsoft.FeatureManagement/Targeting/TargetingFilter.cs +++ b/src/Microsoft.FeatureManagement/Targeting/TargetingFilter.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.FeatureManagement.Targeting; using System; using System.Threading.Tasks; @@ -40,7 +41,14 @@ public TargetingFilter(IOptions options, ITargetingC /// that can later be used in targeting. public object BindParameters(IConfiguration filterParameters) { - return filterParameters.Get() ?? new TargetingFilterSettings(); + TargetingFilterSettings settings = filterParameters.Get() ?? new TargetingFilterSettings(); + + if (!TargetingEvaluator.TryValidateSettings(settings, out string paramName, out string reason)) + { + throw new ArgumentException(reason, paramName); + } + + return settings; } /// From 9824b9158f710b34c544d9a91fcb851600b146e9 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 28 Mar 2024 11:07:37 +0800 Subject: [PATCH 2/2] adjust method order --- .../Targeting/TargetingEvaluator.cs | 128 +++++++++--------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs b/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs index 237c1791..a2249529 100644 --- a/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs +++ b/src/Microsoft.FeatureManagement/Targeting/TargetingEvaluator.cs @@ -20,6 +20,70 @@ private static StringComparison GetComparisonType(bool ignoreCase) => const string OutOfRange = "The value is out of the accepted range."; const string RequiredParameter = "Value cannot be null."; + /// + /// Performs validation of targeting settings. + /// + /// The settings to validate. + /// The name of the invalid setting, if any. + /// The reason that the setting is invalid. + /// True if the provided settings are valid. False if the provided settings are invalid. + public static bool TryValidateSettings(TargetingFilterSettings targetingSettings, out string paramName, out string reason) + { + paramName = null; + + reason = null; + + if (targetingSettings == null) + { + paramName = nameof(targetingSettings); + + reason = RequiredParameter; + + return false; + } + + if (targetingSettings.Audience == null) + { + paramName = nameof(targetingSettings.Audience); + + reason = RequiredParameter; + + return false; + } + + if (targetingSettings.Audience.DefaultRolloutPercentage < 0 || targetingSettings.Audience.DefaultRolloutPercentage > 100) + { + paramName = $"{targetingSettings.Audience}.{targetingSettings.Audience.DefaultRolloutPercentage}"; + + reason = OutOfRange; + + return false; + } + + if (targetingSettings.Audience.Groups != null) + { + int index = 0; + + foreach (GroupRollout groupRollout in targetingSettings.Audience.Groups) + { + index++; + + if (groupRollout.RolloutPercentage < 0 || groupRollout.RolloutPercentage > 100) + { + // + // Audience.Groups[1].RolloutPercentage + paramName = $"{targetingSettings.Audience}.{targetingSettings.Audience.Groups}[{index}].{groupRollout.RolloutPercentage}"; + + reason = OutOfRange; + + return false; + } + } + } + + return true; + } + /// /// Checks if a provided targeting context should be targeted given targeting settings. /// @@ -224,70 +288,6 @@ public static bool IsTargeted( return IsTargeted(defaultContextId, 0, defaultRolloutPercentage); } - /// - /// Performs validation of targeting settings. - /// - /// The settings to validate. - /// The name of the invalid setting, if any. - /// The reason that the setting is invalid. - /// True if the provided settings are valid. False if the provided settings are invalid. - public static bool TryValidateSettings(TargetingFilterSettings targetingSettings, out string paramName, out string reason) - { - paramName = null; - - reason = null; - - if (targetingSettings == null) - { - paramName = nameof(targetingSettings); - - reason = RequiredParameter; - - return false; - } - - if (targetingSettings.Audience == null) - { - paramName = nameof(targetingSettings.Audience); - - reason = RequiredParameter; - - return false; - } - - if (targetingSettings.Audience.DefaultRolloutPercentage < 0 || targetingSettings.Audience.DefaultRolloutPercentage > 100) - { - paramName = $"{targetingSettings.Audience}.{targetingSettings.Audience.DefaultRolloutPercentage}"; - - reason = OutOfRange; - - return false; - } - - if (targetingSettings.Audience.Groups != null) - { - int index = 0; - - foreach (GroupRollout groupRollout in targetingSettings.Audience.Groups) - { - index++; - - if (groupRollout.RolloutPercentage < 0 || groupRollout.RolloutPercentage > 100) - { - // - // Audience.Groups[1].RolloutPercentage - paramName = $"{targetingSettings.Audience}.{targetingSettings.Audience.Groups}[{index}].{groupRollout.RolloutPercentage}"; - - reason = OutOfRange; - - return false; - } - } - } - - return true; - } - /// /// Determines if a given context id should be targeted based off the provided percentage range ///