From 07f037c38e90e068050c76ddc3c074b0670edc34 Mon Sep 17 00:00:00 2001 From: Zhixiang Zhan Date: Wed, 31 Mar 2021 15:23:15 +0800 Subject: [PATCH] fix: use import resolver in LU parsing (#6536) * use import resolver in lu parsing * update ut * handle lu resolve error * use origin fileId to update * update Co-authored-by: Dong Lei --- .../src/pages/design/useEmptyPropsHandler.tsx | 6 +- .../client/src/recoilModel/atoms/botState.ts | 1 + .../dispatchers/__tests__/lu.test.tsx | 2 +- .../client/src/recoilModel/dispatchers/lu.ts | 19 ++-- .../recoilModel/dispatchers/utils/project.ts | 1 + .../parsers/__test__/luWorker.test.ts | 15 +-- .../src/recoilModel/parsers/luWorker.ts | 30 +++-- .../client/src/recoilModel/parsers/types.ts | 6 + .../parsers/workers/luParser.worker.ts | 31 +++--- .../lu/__test__/InsertEntityButton.test.tsx | 1 + .../lib/indexers/__tests__/luIndexer.test.ts | 20 +++- .../lib/indexers/__tests__/luUtil.test.ts | 103 +++++++++++++----- .../packages/lib/indexers/src/luIndexer.ts | 24 ++-- .../packages/lib/indexers/src/utils/luUtil.ts | 94 ++++++++++++---- .../lib/shared/src/resolverFactory.ts | 2 +- Composer/packages/types/src/indexers.ts | 2 + .../ui-plugins/luis/src/LuisIntentEditor.tsx | 32 ++++-- 17 files changed, 277 insertions(+), 112 deletions(-) diff --git a/Composer/packages/client/src/pages/design/useEmptyPropsHandler.tsx b/Composer/packages/client/src/pages/design/useEmptyPropsHandler.tsx index f3da0450cd..1819a7d4ce 100644 --- a/Composer/packages/client/src/pages/design/useEmptyPropsHandler.tsx +++ b/Composer/packages/client/src/pages/design/useEmptyPropsHandler.tsx @@ -17,6 +17,7 @@ import { luFileState, qnaFileState, settingsState, + luFilesSelectorFamily, } from '../../recoilModel'; import { decodeDesignerPathToArrayPath } from '../../utils/convertUtils/designerPathEncoder'; import lgDiagnosticWorker from '../../recoilModel/parsers/lgDiagnosticWorker'; @@ -50,6 +51,7 @@ export const useEmptyPropsHandler = ( qnaFileState({ projectId: activeBot, qnaFileId: `${dialogId}.${locale}` }) ); const lgFiles = useRecoilValue(lgFilesSelectorFamily(projectId)); + const luFiles = useRecoilValue(luFilesSelectorFamily(projectId)); const { updateDialog, setDesignPageLocation, navTo } = useRecoilValue(dispatcherState); // migration: add id to dialog when dialog doesn't have id @@ -84,14 +86,14 @@ export const useEmptyPropsHandler = ( let isMounted = true; if (currentLu.isContentUnparsed) { //for current dialog, check the lu file to make sure the file is parsed. - luWorker.parse(currentLu.id, currentLu.content, settings.luFeatures).then((result) => { + luWorker.parse(currentLu.id, currentLu.content, settings.luFeatures, luFiles).then((result) => { isMounted ? setCurrentLu(result as LuFile) : null; }); return () => { isMounted = false; }; } - }, [currentDialog, currentLu]); + }, [currentDialog, currentLu, luFiles]); useEffect(() => { if (!currentDialog || !currentQna.id) return; diff --git a/Composer/packages/client/src/recoilModel/atoms/botState.ts b/Composer/packages/client/src/recoilModel/atoms/botState.ts index bb2729a3f6..2b1c27e37b 100644 --- a/Composer/packages/client/src/recoilModel/atoms/botState.ts +++ b/Composer/packages/client/src/recoilModel/atoms/botState.ts @@ -64,6 +64,7 @@ const emptyLu: LuFile = { content: '', diagnostics: [], intents: [], + allIntents: [], empty: true, resource: { Sections: [], diff --git a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/lu.test.tsx b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/lu.test.tsx index 7510f85939..61845e072c 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/lu.test.tsx +++ b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/lu.test.tsx @@ -31,7 +31,7 @@ const file1 = { content: `\r\n# Hello\r\n-hi`, }; -const luFiles = [luUtil.parse(file1.id, file1.content, luFeatures)] as LuFile[]; +const luFiles = [luUtil.parse(file1.id, file1.content, luFeatures, [])] as LuFile[]; const getLuIntent = (Name, Body): LuIntentSection => ({ diff --git a/Composer/packages/client/src/recoilModel/dispatchers/lu.ts b/Composer/packages/client/src/recoilModel/dispatchers/lu.ts index 61449a9cf8..810b9a57b9 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/lu.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/lu.ts @@ -101,11 +101,12 @@ const getRelatedLuFileChanges = async ( // sync add/remove intents if (onlyAdds || onlyDeletes) { for (const item of sameIdOtherLocaleFiles) { - let newLuFile = (await luWorker.addIntents(item, addedIntents, luFeatures)) as LuFile; + let newLuFile = (await luWorker.addIntents(item, addedIntents, luFeatures, luFiles)) as LuFile; newLuFile = (await luWorker.removeIntents( newLuFile, deletedIntents.map(({ Name }) => Name), - luFeatures + luFeatures, + luFiles )) as LuFile; changes.push(newLuFile); } @@ -130,7 +131,7 @@ export const createLuFileState = async ( const locale = await snapshot.getPromise(localeState(projectId)); const { languages, luFeatures } = await snapshot.getPromise(settingsState(projectId)); const createdLuId = `${id}.${locale}`; - const createdLuFile = (await luWorker.parse(id, content, luFeatures)) as LuFile; + const createdLuFile = (await luWorker.parse(id, content, luFeatures, luFiles)) as LuFile; if (luFiles.find((lu) => lu.id === createdLuId)) { setError(callbackHelpers, new Error(formatMessage('lu file already exist'))); return; @@ -197,7 +198,7 @@ export const luDispatcher = () => { const { luFeatures } = await snapshot.getPromise(settingsState(projectId)); try { - const updatedFile = (await luWorker.parse(id, content, luFeatures)) as LuFile; + const updatedFile = (await luWorker.parse(id, content, luFeatures, luFiles)) as LuFile; const updatedFiles = await getRelatedLuFileChanges(luFiles, updatedFile, projectId, luFeatures); // compare to drop expired change on current id file. /** @@ -252,7 +253,8 @@ export const luDispatcher = () => { item, intentName, { Name: intent.Name }, - luFeatures + luFeatures, + luFiles )) as LuFile; changes.push(updatedFile); } @@ -263,7 +265,8 @@ export const luDispatcher = () => { luFile, intentName, { Body: intent.Body }, - luFeatures + luFeatures, + luFiles )) as LuFile; updateLuFiles(callbackHelpers, projectId, { updates: [updatedFile] }); } @@ -290,7 +293,7 @@ export const luDispatcher = () => { const file = luFiles.find((temp) => temp.id === id); if (!file) return luFiles; try { - const updatedFile = (await luWorker.addIntent(file, intent, luFeatures)) as LuFile; + const updatedFile = (await luWorker.addIntent(file, intent, luFeatures, luFiles)) as LuFile; const updatedFiles = await getRelatedLuFileChanges(luFiles, updatedFile, projectId, luFeatures); updateLuFiles(callbackHelpers, projectId, { updates: updatedFiles }); } catch (error) { @@ -316,7 +319,7 @@ export const luDispatcher = () => { const file = luFiles.find((temp) => temp.id === id); if (!file) return luFiles; try { - const updatedFile = (await luWorker.removeIntent(file, intentName, luFeatures)) as LuFile; + const updatedFile = (await luWorker.removeIntent(file, intentName, luFeatures, luFiles)) as LuFile; const updatedFiles = await getRelatedLuFileChanges(luFiles, updatedFile, projectId, luFeatures); updateLuFiles(callbackHelpers, projectId, { updates: updatedFiles }); } catch (error) { diff --git a/Composer/packages/client/src/recoilModel/dispatchers/utils/project.ts b/Composer/packages/client/src/recoilModel/dispatchers/utils/project.ts index 9b2c140c43..db417266a0 100644 --- a/Composer/packages/client/src/recoilModel/dispatchers/utils/project.ts +++ b/Composer/packages/client/src/recoilModel/dispatchers/utils/project.ts @@ -247,6 +247,7 @@ const emptyLuFile = (id: string, content: string): LuFile => { content, diagnostics: [], intents: [], + allIntents: [], empty: true, resource: { Sections: [], diff --git a/Composer/packages/client/src/recoilModel/parsers/__test__/luWorker.test.ts b/Composer/packages/client/src/recoilModel/parsers/__test__/luWorker.test.ts index a37cdb70c6..ad94307424 100644 --- a/Composer/packages/client/src/recoilModel/parsers/__test__/luWorker.test.ts +++ b/Composer/packages/client/src/recoilModel/parsers/__test__/luWorker.test.ts @@ -31,7 +31,7 @@ describe('test lu worker', () => { it('get expected parse result', async () => { const content = `# Hello - hi`; - const result: any = await luWorker.parse('', content, luFeatures); + const result: any = await luWorker.parse('', content, luFeatures, []); const expected = [ { Body: '- hi', Entities: [], Name: 'Hello', range: new Range(new Position(1, 0), new Position(2, 4)) }, ]; @@ -47,7 +47,7 @@ hi @ simple friendsName `; - const { intents, diagnostics }: any = await luWorker.parse('', content, luFeatures); + const { intents, diagnostics }: any = await luWorker.parse('', content, luFeatures, []); expect(intents.length).toEqual(1); expect(diagnostics.length).toEqual(1); expect(diagnostics[0].range.start.line).toEqual(2); @@ -57,7 +57,7 @@ hi }); it('get expected add intent result', async () => { - const result: any = await luWorker.addIntent(luFile, getLuIntent('New', '-IntentValue'), luFeatures); + const result: any = await luWorker.addIntent(luFile, getLuIntent('New', '-IntentValue'), luFeatures, []); const expected = { Body: '-IntentValue', Entities: [], @@ -73,7 +73,8 @@ hi const result: any = await luWorker.addIntents( luFile, [getLuIntent('New1', '-IntentValue1'), getLuIntent('New2', '-IntentValue2')], - luFeatures + luFeatures, + [] ); const expected = { Body: '-IntentValue2', @@ -87,7 +88,7 @@ hi }); it('get expected update intent result', async () => { - const result: any = await luWorker.updateIntent(luFile, 'New', getLuIntent('New', '-update'), luFeatures); + const result: any = await luWorker.updateIntent(luFile, 'New', getLuIntent('New', '-update'), luFeatures, []); const expected = { Body: '-update', Entities: [], @@ -100,14 +101,14 @@ hi }); it('get expected remove intent result', async () => { - const result: any = await luWorker.removeIntent(luFile, 'New2', luFeatures); + const result: any = await luWorker.removeIntent(luFile, 'New2', luFeatures, []); expect(result.intents.length).toBe(3); expect(result.intents[3]).toBeUndefined(); luFile = result; }); it('get expected remove intent result', async () => { - const result: any = await luWorker.removeIntents(luFile, ['New1', 'New'], luFeatures); + const result: any = await luWorker.removeIntents(luFile, ['New1', 'New'], luFeatures, []); expect(result.intents.length).toBe(1); }); }); diff --git a/Composer/packages/client/src/recoilModel/parsers/luWorker.ts b/Composer/packages/client/src/recoilModel/parsers/luWorker.ts index 0b15f1b4b5..e200d23fd3 100644 --- a/Composer/packages/client/src/recoilModel/parsers/luWorker.ts +++ b/Composer/packages/client/src/recoilModel/parsers/luWorker.ts @@ -16,8 +16,8 @@ import { } from './types'; // Wrapper class class LuWorker extends BaseWorker { - parse(id: string, content: string, luFeatures) { - const payload = { id, content, luFeatures }; + parse(id: string, content: string, luFeatures, luFiles: LuFile[]) { + const payload = { id, content, luFeatures, luFiles }; return this.sendMsg(LuActionType.Parse, payload); } @@ -26,28 +26,34 @@ class LuWorker extends BaseWorker { return this.sendMsg(LuActionType.ParseAll, payload); } - addIntent(luFile: LuFile, intent: LuIntentSection, luFeatures) { - const payload = { luFile, intent, luFeatures }; + addIntent(luFile: LuFile, intent: LuIntentSection, luFeatures, luFiles: LuFile[]) { + const payload = { luFile, intent, luFeatures, luFiles }; return this.sendMsg(LuActionType.AddIntent, payload); } - updateIntent(luFile: LuFile, intentName: string, intent: { Name?: string; Body?: string }, luFeatures) { - const payload = { luFile, intentName, intent, luFeatures }; + updateIntent( + luFile: LuFile, + intentName: string, + intent: { Name?: string; Body?: string }, + luFeatures, + luFiles: LuFile[] + ) { + const payload = { luFile, intentName, intent, luFeatures, luFiles }; return this.sendMsg(LuActionType.UpdateIntent, payload); } - removeIntent(luFile: LuFile, intentName: string, luFeatures) { - const payload = { luFile, intentName, luFeatures }; + removeIntent(luFile: LuFile, intentName: string, luFeatures, luFiles: LuFile[]) { + const payload = { luFile, intentName, luFeatures, luFiles }; return this.sendMsg(LuActionType.RemoveIntent, payload); } - addIntents(luFile: LuFile, intents: LuIntentSection[], luFeatures) { - const payload = { luFile, intents, luFeatures }; + addIntents(luFile: LuFile, intents: LuIntentSection[], luFeatures, luFiles: LuFile[]) { + const payload = { luFile, intents, luFeatures, luFiles }; return this.sendMsg(LuActionType.AddIntents, payload); } - removeIntents(luFile: LuFile, intentNames: string[], luFeatures) { - const payload = { luFile, intentNames, luFeatures }; + removeIntents(luFile: LuFile, intentNames: string[], luFeatures, luFiles: LuFile[]) { + const payload = { luFile, intentNames, luFeatures, luFiles }; return this.sendMsg(LuActionType.RemoveIntents, payload); } } diff --git a/Composer/packages/client/src/recoilModel/parsers/types.ts b/Composer/packages/client/src/recoilModel/parsers/types.ts index 34a74a85de..a0bb6475e6 100644 --- a/Composer/packages/client/src/recoilModel/parsers/types.ts +++ b/Composer/packages/client/src/recoilModel/parsers/types.ts @@ -8,6 +8,7 @@ export type LuParsePayload = { id: string; content: string; luFeatures: ILUFeaturesConfig; + luFiles: LuFile[]; }; export type LuParseAllPayload = { @@ -19,12 +20,14 @@ export type LuAddIntentPayload = { luFile: LuFile; intent: LuIntentSection; luFeatures: ILUFeaturesConfig; + luFiles: LuFile[]; }; export type LuAddIntentsPayload = { luFile: LuFile; intents: LuIntentSection[]; luFeatures: ILUFeaturesConfig; + luFiles: LuFile[]; }; export type LuUpdateIntentPayload = { @@ -32,18 +35,21 @@ export type LuUpdateIntentPayload = { intentName: string; intent?: { Name?: string; Body?: string }; luFeatures: ILUFeaturesConfig; + luFiles: LuFile[]; }; export type LuRemoveIntentPayload = { luFile: LuFile; intentName: string; luFeatures: ILUFeaturesConfig; + luFiles: LuFile[]; }; export type LuRemoveIntentsPayload = { luFile: LuFile; intentNames: string[]; luFeatures: ILUFeaturesConfig; + luFiles: LuFile[]; }; export type LgParsePayload = { diff --git a/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts b/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts index 97872fadf3..de836021ff 100644 --- a/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts +++ b/Composer/packages/client/src/recoilModel/parsers/workers/luParser.worker.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { luUtil } from '@bfc/indexers'; +import { luImportResolverGenerator } from '@bfc/shared'; import { LuActionType, @@ -66,48 +67,52 @@ type LuMessageEvent = | RemoveIntentsMessage | ParseAllMessage; +const luFileResolver = (luFiles) => { + return luImportResolverGenerator(luFiles, '.lu'); +}; + export const handleMessage = (msg: LuMessageEvent) => { let result: any = null; switch (msg.type) { case LuActionType.Parse: { - const { id, content, luFeatures } = msg.payload; - result = luUtil.parse(id, content, luFeatures); + const { id, content, luFeatures, luFiles } = msg.payload; + result = luUtil.parse(id, content, luFeatures, luFiles); break; } case LuActionType.ParseAll: { const { luResources, luFeatures } = msg.payload; - result = luResources.map(({ id, content }) => luUtil.parse(id, content, luFeatures)); + result = luResources.map(({ id, content }) => luUtil.parse(id, content, luFeatures, luResources)); break; } case LuActionType.AddIntent: { - const { luFile, intent, luFeatures } = msg.payload; - result = luUtil.addIntent(luFile, intent, luFeatures); + const { luFile, intent, luFeatures, luFiles } = msg.payload; + result = luUtil.addIntent(luFile, intent, luFeatures, luFileResolver(luFiles)); break; } case LuActionType.AddIntents: { - const { luFile, intents, luFeatures } = msg.payload; - result = luUtil.addIntents(luFile, intents, luFeatures); + const { luFile, intents, luFeatures, luFiles } = msg.payload; + result = luUtil.addIntents(luFile, intents, luFeatures, luFileResolver(luFiles)); break; } case LuActionType.UpdateIntent: { - const { luFile, intentName, intent, luFeatures } = msg.payload; - result = luUtil.updateIntent(luFile, intentName, intent || null, luFeatures); + const { luFile, intentName, intent, luFeatures, luFiles } = msg.payload; + result = luUtil.updateIntent(luFile, intentName, intent || null, luFeatures, luFileResolver(luFiles)); break; } case LuActionType.RemoveIntent: { - const { luFile, intentName, luFeatures } = msg.payload; - result = luUtil.removeIntent(luFile, intentName, luFeatures); + const { luFile, intentName, luFeatures, luFiles } = msg.payload; + result = luUtil.removeIntent(luFile, intentName, luFeatures, luFileResolver(luFiles)); break; } case LuActionType.RemoveIntents: { - const { luFile, intentNames, luFeatures } = msg.payload; - result = luUtil.removeIntents(luFile, intentNames, luFeatures); + const { luFile, intentNames, luFeatures, luFiles } = msg.payload; + result = luUtil.removeIntents(luFile, intentNames, luFeatures, luFileResolver(luFiles)); break; } } diff --git a/Composer/packages/lib/code-editor/src/lu/__test__/InsertEntityButton.test.tsx b/Composer/packages/lib/code-editor/src/lu/__test__/InsertEntityButton.test.tsx index 8e12cc9b79..5454666b05 100644 --- a/Composer/packages/lib/code-editor/src/lu/__test__/InsertEntityButton.test.tsx +++ b/Composer/packages/lib/code-editor/src/lu/__test__/InsertEntityButton.test.tsx @@ -35,6 +35,7 @@ const luFile: LuFile = { Entities: [{ Name: 'target', Type: 'ml' }], }, ], + allIntents: [], empty: false, content: '', imports: [], diff --git a/Composer/packages/lib/indexers/__tests__/luIndexer.test.ts b/Composer/packages/lib/indexers/__tests__/luIndexer.test.ts index 4d897e01ef..da10dc5595 100644 --- a/Composer/packages/lib/indexers/__tests__/luIndexer.test.ts +++ b/Composer/packages/lib/indexers/__tests__/luIndexer.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { FileInfo } from '@bfc/shared'; +import { FileInfo, luImportResolverGenerator } from '@bfc/shared'; import { luIndexer } from '../src/luIndexer'; @@ -45,13 +45,25 @@ describe('parse', () => { }); it('should parse LU file with import', () => { - const content = `[import](greating.lu) + const content = `[import](greeting.lu) [import](../common/aks.lu) `; - const result = parse(content, '', {}); + const luFiles = [ + { + id: 'greeting', + content: '', + }, + { + id: 'aks', + content: '', + }, + ]; + const luImportResolver = luImportResolverGenerator(luFiles, '.lu'); + + const result = parse(content, 'a.lu', {}, luImportResolver); expect(result.imports.length).toEqual(2); - expect(result.imports[0]).toEqual({ id: 'greating.lu', path: 'greating.lu', description: 'import' }); + expect(result.imports[0]).toEqual({ id: 'greeting.lu', path: 'greeting.lu', description: 'import' }); expect(result.imports[1]).toEqual({ id: 'aks.lu', path: '../common/aks.lu', description: 'import' }); expect(result.empty).toEqual(false); expect(result.intents.length).toEqual(0); diff --git a/Composer/packages/lib/indexers/__tests__/luUtil.test.ts b/Composer/packages/lib/indexers/__tests__/luUtil.test.ts index a78a48b396..4aaa2c850e 100644 --- a/Composer/packages/lib/indexers/__tests__/luUtil.test.ts +++ b/Composer/packages/lib/indexers/__tests__/luUtil.test.ts @@ -3,19 +3,66 @@ import { sectionHandler } from '@microsoft/bf-lu/lib/parser/composerindex'; import { updateIntent, addIntent, removeIntent, checkSection, parse, semanticValidate } from '../src/utils/luUtil'; -import { luIndexer } from '../src/luIndexer'; const { luParser, luSectionTypes } = sectionHandler; describe('LU parse and validation', () => { + it('should parse reference', () => { + const fileContent = `[import](welcome.lu) + # AskForName + - {@userName=Jack} + `; + const luFiles = [ + { + id: 'welcome', + content: `# welcome + - hi + `, + }, + ]; + const result1 = parse('a.lu', fileContent, {}, luFiles); + expect(result1.diagnostics.length).toEqual(0); + expect(result1.intents.length).toEqual(1); + expect(result1.allIntents.length).toEqual(2); + expect(result1.allIntents[1].fileId).toEqual('welcome'); + }); + + it('should parse reference deep and with locale', () => { + const fileContent = `[import](welcome.lu) + # AskForName + - {@userName=Jack} + `; + const luFiles = [ + { + id: 'welcome.en-us', + content: `[import](greeting.lu) + # welcome + - hi + `, + }, + { + id: 'greeting', + content: `# greeting + - hi + `, + }, + ]; + const result1 = parse('a.en-us.lu', fileContent, {}, luFiles); + expect(result1.diagnostics.length).toEqual(0); + expect(result1.intents.length).toEqual(1); + expect(result1.allIntents.length).toEqual(3); + expect(result1.allIntents[1].fileId).toEqual('welcome.en-us'); + expect(result1.allIntents[2].fileId).toEqual('greeting'); + }); + it('Throws when ML entity is disable (validateResource)', () => { const fileContent = `# AskForName - {@userName=Jack} `; - const result1 = luIndexer.parse(fileContent, 'a.lu', { enableMLEntities: false }); + const result1 = parse('a.lu', fileContent, { enableMLEntities: false }, []); expect(result1.diagnostics.length).toEqual(1); - const result2 = luIndexer.parse(fileContent, 'a.lu', { enableMLEntities: true }); + const result2 = parse('a.lu', fileContent, { enableMLEntities: true }, []); expect(result2.diagnostics.length).toEqual(0); }); @@ -75,7 +122,7 @@ hi const luFeatures = {}; it('parse section test', () => { - const luresource = luIndexer.parse(fileContent, fileId1, luFeatures).resource; + const luresource = parse(fileId1, fileContent, luFeatures, []).resource; const { Sections, Errors, Content } = luresource; expect(Content).toEqual(fileContent); @@ -90,7 +137,7 @@ hi }); it('parse section with syntax error test', () => { - const luresource = luIndexer.parse(fileContentError1, fileId2, luFeatures).resource; + const luresource = parse(fileId2, fileContentError1, luFeatures, []).resource; const { Sections, Errors, Content } = luresource; expect(Content).toEqual(fileContentError1); @@ -101,7 +148,7 @@ hi }); it('parse section can get diagnostic line number', () => { - const luFile = parse(fileId2, fileContentError1, luFeatures); + const luFile = parse(fileId2, fileContentError1, luFeatures, []); const { intents, diagnostics, content } = luFile; expect(content).toEqual(fileContentError1); @@ -119,7 +166,7 @@ hi Body: `- check my unread email - show my unread emails`, }; - const luFile1 = luIndexer.parse(fileContent, fileId1, luFeatures); + const luFile1 = parse(fileId1, fileContent, luFeatures, []); const luFile1Updated = addIntent(luFile1, intent, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -151,7 +198,7 @@ hi - show my emails 2 - check my mail box please`, }; - const luFile1 = luIndexer.parse(fileContent, fileId1, luFeatures); + const luFile1 = parse(fileId1, fileContent, luFeatures, []); const updatedLuFile = updateIntent(luFile1, intentName, intent, luFeatures); const luresource = updatedLuFile.resource; @@ -186,7 +233,7 @@ hi it('update section with only name', () => { const intentName = 'CheckEmail'; - const luFile1 = luIndexer.parse(fileContent, fileId1, luFeatures); + const luFile1 = parse(fileId1, fileContent, luFeatures, []); const updatedLuFile = updateIntent(luFile1, intentName, { Name: 'CheckEmail1' }, luFeatures); const luresource = updatedLuFile.resource; @@ -208,7 +255,7 @@ hi - show my emails 2 - check my mail box please`; - const luFile1 = luIndexer.parse(fileContent, fileId1, luFeatures); + const luFile1 = parse(fileId1, fileContent, luFeatures, []); const updatedLuFile = updateIntent(luFile1, intentName, { Body: updatedBody }, luFeatures); const luresource = updatedLuFile.resource; @@ -227,7 +274,7 @@ hi it('update section with empty, should perform a remove', () => { const intentName = 'CheckEmail'; - const luFile1 = luIndexer.parse(fileContent, fileId1, luFeatures); + const luFile1 = parse(fileId1, fileContent, luFeatures, []); const updatedLuFile = updateIntent(luFile1, intentName, null, luFeatures); const luresource = updatedLuFile.resource; @@ -258,7 +305,7 @@ hi `, }; - const luFile1 = luIndexer.parse(validFileContent, 'a.lu', luFeatures); + const luFile1 = parse('a.lu', validFileContent, luFeatures, []); // when intent invalid, after update can still be parsed const updatedContent2 = updateIntent(luFile1, intentName, invalidIntent, luFeatures).content; @@ -266,7 +313,7 @@ hi expect(updatedContent2Parsed.Sections.length).toEqual(1); expect(updatedContent2Parsed.Errors.length).toBeGreaterThan(0); // when file invalid, update with valid intent should fix error. - const luFile2 = luIndexer.parse(updatedContent2, 'a.lu', luFeatures); + const luFile2 = parse('a.lu', updatedContent2, luFeatures, []); const updatedContent3 = updateIntent(luFile2, intentName, validIntent, luFeatures).content; const updatedContent3Parsed = luParser.parse(updatedContent3); @@ -287,7 +334,7 @@ hi - show my emails @`, }; - const luFile1 = luIndexer.parse(validFileContent, 'a.lu', luFeatures); + const luFile1 = parse('a.lu', validFileContent, luFeatures, []); // when intent invalid, after update can still be parsed const updatedContent2 = updateIntent(luFile1, intentName, invalidIntent, luFeatures).content; @@ -313,7 +360,7 @@ hi # UnexpectedIntentDefination `, }; - const luFile1 = luIndexer.parse(validFileContent, 'a.lu', luFeatures); + const luFile1 = parse('a.lu', validFileContent, luFeatures, []); // should auto escape # to \# const updatedContent2 = updateIntent(luFile1, intentName, invalidIntent, luFeatures).content; @@ -328,7 +375,7 @@ hi it('delete section test', () => { const intentName = 'CheckEmail'; - const luFile1 = luIndexer.parse(fileContent, fileId1, luFeatures); + const luFile1 = parse(fileId1, fileContent, luFeatures, []); const fileContentUpdated = removeIntent(luFile1, intentName, luFeatures).content; const luresource = luParser.parse(fileContentUpdated); @@ -372,7 +419,7 @@ describe('LU Nested Section CRUD test', () => { `, }; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = updateIntent(luFile1, intentName, intent, luFeatures); const result = luParser.parse(luFile1Updated.content); const { Sections, Errors } = result; @@ -415,7 +462,7 @@ describe('LU Nested Section CRUD test', () => { `, }; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = addIntent(luFile1, intent, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -447,7 +494,7 @@ describe('LU Nested Section CRUD test', () => { `, }; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = addIntent(luFile1, intent, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -482,7 +529,7 @@ describe('LU Nested Section CRUD test', () => { `, }; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = updateIntent(luFile1, intentName, intent, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -521,7 +568,7 @@ describe('LU Nested Section CRUD test', () => { `, }; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = updateIntent(luFile1, intentName, intent, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -553,7 +600,7 @@ describe('LU Nested Section CRUD test', () => { ### Oops `; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated1 = updateIntent(luFile1, intentName, { Name: intentName, Body: intentBody1 }, luFeatures); const luresource1 = luParser.parse(luFile1Updated1.content); @@ -565,7 +612,7 @@ describe('LU Nested Section CRUD test', () => { `; const luFile1Updated2 = updateIntent(luFile1, intentName, { Name: intentName, Body: intentBody2 }, luFeatures); const luresource2 = luParser.parse(luFile1Updated2.content); - expect(luresource2.Sections.length).toEqual(2); + expect(luresource2.Sections.length).toEqual(3); expect(luresource2.Errors.length).toBeGreaterThan(0); expect(luresource2.Sections[0].SectionType).toEqual(luSectionTypes.MODELINFOSECTION); expect(luresource2.Sections[1].SectionType).toEqual(luSectionTypes.NESTEDINTENTSECTION); @@ -580,7 +627,7 @@ describe('LU Nested Section CRUD test', () => { const intentBody3 = `## Oops ### Oops `; - const luFile2 = luIndexer.parse(fileContent3, fileId, luFeatures); + const luFile2 = parse(fileId, fileContent3, luFeatures, []); const luFile1Updated3 = updateIntent(luFile2, intentName, { Name: intentName, Body: intentBody3 }, luFeatures); const luresource3 = luParser.parse(luFile1Updated3.content); expect(luresource3.Sections.length).toBeGreaterThan(0); @@ -602,7 +649,7 @@ describe('LU Nested Section CRUD test', () => { `, }; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = updateIntent(luFile1, intentName, intent, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -631,7 +678,7 @@ describe('LU Nested Section CRUD test', () => { it('delete nestedIntentSection test', () => { const Name = 'CheckTodo/CheckUnreadTodo'; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = removeIntent(luFile1, Name, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -654,7 +701,7 @@ describe('LU Nested Section CRUD test', () => { it('delete nestedIntentSection test, parrent not exist', () => { const Name = 'CheckTodoNotExist/CheckUnreadTodo'; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = removeIntent(luFile1, Name, luFeatures); const luresource = luParser.parse(luFile1Updated.content); @@ -664,7 +711,7 @@ describe('LU Nested Section CRUD test', () => { it('delete nestedIntentSection test, child not exist', () => { const Name = 'CheckTodo/CheckUnreadTodoNotExist'; - const luFile1 = luIndexer.parse(fileContent, fileId, luFeatures); + const luFile1 = parse(fileId, fileContent, luFeatures, []); const luFile1Updated = removeIntent(luFile1, Name, luFeatures); const luresource = luParser.parse(luFile1Updated.content); diff --git a/Composer/packages/lib/indexers/src/luIndexer.ts b/Composer/packages/lib/indexers/src/luIndexer.ts index ef605db9f7..8d7af6ad7a 100644 --- a/Composer/packages/lib/indexers/src/luIndexer.ts +++ b/Composer/packages/lib/indexers/src/luIndexer.ts @@ -1,17 +1,27 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { FileInfo, LuFile, ILUFeaturesConfig } from '@bfc/shared'; +import { FileInfo, LuFile, ILUFeaturesConfig, LUImportResolverDelegate } from '@bfc/shared'; +import { sectionHandler } from '@microsoft/bf-lu/lib/parser/composerindex'; +import merge from 'lodash/merge'; import { getBaseName } from './utils/help'; import { FileExtensions } from './utils/fileExtensions'; -import * as luUtil from './utils/luUtil'; - -function parse(content: string, id = '', config: ILUFeaturesConfig): LuFile { - return luUtil.parse(id, content, config); +import { convertLuParseResultToLuFile, defaultLUFeatures } from './utils/luUtil'; +const { luParser } = sectionHandler; + +function parse( + content: string, + id = '', + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate +): LuFile { + const appliedConfig = merge(defaultLUFeatures, luFeatures); + const result = luParser.parse(content); + return convertLuParseResultToLuFile(id, result, appliedConfig, importResolver); } -function index(files: FileInfo[], config: ILUFeaturesConfig): LuFile[] { +function index(files: FileInfo[], config: ILUFeaturesConfig, importResolver?: LUImportResolverDelegate): LuFile[] { if (files.length === 0) return []; const filtered = files.filter((file) => file.name.endsWith(FileExtensions.Lu)); @@ -19,7 +29,7 @@ function index(files: FileInfo[], config: ILUFeaturesConfig): LuFile[] { const luFiles = filtered.map((file) => { const { name, content } = file; const id = getBaseName(name); - return parse(content, id, config); + return parse(content, id, config, importResolver); }); return luFiles; diff --git a/Composer/packages/lib/indexers/src/utils/luUtil.ts b/Composer/packages/lib/indexers/src/utils/luUtil.ts index d9db779482..a41ca5ec3e 100644 --- a/Composer/packages/lib/indexers/src/utils/luUtil.ts +++ b/Composer/packages/lib/indexers/src/utils/luUtil.ts @@ -11,6 +11,7 @@ import { sectionHandler, parser as BFLUParser } from '@microsoft/bf-lu/lib/parse import isEmpty from 'lodash/isEmpty'; import get from 'lodash/get'; import merge from 'lodash/merge'; +import clone from 'lodash/clone'; import { LuFile, LuSectionTypes, @@ -21,9 +22,14 @@ import { DiagnosticSeverity, ILUFeaturesConfig, LuParseResource, + TextFile, + luImportResolverGenerator, + LUImportResolverDelegate, } from '@bfc/shared'; import formatMessage from 'format-message'; +import { luIndexer } from '../luIndexer'; + import { buildNewlineText, getFileName, splitNewlineText } from './help'; import { SectionTypes } from './qnaUtil'; @@ -67,11 +73,13 @@ export function convertLuDiagnostic(d: any, source: string, offset = 0): Diagnos export function convertLuParseResultToLuFile( id: string, resource: LuParseResource, - luFeatures: ILUFeaturesConfig + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate ): LuFile { // filter structured-object from LUParser result. const { Sections, Errors, Content } = resource; const intents: LuIntentSection[] = []; + const fileId = id; Sections.forEach((section) => { const { Name, Body, SectionType } = section; const range = new Range( @@ -80,7 +88,7 @@ export function convertLuParseResultToLuFile( ); if (SectionType === LuSectionTypes.SIMPLEINTENTSECTION) { const Entities = section.Entities.map(({ Name, Type }) => ({ Name, Type })); - intents.push({ Name, Body, Entities, range }); + intents.push({ Name, Body, Entities, range, fileId }); } else if (SectionType === LuSectionTypes.NESTEDINTENTSECTION) { const Children = section.SimpleIntentSections.map((subSection) => { const { Name, Body } = subSection; @@ -89,9 +97,9 @@ export function convertLuParseResultToLuFile( new Position(get(section, 'Range.End.Line', 0), get(subSection, 'Range.End.Character', 0)) ); const Entities = subSection.Entities.map(({ Name, Type }) => ({ Name, Type })); - return { Name, Body, Entities, range }; + return { Name, Body, Entities, range, fileId }; }); - intents.push({ Name, Body, Children, range }); + intents.push({ Name, Body, Children, range, fileId }); intents.push( ...Children.map((subSection) => { return { @@ -120,12 +128,38 @@ export function convertLuParseResultToLuFile( } ); - const diagnostics = syntaxDiagnostics.concat(semanticDiagnostics); + // find all reference and parse them. + const allIntents: LuIntentSection[] = clone(intents); + const referenceDiagnostics: Diagnostic[] = []; + if (importResolver) { + Sections.filter(({ SectionType }) => SectionType === SectionTypes.ImportSection).forEach((item) => { + try { + const targetFile = importResolver(id, item.Path); + if (targetFile) { + const targetFileParsed = luIndexer.parse(targetFile.content, targetFile.id, luFeatures, importResolver); + allIntents.push(...targetFileParsed.allIntents); + } + } catch (_error) { + const res = new Diagnostic(_error.message, item.Path, DiagnosticSeverity.Error, item.Path); + const start: Position = item.Range + ? new Position(item.Range.Start.Line, item.Range.Start.Character) + : new Position(0, 0); + const end: Position = item.Range + ? new Position(item.Range.End.Line, item.Range.End.Character) + : new Position(0, 0); + res.range = new Range(start, end); + referenceDiagnostics.push(res); + } + }); + } + + const diagnostics = syntaxDiagnostics.concat(semanticDiagnostics).concat(referenceDiagnostics); return { id, content: Content, empty: !Sections.length, intents, + allIntents, diagnostics, imports, resource: { Sections, Errors, Content }, @@ -222,7 +256,8 @@ export function updateIntent( luFile: LuFile, intentName: string, intent: { Name?: string; Body?: string } | null, - luFeatures: ILUFeaturesConfig + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate ): LuFile { let targetSection; let targetSectionContent; @@ -244,7 +279,7 @@ export function updateIntent( const targetSection = Sections.find(({ Name }) => Name === intentName); if (targetSection) { const result = new sectionOperator(resource).deleteSection(targetSection.Id); - return convertLuParseResultToLuFile(id, result, luFeatures); + return convertLuParseResultToLuFile(id, result, luFeatures, importResolver); } return luFile; } @@ -281,7 +316,7 @@ export function updateIntent( } else { newResource = new sectionOperator(resource).addSection(['', targetSectionContent].join(NEWLINE)); } - return convertLuParseResultToLuFile(id, newResource, luFeatures); + return convertLuParseResultToLuFile(id, newResource, luFeatures, importResolver); } /** @@ -289,20 +324,30 @@ export function updateIntent( * @param content origin lu file content * @param {Name, Body} intent the adds. Name support subSection naming 'CheckEmail/CheckUnreadEmail', if #CheckEmail not exist will do recursive add. */ -export function addIntent(luFile: LuFile, { Name, Body }: LuIntentSection, luFeatures: ILUFeaturesConfig): LuFile { +export function addIntent( + luFile: LuFile, + { Name, Body }: LuIntentSection, + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate +): LuFile { const intentName = Name; if (Name.includes('/')) { const [, childName] = Name.split('/'); Name = childName; } // If the invoker doesn't want to carry Entities, don't pass Entities in. - return updateIntent(luFile, intentName, { Name, Body }, luFeatures); + return updateIntent(luFile, intentName, { Name, Body }, luFeatures, importResolver); } -export function addIntents(luFile: LuFile, intents: LuIntentSection[], luFeatures: ILUFeaturesConfig): LuFile { +export function addIntents( + luFile: LuFile, + intents: LuIntentSection[], + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate +): LuFile { let result = luFile; for (const intent of intents) { - result = addIntent(result, intent, luFeatures); + result = addIntent(result, intent, luFeatures, importResolver); } return result; } @@ -312,21 +357,30 @@ export function addIntents(luFile: LuFile, intents: LuIntentSection[], luFeature * @param content origin lu file content * @param intentName the remove intentName. Name support subSection naming 'CheckEmail/CheckUnreadEmail', if any of them not exist will do nothing. */ -export function removeIntent(luFile: LuFile, intentName: string, luFeatures: ILUFeaturesConfig): LuFile { - return updateIntent(luFile, intentName, null, luFeatures); +export function removeIntent( + luFile: LuFile, + intentName: string, + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate +): LuFile { + return updateIntent(luFile, intentName, null, luFeatures, importResolver); } -export function removeIntents(luFile: LuFile, intentNames: string[], luFeatures: ILUFeaturesConfig): LuFile { +export function removeIntents( + luFile: LuFile, + intentNames: string[], + luFeatures: ILUFeaturesConfig, + importResolver?: LUImportResolverDelegate +): LuFile { let result = luFile; for (const intentName of intentNames) { - result = removeIntent(result, intentName, luFeatures); + result = removeIntent(result, intentName, luFeatures, importResolver); } return result; } -export function parse(id: string, content: string, luFeatures: ILUFeaturesConfig): LuFile { - const appliedConfig = merge(defaultLUFeatures, luFeatures || {}); - const result = luParser.parse(content); - return convertLuParseResultToLuFile(id, result, appliedConfig); +export function parse(id: string, content: string, luFeatures: ILUFeaturesConfig, luFiles: TextFile[]): LuFile { + const luImportResolver = luImportResolverGenerator(luFiles, '.lu'); + return luIndexer.parse(content, id, luFeatures, luImportResolver); } export async function semanticValidate( diff --git a/Composer/packages/lib/shared/src/resolverFactory.ts b/Composer/packages/lib/shared/src/resolverFactory.ts index d903d61c46..941dc7e859 100644 --- a/Composer/packages/lib/shared/src/resolverFactory.ts +++ b/Composer/packages/lib/shared/src/resolverFactory.ts @@ -47,7 +47,7 @@ export function luImportResolverGenerator( if (!targetFile) throw new Error(formatMessage(`File not found`)); return { - id: resourceId, + id: targetFile.id, content: targetFile.content, }; }; diff --git a/Composer/packages/types/src/indexers.ts b/Composer/packages/types/src/indexers.ts index 9a4f16948b..a16b05b7b1 100644 --- a/Composer/packages/types/src/indexers.ts +++ b/Composer/packages/types/src/indexers.ts @@ -89,6 +89,7 @@ export type LuIntentSection = { Entities?: LuEntity[]; Children?: LuIntentSection[]; range?: IRange; + fileId?: string; }; export type LuParsed = { @@ -113,6 +114,7 @@ export type LuFile = { content: string; diagnostics: IDiagnostic[]; intents: LuIntentSection[]; + allIntents: LuIntentSection[]; empty: boolean; resource: LuParseResource; imports: { id: string; path: string; description: string }[]; diff --git a/Composer/packages/ui-plugins/luis/src/LuisIntentEditor.tsx b/Composer/packages/ui-plugins/luis/src/LuisIntentEditor.tsx index 96a27f5486..84fc26de96 100644 --- a/Composer/packages/ui-plugins/luis/src/LuisIntentEditor.tsx +++ b/Composer/packages/ui-plugins/luis/src/LuisIntentEditor.tsx @@ -31,14 +31,26 @@ const LuisIntentEditor: React.FC> = (props) => { } const luIntent = useMemo(() => { - return ( - luFile?.intents.find((intent) => intent.Name === intentName) || - ({ - Name: intentName, - Body: '', - } as LuIntentSection) - ); - }, [intentName]); + /** + * if intent is referenced from imported files, use origin intent. + * because update on origin file won't reparse current file, so the `allIntent` may out of date. + */ + const intentInCurrentFile = luFile?.allIntents.find((intent) => intent.Name === intentName); + if (intentInCurrentFile) { + if (intentInCurrentFile.fileId === luFile?.id) { + return intentInCurrentFile; + } else { + const intentInOriginFile = luFiles + .find(({ id }) => id === intentInCurrentFile.fileId) + ?.intents?.find((intent) => intent.Name === intentName); + if (intentInOriginFile) return intentInOriginFile; + } + } + return { + Name: intentName, + Body: '', + } as LuIntentSection; + }, [intentName, luFiles]); const navigateToLuPage = useCallback( (luFileId: string, sectionId?: string) => { @@ -63,7 +75,9 @@ const LuisIntentEditor: React.FC> = (props) => { } const newIntent = { Name: intentName, Body: newValue }; - shellApi.debouncedUpdateLuIntent(luFile.id, intentName, newIntent)?.then(shellApi.commitChanges); + shellApi + .debouncedUpdateLuIntent(luIntent?.fileId ?? luFile.id, intentName, newIntent) + ?.then(shellApi.commitChanges); onChange(intentName); };