Skip to content

Commit

Permalink
Merge pull request #9593 from microsoft/southworks/update/parsing-lg-…
Browse files Browse the repository at this point in the history
…files-ondemand

fix: [Composer crash issue] Parse LG files when required
  • Loading branch information
OEvgeny committed Jun 5, 2023
2 parents c72cfea + ac53bd0 commit 8856d24
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 38 deletions.
29 changes: 24 additions & 5 deletions Composer/packages/client/src/pages/language-generation/LGPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import React, { Fragment, useCallback, Suspense, useEffect } from 'react';
import formatMessage from 'format-message';
import { ActionButton } from '@fluentui/react/lib/Button';
import { RouteComponentProps, Router } from '@reach/router';
import { useRecoilValue } from 'recoil';
import { useRecoilState, useRecoilValue } from 'recoil';
import { LgFile } from '@botframework-composer/types';

import { LoadingSpinner } from '../../components/LoadingSpinner';
import { navigateTo } from '../../utils/navigation';
import { Page } from '../../components/Page';
import { lgFilesSelectorFamily, localeState } from '../../recoilModel';
import { lgFileState, lgFilesSelectorFamily, localeState } from '../../recoilModel';
import TelemetryClient from '../../telemetry/TelemetryClient';
import lgWorker from '../../recoilModel/parsers/lgWorker';

