Skip to content

Commit

Permalink
Improve FetchRequest & FetchResponse public API and make it more futu…
Browse files Browse the repository at this point in the history
…re-proof + fix bug in UnityWebRequestConfigFetcher
  • Loading branch information
adams85 committed Aug 27, 2024
1 parent 9e93e99 commit 96c1aa1
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 38 deletions.
19 changes: 7 additions & 12 deletions samples/UnityWebGL/Assets/Scripts/SingletonServices.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,11 @@ public async Task<FetchResponse> 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;

Expand All @@ -177,20 +181,11 @@ public async Task<FetchResponse> 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")
{
Expand Down
11 changes: 6 additions & 5 deletions src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace ConfigCat.Client;
internal sealed class DefaultConfigFetcher : IConfigFetcher, IDisposable
{
private readonly object syncObj = new();
private readonly KeyValuePair<string, string> sdkInfoHeader;
private readonly IReadOnlyList<KeyValuePair<string, string>> requestHeaders;
private readonly LoggerWrapper logger;
private readonly IConfigCatConfigFetcher configFetcher;
private readonly bool isCustomUri;
Expand All @@ -25,9 +25,10 @@ public DefaultConfigFetcher(Uri requestUri, string productVersion, LoggerWrapper
IConfigCatConfigFetcher configFetcher, bool isCustomUri, TimeSpan timeout)
{
this.requestUri = requestUri;
this.sdkInfoHeader = new KeyValuePair<string, string>(
"X-ConfigCat-UserAgent",
new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString());
this.requestHeaders = new[]
{
new KeyValuePair<string, string>("X-ConfigCat-UserAgent", new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString())
};
this.logger = logger;
this.configFetcher = configFetcher;
this.isCustomUri = isCustomUri;
Expand Down Expand Up @@ -153,7 +154,7 @@ private async ValueTask<DeserializedResponse> 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);

Expand Down
12 changes: 8 additions & 4 deletions src/ConfigCatClient/ConfigService/FetchRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ public readonly struct FetchRequest
/// <summary>
/// Initializes a new instance of the <see cref="FetchRequest"/> struct.
/// </summary>
public FetchRequest(Uri uri, string? lastETag, KeyValuePair<string, string> sdkInfoHeader, TimeSpan timeout)
public FetchRequest(Uri uri, string? lastETag, IReadOnlyList<KeyValuePair<string, string>> headers, TimeSpan timeout)
{
Uri = uri ?? throw new ArgumentNullException(nameof(uri));
LastETag = lastETag;
SdkInfoHeader = sdkInfoHeader;
Headers = headers;
Timeout = timeout;
}

Expand All @@ -27,13 +27,17 @@ public FetchRequest(Uri uri, string? lastETag, KeyValuePair<string, string> sdkI

/// <summary>
/// The value of the <c>ETag</c> HTTP response header received during the last successful request (if any).
/// If available, should be included in the HTTP request, either in the <c>If-None-Match</c> header or in the <c>ccetag</c> query string parameter.
/// </summary>
/// <remarks>
/// In browser runtime environments the <c>If-None-Match</c> header should be avoided because that may cause unnecessary CORS preflight requests.
/// </remarks>
public string? LastETag { get; }

/// <summary>
/// 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.
/// </summary>
public KeyValuePair<string, string> SdkInfoHeader { get; }
public IReadOnlyList<KeyValuePair<string, string>> Headers { get; }

/// <summary>
/// The request timeout to apply, configured via <see cref="ConfigCatClientOptions.HttpTimeout"/>.
Expand Down
54 changes: 40 additions & 14 deletions src/ConfigCatClient/ConfigService/FetchResponse.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -10,26 +11,51 @@ namespace ConfigCat.Client;
/// </summary>
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;
}

/// <summary>
/// Initializes a new instance of the <see cref="FetchResponse"/> struct.
/// </summary>
public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eTag, string? body)
: this(statusCode, reasonPhrase, (object?)eTag, body) { }
public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, IEnumerable<KeyValuePair<string, string>> 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;
}

/// <summary>
/// The HTTP status code.
Expand All @@ -44,9 +70,9 @@ public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eT
/// <summary>
/// The value of the <c>ETag</c> HTTP response header.
/// </summary>
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; }

/// <summary>
/// The response body.
Expand Down
10 changes: 7 additions & 3 deletions src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ public async Task<FetchResponse> 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)
{
Expand All @@ -90,11 +94,11 @@ public async Task<FetchResponse> 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;
Expand Down

0 comments on commit 96c1aa1

Please sign in to comment.