Skip to content

Commit

Permalink
Don't force users to pass user object attributes as strings
Browse files Browse the repository at this point in the history
  • Loading branch information
adams85 committed Nov 17, 2023
1 parent c148867 commit 4066377
Show file tree
Hide file tree
Showing 8 changed files with 395 additions and 140 deletions.
174 changes: 173 additions & 1 deletion src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/ConfigCat.Client.Tests/MatrixTestRunnerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ internal bool RunTest(IRolloutEvaluator evaluator, LoggerWrapper logger, string
{
user = new User(userId) { Email = userEmail, Country = userCountry };
if (userCustomAttributeValue is not null)
{
user.Custom[userCustomAttributeName!] = userCustomAttributeValue;
}
}

return RunTest(evaluator, logger, settingKey, expectedReturnValue, user);
Expand Down
33 changes: 0 additions & 33 deletions src/ConfigCat.Client.Tests/UserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,37 +125,4 @@ public void CreateUser_ShouldSetIdentifier(string identifier, string expectedVal
Assert.AreEqual(expectedValue, user.Identifier);
Assert.AreEqual(expectedValue, user.GetAllAttributes()[nameof(User.Identifier)]);
}

[DataTestMethod]
[DataRow("datetime", "2023-09-19T11:01:35.0000000+00:00", "1695121295")]
[DataRow("datetime", "2023-09-19T13:01:35.0000000+02:00", "1695121295")]
[DataRow("datetime", "2023-09-19T11:01:35.0510886+00:00", "1695121295.051")]
[DataRow("datetime", "2023-09-19T13:01:35.0510886+02:00", "1695121295.051")]
[DataRow("number", "3", "3")]
[DataRow("number", "3.14", "3.14")]
[DataRow("number", "-1.23e-100", "-1.23e-100")]
[DataRow("stringlist", "a,,b,c", "[\"a\",\"\",\"b\",\"c\"]")]
public void HelperMethodsShouldWork(string type, string value, string expectedAttributeValue)
{
string actualAttributeValue;
switch (type)
{
case "datetime":
var dateTimeOffset = DateTimeOffset.ParseExact(value, "o", CultureInfo.InvariantCulture);
actualAttributeValue = User.AttributeValueFrom(dateTimeOffset);
break;
case "number":
var number = double.Parse(value, NumberStyles.Float, CultureInfo.InvariantCulture);
actualAttributeValue = User.AttributeValueFrom(number);
break;
case "stringlist":
var items = value.Split(',');
actualAttributeValue = User.AttributeValueFrom(items);
break;
default:
throw new InvalidOperationException();
}

Assert.AreEqual(expectedAttributeValue, actualAttributeValue);
}
}
4 changes: 2 additions & 2 deletions src/ConfigCatClient/Evaluation/EvaluateContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ internal struct EvaluateContext
[MemberNotNullWhen(true, nameof(UserAttributes))]
public readonly bool IsUserAvailable => this.user is not null;

private IReadOnlyDictionary<string, string>? userAttributes;
public IReadOnlyDictionary<string, string>? UserAttributes => this.userAttributes ??= this.user?.GetAllAttributes();
private IReadOnlyDictionary<string, object>? userAttributes;
public IReadOnlyDictionary<string, object>? UserAttributes => this.userAttributes ??= this.user?.GetAllAttributes();

private List<string>? visitedFlags;
public List<string> VisitedFlags => this.visitedFlags ??= new List<string>();
Expand Down
182 changes: 128 additions & 54 deletions src/ConfigCatClient/Evaluation/RolloutEvaluator.cs

Large diffs are not rendered by default.

41 changes: 41 additions & 0 deletions src/ConfigCatClient/Extensions/ObjectExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics;
using System.Globalization;
using System.Runtime.CompilerServices;
using ConfigCat.Client;

#if USE_NEWTONSOFT_JSON
Expand Down Expand Up @@ -149,6 +150,46 @@ private static TValue ConvertTo<TValue>(this JsonValue value)
#endif
}

public static bool TryConvertNumericToDouble(this object value, out double number)
{
if (Type.GetTypeCode(value.GetType()) is
TypeCode.SByte or
TypeCode.Byte or
TypeCode.Int16 or
TypeCode.UInt16 or
TypeCode.Int32 or
TypeCode.UInt32 or
TypeCode.Int64 or
TypeCode.UInt64 or
TypeCode.Single or
TypeCode.Double or
TypeCode.Decimal)
{
number = ((IConvertible)value).ToDouble(CultureInfo.InvariantCulture);
return true;
}

number = default;
return false;
}

public static bool TryConvertDateTimeToDateTimeOffset(this object value, out DateTimeOffset dateTimeOffset)
{
if (value is DateTimeOffset dateTimeOffsetLocal)
{
dateTimeOffset = dateTimeOffsetLocal;
return true;
}
else if (value is DateTime dateTime)
{
dateTimeOffset = new DateTimeOffset(dateTime.Kind != DateTimeKind.Unspecified ? dateTime : DateTime.SpecifyKind(dateTime, DateTimeKind.Utc));
return true;
}

dateTimeOffset = default;
return false;
}

private static readonly object BoxedTrue = true;
private static readonly object BoxedFalse = false;

