From 96c1aa16cc7c28abe450bf2145e94966d82a173a Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Tue, 27 Aug 2024 09:45:59 +0200 Subject: [PATCH] Improve FetchRequest & FetchResponse public API and make it more future-proof + fix bug in UnityWebRequestConfigFetcher --- .../Assets/Scripts/SingletonServices.cs | 19 +++---- .../ConfigService/DefaultConfigFetcher.cs | 11 ++-- .../ConfigService/FetchRequest.cs | 12 +++-- .../ConfigService/FetchResponse.cs | 54 ++++++++++++++----- .../ConfigService/HttpClientConfigFetcher.cs | 10 ++-- 5 files changed, 68 insertions(+), 38 deletions(-) diff --git a/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs b/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs index f55142c0..4ddb9113 100644 --- a/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs +++ b/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs @@ -153,7 +153,11 @@ public async Task FetchAsync(FetchRequest request, CancellationTo using var webRequest = UnityWebRequest.Get(uri); - webRequest.SetRequestHeader(request.SdkInfoHeader.Key, request.SdkInfoHeader.Value); + for (int i = 0, n = request.Headers.Count; i < n; i++) + { + var header = request.Headers[i]; + webRequest.SetRequestHeader(header.Key, header.Value); + } webRequest.timeout = (int)request.Timeout.TotalSeconds; @@ -177,20 +181,11 @@ public async Task FetchAsync(FetchRequest request, CancellationTo await tcs.Task; - if (webRequest.result == UnityWebRequest.Result.Success) + if (webRequest.result is UnityWebRequest.Result.Success or UnityWebRequest.Result.ProtocolError) { var statusCode = (HttpStatusCode)webRequest.responseCode; Debug.Log($"Fetching config finished with status code {statusCode}."); - if (statusCode == HttpStatusCode.OK) - { - var eTag = webRequest.GetResponseHeader("etag"); - var text = webRequest.downloadHandler.text; - return new FetchResponse(statusCode, reasonPhrase: null, eTag is { Length: > 0 } ? eTag : null, webRequest.downloadHandler.text); - } - else - { - return new FetchResponse(statusCode, reasonPhrase: null, null, null); - } + return new FetchResponse(statusCode, reasonPhrase: null, webRequest.GetResponseHeaders(), statusCode == HttpStatusCode.OK ? webRequest.downloadHandler.text : null); } else if (webRequest.result == UnityWebRequest.Result.ConnectionError && webRequest.error == "Request timeout") { diff --git a/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs b/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs index d3ba178c..00fab451 100644 --- a/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs +++ b/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs @@ -11,7 +11,7 @@ namespace ConfigCat.Client; internal sealed class DefaultConfigFetcher : IConfigFetcher, IDisposable { private readonly object syncObj = new(); - private readonly KeyValuePair sdkInfoHeader; + private readonly IReadOnlyList> requestHeaders; private readonly LoggerWrapper logger; private readonly IConfigCatConfigFetcher configFetcher; private readonly bool isCustomUri; @@ -25,9 +25,10 @@ public DefaultConfigFetcher(Uri requestUri, string productVersion, LoggerWrapper IConfigCatConfigFetcher configFetcher, bool isCustomUri, TimeSpan timeout) { this.requestUri = requestUri; - this.sdkInfoHeader = new KeyValuePair( - "X-ConfigCat-UserAgent", - new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString()); + this.requestHeaders = new[] + { + new KeyValuePair("X-ConfigCat-UserAgent", new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString()) + }; this.logger = logger; this.configFetcher = configFetcher; this.isCustomUri = isCustomUri; @@ -153,7 +154,7 @@ private async ValueTask FetchRequestAsync(string? httpETag { for (; ; maxExecutionCount--) { - var request = new FetchRequest(this.requestUri, httpETag, this.sdkInfoHeader, this.timeout); + var request = new FetchRequest(this.requestUri, httpETag, this.requestHeaders, this.timeout); var response = await this.configFetcher.FetchAsync(request, this.cancellationTokenSource.Token).ConfigureAwait(TaskShim.ContinueOnCapturedContext); diff --git a/src/ConfigCatClient/ConfigService/FetchRequest.cs b/src/ConfigCatClient/ConfigService/FetchRequest.cs index 2590cb99..3591445b 100644 --- a/src/ConfigCatClient/ConfigService/FetchRequest.cs +++ b/src/ConfigCatClient/ConfigService/FetchRequest.cs @@ -12,11 +12,11 @@ public readonly struct FetchRequest /// /// Initializes a new instance of the struct. /// - public FetchRequest(Uri uri, string? lastETag, KeyValuePair sdkInfoHeader, TimeSpan timeout) + public FetchRequest(Uri uri, string? lastETag, IReadOnlyList> headers, TimeSpan timeout) { Uri = uri ?? throw new ArgumentNullException(nameof(uri)); LastETag = lastETag; - SdkInfoHeader = sdkInfoHeader; + Headers = headers; Timeout = timeout; } @@ -27,13 +27,17 @@ public FetchRequest(Uri uri, string? lastETag, KeyValuePair sdkI /// /// The value of the ETag HTTP response header received during the last successful request (if any). + /// If available, should be included in the HTTP request, either in the If-None-Match header or in the ccetag query string parameter. /// + /// + /// In browser runtime environments the If-None-Match header should be avoided because that may cause unnecessary CORS preflight requests. + /// public string? LastETag { get; } /// - /// The name and value of the HTTP request header containing information about the SDK. Should be included in every request. + /// Additional HTTP request headers. Should be included in every HTTP request. /// - public KeyValuePair SdkInfoHeader { get; } + public IReadOnlyList> Headers { get; } /// /// The request timeout to apply, configured via . diff --git a/src/ConfigCatClient/ConfigService/FetchResponse.cs b/src/ConfigCatClient/ConfigService/FetchResponse.cs index 1ba381f3..1bc43aa2 100644 --- a/src/ConfigCatClient/ConfigService/FetchResponse.cs +++ b/src/ConfigCatClient/ConfigService/FetchResponse.cs @@ -1,7 +1,8 @@ +using System; +using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; -using System.Net.Http.Headers; namespace ConfigCat.Client; @@ -10,26 +11,51 @@ namespace ConfigCat.Client; /// public readonly struct FetchResponse { - internal static FetchResponse From(HttpResponseMessage httpResponse, string? httpResponseBody = null) - { - return new FetchResponse(httpResponse.StatusCode, httpResponse.ReasonPhrase, httpResponse.Headers, httpResponseBody); - } - - private readonly object? headersOrETag; // either null or a string or HttpResponseHeaders - - private FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, object? headersOrETag, string? body) + private FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? body) { StatusCode = statusCode; ReasonPhrase = reasonPhrase; - this.headersOrETag = headersOrETag; Body = body; } + internal FetchResponse(HttpResponseMessage httpResponse, string? httpResponseBody = null) + : this(httpResponse.StatusCode, httpResponse.ReasonPhrase, httpResponseBody) + { + ETag = httpResponse.Headers.ETag?.Tag; + RayId = httpResponse.Headers.TryGetValues("CF-RAY", out var values) ? values.FirstOrDefault() : null; + } + /// /// Initializes a new instance of the struct. /// - public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eTag, string? body) - : this(statusCode, reasonPhrase, (object?)eTag, body) { } + public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, IEnumerable> headers, string? body = null) + : this(statusCode, reasonPhrase, body) + { + string? eTag = null, rayId = null; + + foreach (var header in headers) + { + if (eTag is null && "ETag".Equals(header.Key, StringComparison.OrdinalIgnoreCase)) + { + eTag = header.Value; + if (rayId is not null) + { + break; + } + } + else if (rayId is null && "CF-RAY".Equals(header.Key, StringComparison.OrdinalIgnoreCase)) + { + rayId = header.Value; + if (eTag is not null) + { + break; + } + } + } + + ETag = eTag; + RayId = rayId; + } /// /// The HTTP status code. @@ -44,9 +70,9 @@ public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eT /// /// The value of the ETag HTTP response header. /// - public string? ETag => this.headersOrETag is HttpResponseHeaders headers ? headers.ETag?.Tag : (string?)this.headersOrETag; + public string? ETag { get; } - internal string? RayId => this.headersOrETag is HttpResponseHeaders headers && headers.TryGetValues("CF-RAY", out var values) ? values.FirstOrDefault() : null; + internal string? RayId { get; } /// /// The response body. diff --git a/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs b/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs index 517f0883..edca7183 100644 --- a/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs +++ b/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs @@ -71,7 +71,11 @@ public async Task FetchAsync(FetchRequest request, CancellationTo RequestUri = request.Uri, }; - httpRequest.Headers.Add(request.SdkInfoHeader.Key, request.SdkInfoHeader.Value); + for (int i = 0, n = request.Headers.Count; i < n; i++) + { + var header = request.Headers[i]; + httpRequest.Headers.Add(header.Key, header.Value); + } if (request.LastETag is not null) { @@ -90,11 +94,11 @@ public async Task FetchAsync(FetchRequest request, CancellationTo var httpResponseBody = await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(TaskShim.ContinueOnCapturedContext); #endif - return FetchResponse.From(httpResponse, httpResponseBody); + return new FetchResponse(httpResponse, httpResponseBody); } else { - var response = FetchResponse.From(httpResponse); + var response = new FetchResponse(httpResponse); if (!response.IsExpected) { this.httpClient = null;