Skip to content

Commit

Permalink
Evaluation log improvements (#89)
Browse files Browse the repository at this point in the history
* Align config json error handling of EvaluateLogHelper with error reporting of RolloutEvaluator

* Improve naming

* Improve User Object tests

* Allow leading/trailing whitespace in length prefix for (NOT) STARTS/ENDS WITH ANY OF comparators to align behavior with other SDKs

* Correct grammar mistake

* Make naming of UserComparator members consistent

* Add tests for some number parsing edge cases

* Adjust terminology to docs (eliminate the usage of term 'match' in the context of conditions)

* Bump version
  • Loading branch information
adams85 authored Mar 21, 2024
1 parent 985600c commit ed0a54d
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 108 deletions.
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
environment:
build_version: 9.0.2
build_version: 9.1.0
version: $(build_version)-{build}
image: Visual Studio 2022
configuration: Release
Expand Down
14 changes: 10 additions & 4 deletions src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -504,14 +504,17 @@ public void UserObjectAttributeValueConversion_TextComparisons_Test()
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "decimal:5", ">=5")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "decimal:79228162514264337593543950335", ">5")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "-Infinity", "<2.1")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", " -Infinity ", "<2.1")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "-1", "<2.1")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "2", "<2.1")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "2.1", "<=2,1")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "2,1", "<=2,1")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "3", "<>4.2")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "5", ">=5")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "Infinity", ">5")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", " Infinity ", ">5")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "NaN", "<>4.2")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", " NaN ", "<>4.2")]
[DataRow("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/FCWN-k1dV0iBf8QZrDgjdw", "numberWithPercentage", "12345", "Custom1", "NaNa", "80%")]
// Date time-based comparisons
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "datetime:2023-03-31T23:59:59.9990000Z", false)]
Expand Down Expand Up @@ -542,12 +545,15 @@ public void UserObjectAttributeValueConversion_TextComparisons_Test()
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", 1682899199, true)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", 1682899201, false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "-Infinity", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", " -Infinity ", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "1680307199.999", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "1680307200.001", true)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "1682899199.999", true)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "1682899200.001", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "+Infinity", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", " +Infinity ", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", "NaN", false)]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "boolTrueIn202304", "12345", "Custom1", " NaN ", false)]
// String array-based comparisons
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "stringArrayContainsAnyOfDogDefaultCat", "12345", "Custom1", new string[] { "x", "read" }, "Dog")]
[DataRow("configcat-sdk-1/JcPbCGl_1E-K9M-fJOyKyQ/OfQqcTjfFUGBwMKqtyEOrQ", "stringArrayContainsAnyOfDogDefaultCat", "12345", "Custom1", new string[] { "x", "Read" }, "Cat")]
Expand Down Expand Up @@ -736,10 +742,10 @@ public void ComparisonAttributeTrimming_Test(string key, string expectedReturnVa
[DataRow("notendswithanyof", "no trim")]
[DataRow("arraycontainsanyof", "no trim")]
[DataRow("arraynotcontainsanyof", "no trim")]
[DataRow("startwithanyofhashed", "default")]
[DataRow("notstartwithanyofhashed", "default")]
[DataRow("endswithanyofhashed", "default")]
[DataRow("notendswithanyofhashed", "default")]
[DataRow("startwithanyofhashed", "no trim")]
[DataRow("notstartwithanyofhashed", "no trim")]
[DataRow("endswithanyofhashed", "no trim")]
[DataRow("notendswithanyofhashed", "no trim")]
//semver comparator values trimmed because of backward compatibility
[DataRow("semverisoneof", "4 trim")]
[DataRow("semverisnotoneof", "5 trim")]
Expand Down
24 changes: 15 additions & 9 deletions src/ConfigCat.Client.Tests/UserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public void CreateUser_WithIdAndEmailAndCountry_AllAttributesShouldContainsPasse
var user = new User("id")
{
Email = "[email protected]",

Country = "US"
};

Expand All @@ -25,12 +24,17 @@ public void CreateUser_WithIdAndEmailAndCountry_AllAttributesShouldContainsPasse

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Email), out var s));
Assert.AreEqual("[email protected]", s);
Assert.AreEqual("[email protected]", user.GetAttribute(nameof(User.Email)));

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Country), out s));
Assert.AreEqual("US", s);
Assert.AreEqual("US", user.GetAttribute(nameof(User.Country)));

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Identifier), out s));
Assert.AreEqual("id", s);
Assert.AreEqual("id", user.GetAttribute(nameof(User.Identifier)));

Assert.AreEqual(3, actualAttributes.Count);
}