import TableView from './table-view';
const CodeEditor = React.lazy(() => import('./code-editor'));
Expand All @@ -28,15 +30,32 @@ const LGPage: React.FC<RouteComponentProps<{
const actualProjectId = skillId ?? projectId;
const locale = useRecoilValue(localeState(actualProjectId));
const lgFiles = useRecoilValue(lgFilesSelectorFamily(skillId ?? projectId));
const [currentLg, setCurrentLg] = useRecoilState(
lgFileState({ projectId: actualProjectId, lgFileId: `${dialogId}.${locale}` })
);
const path = props.location?.pathname ?? '';

const edit = /\/edit(\/)?$/.test(path);

const baseURL = skillId == null ? `/bot/${projectId}/` : `/bot/${projectId}/skill/${skillId}/`;

const activeFile = lgFileId
? lgFiles.find(({ id }) => id === lgFileId || id === `${lgFileId}.${locale}`)
: lgFiles.find(({ id }) => id === dialogId || id === `${dialogId}.${locale}`);
const getActiveFile = () => {
let lgFile: LgFile | undefined;
if (lgFileId) {
lgFile = lgFiles.find(({ id }) => id === lgFileId || id === `${lgFileId}.${locale}`);
} else {
lgFile = lgFiles.find(({ id }) => id === dialogId || id === `${dialogId}.${locale}`);
}
if (lgFile?.isContentUnparsed) {
lgWorker.parse(actualProjectId, currentLg.id, currentLg.content, lgFiles).then((result) => {
setCurrentLg(result as LgFile);
return currentLg;
});
}
return lgFile;
};

const activeFile = getActiveFile();

useEffect(() => {
if (!activeFile && lgFiles.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,19 @@ import formatMessage from 'format-message';
import { NeutralColors, FontSizes } from '@fluentui/theme';
import { RouteComponentProps } from '@reach/router';
import { LgTemplate } from '@bfc/shared';
import { useRecoilValue } from 'recoil';
import { useRecoilState, useRecoilValue } from 'recoil';
import { lgUtil } from '@bfc/indexers';
import { LgFile } from '@botframework-composer/types/src';

import { EditableField } from '../../components/EditableField';
import { navigateTo } from '../../utils/navigation';
import { actionButton, editableFieldContainer } from '../language-understanding/styles';
import { dispatcherState, localeState, settingsState, dialogsSelectorFamily } from '../../recoilModel';
import { dispatcherState, localeState, settingsState, dialogsSelectorFamily, lgFileState } from '../../recoilModel';
import { languageListTemplates } from '../../components/MultiLanguage';
import TelemetryClient from '../../telemetry/TelemetryClient';
import { lgFilesSelectorFamily } from '../../recoilModel/selectors/lg';
import { CellFocusZone } from '../../components/CellFocusZone';
import lgWorker from '../../recoilModel/parsers/lgWorker';

interface TableViewProps extends RouteComponentProps<{ dialogId: string; skillId: string; projectId: string }> {
projectId: string;
Expand All @@ -51,10 +52,27 @@ const TableView: React.FC<TableViewProps> = (props) => {
);

const { languages, defaultLanguage } = settings;
const [defaultLg, setDefaultLg] = useRecoilState(
lgFileState({ projectId: actualProjectId, lgFileId: `${dialogId}.${defaultLanguage}` })
);

const getDefaultLangFile = () => {
let lgFile: LgFile | undefined;
if (lgFileId) {
lgFile = lgFiles.find(({ id }) => id === lgFileId);
} else {
lgFile = lgFiles.find(({ id }) => id === `${dialogId}.${defaultLanguage}`);
}
if (lgFile?.isContentUnparsed) {
lgWorker.parse(actualProjectId, defaultLg.id, defaultLg.content, lgFiles).then((result) => {
setDefaultLg(result as LgFile);
return defaultLg;
});
}
return lgFile;
};

const defaultLangFile = lgFileId
? lgFiles.find(({ id }) => id === lgFileId)
: lgFiles.find(({ id }) => id === `${dialogId}.${defaultLanguage}`);
const defaultLangFile = getDefaultLangFile();

const [templates, setTemplates] = useState<LgTemplate[]>([]);
const listRef = useRef(null);
Expand Down
17 changes: 0 additions & 17 deletions Composer/packages/client/src/recoilModel/DispatcherWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { BotAssets } from '@bfc/shared';
import { useRecoilValue } from 'recoil';
import isEmpty from 'lodash/isEmpty';

import { createMissingLgTemplatesForDialogs } from '../utils/lgUtil';

import { dialogsSelectorFamily, luFilesSelectorFamily, qnaFilesSelectorFamily } from './selectors';
import { UndoRoot } from './undo/history';
import { prepareAxios } from './../utils/auth';
Expand All @@ -24,7 +22,6 @@ import {
jsonSchemaFilesState,
crossTrainConfigState,
dispatcherState,
projectIndexingState,
} from './atoms';
import { localBotsWithoutErrorsSelector, formDialogSchemasSelectorFamily } from './selectors';
import { Recognizer } from './Recognizers';
Expand Down Expand Up @@ -94,19 +91,6 @@ const InitDispatcher = ({ onLoad }) => {
return null;
};

const repairBotProject = async (projectId: string, snapshot: Snapshot, previousSnapshot: Snapshot) => {
const indexingState = await snapshot.getPromise(projectIndexingState(projectId));
const preIndexingState = await previousSnapshot.getPromise(projectIndexingState(projectId));
if (indexingState === false && preIndexingState == true) {
const dialogs = await snapshot.getPromise(dialogsSelectorFamily(projectId));
const lgFiles = await snapshot.getPromise(lgFilesSelectorFamily(projectId));

const updatedLgFiles = await createMissingLgTemplatesForDialogs(projectId, dialogs, lgFiles);
const { updateAllLgFiles } = await snapshot.getPromise(dispatcherState);
updateAllLgFiles({ projectId, lgFiles: updatedLgFiles });
}
};

export const DispatcherWrapper = ({ children }) => {
const [loaded, setLoaded] = useState(false);
const botProjects = useRecoilValue(localBotsWithoutErrorsSelector);
Expand All @@ -119,7 +103,6 @@ export const DispatcherWrapper = ({ children }) => {
const previousAssets = await getBotAssets(projectId, previousSnapshot);
const filePersistence = await snapshot.getPromise(filePersistenceState(projectId));
if (!isEmpty(filePersistence)) {
await repairBotProject(projectId, snapshot, previousSnapshot);
if (filePersistence.isErrorHandlerEmpty()) {
filePersistence.registerErrorHandler(setProjectError);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,32 @@ const filterParseResult = (lgFile: LgFile) => {
return cloned;
};

const getTargetFile = (projectId: string, lgFile: LgFile) => {
const getTargetFile = (projectId: string, lgFile: LgFile, lgFiles: LgFile[]) => {
const cachedFile = cache.get(projectId, lgFile.id);

if (cachedFile?.isContentUnparsed) {
const lgFile = lgUtil.parse(cachedFile.id, cachedFile.content, lgFiles);
lgFile.isContentUnparsed = false;
cache.set(projectId, lgFile);
return filterParseResult(lgFile);
}

// Instead of compare content, just use cachedFile as single truth of fact, because all updates are supposed to be happen in worker, and worker will always update cache.
return cachedFile ?? lgFile;
};

const emptyLgFile = (id: string, content: string): LgFile => {
return {
id,
content,
diagnostics: [],
templates: [],
allTemplates: [],
imports: [],
isContentUnparsed: true,
};
};

export const handleMessage = (msg: LgMessageEvent) => {
let payload: any = null;
switch (msg.type) {
Expand All @@ -186,27 +205,27 @@ export const handleMessage = (msg: LgMessageEvent) => {

case LgActionType.ParseAll: {
const { lgResources, projectId } = msg.payload;

// We'll do the parsing when the file is required. Save empty LG instead.
payload = lgResources.map(({ id, content }) => {
const lgFile = lgUtil.parse(id, content, lgResources);
cache.set(projectId, lgFile);
return filterParseResult(lgFile);
const emptyLg = emptyLgFile(id, content);
cache.set(projectId, emptyLg);
return filterParseResult(emptyLg);
});

break;
}

case LgActionType.AddTemplate: {
const { lgFile, template, lgFiles, projectId } = msg.payload;
const result = lgUtil.addTemplate(getTargetFile(projectId, lgFile), template, lgFileResolver(lgFiles));
const result = lgUtil.addTemplate(getTargetFile(projectId, lgFile, lgFiles), template, lgFileResolver(lgFiles));
cache.set(projectId, result);
payload = filterParseResult(result);
break;
}

case LgActionType.AddTemplates: {
const { lgFile, templates, lgFiles, projectId } = msg.payload;
const result = lgUtil.addTemplates(getTargetFile(projectId, lgFile), templates, lgFileResolver(lgFiles));
const result = lgUtil.addTemplates(getTargetFile(projectId, lgFile, lgFiles), templates, lgFileResolver(lgFiles));
cache.set(projectId, result);
payload = filterParseResult(result);
break;
Expand All @@ -215,7 +234,7 @@ export const handleMessage = (msg: LgMessageEvent) => {
case LgActionType.UpdateTemplate: {
const { lgFile, templateName, template, lgFiles, projectId } = msg.payload;
const result = lgUtil.updateTemplate(
getTargetFile(projectId, lgFile),
getTargetFile(projectId, lgFile, lgFiles),
templateName,
template,
lgFileResolver(lgFiles)
Expand All @@ -227,15 +246,23 @@ export const handleMessage = (msg: LgMessageEvent) => {

case LgActionType.RemoveTemplate: {
const { lgFile, templateName, lgFiles, projectId } = msg.payload;
const result = lgUtil.removeTemplate(getTargetFile(projectId, lgFile), templateName, lgFileResolver(lgFiles));
const result = lgUtil.removeTemplate(
getTargetFile(projectId, lgFile, lgFiles),
templateName,
lgFileResolver(lgFiles)
);
cache.set(projectId, result);
payload = filterParseResult(result);
break;
}

case LgActionType.RemoveAllTemplates: {
const { lgFile, templateNames, lgFiles, projectId } = msg.payload;
const result = lgUtil.removeTemplates(getTargetFile(projectId, lgFile), templateNames, lgFileResolver(lgFiles));
const result = lgUtil.removeTemplates(
getTargetFile(projectId, lgFile, lgFiles),
templateNames,
lgFileResolver(lgFiles)
);
cache.set(projectId, result);
payload = filterParseResult(result);
break;
Expand All @@ -244,7 +271,7 @@ export const handleMessage = (msg: LgMessageEvent) => {
case LgActionType.CopyTemplate: {
const { lgFile, toTemplateName, fromTemplateName, lgFiles, projectId } = msg.payload;
const result = lgUtil.copyTemplate(
getTargetFile(projectId, lgFile),
getTargetFile(projectId, lgFile, lgFiles),
fromTemplateName,
toTemplateName,
lgFileResolver(lgFiles)
Expand Down

0 comments on commit 8856d24

Please sign in to comment.