Skip to content

Commit

Permalink
Merge branch 'feature/pipeline_when_downloading_file' into develop
Browse files Browse the repository at this point in the history
Resolves #343
  • Loading branch information
Jericho committed Jun 7, 2024
2 parents 6c23897 + 93d44f5 commit 9e14a38
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 38 deletions.
2 changes: 1 addition & 1 deletion Source/ZoomNet.UnitTests/MockFluentHttpResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public async Task<Stream> AsStream()
#endif
)
.ConfigureAwait(false);
stream.Position = 0;
if (stream.CanSeek) stream.Position = 0;
return stream;
}

Expand Down
49 changes: 17 additions & 32 deletions Source/ZoomNet/Resources/CloudRecordings.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
using Pathoschild.Http.Client;
using Pathoschild.Http.Client.Extensibility;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Text.Json.Nodes;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -361,41 +361,26 @@ public Task RejectRegistrantsAsync(long meetingId, IEnumerable<string> registran
/// <inheritdoc/>
public async Task<Stream> DownloadFileAsync(string downloadUrl, CancellationToken cancellationToken = default)
{
/*
* PLEASE NOTE:
*
* The HttpRequestMessage in this method is dispatched with its completion option set to "ResponseHeadersRead".
* This ensures the content of the response is streamed rather than buffered in memory.
* This is important in cases where the downloaded file is quite large.
* In this scenario, we don't want the entirety of the file to be buffered in a MemoryStream because
* it could lead to "out of memory" exceptions if the file is large enough.
* See https://github.com/Jericho/ZoomNet/pull/342 for a discussion on this topic.
*
* Forthermore, as of this writing, the FluentHttp library does not allow us to stream the content of responses
* which means that the code in this method cannot be simplified like so:
return _client
.GetAsync(downloadUrl)
.WithCancellationToken(cancellationToken)
.AsStream();
*
* The downside of not using the FluentHttp library to dispatch the request is that we lose automatic retries,
* error handling, logging, etc.
*/
// Prepare the request
var request = _client
.GetAsync(downloadUrl)
.WithOptions(completeWhen: HttpCompletionOption.ResponseHeadersRead)
.WithCancellationToken(cancellationToken);

using (var request = new HttpRequestMessage(HttpMethod.Get, downloadUrl))
{
var tokenHandler = _client.Filters.OfType<ITokenHandler>().SingleOrDefault();
if (tokenHandler != null)
{
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", tokenHandler.Token);
}
// Remove our custom error handler because it reads the content of the response to check for error messages returned from the Zoom API.
// This is problematic because we want the content of the response to be streamed.
request = request.WithoutFilter<ZoomErrorHandler>();

var response = await _client.BaseClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);
// We need to add the default error filter to throw an exception if the request fails.
// The error handler is safe to use with streaming responses because it does not read the content to determine if an error occured.
request = request.WithFilter(new DefaultErrorFilter());

response.EnsureSuccessStatusCode();
// Dispatch the request
var response = await request
.AsStream()
.ConfigureAwait(false);

return await response.Content.ReadAsStreamAsync();
}
return response;
}

private Task UpdateRegistrantsStatusAsync(long meetingId, IEnumerable<string> registrantIds, string status, CancellationToken cancellationToken = default)
Expand Down
2 changes: 1 addition & 1 deletion Source/ZoomNet/Utilities/DiagnosticHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void OnRequest(IRequest request)
request.WithHeader(DIAGNOSTIC_ID_HEADER_NAME, diagnosticId);

// Add the diagnostic info to our cache
DiagnosticsInfo.TryAdd(diagnosticId, new DiagnosticInfo(new WeakReference<HttpRequestMessage>(request.Message), Stopwatch.GetTimestamp(), null, long.MinValue));
DiagnosticsInfo.TryAdd(diagnosticId, new DiagnosticInfo(new WeakReference<HttpRequestMessage>(request.Message), Stopwatch.GetTimestamp(), null, long.MinValue, request.Options));
}

/// <summary>Method invoked just after the HTTP response is received. This method can modify the incoming HTTP response.</summary>
Expand Down
13 changes: 10 additions & 3 deletions Source/ZoomNet/Utilities/DiagnosticInfo.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Pathoschild.Http.Client;
using System;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -17,12 +18,15 @@ internal class DiagnosticInfo

public long ResponseTimestamp { get; set; }

public DiagnosticInfo(WeakReference<HttpRequestMessage> requestReference, long requestTimestamp, WeakReference<HttpResponseMessage> responseReference, long responseTimestamp)
public RequestOptions Options { get; set; }

public DiagnosticInfo(WeakReference<HttpRequestMessage> requestReference, long requestTimestamp, WeakReference<HttpResponseMessage> responseReference, long responseTimestamp, RequestOptions options)
{
RequestReference = requestReference;
RequestTimestamp = requestTimestamp;
ResponseReference = responseReference;
ResponseTimestamp = responseTimestamp;
Options = options;
}

public string GetLoggingTemplate()
Expand Down Expand Up @@ -84,12 +88,15 @@ public object[] GetLoggingParameters()

// Get the content to the request/response and calculate how long it took to get the response
var elapsed = TimeSpan.FromTicks(ResponseTimestamp - RequestTimestamp);
var isStreaming = Options.CompleteWhen == HttpCompletionOption.ResponseHeadersRead;
var requestContent = request?.Content?.ReadAsStringAsync(null).GetAwaiter().GetResult();
var responseContent = response?.Content?.ReadAsStringAsync(null).GetAwaiter().GetResult();
var responseContent = isStreaming
? "... content omitted from this log because the response is streaming ..."
: response?.Content?.ReadAsStringAsync(null).GetAwaiter().GetResult();

// Calculate the content size
var requestContentLength = requestContent?.Length ?? 0;
var responseContentLength = responseContent?.Length ?? 0;
var responseContentLength = isStreaming ? -1 : (responseContent?.Length ?? 0); // "-1" means the response is streaming therefore we can't calculate the length

// Get the request headers (please note: intentionally getting headers from "response.RequestMessage" rather than "request")
var requestHeaders = response?.RequestMessage?.Headers ?? Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
Expand Down
2 changes: 1 addition & 1 deletion Source/ZoomNet/ZoomNet.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
<PackageReference Include="jose-jwt" Version="5.0.0" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="8.0.0" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
<PackageReference Include="Pathoschild.Http.FluentClient" Version="4.3.0" />
<PackageReference Include="Pathoschild.Http.FluentClient" Version="4.4.1" />
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.507" PrivateAssets="All" />
<PackageReference Include="System.Text.Json" Version="8.0.3" />
<PackageReference Include="System.ValueTuple" Version="4.5.0" />
Expand Down

0 comments on commit 9e14a38

Please sign in to comment.