Skip to content

Commit

Permalink
Improve error handling consistency in prerequisite flag evaluation
Browse files Browse the repository at this point in the history
  • Loading branch information
@adams85
adams85 committed Nov 15, 2023
1 parent ce15a57 commit c148867
Show file tree
Hide file tree
Showing 9 changed files with 119 additions and 154 deletions.
107 changes: 77 additions & 30 deletions src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,49 +222,96 @@ public void UnicodeMatrixTests(string configLocation, string settingKey, string
userId, userEmail, userCountry, userCustomAttributeName, userCustomAttributeValue);
}

[TestMethod]
public void CircularDependencyTest()
[DataTestMethod]
[DataRow("key1", "'key1' -> 'key1'")]
[DataRow("key2", "'key2' -> 'key3' -> 'key2'")]
[DataRow("key4", "'key4' -> 'key3' -> 'key2' -> 'key3'")]
public void PrerequisiteFlagCircularDependencyTest(string key, string dependencyCycle)
{
var config = new ConfigLocation.LocalFile("data", "test_circulardependency_v6.json").FetchConfig();

var logEvents = new List<LogEvent>();
var logger = LoggingHelper.CreateCapturingLogger(logEvents);

var logger = new Mock<IConfigCatLogger>().Object.AsWrapper();
var evaluator = new RolloutEvaluator(logger);

const string key = "key1";
var evaluationDetails = evaluator.Evaluate<object?>(config!.Settings, key, defaultValue: null, user: null, remoteConfig: null, logger);

Assert.AreEqual(4, logEvents.Count);
var ex = Assert.ThrowsException<InvalidOperationException>(() => evaluator.Evaluate<object?>(config!.Settings, key, defaultValue: null, user: null, remoteConfig: null, logger));

Assert.AreEqual(3, logEvents.Count(evt => evt.EventId == 3005));
StringAssert.Contains(ex.Message, "Circular dependency detected");
StringAssert.Contains(ex.Message, dependencyCycle);
}

