diff --git a/libs/dotnet-sdk-test/FeatureBoardClientTests.cs b/libs/dotnet-sdk-test/FeatureBoardClientTests.cs index 50ce00f6..f150c73c 100644 --- a/libs/dotnet-sdk-test/FeatureBoardClientTests.cs +++ b/libs/dotnet-sdk-test/FeatureBoardClientTests.cs @@ -1,13 +1,13 @@ using System.Text.Json.Nodes; +using Bogus; +using FeatureBoard.DotnetSdk.Helpers; +using FeatureBoard.DotnetSdk.Models; +using FeatureBoard.DotnetSdk.State; using FeatureBoard.DotnetSdk.Test.Extensions; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; -using Bogus; using Shouldly; -using FeatureBoard.DotnetSdk.Helpers; -using FeatureBoard.DotnetSdk.Models; -using FeatureBoard.DotnetSdk.State; namespace FeatureBoard.DotnetSdk.Test; @@ -18,6 +18,7 @@ public class FeatureBoardClientTests public FeatureBoardClientTests() { Services.AddTransient(typeof(ILogger<>), typeof(NullLogger<>)); + Services.AddTransient(typeof(Lazy<>)); Services.AddTransient>(); var audienceMock = Services.AddServiceMock((_, mock) => @@ -29,7 +30,7 @@ public FeatureBoardClientTests() [Fact] public void ItReturnsTheDefaultValueWhenNoValueIsFound() { - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary(0))); + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary(0))); var client = Services.BuildServiceProvider().GetRequiredService>(); @@ -44,7 +45,7 @@ public void ItReturnsTheDefaultValueFromFeatureBoardWhenNoAudienceIsFound() { var faker = new Faker(); var defaultFeatureValue = faker.Lorem.Sentence(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { nameof(TestFeatures.StringFeature).ToFeatureBoardKey(), new FeatureConfiguration @@ -66,7 +67,7 @@ public void ItReturnsTheAudienceValueFromFeatureBoardWhenAnAudienceIsFound() { var faker = new Faker(); var defaultAudienceValue = faker.Lorem.Paragraph(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { nameof(TestFeatures.StringFeature).ToFeatureBoardKey(), new FeatureConfiguration @@ -95,7 +96,7 @@ public void ItReturnsADecimal() { var faker = new Faker(); var defaultAudienceValue = faker.Random.Decimal(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { nameof(TestFeatures.NumberFeature).ToFeatureBoardKey(), new FeatureConfiguration @@ -125,7 +126,7 @@ public void ItReturnsABool() { var faker = new Faker(); var defaultAudienceValue = faker.Random.Bool(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { nameof(TestFeatures.BoolFeature).ToFeatureBoardKey(), new FeatureConfiguration { @@ -153,7 +154,7 @@ public void ItReturnsAString() { var faker = new Faker(); var defaultAudienceValue = Guid.NewGuid().ToString(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { nameof(TestFeatures.StringFeature).ToFeatureBoardKey(), new FeatureConfiguration @@ -182,7 +183,7 @@ public void ItReturnsAnEnum() { var faker = new Faker(); var defaultAudienceValue = faker.PickRandom(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { nameof(TestFeatures.EnumFeature).ToFeatureBoardKey(), new FeatureConfiguration { @@ -211,7 +212,7 @@ public void ItLooksUpTheFeatureKeyWithFeatureKeyNameAttribute() { var faker = new Faker(); var defaultAudienceValue = Guid.NewGuid().ToString(); - Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { + Services.AddTransient(_ => new FeatureBoardStateSnapshot(new Dictionary { { "a-strange-name", new FeatureConfiguration { diff --git a/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs b/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs index 68382c08..bccf0dd0 100644 --- a/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs +++ b/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs @@ -1,14 +1,13 @@ -using System; using System.Linq.Expressions; using System.Net; +using System.Net.Http.Headers; using System.Net.Http.Json; using System.Text.Json.Nodes; -using Microsoft.Extensions.Logging.Abstractions; using Bogus; -using Moq; using FeatureBoard.DotnetSdk.Models; -using System.Net.Http.Headers; -using Shouldly; +using FeatureBoard.DotnetSdk.State; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; namespace FeatureBoard.DotnetSdk.Test { @@ -16,18 +15,18 @@ public class FeatureBoardHttpClientTests { private const string TestETag = @"""test"""; private EntityTagHeaderValue? _nullETag = null; - private RetryConditionHeaderValue? _retryAfter = null; private static readonly Expression> _defaultRequestMatcher = msg => msg.Method == HttpMethod.Get && msg.RequestUri!.OriginalString == FeatureBoardHttpClient.Action; - private readonly Mock _mockHttpClient = new Mock(); + private readonly Mock _mockHttpClient = new(); + private readonly Mock _mockStateUpdater = new(null); - private readonly FeatureConfiguration testFeatureConfig = CreateFeature(); + private readonly FeatureConfiguration _testFeatureConfig = CreateFeature(); public FeatureBoardHttpClientTests() { - var content = JsonContent.Create(new[] { testFeatureConfig }); + var content = JsonContent.Create(new[] { _testFeatureConfig }); _mockHttpClient .Setup(client => client.SendAsync(It.Is(_defaultRequestMatcher), It.IsAny())) .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => @@ -37,6 +36,9 @@ public FeatureBoardHttpClientTests() return response; }); + _mockStateUpdater + .Setup(m => m.UpdateState(It.IsAny>(), It.IsAny())) + .Verifiable(); } [Fact] @@ -44,9 +46,11 @@ public async Task ItReturnsCorrectListOfFeatures() { // Arrange IReadOnlyCollection? actionArg = null; - void captureArgAction(IReadOnlyCollection features) => actionArg = features; - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, captureArgAction, new NullLogger()); + _mockStateUpdater.Setup(m => m.UpdateState(It.IsAny>(), It.IsAny())) + .Callback((IReadOnlyCollection features, CancellationToken token) => actionArg = features); + + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, new[] { _mockStateUpdater.Object }, new NullLogger()); // Act var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); @@ -57,18 +61,18 @@ public async Task ItReturnsCorrectListOfFeatures() // DEBT: Refactor FeatureConfiguration to remove use of JsonValue and override Equals so get *full* ValueType equality semantics hence can just replace below with use of Assert.Equals() Assert.Collection(actionArg, item => { - Assert.Equal(testFeatureConfig.FeatureKey, item.FeatureKey); - Assert.Equal(testFeatureConfig.DefaultValue.GetValue(), item.DefaultValue.GetValue()); + Assert.Equal(_testFeatureConfig.FeatureKey, item.FeatureKey); + Assert.Equal(_testFeatureConfig.DefaultValue.GetValue(), item.DefaultValue.GetValue()); Assert.Collection(item.AudienceExceptions, ex => { - var expected = testFeatureConfig.AudienceExceptions[0]; + var expected = _testFeatureConfig.AudienceExceptions[0]; Assert.Equal(expected.AudienceKey, ex.AudienceKey); Assert.Equal(expected.Value.GetValue(), ex.Value.GetValue()); }, ex => { - var expected = testFeatureConfig.AudienceExceptions[1]; + var expected = _testFeatureConfig.AudienceExceptions[1]; Assert.Equal(expected.AudienceKey, ex.AudienceKey); Assert.Equal(expected.Value.GetValue(), ex.Value.GetValue()); } @@ -80,15 +84,13 @@ public async Task ItReturnsCorrectListOfFeatures() public async Task ItDoesNotProcessResponseIfNotModified() { // Arrange - static void nopAction(IReadOnlyCollection features) { } - Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); _mockHttpClient .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) .ReturnsAsync(new HttpResponseMessage() { StatusCode = HttpStatusCode.NotModified }); - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger()); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, new[] { _mockStateUpdater.Object }, new NullLogger()); // Act var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); @@ -99,154 +101,93 @@ static void nopAction(IReadOnlyCollection features) { } Assert.False(subsequentResult); } - [Fact] - public async Task ItDoesNotProcessResponseIfTooManyRequests() + + [Theory] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.InternalServerError)] + [InlineData(HttpStatusCode.ServiceUnavailable)] + [InlineData(HttpStatusCode.Accepted)] + [InlineData(HttpStatusCode.NoContent)] + public async Task ItDoesNotProcessResponseOnNonOkayResponse(HttpStatusCode httpStatusCode) { // Arrange - var content = JsonContent.Create(new[] { testFeatureConfig }); - static void nopAction(IReadOnlyCollection features) { } - var countHttpClient = 0; - Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); _mockHttpClient - .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) - .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => - { - countHttpClient++; - if (countHttpClient == 1) - { - var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests); - response.Headers.RetryAfter = new RetryConditionHeaderValue(new DateTimeOffset(DateTime.UtcNow.AddSeconds(1))); - return response; - } - else - { - var response = new HttpResponseMessage() { Content = content, RequestMessage = request }; - response.Headers.ETag = new EntityTagHeaderValue(TestETag); - return response; - } - }); - - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger()); + .Setup(client => client.SendAsync(It.Is(_defaultRequestMatcher), It.IsAny())) + .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => new HttpResponseMessage(httpStatusCode) { RequestMessage = request }); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, Array.Empty(), new NullLogger()); // Act - var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - Assert.Equal(1, countHttpClient); - var subsequentResult2 = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - Assert.Equal(1, countHttpClient); - Thread.Sleep(1000); - var subsequentResult3 = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); + var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); // Assert - Assert.True(initialResult); - Assert.False(subsequentResult); - Assert.False(subsequentResult2); - Assert.True(subsequentResult3); - Assert.True(_retryAfter == null); - Assert.Equal(2, countHttpClient); + Assert.Null(result); } - [Fact] - public async Task ItDoesNotProcessResponseIfTooManyRequestsRetryAfterHeaderDate() + public static object[][] ExternalStateUpdateExceptions => new[] { - // Arrange - static void nopAction(IReadOnlyCollection features) { } - var retryAfterDate = new DateTimeOffset(DateTime.UtcNow.AddSeconds(1)); + new [] { new Exception() }, + }; - Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); - _mockHttpClient - .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) - .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => - { - var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests); - response.Headers.RetryAfter = new RetryConditionHeaderValue(retryAfterDate); - return response; - }); - - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger()); - - // Act - var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - - // Assert - Assert.True(initialResult); - Assert.False(subsequentResult); - Assert.True(_retryAfter != null && _retryAfter.Date == retryAfterDate); - } - - [Fact] - public async Task ItDoesNotProcessResponseIfTooManyRequestsRetryAfterHeaderSeconds() + [Theory] + [MemberData(nameof(ExternalStateUpdateExceptions))] + public async Task ItDoesNotAllowExternalStateUpdateHandlerExceptionToBubble(Exception exception) { // Arrange - static void nopAction(IReadOnlyCollection features) { } + var exceptioningUpdater = new Mock(); + exceptioningUpdater.Setup(m => m.UpdateState(It.Is((IReadOnlyCollection features) => features.Any(config => config.FeatureKey == _testFeatureConfig.FeatureKey)), It.IsAny())) + .ThrowsAsync(exception); - Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); - _mockHttpClient - .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) - .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => - { - var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests); - response.Headers.RetryAfter = new RetryConditionHeaderValue(TimeSpan.FromSeconds(1)); - return response; - }); - - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger()); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, new[] { exceptioningUpdater.Object, _mockStateUpdater.Object }, new NullLogger()); // Act - var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); + var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); // Assert - Assert.True(initialResult); - Assert.False(subsequentResult); - Assert.True(_retryAfter != null && _retryAfter.Date != null); + Assert.Null(result); + Assert.NotNull(_nullETag); + _mockStateUpdater.Verify(); // Verify StateUpdater was called, even though exceptioningUpdater appears 1st in the list of update handlers } - [Fact] - public async Task ItDoesNotProcessResponseIfTooManyRequestsRetryAfterHeaderNull() + + public static object[][] FeatureBoardStateUpdaterExceptions => new[] + { + new Exception[] { new ArgumentException() }, + new Exception[] { new TaskCanceledException() } + }; + + [Theory] + [MemberData(nameof(FeatureBoardStateUpdaterExceptions))] + public async Task ItDoesNotAllowFeatureBoardUpdateHandlerExceptionToBubbleAndDoesNotUpdateETag(Exception exception) { // Arrange - static void nopAction(IReadOnlyCollection features) { } + var otherHandler = new Mock(); + otherHandler.Setup(m => m.UpdateState(It.IsAny>(), It.IsAny())); - Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); - _mockHttpClient - .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) - .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => - { - var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests); - return response; - }); + _mockStateUpdater.Setup(m => m.UpdateState(It.Is((IReadOnlyCollection features) => features.Any(config => config.FeatureKey == _testFeatureConfig.FeatureKey)), It.IsAny())) + .ThrowsAsync(exception); // eg. what would happen if duplicate feature keys are returned - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger()); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, new[] { otherHandler.Object, _mockStateUpdater.Object, }, new NullLogger()); // Act - var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); - var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); + var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); // Assert - Assert.True(initialResult); - Assert.False(subsequentResult); - Assert.True(_retryAfter != null && _retryAfter.Date != null); + Assert.Null(result); + Assert.Null(_nullETag); + Assert.Empty(otherHandler.Invocations); // Verify "other" handler is never even called after our FeatureBoardStateUpdater exceptioned } - [Theory] - [InlineData(HttpStatusCode.NotFound)] - [InlineData(HttpStatusCode.InternalServerError)] - [InlineData(HttpStatusCode.ServiceUnavailable)] - [InlineData(HttpStatusCode.Accepted)] - [InlineData(HttpStatusCode.NoContent)] - public async Task ItDoesNotProcessResponseOnNonOkayResponse(HttpStatusCode httpStatusCode) + [Fact] + public async Task ItDoesNotAllowTransientNetworkRequestErrorsToBubble() { // Arrange - static void exceptionAction(IReadOnlyCollection features) => throw new InvalidOperationException(); - _mockHttpClient .Setup(client => client.SendAsync(It.Is(_defaultRequestMatcher), It.IsAny())) - .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => new HttpResponseMessage(httpStatusCode) { RequestMessage = request }); - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, exceptionAction, new NullLogger()); + .ThrowsAsync(new HttpRequestException()); + + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, Array.Empty(), new NullLogger()); // Act var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); @@ -256,6 +197,7 @@ public async Task ItDoesNotProcessResponseOnNonOkayResponse(HttpStatusCode httpS } + private static FeatureConfiguration CreateFeature() { var faker = new Faker(); diff --git a/libs/dotnet-sdk-test/State/FeatureboardStateTests.cs b/libs/dotnet-sdk-test/State/FeatureboardStateTests.cs index 34c8a8df..e49768c1 100644 --- a/libs/dotnet-sdk-test/State/FeatureboardStateTests.cs +++ b/libs/dotnet-sdk-test/State/FeatureboardStateTests.cs @@ -22,12 +22,44 @@ public FeatureBoardStateTests() } - [Fact] - public async Task StartAsyncLoadsStateFromExternalStateIfProvided() + [Theory] + [InlineData(false)] + [InlineData(null)] + public async Task StartAsyncLoadsStateFromExternalStateIfFeatureBoardServiceDoesNotUpdate(bool? serviceReturn) + { + // Arrange + var featureConfiguration = CreateFeature(); + + Services.AddServiceMock((_, mock) => + mock.Setup(x => x.RefreshFeatureConfiguration(It.IsAny())) + .ReturnsAsync(serviceReturn) + ); + Services.AddServiceMock((_, mock) => + mock + .Setup(x => x.GetState(It.IsAny())) + .ReturnsAsync(new[] { featureConfiguration }) + ); + + var featureBoardState = Services.BuildServiceProvider().GetRequiredService(); + + // Act + await featureBoardState.StartAsync(CancellationToken.None); + var resolvedFeature = featureBoardState.GetSnapshot().Get(featureConfiguration.FeatureKey); + + // Assert + resolvedFeature.ShouldBe(featureConfiguration); + } + + [Fact(Skip = "Unclear what the behaviour should be in this scenario")] + public async Task StartAsyncLoadsStateFromExternalStateIfFeatureBoardServiceThrowsException() { // Arrange var featureConfiguration = CreateFeature(); + Services.AddServiceMock((_, mock) => + mock.Setup(x => x.RefreshFeatureConfiguration(It.IsAny())) + .ThrowsAsync(new Exception("Fail!")) + ); Services.AddServiceMock((_, mock) => mock .Setup(x => x.GetState(It.IsAny())) @@ -45,7 +77,7 @@ public async Task StartAsyncLoadsStateFromExternalStateIfProvided() } [Fact] - public async Task StartAsyncLoadsStateFromServiceIfExternalStateNotProvided() + public async Task StartAsyncLoadsStateFromServiceEvenIfExternalStateNotProvided() { // Arrange var featureConfiguration = CreateFeature(); diff --git a/libs/dotnet-sdk/FeatureBoard.DotnetSdk.csproj b/libs/dotnet-sdk/FeatureBoard.DotnetSdk.csproj index a2ce402a..41faff94 100644 --- a/libs/dotnet-sdk/FeatureBoard.DotnetSdk.csproj +++ b/libs/dotnet-sdk/FeatureBoard.DotnetSdk.csproj @@ -35,6 +35,7 @@ + diff --git a/libs/dotnet-sdk/FeatureBoardClient.cs b/libs/dotnet-sdk/FeatureBoardClient.cs index d30c8db7..8871de57 100644 --- a/libs/dotnet-sdk/FeatureBoardClient.cs +++ b/libs/dotnet-sdk/FeatureBoardClient.cs @@ -1,22 +1,21 @@ -using System; using System.Linq.Expressions; using System.Reflection; using System.Text.Json.Nodes; -using Microsoft.Extensions.Logging; using FeatureBoard.DotnetSdk.Attributes; using FeatureBoard.DotnetSdk.Helpers; using FeatureBoard.DotnetSdk.Models; using FeatureBoard.DotnetSdk.State; +using Microsoft.Extensions.Logging; namespace FeatureBoard.DotnetSdk; internal class FeatureBoardClient : IFeatureBoardClient where TFeatures : class, IFeatures { - private readonly FeatureBoardStateSnapshot _state; + private readonly Lazy _state; private readonly IAudienceProvider _audienceProvider; private readonly ILogger _logger; - public FeatureBoardClient(FeatureBoardStateSnapshot state, IAudienceProvider audienceProvider, ILogger> logger) + public FeatureBoardClient(Lazy state, IAudienceProvider audienceProvider, ILogger> logger) { _state = state; _audienceProvider = audienceProvider; @@ -84,7 +83,7 @@ private static TProp GetFeatureValue(Expression> e private JsonValue? GetFeatureConfigurationValue(string featureKey, string? defaultValue) { - var feature = _state.Get(featureKey); + var feature = _state.Value.Get(featureKey); if (feature == null) { _logger.LogDebug("GetFeatureValue - no value, returning user fallback: {defaultValue}", defaultValue); diff --git a/libs/dotnet-sdk/FeatureBoardHttpClient.cs b/libs/dotnet-sdk/FeatureBoardHttpClient.cs index 9ed268ae..065d872f 100644 --- a/libs/dotnet-sdk/FeatureBoardHttpClient.cs +++ b/libs/dotnet-sdk/FeatureBoardHttpClient.cs @@ -1,33 +1,31 @@ using System.Net; +using System.Net.Http.Headers; using System.Net.Http.Json; -using Microsoft.Extensions.Logging; using FeatureBoard.DotnetSdk.Models; -using System.Net.Http.Headers; +using Microsoft.Extensions.Logging; namespace FeatureBoard.DotnetSdk; public delegate ref EntityTagHeaderValue? LastETagProvider(); -public delegate ref RetryConditionHeaderValue? RetryAfterProvider(); + internal sealed class FeatureBoardHttpClient : IFeatureBoardHttpClient { + private readonly IOrderedEnumerable _featureConfigurationUpdatedHandlers; + internal static readonly string Action = "all"; private LastETagProvider _eTag; - private RetryAfterProvider _retryAfter; private readonly HttpClient _httpClient; - private readonly Action> _processResult; private readonly ILogger _logger; - - public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifiedTimeProvider, RetryAfterProvider retryAfterProvider, Action> processResult, ILogger logger) + public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifiedTimeProvider, IEnumerable updateHandlers, ILogger logger) { _httpClient = httpClient; - _processResult = processResult; - _logger = logger; _eTag = lastModifiedTimeProvider; - _retryAfter = retryAfterProvider; + _logger = logger; + _featureConfigurationUpdatedHandlers = updateHandlers.OrderByDescending(h => h is State.FeatureBoardStateUpdater); // register state update handlers, ensuring "ours" is always first } public async Task RefreshFeatureConfiguration(CancellationToken cancellationToken) @@ -37,68 +35,57 @@ public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifi if (null != eTag) request.Headers.IfNoneMatch.Add(eTag); - var retryAfter = _retryAfter(); - if (retryAfter != null && retryAfter.Date > DateTimeOffset.Now) + IReadOnlyCollection? features = null; + try { - // Do not call API - _logger.LogWarning("Failed to get latest flags. Service returned error 429 (Too Many Requests). Retry after: {retryAfter}", retryAfter.Date.Value.ToString()); - return false; - } - using var response = await _httpClient.SendAsync(request, cancellationToken); + using var response = await _httpClient.SendAsync(request, cancellationToken); - retryAfter = updateRetryAfterRef(response.Headers.RetryAfter); + switch (response.StatusCode) + { + case HttpStatusCode.NotModified: + _logger.LogDebug("No changes"); + return false; - switch (response.StatusCode) + case not HttpStatusCode.OK: + _logger.LogError("Failed to get latest flags: Service returned error {statusCode}({responseBody})", response.StatusCode, await response.Content.ReadAsStringAsync()); + return null; + } + + features = await response.Content.ReadFromJsonAsync>(cancellationToken: cancellationToken) + ?? throw new ApplicationException("Unable to retrieve decode response content"); + + updateEtagRef(response.Headers.ETag ?? eTag); + _logger.LogDebug("Fetching updates done, eTag={eTag}", eTag); + } + catch (HttpRequestException e) { - case HttpStatusCode.NotModified: - _logger.LogDebug("No changes"); - return false; - - //HttpStatusCode.TooManyRequests not defined in .NET Standards 2.0 - case (HttpStatusCode)429: - if (response.Headers.RetryAfter == null) - { - // No retry after header set, hold back call to client api for 60 seconds - var retryAfterDate = DateTimeOffset.Now.AddMinutes(1); - retryAfter = updateRetryAfterRef(new RetryConditionHeaderValue(retryAfterDate)); - } - _logger.LogWarning("Failed to get latest flags. Service returned error 429 (Too Many Requests). Retry after: {retryAfter}", retryAfter!.Date!.Value.ToString()); - return false; - - case not HttpStatusCode.OK: - _logger.LogError("Failed to get latest flags: Service returned error {statusCode}({responseBody})", response.StatusCode, await response.Content.ReadAsStringAsync()); - return null; + _logger.LogError(e, "Failed to get latest flags"); + return null; } - var features = await response.Content.ReadFromJsonAsync>(cancellationToken: cancellationToken) - ?? throw new ApplicationException("Unable to retrieve decode response content"); + try + { + foreach (var handler in _featureConfigurationUpdatedHandlers) // DEBT: find way to use Task.WhenAll but if first task exceptions abort/cancel all others + await handler.UpdateState(features, cancellationToken); + return true; + } + catch (Exception e) when (e is ArgumentException // eg. thrown due to null or duplicate feature key + || e is TaskCanceledException) + { + updateEtagRef(eTag); // revert to original eTag since we failed to update featureboard internal state + _logger.LogError(e, "Failed to update flags"); + } + catch (Exception e) // all other exceptions just log but assume we at least successfully updated featureboard internal state + { + _logger.LogError(e, "Failed to update flags"); + } - _processResult(features); - updateEtagRef(response.Headers.ETag); + return null; void updateEtagRef(EntityTagHeaderValue? responseTag) // Sync method to allow use of eTag ref-local variable { ref var eTag = ref _eTag(); - eTag = responseTag ?? eTag; // if didn't get eTag just retain previous eTag - _logger.LogDebug("Fetching updates done, eTag={eTag}", _eTag); + eTag = responseTag; } - - RetryConditionHeaderValue? updateRetryAfterRef(RetryConditionHeaderValue? responseRetryAfter) // Sync method to allow use of eTag ref-local variable - { - ref var retryAfter = ref _retryAfter(); - if (responseRetryAfter?.Date == null && responseRetryAfter?.Delta != null) - { - var retryAfterDate = DateTimeOffset.Now + responseRetryAfter.Delta.Value; - retryAfter = new RetryConditionHeaderValue(retryAfterDate); - } - else - { - retryAfter = responseRetryAfter; - } - - return _retryAfter(); - } - - return true; } } diff --git a/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs b/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs index d01d8958..c4c1e516 100644 --- a/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs +++ b/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs @@ -1,3 +1,4 @@ +using System.Net; using System.Net.Http.Headers; using FeatureBoard.DotnetSdk.Models; using FeatureBoard.DotnetSdk.State; @@ -6,6 +7,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Polly; namespace FeatureBoard.DotnetSdk.Registration; @@ -14,7 +16,6 @@ public static class RegisterFeatureBoard private static UpdateStrategy _updateStrategy = UpdateStrategy.None; private static EntityTagHeaderValue? lastETag = null; - private static RetryConditionHeaderValue? retryAfter = null; public static FeatureBoardBuilder AddFeatureBoard(this IServiceCollection services, IConfigurationRoot configuration) where TFeatures : class, IFeatures { @@ -26,18 +27,31 @@ public static FeatureBoardBuilder AddFeatureBoard(this IServiceCollec client.BaseAddress = options.Value.HttpEndpoint; client.DefaultRequestHeaders.Add("x-environment-key", options.Value.EnvironmentApiKey); // client.Timeout = options.Value.MaxAge - TimeSpan.FromMilliseconds(3); //prevent multiple requests running at the same time. - }); + }).AddTransientHttpErrorPolicy(static policyBuilder => // DEBT: Get number retries from config + policyBuilder +#if NET6_0_OR_GREATER + .OrResult(result => result.StatusCode == HttpStatusCode.TooManyRequests) +#else + .OrResult(result => (int)result.StatusCode == 429) +#endif + .WaitAndRetryAsync( + retryCount: 5, + sleepDurationProvider: static (retryAttempt, response, context) => + response.Result?.Headers.RetryAfter?.Delta // obey retry-after header if present in response + ?? TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)), // fallback to basic exponential backoff if not. TODO: Consider adding jitter + onRetryAsync: static (response, delay, retryAttempt, context) => Task.CompletedTask + ) + ); services.AddSingleton(() => ref lastETag); - services.AddSingleton(() => ref retryAfter); - services.AddScoped, FeatureBoardClient>(); - services.AddScoped(static c => c.GetRequiredService>()); + services.AddScoped, FeatureBoardClient>() + .AddScoped(static c => c.GetRequiredService>()); - services.AddSingleton(); - services.AddHostedService(static provider => provider.GetRequiredService()); - services.AddTransient>>(static provider => provider.GetRequiredService().Update); - services.AddScoped(static provider => provider.GetRequiredService().GetSnapshot()); + services.AddSingleton() + .AddHostedService(static provider => provider.GetRequiredService()) + .AddScoped(static provider => new Lazy(provider.GetRequiredService().GetSnapshot)) + .AddTransient(); return new FeatureBoardBuilder(services); } @@ -73,7 +87,8 @@ public static FeatureBoardBuilder WithPollingUpdateStrategy(this FeatureBoardBui public static FeatureBoardBuilder WithExternalState(this FeatureBoardBuilder builder) where TStateStore : class, IFeatureBoardExternalState { - builder.Services.AddSingleton(); + builder.Services.AddSingleton() + .AddTransient(static provider => provider.GetRequiredService()); return builder; } diff --git a/libs/dotnet-sdk/State/FeatureBoardState.cs b/libs/dotnet-sdk/State/FeatureBoardState.cs index dcdc856a..3a56c408 100644 --- a/libs/dotnet-sdk/State/FeatureBoardState.cs +++ b/libs/dotnet-sdk/State/FeatureBoardState.cs @@ -1,7 +1,6 @@ -using System.Threading; +using FeatureBoard.DotnetSdk.Models; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; -using FeatureBoard.DotnetSdk.Models; namespace FeatureBoard.DotnetSdk.State; @@ -22,16 +21,19 @@ public FeatureBoardState(IServiceScopeFactory scopeFactory, IFeatureBoardExterna public FeatureBoardStateSnapshot GetSnapshot() => new FeatureBoardStateSnapshot(_cache); - public void Update(IReadOnlyCollection state) => _cache = state.ToDictionary(s => s.FeatureKey, s => s); + /// + /// Update internal cache of feature configuration based on supplied collection + /// + /// + /// Thrown if is null or is reused in collection + internal void Update(IReadOnlyCollection state) => _cache = state.ToDictionary(s => s.FeatureKey, s => s); public async Task StartAsync(CancellationToken cancellationToken) { - if (_externalState is null) - { - using var scope = _scopeFactory.CreateScope(); - await scope.ServiceProvider.GetRequiredService().RefreshFeatureConfiguration(cancellationToken); + using var scope = _scopeFactory.CreateScope(); + var updated = await scope.ServiceProvider.GetRequiredService().RefreshFeatureConfiguration(cancellationToken) ?? false; + if (updated || _externalState is null) return; - } var state = await _externalState.GetState(cancellationToken); if (state == null) diff --git a/libs/dotnet-sdk/State/FeatureBoardStateUpdater.cs b/libs/dotnet-sdk/State/FeatureBoardStateUpdater.cs new file mode 100644 index 00000000..844435d3 --- /dev/null +++ b/libs/dotnet-sdk/State/FeatureBoardStateUpdater.cs @@ -0,0 +1,29 @@ +using FeatureBoard.DotnetSdk.Models; + +namespace FeatureBoard.DotnetSdk.State; + +/// +/// Adapter around FeatureBoardState to allow Update to be called in IFeatureBoardStateUpdateHandler conformant manner +/// +internal class FeatureBoardStateUpdater : IFeatureBoardStateUpdateHandler +{ + private readonly FeatureBoardState _state; + + public FeatureBoardStateUpdater(FeatureBoardState state) => _state = state; + + public virtual Task UpdateState(IReadOnlyCollection configuration, CancellationToken cancellation) + { + if (cancellation.IsCancellationRequested) + return Task.FromCanceled(cancellation); + + try + { + _state.Update(configuration); + return Task.CompletedTask; + } + catch (Exception e) + { + return Task.FromException(e); + } + } +} diff --git a/libs/dotnet-sdk/State/IFeatureBoardExternalState.cs b/libs/dotnet-sdk/State/IFeatureBoardExternalState.cs index bcd06cd6..f1218bbb 100644 --- a/libs/dotnet-sdk/State/IFeatureBoardExternalState.cs +++ b/libs/dotnet-sdk/State/IFeatureBoardExternalState.cs @@ -2,8 +2,7 @@ namespace FeatureBoard.DotnetSdk.State; -public interface IFeatureBoardExternalState +public interface IFeatureBoardExternalState : IFeatureBoardStateUpdateHandler { Task> GetState(CancellationToken cancellationToken); - Task UpdateState(IReadOnlyCollection features, CancellationToken cancellationToken); } diff --git a/libs/dotnet-sdk/State/IFeatureBoardStateUpdateHandler.cs b/libs/dotnet-sdk/State/IFeatureBoardStateUpdateHandler.cs new file mode 100644 index 00000000..30def922 --- /dev/null +++ b/libs/dotnet-sdk/State/IFeatureBoardStateUpdateHandler.cs @@ -0,0 +1,8 @@ +using FeatureBoard.DotnetSdk.Models; + +namespace FeatureBoard.DotnetSdk.State; + +public interface IFeatureBoardStateUpdateHandler +{ + Task UpdateState(IReadOnlyCollection configuration, CancellationToken cancellation); +}