Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure kv collection refresh settings are not considered unless the feature is enabled. #633

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Ensure kv collection refresh settings are not considered unless the feature is enabled. #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0a99c5
Ensure that kv collection refresh interval is not used unless collect…
jimmyca15 Mar 5, 2025
2248baf
Remove duplicated tests.
jimmyca15 Mar 7, 2025
71c1ff4
fix.
jimmyca15 Mar 7, 2025
946eb55
Fix formatting.
jimmyca15 Mar 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions ...Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ public AzureAppConfigurationProvider(IConfigurationClientManager configClientMan

if (options.RegisterAllEnabled)
{
if (options.KvCollectionRefreshInterval <= TimeSpan.Zero)
{
throw new ArgumentException(
$"{nameof(options.KvCollectionRefreshInterval)} must be greater than zero seconds when using RegisterAll for refresh",
nameof(options));
}

MinRefreshInterval = TimeSpan.FromTicks(Math.Min(minWatcherRefreshInterval.Ticks, options.KvCollectionRefreshInterval.Ticks));
}
else if (hasWatchers)
Expand Down Expand Up @@ -206,7 +213,7 @@ public async Task RefreshAsync(CancellationToken cancellationToken)
var utcNow = DateTimeOffset.UtcNow;
IEnumerable<KeyValueWatcher> refreshableIndividualKvWatchers = _options.IndividualKvWatchers.Where(kvWatcher => utcNow >= kvWatcher.NextRefreshTime);
IEnumerable<KeyValueWatcher> refreshableFfWatchers = _options.FeatureFlagWatchers.Where(ffWatcher => utcNow >= ffWatcher.NextRefreshTime);
bool isRefreshDue = utcNow >= _nextCollectionRefreshTime;
bool isRefreshDue = _options.RegisterAllEnabled && utcNow >= _nextCollectionRefreshTime;

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

if (isRefreshDue)
if (_options.RegisterAllEnabled && isRefreshDue)
{
_nextCollectionRefreshTime = DateTimeOffset.UtcNow.Add(_options.KvCollectionRefreshInterval);
}
Expand Down
65 changes: 33 additions & 32 deletions tests/Tests.AzureAppConfiguration/RefreshTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public async Task RefreshTests_RefreshIsNotSkippedIfCacheIsExpired()
_kvCollection[0] = TestHelpers.ChangeValue(FirstKeyValue, "newValue");

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

[Fact]
public async void RefreshTests_SingleServerCallOnSimultaneousMultipleRefresh()
public async Task RefreshTests_SingleServerCallOnSimultaneousMultipleRefresh()
{
var keyValueCollection = new List<ConfigurationSetting>(_kvCollection);
var requestCount = 0;
var mockResponse = new Mock<Response>();
var mockClient = new Mock<ConfigurationClient>(MockBehavior.Strict);

// Define delay for async operations
var operationDelay = TimeSpan.FromSeconds(6);

mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
.Returns(() =>
{
requestCount++;
Thread.Sleep(6000);

var copy = new List<ConfigurationSetting>();
foreach (var setting in keyValueCollection)
{
copy.Add(TestHelpers.CloneSetting(setting));
};

return new MockAsyncPageable(copy);
return new MockAsyncPageable(copy, operationDelay);
});

Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken)
async Task<Response<ConfigurationSetting>> GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken)
{
requestCount++;
Thread.Sleep(6000);
await Task.Delay(operationDelay, cancellationToken);

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

mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
.ReturnsAsync((Func<ConfigurationSetting, bool, CancellationToken, Response<ConfigurationSetting>>)GetIfChanged);
.Returns((Func<ConfigurationSetting, bool, CancellationToken, Task<Response<ConfigurationSetting>>>)GetIfChanged);

IConfigurationRefresher refresher = null;

Expand Down Expand Up @@ -512,7 +513,7 @@ Response<ConfigurationSetting> GetIfChanged(ConfigurationSetting setting, bool o
}

[Fact]
public void RefreshTests_RefreshAsyncThrowsOnRequestFailedException()
public async Task RefreshTests_RefreshAsyncThrowsOnRequestFailedException()
{
IConfigurationRefresher refresher = null;
var mockClient = GetMockConfigurationClient();
Expand All @@ -539,7 +540,7 @@ public void RefreshTests_RefreshAsyncThrowsOnRequestFailedException()
.Throws(new RequestFailedException("Request failed."));

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

bool result = await refresher.TryRefreshAsync();
Assert.True(result);
Expand Down Expand Up @@ -651,13 +652,13 @@ public async Task RefreshTests_TryRefreshAsyncReturnsFalseForAuthenticationFaile
FirstKeyValue.Value = "newValue";

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Make sure MinBackoffDuration has ended
Thread.Sleep(100);
await Task.Delay(100);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

Expand Down Expand Up @@ -1020,7 +1021,7 @@ public void RefreshTests_ConfigureRefreshThrowsOnNoRegistration()
}

[Fact]
public void RefreshTests_RefreshIsCancelled()
public async Task RefreshTests_RefreshIsCancelled()
{
IConfigurationRefresher refresher = null;
var mockClient = GetMockConfigurationClient();
Expand All @@ -1043,7 +1044,7 @@ public void RefreshTests_RefreshIsCancelled()
FirstKeyValue.Value = "newValue1";

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

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

// Wait for the cache to expire
Thread.Sleep(1500);
await Task.Delay(1500);

await refresher.RefreshAsync();

Expand Down
13 changes: 9 additions & 4 deletions tests/Tests.AzureAppConfiguration/TestHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,9 @@ class MockAsyncPageable : AsyncPageable<ConfigurationSetting>
{
private readonly List<ConfigurationSetting> _collection = new List<ConfigurationSetting>();
private int _status;
private readonly TimeSpan? _delay;

public MockAsyncPageable(List<ConfigurationSetting> collection)
public MockAsyncPageable(List<ConfigurationSetting> collection, TimeSpan? delay = null)
{
foreach (ConfigurationSetting setting in collection)
{
Expand All @@ -177,6 +178,7 @@ public MockAsyncPageable(List<ConfigurationSetting> collection)
}

_status = 200;
_delay = delay;
}

public void UpdateCollection(List<ConfigurationSetting> newCollection)
Expand Down Expand Up @@ -207,10 +209,13 @@ public void UpdateCollection(List<ConfigurationSetting> newCollection)
}
}

#pragma warning disable 1998
public async override IAsyncEnumerable<Page<ConfigurationSetting>> AsPages(string continuationToken = null, int? pageSizeHint = null)
#pragma warning restore 1998
public override async IAsyncEnumerable<Page<ConfigurationSetting>> AsPages(string continuationToken = null, int? pageSizeHint = null)
{
if (_delay.HasValue)
{
await Task.Delay(_delay.Value);
}

yield return Page<ConfigurationSetting>.FromValues(_collection, null, new MockResponse(_status));
}
}
Expand Down