Skip to content

Commit

Permalink
Merge pull request #21 from configcat/fix-global-json-settings
Browse files Browse the repository at this point in the history
Ensure internal serializer doesn't use the global json serializer settings
  • Loading branch information
z4kn4fein authored Sep 7, 2021
2 parents 4582a83 + a323e69 commit 9182be3
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 57 deletions.
3 changes: 2 additions & 1 deletion src/ConfigCat.Client.Tests/ConfigEvaluatorTestsBase.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using ConfigCat.Client.Evaluate;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Newtonsoft.Json;
using System;
using System.Collections.Generic;
using System.Globalization;
Expand All @@ -23,7 +24,7 @@ public abstract class ConfigEvaluatorTestsBase

public ConfigEvaluatorTestsBase()
{
this.configEvaluator = new RolloutEvaluator(logger, new ConfigDeserializer(logger));
this.configEvaluator = new RolloutEvaluator(logger, new ConfigDeserializer(logger, JsonSerializer.Create()));

this.config = new ProjectConfig(this.GetSampleJson(), DateTime.UtcNow, null);
}
Expand Down
2 changes: 2 additions & 0 deletions src/ConfigCat.Client.Tests/DataGovernanceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public async Task WithDataGovernanceSetting_ShouldUseProperCdnUrl(DataGovernance
"DEMO",
Mock.Of<ILogger>(),
handlerMock.Object,
JsonSerializer.Create(),
configuration.IsCustomBaseUrl);

// Act
Expand Down Expand Up @@ -378,6 +379,7 @@ internal async Task<SortedList<byte, HttpRequestMessage>> Fetch(
"DEMO",
Mock.Of<ILogger>(),
handlerMock.Object,
JsonSerializer.Create(),
fetchConfig.IsCustomBaseUrl);

// Act
Expand Down
27 changes: 27 additions & 0 deletions src/ConfigCat.Client.Tests/DeserializerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using System;
using System.IO;

namespace ConfigCat.Client.Tests
{
[TestClass]
public class DeserializerTests
{
[TestMethod]
public void Ensure_Global_Settings_Doesnt_Interfere()
{
JsonConvert.DefaultSettings = () =>
{
var settings = new JsonSerializerSettings();
settings.Converters.Add(new StringEnumConverter { AllowIntegerValues = false });
return settings;
};

var deserializer = new ConfigDeserializer(new LoggerWrapper(new ConsoleLogger(LogLevel.Debug)), JsonSerializer.Create());

deserializer.TryDeserialize(new ProjectConfig("{\"p\": {\"u\": \"http://example.com\", \"r\": 0}}", DateTime.Now, ""), out var configs);
}
}
}
9 changes: 5 additions & 4 deletions src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Newtonsoft.Json;
using System;
using System.Net;
using System.Threading.Tasks;
Expand All @@ -15,7 +16,7 @@ public async Task HttpConfigFetcher_WithCustomHttpClientHandler_ShouldUsePassedH

var myHandler = new MyFakeHttpClientHandler();

var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false);
var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false);

// Act

Expand All @@ -33,7 +34,7 @@ public void HttpConfigFetcher_WithCustomHttpClientHandler_HandlersDisposeShouldN

var myHandler = new MyFakeHttpClientHandler();

var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false);
var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false);

// Act

Expand All @@ -51,7 +52,7 @@ public async Task HttpConfigFetcher_ResponseHttpCodeIsUnexpected_ShouldReturnsPa

var myHandler = new MyFakeHttpClientHandler(HttpStatusCode.Forbidden);

var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false);
var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false);

var lastConfig = new ProjectConfig("{ }", DateTime.UtcNow, "\"ETAG\"");

Expand All @@ -71,7 +72,7 @@ public async Task HttpConfigFetcher_ThrowAnException_ShouldReturnPassedConfig()

var myHandler = new ExceptionThrowerHttpClientHandler(new WebException());

var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, false);
var instance = new HttpConfigFetcher(new Uri("http://example.com"), "1.0", new MyCounterLogger(), myHandler, JsonSerializer.Create(), false);

var lastConfig = new ProjectConfig("{ }", DateTime.UtcNow, "\"ETAG\"");

Expand Down
12 changes: 8 additions & 4 deletions src/ConfigCatClient/ConfigCatClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using ConfigCat.Client.Cache;
using ConfigCat.Client.Security;
using Newtonsoft.Json;

