fix: [#4325] Skip Storage properties from BotState class (#4326)

* Add skip storage properties functionality to BotState

* Add unit tests for BotState skipProperties

* Fix test:compat

* test for linux

libraries/botbuilder-core/etc/botbuilder-core.api.md

@@ -268,6 +268,9 @@ export class BrowserSessionStorage extends MemoryStorage {
     constructor();
 }
 
+// @public (undocumented)
+export const CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY: unique symbol;
+
 // @public
 export interface CachedBotState {
     hash: string;

libraries/botbuilder-core/src/botState.ts

@@ -25,6 +25,8 @@ export interface CachedBotState {
     hash: string;
 }
 
+export const CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY = Symbol('cachedBotStateSkipPropertiesHandler');
+
 /**
  * Base class for the frameworks state persistance scopes.
  *
@@ -38,6 +40,7 @@ export interface CachedBotState {
  */
 export class BotState implements PropertyManager {
     private stateKey = Symbol('state');
+    private skippedProperties = new Map();
 
     /**
      * Creates a new BotState instance.
@@ -78,6 +81,11 @@ export class BotState implements PropertyManager {
      */
     load(context: TurnContext, force = false): Promise<any> {
         const cached: CachedBotState = context.turnState.get(this.stateKey);
+        if (!context.turnState.get(CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY)) {
+            context.turnState.set(CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY, (key, properties) =>
+                this.skippedProperties.set(key, properties)
+            );
+        }
         if (force || !cached || !cached.state) {
             return Promise.resolve(this.storageKey(context)).then((key: string) => {
                 return this.storage.read([key]).then((items: StoreItems) => {
@@ -110,7 +118,8 @@ export class BotState implements PropertyManager {
      */
     saveChanges(context: TurnContext, force = false): Promise<void> {
         let cached: CachedBotState = context.turnState.get(this.stateKey);
-        if (force || (cached && cached.hash !== calculateChangeHash(cached.state))) {
+        const state = this.skipProperties(cached?.state);
+        if (force || (cached && cached.hash !== calculateChangeHash(state))) {
             return Promise.resolve(this.storageKey(context)).then((key: string) => {
                 if (!cached) {
                     cached = { state: {}, hash: '' };
@@ -121,7 +130,7 @@ export class BotState implements PropertyManager {
 
                 return this.storage.write(changes).then(() => {
                     // Update change hash and cache
-                    cached.hash = calculateChangeHash(cached.state);
+                    cached.hash = calculateChangeHash(state);
                     context.turnState.set(this.stateKey, cached);
                 });
             });
@@ -189,4 +198,44 @@ export class BotState implements PropertyManager {
 
         return typeof cached === 'object' && typeof cached.state === 'object' ? cached.state : undefined;
     }
+
+    /**
+     * Skips properties from the cached state object.
+     *
+     * @param state Dictionary of state values.
+     * @returns Dictionary of state values, without the skipped properties.
+     */
+    private skipProperties(state: CachedBotState['state']): CachedBotState['state'] {
+        if (!state || !this.skippedProperties.size) {
+            return state;
+        }
+
+        const skipHandler = (key: string) => {
+            if (this.skippedProperties.has(key)) {
+                return this.skippedProperties.get(key) ?? [key];
+            }
+        };
+
+        const inner = ([key, value], skip = []) => {
+            if (skip.includes(key)) {
+                return;
+            }
+
+            if (Array.isArray(value)) {
+                return value.map((e) => inner([null, e], skip));
+            }
+
+            if (typeof value !== 'object') {
+                return value;
+            }
+
+            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]);
+    }
 }

libraries/botbuilder-core/tests/botState.test.js

@@ -1,9 +1,52 @@
 const assert = require('assert');
-const { TurnContext, BotState, MemoryStorage, TestAdapter } = require('../');
+const {
+    TurnContext,
+    BotState,
+    MemoryStorage,
+    TestAdapter,
+    CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY,
+} = require('../');
 
 const receivedMessage = { text: 'received', type: 'message' };
 const storageKey = 'stateKey';
 
+const houseSectionsSample = {
+    house: {
+        kitchen: {
+            refrigerator: {
+                fridge: 1,
+                freezer: 1,
+            },
+            chair: 6,
+            table: 1,
+        },
+        bathroom: {
+            toilet: 1,
+            shower: {
+                showerhead: 1,
+                bathtub: 1,
+                shampoo: 2,
+                towel: 3,
+            },
+        },
+        bedroom: {
+            chair: 1,
+            bed: {
+                pillow: 3,
+                sheet: 1,
+                duvet: 1,
+            },
+            closet: {
+                hanger: {
+                    shirt: 6,
+                },
+                shoes: 4,
+                pants: 5,
+            },
+        },
+    },
+};
+
 function cachedState(context, stateKey) {
     const cached = context.turnState.get(stateKey);
     return cached ? cached.state : undefined;
@@ -106,4 +149,35 @@ describe('BotState', function () {
         const count = botState.createProperty('count', 1);
         assert(count !== undefined, 'did not successfully create PropertyAccessor.');
     });
+
+    it('should skip properties in saveChanges()', async function () {
+        // Setup storage base changes.
+        const clone = JSON.parse(JSON.stringify(houseSectionsSample));
+        delete clone.house.kitchen.refrigerator;
+        delete clone.house.kitchen.table;
+        delete clone.house.bedroom.closet.pants;
+        delete clone.house.kitchen.chair;
+        delete clone.house.bedroom.chair;
+        await storage.write({ [storageKey]: clone });
+        await botState.load(context, true);
+
+        // Update bot state.
+        const oldState = context.turnState.get(botState.stateKey);
+        const newState = { ...oldState, state: houseSectionsSample };
+        context.turnState.set(botState.stateKey, newState);
+
+        // Save changes into storage.
+        const skipProperties = context.turnState.get(CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY);
+        skipProperties('house', ['refrigerator', 'table', 'pants']); // Multiple props.
+        skipProperties('chair'); // Single prop (key used as prop).
+        await botState.saveChanges(context);
+        const updatedState = context.turnState.get(botState.stateKey);
+        const storageState = await storage.read([storageKey]);
+
+        // Hash state and storage info shouldn't have changed.
+        const expectedStorage = storageState[storageKey];
+        delete expectedStorage.eTag;
+        assert.equal(oldState.hash, updatedState.hash);
+        assert.deepStrictEqual(clone, expectedStorage);
+    });
 });

libraries/botbuilder-dialogs-adaptive-testing/tests/action.test.js

@@ -1,5 +1,16 @@
+const assert = require('assert');
 const nock = require('nock');
-const { ActivityTypes, MessageFactory, SkillConversationIdFactoryBase, TurnContext } = require('botbuilder-core');
+const {
+    ActivityTypes,
+    MessageFactory,
+    SkillConversationIdFactoryBase,
+    TurnContext,
+    TestAdapter,
+    MemoryStorage,
+    ConversationState,
+    UserState,
+    useBotState,
+} = require('botbuilder-core');
 const { TestUtils } = require('..');
 const { createHash } = require('crypto');
 const { makeResourceExplorer } = require('./utils');
@@ -8,7 +19,11 @@ const {
     LanguageGenerationBotComponent,
     skillConversationIdFactoryKey,
     skillClientKey,
+    AdaptiveDialog,
+    OnBeginDialog,
+    BeginSkill,
 } = require('botbuilder-dialogs-adaptive');
+const { DialogManager } = require('botbuilder-dialogs');
 
 class MockSkillConversationIdFactory extends SkillConversationIdFactoryBase {
     constructor(opts = { useCreateSkillConversationId: false }) {
@@ -147,6 +162,51 @@ describe('ActionTests', function () {
         );
     });
 
+    it('BeginSkill_SkipPropertiesFromBotState', async function () {
+        const beginSkillDialog = new BeginSkill({
+            botId: 'test-bot-id',
+            skill: {
+                appId: 'test-app-id',
+                skillEndpoint: 'http://localhost:39782/api/messages',
+            },
+            skillHostEndpoint: 'http://localhost:39782/api/messages',
+        });
+
+        const root = new AdaptiveDialog('root').configure({
+            autoEndDialog: false,
+            triggers: [new OnBeginDialog([beginSkillDialog])],
+        });
+
+        const dm = new DialogManager(root);
+
+        const adapter = new TestAdapter((context) => {
+            context.turnState.set(skillConversationIdFactoryKey, new MockSkillConversationIdFactory());
+            context.turnState.set(skillClientKey, new MockSkillBotFrameworkClient());
+            return dm.onTurn(context);
+        });
+        const storage = new MemoryStorage();
+        const convoState = new ConversationState(storage);
+        const userState = new UserState(storage);
+        useBotState(adapter, convoState, userState);
+
+        await adapter.send('skill').send('end').startTest();
+
+        const storageKey = 'test/conversations/Convo1/';
+        const {
+            [storageKey]: { DialogState },
+        } = await storage.read([storageKey]);
+
+        const [{ state }] = DialogState.dialogStack;
+        const [actionScope] = state._adaptive.actions;
+        const [, { state: beginSkillState }] = actionScope.dialogStack;
+        const options = beginSkillState['BeginSkill.dialogOptionsData'];
+
+        assert.equal(options.conversationIdFactory, null);
+        assert.equal(options.conversationState, null);
+        assert.notEqual(beginSkillDialog.dialogOptions.conversationIdFactory, null);
+        assert.notEqual(beginSkillDialog.dialogOptions.conversationState, null);
+    });
+
     it('BeginSkillEndDialog', async function () {
         await TestUtils.runTestScript(
             resourceExplorer,

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

@@ -5,7 +5,13 @@
  * Copyright (c) Microsoft Corporation. All rights reserved.
  * Licensed under the MIT License.
  */
-import { Activity, ActivityTypes, StringUtils, TurnContext } from 'botbuilder';
+import {
+    Activity,
+    ActivityTypes,
+    StringUtils,
+    TurnContext,
+    CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY,
+} from 'botbuilder';
 import { ActivityTemplate } from '../templates';
 import { ActivityTemplateConverter } from '../converters';
 import { AdaptiveEvents } from '../adaptiveEvents';
@@ -202,6 +208,9 @@ 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;
+        const skipProperties = dc.context.turnState.get(CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY);
+        const props: (keyof SkillDialogOptions)[] = ['conversationIdFactory', 'conversationState'];
+        skipProperties(this._dialogOptionsStateKey, props);
 
         // Get the activity to send to the skill.
         options = {} as BeginSkillDialogOptions;