Merge pull request #1 from Microsoft/master

merging

libraries/Microsoft.Bot.Builder.Ai/TranslationMiddleware.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Net.Http;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Microsoft.Bot.Builder.Middleware;
@@ -16,13 +17,13 @@ public class TranslationMiddleware : IReceiveActivity, ISendActivity
     {
         private LuisClient luisClient;
         private string[] nativeLanguages;
-        private Translator translator;
+        private Translator translator;        
 
-        public TranslationMiddleware(string[] nativeLanguages, string translatorKey, string luisAppId, string luisAccessKey)
+        public TranslationMiddleware(HttpClient httpClient, string[] nativeLanguages, string translatorKey, string luisAppId, string luisAccessKey)
         {
             this.nativeLanguages = nativeLanguages;
             this.luisClient = new LuisClient(luisAppId, luisAccessKey);
-            this.translator = new Translator(translatorKey);
+            this.translator = new Translator(translatorKey, httpClient);
         }
 
         /// <summary>

libraries/Microsoft.Bot.Builder.Ai/Translator.cs

@@ -17,34 +17,43 @@ namespace Microsoft.Bot.Builder.Ai
 
     internal class Translator
     {
-        AzureAuthToken authToken;
+        private readonly AzureAuthToken _authToken;
+        private readonly HttpClient _httpClient;
 
-        internal Translator(string apiKey)
+        internal Translator(string apiKey, HttpClient httpClient)
         {
-            authToken = new AzureAuthToken(apiKey);
+            _authToken = new AzureAuthToken(apiKey);
+            _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
         }
 
         internal async Task<string> Translate(string textToTranslate, string to)
         {
             string url = "http://api.microsofttranslator.com/v2/Http.svc/Translate";
-            string query = $"?text={System.Net.WebUtility.UrlEncode(textToTranslate)}&to={to}&contentType=text/plain";
+            string query = $"?text={WebUtility.UrlEncode(textToTranslate)}&to={to}&contentType=text/plain";
 
-            using (var client = new HttpClient())
+            var accessToken = await _authToken.GetAccessTokenAsync(_httpClient).ConfigureAwait(false);
+
+            // The HTTPClient is shared across multiple requests, meaning we can't simply 
+            // set the default headers for this query. To do that, we create a specific HTTP Request
+            // and add the relevant headers to it. 
+            using (var requestMessage = new HttpRequestMessage(HttpMethod.Get, url + query))
             {
-                var accessToken = await authToken.GetAccessTokenAsync().ConfigureAwait(false);
-                client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", accessToken);
-                var response = await client.GetAsync(url + query);
+                // Add the headers into the HTTP Request. 
+                requestMessage.Headers.Authorization = new AuthenticationHeaderValue("Bearer", accessToken);
+
+                var response = await _httpClient.SendAsync(requestMessage);
                 var result = await response.Content.ReadAsStringAsync();
 
                 if (!response.IsSuccessStatusCode)
+                {
                     return "ERROR: " + result;
+                }
 
                 var translatedText = XElement.Parse(result).Value;
                 return translatedText;
             }
         }
 
-
         internal async Task<string[]> TranslateArray(string[] translateArraySourceTexts, string from, string to)
         {
             var uri = "https://api.microsofttranslator.com/v2/Http.svc/TranslateArray";
@@ -65,17 +74,16 @@ internal async Task<string[]> TranslateArray(string[] translateArraySourceTexts,
                            $"<To>{to}</To>" +
                        "</TranslateArrayRequest>";
 
-            var accessToken = await authToken.GetAccessTokenAsync().ConfigureAwait(false);
-
-            using (var client = new HttpClient())
+            var accessToken = await _authToken.GetAccessTokenAsync(_httpClient).ConfigureAwait(false);
+
             using (var request = new HttpRequestMessage())
             {
                 request.Method = HttpMethod.Post;
                 request.RequestUri = new Uri(uri);
                 request.Content = new StringContent(body, Encoding.UTF8, "text/xml");
                 request.Headers.Add("Authorization", accessToken);
-
-                var response = await client.SendAsync(request);
+                
+                var response = await _httpClient.SendAsync(request);
                 var responseBody = await response.Content.ReadAsStringAsync();
                 switch (response.StatusCode)
                 {
@@ -98,7 +106,6 @@ internal async Task<string[]> TranslateArray(string[] translateArraySourceTexts,
                 }
             }
         }
-
     }
 
     internal class AzureAuthToken
@@ -149,24 +156,22 @@ internal AzureAuthToken(string key)
         /// invocations of the method return the cached token for the next 5 minutes. After
         /// 5 minutes, a new token is fetched from the token service and the cache is updated.
         /// </remarks>
-        internal async Task<string> GetAccessTokenAsync()
+        internal async Task<string> GetAccessTokenAsync(HttpClient httpClient)
         {
             if (string.IsNullOrWhiteSpace(this.SubscriptionKey))
                 return string.Empty;
 
             // Re-use the cached token if there is one.
             if ((DateTime.Now - _storedTokenTime) < TokenCacheDuration)
                 return _storedTokenValue;
-
-            using (var client = new HttpClient())
+
             using (var request = new HttpRequestMessage())
             {
                 request.Method = HttpMethod.Post;
                 request.RequestUri = ServiceUrl;
                 request.Content = new StringContent(string.Empty);
-                request.Headers.TryAddWithoutValidation(OcpApimSubscriptionKeyHeader, this.SubscriptionKey);
-                client.Timeout = TimeSpan.FromSeconds(2);
-                var response = await client.SendAsync(request);
+                request.Headers.TryAddWithoutValidation(OcpApimSubscriptionKeyHeader, this.SubscriptionKey);                
+                var response = await httpClient.SendAsync(request);
                 this.RequestStatusCode = response.StatusCode;
                 response.EnsureSuccessStatusCode();
                 var token = await response.Content.ReadAsStringAsync();
@@ -176,5 +181,4 @@ internal async Task<string> GetAccessTokenAsync()
             }
         }
     }
-
 }

libraries/Microsoft.Bot.Builder.BotFramework/BotFrameworkAdapter.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Net.Http;
 using System.Threading.Tasks;
 using Microsoft.Bot.Builder.Adapters;
 using Microsoft.Bot.Connector;
@@ -16,15 +17,18 @@ public class BotFrameworkAdapter : ActivityAdapterBase
     {
         private readonly SimpleCredentialProvider _credentialProvider;
         private readonly MicrosoftAppCredentials _credentials;
+        private readonly HttpClient _httpClient; 
 
-        public BotFrameworkAdapter(IConfiguration configuration) : base()
+        public BotFrameworkAdapter(IConfiguration configuration, HttpClient httpClient = null) : base()
         {
+            _httpClient = httpClient ?? new HttpClient();
             _credentialProvider = new ConfigurationCredentialProvider(configuration);
-            _credentials = new MicrosoftAppCredentials(this._credentialProvider.AppId, _credentialProvider.Password);                       
+            _credentials = new MicrosoftAppCredentials(_credentialProvider.AppId, _credentialProvider.Password);                                   
         }
 
-        public BotFrameworkAdapter(string appId, string appPassword) : base()
+        public BotFrameworkAdapter(string appId, string appPassword, HttpClient httpClient = null) : base()
         {
+            _httpClient = httpClient ?? new HttpClient();
             _credentials = new MicrosoftAppCredentials(appId, appPassword);
             _credentialProvider = new SimpleCredentialProvider(appId, appPassword);
         }
@@ -53,11 +57,11 @@ public async override Task Send(IList<IActivity> activities)
         public async Task Receive(string authHeader, Activity activity)
         {
             BotAssert.ActivityNotNull(activity);
-            await JwtTokenValidation.AssertValidActivity(activity, authHeader, _credentialProvider);
+            await JwtTokenValidation.AssertValidActivity(activity, authHeader, _credentialProvider, _httpClient);
 
             if (this.OnReceive != null)
             {
-                await this.OnReceive(activity).ConfigureAwait(false);
+                await OnReceive(activity).ConfigureAwait(false);
             }
         }
     }

libraries/Microsoft.Bot.Builder/Adapters/TestAdapter.cs

@@ -272,10 +272,21 @@ public TestFlow Send(string userSays)
 
             return new TestFlow(this.testTask.ContinueWith((task) =>
             {
-                if (task.IsFaulted)
-                    throw new Exception("failed");
+                // NOTE: we need to .Wait() on the original Task to properly observe any exceptions that might have occurred
+                // and to have them propagate correctly up through the chain to whomever is waiting on the parent task
+                // The following StackOverflow answer provides some more details on why you want to do this: 
+                // https://stackoverflow.com/questions/11904821/proper-way-to-use-continuewith-for-tasks/11906865#11906865
+                //
+                // From the Docs:
+                //  https://docs.microsoft.com/en-us/dotnet/standard/parallel-programming/exception-handling-task-parallel-library
+                //  Exceptions are propagated when you use one of the static or instance Task.Wait or Wait 
+                //  methods, and you handle them by enclosing the call in a try/catch statement. If a task is the 
+                //  parent of attached child tasks, or if you are waiting on multiple tasks, multiple exceptions 
+                //  could be thrown.
+                task.Wait();
+
                 return this._adapter.SendActivityToBot(userSays);
-            }), this._adapter);
+            }).Unwrap(), this._adapter);
         }
 
         /// <summary>
