Skip to content

Commit

Permalink
port: several parity issues (#3540)
Browse files Browse the repository at this point in the history
* port: pass through dialog state manager config

Fixes #3538

* port: turn memory scope exclude from snapshot

Fixes #3539

* port: update default property path for oauth input

Fixes #3536

* port: update http request default result property

Fixes #3537
  • Loading branch information
joshgummersall authored and Josh Gummersall committed Apr 7, 2021
1 parent f6cd3b0 commit 77992ee
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"$ref": "schema:#/definitions/stringExpression",
"title": "Result property",
"description": "A property to store the result of this action. The result can include any of the 4 properties from the HTTP response: statusCode, reasonPhrase, content, and headers. If the content is JSON it will be a deserialized object. The values can be accessed via .content for example.",
"default": "turn.results",
"examples": [
"dialog.contosodata"
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema",
"$role": "implements(Microsoft.IDialog)",
"title": "OAuthInput Dialog",
"description": "Collect login information.",
"description": "Collect login information before each request.",
"type": "object",
"properties": {
"connectionName": {
Expand Down Expand Up @@ -48,7 +48,8 @@
"property": {
"$ref": "schema:#/definitions/stringExpression",
"title": "Token property",
"description": "Property to store the OAuth token result.",
"description": "Property to store the OAuth token result. WARNING: Changing this location is not recommended as you should call OAuthInput immediately before each use of the token.",
"default": "turn.token",
"examples": [
"dialog.token"
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export interface HttpRequestConfiguration extends DialogConfiguration {
headers?: HeadersInput | HeadersOutput;
body?: unknown | ValueExpression;
responseType?: ResponsesTypes | string | Expression | EnumExpression<ResponsesTypes>;
resultProperty?: string | Expression | StringExpression;
resultProperty: string | Expression | StringExpression;
disabled?: boolean | string | Expression | BoolExpression;
}

Expand Down Expand Up @@ -222,7 +222,7 @@ export class HttpRequest<O extends object = {}> extends Dialog<O> implements Htt
/**
* Gets or sets the property expression to store the HTTP response in.
*/
public resultProperty?: StringExpression;
public resultProperty: StringExpression = new StringExpression('turn.results');

/**
* An optional expression which if is true will disable this action.
Expand Down
19 changes: 10 additions & 9 deletions libraries/botbuilder-dialogs/src/dialogHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
* Licensed under the MIT License.
*/

import { AuthenticationConstants, ClaimsIdentity, GovernmentConstants, SkillValidation } from 'botframework-connector';
import { Dialog, DialogTurnStatus, DialogTurnResult } from './dialog';
import { DialogContext, DialogState } from './dialogContext';
import { DialogEvents } from './dialogEvents';
import { DialogSet } from './dialogSet';
import { DialogStateManager, DialogStateManagerConfiguration } from './memory';

import {
Activity,
ActivityEx,
Expand All @@ -18,13 +25,6 @@ import {
StatePropertyAccessor,
TurnContext,
} from 'botbuilder-core';
import { AuthenticationConstants, ClaimsIdentity, GovernmentConstants, SkillValidation } from 'botframework-connector';

import { DialogContext, DialogState } from './dialogContext';
import { Dialog, DialogTurnStatus, DialogTurnResult } from './dialog';
import { DialogEvents } from './dialogEvents';
import { DialogSet } from './dialogSet';
import { DialogStateManager } from './memory';

/**
* Runs a dialog from a given context and accessor.
Expand Down Expand Up @@ -67,14 +67,15 @@ export async function runDialog(
export async function internalRun(
context: TurnContext,
dialogId: string,
dialogContext: DialogContext
dialogContext: DialogContext,
dialogStateManagerConfiguration?: DialogStateManagerConfiguration
): Promise<DialogTurnResult> {
// map TurnState into root dialog context.services
context.turnState.forEach((service, key) => {
dialogContext.services.push(key, service);
});

const dialogStateManager = new DialogStateManager(dialogContext);
const dialogStateManager = new DialogStateManager(dialogContext, dialogStateManagerConfiguration);

await dialogStateManager.loadAllScopes();
dialogContext.context.turnState.push('DialogStateManager', dialogStateManager);
Expand Down
2 changes: 1 addition & 1 deletion libraries/botbuilder-dialogs/src/dialogManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ export class DialogManager extends Configurable {
const dc = new DialogContext(this.dialogs, context, dialogState);

// Call the common dialog "continue/begin" execution pattern shared with the classic RunAsync extension method
const turnResult = await internalRun(context, this._rootDialogId, dc);
const turnResult = await internalRun(context, this._rootDialogId, dc, this.stateConfiguration);

// Save BotState changes
await botStateSet.saveAllChanges(dc.context, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class TurnMemoryScope extends MemoryScope {
* Initializes a new instance of the [TurnMemoryScope](xref:botbuilder-dialogs.TurnMemoryScope) class.
*/
public constructor() {
super(ScopePath.turn);
super(ScopePath.turn, false);
}

/**
Expand Down
107 changes: 76 additions & 31 deletions libraries/botbuilder-dialogs/tests/dialogManager.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { ok, strictEqual } = require('assert');
const { AdaptiveDialog, OnBeginDialog, EndTurn, SendActivity } = require('../../botbuilder-dialogs-adaptive/lib');
const { AuthenticationConstants } = require('botframework-connector');
const { deepStrictEqual, ok, strictEqual } = require('assert');

const {
ActivityTypes,
AutoSaveStateMiddleware,
Expand All @@ -9,20 +12,22 @@ const {
MessageFactory,
SkillConversationReferenceKey,
TestAdapter,
TurnContext,
UserState,
useBotState,
} = require('botbuilder-core');

const {
ComponentDialog,
DialogEvents,
DialogManager,
DialogReason,
DialogTurnStateConstants,
DialogTurnStatus,
MemoryScope,
TextPrompt,
WaterfallDialog,
DialogManager,
DialogTurnStatus,
DialogEvents,
} = require('../');
const { AuthenticationConstants } = require('botframework-connector');
const { AdaptiveDialog, OnBeginDialog, EndTurn, SendActivity } = require('../../botbuilder-dialogs-adaptive/lib');

const FlowTestCase = {
RootBotOnly: 'RootBotOnly',
Expand Down Expand Up @@ -105,28 +110,24 @@ function createTestFlow(dialog, testCase = FlowTestCase.RootBotOnly, enabledTrac
}

// Interceptor to capture the EoC activity if it was sent so we can assert it in the tests.
context.onSendActivities(async (tc, activities, next) => {
for (let idx = 0; idx < activities.length; idx++) {
if (activities[idx].type === ActivityTypes.EndOfConversation) {
_eocSent = activities[idx];
break;
}
}
return await next();
context.onSendActivities((_turnContext, activities, next) => {
_eocSent = activities.find((activity) => activity.type === ActivityTypes.EndOfConversation);
return next();
});

_dmTurnResult = await dm.onTurn(context);
},
convRef,
enabledTrace
);

adapter.use(new AutoSaveStateMiddleware(userState, convoState));

return adapter;
}

class SimpleComponentDialog extends ComponentDialog {
constructor(id, property) {
constructor(id, _property) {
super(id || 'SimpleComponentDialog');
this.TextPrompt = 'TextPrompt';
this.WaterfallDialog = 'WaterfallDialog';
Expand Down Expand Up @@ -159,12 +160,12 @@ class SimpleComponentDialog extends ComponentDialog {
describe('DialogManager', function () {
this.timeout(300);

this.beforeEach(() => {
this.beforeEach(function () {
_dmTurnResult = undefined;
});

describe('HandlesBotAndSkillsTestCases', () => {
this.beforeEach(() => {
describe('HandlesBotAndSkillsTestCases', function () {
this.beforeEach(function () {
_eocSent = undefined;
_dmTurnResult = undefined;
});
Expand All @@ -191,24 +192,24 @@ describe('DialogManager', function () {
}
}

it('rootBotOnly, no sent EoC', async () => {
it('rootBotOnly, no sent EoC', async function () {
await handlesBotAndSkillsTestCases(FlowTestCase.RootBotOnly, false);
});

it('rootBotConsumingSkill, no sent EoC', async () => {
it('rootBotConsumingSkill, no sent EoC', async function () {
await handlesBotAndSkillsTestCases(FlowTestCase.RootBotConsumingSkill, false);
});

it('middleSkill, sent EoC', async () => {
it('middleSkill, sent EoC', async function () {
await handlesBotAndSkillsTestCases(FlowTestCase.MiddleSkill, true);
});

it('leafSkill, sent EoC', async () => {
it('leafSkill, sent EoC', async function () {
await handlesBotAndSkillsTestCases(FlowTestCase.LeafSkill, true);
});
});

it('SkillHandlesEoCFromParent', async () => {
it('SkillHandlesEoCFromParent', async function () {
const dialog = new SimpleComponentDialog();
const testFlow = createTestFlow(dialog, FlowTestCase.LeafSkill);
await testFlow
Expand All @@ -219,7 +220,7 @@ describe('DialogManager', function () {
strictEqual(_dmTurnResult.turnResult.status, DialogTurnStatus.cancelled);
});

it('SkillHandlesRepromptFromParent', async () => {
it('SkillHandlesRepromptFromParent', async function () {
const dialog = new SimpleComponentDialog();
const testFlow = createTestFlow(dialog, FlowTestCase.LeafSkill);
await testFlow
Expand All @@ -231,14 +232,14 @@ describe('DialogManager', function () {
strictEqual(_dmTurnResult.turnResult.status, DialogTurnStatus.waiting);
});

it('SkillShouldReturnEmptyOnRepromptWithNoDialog', async () => {
it('SkillShouldReturnEmptyOnRepromptWithNoDialog', async function () {
const dialog = new SimpleComponentDialog();
const testFlow = createTestFlow(dialog, FlowTestCase.LeafSkill);
await testFlow.send({ type: ActivityTypes.Event, name: DialogEvents.repromptDialog }).startTest();
strictEqual(_dmTurnResult.turnResult.status, DialogTurnStatus.empty);
});

it('Trace skill state', async () => {
it('Trace skill state', async function () {
const dialog = new SimpleComponentDialog();
const testFlow = createTestFlow(dialog, FlowTestCase.LeafSkill, true);
await testFlow
Expand All @@ -258,7 +259,7 @@ describe('DialogManager', function () {
strictEqual(_dmTurnResult.turnResult.status, DialogTurnStatus.complete);
});

it('Trace bot state', async () => {
it('Trace bot state', async function () {
const dialog = new SimpleComponentDialog();
const testFlow = createTestFlow(dialog, FlowTestCase.RootBotOnly, true);
await testFlow
Expand All @@ -278,7 +279,7 @@ describe('DialogManager', function () {
strictEqual(_dmTurnResult.turnResult.status, DialogTurnStatus.complete);
});

it('Gets or sets root dialog', () => {
it('Gets or sets root dialog', function () {
const dm = new DialogManager();
const rootDialog = new SimpleComponentDialog();
dm.rootDialog = rootDialog;
Expand All @@ -288,7 +289,7 @@ describe('DialogManager', function () {
strictEqual(dm.rootDialog, undefined);
});

it('Container registration', async () => {
it('Container registration', async function () {
const root = new AdaptiveDialog('root').configure({
triggers: [
new OnBeginDialog().configure({
Expand All @@ -310,7 +311,7 @@ describe('DialogManager', function () {
ok(dm.dialogs.find('inner'));
});

it('Cyclical dialog structures', async () => {
it('Cyclical dialog structures', async function () {
const trigger = new OnBeginDialog();

const root = new AdaptiveDialog('root').configure({
Expand All @@ -335,7 +336,7 @@ describe('DialogManager', function () {
await adapter.send('hi').startTest();
});

it('Container registration double nesting', async () => {
it('Container registration double nesting', async function () {
const root = new AdaptiveDialog('root').configure({
triggers: [
new OnBeginDialog().configure({
Expand Down Expand Up @@ -380,4 +381,48 @@ describe('DialogManager', function () {
undefined
);
});

it('State Configuration', async function () {
class CustomMemoryScope extends MemoryScope {
constructor() {
super('custom');
}
}

class CustomPathResolver {
transformPath() {
throw new Error('not implemented');
}
}

const dialog = new WaterfallDialog('test-dialog');

const memoryScope = new CustomMemoryScope();
const pathResolver = new CustomPathResolver();

const dialogManager = new DialogManager(dialog);
dialogManager.stateConfiguration = {
memoryScopes: [memoryScope],
pathResolvers: [pathResolver],
};

const adapter = new TestAdapter(async () => {});

const turnContext = new TurnContext(adapter, {
channelId: 'test-channel',
conversation: {
id: 'test-conversation-id',
},
from: {
id: 'test-id',
},
});

turnContext.turnState.set('ConversationState', new ConversationState(new MemoryStorage()));
await dialogManager.onTurn(turnContext);
const actual = turnContext.turnState.get(DialogTurnStateConstants.dialogManager);

deepStrictEqual(actual.stateConfiguration.memoryScopes, [memoryScope]);
deepStrictEqual(actual.stateConfiguration.pathResolvers, [pathResolver]);
});
});
6 changes: 4 additions & 2 deletions libraries/tests.schema
Original file line number Diff line number Diff line change
Expand Up @@ -3835,6 +3835,7 @@
"$ref": "#/definitions/stringExpression",
"title": "Result property",
"description": "A property to store the result of this action. The result can include any of the 4 properties from the HTTP response: statusCode, reasonPhrase, content, and headers. If the content is JSON it will be a deserialized object. The values can be accessed via .content for example.",
"default": "turn.results",
"examples": [
"dialog.contosodata"
]
Expand Down Expand Up @@ -5370,7 +5371,7 @@
"Microsoft.OAuthInput": {
"$role": "implements(Microsoft.IDialog)",
"title": "OAuthInput Dialog",
"description": "Collect login information.",
"description": "Collect login information before each request.",
"type": "object",
"required": [
"connectionName",
Expand Down Expand Up @@ -5432,7 +5433,8 @@
"property": {
"$ref": "#/definitions/stringExpression",
"title": "Token property",
"description": "Property to store the OAuth token result.",
"description": "Property to store the OAuth token result. WARNING: Changing this location is not recommended as you should call OAuthInput immediately before each use of the token.",
"default": "turn.token",
"examples": [
"dialog.token"
]
Expand Down

0 comments on commit 77992ee

Please sign in to comment.