Updated Dialog Manager to work with skills (#2343)

* updated dialog manager to support skills

* reordered properties of dialog manager

* make dialog manager's dialogs property public

* added tests for dialog manager

* code style change

* added test to get/set root dialog of dialog manager

* move sendStateSnapshotTrace() to dialog manager

* added test for state trace in dialog manager

* added skill extensions to inject skill client and conversation id factory

* updated test of beginSkill to use extensions

* use static functions instead of extensions

Co-authored-by: Steven Gum <[email protected]>

Author: chon219

libraries/botbuilder-dialogs-adaptive/src/actions/beginSkill.ts

@@ -5,13 +5,11 @@
  * Copyright (c) Microsoft Corporation. All rights reserved.
  * Licensed under the MIT License.
  */
-import { SkillDialog, SkillDialogOptions, DialogContext, DialogTurnResult, DialogManager, BeginSkillDialogOptions } from 'botbuilder-dialogs';
+import { SkillDialog, SkillDialogOptions, DialogContext, DialogTurnResult, BeginSkillDialogOptions } from 'botbuilder-dialogs';
 import { BoolExpression, StringExpression } from 'adaptive-expressions';
+import { Activity, ActivityTypes } from 'botbuilder-core';
 import { TemplateInterface } from '../template';
-import { Activity, ActivityTypes, BotFrameworkClient, SkillConversationIdFactoryBase } from 'botbuilder-core';
-
-const SKILL_CLIENT = Symbol('skillClient');
-const CONVERSATION_ID_FACTORY = Symbol('conversationIdFactory');
+import { skillClientKey, skillConversationIdFactoryKey } from '../skillExtensions';
 
 export class BeginSkill extends SkillDialog {
 
@@ -87,14 +85,14 @@ export class BeginSkill extends SkillDialog {
         // Setup the skill to call
         const botId = this.botId.getValue(dcState);
         const skillHostEndpoint = this.skillHostEndpoint.getValue(dcState);
-        if (botId) { this.dialogOptions.botId = botId }
-        if (skillHostEndpoint) { this.dialogOptions.skillHostEndpoint = skillHostEndpoint }
-        if (this.skillAppId) { this.dialogOptions.skill.id = this.dialogOptions.skill.appId = this.skillAppId.getValue(dcState) }
-        if (this.skillEndpoint) { this.dialogOptions.skill.skillEndpoint = this.skillEndpoint.getValue(dcState) }
-        if (this.connectionName) { this.dialogOptions.connectionName = this.connectionName.getValue(dcState) }
-        if (!this.dialogOptions.conversationState) { this.dialogOptions.conversationState = dc.dialogManager.conversationState }
-        if (!this.dialogOptions.skillClient) { this.dialogOptions.skillClient = dc.context.turnState.get(SKILL_CLIENT) }
-        if (!this.dialogOptions.conversationIdFactory) { this.dialogOptions.conversationIdFactory = dc.context.turnState.get(CONVERSATION_ID_FACTORY) }
+        if (botId) { this.dialogOptions.botId = botId; }
+        if (skillHostEndpoint) { this.dialogOptions.skillHostEndpoint = skillHostEndpoint; }
+        if (this.skillAppId) { this.dialogOptions.skill.id = this.dialogOptions.skill.appId = this.skillAppId.getValue(dcState); }
+        if (this.skillEndpoint) { this.dialogOptions.skill.skillEndpoint = this.skillEndpoint.getValue(dcState); }
+        if (this.connectionName) { this.dialogOptions.connectionName = this.connectionName.getValue(dcState); }
+        if (!this.dialogOptions.conversationState) { this.dialogOptions.conversationState = dc.dialogManager.conversationState; }
+        if (!this.dialogOptions.skillClient) { this.dialogOptions.skillClient = dc.context.turnState.get(skillClientKey); }
+        if (!this.dialogOptions.conversationIdFactory) { this.dialogOptions.conversationIdFactory = dc.context.turnState.get(skillConversationIdFactoryKey); }
 
         // Get the activity to send to the skill.
         options = {} as BeginSkillDialogOptions;
@@ -126,17 +124,6 @@ export class BeginSkill extends SkillDialog {
     protected onComputeId(): string {
         const appId = this.skillAppId ? this.skillAppId.toString() : '';
         const activity = this.activity ? this.activity.toString() : '<activity>';
-        return `Skill[${appId}:${activity}]`;
-    }
-
-    /**
-     * Configures the skill client and conversation ID factory to use.
-     * @param dm DialogManager to configure.
-     * @param skillClient Skill client instance to use.
-     * @param conversationIdFactory Conversation ID factory to use.
-     */
-    static setSkillHostOptions(dm: DialogManager, skillClient: BotFrameworkClient, conversationIdFactory: SkillConversationIdFactoryBase): void {
-        dm.initialTurnState.set(SKILL_CLIENT, skillClient);
-        dm.initialTurnState.set(CONVERSATION_ID_FACTORY, conversationIdFactory);
+        return `Skill[${ appId }:${ activity }]`;
     }
 }

libraries/botbuilder-dialogs-adaptive/src/index.ts

@@ -19,6 +19,7 @@ export * from './input';
 export * from './luis';
 export * from './recognizers';
 export * from './selectors';
+export * from './skillExtensions';
 export * from './templates';
 export * from './adaptiveDialog';
 export * from './languageGenerationMiddleware';

libraries/botbuilder-dialogs-adaptive/src/skillExtensions.ts

@@ -0,0 +1,31 @@
+/**
+ * @module botbuilder-dialogs-adaptive
+ */
+/**
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { DialogManager } from 'botbuilder-dialogs';
+import { BotFrameworkClient, SkillConversationIdFactoryBase } from 'botbuilder-core';
+
+export const skillClientKey = Symbol('SkillClient');
+export const skillConversationIdFactoryKey = Symbol('SkillConversationIdFactory');
+
+export class SkillExtensions {
+    /**
+     * Configures the skill client to use.
+     */
+    public static useSkillClient(dialogManager: DialogManager, skillClient: BotFrameworkClient): DialogManager {
+        dialogManager.initialTurnState.set(skillClientKey, skillClient);
+        return dialogManager;
+    }
+
+    /**
+     * Configures the skill conversation id factory to use.
+     */
+    public static useSkillConverationIdFactory(dialogManager: DialogManager, skillConversationIdFactory: SkillConversationIdFactoryBase): DialogManager {
+        dialogManager.initialTurnState.set(skillConversationIdFactoryKey, skillConversationIdFactory);
+        return dialogManager;
+    }
+}

libraries/botbuilder-dialogs-adaptive/tests/beginSkill.test.js

@@ -12,7 +12,7 @@ const {
 } = require('botbuilder-core');
 const { BoolExpression, StringExpression } = require('adaptive-expressions');
 const { DialogManager, DialogTurnStatus } = require('botbuilder-dialogs');
-const { BeginSkill } = require('../lib')
+const { BeginSkill, SkillExtensions } = require('../lib')
 
 
 class SimpleConversationIdFactory extends SkillConversationIdFactoryBase {
@@ -81,7 +81,8 @@ describe('BeginSkill', function() {
     const conversationState = new ConversationState(new MemoryStorage());
     const dm = new DialogManager();
     dm.conversationState = conversationState;
-    BeginSkill.setSkillHostOptions(dm, skillClient, new SimpleConversationIdFactory());
+    SkillExtensions.useSkillClient(dm, skillClient);
+    SkillExtensions.useSkillConverationIdFactory(dm, new SimpleConversationIdFactory());
 
     // Setup skill dialog
     const dialog = new BeginSkill();

libraries/botbuilder-dialogs/src/dialogHelper.ts

@@ -14,7 +14,7 @@ import { Activity,
     TurnContext,
 } from 'botbuilder-core';
 import { DialogContext, DialogState } from './dialogContext';
-import { Dialog, DialogTurnStatus } from './dialog';
+import { Dialog, DialogTurnStatus, DialogTurnResult } from './dialog';
 import { DialogEvents } from './dialogEvents';
 import { DialogSet } from './dialogSet';
 import { AuthConstants, GovConstants, isSkillClaim } from './prompts/skillsHelpers';
@@ -80,7 +80,7 @@ export async function runDialog(dialog: Dialog, context: TurnContext, accessor:
     }
 
     if (result.status === DialogTurnStatus.complete || result.status === DialogTurnStatus.cancelled) {
-        if (sendEoCToParent(context)) {
+        if (shouldSendEndOfConversationToParent(context, result)) {
             const endMessageText = `Dialog ${ dialog.id } has **completed**. Sending EndOfConversation.`;
             await context.sendTraceActivity(telemetryEventName, result.result, undefined, `${ endMessageText }`);
 
@@ -95,7 +95,12 @@ export async function runDialog(dialog: Dialog, context: TurnContext, accessor:
  * Helper to determine if we should send an EoC to the parent or not.
  * @param context 
  */
-function sendEoCToParent(context: TurnContext): boolean {
+export function shouldSendEndOfConversationToParent(context: TurnContext, turnResult: DialogTurnResult): boolean {
+    if (!(turnResult.status == DialogTurnStatus.complete || turnResult.status == DialogTurnStatus.cancelled)) {
+        // The dialog is still going, don't return EoC.
+        return false;
+    }
+
     const claimIdentity = context.turnState.get(context.adapter.BotIdentityKey);
     // Inspect the cached ClaimsIdentity to determine if the bot was called from another bot.
     if (claimIdentity && isSkillClaim(claimIdentity.claims)) {
@@ -114,7 +119,7 @@ function sendEoCToParent(context: TurnContext): boolean {
 }
 
 // Recursively walk up the DC stack to find the active DC.
-function getActiveDialogContext(dialogContext: DialogContext): DialogContext {
+export function getActiveDialogContext(dialogContext: DialogContext): DialogContext {
     const child = dialogContext.child;
     if (!child) {
         return dialogContext;
@@ -123,7 +128,7 @@ function getActiveDialogContext(dialogContext: DialogContext): DialogContext {
     return getActiveDialogContext(child);
 }
 
-function isFromParentToSkill(context: TurnContext): boolean {
+export function isFromParentToSkill(context: TurnContext): boolean {
     // If a SkillConversationReference exists, it was likely set by the SkillHandler and the bot is acting as a parent.
     if (context.turnState.get(SkillConversationReferenceKey)) {
         return false;