Skip to content

Commit ec12f2d

Browse files
authored
refactor: change to use TimeProvider (#466)
Signed-off-by: André Silva <[email protected]>
1 parent 12d6775 commit ec12f2d

File tree

9 files changed

+171
-36
lines changed

9 files changed

+171
-36
lines changed

src/OpenFeature.Providers.Ofrep/Client/OfrepClient.cs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ internal sealed partial class OfrepClient : IOfrepClient
2424
private readonly HttpClient _httpClient;
2525
private DateTimeOffset? _retryAfterDate;
2626
private readonly ILogger _logger;
27+
private readonly TimeProvider _timeProvider;
2728
private bool _disposed;
2829

2930
private static readonly JsonSerializerOptions JsonOptions = new()
@@ -37,8 +38,9 @@ internal sealed partial class OfrepClient : IOfrepClient
3738
/// </summary>
3839
/// <param name="configuration">The OFREP configuration.</param>
3940
/// <param name="logger">The logger for the client.</param>
40-
public OfrepClient(OfrepOptions configuration, ILogger? logger = null)
41-
: this(configuration, CreateDefaultHandler(), logger) // Use helper for default handler
41+
/// <param name="timeProvider">The time provider for time-related operations. Defaults to TimeProvider.System.</param>
42+
public OfrepClient(OfrepOptions configuration, ILogger? logger = null, TimeProvider? timeProvider = null)
43+
: this(configuration, CreateDefaultHandler(), logger, timeProvider) // Use helper for default handler
4244
{
4345
}
4446

@@ -47,7 +49,8 @@ public OfrepClient(OfrepOptions configuration, ILogger? logger = null)
4749
/// </summary>
4850
/// <param name="httpClient">The HttpClient to use for requests. Caller may provide one from IHttpClientFactory.</param>
4951
/// <param name="logger">The logger for the client.</param>
50-
internal OfrepClient(HttpClient httpClient, ILogger? logger = null)
52+
/// <param name="timeProvider">The time provider for time-related operations. Defaults to TimeProvider.System.</param>
53+
internal OfrepClient(HttpClient httpClient, ILogger? logger = null, TimeProvider? timeProvider = null)
5154
{
5255
#if NET8_0_OR_GREATER
5356
ArgumentNullException.ThrowIfNull(httpClient);
@@ -60,6 +63,7 @@ internal OfrepClient(HttpClient httpClient, ILogger? logger = null)
6063

6164
this._logger = logger ?? NullLogger<OfrepClient>.Instance;
6265
this._httpClient = httpClient;
66+
this._timeProvider = timeProvider ?? TimeProvider.System;
6367
}
6468

6569
/// <summary>
@@ -68,7 +72,8 @@ internal OfrepClient(HttpClient httpClient, ILogger? logger = null)
6872
/// <param name="configuration">The OFREP configuration.</param>
6973
/// <param name="handler">The HTTP message handler.</param>
7074
/// <param name="logger">The logger for the client.</param>
71-
internal OfrepClient(OfrepOptions configuration, HttpMessageHandler handler, ILogger? logger = null)
75+
/// <param name="timeProvider">The time provider for time-related operations. Defaults to TimeProvider.System.</param>
76+
internal OfrepClient(OfrepOptions configuration, HttpMessageHandler handler, ILogger? logger = null, TimeProvider? timeProvider = null)
7277
{
7378
#if NET8_0_OR_GREATER
7479
ArgumentNullException.ThrowIfNull(configuration);
@@ -96,6 +101,8 @@ internal OfrepClient(OfrepOptions configuration, HttpMessageHandler handler, ILo
96101
{
97102
this._httpClient.DefaultRequestHeaders.TryAddWithoutValidation(header.Key, header.Value);
98103
}
104+
105+
this._timeProvider = timeProvider ?? TimeProvider.System;
99106
}
100107

101108
/// <inheritdoc/>
@@ -111,7 +118,7 @@ public async Task<OfrepResponse<T>> EvaluateFlag<T>(string flagKey, T defaultVal
111118
}
112119
#endif
113120

114-
if (this._retryAfterDate.HasValue && this._retryAfterDate.Value > DateTimeOffset.UtcNow)
121+
if (this._retryAfterDate.HasValue && this._retryAfterDate.Value > this._timeProvider.GetUtcNow())
115122
{
116123
return new OfrepResponse<T>(flagKey, defaultValue)
117124
{
@@ -190,15 +197,15 @@ private OfrepResponse<T> ProcessTooManyRequestsResponse<T>(string key, T default
190197
var retryAfter = response.Headers.RetryAfter;
191198
if (retryAfter?.Delta.HasValue == true)
192199
{
193-
this._retryAfterDate = DateTimeOffset.UtcNow.Add(retryAfter.Delta.Value);
200+
this._retryAfterDate = this._timeProvider.GetUtcNow().Add(retryAfter.Delta.Value);
194201
}
195202
else if (retryAfter?.Date.HasValue == true)
196203
{
197204
this._retryAfterDate = retryAfter.Date.Value;
198205
}
199206
else
200207
{
201-
this._retryAfterDate = DateTimeOffset.UtcNow.AddMinutes(1);
208+
this._retryAfterDate = this._timeProvider.GetUtcNow().AddMinutes(1);
202209
}
203210

204211
return new OfrepResponse<T>(key, defaultValue)

src/OpenFeature.Providers.Ofrep/DependencyInjection/FeatureBuilderExtensions.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ private static OfrepProvider CreateProvider(IServiceProvider sp, string? domain)
6161
// If no factory/client, let OfrepClient create its own HttpClient
6262
if (httpClient == null)
6363
{
64-
return new OfrepProvider(ofrepOptions); // internal client management
64+
var timeProviderService = sp.GetService<TimeProvider>();
65+
return new OfrepProvider(ofrepOptions, timeProviderService); // internal client management
6566
}
6667

6768
// Allow user to configure the HttpClient
@@ -84,7 +85,8 @@ private static OfrepProvider CreateProvider(IServiceProvider sp, string? domain)
8485
// Build OfrepClient using provided HttpClient and wire into OfrepProvider
8586
var loggerFactory = sp.GetService<ILoggerFactory>();
8687
var logger = loggerFactory?.CreateLogger<OfrepClient>();
87-
var ofrepClient = new OfrepClient(httpClient, logger);
88+
var timeProvider = sp.GetService<TimeProvider>();
89+
var ofrepClient = new OfrepClient(httpClient, logger, timeProvider);
8890
return new OfrepProvider(ofrepClient);
8991
}
9092
}

src/OpenFeature.Providers.Ofrep/OfrepProvider.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,23 @@ public sealed class OfrepProvider : FeatureProvider, IDisposable
2222
/// </summary>
2323
/// <param name="configuration">The OFREP provider configuration.</param>
2424
public OfrepProvider(OfrepOptions configuration)
25+
: this(configuration, null)
26+
{
27+
}
28+
29+
/// <summary>
30+
/// Creates new instance of <see cref="OfrepProvider"/>
31+
/// </summary>
32+
/// <param name="configuration">The OFREP provider configuration.</param>
33+
/// <param name="timeProvider">The time provider for time-related operations. Defaults to TimeProvider.System.</param>
34+
public OfrepProvider(OfrepOptions configuration, TimeProvider? timeProvider)
2535
{
2636
if (configuration == null)
2737
{
2838
throw new ArgumentNullException(nameof(configuration));
2939
}
3040

31-
this._client = new OfrepClient(configuration);
41+
this._client = new OfrepClient(configuration, timeProvider: timeProvider);
3242
}
3343

3444
/// <summary>

src/OpenFeature.Providers.Ofrep/OpenFeature.Providers.Ofrep.csproj

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,25 @@
2020
</PropertyGroup>
2121

2222
<ItemGroup>
23-
<PackageReference Include="Microsoft.Extensions.Logging" Version="8.0.0"/>
24-
<PackageReference Include="Microsoft.Extensions.Http" Version="8.0.0"/>
25-
<PackageReference Include="OpenFeature.DependencyInjection" Version="$(OpenFeatureVersion)"/>
23+
<PackageReference Include="Microsoft.Extensions.Logging" Version="8.0.0" />
24+
<PackageReference Include="Microsoft.Extensions.Http" Version="8.0.0" />
25+
<PackageReference Include="OpenFeature.DependencyInjection" Version="$(OpenFeatureVersion)" />
2626
</ItemGroup>
2727
<PropertyGroup>
2828
<OpenFeatureVersion>[2.2,2.99999]</OpenFeatureVersion>
2929
</PropertyGroup>
3030

3131
<ItemGroup
3232
Condition=" '$(TargetFramework)' == 'net462' or '$(TargetFramework)' == 'netstandard2.0'">
33-
<PackageReference Include="System.Net.Http" Version="4.3.4"/>
34-
<PackageReference Include="System.Net.Http.Json" Version="8.0.1"/>
35-
<PackageReference Include="System.Text.Json" Version="8.0.5"/>
33+
<PackageReference Include="Microsoft.Bcl.TimeProvider" Version="8.0.0" />
34+
<PackageReference Include="System.Net.Http" Version="4.3.4" />
35+
<PackageReference Include="System.Net.Http.Json" Version="8.0.1" />
36+
<PackageReference Include="System.Text.Json" Version="8.0.5" />
3637
</ItemGroup>
3738

3839
<!-- documentation -->
3940
<ItemGroup>
40-
<None Include="README.md" Pack="true" PackagePath="\"/>
41+
<None Include="README.md" Pack="true" PackagePath="\" />
4142
</ItemGroup>
4243

4344
<!-- make the internal methods visble to our test project -->

src/OpenFeature.Providers.Ofrep/README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,18 @@ var provider = new OfrepProvider(config);
6767
await Api.Instance.SetProviderAsync(provider);
6868
```
6969

70+
### TimeProvider Support
71+
72+
For testing and scenarios where you need to control time-related behavior (such as rate limiting), you can provide a custom `TimeProvider`:
73+
74+
```csharp
75+
// Using a custom TimeProvider for testing
76+
var customTimeProvider = new FakeTimeProvider(); // From Microsoft.Extensions.TimeProvider.Testing
77+
var provider = new OfrepProvider(config, customTimeProvider);
78+
```
79+
80+
When using dependency injection, the provider will automatically use any registered `TimeProvider` from the service container.
81+
7082
## Configuration Options
7183

7284
The `OfrepOptions` class supports the following options:

test/OpenFeature.Providers.Ofrep.Test/Client/OfrepClientTest.cs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Text.Json;
66
using Microsoft.Extensions.Logging;
77
using Microsoft.Extensions.Logging.Abstractions;
8+
using Microsoft.Extensions.Time.Testing;
89
using OpenFeature.Model;
910
using OpenFeature.Providers.Ofrep.Client;
1011
using OpenFeature.Providers.Ofrep.Configuration;
@@ -750,6 +751,79 @@ public async Task EvaluateFlag_WithRetryAfterSet_ShouldReturnGeneralErrorForSubs
750751
Assert.Equal("Rate limit exceeded.", secondResult.ErrorMessage);
751752
}
752753

754+
[Fact]
755+
public void Constructor_WithTimeProvider_ShouldInitializeSuccessfully()
756+
{
757+
// Arrange
758+
var startTime = new DateTimeOffset(2023, 1, 1, 12, 0, 0, TimeSpan.Zero);
759+
var fakeTimeProvider = new FakeTimeProvider(startTime);
760+
761+
// Act & Assert - Constructor should complete successfully
762+
using var client = new OfrepClient(this._configuration, this._mockHandler, this._mockLogger, fakeTimeProvider);
763+
Assert.NotNull(client);
764+
}
765+
766+
[Fact]
767+
public async Task EvaluateFlag_WithFakeTimeProvider_ShouldRespectRetryAfterTimeout()
768+
{
769+
// Arrange
770+
const string flagKey = "rate-limited-flag";
771+
const bool defaultValue = false;
772+
var startTime = new DateTimeOffset(2023, 1, 1, 12, 0, 0, TimeSpan.Zero);
773+
var fakeTimeProvider = new FakeTimeProvider(startTime);
774+
775+
// Create a custom mock handler that can set RetryAfter header
776+
var mockHandler = new TestHttpMessageHandler();
777+
var responseWithRetryAfter = new HttpResponseMessage((HttpStatusCode)429)
778+
{
779+
Content = new StringContent("Rate limit exceeded"),
780+
Headers = { RetryAfter = new System.Net.Http.Headers.RetryConditionHeaderValue(TimeSpan.FromSeconds(60)) }
781+
};
782+
783+
// Setup the custom response directly
784+
mockHandler.SetupResponse(responseWithRetryAfter);
785+
786+
using var client = new OfrepClient(this._configuration, mockHandler, this._mockLogger, fakeTimeProvider);
787+
788+
// Act & Assert
789+
// First request should trigger rate limit and set retry after
790+
var firstResult = await client.EvaluateFlag(flagKey, defaultValue, EvaluationContext.Empty);
791+
Assert.Equal("general_error", firstResult.ErrorCode);
792+
Assert.Equal("Rate limit exceeded.", firstResult.ErrorMessage);
793+
794+
// Second request immediately should also be rate limited (no HTTP call due to client-side rate limiting)
795+
var secondResult = await client.EvaluateFlag(flagKey, defaultValue, EvaluationContext.Empty);
796+
Assert.Equal("general_error", secondResult.ErrorCode);
797+
Assert.Equal("Rate limit exceeded.", secondResult.ErrorMessage);
798+
799+
// This should have made only 1 HTTP request (the first one), not 2
800+
Assert.Single(mockHandler.Requests);
801+
802+
// Advance time by 30 seconds - should still be rate limited
803+
fakeTimeProvider.Advance(TimeSpan.FromSeconds(30));
804+
var thirdResult = await client.EvaluateFlag(flagKey, defaultValue, EvaluationContext.Empty);
805+
Assert.Equal("general_error", thirdResult.ErrorCode);
806+
Assert.Equal("Rate limit exceeded.", thirdResult.ErrorMessage);
807+
808+
// Still only 1 HTTP request should have been made
809+
Assert.Single(mockHandler.Requests);
810+
811+
// Advance time by another 31 seconds (total 61 seconds) - rate limit should be cleared
812+
fakeTimeProvider.Advance(TimeSpan.FromSeconds(31));
813+
814+
// Setup successful response for the next call
815+
var successResponse = new OfrepResponse<bool>(flagKey, true) { Reason = "STATIC", Variant = "on" };
816+
var json = JsonSerializer.Serialize(successResponse, this._jsonSerializerCamelCase);
817+
mockHandler.SetupResponse(HttpStatusCode.OK, json);
818+
819+
var fourthResult = await client.EvaluateFlag(flagKey, defaultValue, EvaluationContext.Empty);
820+
Assert.Equal("STATIC", fourthResult.Reason);
821+
Assert.True(fourthResult.Value);
822+
823+
// Now we should have made 2 HTTP requests total
824+
Assert.Equal(2, mockHandler.Requests.Count);
825+
}
826+
753827
#endregion
754828

755829
#region Context Validation Tests

test/OpenFeature.Providers.Ofrep.Test/DependencyInjection/FeatureBuilderExtensionsTests.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Microsoft.Extensions.DependencyInjection;
22
using Microsoft.Extensions.Options;
3+
using Microsoft.Extensions.Time.Testing;
34
using OpenFeature.Providers.Ofrep.DependencyInjection;
45
using Xunit;
56

@@ -242,4 +243,29 @@ public void AddOfrepProvider_Named_Domain_Works()
242243
Assert.IsType<OfrepProvider>(keyedProvider);
243244
}
244245

246+
[Fact]
247+
public void AddOfrepProvider_WithRegisteredTimeProvider_UsesCustomTimeProvider()
248+
{
249+
// Arrange
250+
var customTimeProvider = new FakeTimeProvider(new DateTimeOffset(2023, 1, 1, 12, 0, 0, TimeSpan.Zero));
251+
252+
using var services = new ServiceCollection()
253+
.AddSingleton<TimeProvider>(customTimeProvider)
254+
.AddOpenFeature(builder =>
255+
{
256+
builder.AddOfrepProvider(o => { o.BaseUrl = "https://api.example.com/"; });
257+
})
258+
.BuildServiceProvider();
259+
260+
// Act
261+
var provider = services.GetRequiredService<FeatureProvider>();
262+
263+
// Assert
264+
Assert.NotNull(provider);
265+
Assert.IsType<OfrepProvider>(provider);
266+
267+
// The provider should be created successfully with the custom TimeProvider
268+
// We can't directly verify the TimeProvider usage without making the field public,
269+
// but successful construction indicates it was passed correctly
270+
}
245271
}

test/OpenFeature.Providers.Ofrep.Test/Helpers/TestHttpMessageHandler.cs

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,31 @@ namespace OpenFeature.Providers.Ofrep.Test.Helpers;
88

99
internal class TestHttpMessageHandler : HttpMessageHandler
1010
{
11-
private readonly Queue<(HttpStatusCode statusCode, string content, string? etag, Exception? exception)>
11+
private readonly Queue<(HttpResponseMessage? responseMessage, Exception? exception)>
1212
_responses = new();
1313

1414
private readonly List<HttpRequestMessage> _requests = new();
1515

1616
public IReadOnlyList<HttpRequestMessage> Requests => this._requests.AsReadOnly();
1717

18-
public void SetupResponse(HttpStatusCode statusCode, string content, string? etag = null)
18+
public void SetupResponse(HttpStatusCode statusCode, string content)
1919
{
20-
this._responses.Enqueue((statusCode, content, etag, null));
20+
var httpResponse = new HttpResponseMessage(statusCode)
21+
{
22+
Content = new StringContent(content, Encoding.UTF8, "application/json")
23+
};
24+
25+
this.SetupResponse(httpResponse);
26+
}
27+
28+
public void SetupResponse(HttpResponseMessage responseMessage)
29+
{
30+
this._responses.Enqueue((responseMessage, null));
2131
}
2232

2333
public void SetupException(Exception exception)
2434
{
25-
this._responses.Enqueue((HttpStatusCode.OK, "", null, exception));
35+
this._responses.Enqueue((null, exception));
2636
}
2737

2838
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
@@ -31,11 +41,11 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
3141
this._requests.Add(request);
3242

3343
#if NETFRAMEWORK
34-
var response = this._responses.Count > 0 ? this._responses.Dequeue() : (HttpStatusCode.OK, "{}", null, null);
44+
var response = this._responses.Count > 0 ? this._responses.Dequeue() : (new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("{}", Encoding.UTF8, "application/json") }, null);
3545
#else
3646
if (!this._responses.TryDequeue(out var response))
3747
{
38-
response = (HttpStatusCode.OK, "{}", null, null);
48+
response = (new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("{}", Encoding.UTF8, "application/json") }, null);
3949
}
4050
#endif
4151

@@ -44,19 +54,8 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
4454
throw response.exception;
4555
}
4656

47-
var httpResponse = new HttpResponseMessage(response.statusCode)
48-
{
49-
Content = new StringContent(response.content, Encoding.UTF8, "application/json")
50-
};
51-
52-
if (!string.IsNullOrEmpty(response.etag))
53-
{
54-
httpResponse.Headers.ETag = new System.Net.Http.Headers.EntityTagHeaderValue(response.etag);
55-
}
56-
57-
// If status code indicates an error and no specific exception was set,
58-
// let EnsureSuccessStatusCode handle it
59-
return Task.FromResult(httpResponse);
57+
// Return the pre-built response message
58+
return Task.FromResult(response.responseMessage!);
6059
}
6160

6261
protected override void Dispose(bool disposing)

test/OpenFeature.Providers.Ofrep.Test/OpenFeature.Providers.Ofrep.Test.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414
Include="..\..\src\OpenFeature.Providers.Ofrep\OpenFeature.Providers.Ofrep.csproj" />
1515
</ItemGroup>
1616

17+
<ItemGroup>
18+
<PackageReference Include="Microsoft.Extensions.TimeProvider.Testing" Version="9.9.0" />
19+
</ItemGroup>
20+
1721
<ItemGroup Condition=" '$(TargetFramework)' == 'net9.0' ">
1822
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="9.0.9" />
1923
<PackageReference Include="Microsoft.AspNetCore.TestHost" Version="9.0.9" />

0 commit comments

Comments
 (0)