[TestMethod]
Expand All @@ -41,9 +45,7 @@ public void UseWellKnownAttributesAsCustomProperties_ShouldNotAppendAllAttribute
var user = new User("id")
{
Email = "[email protected]",

Country = "US",

Custom =
{
{ "myCustomAttribute", "myCustomAttributeValue"},
Expand All @@ -59,13 +61,17 @@ public void UseWellKnownAttributesAsCustomProperties_ShouldNotAppendAllAttribute

// Assert

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Identifier), out var s));
Assert.AreEqual("id", s);
Assert.AreNotEqual("myIdentifier", s);
Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Email), out var s));
Assert.AreEqual("id@example.com", s);
Assert.AreEqual("[email protected]", user.GetAttribute(nameof(User.Email)));

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Country), out s));
Assert.AreEqual("US", s);
Assert.AreNotEqual("United States", s);
Assert.AreEqual("US", user.GetAttribute(nameof(User.Country)));

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Identifier), out s));
Assert.AreEqual("id", s);
Assert.AreEqual("id", user.GetAttribute(nameof(User.Identifier)));

Assert.IsTrue(actualAttributes.TryGetValue(nameof(User.Email), out s));
Assert.AreEqual("[email protected]", s);
Expand All @@ -89,9 +95,7 @@ public void UseWellKnownAttributesAsCustomPropertiesWithDifferentNames_ShouldApp
var user = new User("id")
{
Email = "[email protected]",

Country = "US",

Custom =
{
{ attributeName, attributeValue}
Expand All @@ -108,6 +112,7 @@ public void UseWellKnownAttributesAsCustomPropertiesWithDifferentNames_ShouldApp

Assert.IsTrue(actualAttributes.TryGetValue(attributeName, out var s));
Assert.AreEqual(attributeValue, s);
Assert.AreEqual(attributeValue, user.GetAttribute(attributeName));
}

[DataTestMethod()]
Expand All @@ -122,5 +127,6 @@ public void CreateUser_ShouldSetIdentifier(string identifier, string expectedVal

Assert.AreEqual(expectedValue, user.Identifier);
Assert.AreEqual(expectedValue, user.GetAllAttributes()[nameof(User.Identifier)]);
Assert.AreEqual(expectedValue, user.GetAttribute(nameof(User.Identifier)));
}
}
63 changes: 36 additions & 27 deletions src/ConfigCatClient/Evaluation/EvaluateLogHelper.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.Generic;
using System.Globalization;
using ConfigCat.Client.Utils;

Expand All @@ -13,11 +14,6 @@ internal static class EvaluateLogHelper

internal const int StringListMaxLength = 10;

public static IndentedTextBuilder AppendEvaluationResult(this IndentedTextBuilder builder, bool result)
{
return builder.Append(result ? "true" : "false");
}

private static IndentedTextBuilder AppendUserCondition(this IndentedTextBuilder builder, string? comparisonAttribute, UserComparator comparator, object? comparisonValue)
{
return builder.Append($"User.{comparisonAttribute} {comparator.ToDisplayText()} '{comparisonValue ?? InvalidValuePlaceholder}'");
Expand Down Expand Up @@ -64,12 +60,14 @@ private static IndentedTextBuilder AppendUserCondition(this IndentedTextBuilder

public static IndentedTextBuilder AppendUserCondition(this IndentedTextBuilder builder, UserCondition condition)
{
var comparisonAttribute = condition.ComparisonAttribute ?? InvalidNamePlaceholder;

return condition.Comparator switch
{
UserComparator.IsOneOf or
UserComparator.IsNotOneOf or
UserComparator.ContainsAnyOf or
UserComparator.NotContainsAnyOf or
UserComparator.TextIsOneOf or
UserComparator.TextIsNotOneOf or
UserComparator.TextContainsAnyOf or
UserComparator.TextNotContainsAnyOf or
UserComparator.SemVerIsOneOf or
UserComparator.SemVerIsNotOneOf or
UserComparator.TextStartsWithAnyOf or
Expand All @@ -78,50 +76,54 @@ UserComparator.TextEndsWithAnyOf or
UserComparator.TextNotEndsWithAnyOf or
UserComparator.ArrayContainsAnyOf or
UserComparator.ArrayNotContainsAnyOf =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.StringListValue, isSensitive: false),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.StringListValue, isSensitive: false),

UserComparator.SemVerLess or
UserComparator.SemVerLessOrEquals or
UserComparator.SemVerGreater or
UserComparator.SemVerGreaterOrEquals or
UserComparator.TextEquals or
UserComparator.TextNotEquals =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.StringValue, isSensitive: false),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.StringValue, isSensitive: false),

UserComparator.NumberEquals or
UserComparator.NumberNotEquals or
UserComparator.NumberLess or
UserComparator.NumberLessOrEquals or
UserComparator.NumberGreater or
UserComparator.NumberGreaterOrEquals =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.DoubleValue),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.DoubleValue),

