From 843d64abfbcd478095258a264d4e67e0636f2f53 Mon Sep 17 00:00:00 2001 From: adams85 <31276480+adams85@users.noreply.github.com> Date: Wed, 26 Jul 2023 09:15:26 +0200 Subject: [PATCH] Fix minor mistakes (#77) * Fix logging of failed GetValue(Async)/GetValueDetails(Async) calls * Remove unintended setter of IPercentageOption.VariationId * Fix XML documentation of Comparator.Contains/NotContains --- src/ConfigCatClient/ConfigCatClient.cs | 68 +++++++++++-------- src/ConfigCatClient/Models/Comparator.cs | 4 +- .../Models/RolloutPercentageItem.cs | 2 +- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/ConfigCatClient/ConfigCatClient.cs b/src/ConfigCatClient/ConfigCatClient.cs index 0b714633..f933806d 100644 --- a/src/ConfigCatClient/ConfigCatClient.cs +++ b/src/ConfigCatClient/ConfigCatClient.cs @@ -419,24 +419,26 @@ public async Task> GetAllKeysAsync(CancellationToken public IReadOnlyDictionary GetAllValues(User? user = null) { const string defaultReturnValue = "empty dictionary"; - EvaluationDetails[]? evaluationDetailsArray = null; + Dictionary result; + EvaluationDetails[]? evaluationDetailsArray; + IReadOnlyList? evaluationExceptions; user ??= this.defaultUser; try { var settings = GetSettings(); - evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out var exceptions); - if (exceptions is { Count: > 0 }) - { - throw new AggregateException(exceptions); - } + evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out evaluationExceptions); + result = evaluationDetailsArray.ToDictionary(details => details.Key, details => details.Value); } catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetAllValues), defaultReturnValue, ex); - evaluationDetailsArray ??= ArrayUtils.EmptyArray(); + return new Dictionary(); } - var result = evaluationDetailsArray.ToDictionary(details => details.Key, details => details.Value); + if (evaluationExceptions is { Count: > 0 }) + { + this.logger.SettingEvaluationError(nameof(GetAllValues), "evaluation result", new AggregateException(evaluationExceptions)); + } foreach (var evaluationDetails in evaluationDetailsArray) { @@ -450,16 +452,15 @@ public async Task> GetAllKeysAsync(CancellationToken public async Task> GetAllValuesAsync(User? user = null, CancellationToken cancellationToken = default) { const string defaultReturnValue = "empty dictionary"; - EvaluationDetails[]? evaluationDetailsArray = null; + Dictionary result; + EvaluationDetails[]? evaluationDetailsArray; + IReadOnlyList? evaluationExceptions; user ??= this.defaultUser; try { var settings = await GetSettingsAsync(cancellationToken).ConfigureAwait(false); - evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out var exceptions); - if (exceptions is { Count: > 0 }) - { - throw new AggregateException(exceptions); - } + evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out evaluationExceptions); + result = evaluationDetailsArray.ToDictionary(details => details.Key, details => details.Value); } catch (OperationCanceledException ex) when (ex.CancellationToken == cancellationToken) { @@ -468,10 +469,13 @@ public async Task> GetAllKeysAsync(CancellationToken catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetAllValuesAsync), defaultReturnValue, ex); - evaluationDetailsArray ??= ArrayUtils.EmptyArray(); + return new Dictionary(); } - var result = evaluationDetailsArray.ToDictionary(details => details.Key, details => details.Value); + if (evaluationExceptions is { Count: > 0 }) + { + this.logger.SettingEvaluationError(nameof(GetAllValuesAsync), "evaluation result", new AggregateException(evaluationExceptions)); + } foreach (var evaluationDetails in evaluationDetailsArray) { @@ -485,21 +489,23 @@ public async Task> GetAllKeysAsync(CancellationToken public IReadOnlyList GetAllValueDetails(User? user = null) { const string defaultReturnValue = "empty list"; - EvaluationDetails[]? evaluationDetailsArray = null; + EvaluationDetails[]? evaluationDetailsArray; + IReadOnlyList? evaluationExceptions; user ??= this.defaultUser; try { var settings = GetSettings(); - evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out var exceptions); - if (exceptions is { Count: > 0 }) - { - throw new AggregateException(exceptions); - } + evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out evaluationExceptions); } catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetAllValueDetails), defaultReturnValue, ex); - evaluationDetailsArray ??= ArrayUtils.EmptyArray(); + return ArrayUtils.EmptyArray(); + } + + if (evaluationExceptions is { Count: > 0 }) + { + this.logger.SettingEvaluationError(nameof(GetAllValueDetails), "evaluation result", new AggregateException(evaluationExceptions)); } foreach (var evaluationDetails in evaluationDetailsArray) @@ -514,16 +520,13 @@ public IReadOnlyList GetAllValueDetails(User? user = null) public async Task> GetAllValueDetailsAsync(User? user = null, CancellationToken cancellationToken = default) { const string defaultReturnValue = "empty list"; - EvaluationDetails[]? evaluationDetailsArray = null; + EvaluationDetails[]? evaluationDetailsArray; + IReadOnlyList? evaluationExceptions; user ??= this.defaultUser; try { var settings = await GetSettingsAsync(cancellationToken).ConfigureAwait(false); - evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out var exceptions); - if (exceptions is { Count: > 0 }) - { - throw new AggregateException(exceptions); - } + evaluationDetailsArray = this.configEvaluator.EvaluateAll(settings.Value, user, settings.RemoteConfig, this.logger, defaultReturnValue, out evaluationExceptions); } catch (OperationCanceledException ex) when (ex.CancellationToken == cancellationToken) { @@ -532,7 +535,12 @@ public async Task> GetAllValueDetailsAsync(User catch (Exception ex) { this.logger.SettingEvaluationError(nameof(GetAllValueDetailsAsync), defaultReturnValue, ex); - evaluationDetailsArray ??= ArrayUtils.EmptyArray(); + return ArrayUtils.EmptyArray(); + } + + if (evaluationExceptions is { Count: > 0 }) + { + this.logger.SettingEvaluationError(nameof(GetAllValueDetailsAsync), "evaluation result", new AggregateException(evaluationExceptions)); } foreach (var evaluationDetails in evaluationDetailsArray) diff --git a/src/ConfigCatClient/Models/Comparator.cs b/src/ConfigCatClient/Models/Comparator.cs index 45264536..71ec74d6 100644 --- a/src/ConfigCatClient/Models/Comparator.cs +++ b/src/ConfigCatClient/Models/Comparator.cs @@ -16,12 +16,12 @@ public enum Comparator : byte NotIn = 1, /// - /// Does the comparison value contain the comparison attribute as a substring? + /// Is the comparison value contained by the comparison attribute as a substring? /// Contains = 2, /// - /// Does the comparison value not contain the comparison attribute as a substring? + /// Is the comparison value not contained by the comparison attribute as a substring? /// NotContains = 3, diff --git a/src/ConfigCatClient/Models/RolloutPercentageItem.cs b/src/ConfigCatClient/Models/RolloutPercentageItem.cs index 64b8c42e..4d4d7a1c 100644 --- a/src/ConfigCatClient/Models/RolloutPercentageItem.cs +++ b/src/ConfigCatClient/Models/RolloutPercentageItem.cs @@ -33,7 +33,7 @@ public interface IPercentageOption /// /// Variation ID. /// - string? VariationId { get; set; } + string? VariationId { get; } } internal sealed class RolloutPercentageItem : IPercentageOption