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

refactor: centralize lg parsing logic to 'shared' lib #1663

Merged
merged 46 commits into from
Dec 2, 2019
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
7088206
implement basic lg apis
yeze322 Nov 26, 2019
e975f16
expose them on 'shared' level
yeze322 Nov 26, 2019
531f317
UT + fix
yeze322 Nov 26, 2019
9e7c541
add static `parse()` method to LgTemplateRef and LgMetaData
yeze322 Nov 26, 2019
b55f845
loose restriction on lg format
yeze322 Nov 26, 2019
aae1d7c
migrate: lgUtils
yeze322 Nov 26, 2019
13ce6b3
migrate: visual editor - hooks
yeze322 Nov 26, 2019
1e6b5ea
move 'extractLgTemplateRefs' inside 'shared' lib
yeze322 Nov 26, 2019
ee68dde
migrate: dialogIndexer
yeze322 Nov 26, 2019
236b2e9
migrate: LgEditorWidget
yeze322 Nov 26, 2019
6049ebc
migrate: ObiEditor
yeze322 Nov 26, 2019
bd7390b
let shared lib built before others
yeze322 Nov 26, 2019
4a1c52c
rename: patterns.ts -> lgPatterns.ts
yeze322 Nov 27, 2019
c5cfb59
fix ut: use the lg pattern in indexer file
yeze322 Nov 27, 2019
e3d30e6
todo: mark up next pr's todo
yeze322 Nov 27, 2019
3291366
Merge branch 'master' into lg/organize
yeze322 Nov 27, 2019
456d08f
comments: explain lg strings
yeze322 Nov 27, 2019
e3a9b69
enhance ut of parseLgTemplateRef
yeze322 Nov 27, 2019
c2a69c7
do not export parsers
yeze322 Nov 27, 2019
1353a50
clarify the concept of 'LgText'
yeze322 Nov 27, 2019
f8b4d66
implement LgField with UT
yeze322 Nov 27, 2019
8254870
add two judement method in LgField
yeze322 Nov 27, 2019
490cc65
remove 'toLgText' method in LgMetaData
yeze322 Nov 27, 2019
fb991d3
purify 'LgMetaData'
yeze322 Nov 28, 2019
06a591d
update LgMetaData usages
yeze322 Nov 28, 2019
b7daa72
update stringTypes
yeze322 Nov 28, 2019
9c31854
remove LgFiled (over abstracted)
yeze322 Nov 28, 2019
02d4c15
fix the usage of LgMetaData
yeze322 Nov 28, 2019
8eb4f40
rename param name to lgTemplateName
yeze322 Nov 28, 2019
67e091a
refactor: apply LgTemplateRef to LgEditorWidget
yeze322 Nov 28, 2019
67c064f
set LgRef default parameters to '[]'
yeze322 Nov 28, 2019
31d6c99
refactor: simplify 'getInitialTemplate'
yeze322 Nov 28, 2019
48b910b
enrich LgTempateRef test cases
yeze322 Nov 28, 2019
284e8ce
folder structure
yeze322 Nov 28, 2019
216d413
take out string logic into string builders
yeze322 Nov 28, 2019
d4738e6
clean: remove parseLgText since it's not referenced
yeze322 Nov 28, 2019
644ae1c
extractLgTemplateNames => extractLgTemplateRefs
yeze322 Nov 28, 2019
fc22977
fix a negative case in 'parseLgTemplateRef'
yeze322 Nov 28, 2019
cd0aa32
merge extractLgTemplateRefs into 'parseLgTemplateRef'
yeze322 Nov 28, 2019
6d1648b
edit comments
yeze322 Nov 28, 2019
0bda795
extract common code from parseLgTemplateRef
yeze322 Nov 28, 2019
c7a289f
clean
yeze322 Nov 28, 2019
17f0929
add a single whitespace aftter '-' in LgEditorWIdget
yeze322 Nov 28, 2019
5aa6c5f
Merge branch 'master' of https://github.com/Microsoft/BotFramework-Co…
yeze322 Nov 29, 2019
bd9930e
Merge branch 'master' into lg/organize
yeze322 Dec 2, 2019
7926a22
Merge branch 'master' into lg/organize
cwhitten Dec 2, 2019
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
19 changes: 0 additions & 19 deletions Composer/packages/client/src/utils/lgUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,25 +129,6 @@ export function getTemplate(content: string, templateName: string): LGTemplate |
return resource.Templates.find(t => t.Name === templateName);
}

