Response handling improvements (#1690)

alexeyzimarev · web-flow · commit b2031e0c9fc2 · 2022-01-10T12:04:24.000+01:00
* * Handle response content with custom encoding detection
* Add ConfigureAwait(false) to async calls

* One more async

* Configure more awaits

* Event more configure await

* Simplify the Get with object parameters
diff --git a/src/RestSharp/Authenticators/AuthenticatorBase.cs b/src/RestSharp/Authenticators/AuthenticatorBase.cs
@@ -22,5 +22,5 @@ public abstract class AuthenticatorBase : IAuthenticator {
     protected abstract ValueTask<Parameter> GetAuthenticationParameter(string accessToken);
 
     public async ValueTask Authenticate(RestClient client, RestRequest request)
-        => request.AddOrUpdateParameter(await GetAuthenticationParameter(Token));
+        => request.AddOrUpdateParameter(await GetAuthenticationParameter(Token).ConfigureAwait(false));
 }
diff --git a/src/RestSharp/Extensions/MiscExtensions.cs b/src/RestSharp/Extensions/MiscExtensions.cs
@@ -31,9 +31,9 @@ public static async Task<byte[]> ReadAsBytes(this Stream input, CancellationToke
 
         int read;
 #if NETSTANDARD
-        while ((read = await input.ReadAsync(buffer, 0, buffer.Length, cancellationToken)) > 0)
+        while ((read = await input.ReadAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false)) > 0)
 #else
-        while ((read = await input.ReadAsync(buffer, cancellationToken)) > 0)
+        while ((read = await input.ReadAsync(buffer, cancellationToken).ConfigureAwait(false)) > 0)
 #endif
             ms.Write(buffer, 0, read);
 
diff --git a/src/RestSharp/Response/ResponseHandling.cs b/src/RestSharp/Response/ResponseHandling.cs
@@ -0,0 +1,43 @@
+//  Copyright © 2009-2020 John Sheehan, Andrew Young, Alexey Zimarev and RestSharp community
+// 
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// 
+// http://www.apache.org/licenses/LICENSE-2.0
+// 
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// 
+
+using System.Text;
+
+namespace RestSharp;
+
+static class ResponseHandling {
+    public static string GetResponseString(this HttpResponseMessage response, byte[] bytes) {
+        var encodingString = response.Content.Headers.ContentEncoding.FirstOrDefault();
+        var encoding       = encodingString != null ? TryGetEncoding(encodingString) : Encoding.Default;
+        return encoding.GetString(bytes);
+
+        Encoding TryGetEncoding(string es) {
+            try {
+                return Encoding.GetEncoding(es);
+            }
+            catch {
+                return Encoding.Default;
+            }
+        }
+    }
+
+    public static Task<Stream?> ReadResponse(this HttpResponseMessage response, CancellationToken cancellationToken) {
+#if NETSTANDARD
+        return response.Content.ReadAsStreamAsync();
+# else
+        return response.Content.ReadAsStreamAsync(cancellationToken);
+#endif
+    }
+}
diff --git a/src/RestSharp/Response/RestResponse.cs b/src/RestSharp/Response/RestResponse.cs
@@ -14,8 +14,6 @@
 
 using System.Diagnostics;
 using System.Net;
-using System.Net.Http.Headers;
-using System.Text;
 using RestSharp.Extensions;
 
 // ReSharper disable SuggestBaseTypeForParameter
@@ -51,8 +49,7 @@ public static RestResponse<T> FromResponse(RestResponse response)
             Server            = response.Server,
             StatusCode        = response.StatusCode,
             StatusDescription = response.StatusDescription,
-            Request           = response.Request,
-            ResponseMessage   = response.ResponseMessage
+            Request           = response.Request
         };
 }
 
