From b9a8ff8f80d61042a5820dc9ef858a2bc3c995ea Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Tue, 1 Sep 2020 22:49:55 -0700 Subject: [PATCH 1/8] Perf: significant resource improvements around garbage collection, memory and connection utilization --- .../BotFrameworkAdapter.cs | 8 ++-- .../Streaming/BotFrameworkHttpAdapterBase.cs | 12 +++-- .../Streaming/StreamingRequestHandler.cs | 2 +- .../ReceiveRequestExtensions.cs | 47 ++++++++++++++----- 4 files changed, 49 insertions(+), 20 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs index 3c4aa07c42..96a592144e 100644 --- a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs +++ b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs @@ -750,9 +750,11 @@ public async Task GetConversationsAsync(string serviceUrl, throw new ArgumentNullException(nameof(credentials)); } - var connectorClient = CreateConnectorClient(serviceUrl, credentials); - var results = await connectorClient.Conversations.GetConversationsAsync(continuationToken, cancellationToken).ConfigureAwait(false); - return results; + using (var connectorClient = CreateConnectorClient(serviceUrl, credentials)) + { + var results = await connectorClient.Conversations.GetConversationsAsync(continuationToken, cancellationToken).ConfigureAwait(false); + return results; + } } /// diff --git a/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs b/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs index fb5f335eb5..4a3b57c38c 100644 --- a/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs +++ b/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs @@ -129,10 +129,16 @@ public async Task ProcessStreamingActivityAsync(Activity activit context.TurnState.Add(BotIdentityKey, ClaimsIdentity); } - var connectorClient = CreateStreamingConnectorClient(activity, requestHandler); - context.TurnState.Add(connectorClient); + using (var connectorClient = CreateStreamingConnectorClient(activity, requestHandler)) + { + // Add connector client to be used throughout the turn + context.TurnState.Add(connectorClient); + + await RunPipelineAsync(context, callbackHandler, cancellationToken).ConfigureAwait(false); - await RunPipelineAsync(context, callbackHandler, cancellationToken).ConfigureAwait(false); + // Cleanup connector client + context.TurnState.Set(null); + } if (activity.Type == ActivityTypes.Invoke) { diff --git a/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs b/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs index 74e7cdf5af..ab6dc2927c 100644 --- a/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs +++ b/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs @@ -170,7 +170,7 @@ public override async Task ProcessRequestAsync(ReceiveRequest string body; try { - body = request.ReadBodyAsString(); + body = await request.ReadBodyAsStringAsync().ConfigureAwait(false); } #pragma warning disable CA1031 // Do not catch general exception types (we log the exception and continue execution) catch (Exception ex) diff --git a/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs b/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs index 86e5387fd0..fc290f715d 100644 --- a/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs +++ b/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using System.Text; +using System.Threading.Tasks; using Newtonsoft.Json; namespace Microsoft.Bot.Streaming @@ -23,6 +24,19 @@ public static class ReceiveRequestExtensions /// Otherwise a default instance of type T. /// public static T ReadBodyAsJson(this ReceiveRequest request) + { + return request.ReadBodyAsJsonAsync().GetAwaiter().GetResult(); + } + + /// + /// Serializes the body of this as JSON. + /// + /// The type to attempt to deserialize the contents of this 's body into. + /// The current instance of . + /// On success, an object of type T populated with data serialized from the body. + /// Otherwise a default instance of type T. + /// + public static async Task ReadBodyAsJsonAsync(this ReceiveRequest request) { // The first stream attached to a ReceiveRequest is always the ReceiveRequest body. // Any additional streams must be defined within the body or they will not be @@ -30,22 +44,17 @@ public static T ReadBodyAsJson(this ReceiveRequest request) var contentStream = request.Streams.FirstOrDefault(); /* If the response had no body we have to return a compatible - * but empty object to avoid throwing exceptions upstream anytime - * an empty response is received. - */ + * but empty object to avoid throwing exceptions upstream anytime + * an empty response is received. + */ if (contentStream == null) { return default; } - using (var reader = new StreamReader(contentStream.Stream, Encoding.UTF8)) - { - using (var jsonReader = new JsonTextReader(reader)) - { - var serializer = JsonSerializer.Create(SerializationSettings.DefaultDeserializationSettings); - return serializer.Deserialize(jsonReader); - } - } + var bodyString = await request.ReadBodyAsStringAsync().ConfigureAwait(false); + + return JsonConvert.DeserializeObject(bodyString, SerializationSettings.DefaultDeserializationSettings); } /// @@ -56,17 +65,29 @@ public static T ReadBodyAsJson(this ReceiveRequest request) /// Otherwise null. /// public static string ReadBodyAsString(this ReceiveRequest request) + { + return request.ReadBodyAsStringAsync().GetAwaiter().GetResult(); + } + + /// + /// Reads the body of this as a string. + /// + /// The current instance of . + /// On success, a string populated with data read from the body. + /// Otherwise null. + /// + public static Task ReadBodyAsStringAsync(this ReceiveRequest request) { var contentStream = request.Streams.FirstOrDefault(); if (contentStream == null) { - return string.Empty; + return Task.FromResult(string.Empty); } using (var reader = new StreamReader(contentStream.Stream, Encoding.UTF8)) { - return reader.ReadToEnd(); + return reader.ReadToEndAsync(); } } } From 0021b9f769aa0b3363905749358d504a8737522f Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Tue, 1 Sep 2020 22:49:55 -0700 Subject: [PATCH 2/8] Perf: significant resource improvements around garbage collection, memory and connection utilization --- .../BotFrameworkAdapter.cs | 8 ++-- .../Streaming/BotFrameworkHttpAdapterBase.cs | 12 +++-- .../Streaming/StreamingRequestHandler.cs | 2 +- .../ReceiveRequestExtensions.cs | 47 ++++++++++++++----- 4 files changed, 49 insertions(+), 20 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs index 3c4aa07c42..96a592144e 100644 --- a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs +++ b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs @@ -750,9 +750,11 @@ public async Task GetConversationsAsync(string serviceUrl, throw new ArgumentNullException(nameof(credentials)); } - var connectorClient = CreateConnectorClient(serviceUrl, credentials); - var results = await connectorClient.Conversations.GetConversationsAsync(continuationToken, cancellationToken).ConfigureAwait(false); - return results; + using (var connectorClient = CreateConnectorClient(serviceUrl, credentials)) + { + var results = await connectorClient.Conversations.GetConversationsAsync(continuationToken, cancellationToken).ConfigureAwait(false); + return results; + } } /// diff --git a/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs b/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs index fb5f335eb5..4a3b57c38c 100644 --- a/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs +++ b/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs @@ -129,10 +129,16 @@ public async Task ProcessStreamingActivityAsync(Activity activit context.TurnState.Add(BotIdentityKey, ClaimsIdentity); } - var connectorClient = CreateStreamingConnectorClient(activity, requestHandler); - context.TurnState.Add(connectorClient); + using (var connectorClient = CreateStreamingConnectorClient(activity, requestHandler)) + { + // Add connector client to be used throughout the turn + context.TurnState.Add(connectorClient); + + await RunPipelineAsync(context, callbackHandler, cancellationToken).ConfigureAwait(false); - await RunPipelineAsync(context, callbackHandler, cancellationToken).ConfigureAwait(false); + // Cleanup connector client + context.TurnState.Set(null); + } if (activity.Type == ActivityTypes.Invoke) { diff --git a/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs b/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs index 74e7cdf5af..ab6dc2927c 100644 --- a/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs +++ b/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs @@ -170,7 +170,7 @@ public override async Task ProcessRequestAsync(ReceiveRequest string body; try { - body = request.ReadBodyAsString(); + body = await request.ReadBodyAsStringAsync().ConfigureAwait(false); } #pragma warning disable CA1031 // Do not catch general exception types (we log the exception and continue execution) catch (Exception ex) diff --git a/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs b/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs index 86e5387fd0..fc290f715d 100644 --- a/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs +++ b/libraries/Microsoft.Bot.Streaming/ReceiveRequestExtensions.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using System.Text; +using System.Threading.Tasks; using Newtonsoft.Json; namespace Microsoft.Bot.Streaming @@ -23,6 +24,19 @@ public static class ReceiveRequestExtensions /// Otherwise a default instance of type T. /// public static T ReadBodyAsJson(this ReceiveRequest request) + { + return request.ReadBodyAsJsonAsync().GetAwaiter().GetResult(); + } + + /// + /// Serializes the body of this as JSON. + /// + /// The type to attempt to deserialize the contents of this 's body into. + /// The current instance of . + /// On success, an object of type T populated with data serialized from the body. + /// Otherwise a default instance of type T. + /// + public static async Task ReadBodyAsJsonAsync(this ReceiveRequest request) { // The first stream attached to a ReceiveRequest is always the ReceiveRequest body. // Any additional streams must be defined within the body or they will not be @@ -30,22 +44,17 @@ public static T ReadBodyAsJson(this ReceiveRequest request) var contentStream = request.Streams.FirstOrDefault(); /* If the response had no body we have to return a compatible - * but empty object to avoid throwing exceptions upstream anytime - * an empty response is received. - */ + * but empty object to avoid throwing exceptions upstream anytime + * an empty response is received. + */ if (contentStream == null) { return default; } - using (var reader = new StreamReader(contentStream.Stream, Encoding.UTF8)) - { - using (var jsonReader = new JsonTextReader(reader)) - { - var serializer = JsonSerializer.Create(SerializationSettings.DefaultDeserializationSettings); - return serializer.Deserialize(jsonReader); - } - } + var bodyString = await request.ReadBodyAsStringAsync().ConfigureAwait(false); + + return JsonConvert.DeserializeObject(bodyString, SerializationSettings.DefaultDeserializationSettings); } /// @@ -56,17 +65,29 @@ public static T ReadBodyAsJson(this ReceiveRequest request) /// Otherwise null. /// public static string ReadBodyAsString(this ReceiveRequest request) + { + return request.ReadBodyAsStringAsync().GetAwaiter().GetResult(); + } + + /// + /// Reads the body of this as a string. + /// + /// The current instance of . + /// On success, a string populated with data read from the body. + /// Otherwise null. + /// + public static Task ReadBodyAsStringAsync(this ReceiveRequest request) { var contentStream = request.Streams.FirstOrDefault(); if (contentStream == null) { - return string.Empty; + return Task.FromResult(string.Empty); } using (var reader = new StreamReader(contentStream.Stream, Encoding.UTF8)) { - return reader.ReadToEnd(); + return reader.ReadToEndAsync(); } } } From 9d84c4854a437cc9645ba3a7be7aaf5a84cf66e8 Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Mon, 28 Sep 2020 13:01:19 -0700 Subject: [PATCH 3/8] Cloud adapter: remove ported memory leaks and GC perf issues --- .../Microsoft.Bot.Builder/CloudAdapterBase.cs | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs b/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs index 230e30e7c1..9a6f79e52a 100644 --- a/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs +++ b/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs @@ -243,13 +243,17 @@ protected async Task ProcessProactiveAsync(ClaimsIdentity claimsIdentity, Conver var credentials = await _botFrameworkAuthentication.GetProactiveCredentialsAsync(claimsIdentity, audience, cancellationToken).ConfigureAwait(false); // Create the connector client to use for outbound requests. - var connectorClient = new ConnectorClient(new Uri(reference.ServiceUrl), credentials, _httpClient); - - // Create a turn context and run the pipeline. - using (var context = CreateTurnContext(reference.GetContinuationActivity(), claimsIdentity, audience, connectorClient, callback)) + using (var connectorClient = new ConnectorClient(new Uri(reference.ServiceUrl), credentials, _httpClient)) { - // Run the pipeline. - await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false); + // Create a turn context and run the pipeline. + using (var context = CreateTurnContext(reference.GetContinuationActivity(), claimsIdentity, audience, connectorClient, callback)) + { + // Run the pipeline. + await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false); + + // Cleanup disposable resources in case other code kept a reference to it. + context.TurnState.Set(null); + } } } @@ -270,16 +274,20 @@ protected async Task ProcessActivityAsync(string authHeader, Act activity.CallerId = authenticateRequestResult.CallerId; // Create the connector client to use for outbound requests. - var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), authenticateRequestResult.Credentials, _httpClient); - - // Create a turn context and run the pipeline. - using (var context = CreateTurnContext(activity, authenticateRequestResult.ClaimsIdentity, authenticateRequestResult.Scope, connectorClient, callback)) + using (var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), authenticateRequestResult.Credentials, _httpClient)) { - // Run the pipeline. - await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false); + // Create a turn context and run the pipeline. + using (var context = CreateTurnContext(activity, authenticateRequestResult.ClaimsIdentity, authenticateRequestResult.Scope, connectorClient, callback)) + { + // Run the pipeline. + await RunPipelineAsync(context, callback, cancellationToken).ConfigureAwait(false); - // If there are any results they will have been left on the TurnContext. - return ProcessTurnResults(context); + // Cleanup disposable resources in case other code kept a reference to it. + context.TurnState.Set(null); + + // If there are any results they will have been left on the TurnContext. + return ProcessTurnResults(context); + } } } From 63f951cb5ab9b6c2170e6df888ab5a76b51d731d Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Wed, 14 Oct 2020 23:01:04 -0700 Subject: [PATCH 4/8] ConnectorClient: Add special constructor to avoid disposing of custom HttpClient when provided --- .../BotFrameworkAdapter.cs | 4 +- .../Microsoft.Bot.Builder/CloudAdapterBase.cs | 4 +- .../Inspection/InspectionSession.cs | 2 +- .../Streaming/BotFrameworkHttpAdapterBase.cs | 2 +- .../ConnectorClientEx.cs | 41 ++++++++++++++++++ .../ConnectorClientTest.cs | 43 ++++++++++++++++++- 6 files changed, 89 insertions(+), 7 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs index 96a592144e..67185f9ceb 100644 --- a/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs +++ b/libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs @@ -1603,14 +1603,14 @@ private IConnectorClient CreateConnectorClient(string serviceUrl, AppCredentials ConnectorClient connectorClient; if (appCredentials != null) { - connectorClient = new ConnectorClient(new Uri(serviceUrl), appCredentials, customHttpClient: _httpClient); + connectorClient = new ConnectorClient(new Uri(serviceUrl), appCredentials, customHttpClient: _httpClient, disposeHttpClient: _httpClient == null); } else { var emptyCredentials = (ChannelProvider != null && ChannelProvider.IsGovernment()) ? MicrosoftGovernmentAppCredentials.Empty : MicrosoftAppCredentials.Empty; - connectorClient = new ConnectorClient(new Uri(serviceUrl), emptyCredentials, customHttpClient: _httpClient); + connectorClient = new ConnectorClient(new Uri(serviceUrl), emptyCredentials, customHttpClient: _httpClient, disposeHttpClient: _httpClient == null); } if (_connectorClientRetryPolicy != null) diff --git a/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs b/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs index 9a6f79e52a..db31c0bb42 100644 --- a/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs +++ b/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs @@ -243,7 +243,7 @@ protected async Task ProcessProactiveAsync(ClaimsIdentity claimsIdentity, Conver var credentials = await _botFrameworkAuthentication.GetProactiveCredentialsAsync(claimsIdentity, audience, cancellationToken).ConfigureAwait(false); // Create the connector client to use for outbound requests. - using (var connectorClient = new ConnectorClient(new Uri(reference.ServiceUrl), credentials, _httpClient)) + using (var connectorClient = new ConnectorClient(new Uri(reference.ServiceUrl), credentials, _httpClient, disposeHttpClient: _httpClient == null)) { // Create a turn context and run the pipeline. using (var context = CreateTurnContext(reference.GetContinuationActivity(), claimsIdentity, audience, connectorClient, callback)) @@ -274,7 +274,7 @@ protected async Task ProcessActivityAsync(string authHeader, Act activity.CallerId = authenticateRequestResult.CallerId; // Create the connector client to use for outbound requests. - using (var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), authenticateRequestResult.Credentials, _httpClient)) + using (var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), authenticateRequestResult.Credentials, _httpClient, disposeHttpClient: _httpClient == null)) { // Create a turn context and run the pipeline. using (var context = CreateTurnContext(activity, authenticateRequestResult.ClaimsIdentity, authenticateRequestResult.Scope, connectorClient, callback)) diff --git a/libraries/Microsoft.Bot.Builder/Inspection/InspectionSession.cs b/libraries/Microsoft.Bot.Builder/Inspection/InspectionSession.cs index 9bb0697c5a..87eeb75f80 100644 --- a/libraries/Microsoft.Bot.Builder/Inspection/InspectionSession.cs +++ b/libraries/Microsoft.Bot.Builder/Inspection/InspectionSession.cs @@ -22,7 +22,7 @@ public InspectionSession(ConversationReference conversationReference, MicrosoftA { _conversationReference = conversationReference; _logger = logger; - _connectorClient = new ConnectorClient(new Uri(_conversationReference.ServiceUrl), credentials, httpClient); + _connectorClient = new ConnectorClient(new Uri(_conversationReference.ServiceUrl), credentials, httpClient, disposeHttpClient: httpClient == null); } public async Task SendAsync(Activity activity, CancellationToken cancellationToken) diff --git a/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs b/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs index 4a3b57c38c..e99185b50f 100644 --- a/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs +++ b/libraries/Microsoft.Bot.Builder/Streaming/BotFrameworkHttpAdapterBase.cs @@ -298,7 +298,7 @@ private IConnectorClient CreateStreamingConnectorClient(Activity activity, Strea #pragma warning disable CA2000 // Dispose objects before losing scope (We need to make ConnectorClient disposable to fix this, ignoring it for now) var streamingClient = new StreamingHttpClient(requestHandler, Logger); #pragma warning restore CA2000 // Dispose objects before losing scope - var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), emptyCredentials, customHttpClient: streamingClient); + var connectorClient = new ConnectorClient(new Uri(activity.ServiceUrl), emptyCredentials, customHttpClient: streamingClient, disposeHttpClient: false); return connectorClient; } } diff --git a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs index e819ddaeee..2a5c94c65a 100644 --- a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs +++ b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs @@ -62,6 +62,42 @@ public ConnectorClient(Uri baseUri, MicrosoftAppCredentials credentials, HttpCli { } + /// + /// Initializes a new instance of the class. + /// + /// Base URI for the Bot Connector service. + /// Credentials for the Bot Connector service. + /// The HTTP client to use for this connector client. + /// Whether to dispose the . + /// Constructor specifically designed to be the one that allows control of the disposing of the custom . + /// only has one constructor that accepts control of the disposing of the , so we call that overload here. + /// All other overloads of will not control this parameter and it will default to true, resulting on disposal of the provided when the is disposed. + /// When reusing instances across connectors, pass 'false' for to avoid . +#pragma warning disable CA1801 // Review unused parameters (we can't change this without breaking binary compat) + public ConnectorClient(Uri baseUri, ServiceClientCredentials credentials, HttpClient customHttpClient, bool disposeHttpClient) +#pragma warning restore CA1801 // Review unused parameters + : base(customHttpClient, disposeHttpClient) + { + this.Credentials = credentials; + if (customHttpClient != null) + { + this.HttpClient = customHttpClient; + + // Note don't call AddDefaultRequestHeaders(HttpClient) here because the BotFrameworkAdapter + // called it. Updating DefaultRequestHeaders is not thread safe this is OK because the + // adapter should be a singleton. + } + + if (baseUri == null) + { + throw new ArgumentNullException(nameof(baseUri)); + } + + BaseUri = baseUri; + + Initialize(); + } + /// /// Initializes a new instance of the class. /// @@ -184,6 +220,11 @@ public static void AddDefaultRequestHeaders(HttpClient httpClient) } } + //protected override void Dispose(bool disposing) + //{ + // base.Dispose(disposing); + //} + /// /// Gets the assembly version for this Bot Connector client. /// diff --git a/tests/Microsoft.Bot.Connector.Tests/ConnectorClientTest.cs b/tests/Microsoft.Bot.Connector.Tests/ConnectorClientTest.cs index fb6ffd333e..c0adf6556e 100644 --- a/tests/Microsoft.Bot.Connector.Tests/ConnectorClientTest.cs +++ b/tests/Microsoft.Bot.Connector.Tests/ConnectorClientTest.cs @@ -3,6 +3,7 @@ using System; using System.Net.Http; +using System.Threading.Tasks; using Microsoft.Bot.Connector.Authentication; using Microsoft.Bot.Schema; using Xunit; @@ -12,7 +13,7 @@ namespace Microsoft.Bot.Connector.Tests public class ConnectorClientTest : BaseTest { [Fact] - public void ConnectorClientWithCustomHttpClientAndMicrosoftCredentials() + public void ConnectorClient_CustomHttpClient_AndMicrosoftCredentials() { var baseUri = new Uri("https://test.coffee"); var customHttpClient = new HttpClient(); @@ -23,5 +24,45 @@ public void ConnectorClientWithCustomHttpClientAndMicrosoftCredentials() Assert.Equal(connector.HttpClient.BaseAddress, baseUri); } + + [Fact] + public async Task ConnectorClient_CustomHttpClientAndCredConstructor_HttpClientDisposed() + { + var baseUri = new Uri("https://test.coffee"); + var customHttpClient = new HttpClient(); + + using (var connector = new ConnectorClient(new Uri("http://localhost/"), new MicrosoftAppCredentials(string.Empty, string.Empty), customHttpClient)) + { + // Use the connector + } + + await Assert.ThrowsAsync(() => customHttpClient.GetAsync("http://bing.com")); + } + + [Fact] + public async Task ConnectorClient_CustomHttpClientAndDisposeFalse_HttpClientNotDisposed() + { + var baseUri = new Uri("https://test.coffee"); + var customHttpClient = new HttpClient(); + + using (var connector = new ConnectorClient(new Uri("http://localhost/"), new MicrosoftAppCredentials(string.Empty, string.Empty), customHttpClient, disposeHttpClient: customHttpClient == null)) + { + // Use the connector + } + + // If the HttpClient were disposed, this would throw ObjectDisposedException + await customHttpClient.GetAsync("http://bing.com"); + } + + [Fact] + public void ConnectorClient_CustomHttpClientNull_Works() + { + var baseUri = new Uri("https://test.coffee"); + + using (var connector = new ConnectorClient(new Uri("http://localhost/"), new MicrosoftAppCredentials(string.Empty, string.Empty), null, disposeHttpClient: true)) + { + // Use the connector + } + } } } From ffcdcffd963046ade6b871f693c6092b2ec4afd5 Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Wed, 14 Oct 2020 23:24:20 -0700 Subject: [PATCH 5/8] ConnectorClient: Override base uri after initialize so it's not overriden --- libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs index 2a5c94c65a..0631737e31 100644 --- a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs +++ b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs @@ -93,9 +93,9 @@ public ConnectorClient(Uri baseUri, ServiceClientCredentials credentials, HttpCl throw new ArgumentNullException(nameof(baseUri)); } - BaseUri = baseUri; - Initialize(); + + BaseUri = baseUri; } /// From 9158206a8217ac2c4af3f0803874f6f4528d34c8 Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Thu, 15 Oct 2020 00:13:24 -0700 Subject: [PATCH 6/8] Streaming: ResponseExtensions - add async version of read as string --- .../ReceiveResponseExtensions.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libraries/Microsoft.Bot.Streaming/ReceiveResponseExtensions.cs b/libraries/Microsoft.Bot.Streaming/ReceiveResponseExtensions.cs index 3fddd5b783..2b8a521409 100644 --- a/libraries/Microsoft.Bot.Streaming/ReceiveResponseExtensions.cs +++ b/libraries/Microsoft.Bot.Streaming/ReceiveResponseExtensions.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using System.Text; +using System.Threading.Tasks; using Newtonsoft.Json; namespace Microsoft.Bot.Streaming @@ -52,12 +53,23 @@ public static T ReadBodyAsJson(this ReceiveResponse response) /// On success, an of the data from the body. /// public static string ReadBodyAsString(this ReceiveResponse response) + { + return response.ReadBodyAsStringAsync().GetAwaiter().GetResult(); + } + + /// + /// Serializes the body of this as a . + /// + /// The current instance of . + /// On success, an of the data from the body. + /// + public static async Task ReadBodyAsStringAsync(this ReceiveResponse response) { var contentStream = response.Streams.FirstOrDefault(); if (contentStream != null) { - return contentStream.Stream.ReadAsUtf8String(); + return await contentStream.Stream.ReadAsUtf8StringAsync().ConfigureAwait(false); } return string.Empty; From 8d4bc8e8ef0951a16ac4a82ccea1061c55306365 Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Thu, 15 Oct 2020 00:31:59 -0700 Subject: [PATCH 7/8] Remove unused dispose --- libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs index 0631737e31..149e2ad4f3 100644 --- a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs +++ b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs @@ -220,11 +220,6 @@ public static void AddDefaultRequestHeaders(HttpClient httpClient) } } - //protected override void Dispose(bool disposing) - //{ - // base.Dispose(disposing); - //} - /// /// Gets the assembly version for this Bot Connector client. /// From 5d4037cc08df08806fe5607453ca9846d697412e Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Tue, 20 Oct 2020 20:24:00 -0700 Subject: [PATCH 8/8] Connectorclient: remove unnecessary customHttpClient assignment --- libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs index 149e2ad4f3..7b05a12829 100644 --- a/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs +++ b/libraries/Microsoft.Bot.Connector/ConnectorClientEx.cs @@ -79,14 +79,6 @@ public ConnectorClient(Uri baseUri, ServiceClientCredentials credentials, HttpCl : base(customHttpClient, disposeHttpClient) { this.Credentials = credentials; - if (customHttpClient != null) - { - this.HttpClient = customHttpClient; - - // Note don't call AddDefaultRequestHeaders(HttpClient) here because the BotFrameworkAdapter - // called it. Updating DefaultRequestHeaders is not thread safe this is OK because the - // adapter should be a singleton. - } if (baseUri == null) {