[DYN-2655]: Fix crash for badly formatted xml docs on ZT node (#10674)

aparajit-pratap · web-flow · commit e38910896707 · 2020-05-21T13:33:36.000-04:00
* fix crash for badly formatted xml docs on ZT node

* correct inline comments

* log missing xml tags to Dynamo console

* add string resource
diff --git a/src/DynamoCore/Library/XmlDocumentationExtensions.cs b/src/DynamoCore/Library/XmlDocumentationExtensions.cs
@@ -7,6 +7,7 @@
 using System.Xml;
 using Dynamo.Interfaces;
 using Dynamo.Library;
+using Dynamo.Logging;
 
 namespace Dynamo.Engine
 {
@@ -20,6 +21,8 @@ public static class XmlDocumentationExtensions
 
         #region Public methods
 
+        public static event Action<string> LogToConsole;
+
         /// <summary>
         /// Returns a description of a parameter from the its documentation xml,
         /// using the corresponding FunctionDescriptor.
@@ -297,6 +300,11 @@ private enum DocumentElementType
             SearchTagWeights
         }
 
+        private static void OnMissingXmlTags(string warning)
+        {
+            LogToConsole?.Invoke(warning);
+        }
+
         private static void LoadDataFromXml(XmlReader reader, string assemblyName)
         {
             if (reader == null)
@@ -369,8 +377,19 @@ private static void LoadDataFromXml(XmlReader reader, string assemblyName)
                             case XmlTagType.Summary:
                                 currentDocNode.Summary = reader.Value.CleanUpDocString();
                                 break;
-                            case XmlTagType.Parameter:
-                                currentDocNode.Parameters.Add(currentParamName, reader.Value.CleanUpDocString());
+                            case XmlTagType.Parameter: 
+                                // If a tag is missing around text after <params> tag, the text can be added as a new parameter
+                                // under the previous parameter name. This check avoids the resulting ArgumentException with the dictionary.
+                                if (!currentDocNode.Parameters.ContainsKey(currentParamName))
+                                {
+                                    currentDocNode.Parameters.Add(currentParamName, reader.Value.CleanUpDocString());
+                                }
+                                else
+                                {
+                                    OnMissingXmlTags(
+                                        $"{currentDocNode.FullyQualifiedName} {Properties.Resources.MissingXmlTagConsoleMessage}");
+                                }
+
                                 break;
                             case XmlTagType.Returns:
                                 currentDocNode.Returns.Add(new Tuple<string,string>(currentParamName, reader.Value.CleanUpDocString()));
diff --git a/src/DynamoCore/Logging/DynamoLogger.cs b/src/DynamoCore/Logging/DynamoLogger.cs
@@ -4,6 +4,7 @@
 using System.Text;
 using Dynamo.Configuration;
 using Dynamo.Core;
+using Dynamo.Engine;
 using Dynamo.Exceptions;
 
 namespace Dynamo.Logging
@@ -188,6 +189,8 @@ public DynamoLogger(DebugSettings debugSettings, string logDirectory, Boolean is
                 {
                     StartLoggingToConsoleAndFile(logDirectory);
                 }
+
+                XmlDocumentationExtensions.LogToConsole += Log;
             }
         }
 
@@ -448,6 +451,8 @@ internal void Dispose(bool isDisposed)
 
             if (ConsoleWriter != null)
                 ConsoleWriter = null;
+
+            XmlDocumentationExtensions.LogToConsole -= Log;
         }
 
         /// <summary>
diff --git a/src/DynamoCore/Properties/Resources.Designer.cs b/src/DynamoCore/Properties/Resources.Designer.cs
diff --git a/src/DynamoCore/Properties/Resources.en-US.resx b/src/DynamoCore/Properties/Resources.en-US.resx
@@ -712,4 +712,7 @@ Restart Dynamo to complete the uninstall.</value>
   <data name="Preview3DOutageTitle" xml:space="preserve">
     <value>3D preview has been deactivated</value>
   </data>
+  <data name="MissingXmlTagConsoleMessage" xml:space="preserve">
+    <value>{0} is missing some XML documentation tags.</value>
+  </data>
 </root>
diff --git a/src/DynamoCore/Properties/Resources.resx b/src/DynamoCore/Properties/Resources.resx
@@ -712,4 +712,7 @@ Restart Dynamo to complete the uninstall.</value>
   <data name="Preview3DOutageTitle" xml:space="preserve">
     <value>3D preview has been deactivated</value>
   </data>
