Simplify recognizer signature by moving constructor args into LuisRec…

…ognizerOptions. Fix all style warnings.
microsoft · Jul 19, 2019 · dd22c89 · dd22c89
1 parent 313a5f2
commit dd22c89
Show file tree

Hide file tree

Showing 28 changed files with 85 additions and 54 deletions.
diff --git a/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisPredictionOptions.cs b/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisPredictionOptions.cs
@@ -25,6 +25,13 @@ public class LuisPredictionOptions
         /// </value>
         public bool IncludeInstanceData { get; set; } = false;
 
+        /// <summary>
+        /// Gets or sets a value indicating whether API results should be included.
+        /// </summary>
+        /// <value>True to include API results.</value>
+        /// <remarks>This is mainly useful for testing or getting access to LUIS features not yet in the SDK.</remarks>
+        public bool IncludeAPIResults { get; set; } = false;
+
         /// <summary>
         /// Gets or sets a value indicating whether queries should be logged in LUIS.
         /// </summary>

diff --git a/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisRecognizer.cs b/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisRecognizer.cs
@@ -33,28 +33,25 @@ public class LuisRecognizer : ITelemetryRecognizer
 
         private readonly LuisApplication _application;
         private readonly LuisPredictionOptions _predictionOptions;
-        private readonly bool _includeApiResults;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="LuisRecognizer"/> class.
         /// </summary>
         /// <param name="application">The LUIS application to use to recognize text.</param>
         /// <param name="recognizerOptions">(Optional) Options for the created recognizer.</param>
         /// <param name="predictionOptions">(Optional) The default LUIS prediction options to use.</param>
-        /// <param name="includeApiResults">(Optional) TRUE to include raw LUIS API response.</param>
         /// <param name="clientHandler">(Optional) Custom handler for LUIS API calls to allow mocking.</param>
