Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SkillDialog to handle all activity types (e.g. set DeliveryMode for Invokes) #2161

Merged
merged 5 commits into from
May 4, 2020
Merged

Update SkillDialog to handle all activity types (e.g. set DeliveryMode for Invokes) #2161

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
19f2f82
refactor SkillsDialog to accept all ActivityTypes
stevengum May 2, 2020
06df4c1
add MULTIPLE_CHOICES (300) to StatusCodes
stevengum May 2, 2020
402d126
rework processActivity() logic around 'expectReplies' and Invoke acti…
stevengum May 2, 2020
6dcc0f2
Merge branch 'master' into stgum/skillDialog2
stevengum May 2, 2020
573823d
Merge branch 'master' into stgum/skillDialog2
gabog May 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion libraries/botbuilder-core/tests/turnContext.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const assert = require('assert');
const { BotAdapter, MessageFactory, TurnContext, ActivityTypes } = require('../');
const { ActivityTypes, BotAdapter, DeliveryModes, MessageFactory, TurnContext } = require('../');

const activityId = `activity ID`;

Expand Down Expand Up @@ -460,4 +460,24 @@ describe(`TurnContext`, function () {
done();
});
});

it('should add to bufferedReplyActivities if TurnContext.activity.deliveryMode === DeliveryModes.ExpectReplies', async () => {
const activity = JSON.parse(JSON.stringify(testMessage));
activity.deliveryMode = DeliveryModes.ExpectReplies;
const context = new TurnContext(new SimpleAdapter(), activity);

const activities = [ MessageFactory.text('test'), MessageFactory.text('second test') ];
const responses = await context.sendActivities(activities);

assert.strictEqual(responses.length, 2);

// For expectReplies all ResourceResponses should have no id.
const ids = responses.filter(response => response.id === undefined);
assert.strictEqual(ids.length, 2);

const replies = context.bufferedReplyActivities;
assert.strictEqual(replies.length, 2);
assert.strictEqual(replies[0].text, 'test');
assert.strictEqual(replies[1].text, 'second test');
})
});
166 changes: 94 additions & 72 deletions libraries/botbuilder-dialogs/src/skillDialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class SkillDialog extends Dialog {
}

