only check response time for refresh trigger engine

zhiyuanliang-ms · zhiyuanliang-ms · commit 5a597c43eb38 · 2025-11-05T01:21:56.000+08:00
diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs
diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs
@@ -74,7 +74,7 @@ public static async Task<KeyValueChange> GetKeyValueChange(this ConfigurationCli
             };
         }
 
-        public static async Task<Page<ConfigurationSetting>> GetPageChange(this ConfigurationClient client, KeyValueSelector keyValueSelector, IEnumerable<PageWatcher> pageWatchers, IConfigurationSettingPageIterator pageIterator, bool makeConditionalRequest, CancellationToken cancellationToken)
+        public static async Task<bool> HavePageChange(this ConfigurationClient client, KeyValueSelector keyValueSelector, IEnumerable<PageWatcher> pageWatchers, IConfigurationSettingPageIterator pageIterator, bool makeConditionalRequest, CancellationToken cancellationToken)
         {
             if (pageWatchers == null)
             {
@@ -99,31 +99,29 @@ public static async Task<Page<ConfigurationSetting>> GetPageChange(this Configur
 
             AsyncPageable<ConfigurationSetting> pageable = client.GetConfigurationSettingsAsync(selector, cancellationToken);
 
+            using IEnumerator<PageWatcher> existingPageWatcherEnumerator = pageWatchers.GetEnumerator();
+
             IAsyncEnumerable<Page<ConfigurationSetting>> pages = makeConditionalRequest
                 ? pageable.AsPages(pageIterator, pageWatchers.Select(p => p.Etag))
                 : pageable.AsPages(pageIterator);
 
-            List<PageWatcher> existingWatcherList = pageWatchers.ToList();
-
-            int i = 0;
-
             await foreach (Page<ConfigurationSetting> page in pages.ConfigureAwait(false))
             {
                 using Response response = page.GetRawResponse();
                 DateTimeOffset timestamp = response.GetDate();
 
-                if (i >= existingWatcherList.Count ||
+                if (!existingPageWatcherEnumerator.MoveNext() ||
                     (response.Status == (int)HttpStatusCode.OK &&
-                    timestamp >= existingWatcherList[i].LastUpdateTime &&
-                    !existingWatcherList[i].Etag.IfNoneMatch.Equals(response.Headers.ETag)))
+                    // if the server response time is later than last server response time, the change is considered detected
+                    timestamp >= existingPageWatcherEnumerator.Current.LastServerResponseTime &&
+                    !existingPageWatcherEnumerator.Current.Etag.IfNoneMatch.Equals(response.Headers.ETag)))
                 {
-                    return page;
+                    return true;
                 }
-
-                i++;
             }
 
-            return null;
+            // Need to check if pages were deleted and no change was found within the new shorter list of page
+            return existingPageWatcherEnumerator.MoveNext();
         }
     }
 }
diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/PageWatcher.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/PageWatcher.cs
@@ -9,6 +9,6 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
     internal class PageWatcher
     {
         public MatchConditions Etag { get; set; }
-        public DateTimeOffset LastUpdateTime { get; set; }
+        public DateTimeOffset LastServerResponseTime { get; set; }
     }
 }
diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/RequestTracingOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/RequestTracingOptions.cs
@@ -3,7 +3,6 @@
 //
 using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions;
 using Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement;
-using Microsoft.Extensions.Configuration.AzureAppConfiguration.SnapshotReference;
 using System.Net.Mime;
 using System.Text;
 
diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/WatchedSetting.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/WatchedSetting.cs
@@ -0,0 +1,14 @@
+﻿// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+//
+using Azure.Data.AppConfiguration;
+using System;
+
+namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
+{
+    internal class WatchedSetting
+    {
+        public ConfigurationSetting Setting { get; set; }
+        public DateTimeOffset LastServerResponseTime { get; set; }
+    }
+}
diff --git a/tests/Tests.AzureAppConfiguration/Unit/AfdTests.cs b/tests/Tests.AzureAppConfiguration/Unit/AfdTests.cs
@@ -270,29 +270,18 @@ public async Task AfdTests_WatchedSettingRefreshAll()
             var mockAsyncPageable1 = new MockAsyncPageable(keyValueCollection1, null, 3, responses);
 
             var keyValueCollection2 = new List<ConfigurationSetting>(_kvCollection);
-            keyValueCollection2[3].Value = "old-value";
+            keyValueCollection2[3].Value = "updated-value";
             string page2_etag2 = Guid.NewGuid().ToString();
             var responses2 = new List<MockResponse>()
             {
-                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00")),
-                new MockResponse(200, page2_etag2, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00")) // stale
+                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00")),
+                new MockResponse(200, page2_etag2, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))
             };
             var mockAsyncPageable2 = new MockAsyncPageable(keyValueCollection2, null, 3, responses2);
 