-        public LuisRecognizer(LuisApplication application, LuisRecognizerOptions recognizerOptions = null, LuisPredictionOptions predictionOptions = null, bool includeApiResults = false, HttpClientHandler clientHandler = null)
+        public LuisRecognizer(LuisApplication application, LuisRecognizerOptions recognizerOptions = null, LuisPredictionOptions predictionOptions = null)
         {
             recognizerOptions = recognizerOptions ?? new LuisRecognizerOptions();
             _application = application ?? throw new ArgumentNullException(nameof(application));
             _predictionOptions = predictionOptions ?? new LuisPredictionOptions();
-            _includeApiResults = includeApiResults;
 
             TelemetryClient = recognizerOptions.TelemetryClient;
             LogPersonalInformation = recognizerOptions.LogPersonalInformation;
 
             var delegatingHandler = new LuisDelegatingHandler();
-            var httpClientHandler = clientHandler ?? CreateRootHandler();
+            var httpClientHandler = recognizerOptions.HttpClient ?? CreateRootHandler();
             var currentHandler = CreateHttpHandlerPipeline(httpClientHandler, delegatingHandler);
 
             DefaultHttpClient = new HttpClient(currentHandler, false)
@@ -221,11 +218,9 @@ public virtual async Task<T> RecognizeAsync<T>(ITurnContext turnContext, Diction
         /// <param name="turnContext">Context object containing information for a single turn of conversation with a user.</param>
         /// <param name="telemetryProperties">Additional properties to be logged to telemetry with the LuisResult event.</param>
         /// <param name="telemetryMetrics">Additional metrics to be logged to telemetry with the LuisResult event.</param>
-        /// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
-        /// <returns><see cref="Task"/>.</returns>
-        protected virtual async Task OnRecognizerResultAsync(RecognizerResult recognizerResult, ITurnContext turnContext, Dictionary<string, string> telemetryProperties = null, Dictionary<string, double> telemetryMetrics = null, CancellationToken cancellationToken = default)
+        protected virtual void OnRecognizerResult(RecognizerResult recognizerResult, ITurnContext turnContext, Dictionary<string, string> telemetryProperties = null, Dictionary<string, double> telemetryMetrics = null)
         {
-            var properties = await FillLuisEventPropertiesAsync(recognizerResult, turnContext, telemetryProperties, cancellationToken).ConfigureAwait(false);
+            var properties = FillLuisEventProperties(recognizerResult, turnContext, telemetryProperties);
 
             // Track the event
             TelemetryClient.TrackEvent(LuisTelemetryConstants.LuisResult, properties, telemetryMetrics);
@@ -238,10 +233,8 @@ protected virtual async Task OnRecognizerResultAsync(RecognizerResult recognizer
         /// <param name="recognizerResult">Last activity sent from user.</param>
         /// <param name="turnContext">Context object containing information for a single turn of conversation with a user.</param>
         /// <param name="telemetryProperties">Additional properties to be logged to telemetry with the LuisResult event.</param>
-        /// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
-        /// additionalProperties
         /// <returns>A dictionary that is sent as "Properties" to IBotTelemetryClient.TrackEvent method for the BotMessageSend event.</returns>
-        protected Task<Dictionary<string, string>> FillLuisEventPropertiesAsync(RecognizerResult recognizerResult, ITurnContext turnContext, Dictionary<string, string> telemetryProperties = null, CancellationToken cancellationToken = default)
+        protected Dictionary<string, string> FillLuisEventProperties(RecognizerResult recognizerResult, ITurnContext turnContext, Dictionary<string, string> telemetryProperties = null)
         {
             var topTwoIntents = (recognizerResult.Intents.Count > 0) ? recognizerResult.Intents.OrderByDescending(x => x.Value.Score).Take(2).ToArray() : null;
 
@@ -281,12 +274,12 @@ protected virtual async Task OnRecognizerResultAsync(RecognizerResult recognizer
             // Additional Properties can override "stock" properties.
             if (telemetryProperties != null)
             {
-                return Task.FromResult(telemetryProperties.Concat(properties)
+                properties = telemetryProperties.Concat(properties)
                            .GroupBy(kv => kv.Key)
-                           .ToDictionary(g => g.Key, g => g.First().Value));
+                           .ToDictionary(g => g.Key, g => g.First().Value);
             }
 
-            return Task.FromResult(properties);
+            return properties;
         }
 
         private string AddParam(string query, string prop, bool? val)
@@ -394,14 +387,14 @@ private async Task<RecognizerResult> RecognizeInternalAsync(ITurnContext turnCon
                     Entities = LuisUtil.ExtractEntitiesAndMetadata(prediction),
                 };
                 LuisUtil.AddProperties(prediction, recognizerResult);
-                if (_includeApiResults)
+                if (options.IncludeAPIResults)
                 {
                     recognizerResult.Properties.Add("luisResult", luisResponse);
                 }
             }
 
             // Log telemetry
-            await OnRecognizerResultAsync(recognizerResult, turnContext, telemetryProperties, telemetryMetrics, cancellationToken).ConfigureAwait(false);
+            OnRecognizerResult(recognizerResult, turnContext, telemetryProperties, telemetryMetrics);
 
             var traceInfo = JObject.FromObject(
                 new

diff --git a/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisRecognizerOptions.cs b/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisRecognizerOptions.cs
@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license.
+using System.Net.Http;
 using Newtonsoft.Json;
 
 namespace Microsoft.Bot.Builder.AI.Luis
@@ -31,5 +32,13 @@ public class LuisRecognizerOptions
         /// </summary>
         /// <value>If true, personal information is logged to Telemetry; otherwise the properties will be filtered.</value>
         public bool LogPersonalInformation { get; set; } = false;
+
+        /// <summary>
+        /// Gets or sets the handler for sending http calls.
+        /// </summary>
+        /// <value>
+        /// Handler for intercepting http calls for logging or testing.
+        /// </value>
+        public HttpClientHandler HttpClient { get; set; }
     }
 }
diff --git a/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisUtil.cs b/libraries/Microsoft.Bot.Builder.AI.LuisV3/LuisUtil.cs
@@ -10,8 +10,8 @@ namespace Microsoft.Bot.Builder.AI.Luis
     // Utility functions used to extract and transform data from Luis SDK
     internal static class LuisUtil
     {
-        internal const string _metadataKey = "$instance";
-        internal const string _geoV2 = "builtin.geographyV2.";
+        internal const string MetadataKey = "$instance";
+        internal const string GeoV2 = "builtin.geographyV2.";
         internal static readonly HashSet<string> _dateSubtypes = new HashSet<string> { "date", "daterange", "datetime", "datetimerange", "duration", "set", "time", "timerange" };
 
         internal static string NormalizedIntent(string intent) => intent.Replace('.', '_').Replace(' ', '_');
@@ -48,11 +48,11 @@ internal static void FindGeographyTypes(JToken source, Dictionary<string, string
             {
                 if (source is JObject obj)
                 {
-                    if (obj.TryGetValue("type", out var type) && type.Type == JTokenType.String && type.Value<string>().StartsWith(_geoV2))
+                    if (obj.TryGetValue("type", out var type) && type.Type == JTokenType.String && type.Value<string>().StartsWith(GeoV2))
                     {
-                        var path = type.Path.Replace(_metadataKey + ".", string.Empty);
+                        var path = type.Path.Replace(MetadataKey + ".", string.Empty);
                         path = path.Substring(0, path.LastIndexOf('.'));
-                        geoTypes.Add(path, type.Value<string>().Substring(_geoV2.Length));
+                        geoTypes.Add(path, type.Value<string>().Substring(GeoV2.Length));
                     }
                     else
                     {
@@ -112,7 +112,7 @@ internal static JToken MapProperties(JToken source, bool inInstance, Dictionary<
                         var isArr = property.Value.Type == JTokenType.Array;
                         var isStr = property.Value.Type == JTokenType.String;
                         var isInt = property.Value.Type == JTokenType.Integer;
-                        var val = MapProperties(property.Value, inInstance || property.Name == _metadataKey, geoTypes);
+                        var val = MapProperties(property.Value, inInstance || property.Name == MetadataKey, geoTypes);
                         if (name == "datetime" && isArr)
                         {
                             nobj.Add("datetimeV1", val);

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/LuisOracleTests.cs b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/LuisOracleTests.cs
@@ -579,7 +579,7 @@ public async Task Telemetry_OverrideOnDeriveAsync()
                 TelemetryClient = telemetryClient.Object,
                 LogPersonalInformation = false,
             };
-            var recognizer = new TelemetryOverrideRecognizer(telemetryClient.Object, luisApp, options, false, false, clientHandler);
+            var recognizer = new TelemetryOverrideRecognizer(luisApp, options, false, false, clientHandler);
 
             var additionalProperties = new Dictionary<string, string>
             {

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TelemetryOverrideRecognizer.cs b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TelemetryOverrideRecognizer.cs
@@ -10,7 +10,7 @@ namespace Microsoft.Bot.Builder.AI.Luis.Tests
 {
     public class TelemetryOverrideRecognizer : LuisRecognizer
     {
-        public TelemetryOverrideRecognizer(IBotTelemetryClient telemetryClient, LuisApplication application, LuisPredictionOptions predictionOptions = null, bool includeApiResults = false, bool logPersonalInformation = false, HttpClientHandler clientHandler = null)
+        public TelemetryOverrideRecognizer(LuisApplication application, LuisPredictionOptions predictionOptions = null, bool includeApiResults = false, bool logPersonalInformation = false, HttpClientHandler clientHandler = null)
            : base(application, predictionOptions, includeApiResults, clientHandler)
         {
             LogPersonalInformation = logPersonalInformation;

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Composite1.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Composite1.json
@@ -1275,6 +1275,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Composite2.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Composite2.json
@@ -280,6 +280,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Composite3.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Composite3.json
@@ -295,6 +295,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/DynamicListsAndList.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/DynamicListsAndList.json
@@ -113,6 +113,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndBuiltin.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndBuiltin.json
@@ -80,6 +80,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndComposite.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndComposite.json
@@ -129,6 +129,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndList.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndList.json
@@ -86,6 +86,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndRegex.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndRegex.json
@@ -79,6 +79,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndSimple.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndSimple.json
@@ -144,6 +144,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndSimpleOverride.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/ExternalEntitiesAndSimpleOverride.json
@@ -149,6 +149,7 @@
         }
       ],
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/GeoPeopleOrdinal.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/GeoPeopleOrdinal.json
@@ -264,6 +264,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Minimal.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Minimal.json
@@ -90,6 +90,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": false,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": false,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Patterns.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Patterns.json
@@ -221,6 +221,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Prebuilt.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Prebuilt.json
@@ -226,6 +226,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/TraceActivity.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/TraceActivity.json
@@ -157,6 +157,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Typed.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/Typed.json
@@ -1275,6 +1275,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/TypedPrebuilt.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/TypedPrebuilt.json
@@ -226,6 +226,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,

diff --git a/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/roles.json b/tests/Microsoft.Bot.Builder.AI.LUIS.Tests/TestData/roles.json
@@ -1504,6 +1504,7 @@
   "v3": {
     "options": {
       "IncludeAllIntents": true,
+      "IncludeAPIResults": true,
       "IncludeInstanceData": true,
       "Log": true,
       "PreferExternalEntities": true,