Skip to content

Commit

Permalink
Minor improvements and fixes (#84)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
adams85 authored Nov 28, 2023
1 parent 70c1d69 commit 00cdcc4
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 30 deletions.
63 changes: 63 additions & 0 deletions src/ConfigCat.Client.Tests/OverrideTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<bool?>(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<bool?>(key, null);

Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue);
}
finally
{
if (File.Exists(filePath))
{
File.Delete(filePath);
}
}
}

[DataRow(true, false, true)]
[DataRow(true, "", "")]
[DataRow(true, 0, 0)]
Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCatClient/Evaluation/EvaluationDetails.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ private protected EvaluationDetails(string key)
public Exception? ErrorException { get; set; }

/// <summary>
/// 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.
/// </summary>
public ITargetingRule? MatchedTargetingRule { get; set; }

/// <summary>
/// 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.
/// </summary>
public IPercentageOption? MatchedPercentageOption { get; set; }
}
Expand Down
18 changes: 10 additions & 8 deletions src/ConfigCatClient/Extensions/SerializationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(this string json) => json.AsMemory().Deserialize<T>();
public static T? Deserialize<T>(this string json, bool tolerant = false) => json.AsMemory().Deserialize<T>(tolerant);

// NOTE: It would be better to use ReadOnlySpan<char>, 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<char>, so we use that until support for .NET 4.5 support is dropped.
public static T? Deserialize<T>(this ReadOnlyMemory<char> json)
public static T? Deserialize<T>(this ReadOnlyMemory<char> json, bool tolerant = false)
{
#if USE_NEWTONSOFT_JSON
using var stringReader = new StringReader(json.ToString());
using var reader = new JsonTextReader(stringReader);
return Serializer.Deserialize<T>(reader);
#else
return JsonSerializer.Deserialize<T>(json.Span);
return JsonSerializer.Deserialize<T>(json.Span, tolerant ? TolerantSerializerOptions : null);
#endif
}

public static T? DeserializeOrDefault<T>(this string json) => json.AsMemory().DeserializeOrDefault<T>();
public static T? DeserializeOrDefault<T>(this string json, bool tolerant = false) => json.AsMemory().DeserializeOrDefault<T>(tolerant);

public static T? DeserializeOrDefault<T>(this ReadOnlyMemory<char> json)
public static T? DeserializeOrDefault<T>(this ReadOnlyMemory<char> json, bool tolerant = false)
{
try
{
return json.Deserialize<T>();
return json.Deserialize<T>(tolerant);
}
catch
{
Expand All @@ -54,7 +56,7 @@ public static string Serialize<T>(this T objectToSerialize)
#if USE_NEWTONSOFT_JSON
return JsonConvert.SerializeObject(objectToSerialize);
#else
return JsonSerializer.Serialize(objectToSerialize, SerializerOptions);
return JsonSerializer.Serialize(objectToSerialize, TolerantSerializerOptions);
#endif
}
}
19 changes: 11 additions & 8 deletions src/ConfigCatClient/Logging/FormattableLogMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,20 @@ internal static FormattableLogMessage FromInterpolated(FormattableString message
message.GetArguments() ?? ArrayUtils.EmptyArray<object?>());
}

private readonly string? format;
private readonly string[]? argNames;
private readonly object?[]? argValues;
private string? invariantFormattedMessage;

/// <summary>
/// Initializes a new instance of the <see cref="FormattableLogMessage"/> struct from a plain log message.
/// </summary>
public FormattableLogMessage(string message)
{
this.invariantFormattedMessage = message ?? throw new ArgumentNullException(nameof(message));
this.format = null;
ArgNames = ArrayUtils.EmptyArray<string>();
ArgValues = ArrayUtils.EmptyArray<object?>();
this.argNames = null;
this.argValues = null;
}

/// <summary>
Expand All @@ -38,16 +43,15 @@ 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));
}
this.invariantFormattedMessage = null;
}