@@ -67,40 +64,13 @@ internal static async Task<RestResponse> FromHttpResponse(
         CookieCollection    cookieCollection,
         CancellationToken   cancellationToken
     ) {
-        return request.AdvancedResponseWriter?.Invoke(httpResponse) ?? await GetDefaultResponse();
+        return request.AdvancedResponseWriter?.Invoke(httpResponse) ?? await GetDefaultResponse().ConfigureAwait(false);
 
         async Task<RestResponse> GetDefaultResponse() {
-            byte[]? bytes;
-            string? content;
-
-            if (request.ResponseWriter != null) {
-#if NETSTANDARD
-                var stream = await httpResponse.Content.ReadAsStreamAsync();
-# else
-                var stream = await httpResponse.Content.ReadAsStreamAsync(cancellationToken);
-#endif
-                var converted = request.ResponseWriter(stream);
-
-                if (converted == null) {
-                    bytes   = null;
-                    content = null;
-                }
-                else {
-                    bytes = await converted.ReadAsBytes(cancellationToken);
-                    var encodingString = httpResponse.Content.Headers.ContentEncoding.FirstOrDefault();
-                    var encoding       = encodingString != null ? Encoding.GetEncoding(encodingString) : Encoding.UTF8;
-                    content = encoding.GetString(bytes);
-                }
-            }
-            else {
-#if NETSTANDARD
-                bytes   = await httpResponse.Content.ReadAsByteArrayAsync();
-                content = await httpResponse.Content.ReadAsStringAsync();
-# else
-                bytes = await httpResponse.Content.ReadAsByteArrayAsync(cancellationToken);
-                content = await httpResponse.Content.ReadAsStringAsync(cancellationToken);
-#endif
-            }
+            var       readTask = request.ResponseWriter == null ? ReadResponse() : ReadAndConvertResponse();
+            using var stream   = await readTask.ConfigureAwait(false);
+            var       bytes    = stream == null ? null : await stream.ReadAsBytes(cancellationToken).ConfigureAwait(false);
+            var       content  = bytes == null ? null : httpResponse.GetResponseString(bytes);
 
             return new RestResponse {
                 Content           = content,
@@ -116,11 +86,17 @@ async Task<RestResponse> GetDefaultResponse() {
                 StatusDescription = httpResponse.ReasonPhrase,
                 IsSuccessful      = httpResponse.IsSuccessStatusCode,
                 Request           = request,
-                ResponseMessage   = httpResponse,
                 Headers           = httpResponse.Headers.GetHeaderParameters(),
                 ContentHeaders    = httpResponse.Content.Headers.GetHeaderParameters(),
                 Cookies           = cookieCollection
             };
+
+            Task<Stream?> ReadResponse() => httpResponse.ReadResponse(cancellationToken);
+
+            async Task<Stream?> ReadAndConvertResponse() {
+                using var original = await ReadResponse().ConfigureAwait(false);
+                return request.ResponseWriter!(original!);
+            }
         }
     }
 }
