Fix bot's state skipped properties in cache hash

microsoft · Jan 15, 2024 · ed5c3a7 · ed5c3a7
1 parent 65f92e1
commit ed5c3a7
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 10 deletions.
diff --git a/libraries/botbuilder-core/src/botState.ts b/libraries/botbuilder-core/src/botState.ts
@@ -206,6 +206,7 @@ export class BotState implements PropertyManager {
     /**
      * Skips properties from the cached state object.
      *
+     * @remarks Primarily used to skip properties before calculating the hash value in the calculateChangeHash function.
      * @param state Dictionary of state values.
      * @returns Dictionary of state values, without the skipped properties.
      */
@@ -221,7 +222,7 @@ export class BotState implements PropertyManager {
         };
 
         const inner = ([key, value], skip = []) => {
-            if (skip.includes(key)) {
+            if (value === null || value === undefined || skip.includes(key)) {
                 return;
             }
 
@@ -230,14 +231,14 @@ export class BotState implements PropertyManager {
             }
 
             if (typeof value !== 'object') {
-                return value;
+                return value.valueOf();
             }
 
             return Object.entries(value).reduce((acc, [k, v]) => {
                 const skipResult = skipHandler(k) ?? [];
                 acc[k] = inner([k, v], [...skip, ...skipResult]);
                 return acc;
-            }, value);
+            }, {});
         };
 
         return inner([null, state]);

diff --git a/libraries/botbuilder-dialogs-adaptive-testing/tests/action.test.js b/libraries/botbuilder-dialogs-adaptive-testing/tests/action.test.js
@@ -201,8 +201,8 @@ describe('ActionTests', function () {
         const [, { state: beginSkillState }] = actionScope.dialogStack;
         const options = beginSkillState['BeginSkill.dialogOptionsData'];
 
-        assert.equal(options.conversationIdFactory, null);
-        assert.equal(options.conversationState, null);
+        assert.notEqual(options.conversationIdFactory, null);
+        assert.notEqual(options.conversationState, null);
         assert.notEqual(beginSkillDialog.dialogOptions.conversationIdFactory, null);
         assert.notEqual(beginSkillDialog.dialogOptions.conversationState, null);
     });

diff --git a/libraries/botbuilder-dialogs-adaptive/src/actions/beginSkill.ts b/libraries/botbuilder-dialogs-adaptive/src/actions/beginSkill.ts
@@ -201,6 +201,7 @@ export class BeginSkill extends SkillDialog implements BeginSkillConfiguration {
 
         // Store the initialized dialogOptions in state so we can restore these values when the dialog is resumed.
         dc.activeDialog.state[this._dialogOptionsStateKey] = this.dialogOptions;
+        // Skip properties from the bot's state cache hash due to unwanted conversationState behavior.
         const skipProperties = dc.context.turnState.get(CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY);
         const props: (keyof SkillDialogOptions)[] = ['conversationIdFactory', 'conversationState', 'skillClient'];
         skipProperties(this._dialogOptionsStateKey, props);

diff --git a/libraries/botbuilder-dialogs/src/skillDialog.ts b/libraries/botbuilder-dialogs/src/skillDialog.ts
@@ -265,12 +265,9 @@ export class SkillDialog extends Dialog<Partial<BeginSkillDialogOptions>> {
         // after saveChanges, this.dialogOptions is missing some required values.
         // this work`around allows this method to work, but 'interceptOAuthCards' (below)
         // could encounter problems (not currently used by UHG).
-        const skillClient = this.dialogOptions.skillClient;
-        const conversationIdFactory = this.dialogOptions.conversationIdFactory;
-
         await this.dialogOptions.conversationState.saveChanges(context, true);
 
-        const response = await skillClient.postActivity<ExpectedReplies>(
+        const response = await this.dialogOptions.skillClient.postActivity<ExpectedReplies>(
             this.dialogOptions.botId,
             skillInfo.appId,
             skillInfo.skillEndpoint,
@@ -300,7 +297,7 @@ export class SkillDialog extends Dialog<Partial<BeginSkillDialogOptions>> {
                     console.log(
                         `SkillHandlerImpl.sendToSkill, deleteConversationReference skillConversationId=${skillConversationId}`
                     );
-                    await conversationIdFactory.deleteConversationReference(skillConversationId);
+                    await this.dialogOptions.conversationIdFactory.deleteConversationReference(skillConversationId);
                 } else if (
                     !sentInvokeResponses &&
                     (await this.interceptOAuthCards(context, activityFromSkill, this.dialogOptions.connectionName))