/**
*
* @param text string
* -[Greeting], I'm a fancy bot, [Bye] ---> ['Greeting', 'Bye']
*
*/
export function extractTemplateNames(text: string): string[] {
const templateNames: string[] = [];
// match a template name match a temlate func e.g. `showDate()`
// eslint-disable-next-line security/detect-unsafe-regex
const reg = /\[([A-Za-z_][-\w]+)(\(.*\))?\]/g;
let matchResult;
while ((matchResult = reg.exec(text)) !== null) {
const templateName = matchResult[1];
templateNames.push(templateName);
}
return templateNames;
}

export function removeTemplate(content: string, templateName: string): string {
const resource = LGParser.parse(content);
return resource.deleteTemplate(templateName).toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ function ItemActions<T extends MicrosoftIDialog>(props: ItemActionsProps<T>) {
const item = formData[index];
// @ts-ignore
if (item.$type === 'Microsoft.SendActivity' && item.activity && item.activity.indexOf('bfdactivity-') !== -1) {
// TODO: (ze) 'removeLgTemplate' -> 'removeLgTemplateRef', it should accept inputs like '[bfdactivity-1234]'
// @ts-ignore
formContext.shellApi.removeLgTemplate('common', item.activity.slice(1, item.activity.length - 1));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import React, { useState, useMemo, useLayoutEffect } from 'react';
import { LgEditor } from '@bfc/code-editor';
import { LgMetaData } from '@bfc/shared';
import debounce from 'lodash/debounce';
import * as monacoEditor from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api';

Expand Down Expand Up @@ -35,7 +36,7 @@ export const LgEditorWidget: React.FC<LgEditorWidgetProps> = props => {
const { formContext, name, value, height = 250 } = props;
const [errorMsg, setErrorMsg] = useState('');
const [editor, setEditor] = useState<monacoEditor.editor.IStandaloneCodeEditor>();
const lgId = `bfd${name}-${formContext.dialogId}`;
const lgId = new LgMetaData(name, formContext.dialogId || '').toLgTemplateName();
const lgFileId = formContext.currentDialog.lgFile || 'common';
const lgFile = formContext.lgFiles.find(file => file.id === lgFileId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { jsx } from '@emotion/core';
import { useContext, FC, useEffect, useState, useRef } from 'react';
import { MarqueeSelection, Selection } from 'office-ui-fabric-react/lib/MarqueeSelection';
import { deleteAction, deleteActions } from '@bfc/shared';
import { deleteAction, deleteActions, LgTemplateRef, LgMetaData } from '@bfc/shared';

import { NodeEventTypes } from '../constants/NodeEventTypes';
import { KeyboardCommandTypes, KeyboardPrimaryTypes } from '../constants/KeyboardCommandTypes';
Expand Down Expand Up @@ -49,12 +49,10 @@ export const ObiEditor: FC<ObiEditorProps> = ({
);

const deleteLgTemplates = (lgTemplates: string[]) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteLgTemplates will be defined as deleteLgText. See my next comments on copyLgTemplate

const lgPattern = /\[(bfd\w+-\d+)\]/;
const normalizedLgTemplates = lgTemplates
.map(x => {
const matches = lgPattern.exec(x);
if (matches && matches.length === 2) return matches[1];
return '';
const lgTemplateRef = LgTemplateRef.parse(x);
return lgTemplateRef ? lgTemplateRef.name : '';
Copy link
Contributor Author

@yeze322 yeze322 Nov 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leverage the 'optional chaining' feature in ts 3.7. LgTemplateRef.parse(x)?.name

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

})
.filter(x => !!x);
return removeLgTemplates('common', normalizedLgTemplates);
Expand Down Expand Up @@ -89,16 +87,19 @@ export const ObiEditor: FC<ObiEditorProps> = ({
if (eventData.$type === 'PASTE') {
handler = e => {
// TODO: clean this along with node deletion.
const copyLgTemplateToNewNode = async (lgTemplateName: string, newNodeId: string) => {
const matches = /\[(bfd\w+-(\d+))\]/.exec(lgTemplateName);
if (Array.isArray(matches) && matches.length === 3) {
const originLgId = matches[1];
const originNodeId = matches[2];
const newLgId = originLgId.replace(originNodeId, newNodeId);
await copyLgTemplate('common', originLgId, newLgId);
return `[${newLgId}]`;
}
return lgTemplateName;
const copyLgTemplateToNewNode = async (input: string, newNodeId: string) => {
const lgTemplateRef = LgTemplateRef.parse(input);
if (!lgTemplateRef) return input;

const lgMetadata = LgMetaData.parse(lgTemplateRef.name);
if (!lgMetadata) return input;

lgMetadata.designerId = newNodeId;
const newLgName = lgMetadata.toLgTemplateName();
const newLgTemplateRefString = lgMetadata.toLgTemplateRefString();

await copyLgTemplate('common', lgTemplateRef.name, newLgName);
Copy link
Contributor Author

@yeze322 yeze322 Nov 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyLgTemplate will be defined as copyLgText in my following PR. Targets to hide LG concepts from visual editor (and Form Editor), will inject a synthesized lg api from Shell level.

return newLgTemplateRefString;
};
pasteNodes(data, e.id, e.position, clipboardActions, copyLgTemplateToNewNode).then(dialog => {
onChange(dialog);
Expand Down
21 changes: 3 additions & 18 deletions Composer/packages/extensions/visual-designer/src/utils/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,18 @@

import { useContext, useState, useEffect, useRef } from 'react';
import debounce from 'lodash/debounce';
import { LgTemplateRef } from '@bfc/shared';

import { NodeRendererContext } from '../store/NodeRendererContext';

// matches [bfd<someName>-123456]
const TEMPLATE_PATTERN = /^\[(bfd.+-\d{6})\]$/;

const getTemplateId = (str?: string): string | null => {
if (!str) {
return null;
}

const match = TEMPLATE_PATTERN.exec(str);

if (!match || !match[1]) {
return null;
}

return match[1];
};

export const useLgTemplate = (str?: string, dialogId?: string) => {
const { getLgTemplates } = useContext(NodeRendererContext);
const [templateText, setTemplateText] = useState('');
let cancelled = false;

const updateTemplateText = async () => {
const templateId = getTemplateId(str);
const lgTemplateRef = LgTemplateRef.parse(str || '');
const templateId = lgTemplateRef ? lgTemplateRef.name : '';

if (templateId && dialogId) {
// this is an LG template, go get it's content
Expand Down
1 change: 1 addition & 0 deletions Composer/packages/lib/indexers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"ts-jest": "^24.1.0"
},
"dependencies": {
"@bfc/shared": "*",
"botbuilder-expression-parser": "^4.5.11",
"botbuilder-lg": "https://botbuilder.myget.org/F/botbuilder-declarative/npm/botbuilder-lg/-/4.7.0-preview2.tgz",
"lodash": "^4.17.15",
Expand Down
10 changes: 2 additions & 8 deletions Composer/packages/lib/indexers/src/dialogIndexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import has from 'lodash/has';
import uniq from 'lodash/uniq';
import { extractLgTemplateNames } from '@bfc/shared';

import { ITrigger, DialogInfo, FileInfo } from './type';
import { DialogChecker } from './utils/dialogChecker';
Expand Down Expand Up @@ -37,14 +38,7 @@ function ExtractLgTemplates(dialog): string[] {
return true;
}
targets.forEach(target => {
// match a template name match a temlate func e.g. `showDate()`
// eslint-disable-next-line security/detect-unsafe-regex
const reg = /\[([A-Za-z_][-\w]+)(\(.*\))?\]/g;
let matchResult;
while ((matchResult = reg.exec(target)) !== null) {
const templateName = matchResult[1];
templates.push(templateName);
}
templates.push(...extractLgTemplateNames(target));
});
}
return false;
Expand Down
2 changes: 1 addition & 1 deletion Composer/packages/lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"build:code-editor": "cd code-editor && yarn build",
"build:shared": "cd shared && yarn build",
"build:indexers": "cd indexers && yarn build",
"build:all": "concurrently --kill-others-on-fail \"yarn:build:code-editor\" \"yarn:build:shared\" \"yarn:build:indexers\""
"build:all": "yarn build:shared && concurrently --kill-others-on-fail \"yarn:build:code-editor\" \"yarn:build:indexers\""
boydc2014 marked this conversation as resolved.
Show resolved Hide resolved
},
"author": "",
"license": "ISC"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { extractLgTemplateNames } from '../../src';

describe('extractLgTemplateRefs', () => {
it('can extract lg refs from input string', () => {
expect(extractLgTemplateNames('Hi')).toEqual([]);
expect(extractLgTemplateNames(`-[Greeting], I'm a fancy bot, [Bye]`)).toEqual(['Greeting', 'Bye']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { LgMetaData } from '../../../src';

describe('LgMetaData', () => {
it('can construct an instance via constructor', () => {
const instance = new LgMetaData('activity', '123456');

expect(instance.type).toEqual('activity');
expect(instance.designerId).toEqual('123456');

expect(instance.toLgTemplateName).toBeDefined();
expect(instance.toLgTemplateRef).toBeDefined();
expect(instance.toLgTemplateRefString).toBeDefined();
expect(instance.toLgText).toBeDefined();
});

it('can generate correct output strings', () => {
const instance = new LgMetaData('activity', '123456');

expect(instance.toLgTemplateName()).toEqual('bfdactivity-123456');

expect(instance.toLgTemplateRefString()).toEqual('[bfdactivity-123456]');
expect(instance.toLgTemplateRefString(['1', '2'])).toEqual('[bfdactivity-123456(1,2)]');

expect(instance.toLgText()).toEqual('- [bfdactivity-123456]');
expect(instance.toLgText([])).toEqual('- [bfdactivity-123456()]');
});

it('can construct instance via `parse()` method', () => {
expect(LgMetaData.parse('bfdactivity-123456')).toBeInstanceOf(LgMetaData);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { LgTemplateRef } from '../../../src';

describe('LgTemplateRef', () => {
it('can construct an instance via constructor', () => {
const a = new LgTemplateRef('a', undefined);
expect(a.name).toEqual('a');
expect(a.parameters).toEqual(undefined);

const b = new LgTemplateRef('b', []);
expect(b.name).toEqual('b');
expect(b.parameters).toEqual([]);

const c = new LgTemplateRef('c', ['1', '2']);
expect(c.name).toEqual('c');
expect(c.parameters).toEqual(['1', '2']);
});

it('can output correct strings', () => {
const a = new LgTemplateRef('a', undefined);
expect(a.toString()).toEqual('[a]');
expect(a.toLgText()).toEqual('- [a]');

const b = new LgTemplateRef('b', []);
expect(b.toString()).toEqual('[b()]');
expect(b.toLgText()).toEqual('- [b()]');

const c = new LgTemplateRef('c', ['1', '2']);
expect(c.toString()).toEqual('[c(1,2)]');
expect(c.toLgText()).toEqual('- [c(1,2)]');
});

it('can construct instance via `parse()`', () => {
expect(LgTemplateRef.parse('[bfdactivity-123456]')).toBeInstanceOf(LgTemplateRef);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import parseLgParamString from '../../../src/lgUtils/parsers/parseLgParamString';

describe('parseLgParamString', () => {
it('should return undefined when no params detected', () => {
expect(parseLgParamString('')).toBeUndefined();
expect(parseLgParamString('xxx')).toBeUndefined();
});

it('should return params array when input valid strings', () => {
expect(parseLgParamString('()')).toEqual([]);
expect(parseLgParamString('(a,b)')).toEqual(['a', 'b']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { parseLgTemplateName, LgMetaData } from '../../../src';

describe('parseLgTemplateName', () => {
it('should return null when inputs are invalid', () => {
expect(parseLgTemplateName('')).toEqual(null);
expect(parseLgTemplateName('xxx')).toEqual(null);
});

it('should return LgMetaData when inputs are valid', () => {
const result = parseLgTemplateName('bfdactivity-123456');
expect(result).toBeInstanceOf(LgMetaData);
expect((result as LgMetaData).designerId).toEqual('123456');
expect((result as LgMetaData).type).toEqual('activity');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { parseLgTemplateRef, LgTemplateRef } from '../../../src';

describe('parseLgTemplateRef', () => {
it('should return null when inputs are invalid', () => {
expect(parseLgTemplateRef('')).toEqual(null);
yeze322 marked this conversation as resolved.
Show resolved Hide resolved
expect(parseLgTemplateRef('xxx')).toEqual(null);
expect(parseLgTemplateRef('[0]')).toEqual(null);
});

it('should return LgTemplateRef when inputs are valid', () => {
const a = parseLgTemplateRef('[bfdactivity-123456]');
expect(a).toBeInstanceOf(LgTemplateRef);
expect((a as LgTemplateRef).name).toEqual('bfdactivity-123456');
expect((a as LgTemplateRef).parameters).toEqual(undefined);

const b = parseLgTemplateRef('[greeting(1,2)]');
expect(b).toBeInstanceOf(LgTemplateRef);
expect((b as LgTemplateRef).name).toEqual('greeting');
expect((b as LgTemplateRef).parameters).toEqual(['1', '2']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { parseLgText, LgTemplateRef } from '../../../src';

describe('parseLgText', () => {
it('should return null when inputs are invalid', () => {
expect(parseLgText('')).toEqual(null);
expect(parseLgText('xxx')).toEqual(null);
expect(parseLgText('[bfdactivity-1234]')).toEqual(null);
});

it('should return LgTemplateRef when inputs are valid', () => {
const result = parseLgText('- [bfdactivity-123456]');
expect(result).toBeInstanceOf(LgTemplateRef);
expect((result as LgTemplateRef).name).toEqual('bfdactivity-123456');
expect((result as LgTemplateRef).parameters).toEqual(undefined);
});
});
1 change: 1 addition & 0 deletions Composer/packages/lib/shared/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ export * from './promptTabs';
export * from './appschema';
export * from './types';
export * from './constant';
export * from './lgUtils';
25 changes: 25 additions & 0 deletions Composer/packages/lib/shared/src/lgUtils/extractLgTemplateNames.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { LgTemplateRefPattern } from './lgPatterns';
import { LgTemplateName } from './models/stringTypes';

/**
*
* @param text string
* -[Greeting], I'm a fancy bot, [Bye] ---> ['Greeting', 'Bye']
*
*/
export default function extractLgTemplateNames(text: string): LgTemplateName[] {
const templateNames: string[] = [];

// eslint-disable-next-line security/detect-non-literal-regexp
const reg = new RegExp(LgTemplateRefPattern, 'g');

let matchResult;
while ((matchResult = reg.exec(text)) !== null) {
const templateName = matchResult[1];
templateNames.push(templateName);
}
return templateNames;
}
13 changes: 13 additions & 0 deletions Composer/packages/lib/shared/src/lgUtils/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

/** models */
export { default as LgMetaData } from './models/LgMetaData';
export { default as LgTemplateRef } from './models/LgTemplateRef';

/** parsers */
export { default as parseLgTemplateName } from './parsers/parseLgTemplateName';
export { default as parseLgTemplateRef } from './parsers/parseLgTemplateRef';
export { default as parseLgText } from './parsers/parseLgText';

export { default as extractLgTemplateNames } from './extractLgTemplateNames';
Loading