namespace ConfigCat.Client
{
Expand All @@ -16,6 +17,8 @@ namespace ConfigCat.Client
/// </summary>
public class ConfigCatClient : IConfigCatClient
{
private readonly JsonSerializer serializer;

private readonly ILogger log;

private readonly IRolloutEvaluator configEvaluator;
Expand Down Expand Up @@ -61,7 +64,7 @@ public ConfigCatClient(AutoPollConfiguration configuration)
: this((ConfigurationBase)configuration)
{
var autoPollService = new AutoPollConfigService(
new HttpConfigFetcher(configuration.CreateUri(), "a-" + version, configuration.Logger, configuration.HttpClientHandler, configuration.IsCustomBaseUrl),
new HttpConfigFetcher(configuration.CreateUri(), "a-" + version, configuration.Logger, configuration.HttpClientHandler, this.serializer, configuration.IsCustomBaseUrl),
this.cacheParameters,
TimeSpan.FromSeconds(configuration.PollIntervalSeconds),
TimeSpan.FromSeconds(configuration.MaxInitWaitTimeSeconds),
Expand All @@ -83,7 +86,7 @@ public ConfigCatClient(LazyLoadConfiguration configuration)
: this((ConfigurationBase)configuration)
{
var lazyLoadService = new LazyLoadConfigService(
new HttpConfigFetcher(configuration.CreateUri(), "l-" + version, configuration.Logger, configuration.HttpClientHandler, configuration.IsCustomBaseUrl),
new HttpConfigFetcher(configuration.CreateUri(), "l-" + version, configuration.Logger, configuration.HttpClientHandler, this.serializer, configuration.IsCustomBaseUrl),
this.cacheParameters,
configuration.Logger,
TimeSpan.FromSeconds(configuration.CacheTimeToLiveSeconds));
Expand All @@ -101,7 +104,7 @@ public ConfigCatClient(ManualPollConfiguration configuration)
: this((ConfigurationBase)configuration)
{
var configService = new ManualPollConfigService(
new HttpConfigFetcher(configuration.CreateUri(), "m-" + version, configuration.Logger, configuration.HttpClientHandler, configuration.IsCustomBaseUrl),
new HttpConfigFetcher(configuration.CreateUri(), "m-" + version, configuration.Logger, configuration.HttpClientHandler, this.serializer, configuration.IsCustomBaseUrl),
this.cacheParameters,
configuration.Logger);

Expand All @@ -117,8 +120,9 @@ private ConfigCatClient(ConfigurationBase configuration)

configuration.Validate();

this.serializer = JsonSerializer.Create();
this.log = configuration.Logger;
this.configDeserializer = new ConfigDeserializer(this.log);
this.configDeserializer = new ConfigDeserializer(this.log, this.serializer);
this.configEvaluator = new RolloutEvaluator(this.log, this.configDeserializer);
this.cacheParameters = new CacheParameters
{
Expand Down
17 changes: 12 additions & 5 deletions src/ConfigCatClient/ConfigDeserializer.cs
Original file line number Diff line number Diff line change
@@ -1,34 +1,41 @@
using ConfigCat.Client.Evaluate;
using Newtonsoft.Json;
using System.Collections.Generic;
using System.IO;

namespace ConfigCat.Client
{
internal class ConfigDeserializer : IConfigDeserializer
{
private readonly ILogger logger;
private readonly JsonSerializer serializer;

public ConfigDeserializer(ILogger logger)
public ConfigDeserializer(ILogger logger, JsonSerializer serializer)
{
this.logger = logger;
this.serializer = serializer;
}

public bool TryDeserialize(ProjectConfig projectConfig, out IDictionary<string, Setting> settings)
{
if (projectConfig.JsonString == null)
{
this.logger.Warning("ConfigJson is not present.");

settings = null;

return false;
}

var settingsWithPreferences = JsonConvert.DeserializeObject<SettingsWithPreferences>(projectConfig.JsonString);
using (var stringReader = new StringReader(projectConfig.JsonString))
using (var reader = new JsonTextReader(stringReader))
{
var settingsWithPreferences = this.serializer.Deserialize<SettingsWithPreferences>(reader);

settings = settingsWithPreferences.Settings;
settings = settingsWithPreferences.Settings;

return true;
return true;
}
}
}
}
91 changes: 48 additions & 43 deletions src/ConfigCatClient/HttpConfigFetcher.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.IO;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
Expand All @@ -17,14 +18,14 @@ internal sealed class HttpConfigFetcher : IConfigFetcher, IDisposable
private readonly ILogger log;

private readonly HttpClientHandler httpClientHandler;

private readonly JsonSerializer serializer;
private readonly bool isCustomUri;

private HttpClient httpClient;

private Uri requestUri;

public HttpConfigFetcher(Uri requestUri, string productVersion, ILogger logger, HttpClientHandler httpClientHandler, bool isCustomUri)
public HttpConfigFetcher(Uri requestUri, string productVersion, ILogger logger, HttpClientHandler httpClientHandler, JsonSerializer serializer, bool isCustomUri)
{
this.requestUri = requestUri;

Expand All @@ -33,7 +34,7 @@ public HttpConfigFetcher(Uri requestUri, string productVersion, ILogger logger,
this.log = logger;

this.httpClientHandler = httpClientHandler;

this.serializer = serializer;
this.isCustomUri = isCustomUri;

ReInitializeHttpClient();
Expand Down Expand Up @@ -98,53 +99,57 @@ private async Task<Tuple<HttpResponseMessage, string>> FetchRequest(ProjectConfi
{
var responseBody = await response.Content.ReadAsStringAsync().ConfigureAwait(false);

var body = JsonConvert.DeserializeObject<SettingsWithPreferences>(responseBody);

if (body?.Preferences != null)
using (var stringReader = new StringReader(responseBody))
using (var reader = new JsonTextReader(stringReader))
{
var newBaseUrl = body.Preferences.Url;

if (newBaseUrl == null || requestUri.Host == new Uri(newBaseUrl).Host)
{
return Tuple.Create(response, responseBody);
}

Evaluate.RedirectMode redirect = body.Preferences.RedirectMode;

if (isCustomUri && redirect != RedirectMode.Force)
{
return Tuple.Create(response, responseBody);
}

UpdateRequestUri(new Uri(newBaseUrl));
var body = this.serializer.Deserialize<SettingsWithPreferences>(reader);

if (redirect == RedirectMode.No)
if (body?.Preferences != null)
{
return Tuple.Create(response, responseBody);
var newBaseUrl = body.Preferences.Url;

if (newBaseUrl == null || requestUri.Host == new Uri(newBaseUrl).Host)
{
return Tuple.Create(response, responseBody);
}

Evaluate.RedirectMode redirect = body.Preferences.RedirectMode;

if (isCustomUri && redirect != RedirectMode.Force)
{
return Tuple.Create(response, responseBody);
}

UpdateRequestUri(new Uri(newBaseUrl));

if (redirect == RedirectMode.No)
{
return Tuple.Create(response, responseBody);
}

if (redirect == RedirectMode.Should)
{
this.log.Warning("Your dataGovernance parameter at ConfigCatClient initialization is not in sync " +
"with your preferences on the ConfigCat Dashboard: " +
"https://app.configcat.com/organization/data-governance. " +
"Only Organization Admins can access this preference.");
}

if (maxExecutionCount <= 1)
{
log.Error("Redirect loop during config.json fetch. Please contact [email protected].");
return Tuple.Create(response, responseBody);
}

return await this.FetchRequest(
lastConfig,
ReplaceUri(request.RequestUri, new Uri(newBaseUrl)),
--maxExecutionCount);
}

if (redirect == RedirectMode.Should)
else
{
this.log.Warning("Your dataGovernance parameter at ConfigCatClient initialization is not in sync " +
"with your preferences on the ConfigCat Dashboard: " +
"https://app.configcat.com/organization/data-governance. " +
"Only Organization Admins can access this preference.");
}

if (maxExecutionCount <= 1)
{
log.Error("Redirect loop during config.json fetch. Please contact [email protected].");
return Tuple.Create(response, responseBody);
}

return await this.FetchRequest(
lastConfig,
ReplaceUri(request.RequestUri, new Uri(newBaseUrl)),
--maxExecutionCount);
}
else
{
return Tuple.Create(response, responseBody);
}
}

Expand Down

0 comments on commit 9182be3

Please sign in to comment.