-            var keyValueCollection3 = new List<ConfigurationSetting>(_kvCollection);
-            keyValueCollection3[3].Value = "new-value";
-            string page2_etag3 = Guid.NewGuid().ToString();
-            var responses3 = new List<MockResponse>()
-            {
-                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:03+08:00")),
-                new MockResponse(200, page2_etag3, DateTimeOffset.Parse("2025-10-17T09:00:03+08:00")) // up-to-date
-            };
-            var mockAsyncPageable3 = new MockAsyncPageable(keyValueCollection3, null, 3, responses3);
-
             mockClient.SetupSequence(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
                 .Returns(mockAsyncPageable1)
-                .Returns(mockAsyncPageable2)
-                .Returns(mockAsyncPageable3);
+                .Returns(mockAsyncPageable2);
 
             string etag1 = Guid.NewGuid().ToString();
             var setting = ConfigurationModelFactory.ConfigurationSetting(
@@ -303,21 +292,28 @@ public async Task AfdTests_WatchedSettingRefreshAll()
                 contentType: "text");
 
             string etag2 = Guid.NewGuid().ToString();
+            var oldSetting = ConfigurationModelFactory.ConfigurationSetting(
+                "Sentinel",
+                "old-value",
+                "label",
+                eTag: new ETag(etag2),
+                contentType: "text");
+
+            string etag3 = Guid.NewGuid().ToString();
             var newSetting = ConfigurationModelFactory.ConfigurationSetting(
                 "Sentinel",
                 "new-value",
                 "label",
-                eTag: new ETag(etag2),
+                eTag: new ETag(etag3),
                 contentType: "text");
 
             mockClient.SetupSequence(c => c.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
                 .ReturnsAsync(Response.FromValue(setting, new MockResponse(200, etag1, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00"))))
-                .ReturnsAsync(Response.FromValue(newSetting, new MockResponse(200, etag2, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))))
-                .ReturnsAsync(Response.FromValue(newSetting, new MockResponse(200, etag2, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))));
+                .ReturnsAsync(Response.FromValue(newSetting, new MockResponse(200, etag3, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))));
 
             mockClient.SetupSequence(c => c.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
-                .ReturnsAsync(Response.FromValue(newSetting, new MockResponse(200, etag2, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))))
-                .ReturnsAsync(Response.FromValue(newSetting, new MockResponse(200, etag2, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))));
+                .ReturnsAsync(Response.FromValue(oldSetting, new MockResponse(200, etag2, DateTimeOffset.Parse("2025-10-17T08:59:59+08:00")))) // stale, should not refresh
+                .ReturnsAsync(Response.FromValue(newSetting, new MockResponse(200, etag3, DateTimeOffset.Parse("2025-10-17T09:00:01+08:00"))));
 
             var afdEndpoint = new Uri("https://test.b01.azurefd.net");
             IConfigurationRefresher refresher = null;
@@ -343,14 +339,14 @@ public async Task AfdTests_WatchedSettingRefreshAll()
 
             await refresher.RefreshAsync();
 
-            Assert.Equal("TestValue4", config["TestKey4"]); // should not refresh, because page 2 is out of date
+            Assert.Equal("TestValue4", config["TestKey4"]); // should not refresh, because sentinel is stale
             Assert.Equal("sentinel-value", config["Sentinel"]);
 
             await Task.Delay(1500);
 
             await refresher.RefreshAsync();
 
-            Assert.Equal("new-value", config["TestKey4"]);
+            Assert.Equal("updated-value", config["TestKey4"]);
             Assert.Equal("new-value", config["Sentinel"]);
         }
 
@@ -370,28 +366,30 @@ public async Task AfdTests_RegisterAllRefresh()
             var mockAsyncPageable1 = new MockAsyncPageable(keyValueCollection1, null, 3, responses);
 
             var keyValueCollection2 = new List<ConfigurationSetting>(_kvCollection);
-            keyValueCollection2[3].Value = "new-value";
+            keyValueCollection2[3].Value = "old-value";
             string page2_etag2 = Guid.NewGuid().ToString();
             var responses2 = new List<MockResponse>()
             {
-                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00")), // stale
-                new MockResponse(200, page2_etag2, DateTimeOffset.Parse("2025-10-17T09:00:02+08:00"))
+                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00")),
+                new MockResponse(200, page2_etag2, DateTimeOffset.Parse("2025-10-17T08:59:59+08:00")) // stale, should not refresh
             };
             var mockAsyncPageable2 = new MockAsyncPageable(keyValueCollection2, null, 3, responses2);
 
+            var keyValueCollection3 = new List<ConfigurationSetting>(_kvCollection);
+            keyValueCollection3[3].Value = "new-value";
             var responses3 = new List<MockResponse>()
             {
-                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00")), // stale
+                new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:00+08:00")), // up-to-date, should refresh
                 new MockResponse(200, page2_etag2, DateTimeOffset.Parse("2025-10-17T09:00:02+08:00"))
             };