Expand Down
5 changes: 5 additions & 0 deletions src/ConfigCatClient/Logging/LogMessages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ public static FormattableLogMessage UserObjectAttributeIsInvalid(this LoggerWrap
$"Cannot evaluate condition ({condition}) for setting '{key}' ({reason}). Please check the User.{attributeName} attribute and make sure that its value corresponds to the comparison operator.",
"CONDITION", "KEY", "REASON", "ATTRIBUTE_NAME");

public static FormattableLogMessage UserObjectAttributeIsAutoConverted(this LoggerWrapper logger, string condition, string key, string attributeName, string attributeValue) => logger.LogInterpolated(
LogLevel.Warning, 3005,
$"Evaluation of condition ({condition}) for setting '{key}' may not produce the expected result (the User.{attributeName} attribute is not a string value, thus it was automatically converted to the string value '{attributeValue}'). Please make sure that using a non-string value was intended.",
"CONDITION", "KEY", "ATTRIBUTE_NAME", "ATTRIBUTE_VALUE");

public static FormattableLogMessage ConfigServiceCannotInitiateHttpCalls(this LoggerWrapper logger) => logger.Log(
LogLevel.Warning, 3200,
"Client is in offline mode, it cannot initiate HTTP calls.");
Expand Down
94 changes: 44 additions & 50 deletions src/ConfigCatClient/User.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using ConfigCat.Client.Utils;
using System.Linq;

#if USE_NEWTONSOFT_JSON
using Newtonsoft.Json;
Expand All @@ -15,49 +12,12 @@ namespace ConfigCat.Client;
/// <summary>
/// User Object. Contains user attributes which are used for evaluating targeting rules and percentage options.
/// </summary>
/// <remarks>
/// Please note that the <see cref="User"/> class is not designed to be used as a DTO (data transfer object).
/// (Since the type of the <see cref="Custom"/> property is polymorphic, it's not guaranteed that deserializing a serialized instance produces an instance with an identical or even valid data content.)
/// </remarks>
public class User
{
/// <summary>
/// Converts the specified <see cref="DateTimeOffset"/> value to the format expected by datetime comparison operators (BEFORE/AFTER).
/// </summary>
/// <param name="dateTime">The <see cref="DateTimeOffset"/> value to convert.</param>
/// <returns>The User Object attribute value in the expected format.</returns>
public static string AttributeValueFrom(DateTimeOffset dateTime)
{
var unixTimeSeconds = DateTimeUtils.ToUnixTimeMilliseconds(dateTime.UtcDateTime) / 1000.0;
return unixTimeSeconds.ToString("0.###", CultureInfo.InvariantCulture);
}

/// <summary>
/// Converts the specified <see cref="double"/> value to the format expected by number comparison operators.
/// </summary>
/// <param name="number">The <see cref="double"/> value to convert.</param>
/// <returns>The User Object attribute value in the expected format.</returns>
public static string AttributeValueFrom(double number)
{
return number.ToString("g", CultureInfo.InvariantCulture); // format "g" allows scientific notation as well
}

/// <summary>
/// Converts the specified <see cref="string"/> items to the format expected by array comparison operators (ARRAY CONTAINS ANY OF/ARRAY NOT CONTAINS ANY OF).
/// </summary>
/// <param name="items">The <see cref="string"/> items to convert.</param>
/// <returns>The User Object attribute value in the expected format.</returns>
public static string AttributeValueFrom(params string[] items)
{
return AttributeValueFrom(items.AsEnumerable());
}

/// <summary>
/// Converts the specified <see cref="string"/> items to the format expected by array comparison operators (ARRAY CONTAINS ANY OF/ARRAY NOT CONTAINS ANY OF).
/// </summary>
/// <param name="items">The <see cref="string"/> items to convert.</param>
/// <returns>The User Object attribute value in the expected format.</returns>
public static string AttributeValueFrom(IEnumerable<string> items)
{
return (items ?? throw new ArgumentNullException(nameof(items))).Serialize();
}

internal const string DefaultIdentifierValue = "";

/// <summary>
Expand All @@ -75,23 +35,57 @@ public static string AttributeValueFrom(IEnumerable<string> items)
/// </summary>
public string? Country { get; set; }

private IDictionary<string, string>? custom;
private IDictionary<string, object>? custom;

/// <summary>
/// Custom attributes of the user for advanced targeting rule definitions (e.g. user role, subscription type, etc.)
/// </summary>
public IDictionary<string, string> Custom
/// <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 descibed below.
/// <para>
/// Text-based comparisons (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).
/// </para>
/// <para>
/// SemVer-based comparisons (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/>
/// * accept <see cref="double"/> values (except for <see cref="double.NaN"/>) 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/>
/// * 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 (except for <see cref="double.NaN"/>) 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/>
/// * 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
{
get => this.custom ??= new Dictionary<string, string>();
get => this.custom ??= new Dictionary<string, object>();
set => this.custom = value;
}

/// <summary>
/// Returns all attributes of the user.
/// </summary>
public IReadOnlyDictionary<string, string> GetAllAttributes()
public IReadOnlyDictionary<string, object> GetAllAttributes()
{
var result = new Dictionary<string, string>();
var result = new Dictionary<string, object>();

result[nameof(Identifier)] = Identifier;

Expand Down

0 comments on commit 4066377

Please sign in to comment.