Skip to content

ClientModel Prototype: Change approach to response.Content #41693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

ClientModel Prototype: Change approach to response.Content #41693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 6 additions & 37 deletions sdk/core/System.ClientModel/src/Message/HttpClientResponseHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,23 @@ namespace System.ClientModel.Primitives;

internal class HttpClientResponseHeaders : PipelineResponseHeaders
{
private readonly HttpResponseMessage _httpResponse;
private readonly HttpHeaders _httpHeaders;
private readonly HttpContent _httpResponseContent;

public HttpClientResponseHeaders(HttpResponseMessage response, HttpContent responseContent)
public HttpClientResponseHeaders(HttpHeaders headers, HttpContent responseContent)
{
_httpResponse = response;
_httpHeaders = headers;
_httpResponseContent = responseContent;
}

public override bool TryGetValue(string name, out string? value)
=> TryGetHeader(_httpResponse.Headers, _httpResponseContent, name, out value);
=> TryGetHeader(_httpHeaders, _httpResponseContent, name, out value);

public override bool TryGetValues(string name, out IEnumerable<string>? values)
=> TryGetHeader(_httpResponse.Headers, _httpResponseContent, name, out values);
=> TryGetHeader(_httpHeaders, _httpResponseContent, name, out values);

public override IEnumerator<KeyValuePair<string, string>> GetEnumerator()
=> GetHeadersStringValues(_httpResponse.Headers, _httpResponseContent).GetEnumerator();
=> GetHeadersStringValues(_httpHeaders, _httpResponseContent).GetEnumerator();

#region Performance-optimized/Platform-specific implementation
private static bool TryGetHeader(HttpHeaders headers, HttpContent? content, string name, [NotNullWhen(true)] out string? value)
Expand Down Expand Up @@ -146,35 +146,4 @@ private static string JoinHeaderValues(IEnumerable<string> values)
}
#endif
#endregion

private static IEnumerable<KeyValuePair<string, IEnumerable<string>>> GetHeadersListValues(HttpHeaders headers, HttpContent? content)
{
#if NET6_0_OR_GREATER
foreach (var (key, value) in headers.NonValidated)
{
yield return new KeyValuePair<string, IEnumerable<string>>(key, value);
}

if (content is not null)
{
foreach (var (key, value) in content.Headers.NonValidated)
{
yield return new KeyValuePair<string, IEnumerable<string>>(key, value);
}
}
#else
foreach (KeyValuePair<string, IEnumerable<string>> header in headers)
{
yield return new KeyValuePair<string, IEnumerable<string>>(header.Key, header.Value);
}

if (content != null)
{
foreach (KeyValuePair<string, IEnumerable<string>> header in content.Headers)
{
yield return new KeyValuePair<string, IEnumerable<string>>(header.Key, header.Value);
}
}
#endif
}
}
12 changes: 7 additions & 5 deletions sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.Response.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace System.ClientModel.Primitives;

public partial class HttpClientPipelineTransport
{
private class HttpPipelineResponse : PipelineResponse
private class HttpClientPipelineResponse : PipelineResponse
{
private readonly HttpResponseMessage _httpResponse;

Expand All @@ -23,7 +23,7 @@ private class HttpPipelineResponse : PipelineResponse

private bool _disposed;

public HttpPipelineResponse(HttpResponseMessage httpResponse)
public HttpClientPipelineResponse(HttpResponseMessage httpResponse)
{
_httpResponse = httpResponse ?? throw new ArgumentNullException(nameof(httpResponse));
_httpResponseContent = _httpResponse.Content;
Expand All @@ -35,14 +35,16 @@ public override string ReasonPhrase
=> _httpResponse.ReasonPhrase ?? string.Empty;

protected override PipelineResponseHeaders GetHeadersCore()
=> new HttpClientResponseHeaders(_httpResponse, _httpResponseContent);
=> new HttpClientResponseHeaders(_httpResponse.Headers, _httpResponseContent);

public override Stream? ContentStream
{
get => _contentStream;
set
{
// Make sure we don't dispose the content if the stream was replaced
// We null the HttpResponseMessage.Content property to ensure
// that when we dispose the message, we don't also dispose the
// content we need for reading header values later on.
_httpResponse.Content = null;

_contentStream = value;
Expand Down Expand Up @@ -99,4 +101,4 @@ protected virtual void Dispose(bool disposing)
}
#endregion
}
}
}
49 changes: 23 additions & 26 deletions sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private async ValueTask ProcessSyncOrAsync(PipelineMessage message, bool async)
OnSendingRequest(message, httpRequest);

HttpResponseMessage responseMessage;
Stream? contentStream = null;
//Stream? contentStream = null;
message.Response = null;

try
Expand All @@ -118,23 +118,23 @@ private async ValueTask ProcessSyncOrAsync(PipelineMessage message, bool async)
#pragma warning restore AZC0110 // DO NOT use await keyword in possibly synchronous scope.
}

if (responseMessage.Content != null)
{
#if NET6_0_OR_GREATER
if (async)
{
contentStream = await responseMessage.Content.ReadAsStreamAsync(message.CancellationToken).ConfigureAwait(false);
}
else
{
contentStream = responseMessage.Content.ReadAsStream(message.CancellationToken);
}
#else
#pragma warning disable AZC0110 // DO NOT use await keyword in possibly synchronous scope.
contentStream = await responseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
#pragma warning restore AZC0110 // DO NOT use await keyword in possibly synchronous scope.
#endif
}
// if (responseMessage.Content != null)
// {
//#if NET6_0_OR_GREATER
// if (async)
// {
// contentStream = await responseMessage.Content.ReadAsStreamAsync(message.CancellationToken).ConfigureAwait(false);
// }
// else
// {
// contentStream = responseMessage.Content.ReadAsStream(message.CancellationToken);
// }
//#else
//#pragma warning disable AZC0110 // DO NOT use await keyword in possibly synchronous scope.
// contentStream = await responseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
//#pragma warning restore AZC0110 // DO NOT use await keyword in possibly synchronous scope.
//#endif
// }
}
// HttpClient on NET5 throws OperationCanceledException from sync call sites, normalize to TaskCanceledException
catch (OperationCanceledException e) when (CancellationHelper.ShouldWrapInOperationCanceledException(e, message.CancellationToken))
Expand All @@ -146,20 +146,17 @@ private async ValueTask ProcessSyncOrAsync(PipelineMessage message, bool async)
throw new ClientResultException(e.Message, response: default, e);
}

message.Response = new HttpPipelineResponse(responseMessage);
message.Response = new HttpClientPipelineResponse(responseMessage);

// This extensibility point lets derived types do the following:
// 1. Set message.Response to an implementation-specific type, e.g. Azure.Core.Response.
// 2. Make any necessary modifications based on the System.Net.Http.HttpResponseMessage.
OnReceivedResponse(message, responseMessage);

// We set derived values on the MessageResponse here, including Content and IsError
// to ensure these things happen in the transport. If derived implementations need
// to override these default transport values, they can do so in pipeline policies.
if (contentStream is not null)
{
message.Response.ContentStream = contentStream;
}
//if (contentStream is not null)
//{
// message.Response.ContentStream = contentStream;
//}
}

/// <summary>
Expand Down