UserComparator.SensitiveIsOneOf or
UserComparator.SensitiveIsNotOneOf or
UserComparator.SensitiveTextIsOneOf or
UserComparator.SensitiveTextIsNotOneOf or
UserComparator.SensitiveTextStartsWithAnyOf or
UserComparator.SensitiveTextNotStartsWithAnyOf or
UserComparator.SensitiveTextEndsWithAnyOf or
UserComparator.SensitiveTextNotEndsWithAnyOf or
UserComparator.SensitiveArrayContainsAnyOf or
UserComparator.SensitiveArrayNotContainsAnyOf =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.StringListValue, isSensitive: true),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.StringListValue, isSensitive: true),

UserComparator.DateTimeBefore or
UserComparator.DateTimeAfter =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.DoubleValue, isDateTime: true),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.DoubleValue, isDateTime: true),

UserComparator.SensitiveTextEquals or
UserComparator.SensitiveTextNotEquals =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.StringValue, isSensitive: true),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.StringValue, isSensitive: true),

_ =>
builder.AppendUserCondition(condition.ComparisonAttribute, condition.Comparator, condition.GetComparisonValue(throwIfInvalid: false)),
builder.AppendUserCondition(comparisonAttribute, condition.Comparator, condition.GetComparisonValue(throwIfInvalid: false)),
};
}

public static IndentedTextBuilder AppendPrerequisiteFlagCondition(this IndentedTextBuilder builder, PrerequisiteFlagCondition condition)
public static IndentedTextBuilder AppendPrerequisiteFlagCondition(this IndentedTextBuilder builder, PrerequisiteFlagCondition condition, IReadOnlyDictionary<string, Setting>? settings = null)
{
var prerequisiteFlagKey = condition.PrerequisiteFlagKey ?? InvalidReferencePlaceholder;
var prerequisiteFlagKey =
condition.PrerequisiteFlagKey is null ? InvalidNamePlaceholder :
settings is not null && !settings.ContainsKey(condition.PrerequisiteFlagKey) ? InvalidReferencePlaceholder :
condition.PrerequisiteFlagKey;

var comparator = condition.Comparator;
var comparisonValue = condition.ComparisonValue.GetValue(throwIfInvalid: false);

Expand All @@ -133,15 +135,22 @@ public static IndentedTextBuilder AppendSegmentCondition(this IndentedTextBuilde
var segment = condition.Segment;
var comparator = condition.Comparator;

var segmentName = segment?.Name ??
(segment is null ? InvalidReferencePlaceholder : InvalidNamePlaceholder);
var segmentName =
segment is null ? InvalidReferencePlaceholder :
segment.Name is not { Length: > 0 } ? InvalidNamePlaceholder :
segment.Name;

return builder.Append($"User {comparator.ToDisplayText()} '{segmentName}'");
}

public static IndentedTextBuilder AppendConditionResult(this IndentedTextBuilder builder, bool result)
{
return builder.Append(result ? "true" : "false");
}

public static IndentedTextBuilder AppendConditionConsequence(this IndentedTextBuilder builder, bool result)
{
builder.Append(" => ").AppendEvaluationResult(result);
builder.Append(" => ").AppendConditionResult(result);
return result ? builder : builder.Append(", skipping the remaining AND conditions");
}

Expand Down Expand Up @@ -303,10 +312,10 @@ public static string ToDisplayText(this UserComparator comparator)
{
return comparator switch
{
UserComparator.IsOneOf or UserComparator.SensitiveIsOneOf or UserComparator.SemVerIsOneOf => "IS ONE OF",
UserComparator.IsNotOneOf or UserComparator.SensitiveIsNotOneOf or UserComparator.SemVerIsNotOneOf => "IS NOT ONE OF",
UserComparator.ContainsAnyOf => "CONTAINS ANY OF",
UserComparator.NotContainsAnyOf => "NOT CONTAINS ANY OF",
UserComparator.TextIsOneOf or UserComparator.SensitiveTextIsOneOf or UserComparator.SemVerIsOneOf => "IS ONE OF",
UserComparator.TextIsNotOneOf or UserComparator.SensitiveTextIsNotOneOf or UserComparator.SemVerIsNotOneOf => "IS NOT ONE OF",
UserComparator.TextContainsAnyOf => "CONTAINS ANY OF",
UserComparator.TextNotContainsAnyOf => "NOT CONTAINS ANY OF",
UserComparator.SemVerLess or UserComparator.NumberLess => "<",
UserComparator.SemVerLessOrEquals or UserComparator.NumberLessOrEquals => "<=",
UserComparator.SemVerGreater or UserComparator.NumberGreater => ">",
Expand Down
Loading

0 comments on commit ed0a54d

Please sign in to comment.