private readonly string? format;
/// <summary>
/// Log message format.
/// </summary>
Expand All @@ -56,14 +60,13 @@ public FormattableLogMessage(string format, string[] argNames, object?[] argValu
/// <summary>
/// Names of the named arguments.
/// </summary>
public readonly string[] ArgNames { get; }
public readonly string[] ArgNames => this.argNames ?? ArrayUtils.EmptyArray<string>();

/// <summary>
/// Values of the named arguments.
/// </summary>
public readonly object?[] ArgValues { get; }
public readonly object?[] ArgValues => this.argValues ?? ArrayUtils.EmptyArray<object?>();

private string? invariantFormattedMessage;
/// <summary>
/// The log message formatted using <see cref="CultureInfo.InvariantCulture"/>.
/// </summary>
Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCatClient/Override/LocalFileDataSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ private async Task ReloadFileAsync(bool isAsync, CancellationToken cancellationT
try
{
var content = File.ReadAllText(this.fullPath);
var simplified = content.DeserializeOrDefault<SimplifiedConfig>();
var simplified = content.DeserializeOrDefault<SimplifiedConfig>(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<Config>()
var deserialized = content.Deserialize<Config>(tolerant: true)
?? throw new InvalidOperationException("Invalid config JSON content: " + content);
this.overrideValues = deserialized.Settings;
break;
Expand Down
18 changes: 8 additions & 10 deletions src/ConfigCatClient/User.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,39 +41,37 @@ public class User
/// Custom attributes of the user for advanced targeting rule definitions (e.g. user role, subscription type, etc.)
/// </summary>
/// <remarks>
/// The set of allowed attribute values depends on the comparison type of the condition which references the User Object attribute.<br/>
/// <see cref="string"/> values are supported by all comparison types (in some cases they need to be provided in a specific format though).<br/>
/// Some of the comparison types work with other types of values, as described below.
/// All comparators support <see cref="string"/> values as User Object attribute (in some cases they need to be provided in a specific format though, see below),<br/>
/// 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:
/// <para>
/// Text-based comparisons (EQUALS, IS ONE OF, etc.)<br/>
/// Text-based comparators (EQUALS, IS ONE OF, etc.)<br/>
/// * accept <see cref="string"/> values,<br/>
/// * 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 <see cref="string"/> (a warning will be logged but evaluation will continue as normal).
/// </para>
/// <para>
/// SemVer-based comparisons (IS ONE OF, &lt;, &gt;=, etc.)<br/>
/// SemVer-based comparators (IS ONE OF, &lt;, &gt;=, etc.)<br/>
/// * accept <see cref="string"/> values containing a properly formatted, valid semver value,<br/>
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped).
/// </para>
/// <para>
/// Number-based comparisons (=, &lt;, &gt;=, etc.)<br/>
/// Number-based comparators (=, &lt;, &gt;=, etc.)<br/>
/// * accept <see cref="double"/> values and all other numeric values which can safely be converted to <see cref="double"/>,<br/>
/// * accept <see cref="string"/> values containing a properly formatted, valid <see cref="double"/> value,<br/>
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped).
/// </para>
/// <para>
/// Date time-based comparisons (BEFORE / AFTER)<br/>
/// Date time-based comparators (BEFORE / AFTER)<br/>
/// * accept <see cref="DateTime"/> or <see cref="DateTimeOffset"/> values, which are automatically converted to a second-based Unix timestamp,<br/>
/// * accept <see cref="double"/> values representing a second-based Unix timestamp and all other numeric values which can safely be converted to <see cref="double"/>,<br/>
/// * accept <see cref="string"/> values containing a properly formatted, valid <see cref="double"/> value,<br/>
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped).
/// </para>
/// <para>
/// String array-based comparisons (ARRAY CONTAINS ANY OF / ARRAY NOT CONTAINS ANY OF)<br/>
/// String array-based comparators (ARRAY CONTAINS ANY OF / ARRAY NOT CONTAINS ANY OF)<br/>
/// * accept arrays of <see cref="string"/>,<br/>
/// * accept <see cref="string"/> values containing a valid JSON string which can be deserialized to an array of <see cref="string"/>,<br/>
/// * all other values are considered invalid (a warning will be logged and the currently evaluated targeting rule will be skipped).
/// </para>
/// In case a non-string attribute value needs to be converted to <see cref="string"/> during evaluation, it will always be done using the same format which is accepted by the comparisons.
/// </remarks>
public IDictionary<string, object> Custom
{
Expand Down

0 comments on commit 00cdcc4

Please sign in to comment.