Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: replace stdlib assertions with zod #3859

Merged
merged 2 commits into from
Jul 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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