-            var mockAsyncPageable3 = new MockAsyncPageable(keyValueCollection2, null, 3, responses2);
+            var mockAsyncPageable3 = new MockAsyncPageable(keyValueCollection3, null, 3, responses3);
 
             var responses4 = new List<MockResponse>()
             {
                 new MockResponse(200, page1_etag, DateTimeOffset.Parse("2025-10-17T09:00:03+08:00")), // up-to-date
                 new MockResponse(200, page2_etag2, DateTimeOffset.Parse("2025-10-17T09:00:03+08:00"))
             };
-            var mockAsyncPageable4 = new MockAsyncPageable(keyValueCollection2, null, 3, responses4);
+            var mockAsyncPageable4 = new MockAsyncPageable(keyValueCollection3, null, 3, responses4);
 
             mockClient.SetupSequence(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
                 .Returns(mockAsyncPageable1)
@@ -423,7 +421,7 @@ public async Task AfdTests_RegisterAllRefresh()
 
             await refresher.RefreshAsync();
 
-            Assert.Equal("TestValue4", config["TestKey4"]); // should not refresh, because page 1 is stale
+            Assert.Equal("TestValue4", config["TestKey4"]); // should not refresh, because page 2 is stale
 
             await Task.Delay(1500);
 

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ public static async Task<KeyValueChange> GetKeyValueChange(this ConfigurationCli`
`74`	`74`	`};`
`75`	`75`	`}`
`76`	`76`
`77`		`- public static async Task<Page<ConfigurationSetting>> GetPageChange(this ConfigurationClient client, KeyValueSelector keyValueSelector, IEnumerable<PageWatcher> pageWatchers, IConfigurationSettingPageIterator pageIterator, bool makeConditionalRequest, CancellationToken cancellationToken)`
	`77`	`+ public static async Task<bool> HavePageChange(this ConfigurationClient client, KeyValueSelector keyValueSelector, IEnumerable<PageWatcher> pageWatchers, IConfigurationSettingPageIterator pageIterator, bool makeConditionalRequest, CancellationToken cancellationToken)`
`78`	`78`	`{`
`79`	`79`	`if (pageWatchers == null)`
`80`	`80`	`{`
`@@ -99,31 +99,29 @@ public static async Task<Page<ConfigurationSetting>> GetPageChange(this Configur`
`99`	`99`
`100`	`100`	`AsyncPageable<ConfigurationSetting> pageable = client.GetConfigurationSettingsAsync(selector, cancellationToken);`
`101`	`101`
	`102`	`+ using IEnumerator<PageWatcher> existingPageWatcherEnumerator = pageWatchers.GetEnumerator();`
	`103`	`+`
`102`	`104`	`IAsyncEnumerable<Page<ConfigurationSetting>> pages = makeConditionalRequest`
`103`	`105`	`? pageable.AsPages(pageIterator, pageWatchers.Select(p => p.Etag))`
`104`	`106`	`: pageable.AsPages(pageIterator);`
`105`	`107`
`106`		`- List<PageWatcher> existingWatcherList = pageWatchers.ToList();`
`107`		`-`
`108`		`- int i = 0;`
`109`		`-`
`110`	`108`	`await foreach (Page<ConfigurationSetting> page in pages.ConfigureAwait(false))`
`111`	`109`	`{`
`112`	`110`	`using Response response = page.GetRawResponse();`
`113`	`111`	`DateTimeOffset timestamp = response.GetDate();`
`114`	`112`
`115`		`- if (i >= existingWatcherList.Count \|\|`
	`113`	`+ if (!existingPageWatcherEnumerator.MoveNext() \|\|`
`116`	`114`	`(response.Status == (int)HttpStatusCode.OK &&`
`117`		`- timestamp >= existingWatcherList[i].LastUpdateTime &&`
`118`		`- !existingWatcherList[i].Etag.IfNoneMatch.Equals(response.Headers.ETag)))`
	`115`	`+ // if the server response time is later than last server response time, the change is considered detected`
	`116`	`+ timestamp >= existingPageWatcherEnumerator.Current.LastServerResponseTime &&`
	`117`	`+ !existingPageWatcherEnumerator.Current.Etag.IfNoneMatch.Equals(response.Headers.ETag)))`
`119`	`118`	`{`
`120`		`- return page;`
	`119`	`+ return true;`
`121`	`120`	`}`
`122`		`-`
`123`		`- i++;`
`124`	`121`	`}`
`125`	`122`
`126`		`- return null;`
	`123`	`+ // Need to check if pages were deleted and no change was found within the new shorter list of page`
	`124`	`+ return existingPageWatcherEnumerator.MoveNext();`
`127`	`125`	`}`
`128`	`126`	`}`
`129`	`127`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,6 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration`
`9`	`9`	`internal class PageWatcher`
`10`	`10`	`{`
`11`	`11`	`public MatchConditions Etag { get; set; }`
`12`		`- public DateTimeOffset LastUpdateTime { get; set; }`
	`12`	`+ public DateTimeOffset LastServerResponseTime { get; set; }`
`13`	`13`	`}`
`14`	`14`	`}`