+  <data name="MissingXmlTagConsoleMessage" xml:space="preserve">
+    <value>{0} is missing some XML documentation tags.</value>
+  </data>
 </root>
diff --git a/test/DynamoCoreTests/XmlDocumentationTests.cs b/test/DynamoCoreTests/XmlDocumentationTests.cs
@@ -41,7 +41,37 @@ Constructor summary.
 </doc>
 "));
 
-        private FunctionDescriptor GetMyMethod()
+        private static XmlReader BadFormatSamples = XmlReader.Create(new StringReader(
+            @"
+<doc>          
+    <members>
+        <member name=""M:MyNamespace.MyClass.MyBadlyDocumentedMethod1(System.Double,System.Double,System.Double)"">
+            <summary>
+            My method bla bla bla bla summary.
+            </summary>
+            <param name=""a"">Double a.</param>
+            <param name=""b"">Double b.</param>
+            <param name=""c"">Double c.</param>
+            <search>
+            move,push
+            </search>
+            <weights>0.4,0.2</weights>
+            Missing returns tag.
+        </member>
+        <member name=""M:MyNamespace.MyClass.MyBadlyDocumentedMethod2(System.Double,System.Double,System.Double)"">
+            <summary>
+            My method bla bla bla bla summary.
+            </summary>
+            <param name=""a"">Double a.</param>
+            <param name=""b"">Double b.</param>
+            <param name=""c"">Double c.</param>
+            Missing tag.
+        </member>
+    </members>
+</doc>
+"));
+
+        private FunctionDescriptor GetMyMethod(string methodName = "MyMethod")
         {
             var parms = new List<TypedParameter>()
             {
@@ -54,7 +84,7 @@ private FunctionDescriptor GetMyMethod()
             {
                 Assembly = "MyAssembly.dll",
                 ClassName = "MyNamespace.MyClass",
-                FunctionName = "MyMethod",
+                FunctionName = methodName,
                 Parameters = parms,
                 ReturnType = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var),
                 FunctionType = FunctionType.InstanceMethod
@@ -137,6 +167,32 @@ public void GetSummary_FromConstructor()
             Assert.AreEqual("Constructor summary.", summary);
         }
 
+        [Test]
+        [Category("UnitTests")]
+        public void GetSummary_FromBadlyDocumentedMethod()
+        {
+            var method = GetMyMethod("MyBadlyDocumentedMethod1");
+
+            Assert.DoesNotThrow(() =>
+            {
+                var searchTags = method.GetSearchTags(BadFormatSamples).ToList();
+
+                Assert.AreEqual(2, searchTags.Count);
+                Assert.AreEqual("move", searchTags[0]);
+                Assert.AreEqual("push", searchTags[1]);
+            });
+
+            method = GetMyMethod("MyBadlyDocumentedMethod2");
+
+            Assert.DoesNotThrow(() =>
+            {
+                var summary = method.GetSummary(BadFormatSamples);
+
+                Assert.AreEqual("My method bla bla bla bla summary.", summary);
+            });
+
+        }
+
         [Test]
         [Category("UnitTests")]
         public void GetReturnKeys_FromMyMethod()

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`using System.Text;`
`5`	`5`	`using Dynamo.Configuration;`
`6`	`6`	`using Dynamo.Core;`
	`7`	`+using Dynamo.Engine;`
`7`	`8`	`using Dynamo.Exceptions;`
`8`	`9`
`9`	`10`	`namespace Dynamo.Logging`
`@@ -188,6 +189,8 @@ public DynamoLogger(DebugSettings debugSettings, string logDirectory, Boolean is`
`188`	`189`	`{`
`189`	`190`	`StartLoggingToConsoleAndFile(logDirectory);`
`190`	`191`	`}`
	`192`	`+`
	`193`	`+ XmlDocumentationExtensions.LogToConsole += Log;`
`191`	`194`	`}`
`192`	`195`	`}`
`193`	`196`
`@@ -448,6 +451,8 @@ internal void Dispose(bool isDisposed)`
`448`	`451`
`449`	`452`	`if (ConsoleWriter != null)`
`450`	`453`	`ConsoleWriter = null;`
	`454`	`+`
	`455`	`+ XmlDocumentationExtensions.LogToConsole -= Log;`
`451`	`456`	`}`
`452`	`457`
`453`	`458`	`/// <summary>`