Skip to content

Commit

Permalink
chore: replace stdlib assertions with zod (#3859)
Browse files Browse the repository at this point in the history
Fixes #3670
  • Loading branch information
Josh Gummersall authored Jul 29, 2021
1 parent c56bbf1 commit 3dc3d7e
Show file tree
Hide file tree
Showing 36 changed files with 644 additions and 1,316 deletions.
1 change: 0 additions & 1 deletion libraries/botbuilder-ai/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
"botbuilder-dialogs": "4.1.6",
"botbuilder-dialogs-adaptive-runtime-core": "4.1.6",
"botbuilder-dialogs-declarative": "4.1.6",
"botbuilder-stdlib": "4.1.6",
"lodash": "^4.17.21",
"node-fetch": "^2.6.0",
"url-parse": "^1.5.1",
Expand Down
26 changes: 15 additions & 11 deletions libraries/botbuilder-ai/src/qnaMakerDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import * as z from 'zod';
import { ActiveLearningUtils, BindToActivity } from './qnamaker-utils';
import { Activity, ActivityTypes, MessageFactory } from 'botbuilder-core';
import { QnACardBuilder } from './qnaCardBuilder';
import { QnAMaker, QnAMakerResult } from './';
import { QnAMakerClient, QnAMakerClientKey } from './qnaMaker';

import {
ArrayExpression,
ArrayExpressionConverter,
Expand All @@ -20,7 +28,7 @@ import {
StringExpression,
StringExpressionConverter,
} from 'adaptive-expressions';
import { Activity, ActivityTypes, MessageFactory } from 'botbuilder-core';

import {
Converter,
ConverterFactory,
Expand All @@ -36,10 +44,7 @@ import {
TurnPath,
WaterfallStepContext,
} from 'botbuilder-dialogs';
import { assert, Test, tests } from 'botbuilder-stdlib';
import { QnAMaker, QnAMakerResult } from './';
import { QnACardBuilder } from './qnaCardBuilder';
import { QnAMakerClient, QnAMakerClientKey } from './qnaMaker';

import {
FeedbackRecord,
FeedbackRecords,
Expand All @@ -48,7 +53,6 @@ import {
QnAMakerOptions,
RankerTypes,
} from './qnamaker-interfaces';
import { ActiveLearningUtils, BindToActivity } from './qnamaker-utils';

class QnAMakerDialogActivityConverter
implements Converter<string, TemplateInterface<Partial<Activity>, DialogStateManager>> {
Expand Down Expand Up @@ -124,9 +128,9 @@ export interface QnAMakerDialogConfiguration extends DialogConfiguration {
*/
export type QnASuggestionsActivityFactory = (suggestionsList: string[], noMatchesText: string) => Partial<Activity>;

const isSuggestionsFactory: Test<QnASuggestionsActivityFactory> = (val): val is QnASuggestionsActivityFactory => {
return tests.isFunc(val);
};
const qnaSuggestionsActivityFactory = z.custom<QnASuggestionsActivityFactory>((val) => typeof val === 'function', {
message: 'QnASuggestionsActivityFactory',
});

/**
* A dialog that supports multi-step and adaptive-learning QnA Maker services.
Expand Down Expand Up @@ -375,7 +379,7 @@ export class QnAMakerDialog extends WaterfallDialog implements QnAMakerDialogCon
if (top) {
this.top = new IntExpression(top);
}
if (isSuggestionsFactory(activeLearningTitleOrFactory)) {
if (qnaSuggestionsActivityFactory.check(activeLearningTitleOrFactory)) {
if (!cardNoMatchText) {
// Without a developer-provided cardNoMatchText, the end user will not be able to tell the convey to the bot and QnA Maker that the suggested alternative questions were not correct.
// When the user's reply to a suggested alternatives Activity matches the cardNoMatchText, the QnAMakerDialog sends this information to the QnA Maker service for active learning.
Expand Down Expand Up @@ -710,7 +714,7 @@ export class QnAMakerDialog extends WaterfallDialog implements QnAMakerDialogCon
dialogOptions.qnaDialogResponseOptions.cardNoMatchText
);

assert.object(message, ['suggestionsActivity']);
z.record(z.unknown()).parse(message, { path: ['message'] });
await step.context.sendActivity(message);

step.activeDialog.state[this.options] = dialogOptions;
Expand Down
15 changes: 9 additions & 6 deletions libraries/botbuilder-ai/tests/qnaMakerDialog.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
const fs = require('fs');
const nock = require('nock');
const sinon = require('sinon');
const { ok, rejects, strictEqual, throws } = require('assert');
const { join } = require('path');
const { BoolExpression } = require('adaptive-expressions');
const { Dialog, DialogSet, DialogTurnStatus, DialogManager, ScopePath } = require('botbuilder-dialogs');
const { QnAMakerDialog, QnAMaker, QnACardBuilder } = require('..');
const { join } = require('path');
const { ok, rejects, strictEqual, throws } = require('assert');

const {
ActionTypes,
ActivityTypes,
Expand All @@ -13,8 +16,6 @@ const {
MessageFactory,
TestAdapter,
} = require('botbuilder-core');
const { Dialog, DialogSet, DialogTurnStatus, DialogManager, ScopePath } = require('botbuilder-dialogs');
const { QnAMakerDialog, QnAMaker, QnACardBuilder } = require('../lib');

const KB_ID = process.env.QNAKNOWLEDGEBASEID;
const ENDPOINT_KEY = process.env.QNAENDPOINTKEY;
Expand Down Expand Up @@ -439,7 +440,9 @@ describe('QnAMakerDialog', function () {

await rejects(
adapter.send('QnaMaker_TopNAnswer.json').startTest(),
(thrown) => thrown.message === '`suggestionsActivity` must be of type "object"'
(thrown) =>
thrown.message.includes('invalid_type at message') &&
thrown.message.includes('Expected object, received number')
);
});

Expand Down Expand Up @@ -477,7 +480,7 @@ describe('QnAMakerDialog', function () {

await rejects(
adapter.send('QnaMaker_TopNAnswer.json').startTest(),
(thrown) => thrown.message === '`suggestionsActivity` must be defined'
(thrown) => thrown.message.includes('invalid_type at message') && thrown.message.includes('Required')
);

sandbox.verify();
Expand Down
15 changes: 10 additions & 5 deletions libraries/botbuilder-core/etc/botbuilder-core.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { Activity } from 'botframework-schema';
import { AdaptiveCardInvokeResponse } from 'botframework-schema';
import { AdaptiveCardInvokeValue } from 'botframework-schema';
import { AnimationCard } from 'botframework-schema';
import { Assertion } from 'botbuilder-stdlib';
import { Attachment } from 'botframework-schema';
import { AudioCard } from 'botframework-schema';
import { AuthenticateRequestResult } from 'botframework-connector';
Expand Down Expand Up @@ -128,11 +127,15 @@ export class ActivityHandlerBase {
run(context: TurnContext): Promise<void>;
}

// @public (undocumented)
export const assertBotComponent: Assertion<BotComponent>;
// Warning: (ae-internal-missing-underscore) The name "assertBotComponent" should be prefixed with an underscore because the declaration is marked as @internal
//
// @internal @deprecated (undocumented)
export function assertBotComponent(val: unknown, ..._args: unknown[]): asserts val is BotComponent;

// @public (undocumented)
export const assertStoreItems: Assertion<StoreItems>;
// Warning: (ae-internal-missing-underscore) The name "assertStoreItems" should be prefixed with an underscore because the declaration is marked as @internal
//
// @internal @deprecated (undocumented)
export function assertStoreItems(val: unknown, ..._args: unknown[]): asserts val is StoreItem;

// @public
export class AutoSaveStateMiddleware implements Middleware {
Expand Down Expand Up @@ -174,6 +177,8 @@ export const BotCallbackHandlerKey = "botCallbackHandler";
export abstract class BotComponent {
// (undocumented)
abstract configureServices(services: ServiceCollection, configuration: Configuration): void;
// (undocumented)
static z: z.ZodType<BotComponent, z.ZodTypeDef>;
}

export { BotFrameworkClient }
Expand Down
19 changes: 14 additions & 5 deletions libraries/botbuilder-core/src/botComponent.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { Assertion, assert } from 'botbuilder-stdlib';
import * as z from 'zod';
import { Configuration, ServiceCollection } from 'botbuilder-dialogs-adaptive-runtime-core';

/**
Expand All @@ -12,10 +12,19 @@ import { Configuration, ServiceCollection } from 'botbuilder-dialogs-adaptive-ru
* gets called automatically on the components by the bot runtime, as long as the components are registered in the configuration.
*/
export abstract class BotComponent {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
static z = z.custom<BotComponent>((val: any) => typeof val.configureServices === 'function', {
message: 'BotComponent',
});

abstract configureServices(services: ServiceCollection, configuration: Configuration): void;
}

export const assertBotComponent: Assertion<BotComponent> = (val, path) => {
assert.unsafe.castObjectAs<BotComponent>(val, path);
assert.func(val.configureServices, path.concat('configureServices'));
};
/**
* @internal
*
* @deprecated Use `BotComponent.z.parse()` instead.
*/
export function assertBotComponent(val: unknown, ..._args: unknown[]): asserts val is BotComponent {
BotComponent.z.parse(val);
}
26 changes: 19 additions & 7 deletions libraries/botbuilder-core/src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import * as z from 'zod';
import { TurnContext } from './turnContext';
import { Assertion, assert } from 'botbuilder-stdlib';

/**
* Callback to calculate a storage key.
*
* ```TypeScript
* type StorageKeyFactory = (context: TurnContext) => Promise<string>;
* ```
*
* @param StorageKeyFactory.context Context for the current turn of conversation with a user.
* @returns A promise resolving to the storage key string
*/
export type StorageKeyFactory = (context: TurnContext) => Promise<string>;

Expand Down Expand Up @@ -71,7 +74,7 @@ export interface StoreItem {
/**
* Key/value pairs.
*/
[key: string]: any;
[key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any

/**
* (Optional) eTag field for stores that support optimistic concurrency.
Expand All @@ -86,12 +89,19 @@ export interface StoreItems {
/**
* List of store items indexed by key.
*/
[key: string]: any;
[key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
}

export const assertStoreItems: Assertion<StoreItems> = (val, path) => {
assert.object(val, path);
};
const storeItems = z.record(z.unknown());

/**
* @internal
*
* @deprecated Use `zod.record(zod.unknown())` instead.
*/
export function assertStoreItems(val: unknown, ..._args: unknown[]): asserts val is StoreItem {
storeItems.parse(val);
}

/**
* Utility function to calculate a change hash for a `StoreItem`.
Expand All @@ -112,10 +122,12 @@ export const assertStoreItems: Assertion<StoreItems> = (val, path) => {
* await storage.write({ 'botState': state });
* }
* ```
*
* @param item Item to calculate the change hash for.
* @returns change hash string
*/
export function calculateChangeHash(item: StoreItem): string {
const cpy: any = { ...item };
const cpy = { ...item };
if (cpy.eTag) {
delete cpy.eTag;
}
Expand Down
8 changes: 5 additions & 3 deletions libraries/botbuilder-dialogs-adaptive-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import {
TelemetryLoggerMiddleware,
TranscriptLoggerMiddleware,
UserState,
assertBotComponent,
createBotFrameworkAuthenticationFromConfiguration,
} from 'botbuilder';

Expand Down Expand Up @@ -390,9 +389,12 @@ async function addSettingsBotComponents(services: ServiceCollection, configurati
}

const instance = new DefaultExport();
assertBotComponent(instance, [`import(${name})`, 'default']);

return instance;
try {
return BotComponent.z.parse(instance);
} catch (err) {
throw new Error(`${name} does not extend BotComponent: ${err}`);
}
};

const components =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export class UserActivity extends TestAction implements UserActivityConfiguratio
// (undocumented)
static $kind: string;
activity: Activity;
execute(testAdapter: TestAdapter, callback: (context: TurnContext) => Promise<void>, inspector?: Inspector): Promise<void>;
execute(testAdapter: TestAdapter, callback: (context: TurnContext) => Promise<void>, _inspector?: Inspector): Promise<void>;
user: string;
}

Expand Down
3 changes: 2 additions & 1 deletion libraries/botbuilder-dialogs-adaptive-testing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
"botbuilder-stdlib": "4.1.6",
"murmurhash-js": "^1.0.0",
"nock": "^11.9.1",
"url-parse": "^1.5.1"
"url-parse": "^1.5.1",
"zod": "~1.11.17"
},
"devDependencies": {
"@microsoft/recognizers-text-suite": "1.1.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Licensed under the MIT License.
*/

import * as z from 'zod';
import { Activity, TurnContext, TestAdapter } from 'botbuilder-core';
import { Inspector, TestAction } from '../testAction';
import { tests } from 'botbuilder-stdlib';

export interface UserActivityConfiguration {
activity?: Activity;
Expand All @@ -33,15 +33,16 @@ export class UserActivity extends TestAction implements UserActivityConfiguratio

/**
* Execute the test.
*
* @param testAdapter Adapter to execute against.
* @param callback Logic for the bot to use.
* @param inspector Inspector for dialog context.
* @param _inspector Inspector for dialog context.
* @returns A Promise that represents the work queued to execute.
*/
public async execute(
testAdapter: TestAdapter,
callback: (context: TurnContext) => Promise<void>,
inspector?: Inspector
_inspector?: Inspector
): Promise<void> {
if (!this.activity) {
throw new Error('You must define one of Text of Activity properties');
Expand All @@ -62,7 +63,7 @@ export class UserActivity extends TestAction implements UserActivityConfiguratio
activity.from = { ...activity.from };
activity.from.id = this.user;
activity.from.name = this.user;
} else if (tests.isObject(this.activity?.from)) {
} else if (z.record(z.unknown()).check(this.activity?.from)) {
activity.from = { ...this.activity.from };
}

Expand Down
3 changes: 2 additions & 1 deletion libraries/botbuilder-dialogs-declarative/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"botbuilder-core": "4.1.6",
"botbuilder-dialogs": "4.1.6",
"botbuilder-stdlib": "4.1.6",
"chokidar": "^3.4.0"
"chokidar": "^3.4.0",
"zod": "~1.11.17"
},
"devDependencies": {
"adaptive-expressions": "4.1.6",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Licensed under the MIT License.
*/

import { tests } from 'botbuilder-stdlib';
import * as z from 'zod';
import { DeclarativeType } from './declarativeType';
import { ResourceExplorer } from './resources';

Expand All @@ -17,6 +17,11 @@ export interface ComponentDeclarativeTypes {
getDeclarativeTypes(resourceExplorer: ResourceExplorer): DeclarativeType[];
}

const componentDeclarativeTypes = z.custom<ComponentDeclarativeTypes>(
(val: any) => typeof val.getDeclarativeTypes === 'function',
{ message: 'ComponentDeclarativeTypes' }
);

/**
* Check if a [ComponentRegistration](xref:botbuilder-core.ComponentRegistration) is
* [ComponentDeclarativeTypes](xref:botbuilder-dialogs-declarative.ComponentDeclarativeTypes) or not.
Expand All @@ -25,5 +30,5 @@ export interface ComponentDeclarativeTypes {
* @returns {boolean} Type check result.
*/
export function isComponentDeclarativeTypes(component: unknown): component is ComponentDeclarativeTypes {
return tests.unsafe.isObjectAs<ComponentDeclarativeTypes>(component) && tests.isFunc(component.getDeclarativeTypes);
return componentDeclarativeTypes.check(component);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ResourceProvider, ResourceChangeEvent } from './resourceProvider';
import { FolderResourceProvider } from './folderResourceProvider';
import { Resource } from './resource';
import { PathUtil } from '../pathUtil';
import { ComponentDeclarativeTypes, isComponentDeclarativeTypes } from '../componentDeclarativeTypes';
import { ComponentDeclarativeTypes } from '../componentDeclarativeTypes';
import { DeclarativeType } from '../declarativeType';
import { CustomDeserializer } from '../customDeserializer';
import { DefaultLoader } from '../defaultLoader';
Expand Down
Loading

0 comments on commit 3dc3d7e

Please sign in to comment.