Skip to content

Commit

Permalink
feat: new validation pipeline - schema existence validation (#6645)
Browse files Browse the repository at this point in the history
* add placeholder for schema validator

* add schema validator pipeline with mocked fn

* add schema visitor

* display diagnostics data in debug panel

* revert sdk.ts

* decrease schema diagnostic severity to 'Warning'

* optmize path join logic

* impl a unified walker covers SwitchCondition

* fix lint error: use BaseSchema

* feat: disable actions without schema

* wrap in useEffect

* optimization: avoid frequent recoil submission

* optimization: aggregate paths rather than updatedDialog to reduce time complexity

* chore: comments & var name

* lint

* add comments

Co-authored-by: Chris Whitten <[email protected]>
  • Loading branch information
yeze322 and cwhitten authored Apr 13, 2021
1 parent b90b17b commit d4461e9
Show file tree
Hide file tree
Showing 13 changed files with 660 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
import { jsx, css } from '@emotion/core';
import formatMessage from 'format-message';

import { useAutoFix } from './useAutoFix';
import { useDiagnosticsStatistics } from './useDiagnostics';

export const DiagnosticsHeader = () => {
const { hasError, hasWarning } = useDiagnosticsStatistics();
useAutoFix();

return (
<div
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import get from 'lodash/get';
import set from 'lodash/set';
import cloneDeep from 'lodash/cloneDeep';
import { useRecoilValue } from 'recoil';
import { useEffect } from 'react';

import { DiagnosticType, SchemaDiagnostic } from '../../../../diagnostics/types';
import { botProjectSpaceSelector, dispatcherState } from '../../../../../recoilModel';

import { useDiagnosticsData } from './useDiagnostics';

export const useAutoFix = () => {
const diagnostics = useDiagnosticsData();
const botProjectSpace = useRecoilValue(botProjectSpaceSelector);
const { updateDialog } = useRecoilValue(dispatcherState);

// Auto fix schema absence by setting 'disabled' to true.
useEffect(() => {
const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[];
/**
* Aggregated diagnostic paths where contains schema problem
*
* Example:
* {
* // projectId
* '2096.637': {
* // dialogId
* 'dialog-1': [
* 'triggers[0].actions[1]', // diagnostics.dialogPath
* 'triggers[2].actions[3]'
* ]
* }
* }
*/
const aggregatedPaths: { [projectId: string]: { [dialogId: string]: string[] } } = {};

// Aggregates schema diagnostics by projectId, dialogId
schemaDiagnostics.forEach((d) => {
const { projectId, id: dialogId, dialogPath } = d;
if (!dialogPath) return;
const currentPaths = get(aggregatedPaths, [projectId, dialogId]);
if (currentPaths) {
currentPaths.push(dialogPath);
} else {
set(aggregatedPaths, [projectId, dialogId], [dialogPath]);
}
});

// Consumes aggregatedPaths to update dialogs in recoil store
for (const [projectId, pathsByDialogId] of Object.entries(aggregatedPaths)) {
// Locates dialogs in current project
const dialogsInProject = botProjectSpace.find((bot) => bot.projectId === projectId)?.dialogs;
if (!Array.isArray(dialogsInProject)) continue;

for (const [dialogId, paths] of Object.entries(pathsByDialogId)) {
// Queries out current dialog data
const dialogData = dialogsInProject.find((dialog) => dialog.id === dialogId)?.content;
if (!dialogData) continue;

// Filters out those paths where action exists and action.disabled !== true
const pathsToUpdate = paths.filter((p) => {
const data = get(dialogData, p);
return data && !get(data, 'disabled');
});
if (!pathsToUpdate.length) continue;

// Manipulates the 'disabled' property and then submit to Recoil store.
const copy = cloneDeep(dialogData);
for (const p of pathsToUpdate) {
set(copy, `${p}.disabled`, true);
}
updateDialog({ id: dialogId, projectId, content: copy });
}
}
}, [diagnostics]);
};
5 changes: 5 additions & 0 deletions Composer/packages/client/src/pages/diagnostics/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export enum DiagnosticType {
SKILL,
SETTING,
GENERAL,
SCHEMA,
}

export interface IDiagnosticInfo {
Expand Down Expand Up @@ -94,6 +95,10 @@ export class DialogDiagnostic extends DiagnosticInfo {
};
}

export class SchemaDiagnostic extends DialogDiagnostic {
type = DiagnosticType.SCHEMA;
}

export class SkillSettingDiagnostic extends DiagnosticInfo {
type = DiagnosticType.SKILL;
constructor(rootProjectId: string, projectId: string, id: string, location: string, diagnostic: Diagnostic) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { BotIndexer } from '@bfc/indexers';
import { BotIndexer, validateSchema } from '@bfc/indexers';
import { selectorFamily, selector } from 'recoil';
import lodashGet from 'lodash/get';
import formatMessage from 'format-message';
Expand All @@ -18,6 +18,7 @@ import {
BotDiagnostic,
SettingDiagnostic,
SkillSettingDiagnostic,
SchemaDiagnostic,
} from '../../pages/diagnostics/types';
import {
botDiagnosticsState,
Expand Down Expand Up @@ -176,7 +177,30 @@ export const dialogsDiagnosticsSelectorFamily = selectorFamily({
});
});

return [];
return diagnosticList;
},
});

