From c3cfaad2dfdfdd4aa1a5feaf11c9fd8e4557d199 Mon Sep 17 00:00:00 2001 From: "Hongyang Du (hond)" Date: Fri, 5 Jun 2020 08:27:57 +0800 Subject: [PATCH] [PORT]Refine template loop detection to enable recursive functions (#2305) * Refine template loop detection to enable recursive functions * Reduce the recursion stack depth to pass MacOS test --- .../src/memory/simpleObjectMemory.ts | 5 ++-- .../botbuilder-lg/src/customizedMemory.ts | 17 +++++++++++- .../botbuilder-lg/src/evaluationTarget.ts | 27 ++++--------------- libraries/botbuilder-lg/src/evaluator.ts | 14 +++++----- libraries/botbuilder-lg/src/expander.ts | 12 ++++----- libraries/botbuilder-lg/tests/lg.test.js | 11 ++++++++ .../botbuilder-lg/tests/lgDiagnostic.test.js | 2 ++ .../testData/examples/RecursiveTemplate.lg | 20 ++++++++++++++ .../exceptionExamples/LoopDetected.lg | 3 +++ 9 files changed, 71 insertions(+), 40 deletions(-) create mode 100644 libraries/botbuilder-lg/tests/testData/examples/RecursiveTemplate.lg diff --git a/libraries/adaptive-expressions/src/memory/simpleObjectMemory.ts b/libraries/adaptive-expressions/src/memory/simpleObjectMemory.ts index b7d370986..858a56fbe 100644 --- a/libraries/adaptive-expressions/src/memory/simpleObjectMemory.ts +++ b/libraries/adaptive-expressions/src/memory/simpleObjectMemory.ts @@ -16,7 +16,6 @@ import { ExpressionFunctions } from '../expressionFunctions'; export class SimpleObjectMemory implements MemoryInterface { private memory: any = undefined; - private versionNumber: number = 0; public constructor(memory: any) { this.memory = memory; @@ -142,13 +141,13 @@ export class SimpleObjectMemory implements MemoryInterface { } } - this.versionNumber++; return; } public version(): string { - return this.versionNumber.toString(); + return this.toString(); } + public toString(): string { return JSON.stringify(this.memory, this.getCircularReplacer()); } diff --git a/libraries/botbuilder-lg/src/customizedMemory.ts b/libraries/botbuilder-lg/src/customizedMemory.ts index ac1b7c2a7..155414c8e 100644 --- a/libraries/botbuilder-lg/src/customizedMemory.ts +++ b/libraries/botbuilder-lg/src/customizedMemory.ts @@ -57,6 +57,21 @@ export class CustomizedMemory implements MemoryInterface { } public version(): string { - return '0'; + let result = ''; + if (this.globalMemory) { + const version = this.globalMemory.version(); + if (version) { + result = result.concat(version); + } + } + + if (this.localMemory) { + const localVersion = this.localMemory.version(); + if (localVersion) { + result = result.concat(localVersion); + } + } + + return result; } } \ No newline at end of file diff --git a/libraries/botbuilder-lg/src/evaluationTarget.ts b/libraries/botbuilder-lg/src/evaluationTarget.ts index b86fe0e3f..210978e31 100644 --- a/libraries/botbuilder-lg/src/evaluationTarget.ts +++ b/libraries/botbuilder-lg/src/evaluationTarget.ts @@ -6,7 +6,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { CustomizedMemory } from './customizedMemory'; +import { MemoryInterface } from 'adaptive-expressions'; /** @@ -22,13 +22,13 @@ export class EvaluationTarget { /** * Scope. */ - public scope: any; + public scope: MemoryInterface; /** * The children templates that this template has evaluated currently. */ public evaluatedChildren: Map; - public constructor(templateName: string, scope: any) { + public constructor(templateName: string, scope: MemoryInterface) { this.templateName = templateName; this.scope = scope; this.evaluatedChildren = new Map(); @@ -40,24 +40,7 @@ export class EvaluationTarget { * @returns Id. */ public getId(): string { - const memory = this.scope as CustomizedMemory; - let result = this.templateName; - if (memory) { - if (memory.globalMemory) { - const version = memory.globalMemory.version(); - if (version) { - result = result.concat(version); - } - } - - if (memory.localMemory) { - const localMemoryString = memory.localMemory.toString(); - if (localMemoryString) { - result = result.concat(localMemoryString); - } - } - } - - return result; + const scopeVersion = this.scope ? this.scope.version() : ''; + return this.templateName + scopeVersion; } } diff --git a/libraries/botbuilder-lg/src/evaluator.ts b/libraries/botbuilder-lg/src/evaluator.ts index 4796963bc..8b8f0b35e 100644 --- a/libraries/botbuilder-lg/src/evaluator.ts +++ b/libraries/botbuilder-lg/src/evaluator.ts @@ -73,6 +73,8 @@ export class Evaluator extends AbstractParseTreeVisitor implements LGTempla * @returns Evaluate result. */ public evaluateTemplate(inputTemplateName: string, scope: any): any { + + const memory = scope instanceof CustomizedMemory ? scope : new CustomizedMemory(scope); let templateName: string; let reExecute: boolean; ({reExecute, pureTemplateName: templateName} = this.parseTemplateName(inputTemplateName)); @@ -81,19 +83,15 @@ export class Evaluator extends AbstractParseTreeVisitor implements LGTempla throw new Error(TemplateErrors.templateNotExist(templateName)); } - if (this.evaluationTargetStack.some((u: EvaluationTarget): boolean => u.templateName === templateName)) { + const templateTarget: EvaluationTarget = new EvaluationTarget(templateName, memory); + const currentEvulateId: string = templateTarget.getId(); + + if (this.evaluationTargetStack.some((u: EvaluationTarget): boolean => u.getId() === currentEvulateId)) { throw new Error(`${ TemplateErrors.loopDetected } ${ this.evaluationTargetStack.reverse() .map((u: EvaluationTarget): string => u.templateName) .join(' => ') }`); } - if (!(scope instanceof CustomizedMemory)) { - scope = new CustomizedMemory(SimpleObjectMemory.wrap(scope)); - } - - const templateTarget: EvaluationTarget = new EvaluationTarget(templateName, scope); - const currentEvulateId: string = templateTarget.getId(); - let previousEvaluateTarget: EvaluationTarget; if (this.evaluationTargetStack.length !== 0) { diff --git a/libraries/botbuilder-lg/src/expander.ts b/libraries/botbuilder-lg/src/expander.ts index 65e263e52..d1be4c18c 100644 --- a/libraries/botbuilder-lg/src/expander.ts +++ b/libraries/botbuilder-lg/src/expander.ts @@ -64,22 +64,22 @@ export class Expander extends AbstractParseTreeVisitor implements LGTe * @returns All possiable results. */ public expandTemplate(templateName: string, scope: any): any[] { + const memory = scope instanceof CustomizedMemory ? scope : new CustomizedMemory(scope); if (!(templateName in this.templateMap)) { throw new Error(TemplateErrors.templateNotExist(templateName)); } - if (this.evaluationTargetStack.find((u: EvaluationTarget): boolean => u.templateName === templateName)) { + const templateTarget: EvaluationTarget = new EvaluationTarget(templateName, memory); + const currentEvulateId: string = templateTarget.getId(); + + if (this.evaluationTargetStack.find((u: EvaluationTarget): boolean => u.getId() === currentEvulateId)) { throw new Error(`${ TemplateErrors.loopDetected } ${ this.evaluationTargetStack.reverse() .map((u: EvaluationTarget): string => u.templateName) .join(' => ') }`); } - if (!(scope instanceof CustomizedMemory)) { - scope = new CustomizedMemory(SimpleObjectMemory.wrap(scope)); - } - // Using a stack to track the evalution trace - this.evaluationTargetStack.push(new EvaluationTarget(templateName, scope)); + this.evaluationTargetStack.push(templateTarget); const result: any[] = this.visit(this.templateMap[templateName].templateBodyParseTree); this.evaluationTargetStack.pop(); diff --git a/libraries/botbuilder-lg/tests/lg.test.js b/libraries/botbuilder-lg/tests/lg.test.js index 8cd191f92..57428bdfd 100644 --- a/libraries/botbuilder-lg/tests/lg.test.js +++ b/libraries/botbuilder-lg/tests/lg.test.js @@ -645,6 +645,17 @@ describe('LG', function() { assert.strictEqual(evaled, 18, `Evaled is ${ evaled }`); }); + it('TestRecursiveTemplate', function() { + var templates = Templates.parseFile(GetExampleFilePath('RecursiveTemplate.lg')); + var evaled = templates.evaluate('RecursiveAccumulate', {number: 10}); + assert.strictEqual(evaled, 55); + + var evaled = templates.evaluate('RecursiveFactorial', {number: 5}); + assert.strictEqual(evaled, 1*2*3*4*5); + + var evaled = templates.evaluate('RecursiveFibonacciSequence', {number: 5}); + assert.strictEqual(evaled, 5); + }); it('TestLGResource', function() { var templates = Templates.parseText(fs.readFileSync(GetExampleFilePath('2.lg'), 'utf-8')); diff --git a/libraries/botbuilder-lg/tests/lgDiagnostic.test.js b/libraries/botbuilder-lg/tests/lgDiagnostic.test.js index 23dfdcfe7..b728eae56 100644 --- a/libraries/botbuilder-lg/tests/lgDiagnostic.test.js +++ b/libraries/botbuilder-lg/tests/lgDiagnostic.test.js @@ -214,6 +214,8 @@ describe(`LGExceptionTest`, function() { assert.throws(() => templates.evaluate(`wPhrase`), Error(`Loop detected: welcome_user => wPhrase [wPhrase] Error occurred when evaluating '-\${wPhrase()}'. [welcome_user] Error occurred when evaluating '-\${welcome_user()}'.`)); assert.throws(() => templates.analyzeTemplate(`wPhrase`), Error('Loop detected: welcome_user => wPhrase'),); + + assert.throws(() => templates.analyzeTemplate(`shouldFail`), Error('Loop detected: shouldFail'),); }); it(`AddTextWithWrongId`, function() { diff --git a/libraries/botbuilder-lg/tests/testData/examples/RecursiveTemplate.lg b/libraries/botbuilder-lg/tests/testData/examples/RecursiveTemplate.lg new file mode 100644 index 000000000..38b2b6899 --- /dev/null +++ b/libraries/botbuilder-lg/tests/testData/examples/RecursiveTemplate.lg @@ -0,0 +1,20 @@ +> 1 + 2 + 3 +...+ number +# RecursiveAccumulate(number) +- IF:${number <= 1} + - ${1} +- ELSE: + - ${RecursiveAccumulate(number - 1) + number} + +> 1 * 2 * 3 * 4 * 5 * ... * number +# RecursiveFactorial(number) +- IF: ${number <= 1} + - ${1} +- ELSE: + - ${RecursiveFactorial(number - 1) * number} + +> 1, 1, 2, 3, 5, 8, 13 ... number th +# RecursiveFibonacciSequence(number) +- IF: ${number <= 2} + - ${1} +- ELSE: + - ${RecursiveFibonacciSequence(number - 1) + RecursiveFibonacciSequence(number - 2)} \ No newline at end of file diff --git a/libraries/botbuilder-lg/tests/testData/exceptionExamples/LoopDetected.lg b/libraries/botbuilder-lg/tests/testData/exceptionExamples/LoopDetected.lg index 60c870010..b13c9abd7 100644 --- a/libraries/botbuilder-lg/tests/testData/exceptionExamples/LoopDetected.lg +++ b/libraries/botbuilder-lg/tests/testData/exceptionExamples/LoopDetected.lg @@ -8,3 +8,6 @@ # welcome_user - ${wPhrase()} +> self loop +# shouldFail(x) +- ${shouldFail(x)} \ No newline at end of file