Skip to content

Commit c1cdea3

Browse files
jimmyca15jimmyca15
authored
Ensure kv collection refresh settings are not considered unless the feature is enabled. (#633)
* Ensure that kv collection refresh interval is not used unless collection based refresh of key-values is enabled. Add tests to ensure that minimum refresh interval is respected for key-values and feature flags. * Remove duplicated tests. * fix. * Fix formatting.
1 parent 961cdeb commit c1cdea3

File tree

3 files changed

+51
-38
lines changed

3 files changed

+51
-38
lines changed

‎src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,13 @@ public AzureAppConfigurationProvider(IConfigurationClientManager configClientMan
118118

119119
if (options.RegisterAllEnabled)
120120
{
121+
if (options.KvCollectionRefreshInterval <= TimeSpan.Zero)
122+
{
123+
throw new ArgumentException(
124+
$"{nameof(options.KvCollectionRefreshInterval)} must be greater than zero seconds when using RegisterAll for refresh",
125+
nameof(options));
126+
}
127+
121128
MinRefreshInterval = TimeSpan.FromTicks(Math.Min(minWatcherRefreshInterval.Ticks, options.KvCollectionRefreshInterval.Ticks));
122129
}
123130
else if (hasWatchers)
@@ -206,7 +213,7 @@ public async Task RefreshAsync(CancellationToken cancellationToken)
206213
var utcNow = DateTimeOffset.UtcNow;
207214
IEnumerable<KeyValueWatcher> refreshableIndividualKvWatchers = _options.IndividualKvWatchers.Where(kvWatcher => utcNow >= kvWatcher.NextRefreshTime);
208215
IEnumerable<KeyValueWatcher> refreshableFfWatchers = _options.FeatureFlagWatchers.Where(ffWatcher => utcNow >= ffWatcher.NextRefreshTime);
209-
bool isRefreshDue = utcNow >= _nextCollectionRefreshTime;
216+
bool isRefreshDue = _options.RegisterAllEnabled && utcNow >= _nextCollectionRefreshTime;
210217

211218
// Skip refresh if mappedData is loaded, but none of the watchers or adapters are refreshable.
212219
if (_mappedData != null &&
@@ -412,7 +419,7 @@ await ExecuteWithFailOverPolicyAsync(clients, async (client) =>
412419
}
413420
}
414421

415-
if (isRefreshDue)
422+
if (_options.RegisterAllEnabled && isRefreshDue)
416423
{
417424
_nextCollectionRefreshTime = DateTimeOffset.UtcNow.Add(_options.KvCollectionRefreshInterval);
418425
}

‎tests/Tests.AzureAppConfiguration/RefreshTests.cs‎

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public async Task RefreshTests_RefreshIsNotSkippedIfCacheIsExpired()
212212
_kvCollection[0] = TestHelpers.ChangeValue(FirstKeyValue, "newValue");
213213

214214
// Wait for the cache to expire
215-
Thread.Sleep(1500);
215+
await Task.Delay(1500);
216216

217217
await refresher.RefreshAsync();
218218

@@ -247,7 +247,7 @@ public async Task RefreshTests_RefreshAllFalseDoesNotUpdateEntireConfiguration()
247247
_kvCollection = _kvCollection.Select(kv => TestHelpers.ChangeValue(kv, "newValue")).ToList();
248248

249249
// Wait for the cache to expire
250-
Thread.Sleep(1500);
250+
await Task.Delay(1500);
251251

252252
await refresher.RefreshAsync();
253253

@@ -284,7 +284,7 @@ public async Task RefreshTests_RefreshAllTrueUpdatesEntireConfiguration()
284284
_kvCollection = _kvCollection.Select(kv => TestHelpers.ChangeValue(kv, "newValue")).ToList();
285285

286286
// Wait for the cache to expire
287-
Thread.Sleep(1500);
287+
await Task.Delay(1500);
288288

289289
await refresher.RefreshAsync();
290290

@@ -356,7 +356,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
356356
keyValueCollection.Remove(keyValueCollection.FirstOrDefault(s => s.Key == "TestKey3" && s.Label == "label"));
357357

358358
// Wait for the cache to expire
359-
Thread.Sleep(1500);
359+
await Task.Delay(1500);
360360

361361
await refresher.RefreshAsync();
362362

@@ -430,7 +430,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
430430
keyValueCollection.Remove(keyValueCollection.FirstOrDefault(s => s.Key == "TestKey3" && s.Label == "label"));
431431

432432
// Wait for the cache to expire
433-
Thread.Sleep(1500);
433+
await Task.Delay(1500);
434434

435435
await refresher.RefreshAsync();
436436

@@ -443,32 +443,33 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
443443
}
444444

445445
[Fact]
446-
public async void RefreshTests_SingleServerCallOnSimultaneousMultipleRefresh()
446+
public async Task RefreshTests_SingleServerCallOnSimultaneousMultipleRefresh()
447447
{
448448
var keyValueCollection = new List<ConfigurationSetting>(_kvCollection);
449449
var requestCount = 0;
450450
var mockResponse = new Mock<Response>();
451451
var mockClient = new Mock<ConfigurationClient>(MockBehavior.Strict);
452452

453+
// Define delay for async operations
454+
var operationDelay = TimeSpan.FromSeconds(6);
455+
453456
mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
454457
.Returns(() =>
455458
{
456459
requestCount++;
457-
Thread.Sleep(6000);
458-
459460
var copy = new List<ConfigurationSetting>();
460461
foreach (var setting in keyValueCollection)
461462
{
462463
copy.Add(TestHelpers.CloneSetting(setting));
463464
};
464465

465-
return new MockAsyncPageable(copy);
466+
return new MockAsyncPageable(copy, operationDelay);
466467
});
467468

468-
Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken)
469+
async Task<Response<ConfigurationSetting>> GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken)
469470
{
470471
requestCount++;
471-
Thread.Sleep(6000);
472+
await Task.Delay(operationDelay, cancellationToken);
472473

473474
var newSetting = keyValueCollection.FirstOrDefault(s => s.Key == setting.Key && s.Label == setting.Label);
474475
var unchanged = (newSetting.Key == setting.Key && newSetting.Label == setting.Label && newSetting.Value == setting.Value);
@@ -477,7 +478,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
477478
}
478479

