Skip to content

Commit

Permalink
Minor improvements and fixes (#87)
Browse files Browse the repository at this point in the history
* Don't swallow exceptions when config json deserialization fails

* Use explicit values in SegmentComparator

* Fix typo

* Bump version

* Improve message of error 1103

* Add special character tests
  • Loading branch information
adams85 authored Jan 9, 2024
1 parent b06181e commit 71f21c0
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 35 deletions.
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
environment:
build_version: 9.0.0
build_version: 9.0.1
version: $(build_version)-{build}
image: Visual Studio 2022
configuration: Release
Expand Down
15 changes: 15 additions & 0 deletions src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -460,4 +460,19 @@ public void Ensure_Multiple_Requests_Doesnt_Interfere_In_ValueTasks()
manualPollClient.ForceRefresh();
});
}

[DataTestMethod]
[DataRow("specialCharacters", "äöüÄÖÜçéèñışğ⢙✓😀", "äöüÄÖÜçéèñışğ⢙✓😀")]
[DataRow("specialCharactersHashed", "äöüÄÖÜçéèñışğ⢙✓😀", "äöüÄÖÜçéèñışğ⢙✓😀")]
public async Task SpecialCharacters_Works(string settingKey, string userId, string expectedValue)
{
// https://app.configcat.com/v2/e7a75611-4256-49a5-9320-ce158755e3ba/08d5a03c-feb7-af1e-a1fa-40b3329f8bed/08dc016a-675e-4aa2-8492-6f572ad98037/244cf8b0-f604-11e8-b543-f23c917f9d8d
using var client = ConfigCatClient.Get("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/u28_1qNyZ0Wz-ldYHIU7-g", options =>
{
options.PollingMode = PollingModes.LazyLoad();
});

var actual = await client.GetValueAsync(settingKey, "NOT_CAT", new User(userId));
Assert.AreEqual(expectedValue, actual);
}
}
2 changes: 1 addition & 1 deletion src/ConfigCat.Client.Tests/ConfigV1EvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class SensitiveTestsDescriptor : IMatrixTestDescriptor
public static IEnumerable<object?[]> GetTests() => MatrixTestRunner<SensitiveTestsDescriptor>.GetTests();
}

public class VariationIdTestsDescriptor : IMatrixTestDescriptor, IVariationIdMatrixText
public class VariationIdTestsDescriptor : IMatrixTestDescriptor, IVariationIdMatrixTest
{
// https://app.configcat.com/08d5a03c-feb7-af1e-a1fa-40b3329f8bed/08d774b9-3d05-0027-d5f4-3e76c3dba752/244cf8b0-f604-11e8-b543-f23c917f9d8d
public ConfigLocation ConfigLocation => new ConfigLocation.Cdn("PKDVCLf-Hq-h-kCzMp-L7Q/nQ5qkhRAUEa6beEyyrVLBA");
Expand Down
2 changes: 1 addition & 1 deletion src/ConfigCat.Client.Tests/ConfigV2EvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public class SensitiveTestsDescriptor : IMatrixTestDescriptor
public static IEnumerable<object?[]> GetTests() => MatrixTestRunner<SensitiveTestsDescriptor>.GetTests();
}

public class VariationIdTestsDescriptor : IMatrixTestDescriptor, IVariationIdMatrixText
public class VariationIdTestsDescriptor : IMatrixTestDescriptor, IVariationIdMatrixTest
{
// https://app.configcat.com/v2/e7a75611-4256-49a5-9320-ce158755e3ba/08d5a03c-feb7-af1e-a1fa-40b3329f8bed/08dbc4dc-30c6-4969-8e4c-03f6a8764199/244cf8b0-f604-11e8-b543-f23c917f9d8d
public ConfigLocation ConfigLocation => new ConfigLocation.Cdn("configcat-sdk-1/PKDVCLf-Hq-h-kCzMp-L7Q/spQnkRTIPEWVivZkWM84lQ");
Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCat.Client.Tests/MatrixTestRunnerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public interface IMatrixTestDescriptor
public string MatrixResultFileName { get; }
}

public interface IVariationIdMatrixText { }
public interface IVariationIdMatrixTest { }

