From 5a6cb51c13b1769d5986600d97632f9029016aca Mon Sep 17 00:00:00 2001 From: skwasjer <11424653+skwasjer@users.noreply.github.com> Date: Tue, 17 Dec 2024 16:49:34 +0100 Subject: [PATCH] fix!: remove default value from CorrelateOptions.RequestHeaders to allow proper configuration binding (from env vars/json). --- Directory.Build.props | 2 +- .../AspNetCore/CorrelateFeature.cs | 5 +- .../AspNetCore/CorrelateOptions.cs | 22 +++----- .../Extensions/HeaderDictionaryExtensions.cs | 2 +- .../AspNetCore/CorrelateFeatureTests.cs | 10 ++-- .../AspNetCore/IntegrationTests.cs | 52 ++++++++++++++++++- 6 files changed, 68 insertions(+), 25 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index c176f36..8086977 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -6,7 +6,7 @@ enable enable false - $(NoWarn);IDE0079;S1135;CA1510;CA1511;CA1512;CA1513;CA1863 + $(NoWarn);IDE0079;S1135;CA1510;CA1511;CA1512;CA1513;CA1863;xUnit1042 $(NoWarn);NETSDK1138 $(WarningsAsErrors);NU1601;NU1603;NU1605;NU1608;NU1701;MSB3644 true diff --git a/src/Correlate.AspNetCore/AspNetCore/CorrelateFeature.cs b/src/Correlate.AspNetCore/AspNetCore/CorrelateFeature.cs index fa1eb76..e5a765e 100644 --- a/src/Correlate.AspNetCore/AspNetCore/CorrelateFeature.cs +++ b/src/Correlate.AspNetCore/AspNetCore/CorrelateFeature.cs @@ -1,4 +1,5 @@ -using Correlate.Http.Extensions; +using Correlate.Http; +using Correlate.Http.Extensions; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -88,7 +89,7 @@ public void StopCorrelating(HttpContext httpContext) private (string? headerName, string correlationId) GetOrCreateCorrelationHeaderAndId(HttpContext httpContext) { - (string requestHeaderName, string? requestCorrelationId) = httpContext.Request.Headers.GetCorrelationIdHeader(_options.RequestHeaders); + (string requestHeaderName, string? requestCorrelationId) = httpContext.Request.Headers.GetCorrelationIdHeader(_options.RequestHeaders ?? [CorrelationHttpHeaders.CorrelationId]); if (requestCorrelationId is not null) { LogRequestHeaderFound(_logger, requestHeaderName, requestCorrelationId, null); diff --git a/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs b/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs index 3c09a48..c1b9e94 100644 --- a/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs +++ b/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs @@ -1,27 +1,21 @@ -using Correlate.Http; - -namespace Correlate.AspNetCore; +namespace Correlate.AspNetCore; /// /// Options for handling correlation id on incoming requests. /// -public sealed class CorrelateOptions: CorrelationManagerOptions +public sealed class CorrelateOptions : CorrelationManagerOptions { - private static readonly string[] DefaultRequestHeaders = { CorrelationHttpHeaders.CorrelationId }; - - private string[]? _requestHeaders; - /// - /// Gets or sets the request headers to retrieve the correlation id from. Defaults to X-Correlation-ID. + /// Gets or sets the request headers to retrieve the correlation id from. + /// + /// To disable using the correlation ID from an incoming request, explicitly set this to an empty list. + /// When , internally defaults to ["X-Correlation-ID"]. + /// /// /// /// The first matching request header will be used. /// - public string[] RequestHeaders - { - get => _requestHeaders ?? DefaultRequestHeaders; - set => _requestHeaders = value; - } + public IReadOnlyList? RequestHeaders { get; set; } /// /// Gets or sets whether to include the correlation id in the response. diff --git a/src/Correlate.Core/Http/Extensions/HeaderDictionaryExtensions.cs b/src/Correlate.Core/Http/Extensions/HeaderDictionaryExtensions.cs index 418321f..7611905 100644 --- a/src/Correlate.Core/Http/Extensions/HeaderDictionaryExtensions.cs +++ b/src/Correlate.Core/Http/Extensions/HeaderDictionaryExtensions.cs @@ -4,7 +4,7 @@ namespace Correlate.Http.Extensions; internal static class HeaderDictionaryExtensions { - internal static KeyValuePair GetCorrelationIdHeader(this IDictionary httpHeaders, ICollection acceptedHeaders) + internal static KeyValuePair GetCorrelationIdHeader(this IDictionary httpHeaders, IReadOnlyCollection acceptedHeaders) { if (acceptedHeaders is null) { diff --git a/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs b/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs index 139a6d9..8579fc4 100644 --- a/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs +++ b/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs @@ -42,7 +42,7 @@ public CorrelateFeatureTests() _correlationIdFactory = Substitute.For(); _correlationIdFactory.Create().Returns(CorrelationId); - _options = new CorrelateOptions(); + _options = new CorrelateOptions { RequestHeaders = [CorrelationHttpHeaders.CorrelationId] }; _sut = new CorrelateFeature( _services.GetRequiredService(), _correlationIdFactory, @@ -67,11 +67,11 @@ public async Task Given_that_correlating_has_started_when_firing_to_send_headers _options.RequestHeaders.Should().NotBeNullOrEmpty(); if (requestHeader is not null) { - _options.RequestHeaders = new[] { requestHeader }; + _options.RequestHeaders = [requestHeader]; } var expectedHeader = new KeyValuePair( - _options.RequestHeaders[0], + _options.RequestHeaders![0], new StringValues(CorrelationId) ); @@ -91,7 +91,7 @@ public async Task Given_that_correlating_has_started_when_firing_to_send_headers [InlineData(CorrelationHttpHeaders.RequestId)] public async Task Given_that_request_contains_correlationId_header_in_allowed_list_when_correlating_has_started_it_should_have_used_that_correlationId(string headerName) { - _options.RequestHeaders = new[] { CorrelationHttpHeaders.CorrelationId, CorrelationHttpHeaders.RequestId }; + _options.RequestHeaders = [CorrelationHttpHeaders.CorrelationId, CorrelationHttpHeaders.RequestId]; string correlationId = Guid.NewGuid().ToString("D"); _httpContext.Features.Get()! @@ -203,7 +203,7 @@ public async Task Given_that_response_already_contains_correlation_header_when_f _options.RequestHeaders.Should().NotBeNullOrEmpty(); const string existingCorrelationId = "existing-id"; - _responseFeature.Headers.Append(_options.RequestHeaders[0], existingCorrelationId); + _responseFeature.Headers.Append(_options.RequestHeaders![0], existingCorrelationId); var expectedHeader = new KeyValuePair( _options.RequestHeaders[0], diff --git a/test/Correlate.AspNetCore.Tests/AspNetCore/IntegrationTests.cs b/test/Correlate.AspNetCore.Tests/AspNetCore/IntegrationTests.cs index 77d7aca..70b7459 100644 --- a/test/Correlate.AspNetCore.Tests/AspNetCore/IntegrationTests.cs +++ b/test/Correlate.AspNetCore.Tests/AspNetCore/IntegrationTests.cs @@ -53,7 +53,7 @@ public void Dispose() public async Task Given_custom_header_is_defined_when_executing_request_the_response_should_contain_custom_header() { const string headerName = "my-header"; - _options.RequestHeaders = new[] { headerName }; + _options.RequestHeaders = [headerName]; // Act HttpClient client = _factory.CreateClient(); @@ -90,7 +90,7 @@ public async Task Given_default_configuration_when_executing_request_the_respons [Fact] public async Task Given_no_headers_are_defined_when_executing_request_the_response_should_contain_default_header() { - _options.RequestHeaders = Array.Empty(); + _options.RequestHeaders = []; // Act HttpClient client = _factory.CreateClient(); @@ -241,4 +241,52 @@ public async Task When_logging_should_have_correlationId_for_all_logged_events() .ContainEquivalentOf(new LoggerExtensions.CorrelatedLogScope(CorrelateConstants.CorrelationIdKey, correlationId)) ); } + + [Theory] + [MemberData(nameof(GetOptionBindingTestCases))] + public void Options_should_deserialize_from_config(Dictionary configDict, CorrelateOptions expectedOptions) + { + IConfigurationRoot config = new ConfigurationBuilder() + .AddInMemoryCollection(configDict) + .Build(); + + // Act + using ServiceProvider services = new ServiceCollection() + .Configure(config.Bind) + .BuildServiceProvider(); + + // Assert + CorrelateOptions opts = services.GetRequiredService>().Value; + opts.Should().BeEquivalentTo(expectedOptions); + } + + public static IEnumerable GetOptionBindingTestCases() + { + yield return + [ + new Dictionary(), + new CorrelateOptions + { + IncludeInResponse = true, + LoggingScopeKey = CorrelateConstants.CorrelationIdKey, + RequestHeaders = null + } + ]; + yield return + [ + new Dictionary + { + { "LoggingScopeKey", "LogKey1" }, + { "IncludeInResponse", "false" }, + { "RequestHeaders:0", "Header1" }, + { "RequestHeaders:1", "Header2" } + }, + new CorrelateOptions + { + LoggingScopeKey = "LogKey1", + IncludeInResponse = false, + RequestHeaders = ["Header1", "Header2"] + } + ]; + } }