export const schemaDiagnosticsSelectorFamily = selectorFamily({
key: 'schemaDiagnosticsSelectorFamily',
get: (projectId: string) => ({ get }) => {
const botAssets = get(botAssetsSelectFamily(projectId));
// Why botAssets.dialogSchemas is a list?
if (botAssets === null) return [];

const rootProjectId = get(rootBotProjectIdSelector) ?? projectId;

const sdkSchemaContent = botAssets.dialogSchemas[0]?.content;
if (!sdkSchemaContent) return [];

const fullDiagnostics: DiagnosticInfo[] = [];
botAssets.dialogs.forEach((dialog) => {
const diagnostics = validateSchema(dialog.id, dialog.content, sdkSchemaContent);
fullDiagnostics.push(
...diagnostics.map((d) => new SchemaDiagnostic(rootProjectId, projectId, dialog.id, `${dialog.id}.dialog`, d))
);
});
return fullDiagnostics;
},
});

Expand Down Expand Up @@ -257,6 +281,7 @@ export const diagnosticsSelectorFamily = selectorFamily({
...get(luDiagnosticsSelectorFamily(projectId)),
...get(lgDiagnosticsSelectorFamily(projectId)),
...get(qnaDiagnosticsSelectorFamily(projectId)),
...get(schemaDiagnosticsSelectorFamily(projectId)),
],
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

export const sendActivityStub = {
$kind: 'Microsoft.SendActivity',
$designer: {
id: 'AwT1u7',
},
};

export const switchConditionStub = {
$kind: 'Microsoft.SwitchCondition',
$designer: {
id: 'sJzdQm',
},
default: [sendActivityStub],
cases: [
{
value: 'case1',
actions: [sendActivityStub],
},
],
};

export const onConversationUpdateActivityStub = {
$kind: 'Microsoft.OnConversationUpdateActivity',
$designer: {
id: '376720',
},
actions: [switchConditionStub],
};

export const simpleGreetingDialog: any = {
$kind: 'Microsoft.AdaptiveDialog',
$designer: {
$designer: {
name: 'EmptyBot-1',
description: '',
id: '47yxe0',
},
},
autoEndDialog: true,
defaultResultProperty: 'dialog.result',
triggers: [onConversationUpdateActivityStub],
$schema:
'https://raw.githubusercontent.com/microsoft/BotFramework-Composer/stable/Composer/packages/server/schemas/sdk.schema',
generator: 'EmptyBot-1.lg',
id: 'EmptyBot-1',
recognizer: 'EmptyBot-1.lu.qna',
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { SDKKinds } from '@botframework-composer/types';

import {
AdaptiveDialogSchema,
IfConditionSchema,
OnConvUpdateSchema,
OnDialogEventSchema,
SwitchConditionSchema,
} from './sdkSchemaMocks';

export const sdkSchemaDefinitionMock = {
[SDKKinds.SwitchCondition]: SwitchConditionSchema,
[SDKKinds.IfCondition]: IfConditionSchema,
[SDKKinds.AdaptiveDialog]: AdaptiveDialogSchema,
[SDKKinds.OnDialogEvent]: OnDialogEventSchema,
[SDKKinds.OnConversationUpdateActivity]: OnConvUpdateSchema,
};
Loading

0 comments on commit d4461e9

Please sign in to comment.