Additional minor improvements (#51)

* Improves error reporting when setting type and default value type mismatch

* Improves ConfigDeserializer performance by using ETag for change detection instead of calculating a hash

* Improves SHA1 hashing implementation

* Eliminates ModuleInitalizer trick used in tests because MSTest provides that feature out of the box

LICENSE

@@ -20,9 +20,6 @@ The files containing such source code are the following:
 * src/ConfigCatClient/Versioning/IntExtensions.cs, which
   is originally available at https://github.com/maxhauser/semver/blob/v2.2.0/Semver/Utility/IntExtensions.cs and
   is licensed under MIT license (see https://github.com/maxhauser/semver/blob/v2.2.0/License.txt).
-* src/ConfigCat.Client.Tests/ModuleInitializerAttribute.cs, which
-  is originally available at https://github.com/dotnet/runtime/blob/v6.0.9/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ModuleInitializerAttribute.cs and
-  is licensed under MIT license (see https://github.com/dotnet/runtime/blob/main/LICENSE.TXT).
 See the file headers for details.
 
 THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

src/ConfigCat.Client.Tests/ModuleInitializer.cs → src/ConfigCat.Client.Tests/AssemblyInitializer.cs

@@ -1,12 +1,13 @@
 using System.Net;
-using System.Runtime.CompilerServices;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace ConfigCat.Client.Tests
 {
-    internal class ModuleInitializer
+    [TestClass]
+    public class AssemblyInitializer
     {
-        [ModuleInitializer]
-        internal static void Setup()
+        [AssemblyInitialize]
+        public static void AssemblyInitialize(TestContext context)
         {
 #if NET45
             // TLS 1.2 was not enabled before .NET 4.6 by default (see https://stackoverflow.com/a/58195987/8656352),

src/ConfigCat.Client.Tests/BasicConfigEvaluatorTests.cs

@@ -1,6 +1,11 @@
 using ConfigCat.Client.Evaluation;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using System;
 using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+using System.Runtime.CompilerServices;
 
 namespace ConfigCat.Client.Tests
 {
@@ -48,5 +53,72 @@ public void GetValue_WithUser_ShouldReturnEvaluatedValue()
 
             Assert.AreEqual(3.1415, actual);
         }
+
+        private delegate object EvaluateDelegate(IRolloutEvaluator evaluator, IDictionary<string, Setting> settings, string key, object defaultValue, User user,
+            ProjectConfig remoteConfig, ILogger logger, out EvaluationDetails<object> evaluationDetails);
+
+        private static readonly MethodInfo evaluateMethodDefinition = new EvaluateDelegate(RolloutEvaluatorExtensions.Evaluate).Method.GetGenericMethodDefinition();
+
+        [DataRow("stringDefaultCat", "", "Cat", typeof(string))]
+        [DataRow("stringDefaultCat", "", "Cat", typeof(object))]
+        [DataRow("boolDefaultTrue", false, true, typeof(bool))]
+        [DataRow("boolDefaultTrue", false, true, typeof(bool?))]
+        [DataRow("boolDefaultTrue", false, true, typeof(object))]
+        [DataRow("integerDefaultOne", 0, 1, typeof(int))]
+        [DataRow("integerDefaultOne", 0, 1, typeof(int?))]
+        [DataRow("integerDefaultOne", 0L, 1L, typeof(long))]
+        [DataRow("integerDefaultOne", 0L, 1L, typeof(long?))]
+        [DataRow("integerDefaultOne", 0, 1, typeof(object))]
+        [DataRow("doubleDefaultPi", 0.0, 3.1415, typeof(double))]
+        [DataRow("doubleDefaultPi", 0.0, 3.1415, typeof(double?))]
+        [DataRow("doubleDefaultPi", 0.0, 3.1415, typeof(object))]
+        [DataTestMethod]
+        public void GetValue_WithCompatibleDefaultValue_ShouldSucceed(string key, object defaultValue, object expectedValue, Type settingClrType)
+        {
+            var args = new object[]
+            {
+                this.configEvaluator,
+                this.config,
+                key,
+                defaultValue,
+                null,
+                null,
+                this.logger,
+                null
+            };
+
+            var actualValue = evaluateMethodDefinition.MakeGenericMethod(settingClrType).Invoke(null, args);
+            var evaluationDetails = (EvaluationDetails)args.Last();
+
+            Assert.AreEqual(expectedValue, actualValue);
+            Assert.AreEqual(expectedValue, evaluationDetails.Value);
+        }
+
+        [DataRow("stringDefaultCat", 0.0, typeof(double?))]
+        [DataRow("boolDefaultTrue", "false", typeof(string))]
+        [DataRow("integerDefaultOne", "0", typeof(string))]
+        [DataRow("doubleDefaultPi", 0, typeof(int))]
+        [DataTestMethod]
+        public void GetValue_WithIncompatibleDefaultValueType_ShouldThrowWithImprovedErrorMessage(string key, object defaultValue, Type settingClrType)
+        {
+            var args = new object[]
+            {
+                this.configEvaluator,
+                this.config,
+                key,
+                defaultValue,
+                null,
+                null,
+                this.logger,
+                null
+            };
+
+            var ex = Assert.ThrowsException<InvalidOperationException>(() =>
+            {
+                try { evaluateMethodDefinition.MakeGenericMethod(settingClrType).Invoke(null, args); }
+                catch (TargetInvocationException ex) { throw ex.InnerException; }
+            });
+            StringAssert.Contains(ex.Message, $"Setting's type was {this.config[key].SettingType} but the default value's type was {settingClrType}.");
+        }
     }
 }

src/ConfigCat.Client.Tests/ConfigCatClientTests.cs

@@ -706,7 +706,7 @@ public void GetAllKeys_DeserializerThrowException_ShouldReturnsWithEmptyArray()
             configServiceMock.Setup(m => m.GetConfigAsync()).ReturnsAsync(ProjectConfig.Empty);
             var o = new SettingsWithPreferences();
             configDeserializerMock
-                .Setup(m => m.TryDeserialize(It.IsAny<string>(), out o))
+                .Setup(m => m.TryDeserialize(It.IsAny<string>(), It.IsAny<string>(), out o))
                 .Throws<Exception>();
 
             IConfigCatClient instance = new ConfigCatClient(
@@ -734,7 +734,7 @@ public async Task GetAllKeysAsync_DeserializeFailed_ShouldReturnsWithEmptyArray(
             configServiceMock.Setup(m => m.GetConfigAsync()).ReturnsAsync(ProjectConfig.Empty);
             var o = new SettingsWithPreferences();
             configDeserializerMock
-                .Setup(m => m.TryDeserialize(It.IsAny<string>(), out o))
+                .Setup(m => m.TryDeserialize(It.IsAny<string>(), It.IsAny<string>(), out o))
                 .Returns(false);
 
             IConfigCatClient instance = new ConfigCatClient(
@@ -762,7 +762,7 @@ public void GetAllKeys_DeserializeFailed_ShouldReturnsWithEmptyArray()
             configServiceMock.Setup(m => m.GetConfig()).Returns(ProjectConfig.Empty);
             var o = new SettingsWithPreferences();
             configDeserializerMock
-                .Setup(m => m.TryDeserialize(It.IsAny<string>(), out o))
+                .Setup(m => m.TryDeserialize(It.IsAny<string>(), It.IsAny<string>(), out o))
                 .Returns(false);
 
             IConfigCatClient instance = new ConfigCatClient(
@@ -849,7 +849,7 @@ public void GetVariationId_DeserializeFailed_ShouldReturnsWithEmptyArray()
             configServiceMock.Setup(m => m.GetConfigAsync()).ReturnsAsync(ProjectConfig.Empty);
             var o = new SettingsWithPreferences();
             configDeserializerMock
-                .Setup(m => m.TryDeserialize(It.IsAny<string>(), out o))
+                .Setup(m => m.TryDeserialize(It.IsAny<string>(), It.IsAny<string>(), out o))
                 .Returns(false);
 
             IConfigCatClient instance = new ConfigCatClient(
@@ -877,7 +877,7 @@ public async Task GetVariationIdAsync_DeserializeFailed_ShouldReturnsWithEmptyAr
             configServiceMock.Setup(m => m.GetConfigAsync()).ReturnsAsync(ProjectConfig.Empty);
             var o = new SettingsWithPreferences();
             configDeserializerMock
-                .Setup(m => m.TryDeserialize(It.IsAny<string>(), out o))
+                .Setup(m => m.TryDeserialize(It.IsAny<string>(), It.IsAny<string>(), out o))
                 .Returns(false);
 
             IConfigCatClient instance = new ConfigCatClient(

src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs

@@ -60,14 +60,14 @@ protected virtual void AssertValue(string keyName, string expected, User user)
 
         protected string GetSampleJson()
         {
-            using Stream stream = File.OpenRead("data" + Path.DirectorySeparatorChar + this.SampleJsonFileName);
+            using Stream stream = File.OpenRead(Path.Combine("data", this.SampleJsonFileName));
             using StreamReader reader = new(stream);
             return reader.ReadToEnd();
         }
 
         public async Task MatrixTest(Action<string, string, User> assertation)
         {
-            using Stream stream = File.OpenRead("data" + Path.DirectorySeparatorChar + this.MatrixResultFileName);
+            using Stream stream = File.OpenRead(Path.Combine("data", this.MatrixResultFileName));
             using StreamReader reader = new(stream);
             var header = await reader.ReadLineAsync();
 

src/ConfigCat.Client.Tests/DeserializerTests.cs

@@ -18,7 +18,7 @@ public void Ensure_Global_Settings_Doesnt_Interfere()
             };
 
             var deserializer = new ConfigDeserializer();
-            deserializer.TryDeserialize("{\"p\": {\"u\": \"http://example.com\", \"r\": 0}}", out var configs);
+            deserializer.TryDeserialize("{\"p\": {\"u\": \"http://example.com\", \"r\": 0}}", httpETag: null, out var configs);
         }
     }
 }

src/ConfigCatClient/ConfigCatClient.cs

@@ -257,6 +257,7 @@ public T GetValue<T>(string key, T defaultValue, User user = null)
             user ??= this.defaultUser;
             try
             {
+                typeof(T).EnsureSupportedSettingClrType();
                 settings = this.GetSettings();
                 value = this.configEvaluator.Evaluate(settings.Value, key, defaultValue, user, settings.RemoteConfig, this.log, out evaluationDetails);
             }
@@ -280,6 +281,7 @@ public async Task<T> GetValueAsync<T>(string key, T defaultValue, User user = nu
             user ??= this.defaultUser;
             try
             {
+                typeof(T).EnsureSupportedSettingClrType();
                 settings = await this.GetSettingsAsync().ConfigureAwait(false);
                 value = this.configEvaluator.Evaluate(settings.Value, key, defaultValue, user, settings.RemoteConfig, this.log, out evaluationDetails);
             }
@@ -302,6 +304,7 @@ public EvaluationDetails<T> GetValueDetails<T>(string key, T defaultValue, User
             user ??= this.defaultUser;
             try
             {
+                typeof(T).EnsureSupportedSettingClrType();
                 settings = this.GetSettings();
                 this.configEvaluator.Evaluate(settings.Value, key, defaultValue, user, settings.RemoteConfig, this.log, out evaluationDetails);
             }
@@ -323,6 +326,7 @@ public async Task<EvaluationDetails<T>> GetValueDetailsAsync<T>(string key, T de
             user ??= this.defaultUser;
             try
             {
+                typeof(T).EnsureSupportedSettingClrType();
                 settings = await this.GetSettingsAsync().ConfigureAwait(false);
                 this.configEvaluator.Evaluate(settings.Value, key, defaultValue, user, settings.RemoteConfig, this.log, out evaluationDetails);
             }
@@ -684,7 +688,7 @@ private SettingsWithRemoteConfig GetSettings()
             SettingsWithRemoteConfig GetRemoteConfig()
             {
                 var config = this.configService.GetConfig();
-                if (!this.configDeserializer.TryDeserialize(config.JsonString, out var deserialized))
+                if (!this.configDeserializer.TryDeserialize(config.JsonString, config.HttpETag, out var deserialized))
                     return new SettingsWithRemoteConfig(new Dictionary<string, Setting>(), config);
 
                 return new SettingsWithRemoteConfig(deserialized.Settings, config);
@@ -717,7 +721,7 @@ private async Task<SettingsWithRemoteConfig> GetSettingsAsync()
             async Task<SettingsWithRemoteConfig> GetRemoteConfigAsync()
             {
                 var config = await this.configService.GetConfigAsync().ConfigureAwait(false);
-                if (!this.configDeserializer.TryDeserialize(config.JsonString, out var deserialized))
+                if (!this.configDeserializer.TryDeserialize(config.JsonString, config.HttpETag, out var deserialized))
                     return new SettingsWithRemoteConfig(new Dictionary<string, Setting>(), config);
 
                 return new SettingsWithRemoteConfig(deserialized.Settings, config);

src/ConfigCatClient/ConfigCatClient.csproj

@@ -59,7 +59,6 @@ Works with .NET, .NET Core, .NET Standard</Description>
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="System.Data.HashFunction.MurmurHash" Version="2.0.0" />
     <PackageReference Include="System.Net.Http" Version="4.3.4" />
     <!--Do not remove this reference, it was added due to a SNYK security report-->
     <PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />

src/ConfigCatClient/ConfigDeserializer.cs

@@ -1,36 +1,36 @@
 using ConfigCat.Client.Evaluation;
 using System;
-using System.Data.HashFunction;
-using System.Data.HashFunction.MurmurHash;
 
 namespace ConfigCat.Client
 {
     internal class ConfigDeserializer : IConfigDeserializer
     {
-        private readonly IHashFunction hasher = MurmurHash3Factory.Instance.Create(new MurmurHash3Config { HashSizeInBits = 128 });
-        private SettingsWithPreferences lastSerializedSettings;
-        private byte[] hashToCompare;
+        private SettingsWithPreferences lastDeserializedSettings;
+        private string lastConfig;
+        private string lastHttpETag;
 
-        public bool TryDeserialize(string config, out SettingsWithPreferences settings)
+        public bool TryDeserialize(string config, string httpETag, out SettingsWithPreferences settings)
         {
             if (config == null)
             {
                 settings = null;
                 return false;
             }
 
-            var hash = this.hasher.ComputeHash(config).Hash;
-            if (CompareByteArrays(this.hashToCompare, hash))
+            var configContentHasChanged = this.lastHttpETag is not null && httpETag is not null
+                ? this.lastHttpETag != httpETag
+                : this.lastConfig != config;
+
+            if (!configContentHasChanged)
             {
-                settings = this.lastSerializedSettings;
+                settings = this.lastDeserializedSettings;
                 return true;
             }
 
-            this.lastSerializedSettings = settings = config.Deserialize<SettingsWithPreferences>();
-            this.hashToCompare = hash;
+            this.lastDeserializedSettings = settings = config.Deserialize<SettingsWithPreferences>();
+            this.lastConfig = config;
+            this.lastHttpETag = httpETag;
             return true;
         }
-
-        private static bool CompareByteArrays(ReadOnlySpan<byte> b1, ReadOnlySpan<byte> b2) => b1.SequenceEqual(b2);
     }
 }