479480
mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
480-
.ReturnsAsync((Func<ConfigurationSetting, bool, CancellationToken, Response<ConfigurationSetting>>)GetIfChanged);
481+
.Returns((Func<ConfigurationSetting, bool, CancellationToken, Task<Response<ConfigurationSetting>>>)GetIfChanged);
481482

482483
IConfigurationRefresher refresher = null;
483484

@@ -512,7 +513,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
512513
}
513514

514515
[Fact]
515-
public void RefreshTests_RefreshAsyncThrowsOnRequestFailedException()
516+
public async Task RefreshTests_RefreshAsyncThrowsOnRequestFailedException()
516517
{
517518
IConfigurationRefresher refresher = null;
518519
var mockClient = GetMockConfigurationClient();
@@ -539,7 +540,7 @@ public void RefreshTests_RefreshAsyncThrowsOnRequestFailedException()
539540
.Throws(new RequestFailedException("Request failed."));
540541

541542
// Wait for the cache to expire
542-
Thread.Sleep(1500);
543+
await Task.Delay(1500);
543544

544545
Action action = () => refresher.RefreshAsync().Wait();
545546
Assert.Throws<AggregateException>(action);
@@ -575,7 +576,7 @@ public async Task RefreshTests_TryRefreshAsyncReturnsFalseOnRequestFailedExcepti
575576
.Throws(new RequestFailedException("Request failed."));
576577

577578
// Wait for the cache to expire
578-
Thread.Sleep(1500);
579+
await Task.Delay(1500);
579580

580581
bool result = await refresher.TryRefreshAsync();
581582
Assert.False(result);
@@ -608,7 +609,7 @@ public async Task RefreshTests_TryRefreshAsyncUpdatesConfigurationAndReturnsTrue
608609
_kvCollection[0] = TestHelpers.ChangeValue(_kvCollection[0], "newValue");
609610

610611
// Wait for the cache to expire
611-
Thread.Sleep(1500);
612+
await Task.Delay(1500);
612613

613614
bool result = await refresher.TryRefreshAsync();
614615
Assert.True(result);
@@ -651,13 +652,13 @@ public async Task RefreshTests_TryRefreshAsyncReturnsFalseForAuthenticationFaile
651652
FirstKeyValue.Value = "newValue";
652653

653654
// Wait for the cache to expire
654-
Thread.Sleep(1500);
655+
await Task.Delay(1500);
655656

656657
// First call to GetConfigurationSettingAsync does not throw
657658
Assert.True(await refresher.TryRefreshAsync());
658659

659660
// Wait for the cache to expire
660-
Thread.Sleep(1500);
661+
await Task.Delay(1500);
661662

662663
// Second call to GetConfigurationSettingAsync throws KeyVaultReferenceException
663664
Assert.False(await refresher.TryRefreshAsync());
@@ -704,7 +705,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
704705
_kvCollection[0] = TestHelpers.ChangeValue(_kvCollection[0], "newValue");
705706

706707
// Wait for the cache to expire
707-
Thread.Sleep(1500);
708+
await Task.Delay(1500);
708709

709710
await Assert.ThrowsAsync<RequestFailedException>(async () =>
710711
await refresher.RefreshAsync()
@@ -748,7 +749,7 @@ public async Task RefreshTests_UpdatesAllSettingsIfInitialLoadFails()
748749
Assert.Null(configuration["TestKey3"]);
749750

750751
// Make sure MinBackoffDuration has ended
751-
Thread.Sleep(100);
752+
await Task.Delay(100);
752753

753754
// Act
754755
await Assert.ThrowsAsync<RequestFailedException>(async () =>
@@ -763,7 +764,7 @@ await Assert.ThrowsAsync<RequestFailedException>(async () =>
763764
Assert.Null(configuration["TestKey3"]);
764765

765766
// Wait for the cache to expire
766-
Thread.Sleep(1500);
767+
await Task.Delay(1500);
767768

768769
await refresher.RefreshAsync();
769770

@@ -825,7 +826,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
825826
keyValueCollection = keyValueCollection.Select(kv => TestHelpers.ChangeValue(kv, "newValue")).ToList();
826827

827828
// Wait for the cache to expire
828-
Thread.Sleep(1500);
829+
await Task.Delay(1500);
829830

830831
bool firstRefreshResult = await refresher.TryRefreshAsync();
831832
Assert.False(firstRefreshResult);
@@ -835,7 +836,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
835836
Assert.Equal("TestValue3", config["TestKey3"]);
836837

837838
// Wait for the cache to expire
838-
Thread.Sleep(1500);
839+
await Task.Delay(1500);
839840

840841
bool secondRefreshResult = await refresher.TryRefreshAsync();
841842
Assert.True(secondRefreshResult);
@@ -876,7 +877,7 @@ public async Task RefreshTests_RefreshAllTrueForOverwrittenSentinelUpdatesEntire
876877
_kvCollection = _kvCollection.Select(kv => TestHelpers.ChangeValue(kv, "newValue")).ToList();
877878

878879
// Wait for the cache to expire
879-
Thread.Sleep(1500);
880+
await Task.Delay(1500);
880881

881882
await refresher.RefreshAsync();
882883

@@ -917,7 +918,7 @@ public async Task RefreshTests_RefreshAllFalseForOverwrittenSentinelUpdatesConfi
917918
_kvCollection[_kvCollection.IndexOf(refreshRegisteredSetting)] = TestHelpers.ChangeValue(refreshRegisteredSetting, "UpdatedValueForLabel1");
918919

919920
// Wait for the cache to expire
920-
Thread.Sleep(1500);
921+
await Task.Delay(1500);
921922

922923
await refresher.RefreshAsync();
923924

@@ -959,7 +960,7 @@ public async Task RefreshTests_RefreshRegisteredKvOverwritesSelectedKv()
959960
_kvCollection[_kvCollection.IndexOf(refreshAllRegisteredSetting)] = TestHelpers.ChangeValue(refreshAllRegisteredSetting, "UpdatedValueForLabel1");
960961

961962
// Wait for the cache to expire
962-
Thread.Sleep(1500);
963+
await Task.Delay(1500);
963964

964965
await refresher.RefreshAsync();
965966

@@ -1020,7 +1021,7 @@ public void RefreshTests_ConfigureRefreshThrowsOnNoRegistration()
10201021
}
10211022

10221023
[Fact]
1023-
public void RefreshTests_RefreshIsCancelled()
1024+
public async Task RefreshTests_RefreshIsCancelled()
10241025
{
10251026
IConfigurationRefresher refresher = null;
10261027
var mockClient = GetMockConfigurationClient();
@@ -1043,7 +1044,7 @@ public void RefreshTests_RefreshIsCancelled()
10431044
FirstKeyValue.Value = "newValue1";
10441045

10451046
// Wait for the cache to expire
1046-
Thread.Sleep(1500);
1047+
await Task.Delay(1500);
10471048

10481049
using var cancellationSource = new CancellationTokenSource();
10491050
cancellationSource.Cancel();
@@ -1087,7 +1088,7 @@ public async Task RefreshTests_SelectedKeysRefreshWithRegisterAll()
10871088
_kvCollection[2].Value = "newValue3";
10881089

10891090
// Wait for the cache to expire
1090-
Thread.Sleep(1500);
1091+
await Task.Delay(1500);
10911092

10921093
await refresher.RefreshAsync();
10931094

@@ -1097,7 +1098,7 @@ public async Task RefreshTests_SelectedKeysRefreshWithRegisterAll()
10971098
_kvCollection.RemoveAt(2);
10981099

10991100
// Wait for the cache to expire
1100-
Thread.Sleep(1500);
1101+
await Task.Delay(1500);
11011102

11021103
await refresher.RefreshAsync();
11031104

@@ -1198,7 +1199,7 @@ MockAsyncPageable GetTestKeys(SettingSelector selector, CancellationToken ct)
11981199
eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1"));
11991200

12001201
// Wait for the cache to expire
1201-
Thread.Sleep(1500);
1202+
await Task.Delay(1500);
12021203

12031204
await refresher.RefreshAsync();
12041205

@@ -1209,7 +1210,7 @@ MockAsyncPageable GetTestKeys(SettingSelector selector, CancellationToken ct)
12091210
featureFlags.RemoveAt(0);
12101211

12111212
// Wait for the cache to expire
1212-
Thread.Sleep(1500);
1213+
await Task.Delay(1500);
12131214

12141215
await refresher.RefreshAsync();
12151216

‎tests/Tests.AzureAppConfiguration/TestHelper.cs‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,9 @@ class MockAsyncPageable : AsyncPageable<ConfigurationSetting>
164164
{
165165
private readonly List<ConfigurationSetting> _collection = new List<ConfigurationSetting>();
166166
private int _status;
167+
private readonly TimeSpan? _delay;
167168

168-
public MockAsyncPageable(List<ConfigurationSetting> collection)
169+
public MockAsyncPageable(List<ConfigurationSetting> collection, TimeSpan? delay = null)
169170
{
170171
foreach (ConfigurationSetting setting in collection)
171172
{
@@ -177,6 +178,7 @@ public MockAsyncPageable(List<ConfigurationSetting> collection)
177178
}
178179

179180
_status = 200;
181+
_delay = delay;
180182
}
181183

182184
public void UpdateCollection(List<ConfigurationSetting> newCollection)
@@ -207,10 +209,13 @@ public void UpdateCollection(List<ConfigurationSetting> newCollection)
207209
}
208210
}
209211

210-
#pragma warning disable 1998
211-
public async override IAsyncEnumerable<Page<ConfigurationSetting>> AsPages(string continuationToken = null, int? pageSizeHint = null)
212-
#pragma warning restore 1998
212+
public override async IAsyncEnumerable<Page<ConfigurationSetting>> AsPages(string continuationToken = null, int? pageSizeHint = null)
213213
{
214+
if (_delay.HasValue)
215+
{
216+
await Task.Delay(_delay.Value);
217+
}
218+
214219
yield return Page<ConfigurationSetting>.FromValues(_collection, null, new MockResponse(_status));
215220
}
216221
}

0 commit comments

Comments
 (0)