[Templating] Expose warnings/errors to callers (#7437)

* In progress: log warnings * Error log for .NET * Add error logging for NodeJS * Resolve comments on PR * Consolidate ArrayList declarations and fix testing nuget package * Rearrange Expand methods and add comments * Refactor to add the errors to template instance * Update warning message * Get warnings from last template expansion * Add markdown file changes
microsoft · May 12, 2022 · 71bfab3 · 71bfab3
1 parent 2e213f4
commit 71bfab3
Show file tree

Hide file tree

Showing 4 changed files with 155 additions and 18 deletions.
diff --git a/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardTemplate.cs b/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardTemplate.cs
@@ -4,6 +4,7 @@
 using Antlr4.Runtime.Tree;
 using Newtonsoft.Json;
 using System;
+using System.Collections;
 
 namespace AdaptiveCards.Templating
 {
@@ -21,6 +22,7 @@ public sealed class AdaptiveCardTemplate
     {
         private IParseTree parseTree;
         private string jsonTemplateString;
+        private ArrayList templateExpansionWarnings;
 
         /// <summary>
         /// <para>Creates an instance of AdaptiveCardTemplate</para>
@@ -109,7 +111,11 @@ public string Expand(EvaluationContext context, Func<string, object> nullSubstit
             }
 
             AdaptiveCardsTemplateVisitor eval = new AdaptiveCardsTemplateVisitor(nullSubstitutionOption, jsonData);
-            return eval.Visit(parseTree).ToString();
+            AdaptiveCardsTemplateResult result = eval.Visit(parseTree);
+
+            templateExpansionWarnings = eval.getTemplateVisitorWarnings();
+
+            return result.ToString();
         }
 
         /// <summary>
@@ -135,8 +141,20 @@ public string Expand(EvaluationContext context, Func<string, object> nullSubstit
         public string Expand(object rootData, Func<string, object> nullSubstitutionOption = null)
         {
             var context = new EvaluationContext(rootData);
-
             return Expand(context, nullSubstitutionOption);
         }
+
+        /// <summary>
+        /// Getter method for the array of warning strings from the last template expansion
+        /// </summary>
+        /// <returns>ArrayList</returns>
+        public ArrayList GetLastTemplateExpansionWarnings()
+        {
+            if (templateExpansionWarnings != null)
+            {
+                return templateExpansionWarnings;
+            }
+            return new ArrayList();
+        }
     }
 }
diff --git a/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardsTemplateVisitor.cs b/source/dotnet/Library/AdaptiveCards.Templating/AdaptiveCardsTemplateVisitor.cs
@@ -8,6 +8,7 @@
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
 using System.Text;
@@ -23,6 +24,7 @@ public sealed class AdaptiveCardsTemplateVisitor : AdaptiveCardsTemplateParserBa
         private Stack<DataContext> dataContext = new Stack<DataContext>();
         private readonly JToken root;
         private readonly Options options;
+        private ArrayList templateVisitorWarnings;
 
         /// <summary>
         /// maintains data context
@@ -126,6 +128,8 @@ public AdaptiveCardsTemplateVisitor(Func<string, object> nullSubstitutionOption,
             {
                 NullSubstitution = nullSubstitutionOption != null? nullSubstitutionOption : (path) => $"${{{path}}}"
             };
+
+            templateVisitorWarnings = new ArrayList();
         }
 
         /// <summary>
@@ -198,6 +202,15 @@ private bool HasDataContext()
             return dataContext.Count != 0;
         }
 
+        /// <summary>
+        /// Getter for templateVisitorWarnings
+        /// </summary>
+        /// <returns>ArrayList</returns>
+        public ArrayList getTemplateVisitorWarnings()
+        {
+            return templateVisitorWarnings;
+        }
+
         /// <summary>
         /// antlr runtime wil call this method when parse tree's context is <see cref="AdaptiveCardsTemplateParser.TemplateDataContext"/>
         /// <para>It is used in parsing a pair that has $data as key</para>
