Merge pull request #3919 from microsoft/tomlm/bug3980

Fix QnAMakerDialog to handle interuption scenarios

.gitignore

@@ -293,3 +293,4 @@ appsettings.Development.json
 **/*/.luisrc
 /.vscode/launch.json
 /outputpackages
+/tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Tests/ComposerBugs

libraries/Microsoft.Bot.Builder.AI.QnA/Dialogs/QnAMakerDialog.cs

@@ -333,6 +333,59 @@ public QnAMakerDialog([CallerFilePath] string sourceFilePath = "", [CallerLineNu
             return await base.BeginDialogAsync(dc, dialogOptions, cancellationToken).ConfigureAwait(false);
         }
 
+        public override Task<DialogTurnResult> ContinueDialogAsync(DialogContext dc, CancellationToken cancellationToken = default)
+        {
+            var interrupted = dc.State.GetValue<bool>(TurnPath.Interrupted, () => false);
+            if (interrupted)
+            {
+                // if qnamaker was interrupted then end the qnamaker dialog
+                return dc.EndDialogAsync(cancellationToken: cancellationToken);
+            }
+
+            return base.ContinueDialogAsync(dc, cancellationToken);
+        }
+
+        protected override async Task<bool> OnPreBubbleEventAsync(DialogContext dc, DialogEvent e, CancellationToken cancellationToken)
+        {
+            if (dc.Context.Activity.Type == ActivityTypes.Message)
+            {
+                // decide whether we want to allow interruption or not.
+                // if we don't get a response from QnA which signifies we expected it,
+                // then we allow interruption.
+
+                var reply = dc.Context.Activity.Text;
+                var dialogOptions = ObjectPath.GetPathValue<QnAMakerDialogOptions>(dc.ActiveDialog.State, Options);
+
+                if (reply.Equals(dialogOptions.ResponseOptions.CardNoMatchText, StringComparison.OrdinalIgnoreCase))
+                {
+                    // it matches nomatch text, we like that.
+                    return true;
+                }
+
+                var suggestedQuestions = dc.State.GetValue<List<string>>($"this.suggestedQuestions");
+                if (suggestedQuestions != null && suggestedQuestions.Any(question => string.Compare(question, reply.Trim(), ignoreCase: true) == 0))
+                {
+                    // it matches one of the suggested actions, we like that.
+                    return true;
+                }
+
+                // Calling QnAMaker to get response.
+                var qnaClient = await GetQnAMakerClientAsync(dc).ConfigureAwait(false);
+                ResetOptions(dc, dialogOptions);
+
+                var response = await qnaClient.GetAnswersRawAsync(dc.Context, dialogOptions.QnAMakerOptions).ConfigureAwait(false);
+
+                // cache result so step doesn't have to do it again, this is a turn cache and we use hashcode so we don't conflict with any other qnamakerdialogs out there.
+                dc.State.SetValue($"turn.qnaresult{this.GetHashCode()}", response);
+
+                // disable interruption if we have answers.
+                return response.Answers.Any();
+            }
+
+            // call base for default behavior.
+            return await OnPostBubbleEventAsync(dc, e, cancellationToken).ConfigureAwait(false);
+        }
+
         /// <summary>
         /// Gets an <see cref="IQnAMakerClient"/> to use to access the QnA Maker knowledge base.
         /// </summary>
@@ -397,39 +450,25 @@ protected async virtual Task<QnADialogResponseOptions> GetQnAResponseOptionsAsyn
 
         private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContext stepContext, CancellationToken cancellationToken)
         {
-            var dialogOptions = ObjectPath.GetPathValue<QnAMakerDialogOptions>(stepContext.ActiveDialog.State, Options);
+            // clear suggestedQuestions between turns.
+            stepContext.State.RemoveValue($"this.suggestedQuestions");
 
-            // Resetting context and QnAId
-            dialogOptions.QnAMakerOptions.QnAId = 0;
-            dialogOptions.QnAMakerOptions.Context = new QnARequestContext();
+            var dialogOptions = ObjectPath.GetPathValue<QnAMakerDialogOptions>(stepContext.ActiveDialog.State, Options);
+            ResetOptions(stepContext, dialogOptions);
 
             // Storing the context info
             stepContext.Values[ValueProperty.CurrentQuery] = stepContext.Context.Activity.Text;
 
-            // -Check if previous context is present, if yes then put it with the query
-            // -Check for id if query is present in reverse index.
-            var previousContextData = ObjectPath.GetPathValue<Dictionary<string, int>>(stepContext.ActiveDialog.State, QnAContextData, new Dictionary<string, int>());
-            var previousQnAId = ObjectPath.GetPathValue<int>(stepContext.ActiveDialog.State, PreviousQnAId, 0);
-
-            if (previousQnAId > 0)
-            {
-                dialogOptions.QnAMakerOptions.Context = new QnARequestContext
-                {
-                    PreviousQnAId = previousQnAId
-                };
-
-                if (previousContextData.TryGetValue(stepContext.Context.Activity.Text, out var currentQnAId))
-                {
-                    dialogOptions.QnAMakerOptions.QnAId = currentQnAId;
-                }
-            }
-
             // Calling QnAMaker to get response.
             var qnaClient = await GetQnAMakerClientAsync(stepContext).ConfigureAwait(false);
-            var response = await qnaClient.GetAnswersRawAsync(stepContext.Context, dialogOptions.QnAMakerOptions).ConfigureAwait(false);
+            var response = stepContext.State.GetValue<QueryResults>($"turn.qnaresult{this.GetHashCode()}");
+            if (response == null)
+            {
+                response = await qnaClient.GetAnswersRawAsync(stepContext.Context, dialogOptions.QnAMakerOptions).ConfigureAwait(false);
+            }
 
             // Resetting previous query.
-            previousQnAId = -1;
+            var previousQnAId = -1;
             ObjectPath.SetPathValue(stepContext.ActiveDialog.State, PreviousQnAId, previousQnAId);
 
             // Take this value from GetAnswerResponse 
@@ -457,6 +496,7 @@ private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContex
                     await stepContext.Context.SendActivityAsync(message).ConfigureAwait(false);
 
                     ObjectPath.SetPathValue(stepContext.ActiveDialog.State, Options, dialogOptions);
+                    stepContext.State.SetValue($"this.suggestedQuestions", suggestedQuestions);
                     return new DialogTurnResult(DialogTurnStatus.Waiting);
                 }
             }
@@ -474,6 +514,31 @@ private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContex
             return await stepContext.NextAsync(result, cancellationToken).ConfigureAwait(false);
         }
 
+        private void ResetOptions(DialogContext dc, QnAMakerDialogOptions dialogOptions)
+        {
+            // Resetting context and QnAId
+            dialogOptions.QnAMakerOptions.QnAId = 0;
+            dialogOptions.QnAMakerOptions.Context = new QnARequestContext();
+
+            // -Check if previous context is present, if yes then put it with the query
+            // -Check for id if query is present in reverse index.
+            var previousContextData = ObjectPath.GetPathValue<Dictionary<string, int>>(dc.ActiveDialog.State, QnAContextData, new Dictionary<string, int>());
+            var previousQnAId = ObjectPath.GetPathValue<int>(dc.ActiveDialog.State, PreviousQnAId, 0);
+
+            if (previousQnAId > 0)
+            {
+                dialogOptions.QnAMakerOptions.Context = new QnARequestContext
+                {
+                    PreviousQnAId = previousQnAId
+                };
+
+                if (previousContextData.TryGetValue(dc.Context.Activity.Text, out var currentQnAId))
+                {
+                    dialogOptions.QnAMakerOptions.QnAId = currentQnAId;
+                }
+            }
+        }
+
         private async Task<DialogTurnResult> CallTrainAsync(WaterfallStepContext stepContext, CancellationToken cancellationToken)
         {
             var dialogOptions = ObjectPath.GetPathValue<QnAMakerDialogOptions>(stepContext.ActiveDialog.State, Options);

libraries/Microsoft.Bot.Builder.Dialogs.Adaptive.Testing/TestActions/UserActivity.cs

@@ -67,7 +67,7 @@ public async override Task ExecuteAsync(TestAdapter adapter, BotCallbackHandler
             Stopwatch sw = new Stopwatch();
             sw.Start();
 
-            await adapter.ProcessActivityAsync(this.Activity, callback, default(CancellationToken)).ConfigureAwait(false);
+            await adapter.ProcessActivityAsync(activity, callback, default(CancellationToken)).ConfigureAwait(false);
 
             sw.Stop();
             Trace.TraceInformation($"[Turn Ended => {sw.ElapsedMilliseconds} ms processing UserActivity: {this.Activity.Text} ]");

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

@@ -48,6 +48,7 @@ public TestAdapter(string channelId, bool sendTraceActivity = false)
                 User = new ChannelAccount("user1", "User1"),
                 Bot = new ChannelAccount("bot", "Bot"),
                 Conversation = new ConversationAccount(false, "convo1", "Conversation1"),
+                Locale = this.Locale,
             };
         }
 
@@ -73,6 +74,7 @@ public TestAdapter(ConversationReference conversation = null, bool sendTraceActi
                     User = new ChannelAccount("user1", "User1"),
                     Bot = new ChannelAccount("bot", "Bot"),
                     Conversation = new ConversationAccount(false, "convo1", "Conversation1"),
+                    Locale = this.Locale,
                 };
             }
         }
@@ -125,6 +127,7 @@ public static ConversationReference CreateConversation(string name, string user
                 Conversation = new ConversationAccount(false, name, name),
                 User = new ChannelAccount(id: user.ToLower(), name: user),
                 Bot = new ChannelAccount(id: bot.ToLower(), name: bot),
+                Locale = "en-us"
             };
         }
 
@@ -427,7 +430,7 @@ public Activity MakeActivity(string text = null)
             Activity activity = new Activity
             {
                 Type = ActivityTypes.Message,
-                Locale = this.Locale,
+                Locale = this.Locale ?? "en-us",
                 From = Conversation.User,
                 Recipient = Conversation.Bot,
                 Conversation = Conversation.Conversation,

libraries/Microsoft.Bot.Schema/ActivityEx.cs

@@ -487,7 +487,7 @@ public Activity ApplyConversationReference(ConversationReference reference, bool
             this.ChannelId = reference.ChannelId;
             this.ServiceUrl = reference.ServiceUrl;
             this.Conversation = reference.Conversation;
-            this.Locale = reference.Locale;
+            this.Locale = reference.Locale ?? this.Locale;
 
             if (isIncoming)
             {

tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/DebugComposer.cs

@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+#pragma warning disable SA1118 // Parameter should not span multiple lines
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Threading.Tasks;
+using Microsoft.Bot.Builder.Dialogs.Adaptive.Testing;
+using Microsoft.Bot.Builder.Dialogs.Declarative.Resources;
+using Microsoft.Extensions.Configuration;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.Bot.Builder.Dialogs.Adaptive.Tests
+{
+    [TestClass]
+    public class DebugComposer
+    {
+        public TestContext TestContext { get; set; }
+
+        // This test can be used to debug a composer bot, simple point botPath => composer bot
+        // and add debug.test.dialog script to that folder
+        // {
+        //  "$schema": "../../../tests.schema",
+        //  "$kind": "Microsoft.Test.Script",
+        //  "dialog": "bot.root.dialog",
+        //  "script": [...] 
+        // }
+        [Ignore]
+        [TestMethod]
+        public async Task DebugComposerBot()
+        {
+            // luis.settings.{environment}.{region}.json
+            var environment = Environment.UserName;
+            var region = "westus";
+            var botPath = Path.Combine(TestUtils.GetProjectPath(), "Tests");
+            var testScript = "debug.test.dialog";
+            var locale = "en-US";
+
+            var resourceExplorer = new ResourceExplorer()
+                .AddFolder(botPath, monitorChanges: false);
+
+            // add luis settings if there are luis assets
+            var resource = resourceExplorer.GetResource($"luis.settings.{environment}.{region}.json") as FileResource;
+            var builder = new ConfigurationBuilder().AddInMemoryCollection();
+            if (resource != null)
+            {
+                builder.AddJsonFile(resource.FullName);
+            }
+
+            var script = resourceExplorer.LoadType<TestScript>(testScript);
+            script.Locale = locale;
+            script.Configuration = builder.Build();
+            await script.ExecuteAsync(resourceExplorer: resourceExplorer).ConfigureAwait(false);
+        }
+    }
+}

tests/Microsoft.Bot.Builder.Dialogs.Adaptive.Tests/Startup.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using Microsoft.Bot.Builder.AI.Luis;
+using Microsoft.Bot.Builder.AI.QnA;
 using Microsoft.Bot.Builder.Dialogs;
 using Microsoft.Bot.Builder.Dialogs.Adaptive;
 using Microsoft.Bot.Builder.Dialogs.Adaptive.Testing;
@@ -33,6 +34,7 @@ public static void Initialize(TestContext testContext)
             ComponentRegistration.Add(new LanguageGenerationComponentRegistration());
             ComponentRegistration.Add(new AdaptiveTestingComponentRegistration());
             ComponentRegistration.Add(new LuisComponentRegistration());
+            ComponentRegistration.Add(new QnAMakerComponentRegistration());
         }
 
         public void ConfigureServices(IServiceCollection services)