diff --git a/src/RestSharp/Response/RestResponseBase.cs b/src/RestSharp/Response/RestResponseBase.cs
@@ -34,8 +34,6 @@ public abstract class RestResponseBase {
     /// Mainly for debugging if ResponseStatus is not OK
     /// </remarks>
     public RestRequest? Request { get; set; }
-    
-    public HttpResponseMessage? ResponseMessage { get; init; }
 
     /// <summary>
     /// MIME content type of response
diff --git a/src/RestSharp/RestClient.Async.cs b/src/RestSharp/RestClient.Async.cs
@@ -23,17 +23,18 @@ public partial class RestClient {
     /// <param name="request">Request to be executed</param>
     /// <param name="cancellationToken">Cancellation token</param>
     public async Task<RestResponse> ExecuteAsync(RestRequest request, CancellationToken cancellationToken = default) {
-        var internalResponse = await ExecuteInternal(request, cancellationToken);
+        var internalResponse = await ExecuteInternal(request, cancellationToken).ConfigureAwait(false);
 
         var response = new RestResponse();
 
         response = internalResponse.Exception == null
             ? await RestResponse.FromHttpResponse(
-                internalResponse.ResponseMessage!,
-                request,
-                CookieContainer.GetCookies(internalResponse.Url),
-                cancellationToken
-            )
+                    internalResponse.ResponseMessage!,
+                    request,
+                    CookieContainer.GetCookies(internalResponse.Url),
+                    cancellationToken
+                )
+                .ConfigureAwait(false)
             : ReturnErrorOrThrow(response, internalResponse.Exception, internalResponse.TimeoutToken);
 
         response.Request = request;
@@ -47,7 +48,7 @@ async Task<InternalResponse> ExecuteInternal(RestRequest request, CancellationTo
         using var requestContent = new RequestContent(this, request);
 
         if (Authenticator != null)
-            await Authenticator.Authenticate(this, request);
+            await Authenticator.Authenticate(this, request).ConfigureAwait(false);
 
         var httpMethod = AsHttpMethod(request.Method);
         var url        = BuildUri(request);
@@ -67,12 +68,12 @@ async Task<InternalResponse> ExecuteInternal(RestRequest request, CancellationTo
             message.AddHeaders(headers);
 
             if (request.OnBeforeRequest != null)
-                await request.OnBeforeRequest(message);
+                await request.OnBeforeRequest(message).ConfigureAwait(false);
 
-            var responseMessage = await HttpClient.SendAsync(message, request.CompletionOption, ct);
+            var responseMessage = await HttpClient.SendAsync(message, request.CompletionOption, ct).ConfigureAwait(false);
 
             if (request.OnAfterRequest != null)
-                await request.OnAfterRequest(responseMessage);
+                await request.OnAfterRequest(responseMessage).ConfigureAwait(false);
 
             return new InternalResponse(responseMessage, url, null, timeoutCts.Token);
         }
@@ -89,8 +90,9 @@ record InternalResponse(HttpResponseMessage? ResponseMessage, Uri Url, Exception
     /// <param name="request">Pre-configured request instance.</param>
     /// <param name="cancellationToken"></param>
     /// <returns>The downloaded stream.</returns>
+    [PublicAPI]
     public async Task<Stream?> DownloadStreamAsync(RestRequest request, CancellationToken cancellationToken = default) {
-        var response = await ExecuteInternal(request, cancellationToken);
+        var response = await ExecuteInternal(request, cancellationToken).ConfigureAwait(false);
 
         if (response.Exception != null) {
             return Options.ThrowOnAnyError ? throw response.Exception : null;
@@ -99,18 +101,11 @@ record InternalResponse(HttpResponseMessage? ResponseMessage, Uri Url, Exception
         if (response.ResponseMessage == null) return null;
 
         if (request.ResponseWriter != null) {
-#if NETSTANDARD
-            var stream = await response.ResponseMessage.Content.ReadAsStreamAsync();
-# else
-            var stream = await response.ResponseMessage.Content.ReadAsStreamAsync(cancellationToken);
-#endif
-            return request.ResponseWriter(stream);
+            using var stream = await response.ResponseMessage.ReadResponse(cancellationToken).ConfigureAwait(false);
+            return request.ResponseWriter(stream!);
         }
-#if NETSTANDARD
-        return await response.ResponseMessage.Content.ReadAsStreamAsync();
-# else
-        return await response.ResponseMessage.Content.ReadAsStreamAsync(cancellationToken);
-#endif
+
+        return await response.ResponseMessage.ReadResponse(cancellationToken).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -119,35 +114,10 @@ record InternalResponse(HttpResponseMessage? ResponseMessage, Uri Url, Exception
     /// <param name="request">Pre-configured request instance.</param>
     /// <param name="cancellationToken"></param>
     /// <returns>The downloaded file.</returns>
+    [PublicAPI]
     public async Task<byte[]?> DownloadDataAsync(RestRequest request, CancellationToken cancellationToken = default) {
-        var response = await ExecuteInternal(request, cancellationToken);
-
-        if (response.Exception != null) {
-            return Options.ThrowOnAnyError ? throw response.Exception : null;
-        }
-
-        if (response.ResponseMessage == null) return null;
-
-        byte[]? bytes;
-
-        if (request.ResponseWriter != null) {
-#if NETSTANDARD
-            var stream = await response.ResponseMessage.Content.ReadAsStreamAsync();
-# else
-            var stream = await response.ResponseMessage.Content.ReadAsStreamAsync(cancellationToken);
-#endif
-            var converted = request.ResponseWriter(stream);
-            bytes = converted == null ? null : await converted.ReadAsBytes(cancellationToken);
-        }
-        else {
-#if NETSTANDARD
-            bytes = await response.ResponseMessage.Content.ReadAsByteArrayAsync();
-# else
-            bytes = await response.ResponseMessage.Content.ReadAsByteArrayAsync(cancellationToken);
-#endif
-        }
-
-        return bytes;
+        using var stream = await DownloadStreamAsync(request, cancellationToken).ConfigureAwait(false);
+        return stream == null ? null : await stream.ReadAsBytes(cancellationToken).ConfigureAwait(false);
     }
 
     RestResponse ReturnErrorOrThrow(RestResponse response, Exception exception, CancellationToken timeoutToken) {
diff --git a/src/RestSharp/RestClientExtensions.Json.cs b/src/RestSharp/RestClientExtensions.Json.cs
@@ -39,22 +39,10 @@ public static partial class RestClientExtensions {
         CancellationToken cancellationToken = default
     ) {
         var props = parameters.GetProperties();
-        var query = new List<Parameter>();
-
-        foreach (var (name, value) in props) {
-            var param = $"{name}";
-
-            if (resource.Contains(param)) {
-                resource = resource.Replace(param, value);
-            }
-            else {
-                query.Add(new QueryParameter(name, value));
-            }
-        }
-
         var request = new RestRequest(resource);
 
-        foreach (var parameter in query) {
+        foreach (var (name, value) in props) {
+            Parameter parameter = resource.Contains($"{name}") ? new UrlSegmentParameter(name, value!) : new QueryParameter(name, value);
             request.AddParameter(parameter);
         }
 
@@ -99,7 +87,7 @@ public static async Task<HttpStatusCode> PostJsonAsync<TRequest>(
         CancellationToken cancellationToken = default
     ) where TRequest : class {
         var restRequest = new RestRequest().AddJsonBody(request);
-        var response    = await client.PostAsync(restRequest, cancellationToken);
+        var response    = await client.PostAsync(restRequest, cancellationToken).ConfigureAwait(false);
         return response.StatusCode;
     }
 
@@ -141,7 +129,7 @@ public static async Task<HttpStatusCode> PutJsonAsync<TRequest>(
         CancellationToken cancellationToken = default
     ) where TRequest : class {
         var restRequest = new RestRequest().AddJsonBody(request);
-        var response    = await client.PutAsync(restRequest, cancellationToken);
+        var response    = await client.PutAsync(restRequest, cancellationToken).ConfigureAwait(false);
         return response.StatusCode;
     }
 }
diff --git a/src/RestSharp/RestClientExtensions.cs b/src/RestSharp/RestClientExtensions.cs
diff --git a/test/RestSharp.Tests.Shared/Extensions/StreamExtensions.cs b/test/RestSharp.Tests.Shared/Extensions/StreamExtensions.cs

Original file line number	Diff line number	Diff line change
`@@ -22,5 +22,5 @@ public abstract class AuthenticatorBase : IAuthenticator {`
`22`	`22`	`protected abstract ValueTask<Parameter> GetAuthenticationParameter(string accessToken);`
`23`	`23`
`24`	`24`	`public async ValueTask Authenticate(RestClient client, RestRequest request)`
`25`		`- => request.AddOrUpdateParameter(await GetAuthenticationParameter(Token));`
	`25`	`+ => request.AddOrUpdateParameter(await GetAuthenticationParameter(Token).ConfigureAwait(false));`
`26`	`26`	`}`