public async beginDialog(dc: DialogContext, options?: any): Promise<DialogTurnResult> {
const dialogArgs = SkillDialog.validateBeginDialogArgs(options);
const dialogArgs = this.validateBeginDialogArgs(options);

await dc.context.sendTraceActivity(`${ this.id }.beginDialog()`, undefined, undefined, `Using activity of type: ${ dialogArgs.activity.type }`);

Expand All @@ -70,7 +70,7 @@ export class SkillDialog extends Dialog {
// Apply conversation reference and common properties from incoming activity before sending.
const skillActivity = TurnContext.applyConversationReference(clonedActivity, TurnContext.getConversationReference(dc.context.activity), true) as Activity;

// Store the deliveryMode of the first forwarded activity
// Store delivery mode and connection name in dialog state for later use.
dc.activeDialog.state[this.DeliveryModeStateKey] = dialogArgs.activity.deliveryMode;
dc.activeDialog.state[this.SsoConnectionNameKey] = dialogArgs.connectionName;

Expand All @@ -83,27 +83,28 @@ export class SkillDialog extends Dialog {
}

public async continueDialog(dc: DialogContext): Promise<DialogTurnResult> {
await dc.context.sendTraceActivity(`${ this.id }.continueDialog()`, undefined, undefined, `ActivityType: ${ dc.context.activity.type }`);
if (!this.onValidateActivity(dc.context.activity)) {
return Dialog.EndOfTurn;
}

await dc.context.sendTraceActivity(`${ this.id }.continueDialog()`, undefined, undefined, `ActivityType: ${ dc.context.activity.type }`);

// Handle EndOfConversation from the skill (this will be sent to the this dialog by the SkillHandler if received from the Skill)
if (dc.context.activity.type === ActivityTypes.EndOfConversation) {
await dc.context.sendTraceActivity(`${ this.id }.continueDialog()`, undefined, undefined, `Got ${ ActivityTypes.EndOfConversation }`);
return await dc.endDialog(dc.context.activity.value);
}

// Forward only Message and Event activities to the skill
if (dc.context.activity.type === ActivityTypes.Message || dc.context.activity.type === ActivityTypes.Event) {
// Create deep clone of the original activity to avoid altering it before forwarding it.
const skillActivity = this.cloneActivity(dc.context.activity);
skillActivity.deliveryMode = dc.activeDialog.state[this.DeliveryModeStateKey] as string;
const connectionName = dc.activeDialog.state[this.SsoConnectionNameKey] as string;

// Just forward to the remote skill
const eocActivity = await this.sendToSkill(dc.context, skillActivity, connectionName);
if (eocActivity) {
return await dc.endDialog(eocActivity.value);
}
// Create deep clone of the original activity to avoid altering it before forwarding it.
const skillActivity: Activity = this.cloneActivity(dc.context.activity);

skillActivity.deliveryMode = dc.activeDialog.state[this.DeliveryModeStateKey] as string;
const connectionName = dc.activeDialog.state[this.SsoConnectionNameKey] as string;

// Just forward to the remote skill
const eocActivity = await this.sendToSkill(dc.context, skillActivity, connectionName);
if (eocActivity) {
return await dc.endDialog(eocActivity.value);
}

return Dialog.EndOfTurn;
Expand Down Expand Up @@ -144,14 +145,28 @@ export class SkillDialog extends Dialog {
}

/**
* @protected
* Validates the activity sent during continueDialog.
* @remarks
* Override this method to implement a custom validator for the activity being sent during the continueDialog.
* This method can be used to ignore activities of a certain type if needed.
* If this method returns false, the dialog will end the turn without processing the activity.
* @param activity The Activity for the current turn of conversation.
*/
protected onValidateActivity(activity: Activity): boolean {
return true;
}

/**
* @private
* Clones the Activity entity.
* @param activity Activity to clone.
*/
private cloneActivity(activity: Partial<Activity>): Activity {
return Object.assign({} as Activity, activity);
return JSON.parse(JSON.stringify(activity));
}

private static validateBeginDialogArgs(options: any): BeginSkillDialogOptions {
private validateBeginDialogArgs(options: any): BeginSkillDialogOptions {
if (!options) {
throw new TypeError('Missing options parameter');
}
Expand All @@ -162,35 +177,18 @@ export class SkillDialog extends Dialog {
throw new TypeError(`"activity" is undefined or null in options.`);
}

// Only accept Message or Event activities
if (dialogArgs.activity.type !== ActivityTypes.Message && dialogArgs.activity.type !== ActivityTypes.Event) {
// Just forward to the remote skill
throw new TypeError(`Only "${ ActivityTypes.Message }" and "${ ActivityTypes.Event }" activities are supported. Received activity of type "${ dialogArgs.activity.type }" in options.`);
}

return dialogArgs;
}

private async sendToSkill(context: TurnContext, activity: Activity, connectionName: string): Promise<Activity> {
// Create a conversationId to interact with the skill and send the activity
const conversationIdFactoryOptions: SkillConversationIdFactoryOptions = {
fromBotOAuthScope: context.turnState.get(context.adapter.OAuthScopeKey),
fromBotId: this.dialogOptions.botId,
activity: activity,
botFrameworkSkill: this.dialogOptions.skill
};

// Create a conversationId to interact with the skill and send the activity
let skillConversationId: string;
try {
skillConversationId = await this.dialogOptions.conversationIdFactory.createSkillConversationIdWithOptions(conversationIdFactoryOptions);
} catch (err) {
if (err.message !== 'Not Implemented') throw err;
// If the SkillConversationIdFactoryBase implementation doesn't support createSkillConversationIdWithOptions(),
// use createSkillConversationId() instead.
skillConversationId = await this.dialogOptions.conversationIdFactory.createSkillConversationId(TurnContext.getConversationReference(activity) as ConversationReference);
if (activity.type === ActivityTypes.Invoke) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having to switch expectReplies on for an invoke seems a little odd.

Copy link
Contributor

@gabog gabog May 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the dialog is interacting with a skill that only works with invokes, this makes sure that EoC replies are buffered with the invokeResponse and the dialog ends when an EoC comes back.

Here is an example on how this would look on a skill for teams:

protected override async Task<TaskModuleResponse> OnTeamsTaskModuleSubmitAsync(ITurnContext<IInvokeActivity> turnContext, TaskModuleRequest taskModuleRequest, CancellationToken cancellationToken)
{
    var reply = MessageFactory.Text("OnTeamsTaskModuleSubmitAsync Value: " + JsonConvert.SerializeObject(taskModuleRequest));
    await turnContext.SendActivityAsync(reply, cancellationToken);
            
    // Send End of conversation at the end.
    var activity = new Activity(ActivityTypes.EndOfConversation) { Value = taskModuleRequest.Data, Locale = ((Activity)turnContext.Activity).Locale };
    await turnContext.SendActivityAsync(activity, cancellationToken);

    return new TaskModuleResponse
    {
        Task = new TaskModuleMessageResponse
        {
            Value = "Thanks!",
        },
    };
}

// Force ExpectReplies for invoke activities so we can get the replies right away and send them back to the channel if needed.
// This makes sure that the dialog will receive the Invoke response from the skill and any other activities sent, including EoC.
activity.deliveryMode = DeliveryModes.ExpectReplies;
}

const skillConversationId = await this.createSkillConversationId(context, activity);

// Always save state before forwarding
// (the dialog stack won't get updated with the skillDialog and things won't work if you don't)
const skillInfo = this.dialogOptions.skill;
Expand All @@ -199,53 +197,50 @@ export class SkillDialog extends Dialog {
const response = await this.dialogOptions.skillClient.postActivity<ExpectedReplies>(this.dialogOptions.botId, skillInfo.appId, skillInfo.skillEndpoint, this.dialogOptions.skillHostEndpoint, skillConversationId, activity);

// Inspect the skill response status
if (!(response.status >= 200 && response.status <= 299)) {
if (!isSuccessStatusCode(response.status)) {
throw new Error(`Error invoking the skill id: "${ skillInfo.id }" at "${ skillInfo.skillEndpoint }" (status is ${ response.status }). \r\n ${ response.body }`);
}

let eocActivity: Activity;
if (activity.deliveryMode == DeliveryModes.ExpectReplies && response.body && response.body.activities) {
// Process replies in the response.Body.
if (Array.isArray(response.body.activities)) {
response.body.activities.forEach(async (fromSkillActivity: Activity): Promise<void> => {
if (fromSkillActivity.type === ActivityTypes.EndOfConversation) {
// Capture the EndOfConversation activity if it was sent from skill
eocActivity = fromSkillActivity;
} else if (await this.interceptOAuthCards(context, fromSkillActivity, connectionName)) {
// Do nothing. The token exchange succeeded, so no OAuthCard needs to be shown to the user.
} else {
await context.sendActivity(fromSkillActivity);
}
});
}
const activities = typeof response.body !== 'undefined' && (response.body as ExpectedReplies).activities;
if (activity.deliveryMode === DeliveryModes.ExpectReplies && Array.isArray(activities)) {
// Process replies in the response.body.
for (const activityFromSkill of activities) {
if (activityFromSkill.type === ActivityTypes.EndOfConversation) {
// Capture the EndOfConversation activity if it was sent from skill
eocActivity = activityFromSkill;
} else if (await this.interceptOAuthCards(context, activityFromSkill, connectionName)) {
// Do nothing. The token exchange succeeded, so no OAuthCard needs to be shown to the user.
} else {
await context.sendActivity(activityFromSkill);
}
};
}

return eocActivity;
}

/**
* Intercept any Skill-sent OAuthCards for SSO.
* Tells us if we should intercept the OAuthCard message.
* @remarks
* This method will attempt to exchange tokens for the Skill.
*
* Returns true for a successful token exchange and false to indicate the activity should be
* forwarded to its recipient.
* The SkillDialog only attempts to intercept OAuthCards when the following criteria are met:
* 1. An OAuthCard was sent from the skill
* 2. The SkillDialog was called with a connectionName
* 3. The current adapter supports token exchange
* If any of these criteria are false, return false.
* @private
*/
private async interceptOAuthCards(context: TurnContext, activity: Activity, connectionName: string): Promise<boolean> {
const attachments = activity.attachments || [];
const oAuthCardAttachment: Attachment = attachments.find((c) => c.contentType === CardFactory.contentTypes.oauthCard);

// The SkillDialog only attempts to intercept OAuthCards when the following criteria are met:
// 1. An OAuthCard was sent from the skill
// 2. The SkillDialog was called with a connectionName
// 3. The current adapter supports token exchange
// If any of these criteria are false, return false.
if (oAuthCardAttachment && connectionName && 'exchangeToken' in context.adapter) {
if (!connectionName || !('exchangeToken' in context.adapter)) {
// The adapter may choose not to support token exchange, in which case we fallback to showing skill's OAuthCard to the user.
return false;
}

const oAuthCardAttachment: Attachment = (activity.attachments || []).find((c) => c.contentType === CardFactory.contentTypes.oauthCard);
if (oAuthCardAttachment) {
const tokenExchangeProvider: ExtendedUserTokenProvider = context.adapter as ExtendedUserTokenProvider;
const oAuthCard: OAuthCard = oAuthCardAttachment.content;

// The value for uri is either tokenExchangeResource.uri or undefined.
const uri = oAuthCard && oAuthCard.tokenExchangeResource && oAuthCard.tokenExchangeResource.uri;
if (uri) {
try {
Expand All @@ -257,11 +252,12 @@ export class SkillDialog extends Dialog {

if (result && result.token) {
// If token above is null or undefined, then SSO has failed and we return false.
// Send an invoke back to the skill
// If not, send an invoke to the skill with the token.
return await this.sendTokenExchangeInvokeToSkill(activity, oAuthCard.tokenExchangeResource.id, oAuthCard.connectionName, result.token);
}
} catch (err) {
// Failures in token exchange are not fatal. They simply mean that the user needs to be shown the skill's OAuthCard.
return false;
}
}
}
Expand All @@ -280,6 +276,32 @@ export class SkillDialog extends Dialog {
const response = await this.dialogOptions.skillClient.postActivity<ExpectedReplies>(this.dialogOptions.botId, skillInfo.appId, skillInfo.skillEndpoint, this.dialogOptions.skillHostEndpoint, incomingActivity.conversation.id, activity);

// Check response status: true if success, false if failure
return response.status === StatusCodes.OK;
return isSuccessStatusCode(response.status);
}

private async createSkillConversationId(context: TurnContext, activity: Activity) {
// Create a conversationId to interact with the skill and send the activity
const conversationIdFactoryOptions: SkillConversationIdFactoryOptions = {
fromBotOAuthScope: context.turnState.get(context.adapter.OAuthScopeKey),
fromBotId: this.dialogOptions.botId,
activity: activity,
botFrameworkSkill: this.dialogOptions.skill
};

// Create a conversationId to interact with the skill and send the activity
let skillConversationId: string;
try {
skillConversationId = await this.dialogOptions.conversationIdFactory.createSkillConversationIdWithOptions(conversationIdFactoryOptions);
} catch (err) {
if (err.message !== 'Not Implemented') throw err;
// If the SkillConversationIdFactoryBase implementation doesn't support createSkillConversationIdWithOptions(),
// use createSkillConversationId() instead.
skillConversationId = await this.dialogOptions.conversationIdFactory.createSkillConversationId(TurnContext.getConversationReference(activity) as ConversationReference);
}
return skillConversationId;
}
}

function isSuccessStatusCode(status: number): boolean {
return status >= StatusCodes.OK && status < StatusCodes.MULTIPLE_CHOICES;
}
Loading