@@ -290,10 +301,11 @@ public TestFlow Send(Activity userActivity)
 
             return new TestFlow(this.testTask.ContinueWith((task) =>
             {
-                if (task.IsFaulted)
-                    throw new Exception("failed");
+                // NOTE: See details code in above method. 
+                task.Wait();
+
                 return this._adapter.SendActivityToBot(userActivity);
-            }), this._adapter);
+            }).Unwrap(), this._adapter);
         }
 
         /// <summary>
@@ -305,10 +317,11 @@ public TestFlow Delay(UInt32 ms)
         {
             return new TestFlow(this.testTask.ContinueWith((task) =>
             {
-                if (task.IsFaulted)
-                    throw new Exception("failed");
+                // NOTE: See details code in above method. 
+                task.Wait();
+
                 return Task.Delay((int)ms);
-            }), this._adapter);
+            }).Unwrap(), this._adapter);
         }
 
         /// <summary>
@@ -353,16 +366,17 @@ public TestFlow AssertReply(Action<IActivity> validateActivity, string descripti
         {
             return new TestFlow(this.testTask.ContinueWith((task) =>
             {
-                if (task.IsFaulted)
-                    throw new Exception("failed");
+                // NOTE: See details code in above method. 
+                task.Wait();
+
                 var start = DateTime.UtcNow;
                 while (true)
                 {
                     var current = DateTime.UtcNow;
 
                     if ((current - start).TotalMilliseconds > timeout)
                     {
-                        throw new TimeoutException($"{timeout}ms Timed out waiting for:${description}");
+                        throw new TimeoutException($"{timeout}ms Timed out waiting for:'{description}'");
                     }
 
                     IActivity replyActivity = this._adapter.GetNextReply();                    

libraries/Microsoft.Bot.Connector.AspNetCore/BotAuthenticationHandler.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.IdentityModel.Tokens.Jwt;
+using System.Net.Http;
 using System.Security.Claims;
 using System.Text.Encodings.Web;
 using System.Threading;
@@ -15,12 +16,13 @@
 
 namespace Microsoft.Bot.Connector
 {
-
     /// <summary>
     /// Bot authentication hanlder used by <see cref="BotAuthenticationMiddleware"/>.
     /// </summary>
     public class BotAuthenticationHandler : AuthenticationHandler<BotAuthenticationOptions>
     {
+        private static readonly HttpClient _httpClient = new HttpClient();
+
         public BotAuthenticationHandler(IOptionsMonitor<BotAuthenticationOptions> options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock)
             : base(options, logger, encoder, clock)
         {
@@ -53,7 +55,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
                 }
 
                 string authHeader = Request.Headers["Authorization"];
-                ClaimsIdentity claimsIdentity = await JwtTokenValidation.ValidateAuthHeader(authHeader, Options.CredentialProvider);
+                ClaimsIdentity claimsIdentity = await JwtTokenValidation.ValidateAuthHeader(authHeader, Options.CredentialProvider, _httpClient);
 
                 Logger.TokenValidationSucceeded();
 
@@ -74,7 +76,6 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
                     return tokenValidatedContext.Result;
                 }
 
-
                 tokenValidatedContext.Success();
                 return tokenValidatedContext.Result;
             }            

libraries/Microsoft.Bot.Connector/AttachmentsEx.cs

@@ -14,6 +14,18 @@ namespace Microsoft.Bot.Connector
     /// </summary>
     public partial class Attachments
     {
+        /// <summary>
+        /// The attachment code uses this client. Ideally, this would be passed in or set via a DI system to 
+        /// allow developer control over behavior / headers / timesouts and such. Unfortunatly this is buried
+        /// pretty deep, the static solution used here is much cleaner. If this becomes an issue we could
+        /// consider circling back and exposing developer control over this HttpClient. 
+        /// </summary>
+        /// <remarks>
+        /// Relativly few bots use attachments, so rather than paying the startup cost, this is
+        /// a Lazy<> simply to avoid paying a static initialization penalty for every bot. 
+        /// </remarks>
+        private static Lazy<HttpClient> _httpClient = new Lazy<HttpClient>(); 
+
         /// <summary>
         /// Get the URI of an attachment view
         /// </summary>
@@ -43,10 +55,7 @@ public string GetAttachmentUri(string attachmentId, string viewId = "original")
         /// <returns>stream of attachment</returns>
         public Task<Stream> GetAttachmentStreamAsync(string attachmentId, string viewId = "original")
         {
-            using (HttpClient client = new HttpClient())
-            {
-                return client.GetStreamAsync(GetAttachmentUri(attachmentId, viewId));
-            }
+            return _httpClient.Value.GetStreamAsync(GetAttachmentUri(attachmentId, viewId));
         }
     }
 }

libraries/Microsoft.Bot.Connector/Authentication/ChannelValidation.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Linq;
+using System.Net.Http;
 using System.Security.Claims;
 using System.Threading.Tasks;
 using Microsoft.IdentityModel.Tokens;
@@ -37,12 +38,15 @@ public static class ChannelValidation
         /// </remarks>
         /// <param name="authHeader">The raw HTTP header in the format: "Bearer [longString]"</param>
         /// <param name="credentials">The user defined set of valid credentials, such as the AppId.</param>        
+        /// <param name="httpClient">Authentication of tokens requires calling out to validate Endorsements and related documents. The
+        /// HttpClient is used for making those calls. Those calls generally require TLS connections, which are expensive to 
+        /// setup and teardown, so a shared HttpClient is recommended.</param>
         /// <returns>
         /// A valid ClaimsIdentity. 
         /// </returns>
-        public static async Task<ClaimsIdentity> AuthenticateChannelToken(string authHeader, ICredentialProvider credentials)
+        public static async Task<ClaimsIdentity> AuthenticateChannelToken(string authHeader, ICredentialProvider credentials, HttpClient httpClient)
         {
-            var tokenExtractor = new JwtTokenExtractor(
+            var tokenExtractor = new JwtTokenExtractor(httpClient,
                   ToBotFromChannelTokenValidationParameters,
                   AuthenticationConstants.ToBotFromChannelOpenIdMetadataUrl,
                   AuthenticationConstants.AllowedSigningAlgorithms, null);
@@ -92,10 +96,19 @@ public static async Task<ClaimsIdentity> AuthenticateChannelToken(string authHea
 
             return identity;
         }
-
-        public static async Task<ClaimsIdentity> AuthenticateChannelToken(string authHeader, ICredentialProvider credentials, string serviceUrl)
+        /// <summary>        
+        /// Validate the incoming Auth Header as a token sent from the Bot Framework Service.
+        /// </summary>
+        /// <param name="authHeader">The raw HTTP header in the format: "Bearer [longString]"</param>
+        /// <param name="credentials">The user defined set of valid credentials, such as the AppId.</param>        
+        /// <param name="serviceUrl"></param>
+        /// <param name="httpClient">Authentication of tokens requires calling out to validate Endorsements and related documents. The
+        /// HttpClient is used for making those calls. Those calls generally require TLS connections, which are expensive to 
+        /// setup and teardown, so a shared HttpClient is recommended.</param>
+        /// <returns></returns>
+        public static async Task<ClaimsIdentity> AuthenticateChannelToken(string authHeader, ICredentialProvider credentials, string serviceUrl, HttpClient httpClient)
         {
-            var identity = await AuthenticateChannelToken(authHeader, credentials);      
+            var identity = await AuthenticateChannelToken(authHeader, credentials, httpClient);      
 
             var serviceUrlClaim = identity.Claims.FirstOrDefault(claim => claim.Type == ServiceUrlClaim)?.Value;
             if (string.IsNullOrWhiteSpace(serviceUrlClaim))