public class MatrixTestRunnerBase<TDescriptor> where TDescriptor : IMatrixTestDescriptor, new()
{
Expand All @@ -34,7 +34,7 @@ public interface IVariationIdMatrixText { }

public MatrixTestRunnerBase()
{
this.isVariationIdMatrixTest = DescriptorInstance is IVariationIdMatrixText;
this.isVariationIdMatrixTest = DescriptorInstance is IVariationIdMatrixTest;
this.config = DescriptorInstance.ConfigLocation.FetchConfigCached().Settings;
}

Expand Down
54 changes: 36 additions & 18 deletions src/ConfigCatClient/HttpConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@
using System.Threading;
using System.Threading.Tasks;

#if NET45
using ResponseWithBody = System.Tuple<System.Net.Http.HttpResponseMessage, string?, ConfigCat.Client.Config?>;
#else
using ResponseWithBody = System.ValueTuple<System.Net.Http.HttpResponseMessage, string?, ConfigCat.Client.Config?>;
#endif

namespace ConfigCat.Client;

internal sealed class HttpConfigFetcher : IConfigFetcher, IDisposable
Expand Down Expand Up @@ -85,22 +79,24 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
{
var responseWithBody = await FetchRequestAsync(lastConfig.HttpETag, this.requestUri).ConfigureAwait(false);

var response = responseWithBody.Item1;
var response = responseWithBody.Response;

switch (response.StatusCode)
{
case HttpStatusCode.OK:
if (responseWithBody.Item2 is null)
var config = responseWithBody.Config;
if (config is null)
{
logMessage = this.logger.FetchReceived200WithInvalidBody();
return FetchResult.Failure(lastConfig, logMessage.InvariantFormattedMessage);
var exception = responseWithBody.Exception;
logMessage = this.logger.FetchReceived200WithInvalidBody(exception);
return FetchResult.Failure(lastConfig, logMessage.InvariantFormattedMessage, exception);
}

return FetchResult.Success(new ProjectConfig
(
httpETag: response.Headers.ETag?.Tag,
configJson: responseWithBody.Item2,
config: responseWithBody.Item3,
configJson: responseWithBody.ResponseBody,
config: config,
timeStamp: ProjectConfig.GenerateTimeStamp()
));

Expand Down Expand Up @@ -177,11 +173,9 @@ private async ValueTask<ResponseWithBody> FetchRequestAsync(string? httpETag, Ur
var responseBody = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
#endif

var config = responseBody.DeserializeOrDefault<Config>();
if (config is null)
{
return new ResponseWithBody(response, null, null);
}
Config config;
try { config = Config.Deserialize(responseBody.AsMemory()); }
catch (Exception ex) { return new ResponseWithBody(response, responseBody, ex); }

if (config.Preferences is not null)
{
Expand Down Expand Up @@ -224,7 +218,7 @@ private async ValueTask<ResponseWithBody> FetchRequestAsync(string? httpETag, Ur
return new ResponseWithBody(response, responseBody, config);
}

return new ResponseWithBody(response, null, null);
return new ResponseWithBody(response, null, (Exception?)null);
}
}

Expand Down Expand Up @@ -277,4 +271,28 @@ public void Dispose()
this.cancellationTokenSource.Cancel();
this.httpClient?.Dispose();
}

private readonly struct ResponseWithBody
{
private readonly object? configOrException;

public ResponseWithBody(HttpResponseMessage response, string responseBody, Config config)
{
Response = response;
ResponseBody = responseBody;
this.configOrException = config;
}

public ResponseWithBody(HttpResponseMessage response, string? responseBody, Exception? exception)
{
Response = response;
ResponseBody = responseBody;
this.configOrException = exception;
}

public HttpResponseMessage Response { get; }
public string? ResponseBody { get; }
public Config? Config => this.configOrException as Config;
public Exception? Exception => this.configOrException as Exception;
}
}
6 changes: 3 additions & 3 deletions src/ConfigCatClient/Logging/LogMessages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ public static FormattableLogMessage FetchFailedDueToRequestTimeout(this LoggerWr

public static FormattableLogMessage FetchFailedDueToUnexpectedError(this LoggerWrapper logger, Exception ex) => logger.Log(
LogLevel.Error, 1103, ex,
"Unexpected error occurred while trying to fetch config JSON.");
"Unexpected error occurred while trying to fetch config JSON. It is most likely due to a local network issue. Please make sure your application can reach the ConfigCat CDN servers (or your proxy server) over HTTP.");

public static FormattableLogMessage FetchFailedDueToRedirectLoop(this LoggerWrapper logger) => logger.Log(
LogLevel.Error, 1104,
"Redirection loop encountered while trying to fetch config JSON. Please contact us at https://configcat.com/support/");

public static FormattableLogMessage FetchReceived200WithInvalidBody(this LoggerWrapper logger) => logger.Log(
LogLevel.Error, 1105,
public static FormattableLogMessage FetchReceived200WithInvalidBody(this LoggerWrapper logger, Exception? ex) => logger.Log(
LogLevel.Error, 1105, ex,
"Fetching config JSON was successful but the HTTP response content was invalid.");

public static FormattableLogMessage FetchReceived304WhenLocalCacheIsEmpty(this LoggerWrapper logger, int statusCode, string? reasonPhrase) => logger.LogInterpolated(
Expand Down
7 changes: 7 additions & 0 deletions src/ConfigCatClient/Models/Config.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -39,6 +40,12 @@ internal sealed class Config : IConfig
, IJsonOnDeserialized
#endif
{
public static Config Deserialize(ReadOnlyMemory<char> configJson, bool tolerant = false)
{
return configJson.Deserialize<Config>(tolerant)
?? throw new ArgumentException("Invalid config JSON content: " + configJson, nameof(configJson));
}

#if USE_NEWTONSOFT_JSON
[JsonProperty(PropertyName = "p")]
#else
Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCatClient/Models/SegmentComparator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ public enum SegmentComparator : byte
/// <summary>
/// IS IN SEGMENT - It matches when the conditions of the specified segment are evaluated to true.
/// </summary>
IsIn,
IsIn = 0,

/// <summary>
/// IS NOT IN SEGMENT - It matches when the conditions of the specified segment are evaluated to false.
/// </summary>
IsNotIn,
IsNotIn = 1,
}
3 changes: 1 addition & 2 deletions src/ConfigCatClient/Override/LocalFileDataSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ private async Task ReloadFileAsync(bool isAsync, CancellationToken cancellationT
break;
}

var deserialized = content.Deserialize<Config>(tolerant: true)
?? throw new InvalidOperationException("Invalid config JSON content: " + content);
var deserialized = Config.Deserialize(content.AsMemory(), tolerant: true);
this.overrideValues = deserialized.Settings;
break;
}
Expand Down
6 changes: 1 addition & 5 deletions src/ConfigCatClient/ProjectConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ public static ProjectConfig Deserialize(string value)
string? configJson;
if (configJsonSpan.Length > 0)
{
config = configJsonSpan.DeserializeOrDefault<Config>();
if (config is null)
{
throw new FormatException("Invalid config JSON content: " + configJsonSpan.ToString());
}
config = Config.Deserialize(configJsonSpan);
configJson = configJsonSpan.ToString();
}
else
Expand Down

0 comments on commit 71f21c0

Please sign in to comment.