Assert.IsTrue(logEvents.Any(evt => evt.Level == LogLevel.Warning
&& (string?)evt.Message.ArgValues[1] == "key1"
&& (string?)evt.Message.ArgValues[2] == "'key1' -> 'key1'"));
[DataTestMethod]
[DataRow("stringDependsOnBool", "mainBoolFlag", true, "Dog")]
[DataRow("stringDependsOnBool", "mainBoolFlag", false, "Cat")]
[DataRow("stringDependsOnBool", "mainBoolFlag", "1", null)]
[DataRow("stringDependsOnBool", "mainBoolFlag", 1, null)]
[DataRow("stringDependsOnBool", "mainBoolFlag", 1.0, null)]
[DataRow("stringDependsOnBool", "mainBoolFlag", new[] { true }, null)]
[DataRow("stringDependsOnBool", "mainBoolFlag", null, null)]
[DataRow("stringDependsOnString", "mainStringFlag", "private", "Dog")]
[DataRow("stringDependsOnString", "mainStringFlag", "Private", "Cat")]
[DataRow("stringDependsOnString", "mainStringFlag", true, null)]
[DataRow("stringDependsOnString", "mainStringFlag", 1, null)]
[DataRow("stringDependsOnString", "mainStringFlag", 1.0, null)]
[DataRow("stringDependsOnString", "mainStringFlag", new[] { "private" }, null)]
[DataRow("stringDependsOnString", "mainStringFlag", null, null)]
[DataRow("stringDependsOnInt", "mainIntFlag", 2, "Dog")]
[DataRow("stringDependsOnInt", "mainIntFlag", 1, "Cat")]
[DataRow("stringDependsOnInt", "mainIntFlag", "2", null)]
[DataRow("stringDependsOnInt", "mainIntFlag", true, null)]
[DataRow("stringDependsOnInt", "mainIntFlag", 2.0, null)]
[DataRow("stringDependsOnInt", "mainIntFlag", new[] { 2 }, null)]
[DataRow("stringDependsOnInt", "mainIntFlag", null, null)]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", 0.1, "Dog")]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", 0.11, "Cat")]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", "0.1", null)]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", true, null)]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", 1, null)]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", new[] { 0.1 }, null)]
[DataRow("stringDependsOnDouble", "mainDoubleFlag", null, null)]
public async Task PrerequisiteFlagComparisonValueTypeMismatchTest(string key, string prerequisiteFlagKey, object? prerequisiteFlagValue, object? expectedValue)
{
var cdnLocation = (ConfigLocation.Cdn)new FlagDependencyMatrixTestsDescriptor().ConfigLocation;

Assert.IsTrue(logEvents.Any(evt => evt.Level == LogLevel.Warning
&& (string?)evt.Message.ArgValues[1] == "key2"
&& (string?)evt.Message.ArgValues[2] == "'key1' -> 'key2' -> 'key1'"));
var logEvents = new List<LogEvent>();
var logger = LoggingHelper.CreateCapturingLogger(logEvents);

Assert.IsTrue(logEvents.Any(evt => evt.Level == LogLevel.Warning
&& (string?)evt.Message.ArgValues[1] == "key3"
&& (string?)evt.Message.ArgValues[2] == "'key1' -> 'key3' -> 'key3'"));
var overrideDictionary = new Dictionary<string, object> { [prerequisiteFlagKey] = prerequisiteFlagValue! };

var evaluateLogEvent = logEvents.FirstOrDefault(evt => evt.Level == LogLevel.Info && evt.EventId == 5000);
Assert.IsNotNull(evaluateLogEvent);
var options = new ConfigCatClientOptions
{
FlagOverrides = FlagOverrides.LocalDictionary(overrideDictionary, OverrideBehaviour.LocalOverRemote),
PollingMode = PollingModes.ManualPoll,
Logger = logger
};
cdnLocation.ConfigureBaseUrl(options);

StringAssert.Matches((string?)evaluateLogEvent.Message.ArgValues[0], new Regex(
"THEN 'key1-prereq1' => " + Regex.Escape(RolloutEvaluator.CircularDependencyError) + Environment.NewLine
+ @"\s+" + Regex.Escape(RolloutEvaluator.TargetingRuleIgnoredMessage)));
using var client = new ConfigCatClient(cdnLocation.SdkKey, options);
await client.ForceRefreshAsync();

StringAssert.Matches((string?)evaluateLogEvent.Message.ArgValues[0], new Regex(
"THEN 'key2-prereq1' => " + Regex.Escape(RolloutEvaluator.CircularDependencyError) + Environment.NewLine
+ @"\s+" + Regex.Escape(RolloutEvaluator.TargetingRuleIgnoredMessage)));
var actualValue = await client.GetValueAsync(key, (object?)null);
Assert.AreEqual(expectedValue, actualValue);

StringAssert.Matches((string?)evaluateLogEvent.Message.ArgValues[0], new Regex(
"THEN 'key3-prereq1' => " + Regex.Escape(RolloutEvaluator.CircularDependencyError) + Environment.NewLine
+ @"\s+" + Regex.Escape(RolloutEvaluator.TargetingRuleIgnoredMessage)));
if (expectedValue is null)
{
var errors = logEvents.Where(evt => evt.Level == LogLevel.Error).ToArray();
Assert.AreEqual(1, errors.Length);
Assert.AreEqual(1002, errors[0].EventId);
var ex = errors[0].Exception;
Assert.IsInstanceOfType(ex, typeof(InvalidOperationException));

if (prerequisiteFlagValue == null)
{
StringAssert.Contains(ex!.Message, "Setting value is null");
}
else if (prerequisiteFlagValue.GetType().ToSettingType() == Setting.UnknownType)
{
StringAssert.Matches(ex!.Message, new Regex("Setting value '[^']+' is of an unsupported type"));
}
else
{
StringAssert.Matches(ex!.Message, new Regex("Type mismatch between comparison value '[^']+' and prerequisite flag '[^']+'"));
}
}
}

[DataTestMethod]
Expand Down
14 changes: 2 additions & 12 deletions src/ConfigCat.Client.Tests/EvaluationLogTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,6 @@ public void ComparatorsTests(string testSetName, string? sdkKey, string? baseUrl
RunTest(testSetName, sdkKey, baseUrlOrOverrideFileName, key, defaultValue, userObject, expectedReturnValue, expectedLogFileName);
}

private static IEnumerable<object?[]> GetPrerequisiteFlagConditionsWithCircularDependencyTests() => GetTests("circular_dependency");

[DataTestMethod]
[DynamicData(nameof(GetPrerequisiteFlagConditionsWithCircularDependencyTests), DynamicDataSourceType.Method)]
public void PrerequisiteFlagConditionsWithCircularDependencyTests(string testSetName, string? sdkKey, string? baseUrlOrOverrideFileName,
string key, string? defaultValue, string userObject, string? expectedReturnValue, string expectedLogFileName)
{
RunTest(testSetName, sdkKey, baseUrlOrOverrideFileName, key, defaultValue, userObject, expectedReturnValue, expectedLogFileName);
}

private static IEnumerable<object?[]> GetEpochDateValidationTests() => GetTests("epoch_date_validation");

[DataTestMethod]
Expand Down Expand Up @@ -243,7 +233,7 @@ private static void RunTest(string testSetName, string? sdkKey, string? baseUrlO
}