@@ -336,8 +349,20 @@ public override AdaptiveCardsTemplateResult VisitValueTemplateExpression([NotNul
                 return result;
             }
 
+            bool isTrue = false;
+
+            try
+            {
+                isTrue = IsTrue(result.Predicate, dataContext.token);
+            }
+            catch (System.FormatException)
+            {
+                templateVisitorWarnings.Add($"WARN: Could not evaluate {result.Predicate} because it could not be found in the provided data. " +
+                                    "The condition has been set to false by default.");
+            }
+
             // evaluate $when
-            result.WhenEvaluationResult = IsTrue(result.Predicate, dataContext.token) ?
+            result.WhenEvaluationResult = isTrue ?
                 AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToTrue :
                 AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToFalse;
 
@@ -499,8 +524,8 @@ public override AdaptiveCardsTemplateResult VisitObj([NotNull] AdaptiveCardsTemp
                             {
                                 // The when expression could not be evaluated, so we are defaulting the value to false
                                 whenEvaluationResult = AdaptiveCardsTemplateResult.EvaluationResult.EvaluatedToFalse;
-                                // TODO: Expose this warning to caller - documented in issue #7433
-                                Console.WriteLine($"WARN: Could not evaluate {returnedResult} because it is not an expression or the " +
+
+                                templateVisitorWarnings.Add($"WARN: Could not evaluate {returnedResult} because it is not an expression or the " +
                                     $"expression is invalid. The $when condition has been set to false by default.");
 
                             }
@@ -749,18 +774,7 @@ public static bool IsTrue(string predicate, JToken data)
             var (value, error) = new ValueExpression(Regex.Unescape(predicate)).TryGetValue(data);
             if (error == null)
             {
-                try
-                {
-                    return bool.Parse(value as string);
-                }
-                catch (System.FormatException)
-                {
-                    // If the expression didn't evaluate to a boolean, we need to return false
-                    // TODO: Expose this warning to caller - documented in issue #7433
-                    Console.WriteLine($"WARN: Could not evaluate boolean expression because it could not be found in the provided data. " +
-                                    "The condition has been set to false by default.");
-                    return false;
-                }
+                return bool.Parse(value as string);
             }
             return true;
         }

diff --git a/source/dotnet/Library/AdaptiveCards.Templating/docs/AdaptiveCardsTemplate.md b/source/dotnet/Library/AdaptiveCards.Templating/docs/AdaptiveCardsTemplate.md
@@ -7,6 +7,7 @@
   - [#ctor(jsonTemplate)](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-#ctor-System-Object- 'AdaptiveCards.Templating.AdaptiveCardTemplate.#ctor(System.Object)')
   - [Expand(context,nullSubstitutionOption)](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-Expand-AdaptiveCards-Templating-EvaluationContext,System-Func{System-String,System-Object}- 'AdaptiveCards.Templating.AdaptiveCardTemplate.Expand(AdaptiveCards.Templating.EvaluationContext,System.Func{System.String,System.Object})')
   - [Expand(rootData,nullSubstitutionOption)](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-Expand-System-Object,System-Func{System-String,System-Object}- 'AdaptiveCards.Templating.AdaptiveCardTemplate.Expand(System.Object,System.Func{System.String,System.Object})')
+  - [GetLastTemplateExpansionWarnings()](#M-AdaptiveCards-Templating-AdaptiveCardTemplate-GetLastTemplateExpansionWarnings 'AdaptiveCards.Templating.AdaptiveCardTemplate.GetLastTemplateExpansionWarnings')
 - [AdaptiveCardsTemplateParserBaseVisitor\`1](#T-AdaptiveCardsTemplateParserBaseVisitor`1 'AdaptiveCardsTemplateParserBaseVisitor`1')
   - [VisitArray(context)](#M-AdaptiveCardsTemplateParserBaseVisitor`1-VisitArray-AdaptiveCardsTemplateParser-ArrayContext- 'AdaptiveCardsTemplateParserBaseVisitor`1.VisitArray(AdaptiveCardsTemplateParser.ArrayContext)')
   - [VisitJson(context)](#M-AdaptiveCardsTemplateParserBaseVisitor`1-VisitJson-AdaptiveCardsTemplateParser-JsonContext- 'AdaptiveCardsTemplateParserBaseVisitor`1.VisitJson(AdaptiveCardsTemplateParser.JsonContext)')
@@ -65,6 +66,7 @@
   - [VisitValueObject(context)](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-VisitValueObject-AdaptiveCardsTemplateParser-ValueObjectContext- 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.VisitValueObject(AdaptiveCardsTemplateParser.ValueObjectContext)')
   - [VisitValueTemplateExpression(context)](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-VisitValueTemplateExpression-AdaptiveCardsTemplateParser-ValueTemplateExpressionContext- 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.VisitValueTemplateExpression(AdaptiveCardsTemplateParser.ValueTemplateExpressionContext)')
   - [VisitValueTemplateString(context)](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-VisitValueTemplateString-AdaptiveCardsTemplateParser-ValueTemplateStringContext- 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.VisitValueTemplateString(AdaptiveCardsTemplateParser.ValueTemplateStringContext)')
+  - [getTemplateVisitorWarnings()](#M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-getTemplateVisitorWarnings 'AdaptiveCards.Templating.AdaptiveCardsTemplateVisitor.getTemplateVisitorWarnings')
 - [AdaptiveTemplateException](#T-AdaptiveCards-Templating-AdaptiveTemplateException 'AdaptiveCards.Templating.AdaptiveTemplateException')
   - [#ctor()](#M-AdaptiveCards-Templating-AdaptiveTemplateException-#ctor 'AdaptiveCards.Templating.AdaptiveTemplateException.#ctor')
   - [#ctor(message)](#M-AdaptiveCards-Templating-AdaptiveTemplateException-#ctor-System-String- 'AdaptiveCards.Templating.AdaptiveTemplateException.#ctor(System.String)')
@@ -239,6 +241,21 @@ Default behavior is leaving templated string unchanged
 
 - [AdaptiveCards.Templating.EvaluationContext](#T-AdaptiveCards-Templating-EvaluationContext 'AdaptiveCards.Templating.EvaluationContext')
 
+<a name='M-AdaptiveCards-Templating-AdaptiveCardTemplate-GetLastTemplateExpansionWarnings'></a>
+### GetLastTemplateExpansionWarnings() `method`
+
+##### Summary
+
+Getter method for the array of warning strings from the last template expansion
+
+##### Returns
+
+ArrayList
+
+##### Parameters
+
+This method has no parameters.
+
 <a name='T-AdaptiveCardsTemplateParserBaseVisitor`1'></a>
 ## AdaptiveCardsTemplateParserBaseVisitor\`1 `type`
 
@@ -1131,6 +1148,21 @@ Visitor method for `valueTemplateString` grammar rule `AdaptiveCardsTemplatePars
 | ---- | ---- | ----------- |
 | context | [AdaptiveCardsTemplateParser.ValueTemplateStringContext](#T-AdaptiveCardsTemplateParser-ValueTemplateStringContext 'AdaptiveCardsTemplateParser.ValueTemplateStringContext') |  |
 
+<a name='M-AdaptiveCards-Templating-AdaptiveCardsTemplateVisitor-getTemplateVisitorWarnings'></a>
+### getTemplateVisitorWarnings() `method`
+
+##### Summary
+
+Getter for templateVisitorWarnings
+
+##### Returns
+
+ArrayList
+
+##### Parameters
+
+This method has no parameters.
+
 <a name='T-AdaptiveCards-Templating-AdaptiveTemplateException'></a>
 ## AdaptiveTemplateException `type`
 

diff --git a/source/dotnet/Test/AdaptiveCards.Templating.Test/TestTransform.cs b/source/dotnet/Test/AdaptiveCards.Templating.Test/TestTransform.cs
@@ -5,7 +5,7 @@
 using System.Diagnostics;
 using System;
 using AdaptiveExpressions.Memory;
-using System.Collections.Generic;
+using System.Collections;
 
 namespace AdaptiveCards.Templating.Test
 {
@@ -13269,6 +13269,79 @@ public void TestWhenExpressionNotInData()
 
             Assert.AreEqual(expectedJson, st);
         }
+
+        [TestMethod]
+        public void TestWhenNotExpressionWithLog()
+        {
+            string cardJson = "{\"type\": \"AdaptiveCard\", \"body\": [{\"type\": \"TextBlock\"," +
+                "\"size\": \"Medium\", \"weight\": \"Bolder\", \"text\": \"${title}\", \"$when\": \"notAnExpression\"}]," +
+                "\"$schema\": \"http://adaptivecards.io/schemas/adaptive-card.json\",\"version\": \"1.5\"}";
+
+            string expectedJson = "{\"type\":\"AdaptiveCard\",\"body\":[]," +
+                "\"$schema\":\"http://adaptivecards.io/schemas/adaptive-card.json\",\"version\":\"1.5\"}";
+
+            var context = new EvaluationContext();
+            var template = new AdaptiveCardTemplate(cardJson);
+            string st = template.Expand(context);
+
+            Assert.AreEqual(expectedJson, st);
+
+            ArrayList log = template.GetLastTemplateExpansionWarnings();
+            string expectedWarning = "WARN: Could not evaluate \"notAnExpression\" because it is not " +
+                "an expression or the expression is invalid. The $when condition has been set to false by default.";
+
+            Assert.AreEqual(expectedWarning, log[0]);
+        }
+
+        [TestMethod]
+        public void TestWhenInvalidExpressionNoDataWithLog()
+        {
+            string cardJson = "{\"type\": \"AdaptiveCard\", \"body\": [{\"type\": \"TextBlock\"," +
+                "\"size\": \"Medium\", \"weight\": \"Bolder\", \"text\": \"${title}\", \"$when\": \"${invalidExpression}\"}]," +
+                "\"$schema\": \"http://adaptivecards.io/schemas/adaptive-card.json\",\"version\": \"1.5\"}";
+
+            string expectedJson = "{\"type\":\"AdaptiveCard\",\"body\":[]," +
+                "\"$schema\":\"http://adaptivecards.io/schemas/adaptive-card.json\",\"version\":\"1.5\"}";
+
+            var context = new EvaluationContext();
+            var template = new AdaptiveCardTemplate(cardJson);
+            string st = template.Expand(context);
+
+            Assert.AreEqual(expectedJson, st);
+
+            ArrayList log = template.GetLastTemplateExpansionWarnings();
+            string expectedWarning = "WARN: Could not evaluate \"${invalidExpression}\" because it is not " +
+                "an expression or the expression is invalid. The $when condition has been set to false by default.";
+
+            Assert.AreEqual(expectedWarning, log[0]);
+        }
+
+        [TestMethod]
+        public void TestWhenExpressionNotInDataWithLog()
+        {
+            string cardJson = "{\"type\": \"AdaptiveCard\", \"body\": [{\"type\": \"TextBlock\"," +
+                "\"size\": \"Medium\", \"weight\": \"Bolder\", \"text\": \"${title}\", \"$when\": \"${notInData}\"}]," +
+                "\"$schema\": \"http://adaptivecards.io/schemas/adaptive-card.json\",\"version\": \"1.5\"}";
+
+            string expectedJson = "{\"type\":\"AdaptiveCard\",\"body\":[]," +
+                "\"$schema\":\"http://adaptivecards.io/schemas/adaptive-card.json\",\"version\":\"1.5\"}";
+
+            Data dt = new Data()
+            {
+                title = ""
+            };
+
+            var template = new AdaptiveCardTemplate(cardJson);
+            string st = template.Expand(dt);
+
+            Assert.AreEqual(expectedJson, st);
+
+            ArrayList log = template.GetLastTemplateExpansionWarnings();
+            string expectedWarning = "WARN: Could not evaluate ${notInData} " +
+                "because it could not be found in the provided data. The condition has been set to false by default.";
+
+            Assert.AreEqual(expectedWarning, log[0]);
+        }
     }
     [TestClass]
     public sealed class TestRootKeyword