From 00cdcc4b400fa9875f07a774516b869b1d044986 Mon Sep 17 00:00:00 2001 From: adams85 <31276480+adams85@users.noreply.github.com> Date: Tue, 28 Nov 2023 14:57:41 +0100 Subject: [PATCH] Minor improvements and fixes (#84) * Sync intellisense docs with SDK reference * Ensure non-nullability of FormattableLogMessage.ArgNames and ArgValues for all cases (including default instances) * Make deserialization of flag override json files tolerant to comments and trailing commas in builds other than .NET 4.5 as well --- src/ConfigCat.Client.Tests/OverrideTests.cs | 63 +++++++++++++++++++ .../Evaluation/EvaluationDetails.cs | 4 +- .../Extensions/SerializationExtensions.cs | 18 +++--- .../Logging/FormattableLogMessage.cs | 19 +++--- .../Override/LocalFileDataSource.cs | 4 +- src/ConfigCatClient/User.cs | 18 +++--- 6 files changed, 96 insertions(+), 30 deletions(-) diff --git a/src/ConfigCat.Client.Tests/OverrideTests.cs b/src/ConfigCat.Client.Tests/OverrideTests.cs index 83be4c10..1ba1f9c1 100644 --- a/src/ConfigCat.Client.Tests/OverrideTests.cs +++ b/src/ConfigCat.Client.Tests/OverrideTests.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Linq; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; #if USE_NEWTONSOFT_JSON using JsonValue = Newtonsoft.Json.Linq.JValue; @@ -473,6 +475,67 @@ public async Task LocalFile_Watcher_Reload_Sync() File.Delete(SampleFileToCreate); } + [TestMethod] + public async Task LocalFile_TolerantJsonParsing_SimplifiedConfig() + { + const string key = "flag"; + const bool expectedEvaluatedValue = true; + var overrideValue = expectedEvaluatedValue.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(); + + var filePath = Path.GetTempFileName(); + File.WriteAllText(filePath, $"{{ \"flags\": {{ \"{key}\": {overrideValue} }}, /* comment */ }}"); + + try + { + using var client = ConfigCatClient.Get("localhost", options => + { + options.PollingMode = PollingModes.ManualPoll; + options.FlagOverrides = FlagOverrides.LocalFile(filePath, autoReload: false, OverrideBehaviour.LocalOnly); + }); + var actualEvaluatedValue = await client.GetValueAsync(key, null); + + Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue); + } + finally + { + if (File.Exists(filePath)) + { + File.Delete(filePath); + } + } + } + + [TestMethod] + public async Task LocalFile_TolerantJsonParsing_ComplexConfig() + { + const string key = "flag"; + const bool expectedEvaluatedValue = true; + var overrideValue = expectedEvaluatedValue.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(); + var settingType = ((int)SettingType.Boolean).ToString(CultureInfo.InvariantCulture); + + var filePath = Path.GetTempFileName(); + File.WriteAllText(filePath, $"{{ \"f\": {{ \"{key}\": {{ \"t\": {settingType}, \"v\": {{ \"b\": {overrideValue} }} }} }}, /* comment */ }}"); + + try + { + using var client = ConfigCatClient.Get("localhost", options => + { + options.PollingMode = PollingModes.ManualPoll; + options.FlagOverrides = FlagOverrides.LocalFile(filePath, autoReload: false, OverrideBehaviour.LocalOnly); + }); + var actualEvaluatedValue = await client.GetValueAsync(key, null); + + Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue); + } + finally + { + if (File.Exists(filePath)) + { + File.Delete(filePath); + } + } + } + [DataRow(true, false, true)] [DataRow(true, "", "")] [DataRow(true, 0, 0)] diff --git a/src/ConfigCatClient/Evaluation/EvaluationDetails.cs b/src/ConfigCatClient/Evaluation/EvaluationDetails.cs index 2349e348..4ee38bf0 100644 --- a/src/ConfigCatClient/Evaluation/EvaluationDetails.cs +++ b/src/ConfigCatClient/Evaluation/EvaluationDetails.cs @@ -95,12 +95,12 @@ private protected EvaluationDetails(string key) public Exception? ErrorException { get; set; } /// - /// The targeting rule which was used to select the evaluated value (if any). + /// The targeting rule (if any) that matched during the evaluation and was used to return the evaluated value. /// public ITargetingRule? MatchedTargetingRule { get; set; } /// - /// The percentage option which was used to select the evaluated value (if any). + /// The percentage option (if any) that was used to select the evaluated value. /// public IPercentageOption? MatchedPercentageOption { get; set; } } diff --git a/src/ConfigCatClient/Extensions/SerializationExtensions.cs b/src/ConfigCatClient/Extensions/SerializationExtensions.cs index 2f3d3e86..9c483ccb 100644 --- a/src/ConfigCatClient/Extensions/SerializationExtensions.cs +++ b/src/ConfigCatClient/Extensions/SerializationExtensions.cs @@ -14,34 +14,36 @@ internal static class SerializationExtensions #if USE_NEWTONSOFT_JSON private static readonly JsonSerializer Serializer = JsonSerializer.Create(); #else - private static readonly JsonSerializerOptions SerializerOptions = new() + private static readonly JsonSerializerOptions TolerantSerializerOptions = new() { + AllowTrailingCommas = true, + ReadCommentHandling = JsonCommentHandling.Skip, Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; #endif - public static T? Deserialize(this string json) => json.AsMemory().Deserialize(); + public static T? Deserialize(this string json, bool tolerant = false) => json.AsMemory().Deserialize(tolerant); // NOTE: It would be better to use ReadOnlySpan, however when the full string is wrapped in a span, json.ToString() result in a copy of the string. // This is not the case with ReadOnlyMemory, so we use that until support for .NET 4.5 support is dropped. - public static T? Deserialize(this ReadOnlyMemory json) + public static T? Deserialize(this ReadOnlyMemory json, bool tolerant = false) { #if USE_NEWTONSOFT_JSON using var stringReader = new StringReader(json.ToString()); using var reader = new JsonTextReader(stringReader); return Serializer.Deserialize(reader); #else - return JsonSerializer.Deserialize(json.Span); + return JsonSerializer.Deserialize(json.Span, tolerant ? TolerantSerializerOptions : null); #endif } - public static T? DeserializeOrDefault(this string json) => json.AsMemory().DeserializeOrDefault(); + public static T? DeserializeOrDefault(this string json, bool tolerant = false) => json.AsMemory().DeserializeOrDefault(tolerant); - public static T? DeserializeOrDefault(this ReadOnlyMemory json) + public static T? DeserializeOrDefault(this ReadOnlyMemory json, bool tolerant = false) { try { - return json.Deserialize(); + return json.Deserialize(tolerant); } catch { @@ -54,7 +56,7 @@ public static string Serialize(this T objectToSerialize) #if USE_NEWTONSOFT_JSON return JsonConvert.SerializeObject(objectToSerialize); #else - return JsonSerializer.Serialize(objectToSerialize, SerializerOptions); + return JsonSerializer.Serialize(objectToSerialize, TolerantSerializerOptions); #endif } } diff --git a/src/ConfigCatClient/Logging/FormattableLogMessage.cs b/src/ConfigCatClient/Logging/FormattableLogMessage.cs index 0a147c2c..c84b1b64 100644 --- a/src/ConfigCatClient/Logging/FormattableLogMessage.cs +++ b/src/ConfigCatClient/Logging/FormattableLogMessage.cs @@ -21,6 +21,11 @@ internal static FormattableLogMessage FromInterpolated(FormattableString message message.GetArguments() ?? ArrayUtils.EmptyArray()); } + private readonly string? format; + private readonly string[]? argNames; + private readonly object?[]? argValues; + private string? invariantFormattedMessage; + /// /// Initializes a new instance of the struct from a plain log message. /// @@ -28,8 +33,8 @@ public FormattableLogMessage(string message) { this.invariantFormattedMessage = message ?? throw new ArgumentNullException(nameof(message)); this.format = null; - ArgNames = ArrayUtils.EmptyArray(); - ArgValues = ArrayUtils.EmptyArray(); + this.argNames = null; + this.argValues = null; } /// @@ -38,8 +43,8 @@ public FormattableLogMessage(string message) public FormattableLogMessage(string format, string[] argNames, object?[] argValues) { this.format = format ?? throw new ArgumentNullException(nameof(format)); - ArgNames = argNames ?? throw new ArgumentNullException(nameof(argNames)); - ArgValues = argValues ?? throw new ArgumentNullException(nameof(argValues)); + this.argNames = argNames ?? throw new ArgumentNullException(nameof(argNames)); + this.argValues = argValues ?? throw new ArgumentNullException(nameof(argValues)); if (argNames.Length != argValues.Length) { throw new ArgumentException($"Number of argument names ({argNames.Length}) and argument values ({argValues.Length}) mismatch.", nameof(argNames)); @@ -47,7 +52,6 @@ public FormattableLogMessage(string format, string[] argNames, object?[] argValu this.invariantFormattedMessage = null; } - private readonly string? format; /// /// Log message format. /// @@ -56,14 +60,13 @@ public FormattableLogMessage(string format, string[] argNames, object?[] argValu /// /// Names of the named arguments. /// - public readonly string[] ArgNames { get; } + public readonly string[] ArgNames => this.argNames ?? ArrayUtils.EmptyArray(); /// /// Values of the named arguments. /// - public readonly object?[] ArgValues { get; } + public readonly object?[] ArgValues => this.argValues ?? ArrayUtils.EmptyArray(); - private string? invariantFormattedMessage; /// /// The log message formatted using . /// diff --git a/src/ConfigCatClient/Override/LocalFileDataSource.cs b/src/ConfigCatClient/Override/LocalFileDataSource.cs index 33f3bb6e..e089175c 100644 --- a/src/ConfigCatClient/Override/LocalFileDataSource.cs +++ b/src/ConfigCatClient/Override/LocalFileDataSource.cs @@ -104,14 +104,14 @@ private async Task ReloadFileAsync(bool isAsync, CancellationToken cancellationT try { var content = File.ReadAllText(this.fullPath); - var simplified = content.DeserializeOrDefault(); + var simplified = content.DeserializeOrDefault(tolerant: true); if (simplified?.Entries is not null) { this.overrideValues = simplified.Entries.ToDictionary(kv => kv.Key, kv => kv.Value.ToSetting()); break; } - var deserialized = content.Deserialize() + var deserialized = content.Deserialize(tolerant: true) ?? throw new InvalidOperationException("Invalid config JSON content: " + content); this.overrideValues = deserialized.Settings; break; diff --git a/src/ConfigCatClient/User.cs b/src/ConfigCatClient/User.cs index f49ab126..1db63f8e 100644 --- a/src/ConfigCatClient/User.cs +++ b/src/ConfigCatClient/User.cs @@ -41,39 +41,37 @@ public class User /// Custom attributes of the user for advanced targeting rule definitions (e.g. user role, subscription type, etc.) /// /// - /// The set of allowed attribute values depends on the comparison type of the condition which references the User Object attribute.
- /// values are supported by all comparison types (in some cases they need to be provided in a specific format though).
- /// Some of the comparison types work with other types of values, as described below. + /// All comparators support values as User Object attribute (in some cases they need to be provided in a specific format though, see below),
+ /// but some of them also support other types of values. It depends on the comparator how the values will be handled. The following rules apply: /// - /// Text-based comparisons (EQUALS, IS ONE OF, etc.)
+ /// Text-based comparators (EQUALS, IS ONE OF, etc.)
/// * accept values,
- /// * all other values are automatically converted to string (a warning will be logged but evaluation will continue as normal). + /// * all other values are automatically converted to (a warning will be logged but evaluation will continue as normal). /// /// - /// SemVer-based comparisons (IS ONE OF, <, >=, etc.)
+ /// SemVer-based comparators (IS ONE OF, <, >=, etc.)
/// * accept values containing a properly formatted, valid semver value,
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped). /// /// - /// Number-based comparisons (=, <, >=, etc.)
+ /// Number-based comparators (=, <, >=, etc.)
/// * accept values and all other numeric values which can safely be converted to ,
/// * accept values containing a properly formatted, valid value,
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped). /// /// - /// Date time-based comparisons (BEFORE / AFTER)
+ /// Date time-based comparators (BEFORE / AFTER)
/// * accept or values, which are automatically converted to a second-based Unix timestamp,
/// * accept values representing a second-based Unix timestamp and all other numeric values which can safely be converted to ,
/// * accept values containing a properly formatted, valid value,
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped). /// /// - /// String array-based comparisons (ARRAY CONTAINS ANY OF / ARRAY NOT CONTAINS ANY OF)
+ /// String array-based comparators (ARRAY CONTAINS ANY OF / ARRAY NOT CONTAINS ANY OF)
/// * accept arrays of ,
/// * accept values containing a valid JSON string which can be deserialized to an array of ,
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped). /// - /// In case a non-string attribute value needs to be converted to during evaluation, it will always be done using the same format which is accepted by the comparisons. /// public IDictionary Custom {