Skip to content

Commit 6468503

Browse files
zhiyuanliang-msamerjusupovic
zhiyuanliang-ms
and
amerjusupovic
authored
Merge main to release/v8 (#691)
* Merge pull request #684 from Azure/zhiyuanliang/fix-test Fix activity source test bug * Fix bug with endpoint failover (#686) * update endpoint in do while * add test * in progress * update test, update logic to backoff using correct endpoint * make test more specific --------- Co-authored-by: AMER JUSUPOVIC <[email protected]> Co-authored-by: Amer Jusupovic <[email protected]> * Support comment in json key value (#685) * support jsonc * remove unused reference * use private static option * Add request tracing for Aspire usage (#687) * add request tracing for aspire component * update tag name * version bump 8.4.0 (#690) --------- Co-authored-by: AMER JUSUPOVIC <[email protected]> Co-authored-by: Amer Jusupovic <[email protected]>
2 parents 51ad0c6 + c17524a commit 6468503

File tree

11 files changed

+200
-7
lines changed

11 files changed

+200
-7
lines changed

‎src/Microsoft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
<!-- Nuget Package Version Settings -->
2222

2323
<PropertyGroup>
24-
<OfficialVersion>8.3.0</OfficialVersion>
24+
<OfficialVersion>8.4.0</OfficialVersion>
2525
</PropertyGroup>
2626

2727
<PropertyGroup Condition="'$(CDP_PATCH_NUMBER)'!='' AND '$(CDP_BUILD_TYPE)'=='Official'">

‎src/Microsoft.Azure.AppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
<!-- Nuget Package Version Settings -->
2525

2626
<PropertyGroup>
27-
<OfficialVersion>8.3.0</OfficialVersion>
27+
<OfficialVersion>8.4.0</OfficialVersion>
2828
</PropertyGroup>
2929

3030
<PropertyGroup Condition="'$(CDP_PATCH_NUMBER)'!='' AND '$(CDP_BUILD_TYPE)'=='Official'">

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,7 @@ private async Task<T> ExecuteWithFailOverPolicyAsync<T>(
12141214

12151215
do
12161216
{
1217-
UpdateClientBackoffStatus(previousEndpoint, success);
1217+
UpdateClientBackoffStatus(_configClientManager.GetEndpointForClient(currentClient), success);
12181218

12191219
clientEnumerator.MoveNext();
12201220

@@ -1331,6 +1331,8 @@ private void EnsureAssemblyInspected()
13311331

13321332
_requestTracingOptions.FeatureManagementAspNetCoreVersion = TracingUtils.GetAssemblyVersion(RequestTracingConstants.FeatureManagementAspNetCoreAssemblyName);
13331333

1334+
_requestTracingOptions.AspireComponentVersion = TracingUtils.GetAssemblyVersion(RequestTracingConstants.AspireComponentAssemblyName);
1335+
13341336
if (TracingUtils.GetAssemblyVersion(RequestTracingConstants.SignalRAssemblyName) != null)
13351337
{
13361338
_requestTracingOptions.IsSignalRUsed = true;

‎src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/RequestTracingConstants.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ internal class RequestTracingConstants
2424
public const string EnvironmentKey = "Env";
2525
public const string FeatureManagementVersionKey = "FMVer";
2626
public const string FeatureManagementAspNetCoreVersionKey = "FMANCVer";
27+
public const string AspireComponentVersionKey = "DNACVer";
2728
public const string DevEnvironmentValue = "Dev";
2829
public const string KeyVaultConfiguredTag = "UsesKeyVault";
2930
public const string KeyVaultRefreshConfiguredTag = "RefreshesKeyVault";
@@ -53,6 +54,7 @@ internal class RequestTracingConstants
5354

5455
public const string FeatureManagementAssemblyName = "Microsoft.FeatureManagement";
5556
public const string FeatureManagementAspNetCoreAssemblyName = "Microsoft.FeatureManagement.AspNetCore";
57+
public const string AspireComponentAssemblyName = "Aspire.Microsoft.Extensions.Configuration.AzureAppConfiguration";
5658
public const string SignalRAssemblyName = "Microsoft.AspNetCore.SignalR";
5759

5860
public const string Delimiter = "+";

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
1515
{
1616
internal class JsonKeyValueAdapter : IKeyValueAdapter
1717
{
18+
private static readonly JsonDocumentOptions JsonParseOptions = new JsonDocumentOptions
19+
{
20+
CommentHandling = JsonCommentHandling.Skip
21+
};
22+
1823
public Task<IEnumerable<KeyValuePair<string, string>>> ProcessKeyValue(ConfigurationSetting setting, Uri endpoint, Logger logger, CancellationToken cancellationToken)
1924
{
2025
if (setting == null)
@@ -28,7 +33,7 @@ public Task<IEnumerable<KeyValuePair<string, string>>> ProcessKeyValue(Configura
2833

2934
try
3035
{
31-
using (JsonDocument document = JsonDocument.Parse(rootJson))
36+
using (JsonDocument document = JsonDocument.Parse(rootJson, JsonParseOptions))
3237
{
3338
keyValuePairs = new JsonFlattener().FlattenJson(document.RootElement);
3439
}

‎src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
<!-- Nuget Package Version Settings -->
3939

4040
<PropertyGroup>
41-
<OfficialVersion>8.3.0</OfficialVersion>
41+
<OfficialVersion>8.4.0</OfficialVersion>
4242
</PropertyGroup>
4343

4444
<PropertyGroup Condition="'$(CDP_PATCH_NUMBER)'!='' AND '$(CDP_BUILD_TYPE)'=='Official'">

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ internal class RequestTracingOptions
5050
/// </summary>
5151
public string FeatureManagementAspNetCoreVersion { get; set; }
5252

53+
/// <summary>
54+
/// Version of the Aspire.Microsoft.Extensions.Configuration.AzureAppConfiguration assembly, if present in the application.
55+
/// </summary>
56+
public string AspireComponentVersion { get; set; }
57+
5358
/// <summary>
5459
/// Flag to indicate whether Microsoft.AspNetCore.SignalR assembly is present in the application.
5560
/// </summary>

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,11 @@ private static string CreateCorrelationContextHeader(RequestType requestType, Re
181181
correlationContextKeyValues.Add(new KeyValuePair<string, string>(RequestTracingConstants.FeatureManagementAspNetCoreVersionKey, requestTracingOptions.FeatureManagementAspNetCoreVersion));
182182
}
183183

184+
if (requestTracingOptions.AspireComponentVersion != null)
185+
{
186+
correlationContextKeyValues.Add(new KeyValuePair<string, string>(RequestTracingConstants.AspireComponentVersionKey, requestTracingOptions.AspireComponentVersion));
187+
}
188+
184189
if (requestTracingOptions.UsesAnyTracingFeature())
185190
{
186191
correlationContextKeyValues.Add(new KeyValuePair<string, string>(RequestTracingConstants.FeaturesKey, requestTracingOptions.CreateFeaturesString()));

‎tests/Tests.AzureAppConfiguration/Unit/FailoverTests.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,5 +415,89 @@ ae.InnerException is AggregateException ae2 &&
415415
ae2.InnerExceptions.All(ex => ex is TaskCanceledException) &&
416416
ae2.InnerException is TaskCanceledException tce);
417417
}
418+
419+
[Fact]
420+
public async Task FailOverTests_AllClientsBackedOffAfterNonFailoverableException()
421+
{
422+
IConfigurationRefresher refresher = null;
423+
var mockResponse = new Mock<Response>();
424+
425+
// Setup first client - succeeds on startup, fails with 404 (non-failoverable) on first refresh
426+
var mockClient1 = new Mock<ConfigurationClient>();
427+
mockClient1.SetupSequence(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
428+
.Returns(new MockAsyncPageable(Enumerable.Empty<ConfigurationSetting>().ToList()))
429+
.Throws(new RequestFailedException(412, "Request failed."))
430+
.Throws(new RequestFailedException(412, "Request failed."));
431+
mockClient1.SetupSequence(c => c.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
432+
.Returns(Task.FromResult(Response.FromValue<ConfigurationSetting>(kv, mockResponse.Object)))
433+
.Throws(new RequestFailedException(412, "Request failed."))
434+
.Throws(new RequestFailedException(412, "Request failed."));
435+
mockClient1.SetupSequence(c => c.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
436+
.Throws(new RequestFailedException(412, "Request failed."))
437+
.Throws(new RequestFailedException(412, "Request failed."));
438+
mockClient1.Setup(c => c.Equals(mockClient1)).Returns(true);
439+
440+
// Setup second client - succeeds on startup, should not be called during refresh
441+
var mockClient2 = new Mock<ConfigurationClient>();
442+
mockClient2.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
443+
.Returns(new MockAsyncPageable(Enumerable.Empty<ConfigurationSetting>().ToList()));
444+
mockClient2.Setup(c => c.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
445+
.Returns(Task.FromResult(Response.FromValue<ConfigurationSetting>(kv, mockResponse.Object)));
446+
mockClient2.Setup(c => c.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
447+
.Returns(Task.FromResult(Response.FromValue<ConfigurationSetting>(kv, mockResponse.Object)));
448+
mockClient2.Setup(c => c.Equals(mockClient2)).Returns(true);
449+
450+
ConfigurationClientWrapper cw1 = new ConfigurationClientWrapper(TestHelpers.PrimaryConfigStoreEndpoint, mockClient1.Object);
451+
ConfigurationClientWrapper cw2 = new ConfigurationClientWrapper(TestHelpers.SecondaryConfigStoreEndpoint, mockClient2.Object);
452+
453+
var clientList = new List<ConfigurationClientWrapper>() { cw1, cw2 };
454+
var configClientManager = new ConfigurationClientManager(clientList);
455+
456+
// Verify 2 clients are available
457+
Assert.Equal(2, configClientManager.GetClients().Count());
458+
459+
// Act & Assert - Build configuration successfully with both clients
460+
var config = new ConfigurationBuilder()
461+
.AddAzureAppConfiguration(options =>
462+
{
463+
options.ClientManager = configClientManager;
464+
options.Select("TestKey*");
465+
options.ConfigureRefresh(refreshOptions =>
466+
{
467+
refreshOptions.Register("TestKey1", "label")
468+
.SetRefreshInterval(TimeSpan.FromSeconds(1));
469+
});
470+
471+
options.ReplicaDiscoveryEnabled = false;
472+
refresher = options.GetRefresher();
473+
}).Build();
474+
475+
// First refresh - should call client 1 and fail with non-failoverable exception
476+
// This should cause all clients to be backed off
477+
await Task.Delay(1500);
478+
await refresher.TryRefreshAsync();
479+
480+
// Verify that client 1 was called during the first refresh
481+
mockClient1.Verify(mc => mc.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
482+
mockClient1.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
483+
mockClient1.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
484+
485+
// Verify that client 2 was not called during the first refresh
486+
mockClient2.Verify(mc => mc.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()), Times.Never);
487+
mockClient2.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never);
488+
mockClient2.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()), Times.Never);
489+
490+
// Second refresh - no clients should be called as all are backed off
491+
await Task.Delay(1500);
492+
await refresher.TryRefreshAsync();
493+
494+
// Verify that no additional calls were made to any client during the second refresh
495+
mockClient1.Verify(mc => mc.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
496+
mockClient1.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
497+
mockClient1.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()), Times.Exactly(1));
498+
mockClient2.Verify(mc => mc.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()), Times.Never);
499+
mockClient2.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never);
500+
mockClient2.Verify(mc => mc.GetConfigurationSettingAsync(It.IsAny<ConfigurationSetting>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()), Times.Never);
501+
}
418502
}
419503
}

‎tests/Tests.AzureAppConfiguration/Unit/JsonContentTypeTests.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,90 @@ public void JsonContentTypeTests_JsonKeyValueAdapterCannotProcessKeyVaultReferen
276276
Assert.False(jsonKeyValueAdapter.CanProcess(setting));
277277
}
278278

279+
[Fact]
280+
public void JsonContentTypeTests_LoadJsonValuesWithComments()
281+
{
282+
List<ConfigurationSetting> _kvCollection = new List<ConfigurationSetting>
283+
{
284+
// Test various comment styles and positions
285+
ConfigurationModelFactory.ConfigurationSetting(
286+
key: "MixedCommentStyles",
287+
value: @"{
288+
// Single line comment at start
289+
""ApiSettings"": {
290+
""BaseUrl"": ""https://api.example.com"", // Inline single line
291+
/* Multi-line comment
292+
spanning multiple lines */
293+
""ApiKey"": ""secret-key"",
294+
""Endpoints"": [
295+
// Comment before array element
296+
""/users"",
297+
/* Comment between elements */
298+
""/orders"",
299+
""/products"" // Comment after element
300+
]
301+
},
302+
// Test edge cases
303+
""StringWithSlashes"": ""This is not a // comment"",
304+
""StringWithStars"": ""This is not a /* comment */"",
305+
""UrlValue"": ""https://example.com/path"", // This is a real comment
306+
""EmptyComment"": ""value"", //
307+
/**/
308+
""AfterEmptyComment"": ""value2""
309+
/* Final multi-line comment */
310+
}",
311+
contentType: "application/json"),
312+
// Test invalid JSON with comments
313+
ConfigurationModelFactory.ConfigurationSetting(
314+
key: "InvalidJsonWithComments",
315+
value: @"// This is a comment
316+
{ invalid json structure
317+
// Another comment
318+
missing quotes and braces",
319+
contentType: "application/json"),
320+
// Test only comments (should be invalid JSON)
321+
ConfigurationModelFactory.ConfigurationSetting(
322+
key: "OnlyComments",
323+
value: @"
324+
// Just comments
325+
/* No actual content */
326+
",
327+
contentType: "application/json")
328+
};
329+
330+
var mockClientManager = GetMockConfigurationClientManager(_kvCollection);
331+
332+
var config = new ConfigurationBuilder()
333+
.AddAzureAppConfiguration(options => options.ClientManager = mockClientManager)
334+
.Build();
335+
336+
// Verify mixed comment styles are properly parsed
337+
Assert.Equal("https://api.example.com", config["MixedCommentStyles:ApiSettings:BaseUrl"]);
338+
Assert.Equal("secret-key", config["MixedCommentStyles:ApiSettings:ApiKey"]);
339+
Assert.Equal("/users", config["MixedCommentStyles:ApiSettings:Endpoints:0"]);
340+
Assert.Equal("/orders", config["MixedCommentStyles:ApiSettings:Endpoints:1"]);
341+
Assert.Equal("/products", config["MixedCommentStyles:ApiSettings:Endpoints:2"]);
342+
343+
// Verify edge cases where comment-like text appears in strings
344+
Assert.Equal("This is not a // comment", config["MixedCommentStyles:StringWithSlashes"]);
345+
Assert.Equal("This is not a /* comment */", config["MixedCommentStyles:StringWithStars"]);
346+
Assert.Equal("https://example.com/path", config["MixedCommentStyles:UrlValue"]);
347+
Assert.Equal("value", config["MixedCommentStyles:EmptyComment"]);
348+
Assert.Equal("value2", config["MixedCommentStyles:AfterEmptyComment"]);
349+
350+
// Invalid JSON should fall back to string value
351+
Assert.Equal(@"// This is a comment
352+
{ invalid json structure
353+
// Another comment
354+
missing quotes and braces", config["InvalidJsonWithComments"]);
355+
356+
// Only comments should be treated as string value (invalid JSON)
357+
Assert.Equal(@"
358+
// Just comments
359+
/* No actual content */
360+
", config["OnlyComments"]);
361+
}
362+
279363
private IConfigurationClientManager GetMockConfigurationClientManager(List<ConfigurationSetting> _kvCollection)
280364
{
281365
var mockResponse = new Mock<Response>();

0 commit comments

Comments
 (0)