refactor: AgentSettings Circular Structure and improve internals (#4641)

* Fix circular structure and allow agent configuration when connecting to the skill

* Add unit stringify unit test and improve functionality

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

@@ -277,9 +277,6 @@ 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,8 +25,6 @@ export interface CachedBotState {
     hash: string;
 }
 
-export const CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY = Symbol('cachedBotStateSkipPropertiesHandler');
-
 /**
  * Base class for the frameworks state persistance scopes.
  *
@@ -40,7 +38,6 @@ export const CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY = Symbol('cachedBotSta
  */
 export class BotState implements PropertyManager {
     private stateKey = Symbol('state');
-    private skippedProperties = new Map();
 
     /**
      * Creates a new BotState instance.
@@ -81,11 +78,6 @@ 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) => {
@@ -118,8 +110,7 @@ export class BotState implements PropertyManager {
      */
     saveChanges(context: TurnContext, force = false): Promise<void> {
         let cached: CachedBotState = context.turnState.get(this.stateKey);
-        const state = this.skipProperties(cached?.state);
-        if (force || (cached && cached.hash !== calculateChangeHash(state))) {
+        if (force || (cached && cached.hash !== calculateChangeHash(cached?.state))) {
             return Promise.resolve(this.storageKey(context)).then((key: string) => {
                 if (!cached) {
                     cached = { state: {}, hash: '' };
@@ -130,7 +121,7 @@ export class BotState implements PropertyManager {
 
                 return this.storage.write(changes).then(() => {
                     // Update change hash and cache
-                    cached.hash = calculateChangeHash(state);
+                    cached.hash = calculateChangeHash(cached.state);
                     context.turnState.set(this.stateKey, cached);
                 });
             });
@@ -198,45 +189,4 @@ export class BotState implements PropertyManager {
 
         return typeof cached === 'object' && typeof cached.state === 'object' ? cached.state : undefined;
     }
-
-    /**
-     * 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.
-     */
-    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 (value === null || value === undefined || skip.includes(key)) {
-                return;
-            }
-
-            if (Array.isArray(value)) {
-                return value.map((e) => inner([null, e], skip));
-            }
-
-            if (typeof value !== 'object') {
-                return value.valueOf();
-            }
-
-            return Object.entries(value).reduce((acc, [k, v]) => {
-                const skipResult = skipHandler(k) ?? [];
-                acc[k] = inner([k, v], [...skip, ...skipResult]);
-                return acc;
-            }, {});
-        };
-
-        return inner([null, state]);
-    }
 }

libraries/botbuilder-core/src/storage.ts

@@ -7,6 +7,8 @@
  */
 
 import * as z from 'zod';
+import { createHash } from 'crypto';
+import { stringify } from 'botbuilder-stdlib';
 import { TurnContext } from './turnContext';
 
 /**
@@ -127,10 +129,11 @@ export function assertStoreItems(val: unknown, ..._args: unknown[]): asserts val
  * @returns change hash string
  */
 export function calculateChangeHash(item: StoreItem): string {
-    const cpy = { ...item };
-    if (cpy.eTag) {
-        delete cpy.eTag;
-    }
+    // eslint-disable-next-line  @typescript-eslint/no-unused-vars
+    const { eTag, ...rest } = item;
 
-    return JSON.stringify(cpy);
+    const result = stringify(rest);
+    const hash = createHash('sha256', { encoding: 'utf-8' });
+    const hashed = hash.update(result).digest('hex');
+    return hashed;
 }

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

@@ -1,52 +1,9 @@
 const assert = require('assert');
-const {
-    TurnContext,
-    BotState,
-    MemoryStorage,
-    TestAdapter,
-    CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY,
-} = require('../');
+const { TurnContext, BotState, MemoryStorage, TestAdapter } = 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;
@@ -149,35 +106,4 @@ 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-runtime-integration-express/package.json

@@ -42,6 +42,7 @@
     "botbuilder": "4.1.6",
     "botbuilder-dialogs-adaptive-runtime": "4.1.6",
     "botbuilder-dialogs-adaptive-runtime-core": "4.1.6",
+    "botframework-connector": "4.1.6",
     "express": "^4.19.2",
     "zod": "^3.22.4"
   }

libraries/botbuilder-dialogs-adaptive-runtime-integration-express/src/index.ts

@@ -8,6 +8,7 @@ import type { ActivityHandlerBase, BotFrameworkHttpAdapter, ChannelServiceRoutes
 import type { Server } from 'http';
 import type { ServiceCollection } from 'botbuilder-dialogs-adaptive-runtime-core';
 import { Configuration, getRuntimeServices } from 'botbuilder-dialogs-adaptive-runtime';
+import type { ConnectorClientOptions } from 'botframework-connector';
 import { json, urlencoded } from 'body-parser';
 
 // Explicitly fails checks for `""`
@@ -38,6 +39,12 @@ const TypedOptions = z.object({
      * Path inside applicationRoot that should be served as static files
      */
     staticDirectory: NonEmptyString,
+
+    /**
+     * Used when creating ConnectorClients.
+     */
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    connectorClientOptions: z.object({}) as z.ZodObject<any, any, any, ConnectorClientOptions>,
 });
 
 /**
@@ -51,6 +58,7 @@ const defaultOptions: Options = {
     skillsEndpointPrefix: '/api/skills',
     port: 3978,
     staticDirectory: 'wwwroot',
+    connectorClientOptions: {},
 };
 
 /**
@@ -65,7 +73,9 @@ export async function start(
     settingsDirectory: string,
     options: Partial<Options> = {}
 ): Promise<void> {
-    const [services, configuration] = await getRuntimeServices(applicationRoot, settingsDirectory);
+    const [services, configuration] = await getRuntimeServices(applicationRoot, settingsDirectory, {
+        connectorClientOptions: options.connectorClientOptions,
+    });
     const [, listen] = await makeApp(services, configuration, applicationRoot, options);
 
     listen();

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

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
 import * as z from 'zod';
 import fs from 'fs';
 import path from 'path';
@@ -529,6 +531,7 @@ function registerQnAComponents(services: ServiceCollection, configuration: Confi
  *
  * @param applicationRoot absolute path to root of application
  * @param settingsDirectory directory where settings files are located
+ * @param defaultServices services to use as default
  * @returns service collection and configuration
  *
  * @remarks
@@ -567,27 +570,31 @@ function registerQnAComponents(services: ServiceCollection, configuration: Confi
  */
 export async function getRuntimeServices(
     applicationRoot: string,
-    settingsDirectory: string
+    settingsDirectory: string,
+    defaultServices?: Record<string, any>
 ): Promise<[ServiceCollection, Configuration]>;
 
 /**
  * Construct all runtime services.
  *
  * @param applicationRoot absolute path to root of application
  * @param configuration a fully initialized configuration instance to use
+ * @param defaultServices services to use as default
  * @returns service collection and configuration
  */
 export async function getRuntimeServices(
     applicationRoot: string,
-    configuration: Configuration
+    configuration: Configuration,
+    defaultServices?: Record<string, any>
 ): Promise<[ServiceCollection, Configuration]>;
 
 /**
  * @internal
  */
 export async function getRuntimeServices(
     applicationRoot: string,
-    configurationOrSettingsDirectory: Configuration | string
+    configurationOrSettingsDirectory: Configuration | string,
+    defaultServices: Record<string, any> = {}
 ): Promise<[ServiceCollection, Configuration]> {
     // Resolve configuration
     let configuration: Configuration;
@@ -619,6 +626,7 @@ export async function getRuntimeServices(
         middlewares: new MiddlewareSet(),
         pathResolvers: [],
         serviceClientCredentialsFactory: undefined,
+        ...defaultServices,
     });
 
     services.addFactory<ResourceExplorer, { declarativeTypes: ComponentDeclarativeTypes[] }>(

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

@@ -5,13 +5,7 @@
  * Copyright (c) Microsoft Corporation. All rights reserved.
  * Licensed under the MIT License.
  */
-import {
-    Activity,
-    ActivityTypes,
-    StringUtils,
-    TurnContext,
-    CACHED_BOT_STATE_SKIP_PROPERTIES_HANDLER_KEY,
-} from 'botbuilder';
+import { Activity, ActivityTypes, StringUtils, TurnContext } from 'botbuilder';
 import { ActivityTemplate } from '../templates';
 import { ActivityTemplateConverter } from '../converters';
 import { AdaptiveEvents } from '../adaptiveEvents';
@@ -162,7 +156,17 @@ export class BeginSkill extends SkillDialog implements BeginSkillConfiguration {
      * @param options Optional options used to configure the skill dialog.
      */
     constructor(options?: SkillDialogOptions) {
-        super(Object.assign({ skill: {} } as SkillDialogOptions, options));
+        super(
+            Object.assign({ skill: {} } as SkillDialogOptions, options, {
+                // This is an alternative to the toJSON function because when the SkillDialogOptions are saved into the Storage,
+                // when the information is retrieved, it doesn't have the properties that were declared in the toJSON function.
+                _replacer(): Omit<SkillDialogOptions, 'conversationState' | 'skillClient' | 'conversationIdFactory'> {
+                    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+                    const { conversationState, skillClient, conversationIdFactory, ...rest } = this;
+                    return rest;
+                },
+            })
+        );
     }
 
     /**
@@ -208,10 +212,6 @@ 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);
 
         // Get the activity to send to the skill.
         options = {} as BeginSkillDialogOptions;

libraries/botbuilder-stdlib/src/index.ts

@@ -7,3 +7,4 @@ export * as stringExt from './stringExt';
 export { delay } from './delay';
 export { maybeCast } from './maybeCast';
 export { retry } from './retry';
+export { stringify } from './stringify';