diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/EvaluationTarget.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/EvaluationTarget.cs index 2749158582..383075fefd 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/EvaluationTarget.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/EvaluationTarget.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Collections.Generic; +using System.Linq; using AdaptiveExpressions.Memory; namespace Microsoft.Bot.Builder.LanguageGeneration @@ -46,14 +48,40 @@ public EvaluationTarget(string templateName, IMemory scope) /// public IMemory Scope { get; set; } + /// Throws an exception if any of the specified targets equals the specified id such that a loop is detected. + /// The targets to compare. + /// The id against which the targets should be compared. + /// The template name to include in any resulting exception. + public static void ThrowIfLoopDetected( + Stack targets, + (string TemplateName, string ScopeVersion) id, + string templateName) + { + foreach (var target in targets) + { + if (target.TemplateName == id.TemplateName && target.Scope?.Version() == id.ScopeVersion) + { + throw new InvalidOperationException($"{TemplateErrors.LoopDetected} {string.Join(" => ", targets.Reverse().Select(e => e.TemplateName))} => {templateName}"); + } + } + } + + /// Combines the components of the specified to create a string key. + /// The id retrieved from . + /// The created string key. + public static string CreateKey((string TemplateName, string ScopeVersion) id) + { + return id.TemplateName + id.ScopeVersion; + } + /// /// Get current instance id. If two target has the same Id, /// we can say they have the same template evaluation. /// /// Id. - public string GetId() + public (string TemplateName, string ScopeVersion) GetId() { - return TemplateName + Scope?.Version(); + return (TemplateName, Scope?.Version()); } } } diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/Evaluator.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/Evaluator.cs index 7f10550707..55b65adc52 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/Evaluator.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/Evaluator.cs @@ -108,20 +108,18 @@ public object EvaluateTemplate(string inputTemplateName, object scope) var templateTarget = new EvaluationTarget(templateName, memory); var currentEvaluateId = templateTarget.GetId(); - if (_evaluationTargetStack.Any(e => e.GetId() == currentEvaluateId)) - { - throw new InvalidOperationException($"{TemplateErrors.LoopDetected} {string.Join(" => ", _evaluationTargetStack.Reverse().Select(e => e.TemplateName))} => {templateName}"); - } + EvaluationTarget.ThrowIfLoopDetected(_evaluationTargetStack, currentEvaluateId, templateName); + string key = null; object result = null; var hasResult = false; if (!reExecute) { if (_lgOptions.CacheScope == LGCacheScope.Global) { - if (_cachedResult.ContainsKey(currentEvaluateId)) + key ??= EvaluationTarget.CreateKey(currentEvaluateId); + if (_cachedResult.TryGetValue(key, out result)) { - result = _cachedResult[currentEvaluateId]; hasResult = true; } } @@ -132,9 +130,9 @@ public object EvaluateTemplate(string inputTemplateName, object scope) { previousEvaluateTarget = CurrentTarget(); - if (previousEvaluateTarget.CachedEvaluatedChildren.ContainsKey(currentEvaluateId)) + key ??= EvaluationTarget.CreateKey(currentEvaluateId); + if (previousEvaluateTarget.CachedEvaluatedChildren.TryGetValue(key, out result)) { - result = previousEvaluateTarget.CachedEvaluatedChildren[currentEvaluateId]; hasResult = true; } } @@ -162,13 +160,15 @@ public object EvaluateTemplate(string inputTemplateName, object scope) { if (_lgOptions.CacheScope == LGCacheScope.Global) { - _cachedResult[currentEvaluateId] = result; + key ??= EvaluationTarget.CreateKey(currentEvaluateId); + _cachedResult[key] = result; } else if (_lgOptions.CacheScope == null || _lgOptions.CacheScope == LGCacheScope.Local) { if (_evaluationTargetStack.Count > 0) { - _evaluationTargetStack.Peek().CachedEvaluatedChildren[currentEvaluateId] = result; + key ??= EvaluationTarget.CreateKey(currentEvaluateId); + _evaluationTargetStack.Peek().CachedEvaluatedChildren[key] = result; } } } @@ -524,13 +524,14 @@ private EvaluationTarget CurrentTarget() => if (currentTemplate != null) { var source = currentTemplate.SourceRange.Source; - if (expressionContext != null && _lgOptions.OnEvent != null) + if (expressionContext != null && _lgOptions.OnEvent != null && currentTemplate.Expressions.Count != 0) { var lineOffset = currentTemplate.SourceRange.Range.Start.Line; var sourceRange = new SourceRange(expressionContext, source, lineOffset); var expressionRef = new ExpressionRef(exp, sourceRange); + var expressionRefId = expressionRef.GetId(); - var expression = currentTemplate.Expressions.FirstOrDefault(u => u.GetId() == expressionRef.GetId()); + var expression = currentTemplate.Expressions.FirstOrDefault(u => u.Equals(expressionRefId)); if (expression != null) { _lgOptions.OnEvent(expression, new BeginExpressionEvaluationArgs { Source = source, Expression = exp }); diff --git a/libraries/Microsoft.Bot.Builder.LanguageGeneration/Expander.cs b/libraries/Microsoft.Bot.Builder.LanguageGeneration/Expander.cs index 4c401068d3..73aa34033e 100644 --- a/libraries/Microsoft.Bot.Builder.LanguageGeneration/Expander.cs +++ b/libraries/Microsoft.Bot.Builder.LanguageGeneration/Expander.cs @@ -89,12 +89,7 @@ public List ExpandTemplate(string templateName, object scope) var templateTarget = new EvaluationTarget(templateName, memory); - var currentEvaluateId = templateTarget.GetId(); - - if (_evaluationTargetStack.Any(e => e.GetId() == currentEvaluateId)) - { - throw new InvalidOperationException($"{TemplateErrors.LoopDetected} {string.Join(" => ", _evaluationTargetStack.Reverse().Select(e => e.TemplateName))} => {templateName}"); - } + EvaluationTarget.ThrowIfLoopDetected(_evaluationTargetStack, templateTarget.GetId(), templateName); // Using a stack to track the evaluation trace _evaluationTargetStack.Push(templateTarget);