var logEvents = new List<LogEvent>();
var logger = LoggingHelper.CreateCapturingLogger(logEvents);
var logger = LoggingHelper.CreateCapturingLogger(logEvents).AsWrapper();

ConfigLocation configLocation = sdkKey is { Length: > 0 }
? new ConfigLocation.Cdn(sdkKey, baseUrlOrOverrideFileName)
Expand Down Expand Up @@ -313,7 +303,7 @@ public void EvaluationLogShouldBeBuiltOnlyWhenNecessary(LogLevel logLevel, bool
var settings = new ConfigLocation.Cdn("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/AG6C1ngVb0CvM07un6JisQ").FetchConfigCached().Settings;

var logEvents = new List<LogEvent>();
var logger = LoggingHelper.CreateCapturingLogger(logEvents, logLevel);
var logger = LoggingHelper.CreateCapturingLogger(logEvents, logLevel).AsWrapper();

var evaluator = new RolloutEvaluator(logger);

Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCat.Client.Tests/Helpers/LoggingHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static LoggerWrapper AsWrapper(this IConfigCatLogger logger, Hooks? hooks
return new LoggerWrapper(logger, hooks);
}

public static LoggerWrapper CreateCapturingLogger(List<LogEvent> logEvents, LogLevel logLevel = LogLevel.Info)
public static IConfigCatLogger CreateCapturingLogger(List<LogEvent> logEvents, LogLevel logLevel = LogLevel.Info)
{
var loggerMock = new Mock<IConfigCatLogger>();

Expand All @@ -33,6 +33,6 @@ public static LoggerWrapper CreateCapturingLogger(List<LogEvent> logEvents, LogL
loggerMock.Setup(logger => logger.Log(It.IsAny<LogLevel>(), It.IsAny<LogEventId>(), ref It.Ref<FormattableLogMessage>.IsAny, It.IsAny<Exception>()))
.Callback(delegate (LogLevel level, LogEventId eventId, ref FormattableLogMessage msg, Exception ex) { logEvents.Add(new LogEvent(level, eventId, ref msg, ex)); });

return loggerMock.Object.AsWrapper();
return loggerMock.Object;
}
}
28 changes: 0 additions & 28 deletions src/ConfigCat.Client.Tests/data/evaluationlog/_overrides/circular_dependency_override.json
View file

This file was deleted.

14 changes: 0 additions & 14 deletions src/ConfigCat.Client.Tests/data/evaluationlog/circular_dependency.json
View file

This file was deleted.

21 changes: 0 additions & 21 deletions src/ConfigCat.Client.Tests/data/evaluationlog/circular_dependency/circular_dependency.txt
View file

This file was deleted.

48 changes: 21 additions & 27 deletions src/ConfigCat.Client.Tests/data/test_circulardependency_v6.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,79 +6,73 @@
"f": {
"key1": {
"t": 1,
"v": { "s": "value1" },
"v": { "s": "key1-value" },
"r": [
{
"c": [
{
"p": {
"f": "key1",
"c": 0,
"v": { "s": "key1-prereq1" }
"v": { "s": "key1-prereq" }
}
}
],
"s": { "v": { "s": "key1-prereq1" } }
},
{
"c": [
{
"p": {
"f": "key2",
"c": 0,
"v": { "s": "key1-prereq2" }
}
}
],
"s": { "v": { "s": "key1-prereq2" } }
},
"s": { "v": { "s": "key1-prereq" } }
}
]
},
"key2": {
"t": 1,
"v": { "s": "key2-value" },
"r": [
{
"c": [
{
"p": {
"f": "key3",
"c": 0,
"v": { "s": "key1-prereq3" }
"v": { "s": "key3-prereq" }
}
}
],
"s": { "v": { "s": "key1-prereq3" } }
"s": { "v": { "s": "key2-prereq" } }
}
]
},
"key2": {
"key3": {
"t": 1,
"v": { "s": "value2" },
"v": { "s": "key3-value" },
"r": [
{
"c": [
{
"p": {
"f": "key1",
"f": "key2",
"c": 0,
"v": { "s": "key2-prereq1" }
"v": { "s": "key2-prereq" }
}
}
],
"s": { "v": { "s": "key2-prereq1" } }
"s": { "v": { "s": "key3-prereq" } }
}
]
},
"key3": {
"key4": {
"t": 1,
"v": { "s": "value3" },
"v": { "s": "key4-value" },
"r": [
{
"c": [
{
"p": {
"f": "key3",
"c": 0,
"v": { "s": "key3-prereq1" }
"v": { "s": "key3-prereq" }
}
}
],
"s": { "v": { "s": "key3-prereq1" } }
"s": { "v": { "s": "key4-prereq" } }
}
]
}
Expand Down
Loading

0 comments on commit c148867

Please sign in to comment.